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

[Rafael J. Wysocki]

On Wed, Oct 27, 2021 at 8:11 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 27, 2021 at 08:34:49PM +0300, Andy Shevchenko 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@xxxxxxxxxxxxxxx>
> > >
> > > 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
> >     Name (_CID, Package (0x02)  // _CID: Compatible ID
> >     {
> >         "PRP00001",
> >         "PNP0A05" /* Generic Container Device */
> >     })
> >
> > Case 2 - GP DMA devices (3 times):
> >
> >     Name (_ADR, 0x001D0003)  // _ADR: Address
> >     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
> >     Name (_HID, "INT3472")  // _HID: Hardware ID
> >     Name (_CID, "INT3472")  // _CID: Compatible ID
> >
> > Case 4 - LNK devices (6 times):
> >
> >     Name (_ADR, Zero)  // _ADR: Address
> >     ...
> >
> >     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
> >     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).
> >
> > Can you confirm that your change won't modify behaviour for these devices?
>
> Okay, I have looked into acpi_dma_parse_resource_group() and I don't see that
> we actually use _HID there. We definitely use _CRS. However, _HID is used in
> case when device is ACPI-enumerated (drivers/dma/dw/platform.c). Seems like
> firmware should provide this part runtime (either _HID or _ADR, but not both).

Right.

So after patch [2/2] _HID will always be used for the enumeration in
such cases which may not be what happens now - and what happens now
depends on the ordering in which the objects in question are seen
during the namespace walk.  If the DMA device is seen before the PCI
host bridge, it will be enumerated using the _HID.