Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Wed, 27 Oct 2021 20:12:20 +0200 | Subject | Re: [PATCH v1 0/2] ACPI: scan: Honor certain device identification rules |
| |
On Wed, Oct 27, 2021 at 7:35 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Oct 26, 2021 at 10:33:17PM +0300, Andy Shevchenko wrote: > > On Tue, Oct 26, 2021 at 08:51:49PM +0200, Rafael J. Wysocki wrote: > > > Hi All, > > > > > > There are some rules in the ACPI spec regarding which device identification > > > objects can be used together etc., but they are not followed by the kernel > > > code. > > > > > > This series modifies the code to follow the spec more closely (see patch > > > changelogs for details). > > > > I understand the motivation, but afraid about consequences on the OEM cheap > > devices that are not always follow letter of the specification. > > > > As per Intel platforms I would look into Baytrail / Cherrytrail devices for > > the past (I think Hans may help here a lot) and into Elkhart Lake in the > > present (for the letter I mostly refer to CSRT + DSDT cooperation to get > > GP DMA devices enumerated, so I _hope_ DSDT shouldn't have _ADR and _HID > > together). > > > > Hence, from the code perspective > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > From the practice I would wait for some tests. I will try to find any new > > information about latest firmware tables on Elkhart Lake machines. > > So, what I see in Elkhart Lake > > Case 1 - Sound Wire devices (2 times): > > Name (_ADR, 0x40000000) // _ADR: Address
No _HID, so the IDs returned by the _CID below won't be used.
> Name (_CID, Package (0x02) // _CID: Compatible ID > { > "PRP00001",
The above device ID is invalid (one 0 too many).
> "PNP0A05" /* Generic Container Device */
Without the change this causes a container device to be created, but the only purpose of it may be offline/online (if the child devices support offline/online).
This change should not be functionally relevant.
> }) > > Case 2 - GP DMA devices (3 times): > > Name (_ADR, 0x001D0003) // _ADR: Address
_ADR will be ignored which may not be expected. Is this a PCI device?
> Name (_HID, "80864BB4") // _HID: Hardware ID > > Case 3 - Camera PMIC devices (5 x 2 (CLPn/DSCn) + 1 (PMIC) times = 11x): > > Name (_ADR, Zero) // _ADR: Address
_ADR will be ignored, which shouldn't matter.
> Name (_HID, "INT3472") // _HID: Hardware ID > Name (_CID, "INT3472") // _CID: Compatible ID > > Case 4 - LNK devices (6 times): > > Name (_ADR, Zero) // _ADR: Address
Same here.
> ... > > Name (_UID, One) // _UID: Unique ID > Method (_HID, 0, NotSerialized) // _HID: Hardware ID > { > Return (HCID (One)) > } > > Case 5 - Camera sensors (2 times): > > Name (_ADR, Zero) // _ADR: Address
And same here.
> Name (_HID, "INT34xx") // _HID: Hardware ID > Name (_CID, "INT34xx") // _CID: Compatible ID > > > I have no idea about cameras or audio devices, but what I'm worrying about > is GP DMA. This kind of devices are PCI, but due to Microsoft hack, called > CSRT, we have to have a possibility to match DSDT with CSRT ot retrieve > the crucial information from the latter while being enumerated by the former. > > While it may be against the specification, there is no other way to achieve > that as far as I understand (without either breaking things in Linux or > getting yellow bang in Windows).
I'm not really sure why _HID is needed for this. The PCI device ID could be used for CRST matching just fine.
> Can you confirm that your change won't modify behaviour for these devices?
Well, the GP DMA thing may be broken by patch [2/2], but does Windows actually use _ADR if _HID is provided?
| |