Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32651 )
Change subject: soc/amd/stoneyridge: Move GPIO support to common ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/#/c/32651/4/src/soc/amd/common/block/gpio_banks/... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/#/c/32651/4/src/soc/amd/common/block/gpio_banks/... PS4, Line 33: soc_get_gpio_event_table(&table, &items);
If you put this call in program_gpios() before the for(... […]
Done
https://review.coreboot.org/#/c/32651/4/src/soc/amd/common/block/gpio_banks/... PS4, Line 53: configure_gevent_smi
Should this be a weak function or something that fails to compile if not provided by the SoC?
I don't recall my exact line of thinking now, seems reasonable to remove this.
https://review.coreboot.org/#/c/32651/4/src/soc/amd/common/block/gpio_banks/... PS4, Line 241: route_sci(event);
Is this always going to be true for all SoCs using this common code?
I decided to just call it a hook rather than predict what may come in the future. Also, it occurred to me that route_sci writes one of the registers in SMI's AcpiMmio space and doesn't really belong in this file.
https://review.coreboot.org/#/c/32651/5/src/soc/amd/common/block/include/amd... File src/soc/amd/common/block/include/amdblocks/gpio.h:
https://review.coreboot.org/#/c/32651/5/src/soc/amd/common/block/include/amd... PS5, Line 304: void soc_get_gpio_event_table(const struct soc_amd_event **table, size_t *items);
line over 80 characters
No longer an error, right? Or at this point is it easier to shorten the line?
https://review.coreboot.org/#/c/32651/4/src/soc/amd/stoneyridge/gpio.c File src/soc/amd/stoneyridge/gpio.c:
https://review.coreboot.org/#/c/32651/4/src/soc/amd/stoneyridge/gpio.c@49 PS4, Line 49: int
size_t?
Done