[coreboot-gerrit] Change in coreboot[master]: cpu/amd/pi: Split CAR setup for use in multiple stages

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/18437 )

Change subject: cpu/amd/pi: Split CAR setup for use in multiple stages
......................................................................


Patch Set 7:

(2 comments)

https://review.coreboot.org/#/c/18437/7/src/cpu/amd/pi/after_raminit.S
File src/cpu/amd/pi/after_raminit.S:

Line 17: 
> Since you are relying on this code to be included plus actually having this
I'll look at that.


PS7, Line 46: cache_as_ram_setup_out:
            : #ifdef __x86_64__
            : .code64
            : #endif
> Why is this here if disable_cache_as_ram is supposed to be called?
Hmm.  This was only cut/past from one file into this one.  The cache_as_ram_setup_out has been around for nearly 6 years but I can't see any reason for it.

I'm not sure whether an x86_64 build has ever worked properly.  And I'm not sure that the .code64 is necessary or just for good measure.  cache_as_ram.inc (which now includes this file) is the only crt0s item.  I assume it can probably come out.  Agree?


-- 
To view, visit https://review.coreboot.org/18437
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fe53ca1fd6b1edd4d5e072849f3962c8bc95df0
Gerrit-PatchSet: 7
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki at gmail.com>
Gerrit-Reviewer: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd at gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes