Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37935 )
Change subject: src/*.asl: Remove _HID / _ADR objects ......................................................................
Patch Set 3:
(4 comments)
This is untested and no objective was given why one of _ADR or _HID was preferred over the other. It's unclear if there are OS that requires one of them to be present.
SerialIO devices are always in ACPI mode for these platforms on all boards in the tree, and _HID is required for ACPI mode, so makes sense to drop _ADR here
grep says no.
Yes this is correct these were always just put in ACPI mode on baytrail. This is something where Intel went back and forth on and eventually settled PCI mode in future chipsets.
I tried to support both modes in coreboot (which is why it had both _HID and _ADR) but it may be easier to settle on one path, or put this behind a Kconfig variable.
We just discovered something related a few days ago: There's a chromium kernel driver that expects a specific setting per board. So if upstream coreboot differs from the original firmware, the driver fails. Settling on one path (_HID seems to be the only choice because of Windows compatibility) might be a good idea, but we have to keep in mind that it affects the whole ecosystem.
If we want a quick solution (some people seem anxious to update iasl), I'd prefer that we put the correct method into an SSDT for now.
https://review.coreboot.org/c/coreboot/+/37935/3/src/mainboard/aopen/dxplplu... File src/mainboard/aopen/dxplplusu/acpi/p64h2.asl:
PS3: Changes here seem correct (albeit, the device nodes seem dead anyway?). Checked that the PCI devices show up in a kernel log in board-status.
https://review.coreboot.org/c/coreboot/+/37935/3/src/mainboard/google/kahlee... File src/mainboard/google/kahlee/variants/baseboard/include/baseboard/acpi/audio.asl:
PS3: Device is not on any bus at all, so this seems right.
https://review.coreboot.org/c/coreboot/+/37935/3/src/mainboard/intel/strago/... File src/mainboard/intel/strago/acpi/mainboard.asl:
PS3: I2C is generally not considered a probed bus, so this should be correct too.
https://review.coreboot.org/c/coreboot/+/37935/3/src/soc/intel/broadwell/acp... File src/soc/intel/broadwell/acpi/serialio.asl:
PS3: See CB:36318, CB:37710.