Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46531 )
Change subject: util/sconfig: allow to override chip-wrapped devices ......................................................................
Patch Set 7:
Patch Set 7:
Patch Set 7:
Patch Set 7:
I'm not sure if we actually want to allow changing the chip. Alternatively, we could error out and fix the drivers. Some- thing like CB:41745 might help.
Well, when we turn it around (having the driver inside the device instead of wrapping the device) the problem should be gone. This is what CB:41745 does, right?
It also adds another pointer to a config struct separate from the chip's one. It wouldn't solve the problem if trees conflict, but could avoid conflicting trees in some cases.
I agree with Nico, this is, as things currently stand, just an error in whoever is writing the devicetrees. Technically, it is wrong to not have both devices wrapped with the same chip driver (sconfig assumes the hierarchy w/r/t devices & chips is the same in that respect)
Just to be sure I get this right, you want to 1) have the devices wrapped by drivers in the chipset dt and 2) wrap them even when just disabling a device in a devtree?
The PCI devices corresponding to the internal controllers must not be wrapped by any chip drivers. This is how almost all the internal chip devices are organized in coreboot. The only exception is the CNVi device which happened to be wrapped inconsistently. I have some patches that I plan to push this week to fix that.
If there is a need to wrap a chip driver for the internal devices for any reason (ACPI generation, etc.), then that can be done by adding a virtual/dummy device under the controller device. This device wouldn't have to be modified by the mainboard unless it needs to override some chip config. Thus, the only cases where a mainboard would have to follow the chip wrapping to match the chipset tree is: 1. For SoC chip 2. For any virtual device chips that require chip config changes.