Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35849 )
Change subject: superio/nuvoton/nct5104d: assign IO port range to control GPIO ......................................................................
Patch Set 2:
(5 comments)
please split into 3 patches: one with the pnp device change, one with the devicetree changes and one with the sio changes
https://review.coreboot.org/c/coreboot/+/35849/2/src/device/pnp_device.c File src/device/pnp_device.c:
https://review.coreboot.org/c/coreboot/+/35849/2/src/device/pnp_device.c@90 PS2, Line 90: , index parameter missing; see pnp_set_iobase. without this, this would only work for the 1st io range of a ldn
https://review.coreboot.org/c/coreboot/+/35849/2/src/include/device/pnp.h File src/include/device/pnp.h:
https://review.coreboot.org/c/coreboot/+/35849/2/src/include/device/pnp.h@19 PS2, Line 19: , index parameter missing
https://review.coreboot.org/c/coreboot/+/35849/2/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35849/2/src/mainboard/pcengines/apu... PS2, Line 65: device pnp 2e.108 on don't remove "device pnp 2e.8 off end". same for the variants devicetrees
https://review.coreboot.org/c/coreboot/+/35849/2/src/superio/nuvoton/nct5104... File src/superio/nuvoton/nct5104d/superio.c:
https://review.coreboot.org/c/coreboot/+/35849/2/src/superio/nuvoton/nct5104... PS2, Line 111: static void disable_gpio_io_port(struct device *dev) this function seems to be rather board-specific to me. also why disable the io-maped gpio interface anyway? the gpio/serial port selection for the pins is via a multiplexer controlled in route_pins_to_uart via some registers that can't be written by the io-mapped gpio control
https://review.coreboot.org/c/coreboot/+/35849/2/src/superio/nuvoton/nct5104... PS2, Line 208: { NULL, NCT5104D_GPIO_IO, PNP_IO0, 0x07f8, }, { NULL, NCT5104D_GPIO_WDT}, shouldn't be removed here; { NULL, NCT5104D_GPIO_IO, PNP_IO0, 0x07f8, } should be additional to that