Re: [PATCH v3 3/7] acpi: add acpi_of_match_device_ids

[Rafael J. Wysocki]

On Tue, Mar 24, 2026 at 5:26 PM Markus Probst <markus.probst@xxxxxxxxx> wrote:
>
> On Tue, 2026-03-24 at 17:01 +0100, Rafael J. Wysocki wrote:
> > On Tue, Mar 24, 2026 at 4:30 PM Markus Probst <markus.probst@xxxxxxxxx> wrote:
> > >
> > > On Mon, 2026-03-23 at 20:57 +0100, Rafael J. Wysocki wrote:
> > > > On Fri, Mar 13, 2026 at 8:03 PM Markus Probst via B4 Relay
> > > > <devnull+markus.probst.posteo.de@xxxxxxxxxx> wrote:
> > > > >
> > > > > From: Markus Probst <markus.probst@xxxxxxxxx>
> > > > >
> > > > > Add a function to match acpi devices against of_device_ids. This will be
> > > > > used in the following commit ("mfd: match acpi devices against PRP0001")
> > > > > to match mfd sub-devices against a of compatible string.
> > > >
> > > > Please always spell ACPI in capitals in patch subjects, comments,
> > > > changelogs, etc.  It is not a regular word.
> > > Ok.
> > > >
> > > > > Signed-off-by: Markus Probst <markus.probst@xxxxxxxxx>
> > > > > ---
> > > > >  drivers/acpi/bus.c      | 7 +++++++
> > > > >  include/acpi/acpi_bus.h | 2 ++
> > > > >  2 files changed, 9 insertions(+)
> > > > >
> > > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > > > index f6707325f582..5ddcc56edc87 100644
> > > > > --- a/drivers/acpi/bus.c
> > > > > +++ b/drivers/acpi/bus.c
> > > > > @@ -1044,6 +1044,13 @@ int acpi_match_device_ids(struct acpi_device *device,
> > > > >  }
> > > > >  EXPORT_SYMBOL(acpi_match_device_ids);
> > > > >
> > > >
> > > > Missing kerneldoc.
> > > The same amount of kerneldoc as `acpi_match_device_ids`, if I am not
> > > mistaken.
> > > >
> > > > > +int acpi_of_match_device_ids(struct acpi_device *device,
> > > > > +                         const struct of_device_id *ids)
> > > > > +{
> > > > > +       return __acpi_match_device(device, NULL, ids, NULL, NULL) ? 0 : -ENOENT;
> > > > > +}
> > > > > +EXPORT_SYMBOL(acpi_of_match_device_ids);
> > > >
> > > > Are you aware of the consensus that using PRP0001 in production
> > > > platform firmware will be regarded as invalid?
> > > >
> > > > Because of that, it is not an option for a driver to avoid providing
> > > > ACPI match data on a platform that uses ACPI.
> > > First of all, the driver that would have made use of it has been
> > > restructed to not use mfd subdevices. It would not be affected anymore
> > > through this patch set.
> >
> > So what exactly would be affected by it?
> I won't have a use for myself anymore, but I still think the patch is
> useful. Anyway,
>
> MFD Devices without an assigned ACPI ID, if they are present on devices
> with ACPI platform firmware.
>
> >
> > > Not sure if I should still send it as its own patch series though.
> That is why I asked this question (see 1. sentence in the paragraph
> above).
>
> > >
> > > The device of the driver has no ACPI ID allocated by the manufacturer,
> > > as it is only used on a proprietary Linux OS (with their own modified
> > > kernel).
> >
> > Do I understand correctly that there is an ACPI platform firmware on
> > the board, but it doesn't enumerate the given device properly (that
> > is, as an ACPI device object with a specific device ID)?
> There is only a serial device in the ACPI platform firmware.
> The device connected to the bus isn't specified.
> >
> > In which case there probably is a driver that can find that device
> > somehow (it has hardcoded resources or similar).
> Yes, that driver has `filp_open("/dev/ttyS1")` hardcoded.
> >
> > > The driver would have only been useful via device tree or an ACPI
> > > Overlay.
> >
> > Do you mean a custom SSDT loaded via configfs or something else?
> Yes, in my case via initrd.
>
> >
> > > Obviously, I don't have a PNP or ACPI Vendor ID, so I can't
> > > assign one. The parent/main driver does only have a of compatible id.
> > > As it needs to use PRP0001 anyway on ACPI, I thought it makes more
> > > sense to also use PRP0001 there instead of matching it with a _ADR
> > > which is "a grey area in the ACPI specification".
> >
> > You can't match a device with _ADR.  By itself, _ADR doesn't provide
> > you with any information on the device in question, it only helps to
> > connect it to some information that can be collected by other means.
> > The role of it, at least in principle, is to allow some device objects
> > in the ACPI hierarchy to be associated with devices enumerated by
> > other means (like on a PCI bus).
>
> This patch affects mfd devices. A bus device can via mfd register child
> devices and those child devices will be matched to a fwnode if
> available.

That only works because the parent can be recognized and properly
enumerated, so it is parent-relative.

> According to commit 98a3be44ffa67b812de7aa7aed9f2331edcfb1a5, there is
> a board on the market with a sub-device that will be matched using _ADR
> [1].

With the help of a quirk though.

> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=98a3be44ffa67b812de7aa7aed9f2331edcfb1a5
>
> >
> > The enumeration with the help of PRP0001 only works if there is a
> > device object in the ACPI hierarchy and its _HID is PRP0001 or its
> > _CID list contains PRP0001, there is a _DSD under it and a
> > "compatible" property is returned by that _DSD.  Who's going to
> > provide all of that for the given device?
> A ACPI Overlay would do that.
>
> >
> > Moreover, if the device has some resources that the kernel needs to
> > know about, there should be a _CRS under the device object in question
> > and the resources should be listed there.  Or how are the resources
> > going to be found otherwise?
> Resources in mfd are usually handled by the parent device, not the mfd
> child device. But yes, it would be using _CRS if any.

So this is kind of a valid use case, but since you don't need it any
more, I'd rather not put it in without a clear need.

Also, it's not a huge deal for a vendor to allocate a proper ACPI
device ID for a piece of hardware.  It just involves some due
diligence.