Felix Held 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:
(3 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_VA […]
the code you removed here might cause glitches and doesn't do what the comment says. i don't think that copying the pin input value to the output register before switching to output mode should be the way to go here; either set all bits in one atomic operation or set the output value first and then the direction to output to avoid output glitches
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?
good question. see my comment above on why it shouldn't matter
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);
Left more unresolved comments about this sequence.
this isn't needed any more after my common gpio patches. See CB:49119