* [PATCH v2 0/4] iio: adc: at91: misc driver cleanups
@ 2020-09-30 6:00 ` Alexandru Ardelean
0 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 UTC (permalink / raw)
To: linux-iio, linux-arm-kernel
Cc: linux-kernel, jic23, nicolas.ferre, alexandre.belloni,
ludovic.desroches, Alexandru Ardelean
This whole thing started because the lkp bot haunted me for a while with
this build warning:
>> 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.
The fix may likely be patch 'iio: adc: at91_adc: add Kconfig dependency
on the OF symbol'; was pointed out by Jonathan.
Changelog v1 -> v2:
- https://lore.kernel.org/linux-iio/CA+U=Dspd11N-pXXnnY_5CSzNp50iRr7h16zXTCxo8Fk+v48F7g@mail.gmail.com/T/#m7c0efef4dc623776fe8bafdb5f734b0eaca50f82
- for patch 'iio: adc: at91_adc: use of_device_get_match_data() helper'
changed description; it's just tidy-up patch, not a fix
- added 2 more patches:
- iio: adc: at91_adc: add Kconfig dependency on the OF symbol
- iio: adc: at91_adc: remove of_match_ptr() usage
Alexandru Ardelean (4):
iio: adc: at91_adc: use of_device_get_match_data() helper
iio: adc: at91_adc: const-ify some driver data
iio: adc: at91_adc: add Kconfig dependency on the OF symbol
iio: adc: at91_adc: remove of_match_ptr() usage
drivers/iio/adc/Kconfig | 2 +-
drivers/iio/adc/at91_adc.c | 13 +++++--------
2 files changed, 6 insertions(+), 9 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 0/4] iio: adc: at91: misc driver cleanups
@ 2020-09-30 6:00 ` Alexandru Ardelean
0 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 UTC (permalink / raw)
To: linux-iio, linux-arm-kernel
Cc: alexandre.belloni, linux-kernel, ludovic.desroches,
Alexandru Ardelean, jic23
This whole thing started because the lkp bot haunted me for a while with
this build warning:
>> 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.
The fix may likely be patch 'iio: adc: at91_adc: add Kconfig dependency
on the OF symbol'; was pointed out by Jonathan.
Changelog v1 -> v2:
- https://lore.kernel.org/linux-iio/CA+U=Dspd11N-pXXnnY_5CSzNp50iRr7h16zXTCxo8Fk+v48F7g@mail.gmail.com/T/#m7c0efef4dc623776fe8bafdb5f734b0eaca50f82
- for patch 'iio: adc: at91_adc: use of_device_get_match_data() helper'
changed description; it's just tidy-up patch, not a fix
- added 2 more patches:
- iio: adc: at91_adc: add Kconfig dependency on the OF symbol
- iio: adc: at91_adc: remove of_match_ptr() usage
Alexandru Ardelean (4):
iio: adc: at91_adc: use of_device_get_match_data() helper
iio: adc: at91_adc: const-ify some driver data
iio: adc: at91_adc: add Kconfig dependency on the OF symbol
iio: adc: at91_adc: remove of_match_ptr() usage
drivers/iio/adc/Kconfig | 2 +-
drivers/iio/adc/at91_adc.c | 13 +++++--------
2 files changed, 6 insertions(+), 9 deletions(-)
--
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 [flat|nested] 24+ messages in thread
* [PATCH v2 1/4] iio: adc: at91_adc: use of_device_get_match_data() helper
2020-09-30 6:00 ` Alexandru Ardelean
@ 2020-09-30 6:00 ` Alexandru Ardelean
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 UTC (permalink / raw)
To: linux-iio, linux-arm-kernel
Cc: linux-kernel, jic23, nicolas.ferre, alexandre.belloni,
ludovic.desroches, Alexandru Ardelean
This is a small tidy-up. The of_device_get_match_data() helper retrieves
the driver data from the OF table, without needed to explicitly know the
table variable (since it can retrieve it from the driver object).
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] 24+ messages in thread
* [PATCH v2 1/4] iio: adc: at91_adc: use of_device_get_match_data() helper
@ 2020-09-30 6:00 ` Alexandru Ardelean
0 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 UTC (permalink / raw)
To: linux-iio, linux-arm-kernel
Cc: alexandre.belloni, linux-kernel, ludovic.desroches,
Alexandru Ardelean, jic23
This is a small tidy-up. The of_device_get_match_data() helper retrieves
the driver data from the OF table, without needed to explicitly know the
table variable (since it can retrieve it from the driver object).
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] 24+ messages in thread
* [PATCH v2 2/4] iio: adc: at91_adc: const-ify some driver data
2020-09-30 6:00 ` Alexandru Ardelean
@ 2020-09-30 6:00 ` Alexandru Ardelean
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 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] 24+ messages in thread
* [PATCH v2 2/4] iio: adc: at91_adc: const-ify some driver data
@ 2020-09-30 6:00 ` Alexandru Ardelean
0 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 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] 24+ messages in thread
* [PATCH v2 3/4] iio: adc: at91_adc: add Kconfig dependency on the OF symbol
2020-09-30 6:00 ` Alexandru Ardelean
@ 2020-09-30 6:00 ` Alexandru Ardelean
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 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.
Fixes: 4027860dcc4c ("iio: Kconfig: at91_adc: add COMPILE_TEST dependency to driver")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 91ae90514aff..17e9ceb9c6c4 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -295,7 +295,7 @@ config ASPEED_ADC
config AT91_ADC
tristate "Atmel AT91 ADC"
depends on ARCH_AT91 || COMPILE_TEST
- depends on INPUT && SYSFS
+ depends on INPUT && SYSFS && OF
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
help
--
2.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 3/4] iio: adc: at91_adc: add Kconfig dependency on the OF symbol
@ 2020-09-30 6:00 ` Alexandru Ardelean
0 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 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.
Fixes: 4027860dcc4c ("iio: Kconfig: at91_adc: add COMPILE_TEST dependency to driver")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 91ae90514aff..17e9ceb9c6c4 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -295,7 +295,7 @@ config ASPEED_ADC
config AT91_ADC
tristate "Atmel AT91 ADC"
depends on ARCH_AT91 || COMPILE_TEST
- depends on INPUT && SYSFS
+ depends on INPUT && SYSFS && OF
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
help
--
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] 24+ messages in thread
* [PATCH v2 4/4] iio: adc: at91_adc: remove of_match_ptr() usage
2020-09-30 6:00 ` Alexandru Ardelean
@ 2020-09-30 6:00 ` Alexandru Ardelean
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 UTC (permalink / raw)
To: linux-iio, linux-arm-kernel
Cc: linux-kernel, jic23, nicolas.ferre, alexandre.belloni,
ludovic.desroches, Alexandru Ardelean
Since the driver should be allowed to build without OF support, the
of_match_ptr() is redundant.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/at91_adc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 7d846a2852a5..473bffe84fbd 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -1466,7 +1466,7 @@ static struct platform_driver at91_adc_driver = {
.id_table = at91_adc_ids,
.driver = {
.name = DRIVER_NAME,
- .of_match_table = of_match_ptr(at91_adc_dt_ids),
+ .of_match_table = at91_adc_dt_ids,
.pm = &at91_adc_pm_ops,
},
};
--
2.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 4/4] iio: adc: at91_adc: remove of_match_ptr() usage
@ 2020-09-30 6:00 ` Alexandru Ardelean
0 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 6:00 UTC (permalink / raw)
To: linux-iio, linux-arm-kernel
Cc: alexandre.belloni, linux-kernel, ludovic.desroches,
Alexandru Ardelean, jic23
Since the driver should be allowed to build without OF support, the
of_match_ptr() is redundant.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/at91_adc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 7d846a2852a5..473bffe84fbd 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -1466,7 +1466,7 @@ static struct platform_driver at91_adc_driver = {
.id_table = at91_adc_ids,
.driver = {
.name = DRIVER_NAME,
- .of_match_table = of_match_ptr(at91_adc_dt_ids),
+ .of_match_table = at91_adc_dt_ids,
.pm = &at91_adc_pm_ops,
},
};
--
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] 24+ messages in thread
* Re: [PATCH v2 1/4] iio: adc: at91_adc: use of_device_get_match_data() helper
2020-09-30 6:00 ` Alexandru Ardelean
@ 2020-09-30 7:30 ` Alexandre Belloni
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 7:30 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-arm-kernel, linux-kernel, jic23, nicolas.ferre,
ludovic.desroches
On 30/09/2020 09:00:05+0300, Alexandru Ardelean wrote:
> This is a small tidy-up. The of_device_get_match_data() helper retrieves
> the driver data from the OF table, without needed to explicitly know the
> table variable (since it can retrieve it from the driver object).
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.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
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/4] iio: adc: at91_adc: use of_device_get_match_data() helper
@ 2020-09-30 7:30 ` Alexandre Belloni
0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 7:30 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, ludovic.desroches, linux-arm-kernel, jic23
On 30/09/2020 09:00:05+0300, Alexandru Ardelean wrote:
> This is a small tidy-up. The of_device_get_match_data() helper retrieves
> the driver data from the OF table, without needed to explicitly know the
> table variable (since it can retrieve it from the driver object).
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.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
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH v2 2/4] iio: adc: at91_adc: const-ify some driver data
2020-09-30 6:00 ` Alexandru Ardelean
@ 2020-09-30 7:32 ` Alexandre Belloni
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 7:32 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-arm-kernel, linux-kernel, jic23, nicolas.ferre,
ludovic.desroches
On 30/09/2020 09:00:06+0300, Alexandru Ardelean 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>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.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
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/4] iio: adc: at91_adc: const-ify some driver data
@ 2020-09-30 7:32 ` Alexandre Belloni
0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 7:32 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, ludovic.desroches, linux-arm-kernel, jic23
On 30/09/2020 09:00:06+0300, Alexandru Ardelean 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>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.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
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH v2 3/4] iio: adc: at91_adc: add Kconfig dependency on the OF symbol
2020-09-30 6:00 ` Alexandru Ardelean
@ 2020-09-30 7:37 ` Alexandre Belloni
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 7:37 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-arm-kernel, linux-kernel, jic23, nicolas.ferre,
ludovic.desroches
Hi,
On 30/09/2020 09:00:07+0300, Alexandru Ardelean 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.
>
> 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.
>
Shouldn't all of that be part of the commit message for 1/4? This
doesn't explicitly explain why you add a dependency on OF.
> Fixes: 4027860dcc4c ("iio: Kconfig: at91_adc: add COMPILE_TEST dependency to driver")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> drivers/iio/adc/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 91ae90514aff..17e9ceb9c6c4 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -295,7 +295,7 @@ config ASPEED_ADC
> config AT91_ADC
> tristate "Atmel AT91 ADC"
> depends on ARCH_AT91 || COMPILE_TEST
> - depends on INPUT && SYSFS
> + depends on INPUT && SYSFS && OF
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
> help
> --
> 2.17.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/4] iio: adc: at91_adc: add Kconfig dependency on the OF symbol
@ 2020-09-30 7:37 ` Alexandre Belloni
0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 7:37 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, ludovic.desroches, linux-arm-kernel, jic23
Hi,
On 30/09/2020 09:00:07+0300, Alexandru Ardelean 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.
>
> 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.
>
Shouldn't all of that be part of the commit message for 1/4? This
doesn't explicitly explain why you add a dependency on OF.
> Fixes: 4027860dcc4c ("iio: Kconfig: at91_adc: add COMPILE_TEST dependency to driver")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> drivers/iio/adc/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 91ae90514aff..17e9ceb9c6c4 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -295,7 +295,7 @@ config ASPEED_ADC
> config AT91_ADC
> tristate "Atmel AT91 ADC"
> depends on ARCH_AT91 || COMPILE_TEST
> - depends on INPUT && SYSFS
> + depends on INPUT && SYSFS && OF
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
> help
> --
> 2.17.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH v2 4/4] iio: adc: at91_adc: remove of_match_ptr() usage
2020-09-30 6:00 ` Alexandru Ardelean
@ 2020-09-30 7:47 ` Alexandre Belloni
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 7:47 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-arm-kernel, linux-kernel, jic23, nicolas.ferre,
ludovic.desroches
Hi,
On 30/09/2020 09:00:08+0300, Alexandru Ardelean wrote:
> Since the driver should be allowed to build without OF support, the
> of_match_ptr() is redundant.
>
This can probably be squashed with the previous commit.
Also, I think that you should really make the driver DT only else, the
driver will carry dead code and there will be no reminder that a cleanup
is needed.
I can take care of that if you feel that this is more work than what you
wanted to spend on this driver.
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> drivers/iio/adc/at91_adc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 7d846a2852a5..473bffe84fbd 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -1466,7 +1466,7 @@ static struct platform_driver at91_adc_driver = {
> .id_table = at91_adc_ids,
> .driver = {
> .name = DRIVER_NAME,
> - .of_match_table = of_match_ptr(at91_adc_dt_ids),
> + .of_match_table = at91_adc_dt_ids,
> .pm = &at91_adc_pm_ops,
> },
> };
> --
> 2.17.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 4/4] iio: adc: at91_adc: remove of_match_ptr() usage
@ 2020-09-30 7:47 ` Alexandre Belloni
0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 7:47 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, ludovic.desroches, linux-arm-kernel, jic23
Hi,
On 30/09/2020 09:00:08+0300, Alexandru Ardelean wrote:
> Since the driver should be allowed to build without OF support, the
> of_match_ptr() is redundant.
>
This can probably be squashed with the previous commit.
Also, I think that you should really make the driver DT only else, the
driver will carry dead code and there will be no reminder that a cleanup
is needed.
I can take care of that if you feel that this is more work than what you
wanted to spend on this driver.
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> drivers/iio/adc/at91_adc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 7d846a2852a5..473bffe84fbd 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -1466,7 +1466,7 @@ static struct platform_driver at91_adc_driver = {
> .id_table = at91_adc_ids,
> .driver = {
> .name = DRIVER_NAME,
> - .of_match_table = of_match_ptr(at91_adc_dt_ids),
> + .of_match_table = at91_adc_dt_ids,
> .pm = &at91_adc_pm_ops,
> },
> };
> --
> 2.17.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH v2 4/4] iio: adc: at91_adc: remove of_match_ptr() usage
2020-09-30 7:47 ` Alexandre Belloni
@ 2020-09-30 8:46 ` Alexandru Ardelean
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 8:46 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alexandru Ardelean, linux-iio, linux-arm-kernel, LKML,
Jonathan Cameron, Nicolas Ferre, Ludovic Desroches
On Wed, Sep 30, 2020 at 10:48 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hi,
>
> On 30/09/2020 09:00:08+0300, Alexandru Ardelean wrote:
> > Since the driver should be allowed to build without OF support, the
> > of_match_ptr() is redundant.
> >
>
> This can probably be squashed with the previous commit.
>
> Also, I think that you should really make the driver DT only else, the
> driver will carry dead code and there will be no reminder that a cleanup
> is needed.
>
> I can take care of that if you feel that this is more work than what you
> wanted to spend on this driver.
Ah, you're saying remove the old pdata?
I can do that while waiting for other of my patches to go in.
>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/iio/adc/at91_adc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> > index 7d846a2852a5..473bffe84fbd 100644
> > --- a/drivers/iio/adc/at91_adc.c
> > +++ b/drivers/iio/adc/at91_adc.c
> > @@ -1466,7 +1466,7 @@ static struct platform_driver at91_adc_driver = {
> > .id_table = at91_adc_ids,
> > .driver = {
> > .name = DRIVER_NAME,
> > - .of_match_table = of_match_ptr(at91_adc_dt_ids),
> > + .of_match_table = at91_adc_dt_ids,
> > .pm = &at91_adc_pm_ops,
> > },
> > };
> > --
> > 2.17.1
> >
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 4/4] iio: adc: at91_adc: remove of_match_ptr() usage
@ 2020-09-30 8:46 ` Alexandru Ardelean
0 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 8:46 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-iio, LKML, Ludovic Desroches, linux-arm-kernel,
Alexandru Ardelean, Jonathan Cameron
On Wed, Sep 30, 2020 at 10:48 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hi,
>
> On 30/09/2020 09:00:08+0300, Alexandru Ardelean wrote:
> > Since the driver should be allowed to build without OF support, the
> > of_match_ptr() is redundant.
> >
>
> This can probably be squashed with the previous commit.
>
> Also, I think that you should really make the driver DT only else, the
> driver will carry dead code and there will be no reminder that a cleanup
> is needed.
>
> I can take care of that if you feel that this is more work than what you
> wanted to spend on this driver.
Ah, you're saying remove the old pdata?
I can do that while waiting for other of my patches to go in.
>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/iio/adc/at91_adc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> > index 7d846a2852a5..473bffe84fbd 100644
> > --- a/drivers/iio/adc/at91_adc.c
> > +++ b/drivers/iio/adc/at91_adc.c
> > @@ -1466,7 +1466,7 @@ static struct platform_driver at91_adc_driver = {
> > .id_table = at91_adc_ids,
> > .driver = {
> > .name = DRIVER_NAME,
> > - .of_match_table = of_match_ptr(at91_adc_dt_ids),
> > + .of_match_table = at91_adc_dt_ids,
> > .pm = &at91_adc_pm_ops,
> > },
> > };
> > --
> > 2.17.1
> >
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH v2 3/4] iio: adc: at91_adc: add Kconfig dependency on the OF symbol
2020-09-30 7:37 ` Alexandre Belloni
@ 2020-09-30 8:47 ` Alexandru Ardelean
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 8:47 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alexandru Ardelean, linux-iio, linux-arm-kernel, LKML,
Jonathan Cameron, Nicolas Ferre, Ludovic Desroches
On Wed, Sep 30, 2020 at 10:39 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hi,
>
> On 30/09/2020 09:00:07+0300, Alexandru Ardelean 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.
> >
> > 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.
> >
>
> Shouldn't all of that be part of the commit message for 1/4? This
> doesn't explicitly explain why you add a dependency on OF.
Hmm, I think I did this patch before drinking my morning coffee.
Looks like it's also a bad commit description.
>
> > Fixes: 4027860dcc4c ("iio: Kconfig: at91_adc: add COMPILE_TEST dependency to driver")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/iio/adc/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 91ae90514aff..17e9ceb9c6c4 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -295,7 +295,7 @@ config ASPEED_ADC
> > config AT91_ADC
> > tristate "Atmel AT91 ADC"
> > depends on ARCH_AT91 || COMPILE_TEST
> > - depends on INPUT && SYSFS
> > + depends on INPUT && SYSFS && OF
> > select IIO_BUFFER
> > select IIO_TRIGGERED_BUFFER
> > help
> > --
> > 2.17.1
> >
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/4] iio: adc: at91_adc: add Kconfig dependency on the OF symbol
@ 2020-09-30 8:47 ` Alexandru Ardelean
0 siblings, 0 replies; 24+ messages in thread
From: Alexandru Ardelean @ 2020-09-30 8:47 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-iio, LKML, Ludovic Desroches, linux-arm-kernel,
Alexandru Ardelean, Jonathan Cameron
On Wed, Sep 30, 2020 at 10:39 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hi,
>
> On 30/09/2020 09:00:07+0300, Alexandru Ardelean 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.
> >
> > 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.
> >
>
> Shouldn't all of that be part of the commit message for 1/4? This
> doesn't explicitly explain why you add a dependency on OF.
Hmm, I think I did this patch before drinking my morning coffee.
Looks like it's also a bad commit description.
>
> > Fixes: 4027860dcc4c ("iio: Kconfig: at91_adc: add COMPILE_TEST dependency to driver")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/iio/adc/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 91ae90514aff..17e9ceb9c6c4 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -295,7 +295,7 @@ config ASPEED_ADC
> > config AT91_ADC
> > tristate "Atmel AT91 ADC"
> > depends on ARCH_AT91 || COMPILE_TEST
> > - depends on INPUT && SYSFS
> > + depends on INPUT && SYSFS && OF
> > select IIO_BUFFER
> > select IIO_TRIGGERED_BUFFER
> > help
> > --
> > 2.17.1
> >
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH v2 4/4] iio: adc: at91_adc: remove of_match_ptr() usage
2020-09-30 8:46 ` Alexandru Ardelean
@ 2020-09-30 9:14 ` Alexandre Belloni
-1 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 9:14 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: Alexandru Ardelean, linux-iio, linux-arm-kernel, LKML,
Jonathan Cameron, Nicolas Ferre, Ludovic Desroches
On 30/09/2020 11:46:06+0300, Alexandru Ardelean wrote:
> On Wed, Sep 30, 2020 at 10:48 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> >
> > Hi,
> >
> > On 30/09/2020 09:00:08+0300, Alexandru Ardelean wrote:
> > > Since the driver should be allowed to build without OF support, the
> > > of_match_ptr() is redundant.
> > >
> >
> > This can probably be squashed with the previous commit.
> >
> > Also, I think that you should really make the driver DT only else, the
> > driver will carry dead code and there will be no reminder that a cleanup
> > is needed.
> >
> > I can take care of that if you feel that this is more work than what you
> > wanted to spend on this driver.
>
> Ah, you're saying remove the old pdata?
Yes, since you are making the driver DT only, the pdata has to be
removed, don't forget to also remove the header file.
> I can do that while waiting for other of my patches to go in.
>
> >
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > ---
> > > drivers/iio/adc/at91_adc.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> > > index 7d846a2852a5..473bffe84fbd 100644
> > > --- a/drivers/iio/adc/at91_adc.c
> > > +++ b/drivers/iio/adc/at91_adc.c
> > > @@ -1466,7 +1466,7 @@ static struct platform_driver at91_adc_driver = {
> > > .id_table = at91_adc_ids,
> > > .driver = {
> > > .name = DRIVER_NAME,
> > > - .of_match_table = of_match_ptr(at91_adc_dt_ids),
> > > + .of_match_table = at91_adc_dt_ids,
> > > .pm = &at91_adc_pm_ops,
> > > },
> > > };
> > > --
> > > 2.17.1
> > >
> >
> > --
> > Alexandre Belloni, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 4/4] iio: adc: at91_adc: remove of_match_ptr() usage
@ 2020-09-30 9:14 ` Alexandre Belloni
0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2020-09-30 9:14 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, LKML, Ludovic Desroches, linux-arm-kernel,
Alexandru Ardelean, Jonathan Cameron
On 30/09/2020 11:46:06+0300, Alexandru Ardelean wrote:
> On Wed, Sep 30, 2020 at 10:48 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> >
> > Hi,
> >
> > On 30/09/2020 09:00:08+0300, Alexandru Ardelean wrote:
> > > Since the driver should be allowed to build without OF support, the
> > > of_match_ptr() is redundant.
> > >
> >
> > This can probably be squashed with the previous commit.
> >
> > Also, I think that you should really make the driver DT only else, the
> > driver will carry dead code and there will be no reminder that a cleanup
> > is needed.
> >
> > I can take care of that if you feel that this is more work than what you
> > wanted to spend on this driver.
>
> Ah, you're saying remove the old pdata?
Yes, since you are making the driver DT only, the pdata has to be
removed, don't forget to also remove the header file.
> I can do that while waiting for other of my patches to go in.
>
> >
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > ---
> > > drivers/iio/adc/at91_adc.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> > > index 7d846a2852a5..473bffe84fbd 100644
> > > --- a/drivers/iio/adc/at91_adc.c
> > > +++ b/drivers/iio/adc/at91_adc.c
> > > @@ -1466,7 +1466,7 @@ static struct platform_driver at91_adc_driver = {
> > > .id_table = at91_adc_ids,
> > > .driver = {
> > > .name = DRIVER_NAME,
> > > - .of_match_table = of_match_ptr(at91_adc_dt_ids),
> > > + .of_match_table = at91_adc_dt_ids,
> > > .pm = &at91_adc_pm_ops,
> > > },
> > > };
> > > --
> > > 2.17.1
> > >
> >
> > --
> > Alexandre Belloni, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
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] 24+ messages in thread
end of thread, other threads:[~2020-09-30 9:17 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-30 6:00 [PATCH v2 0/4] iio: adc: at91: misc driver cleanups Alexandru Ardelean
2020-09-30 6:00 ` Alexandru Ardelean
2020-09-30 6:00 ` [PATCH v2 1/4] iio: adc: at91_adc: use of_device_get_match_data() helper Alexandru Ardelean
2020-09-30 6:00 ` Alexandru Ardelean
2020-09-30 7:30 ` Alexandre Belloni
2020-09-30 7:30 ` Alexandre Belloni
2020-09-30 6:00 ` [PATCH v2 2/4] iio: adc: at91_adc: const-ify some driver data Alexandru Ardelean
2020-09-30 6:00 ` Alexandru Ardelean
2020-09-30 7:32 ` Alexandre Belloni
2020-09-30 7:32 ` Alexandre Belloni
2020-09-30 6:00 ` [PATCH v2 3/4] iio: adc: at91_adc: add Kconfig dependency on the OF symbol Alexandru Ardelean
2020-09-30 6:00 ` Alexandru Ardelean
2020-09-30 7:37 ` Alexandre Belloni
2020-09-30 7:37 ` Alexandre Belloni
2020-09-30 8:47 ` Alexandru Ardelean
2020-09-30 8:47 ` Alexandru Ardelean
2020-09-30 6:00 ` [PATCH v2 4/4] iio: adc: at91_adc: remove of_match_ptr() usage Alexandru Ardelean
2020-09-30 6:00 ` Alexandru Ardelean
2020-09-30 7:47 ` Alexandre Belloni
2020-09-30 7:47 ` Alexandre Belloni
2020-09-30 8:46 ` Alexandru Ardelean
2020-09-30 8:46 ` Alexandru Ardelean
2020-09-30 9:14 ` Alexandre Belloni
2020-09-30 9:14 ` Alexandre Belloni
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.