On Sun, 12 Mar 2023 11:17:05 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > On 11/03/2023 19:44, Jonathan Cameron wrote: > > On Sat, 11 Mar 2023 13:30:01 +0100 > > Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > >> On 11/03/2023 13:28, Jonathan Cameron wrote: > >>> On Sat, 11 Mar 2023 12:14:57 +0100 > >>> Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > >>> > >>>> The driver can be compile tested with !CONFIG_OF or !CONFIG_ACPI making > >>>> certain data unused: > >>>> > >>>> drivers/iio/proximity/sx9500.c:1039:34: error: ‘sx9500_of_match’ defined but not used [-Werror=unused-const-variable=] > >>>> > >>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > >>> > >>> Hi Krysztof > >>> > >>> Thanks for looking at these warnings. > >>> > >>> Drop the protection macros instead. The tables are trivial in size and > >>> the of_match_ptr() breaks some ways this driver can be used. > >>> ACPI_PTR() isn't as bad, but is pretty much pointless given this size of > >>> the array. > >>> > >> > >> For ACPI platform, ACPI table is used, so nothing for PRP0001. For OF > >> platform, OF table is used. > > > > So you would think, but nope.. That's not how it works (I was surprised > > when I came across this the first time too) > > > > PRP0001 is magic and requires no specific support in an individual > > driver beyond not using that of_match_ptr() macro! > > I know, we talk about ACPI table. I'm not sure I follow. I thought by ACPI table you meant the acpi_device_id table pointed to by acpi_match_table in struct device_driver. That one is not needed for PRP0001. It is irrelevant if there is one or not. Maybe the confusion is that you think the presence of an acpi_match table means we don't also check PRP0001? As you can see here https://elixir.bootlin.com/linux/latest/source/drivers/acpi/bus.c#L886 it is checked in all cases. If you meant the DSDT table being provide by the firmware I don't see the relevance to this discussion. > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/acpi/bus.c#L754 > > Docs here > > https://elixir.bootlin.com/linux/latest/source/Documentation/firmware-guide/acpi/enumeration.rst#L450 > > The code is compile when CONFIG_ACPI is defined, right? Then you have > ACPI table, so what for ACPI platform is missing? ACPI platform has ACPI > table. I don't follow. A given ACPI platform may provide in DSDT one of two choices to bind to this driver. Either it provides an entry from the acpi_device_id table, or it must use PRP0001 and a compatible entry from the of_device_id table. That only works if of_match_ptr() is not used. As a side note, both the IDs in the ACPI match table are not valid IDs for use in DSDT. We are supporting them only because they have been used on shipping devices. Semtech does have a PNP ID of STH but that's not the one used. Anyhow, to be clear. For IIO drivers, don't use of_match_ptr() or ACPI_PTR(). There are some legacy cases that we haven't cleaned up yet, but I'm not taking patches that add any new ones without a very very strong argument in favour and so far no one has successfully made one. Jonathan > > Best regards, > Krzysztof >
