Re: [PATCH 3/4] iio: light: max44009: add missing OF device matching

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 11 Mar 2023 13:28:17 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 11/03/2023 13:26, Jonathan Cameron wrote:
> > On Sat, 11 Mar 2023 12:14:56 +0100
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >   
> >> The driver currently matches only via i2c_device_id, but also has
> >> of_device_id table:
> >>
> >>   drivers/iio/light/max44009.c:545:34: error: ‘max44009_of_match’ defined but not used [-Werror=unused-const-variable=]
> >>
> >> Fixes: 6aef699a7d7e ("iio: light: add driver for MAX44009")
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>  
> > 
> > Don't use of_match_ptr() unless you are absolutely sure no other firmware
> > route will make use of the of_match_table.
> > 
> > In this particular case ACPI using PRP0001 is broken by that macro.  
> 
> It's not broken because there was no matching via PRP0001 due to missing
> table.
> 
> > 
> > So good to set the of_match_table, but make sure to always set it
> > and hence you don't need the __maybe_unused.  
> 
> So you want to add PRP0001? We can, the fix is for different issue, though.

There is nothing to add.  You need to do less than you have done in this patch.
Drop the of_match_ptr() and the __maybe_unused and PRP0001 based matching will just
work. The PRP0001 path just uses the of_device_id table and needs no
specific support in a driver - it doesn't need an ACPI id table or anything like
that.

It's a long story, but hindsight says that of_match_ptr() should never have
existed as it only serves to stop things working that otherwise work for free.

Jonathan


> 
> Best regards,
> Krzysztof
>