Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37490 )
Change subject: soc/amd/picasso: Add bootblock support ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37490/7/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/37490/7/src/soc/amd/picasso/Kconfig... PS7, Line 225: 0x807fff0
Is this architectural?
the last 16 bits are architectural; have to look into the first 16 bits, but my guess would be that they are not architectural
https://review.coreboot.org/c/coreboot/+/37490/7/src/soc/amd/picasso/Makefil... File src/soc/amd/picasso/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/37490/7/src/soc/amd/picasso/Makefil... PS7, Line 44: bootblock-y += smi_util.c
What's this?
some functions in there are used by the gpio setup function. will look into this as this indeed smells a bit weird to me
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 14: static void amd_initmmio(void)
If this is only about PCI config space, please name it accordingly. […]
it is; good catch. removed it and as expected mandolin still boots. will marks as resolved when i've pushed the new version of this patch
https://review.coreboot.org/c/coreboot/+/37490/7/src/soc/amd/picasso/bootblo... PS7, Line 44: i2c_soc_early_init();
I'm new to AMD. […]
I'm also rather new to AMD. will look into that