Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper

From: Jonathan Cameron
Date: Tue Sep 29 2020 - 11:54:36 EST


On Mon, 28 Sep 2020 15:54:23 +0300
Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote:

> This tries to solve a warning reported by the lkp bot:
>
> >> drivers/iio/adc/at91_adc.c:1439:34: warning: unused variable
> >> 'at91_adc_dt_ids' [-Wunused-const-variable]
> static const struct of_device_id at91_adc_dt_ids[] = {
> ^
> 1 warning generated.
>
> This shows up with 'compiler: clang version 12.0.0' and W=1 (as the bot
> mentions).
>
> Forward declarations for global variables can be a bit weird; forward
> function declarations are more common.
Hi,

That's not the forward declaration that it is complaining about...

It's a reasonable patch anyway, but doesn't fix that warning which is
about of_match_ptr and the lack of #ifdef CONFIG_OF around the
actual definition.

For the bug warning, I'd add add a Kconfig dependency on OF.
It doesn't make any sense to allow building this driver without that.

So resend this as a simple tidy up patch and another one adding
that build dependency.

Thanks,

Jonathan

>
> Maybe another fix for this would have been to prefix with 'extern' the
> 'at91_adc_dt_ids' variable, thus making it more friendly as a forward
> declaration. It would look weird, but it would work.
>
> But, we can avoid that forward declaration altogether simply by obtaining
> the private data with of_device_get_match_data().
>
> This appeared after commit 4027860dcc4c ("iio: Kconfig: at91_adc: add
> COMPILE_TEST dependency to driver"), which put this driver on the lkp's bot
> radar.
>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> ---
> drivers/iio/adc/at91_adc.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 9b2c548fae95..c9ec0a4a357e 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -829,8 +829,6 @@ static u32 calc_startup_ticks_9x5(u32 startup_time, u32 adc_clk_khz)
> return ticks;
> }
>
> -static const struct of_device_id at91_adc_dt_ids[];
> -
> static int at91_adc_probe_dt_ts(struct device_node *node,
> struct at91_adc_state *st, struct device *dev)
> {
> @@ -878,8 +876,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
> if (!node)
> return -EINVAL;
>
> - st->caps = (struct at91_adc_caps *)
> - of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> + st->caps = (struct at91_adc_caps *)of_device_get_match_data(&pdev->dev);
>
> st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
>