Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42687 )
Change subject: soc/amd/common: Refactor GPIO SCI/SMI interrupts ......................................................................
Patch Set 8:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42687/4/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42687/4/src/soc/amd/common/block/gp... PS4, Line 73: regs->polarity &= ~mask;
It'd be helpful to add comments to handling of LEVEL vs EDGE stanzas here.
Done
https://review.coreboot.org/c/coreboot/+/42687/5/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42687/5/src/soc/amd/common/block/gp... PS5, Line 52: sci_trigger_regs
Since you are deleting the SCI_TRIGGER_X macros, you add a comment saying 0 -> Low, 1 -> High for th […]
Left as a followup exercise due the requested change of name 'level' to 'trigger' which calls for a rewrite on such added comment too.
https://review.coreboot.org/c/coreboot/+/42687/5/src/soc/amd/common/block/gp... PS5, Line 55: level
Can you name this trigger? Level makes me think Level high or Level low. […]
And have a field named trigger not being written to register named trigger? I find that equally confusing.
https://review.coreboot.org/c/coreboot/+/42687/5/src/soc/amd/common/block/gp... PS5, Line 94: SMI_SCI_LEVEL
I hate the name of these registers.
Not my choice. I have assumed these match the datasheets.