Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42521 )
Change subject: mb/pcengines/apu2: Switch to proper GPIO API ......................................................................
Patch Set 14:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42521/8/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/gpio_ftns.c:
https://review.coreboot.org/c/coreboot/+/42521/8/src/mainboard/pcengines/apu... PS8, Line 41: /* out the data value to prevent glitches */ If the IOMUX was already programmed for GPIO, this would need to copy GPIO_PIN_STS to GPIO_OUTPUT_VALUE. I don't think this currently does what the comment says.
https://review.coreboot.org/c/coreboot/+/42521/8/src/mainboard/pcengines/apu... PS8, Line 50: iomux_write8(gpio, iomux_ftn & 0x3); Does GPIO_PIN_STS follow the pad state/voltage even before IOMUX is set to GPIO function?
https://review.coreboot.org/c/coreboot/+/42521/8/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/romstage.c:
https://review.coreboot.org/c/coreboot/+/42521/8/src/mainboard/pcengines/apu... PS8, Line 43: gpio_output(gpio, 0);
if i saw that correctly this is the only unresolved comment in this patch, but i'd say that this isn […]
Left more unresolved comments about this sequence.
https://review.coreboot.org/c/coreboot/+/42521/14/src/mainboard/pcengines/ap... File src/mainboard/pcengines/apu2/romstage.c:
https://review.coreboot.org/c/coreboot/+/42521/14/src/mainboard/pcengines/ap... PS14, Line 19: early_lpc_init(); Not about LPC anymore.
https://review.coreboot.org/c/coreboot/+/42521/14/src/mainboard/pcengines/ap... PS14, Line 28: /* Release GPIO32/33 for other uses. */ Hmm.. early_lpc_init() already wanted 32 and 33.
From amd/thatcher/bootblock.c: /* Disable PCI-PCI bridge and release GPIO32/33 for other uses. */ pm_write8(0xea, 0x1);