* [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper @ 2020-09-28 12:54 ` Alexandru Ardelean 0 siblings, 0 replies; 12+ 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] 12+ messages in thread
* [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper @ 2020-09-28 12:54 ` Alexandru Ardelean 0 siblings, 0 replies; 12+ messages in thread From: Alexandru Ardelean @ 2020-09-28 12:54 UTC (permalink / raw) To: linux-iio, linux-arm-kernel Cc: alexandre.belloni, linux-kernel, ludovic.desroches, Alexandru Ardelean, jic23 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data 2020-09-28 12:54 ` Alexandru Ardelean @ 2020-09-28 12:54 ` Alexandru Ardelean -1 siblings, 0 replies; 12+ 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] 12+ messages in thread
* [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data @ 2020-09-28 12:54 ` Alexandru Ardelean 0 siblings, 0 replies; 12+ messages in thread From: Alexandru Ardelean @ 2020-09-28 12:54 UTC (permalink / raw) To: linux-iio, linux-arm-kernel Cc: alexandre.belloni, linux-kernel, ludovic.desroches, Alexandru Ardelean, jic23 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data 2020-09-28 12:54 ` Alexandru Ardelean @ 2020-09-29 15:55 ` Jonathan Cameron -1 siblings, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data @ 2020-09-29 15:55 ` Jonathan Cameron 0 siblings, 0 replies; 12+ messages in thread From: Jonathan Cameron @ 2020-09-29 15:55 UTC (permalink / raw) To: Alexandru Ardelean Cc: alexandre.belloni, linux-iio, linux-kernel, ludovic.desroches, linux-arm-kernel 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"); > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper 2020-09-28 12:54 ` Alexandru Ardelean @ 2020-09-29 15:54 ` Jonathan Cameron -1 siblings, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper @ 2020-09-29 15:54 ` Jonathan Cameron 0 siblings, 0 replies; 12+ messages in thread From: Jonathan Cameron @ 2020-09-29 15:54 UTC (permalink / raw) To: Alexandru Ardelean Cc: alexandre.belloni, linux-iio, linux-kernel, ludovic.desroches, linux-arm-kernel 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"); > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper 2020-09-29 15:54 ` Jonathan Cameron @ 2020-09-30 5:28 ` Alexandru Ardelean -1 siblings, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper @ 2020-09-30 5:28 ` Alexandru Ardelean 0 siblings, 0 replies; 12+ messages in thread From: Alexandru Ardelean @ 2020-09-30 5:28 UTC (permalink / raw) To: Jonathan Cameron Cc: alexandre.belloni, linux-iio, LKML, Ludovic Desroches, Alexandru Ardelean, linux-arm-kernel 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"); > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper 2020-09-29 15:54 ` Jonathan Cameron @ 2020-09-30 5:30 ` Alexandru Ardelean -1 siblings, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH 1/2] iio: adc: at91_adc: use of_device_get_match_data() helper @ 2020-09-30 5:30 ` Alexandru Ardelean 0 siblings, 0 replies; 12+ messages in thread From: Alexandru Ardelean @ 2020-09-30 5:30 UTC (permalink / raw) To: Jonathan Cameron Cc: alexandre.belloni, linux-iio, LKML, Ludovic Desroches, Alexandru Ardelean, linux-arm-kernel 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"); > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-09-30 5:31 UTC | newest] Thread overview: 12+ 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 ` Alexandru Ardelean 2020-09-28 12:54 ` [PATCH 2/2] iio: adc: at91_adc: const-ify some driver data Alexandru Ardelean 2020-09-28 12:54 ` Alexandru Ardelean 2020-09-29 15:55 ` Jonathan Cameron 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-29 15:54 ` Jonathan Cameron 2020-09-30 5:28 ` Alexandru Ardelean 2020-09-30 5:28 ` Alexandru Ardelean 2020-09-30 5:30 ` Alexandru Ardelean 2020-09-30 5:30 ` Alexandru Ardelean
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.