Re: [PATCH v3] ACPI: bus: Use OF match data for PRP0001 matched devices

From: Andy Shevchenko

Date: Mon Jan 12 2026 - 03:56:02 EST


On Mon, Jan 12, 2026 at 02:12:49PM +0530, Kartik Rajput wrote:
> On 12/01/26 03:31, Sakari Ailus wrote:
> > On Fri, Jan 09, 2026 at 10:11:26PM +0100, Rafael J. Wysocki wrote:
> > > On Fri, Jan 9, 2026 at 6:02 PM Andy Shevchenko
> > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > > On Fri, Jan 09, 2026 at 01:29:59PM +0200, Sakari Ailus wrote:
> > > > > On Fri, Jan 09, 2026 at 01:05:52PM +0200, Andy Shevchenko wrote:
> > > > > > On Fri, Jan 09, 2026 at 11:13:02AM +0100, Mika Westerberg wrote:
> > > > > > > On Fri, Jan 09, 2026 at 03:23:58PM +0530, Kartik Rajput wrote:

...

> > > > > > On top of what Mika asked, shouldn't we check CID as well? Theoretically it's
> > > > > > possible that some device may have HID "blablabla" and CID PRP0001, I don't
> > > > > > remember what documentation says about this case, though.
> > > > >
> > > > > According to Documentation/firmware-guide/acpi/enumeration.rst PRP0001 is
> > > > > also valid for _CID. So yes, I think this should be checked as well -- I'd
> > > > > loop over the &device->pnp.ids list.
> > > >
> > > > Yeah, but if we have a device with
> > > >
> > > > HID "blablabla"
> > > > CID "PRP0001"
> > > >
> > > > and at the same time the driver has ACPI ID listed, we should probably use that
> > > > one as HID should have higher weight for matching. Logic here is not just as simple
> > > > as looping over pnp.ids how I see it.
> > >
> > > Right.
> > >
> > > What about:
> > >
> > > if (acpi_ids) {
> > > match = acpi_match_device(acpi_ids, dev);
> > > if (match)
> > > return (const void *)match->driver_data;
> > > }
> > > return acpi_of_device_get_match_data(dev);
> >
> > That would mean that any ACPI (or PNP) ID has priority over compatible
> > matching, wouldn't it? AFAIU the documentation says effectively that
> > _HID/_CID priority is upheld, whether matching with PRP0001 or without.
>
> Since we seem to be using __acpi_match_device() match the device.
> What if we directly utilise __acpi_match_device() here?
>
> Something like:
>
> if (!__acpi_match_device(adev, acpi_ids, of_ids, &acpi_id, &of_id))
> return NULL;
>
> if (acpi_id)
> return (const void *)acpi_id->driver_data;
>
> if (of_id)
> return of_id->data;
>
> return NULL;
>
> Then, we can also remove acpi_of_device_get_match_data()?

At brief look it's indeed seems to be a good optimisation as well.

--
With Best Regards,
Andy Shevchenko