Re: [PATCH v1 0/2] ACPI: scan: Honor certain device identification rules

[Andy Shevchenko]

On Wed, Oct 27, 2021 at 08:12:20PM +0200, Rafael J. Wysocki wrote:
> On Wed, Oct 27, 2021 at 7:35 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> 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:

...

> > > 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).

Probably we have to communicate this to EHL program owners internally...
I dunno what this means in case of Sound Wire.

> >         "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?

It depends on the BIOS decision at boot time. No idea if it's only one
possibility (what I have heard is that device is PCI enumerated, that's
why they chose PCI ID in the CSRT, to avoid allocating new IDs for truly
ACPI-enumerated device).

But seems another point to discuss internally.

> >     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?

No idea. Let's discuss internally.

P.S. The issue here is that some BIOS versions are floating around and
we never know who is using what... :-(

-- 
With Best Regards,
Andy Shevchenko