[coreboot] [PATCH] Move "select CACHE_AS_RAM" lines from boards into CPU socket

Stefan Reinauer stepan at coreboot.org
Tue Dec 7 23:05:19 CET 2010


Hi Uwe,

good approach!

* Uwe Hermann <uwe at hermann-uwe.de> [101206 23:45]:
> Move "select CACHE_AS_RAM" lines from boards into CPU socket.

I generally like this idea, but that is not the only thing the patch
does. If it did, it were Acked-by: Stefan Reinauer <stepan at coreboot.org>

> All K8/Fam10h boards use CAR, so move the "select CACHE_AS_RAM"
> into model_fxx/Kconfig and model_10xxx/Kconfig, and remove it from the
> individual boards and the individual sockets using model_fxx/model_10xxx.
 
I think CAR should be selected in the socket, not in the model. In case
of the AMD K8 CPUs it is not a big difference, but in general, a socket
should be the entity to decide whether a given setup can run CAR or not.

The rationale is that the socket is what enforces which CPU models can
run on a board. And if there is only one model for that socket that does
not support CAR, we can't do CAR for that socket (or would need to
define a new socket for that) 

Unfortunately we can not well model the above logic in model_* Kconfig
files very nicely, so it will have keep living in the socket.

In short, please drop the following part of the patch, add select
CACHE_AS_RAM to the K8/Fam10 sockets if it's missing there instead, and
it's all fine.

> Index: src/cpu/amd/socket_940/Kconfig
> ===================================================================
> --- src/cpu/amd/socket_940/Kconfig	(Revision 6145)
> +++ src/cpu/amd/socket_940/Kconfig	(Arbeitskopie)
> @@ -7,7 +7,6 @@
>  	def_bool y
>  	select K8_HT_FREQ_1G_SUPPORT
>  	select CPU_AMD_MODEL_FXX
> -	select CACHE_AS_RAM
>  
>  config CPU_ADDR_BITS
>  	int
> Index: src/cpu/amd/model_fxx/Kconfig
> ===================================================================
> --- src/cpu/amd/model_fxx/Kconfig	(Revision 6145)
> +++ src/cpu/amd/model_fxx/Kconfig	(Arbeitskopie)
> @@ -3,6 +3,7 @@
>  	select MMX
>  	select SSE
>  	select SSE2
> +	select CACHE_AS_RAM
>  
>  if CPU_AMD_MODEL_FXX
>  config UDELAY_IO
> Index: src/cpu/amd/socket_754/Kconfig
> ===================================================================
> --- src/cpu/amd/socket_754/Kconfig	(Revision 6145)
> +++ src/cpu/amd/socket_754/Kconfig	(Arbeitskopie)
> @@ -6,7 +6,6 @@
>  config SOCKET_SPECIFIC_OPTIONS
>  	def_bool y
>  	select CPU_AMD_MODEL_FXX
> -	select CACHE_AS_RAM
>  
>  config CPU_ADDR_BITS
>  	int

In addition, this is ugly because the actual code selection for CAR
still happens in the socket_*** Makefile.inc, so if there is a good
reason for moving from socket to model (except we wouldn't have to add
a line to each socket rather than just two models, i think this is no
sufficient reason to start becoming inconsistent across CPUs), the
Makefile.inc portion should be moved, too, and live where the Kconfig
portion lives.

Best case would be to remove CACHE_AS_RAM/ROMCC from Kconfig all
together, and plug it into the Makefiles, so we have less files to look
at for this one piece of logic. (By forgetting the Makefile.inc change
you proved this point ;)

Stefan






More information about the coreboot mailing list