Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38862 )
Change subject: superio/nuvoton/npcd378: Switch to superio/common ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38862/2/src/superio/nuvoton/npcd378... File src/superio/nuvoton/npcd378/superio.c:
https://review.coreboot.org/c/coreboot/+/38862/2/src/superio/nuvoton/npcd378... PS2, Line 411: if (dev->path.pnp.device == NPCD378_PWR) : npcd378_ssdt_pwr(dev); : else if (dev->path.pnp.device == NPCD378_AUX) : npcd378_ssdt_aux(dev); : else if (dev->path.pnp.device == NPCD378_KBC) : npcd378_ssdt_kbc(dev);
i'd write this as switch/case statement
Done
https://review.coreboot.org/c/coreboot/+/38862/2/src/superio/nuvoton/npcd378... PS2, Line 423: "L%02X0"
I see the acpi name has changed, but I can't picture what the result of this is. Got any examples? […]
by using the SIO ssdtgen the SuperIO LDNs appear as own device in the devicetree and in the ACPI code. Thus every LDN has it's own unique name. This driver no longer represents a single SuperIO, but the SuperIO LDNs. The parent of those devices is now superio/common, which name is SIO0. This should use superio_common_ldn_acpi_name() instead, I need to check why I had implemented it here...
https://review.coreboot.org/c/coreboot/+/38862/2/src/superio/nuvoton/npcd378... PS2, Line 435: pnp_enable
is it intended that the pnp_alt_enable is replaced by a pnp_enable?
copied from another SIO. pnp_alt_enable looks better.