Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48385 )
Change subject: soc/intel/common/block/uart: rework to use dummy device ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
You really should check the history of drivers/wifi/generic, commit d436750 for example...
Well, the history is one thing. The current code is what matters however. How about you read that?
I did ;) Actually I meant that one change, the wording is just misleading
One major difference in the way UART driver is being handled in this CL and how CNVi device is handled is: For CNVi:
- PCI operations are still performed as part of the PCI driver for the CNVi controller
- ACPI operations are performed as part of the chip driver written for the dummy device under CNVi controller
For UART:
- This change performs PCI and ACPI operations on the parent of the dummy device. This does not work for the platforms that do not add the dummy device in devicetree/overridetree.
So, that's actually what I had marked here already, right? https://review.coreboot.org/c/coreboot/+/48385/1/src/soc/intel/common/block/...
You are going to need to allow PCI ops on the PCI device directly. And the separate chip driver for the dummy device will perform ACPI ops only. That will allow other mainboards to not use the dummy device at all. The way it is written currently in the link you pasted above - it assumes that the PCI device is always a parent of the dummy device which is not true for any other mainboards and hence it breaks the behavior.
-
Furquan Shaikh (Code Review)