Change in coreboot[master]: device/pnp,util/sconfig: introduce and use `REG8` resource type

[Michael Niewöhner (Code Review)]

Michael Niewöhner 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 io registers). MSC0-MSCE (0xf0-0xfe) are generic registers, and there are even more 0x2*, 0xe* and some other ranges. So, either should we handle *all* registers or none.

I have not yet checked that part of the code exactly, but registers that are not handled as resources probably are still written somewhere - at least acer/g43t-am3 sets 0xc* registers for pnp 2.e in the dt. maybe they don't get written but noone noticed.

What do you think? What am I missing?