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 3:
(2 comments)
https://review.coreboot.org/#/c/32651/3/src/soc/amd/common/block/include/amd... File src/soc/amd/common/block/include/amdblocks/gpio.h:
https://review.coreboot.org/#/c/32651/3/src/soc/amd/common/block/include/amd... PS3, Line 303: size_t soc_gpio_event_table_size(void); : int soc_override_event(uint8_t gpio, uint8_t mux, uint8_t *event); It would be helpful to add comments indicating: 1. That these functions are expected to be implemented by the SoC. 2. What the function is supposed to do.
https://review.coreboot.org/#/c/32651/3/src/soc/amd/stoneyridge/gpio.c File src/soc/amd/stoneyridge/gpio.c:
https://review.coreboot.org/#/c/32651/3/src/soc/amd/stoneyridge/gpio.c@49 PS3, Line 49: soc_gpio_event_table_size Can this be instead renamed to soc_gpio_get_event_table_details which can fill in pointer to event table as well as the size and return back to common code? gpio_event_table[] then wouldn't have to be exposed directly outside the scope of this file.