Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48925 )
Change subject: device/pnp,util/sconfig: introduce and use `REG8` resource type ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48925/6/src/device/pnp_device.c File src/device/pnp_device.c:
https://review.coreboot.org/c/coreboot/+/48925/6/src/device/pnp_device.c@105 PS6, Line 105: 0x30
well, there is PNP_EN, which also is no irq, but was handled as such, see line 280
Yup, it's there and even in some devicetrees touched by this commit. I still would not like it as an example here as it's usage with `irq` (or now `reg8`) is discouraged. The register is generally controlled by pnp_set_enable() above. Providing a static value in the devicetree might not do what one would expect as the final value depends on the on/off state of individual devices and their advertisement in the superio driver.
https://review.coreboot.org/c/coreboot/+/48925/6/src/device/pnp_device.c@105 PS6, Line 105: 0xf0-0xfe
yes, thus 'e. […]
Ack
https://review.coreboot.org/c/coreboot/+/48925/6/src/device/pnp_device.c@137 PS6, Line 137: } else if (resource->flags & IORESOURCE_DRQ) { : pnp_set_drq(dev, resource->index, resource->base); : } else if (resource->flags & IORESOURCE_IRQ) { : pnp_set_irq(dev, resource->index, resource->base); : } else if (resource->flags & IORESOURCE_REG8) { : pnp_set_reg8(dev, resource->index, resource->base);
wdym with "coreboot has no rules"?
The generic code in `src/device/` does nothing about them. For I/O we have a whole allocator taking care of things, but for [DI]RQ there is nothing. So, semantically, IORESOURCE_DRQ/IRQ/REG8 seem the same to me.