Felix Held 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 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48925/8/src/device/pnp_device.c File src/device/pnp_device.c:
https://review.coreboot.org/c/coreboot/+/48925/8/src/device/pnp_device.c@286 PS8, Line 286: if (info->flags & PNP_MSC0) { : resource = new_resource(dev, PNP_IDX_MSC0); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSC1) { : resource = new_resource(dev, PNP_IDX_MSC1); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSC2) { : resource = new_resource(dev, PNP_IDX_MSC2); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSC3) { : resource = new_resource(dev, PNP_IDX_MSC3); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSC4) { : resource = new_resource(dev, PNP_IDX_MSC4); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSC5) { : resource = new_resource(dev, PNP_IDX_MSC5); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSC6) { : resource = new_resource(dev, PNP_IDX_MSC6); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSC7) { : resource = new_resource(dev, PNP_IDX_MSC7); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSC8) { : resource = new_resource(dev, PNP_IDX_MSC8); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSC9) { : resource = new_resource(dev, PNP_IDX_MSC9); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSCA) { : resource = new_resource(dev, PNP_IDX_MSCA); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSCB) { : resource = new_resource(dev, PNP_IDX_MSCB); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSCC) { : resource = new_resource(dev, PNP_IDX_MSCC); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSCD) { : resource = new_resource(dev, PNP_IDX_MSCD); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : if (info->flags & PNP_MSCE) { : resource = new_resource(dev, PNP_IDX_MSCE); : resource->size = 1; : resource->flags |= IORESOURCE_REG8; : } : }
I am starting to wonder if we really need to handle these registers as resources (in contrast to the […]
it's been a long time ago that i looked at that code, but i think those needed to be present in the super i/o driver in order for the devicetree setting to be actually applied. not too certain about that though, so please verify before making changes to that. it would probably be good if all reg8 devicetree settings will be applied without any additional code or flags in the sio driver. the MSC0-MSCE flags for the different LDNs don't feel like a proper abstraction to me and there are also plenty of registers outside of that range being used for generic configuration (0xeX range for LDN-specific registers and some in the 0x2X range for sio-specific registers like I/O mux settings)