Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37490 )
Change subject: soc/amd/picasso: Add bootblock support ......................................................................
Patch Set 9:
(6 comments)
https://review.coreboot.org/c/coreboot/+/37490/9/src/soc/amd/picasso/Makefil... File src/soc/amd/picasso/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/37490/9/src/soc/amd/picasso/Makefil... PS9, Line 207: 0x10 Why the magic number?
https://review.coreboot.org/c/coreboot/+/37490/7/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/37490/7/src/soc/amd/picasso/bootblo... PS7, Line 34: sb_reset_i2c_slaves();
How is this expected to make a difference for consoles? Why is this done […]
+1. I think we can remove this here. fch_pre_init will take care of it.
We do want the pins to float: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...
https://review.coreboot.org/c/coreboot/+/37490/7/src/soc/amd/picasso/bootblo... PS7, Line 41: Family_Model
In its encoded form, it's usually called cpuid signature. You can decode […]
This was the same format that was followed on stoney: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...
https://review.coreboot.org/c/coreboot/+/37490/7/src/soc/amd/picasso/bootblo... PS7, Line 44: i2c_soc_early_init();
I'm also rather new to AMD. […]
I think sb_ and fch_ are synonyms. As for why i2c has _soc_ in the name, I don't know. The I2C block is under the FCH.
https://review.coreboot.org/c/coreboot/+/37490/9/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/37490/9/src/soc/amd/picasso/bootblo... PS9, Line 7: cpu/amd/msr.h Maybe just arch/cpu.h?
https://review.coreboot.org/c/coreboot/+/37490/9/src/soc/amd/picasso/bootblo... PS9, Line 8: #include <cpu/x86/mtrr.h> Is this one used?