Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35482 )
Change subject: superio/nuvoton/nct5104d: soft reset GPIO ......................................................................
Patch Set 2:
(2 comments)
please also split the devicetree changes into a separate patch
https://review.coreboot.org/c/coreboot/+/35482/2/src/superio/nuvoton/nct5104... File src/superio/nuvoton/nct5104d/superio.c:
https://review.coreboot.org/c/coreboot/+/35482/2/src/superio/nuvoton/nct5104... PS2, Line 23: nt gpio0_enabled = 0; : int gpio1_enabled = 0; : int gpio6_enabled = 0; this just duplicates information from the devicetree
https://review.coreboot.org/c/coreboot/+/35482/2/src/superio/nuvoton/nct5104... PS2, Line 142: : if (gpio0_enabled) : pnp_write_config(dev, NCT5104D_GPIO0_PP_OD, 0xFF); : : if (gpio1_enabled) : pnp_write_config(dev, NCT5104D_GPIO1_PP_OD, 0xFF); : : if (gpio6_enabled) : pnp_write_config(dev, NCT5104D_GPIO6_PP_OD, 0xFF); do these writes need to be conditional? if so I'd rather use the information from the devicetree