lkml.org 
[lkml]   [2021]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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?

\
 
 \ /
  Last update: 2021-10-27 20:13    [W:0.047 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site