Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42567 )
Change subject: soc/amd/picasso: Populate EFS SPI values from Kconfig options ......................................................................
Patch Set 11:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42567/11/src/soc/amd/picasso/Kconfi... File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/42567/11/src/soc/amd/picasso/Kconfi... PS11, Line 379: default SPI_READ_MODE_DUAL_IO_112 if DEFAULT_SPI_READ_MODE_DUAL_IO_112 If you want the default default to be 112, leave off the "if statement". Kconfig will select the first value in the choice list below (NORMAL_33M) as the default if there's no other default selected.
https://review.coreboot.org/c/coreboot/+/42567/11/src/soc/amd/picasso/Kconfi... PS11, Line 381: numbers numbers X-Y-Z values?
https://review.coreboot.org/c/coreboot/+/42567/11/src/soc/amd/picasso/Kconfi... PS11, Line 456: SPI_MICRON_ALWAYS So I'm still not certain of the actual impact of this. You probably also want to add a way to have the board select SPI_MICRON_NONE. Not being able to directly select the choices is a definite downside to the choice menus.
https://review.coreboot.org/c/coreboot/+/42567/11/src/soc/amd/picasso/Kconfi... PS11, Line 471: default 1 if SPI_MICRON_ALWAYS
Why are these placed in amd/picasso and not amd/common? And since everything depends of the BOM noth […]
I agree, this should probably go in the src/soc/amd/common/block/spi/Kconfig file.
https://review.coreboot.org/c/coreboot/+/42567/11/src/soc/amd/picasso/Makefi... File src/soc/amd/picasso/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/42567/11/src/soc/amd/picasso/Makefi... PS11, Line 379: --soc-name "Picasso" Does this apply for Pollock and Dali as well?