linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper
@ 2020-09-28 12:54 Alexandru Ardelean
  2020-09-28 12:54 ` [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data Alexandru Ardelean
  2020-09-29 15:54 ` [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper Jonathan Cameron
  0 siblings, 2 replies; 6+ messages in thread
From: Alexandru Ardelean @ 2020-09-28 12:54 UTC (permalink / raw)
  To: linux-iio, linux-arm-kernel
  Cc: linux-kernel, jic23, nicolas.ferre, alexandre.belloni,
	ludovic.desroches, Alexandru Ardelean

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.

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data
  2020-09-28 12:54 [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper Alexandru Ardelean
@ 2020-09-28 12:54 ` Alexandru Ardelean
  2020-09-29 15:55   ` Jonathan Cameron
  2020-09-29 15:54 ` [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper Jonathan Cameron
  1 sibling, 1 reply; 6+ messages in thread
From: Alexandru Ardelean @ 2020-09-28 12:54 UTC (permalink / raw)
  To: linux-iio, linux-arm-kernel
  Cc: linux-kernel, jic23, nicolas.ferre, alexandre.belloni,
	ludovic.desroches, Alexandru Ardelean

The main intent is to get rid of the cast for the void-pointer returned by
of_device_get_match_data().

This requires const-ifying the 'caps' and 'registers' references on the
at91_adc_state struct.

The caps can be obtained also from the old platform_data (in the
at91_adc_probe_pdata() function), but that cast is not touched in this
patch, since the old platform_data should be removed/cleaned-away.
Also, that cast deals with converting a kernel_ulong_t type to a pointer.
So, updating that cast doesn't yield any benefit.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/adc/at91_adc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index c9ec0a4a357e..7d846a2852a5 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -202,7 +202,7 @@ struct at91_adc_state {
 	struct mutex		lock;
 	u8			num_channels;
 	void __iomem		*reg_base;
-	struct at91_adc_reg_desc *registers;
+	const struct at91_adc_reg_desc *registers;
 	u32			startup_time;
 	u8			sample_hold_time;
 	bool			sleep_mode;
@@ -214,7 +214,7 @@ struct at91_adc_state {
 	u32			res;		/* resolution used for convertions */
 	bool			low_res;	/* the resolution corresponds to the lowest one */
 	wait_queue_head_t	wq_data_avail;
-	struct at91_adc_caps	*caps;
+	const struct at91_adc_caps	*caps;
 
 	/*
 	 * Following ADC channels are shared by touchscreen:
@@ -550,7 +550,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
 {
 	struct iio_dev *idev = iio_trigger_get_drvdata(trig);
 	struct at91_adc_state *st = iio_priv(idev);
-	struct at91_adc_reg_desc *reg = st->registers;
+	const struct at91_adc_reg_desc *reg = st->registers;
 	u32 status = at91_adc_readl(st, reg->trigger_register);
 	int value;
 	u8 bit;
@@ -876,7 +876,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 	if (!node)
 		return -EINVAL;
 
-	st->caps = (struct at91_adc_caps *)of_device_get_match_data(&pdev->dev);
+	st->caps = of_device_get_match_data(&pdev->dev);
 
 	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper
  2020-09-28 12:54 [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper Alexandru Ardelean
  2020-09-28 12:54 ` [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data Alexandru Ardelean
@ 2020-09-29 15:54 ` Jonathan Cameron
  2020-09-30  5:28   ` Alexandru Ardelean
  2020-09-30  5:30   ` Alexandru Ardelean
  1 sibling, 2 replies; 6+ messages in thread
From: Jonathan Cameron @ 2020-09-29 15:54 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, linux-arm-kernel, linux-kernel, nicolas.ferre,
	alexandre.belloni, ludovic.desroches

On Mon, 28 Sep 2020 15:54:23 +0300
Alexandru Ardelean <alexandru.ardelean@analog.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.

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


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data
  2020-09-28 12:54 ` [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data Alexandru Ardelean
@ 2020-09-29 15:55   ` Jonathan Cameron
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2020-09-29 15:55 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, linux-arm-kernel, linux-kernel, nicolas.ferre,
	alexandre.belloni, ludovic.desroches

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

> The main intent is to get rid of the cast for the void-pointer returned by
> of_device_get_match_data().
> 
> This requires const-ifying the 'caps' and 'registers' references on the
> at91_adc_state struct.
> 
> The caps can be obtained also from the old platform_data (in the
> at91_adc_probe_pdata() function), but that cast is not touched in this
> patch, since the old platform_data should be removed/cleaned-away.
> Also, that cast deals with converting a kernel_ulong_t type to a pointer.
> So, updating that cast doesn't yield any benefit.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Good change. I'll pick this up when patch 1 is ready.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/at91_adc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index c9ec0a4a357e..7d846a2852a5 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -202,7 +202,7 @@ struct at91_adc_state {
>  	struct mutex		lock;
>  	u8			num_channels;
>  	void __iomem		*reg_base;
> -	struct at91_adc_reg_desc *registers;
> +	const struct at91_adc_reg_desc *registers;
>  	u32			startup_time;
>  	u8			sample_hold_time;
>  	bool			sleep_mode;
> @@ -214,7 +214,7 @@ struct at91_adc_state {
>  	u32			res;		/* resolution used for convertions */
>  	bool			low_res;	/* the resolution corresponds to the lowest one */
>  	wait_queue_head_t	wq_data_avail;
> -	struct at91_adc_caps	*caps;
> +	const struct at91_adc_caps	*caps;
>  
>  	/*
>  	 * Following ADC channels are shared by touchscreen:
> @@ -550,7 +550,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
>  {
>  	struct iio_dev *idev = iio_trigger_get_drvdata(trig);
>  	struct at91_adc_state *st = iio_priv(idev);
> -	struct at91_adc_reg_desc *reg = st->registers;
> +	const struct at91_adc_reg_desc *reg = st->registers;
>  	u32 status = at91_adc_readl(st, reg->trigger_register);
>  	int value;
>  	u8 bit;
> @@ -876,7 +876,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>  	if (!node)
>  		return -EINVAL;
>  
> -	st->caps = (struct at91_adc_caps *)of_device_get_match_data(&pdev->dev);
> +	st->caps = of_device_get_match_data(&pdev->dev);
>  
>  	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
>  


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper
  2020-09-29 15:54 ` [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper Jonathan Cameron
@ 2020-09-30  5:28   ` Alexandru Ardelean
  2020-09-30  5:30   ` Alexandru Ardelean
  1 sibling, 0 replies; 6+ messages in thread
From: Alexandru Ardelean @ 2020-09-30  5:28 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Alexandru Ardelean, linux-iio, linux-arm-kernel, LKML,
	Nicolas Ferre, alexandre.belloni, Ludovic Desroches

On Tue, Sep 29, 2020 at 6:55 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 28 Sep 2020 15:54:23 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.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@intel.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.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");
> >
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper
  2020-09-29 15:54 ` [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper Jonathan Cameron
  2020-09-30  5:28   ` Alexandru Ardelean
@ 2020-09-30  5:30   ` Alexandru Ardelean
  1 sibling, 0 replies; 6+ messages in thread
From: Alexandru Ardelean @ 2020-09-30  5:30 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Alexandru Ardelean, linux-iio, linux-arm-kernel, LKML,
	Nicolas Ferre, alexandre.belloni, Ludovic Desroches

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

Also, silly me here.
I was just annoyed with the email from the bot, so I didn't dig too
deep to understand.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-30  5:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-28 12:54 [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper Alexandru Ardelean
2020-09-28 12:54 ` [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data Alexandru Ardelean
2020-09-29 15:55   ` Jonathan Cameron
2020-09-29 15:54 ` [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper Jonathan Cameron
2020-09-30  5:28   ` Alexandru Ardelean
2020-09-30  5:30   ` Alexandru Ardelean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).