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

[Nico Huber (Code Review)]

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 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;
             : 	}
             : }
AFAIK, all the resources are written to registers. Why wouldn't pnp_set_*()
above be called?

This code here merely adds information beyond what is given in the
`devicetree.cb`.

For MSC* it only sets some expectations, i.e. if an sio driver says
there is MSC*, and no value is provided in the dt, we warn.
> So, either should we handle *all* registers or none.

I think the author of the sio driver should decide that.