Piotr Kleinschmidt has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35849 )
Change subject: src/superio/nuvoton/nct5104d: assign IO port range to control GPIO ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35849/1/src/superio/nuvoton/nct5104... File src/superio/nuvoton/nct5104d/superio.c:
https://review.coreboot.org/c/coreboot/+/35849/1/src/superio/nuvoton/nct5104... PS1, Line 127: reg = pnp_read_config(dev, 0x30); : pnp_write_config(dev, 0x30, reg & 0xFE);
ah, so this is to make that setting user-configurable during runtime instead of the devicetree setti […]
Not exactly. So far, I thought if in devictree Logical Device is turned on, always Control Register 30h bit 0 is set. For LD 8 it is WDT1, but what I want to enable/disable is only CR30h bit 1 (GPIO Base Address Mode). In the comment below you explain that actually any bit can be enabled/disabled according to devictree settings. Therefore, WDT1 bit won't be affected at all, so the above functionality is not needed.
https://review.coreboot.org/c/coreboot/+/35849/1/src/superio/nuvoton/nct5104... PS1, Line 152: io_base_address = pnp_read_config(dev, 0x61); : io_base_address |= (pnp_read_config(dev, 0x60) << 8);
oh, pnp_read_iobase is only for the ENV_PNP_SIMPLE_DEVICE case and not ramstage where this code runs […]
So, as I understand correctly, that function should only check if uartc and uartd are enabled (and possibly disable GPIO Base Address Mode then) because every chip's register, defined with devictree, is already set at this stage?