Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44534 )
Change subject: soc/amd/common: add gpio subsystem event reporting ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44534/4/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/44534/4/src/soc/amd/common/block/gp... PS4, Line 328: (1U << i)
nit: BIT(i)
Done
https://review.coreboot.org/c/coreboot/+/44534/5/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/44534/5/src/soc/amd/common/block/gp... PS5, Line 352: control_switch
Just a note: This isn't really used anywhere except for printing below. […]
I added that because the PPR isn't clear on what bit is blocking the other registers. In my mind, I assumed GpioWakeEn blocks the update of GPIOWakeStatIndex0|1. However, nothing indicates actual behavior. i.e. does it block actual wake or these register fields or both? Because of that I included it in case we needed to conditionally check GpioWakeEn. I can remove it, but I feel like the info may be helpful in the future when we discover the hardware is doing odd things. Let me know what you'd like me to do.