linux-kernel - Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+U=DsrKGTE3nC09NEHfksF=1WQaUrM1jV5zpw3u272+zEt1cQ@mail.gmail.com>
Date:   Wed, 30 Sep 2020 08:28:56 +0300
From:   Alexandru Ardelean <ardeleanalex@...il.com>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Alexandru Ardelean <alexandru.ardelean@...log.com>,
        linux-iio <linux-iio@...r.kernel.org>,
        linux-arm-kernel@...ts.infradead.org,
        LKML <linux-kernel@...r.kernel.org>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        alexandre.belloni@...tlin.com,
        Ludovic Desroches <ludovic.desroches@...rochip.com>
Subject: Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper

On Tue, Sep 29, 2020 at 6:55 PM Jonathan Cameron <jic23@...nel.org> wrote:
>
> On Mon, 28 Sep 2020 15:54:23 +0300
> Alexandru Ardelean <alexandru.ardelean@...log.com> 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.
>

At this point it might make sense to also remove the of_match_ptr() helper.
If adding the OF build dependency, the of_match_ptr() would always
return non-NULL.

> 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@...el.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@...log.com>
> > ---
> >  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");
> >
>