Attention is currently required from: Felix Singer, Tim Crawford, Patrick Rudolph, Jeremy Soller, Christian Walter, Tim Wawrzynczak, Michael Niewöhner. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59887 )
Change subject: soc/intel/cannonlake: Hook up DPTF device to devicetree ......................................................................
Patch Set 8:
(1 comment)
File src/mainboard/google/hatch/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/59887/comment/deb9f1b9_52f292a3 PS4, Line 204: device pci 04.0 on end # SA Thermal device Well, what I said:
However we shouldn't mix clean-ups without behavioral changes with fix-ups that change behavior in one commit.
This change does 4 things:
* Fix Hatch where coreboot should have seen but ignored a PCI device. Now it should configure the device, e.g. assign resources. So the state after coreboot ran might differ. * Fix other boards where the device is not expected to show up. coreboot should never have seen the device, so the state after coreboot shouldn't differ. * Configure FSP based on the devicetree on/off state. * Remove the obsoleted chip.h setting.
For the latter two it definitely makes sense to do them in one commit. It's just a refactoring.
However, the former two, especially the first, shouldn't be mixed with refactorings.
Also, the commit summary (first line) should cover all changes. Currently it doesn't even suggest that behavior for random boards changes.
-
Nico Huber (Code Review)