[coreboot-gerrit] Change in coreboot[master]: [RFC] device: Introduce new method for setting device states

[Michael Niewöhner (Code Review)]

Attention is currently required from:  Felix Singer, Nico Huber, Raul Rangel, Furquan Shaikh, Matt DeVillier, Patrick Rudolph, Felix Held.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52493 )

Change subject: [RFC] device: Introduce new method for setting device states
......................................................................

Patch Set 17:

(1 comment)

File src/soc/intel/skylake/chip.c:

https://review.coreboot.org/c/coreboot/+/52493/comment/57a19f3e_01204329 
PS13, Line 461: if (!xdci_can_enable())
              : 		params->XdciEnable = 0;
> > params->XdciEnable = xdci_can_enable(); […]

Agreed, I simply missed that we set it before. And now I wonder, if that might become a bigger problem, now that we set options in two places. Some idea would be providing an optional function in the struct above for such cases. Then we have it all in a single place:

~~~
const struct device_enable_map devmap[] = {
    { &params->XdciEnable,	PCH_DEVFN_USBOTG,	.enable_disable_fn=xdci_can_enable, },
},
~~~

The code would then do:

~~~
bool dev_en = ...... dev->enabled;

if (devmap.enable_disable_fn)
  dev_en &= devmap.enable_disable_fn();

...
~~~