Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36811 )
Change subject: soc/amd: Move SCI enable outside table creation ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36811/3/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/36811/3/src/soc/amd/picasso/acpi.c@... PS3, Line 105: =
fadt->smi_cmd = 0; /* disable system management mode */ […]
Done
https://review.coreboot.org/c/coreboot/+/36811/3/src/soc/amd/picasso/acpi.c@... PS3, Line 98: fadt->s4bios_req = 0; /* Not supported */ : fadt->pstate_cnt = 0; /* Not supported */ :
Same comment as for stoney, 3 lines setting values to 0 should be outside the if/else
Done
https://review.coreboot.org/c/coreboot/+/36811/3/src/soc/amd/stoneyridge/acp... File src/soc/amd/stoneyridge/acpi.c:
https://review.coreboot.org/c/coreboot/+/36811/3/src/soc/amd/stoneyridge/acp... PS3, Line 105: fadt->smi_cmd = 0; /* disable system management mode */ : fadt->acpi_enable = 0; /* unused if SMI_CMD = 0 */ :
These 3 line: […]
Done
https://review.coreboot.org/c/coreboot/+/36811/3/src/soc/amd/stoneyridge/acp... PS3, Line 81: memset((void *)fadt, 0, sizeof(acpi_fadt_t));
I would say only code clarity, as the whole area is 0 initialized. […]
Done
https://review.coreboot.org/c/coreboot/+/36811/3/src/soc/amd/stoneyridge/acp... PS3, Line 100: fadt->s4bios_req = 0; /* Not supported */
These 3 lines (setting variables to 0) were common on both sides of the if/else, so now it should be […]
Done