On 12/03/2023 15:14, Jonathan Cameron wrote: > 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. OK, I understand your point. I assumed we do not care at all about PRP0001 if ACPI is enabled, because then we simply use ACPI table. But indeed they might for example be not in sync... > > 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. Best regards, Krzysztof
