[coreboot-gerrit] [XL] Change in coreboot[main]: soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC

[Name of user not set (Code Review)]

Attention is currently required from: Elyes Haouas, Felix Singer, Jérémy Compostella, Shuo Liu, Vasiliy Khoruzhick.

[e-mail] has posted comments on this change by [e-mail]. ( https://review.coreboot.org/c/coreboot/+/83321?usp=email )

Change subject: soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC
......................................................................

Patch Set 22:

(8 comments)

File src/soc/intel/snowridge/Kconfig:

https://review.coreboot.org/c/coreboot/+/83321/comment/a9f3ab8b_6b46821e?usp... :
PS15, Line 123: 	default 0x3ff00 if FSP_CAR
> Only FSP_CAR is valid, I will remove the untested one.

Done

File src/soc/intel/snowridge/acpi.c:

https://review.coreboot.org/c/coreboot/+/83321/comment/5d5f2b0e_9249e6b2?usp... :
PS15, Line 243: 		if (read32p(HPET_BASE_ADDRESS + 0x100) & 0x00008000) {
> The same code also exists in src/soc/intel/xeon_sp/uncore_acpi.c. […]

Patch link https://review.coreboot.org/c/coreboot/+/84252.

File src/soc/intel/snowridge/chip.h:

https://review.coreboot.org/c/coreboot/+/83321/comment/57893b82_d5f9056c?usp... :
PS15, Line 12:  * additional root bus in stack 2 and 7 (UBox1).
> It's Intel Dynamic Load Balancer, I will update the comments.

Done

File src/soc/intel/snowridge/chip.c:

https://review.coreboot.org/c/coreboot/+/83321/comment/e93cce7e_5685652a?usp... :
PS15, Line 75: 		if (sr[i].Personality >= BL_TYPE_DISABLED)
> For SNR, each type of stack has its own disabled enumeration value, see BL_STACK_TYPE in src/vendorc […]

Done

File src/soc/intel/snowridge/include/soc/gpio_defs.h:

https://review.coreboot.org/c/coreboot/+/83321/comment/167a6181_9215a0b2?usp... :
PS15, Line 22: #define GPIO_WEST2_PADCFGLOCKTX 0x00c4
> These sets of values are used to define struct pad_community, see src/soc/intel/snowridge/common/gpi […]

Done

File src/soc/intel/snowridge/lockdown.c:

https://review.coreboot.org/c/coreboot/+/83321/comment/b792c53f_63886b45?usp... :
PS15, Line 22: 	if (chipset_lockdown == CHIPSET_LOCKDOWN_COREBOOT)
> It's not tested, replacing it with an assert is more reasonable.

Done

File src/soc/intel/snowridge/romstage/romstage.c:

https://review.coreboot.org/c/coreboot/+/83321/comment/c3f760c1_6ce3fd93?usp... :
PS15, Line 133: 	     channel < ARRAY_SIZE(fsp_smbios_memory_info->ChannelInfo) &&
> From FSP outputs, it seems ChannelCount is equal to array size of ChannelInfo. […]

Done

https://review.coreboot.org/c/coreboot/+/83321/comment/eecde6a5_459ee689?usp... :
PS15, Line 139: 			       dimm < ARRAY_SIZE(channel_info->DimmInfo) &&
> I will add an assert for DimmCount and DimmInfo.

Done