Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35482 )
Change subject: superio/nuvoton/nct5104d: Add soft reset GPIO functionality ......................................................................
Patch Set 3:
Patch Set 3:
Hm, I'm unsure if this functionality is board-dependent or if it something that should be done for every case the SIO will be used in, but I'd guess the former. Also IIRC the functionality added here will overwrite devicetree settings that would be written to the same registers, which might end up being surprising and hard to debug. Haven't really looked into if everything is set up so that setting this via the devicetree would be possible here though; might be an issue that it doesn\t only use registers in the 0xf? range, but also in the 0xe? range. Maybe have a configuration option in the chip.h file to enable this? I'm not sure what would be the best option here. The SIO/devicetree integration isn't that nice and requires a bit too much boilerplate, but that's what we currently have...
This is rather SIO specific. The GPIO configuration is maintained after LPC_RESET/SIO_RESET and kept by VSB(standby power supply). The registers are reset only by RSMRST (not documented in datasheet at all..). If a user plays with GPIOs in the userspace, the GPIO states are maintained after reboot. See for reference: https://github.com/pcengines/coreboot/issues/314
Research conducted long time ago by us: https://cloud.3mdeb.com/index.php/s/4iRWE4Xfse3rD2T
I think it's not apu2 specific. The same behavior occurred on apu1 which is an entirely different processor. The datasheet of the NCT5104d can be also found in the public. I leave the judgment to you.
The devicetree integration is not so perfect because of the mixed LDN functionalities Nuvoton introduced in this chip. I also think this way: this chip will not be used in new designs so probably there are only apu1 and apu2 boards which need this mechanism. This boilerplate will be required only for these devices. But if you decide to keep it board-specific, I'm ok with that and will update the patch.