Furquan Shaikh 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 4: Code-Review+1
(4 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(...) loop, you can avoid making the callback for every index and instead just pass in table and items to get_gpio_event(gpio, table, items)
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?
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? Or should this just be a soc_force_event() or soc_override_event() that the SoC implementation can use to route the event as required i.e. make the call to route_sci() within the SoC implementation.
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?