diff mbox series

iio: adc128s052: add proper .data members in adc128_of_match table

Message ID 20221115132324.1078169-1-linux@rasmusvillemoes.dk
State New
Headers
Series iio: adc128s052: add proper .data members in adc128_of_match table |

Commit Message

Rasmus Villemoes Nov. 15, 2022, 1:23 p.m. UTC 
  Prior to commit bd5d54e4d49d ("iio: adc128s052: add ACPI _HID
AANT1280"), the driver unconditionally used spi_get_device_id() to get
the index into the adc128_config array.

However, with that commit, OF-based boards now incorrectly treat all
supported sensors as if they are an adc128s052, because all the .data
members of the adc128_of_match table are implicitly 0. Our board,
which has an adc122s021, thus exposes 8 channels whereas it really
only has two.

Fixes: bd5d54e4d49d ("iio: adc128s052: add ACPI _HID AANT1280")
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---

I think the driver could be simplified somewhat by having just one
single adc_channels[] array with the 8 entries, unconditionally use
that as ->channels, with ->num_channels being taken from the match
data instead of having this indirection through the auxiliary config
array.

But this patch is properly more suited for -stable.

 drivers/iio/adc/ti-adc128s052.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Andy Shevchenko Nov. 15, 2022, 1:35 p.m. UTC | #1 
On Tue, Nov 15, 2022 at 02:23:23PM +0100, Rasmus Villemoes wrote:
> Prior to commit bd5d54e4d49d ("iio: adc128s052: add ACPI _HID
> AANT1280"), the driver unconditionally used spi_get_device_id() to get
> the index into the adc128_config array.
> 
> However, with that commit, OF-based boards now incorrectly treat all
> supported sensors as if they are an adc128s052, because all the .data
> members of the adc128_of_match table are implicitly 0. Our board,
> which has an adc122s021, thus exposes 8 channels whereas it really
> only has two.
> 
> Fixes: bd5d54e4d49d ("iio: adc128s052: add ACPI _HID AANT1280")
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> 
> I think the driver could be simplified somewhat by having just one
> single adc_channels[] array with the 8 entries, unconditionally use
> that as ->channels, with ->num_channels being taken from the match
> data instead of having this indirection through the auxiliary config
> array.

Hmm... I have a patch locally that changes this to take pointers instead of
numbers and using spi_get_device_match_data() (but the latter is only available
in Linux Next so far).

> But this patch is properly more suited for -stable.

LGTM for time being
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

>  drivers/iio/adc/ti-adc128s052.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> index 622fd384983c..b3d5b9b7255b 100644
> --- a/drivers/iio/adc/ti-adc128s052.c
> +++ b/drivers/iio/adc/ti-adc128s052.c
> @@ -181,13 +181,13 @@ static int adc128_probe(struct spi_device *spi)
>  }
>  
>  static const struct of_device_id adc128_of_match[] = {
> -	{ .compatible = "ti,adc128s052", },
> -	{ .compatible = "ti,adc122s021", },
> -	{ .compatible = "ti,adc122s051", },
> -	{ .compatible = "ti,adc122s101", },
> -	{ .compatible = "ti,adc124s021", },
> -	{ .compatible = "ti,adc124s051", },
> -	{ .compatible = "ti,adc124s101", },
> +	{ .compatible = "ti,adc128s052", .data = (void*)0L, },
> +	{ .compatible = "ti,adc122s021", .data = (void*)1L, },
> +	{ .compatible = "ti,adc122s051", .data = (void*)1L, },
> +	{ .compatible = "ti,adc122s101", .data = (void*)1L, },
> +	{ .compatible = "ti,adc124s021", .data = (void*)2L, },
> +	{ .compatible = "ti,adc124s051", .data = (void*)2L, },
> +	{ .compatible = "ti,adc124s101", .data = (void*)2L, },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, adc128_of_match);
> -- 
> 2.37.2
>
Rasmus Villemoes Nov. 22, 2022, 2:41 p.m. UTC | #2 
On 15/11/2022 14.23, Rasmus Villemoes wrote:
> Prior to commit bd5d54e4d49d ("iio: adc128s052: add ACPI _HID
> AANT1280"), the driver unconditionally used spi_get_device_id() to get
> the index into the adc128_config array.
> 
> However, with that commit, OF-based boards now incorrectly treat all
> supported sensors as if they are an adc128s052, because all the .data
> members of the adc128_of_match table are implicitly 0. Our board,
> which has an adc122s021, thus exposes 8 channels whereas it really
> only has two.
> 
> Fixes: bd5d54e4d49d ("iio: adc128s052: add ACPI _HID AANT1280")
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---

Ping. Any chance this could be picked up before the merge window for 6.2
opens?
Jonathan Cameron Nov. 23, 2022, 9 p.m. UTC | #3 
On Tue, 22 Nov 2022 15:41:21 +0100
Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> On 15/11/2022 14.23, Rasmus Villemoes wrote:
> > Prior to commit bd5d54e4d49d ("iio: adc128s052: add ACPI _HID
> > AANT1280"), the driver unconditionally used spi_get_device_id() to get
> > the index into the adc128_config array.
> > 
> > However, with that commit, OF-based boards now incorrectly treat all
> > supported sensors as if they are an adc128s052, because all the .data
> > members of the adc128_of_match table are implicitly 0. Our board,
> > which has an adc122s021, thus exposes 8 channels whereas it really
> > only has two.
> > 
> > Fixes: bd5d54e4d49d ("iio: adc128s052: add ACPI _HID AANT1280")
> > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > ---  
> 
> Ping. Any chance this could be picked up before the merge window for 6.2
> opens?

Given it is a clear fix, but for an issue that is multiple versions old
(so I'm not sneaking it in post rc6)..
No actual rush on this, but meh I'm queuing a bunch of other stuff that
will hopefully just make it this cycle so applied to the togreg branch of iio.git
and marked for stable.

Thanks,

Jonathan
diff mbox series

Patch

diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
index 622fd384983c..b3d5b9b7255b 100644
--- a/drivers/iio/adc/ti-adc128s052.c
+++ b/drivers/iio/adc/ti-adc128s052.c
@@ -181,13 +181,13 @@  static int adc128_probe(struct spi_device *spi)
 }
 
 static const struct of_device_id adc128_of_match[] = {
-	{ .compatible = "ti,adc128s052", },
-	{ .compatible = "ti,adc122s021", },
-	{ .compatible = "ti,adc122s051", },
-	{ .compatible = "ti,adc122s101", },
-	{ .compatible = "ti,adc124s021", },
-	{ .compatible = "ti,adc124s051", },
-	{ .compatible = "ti,adc124s101", },
+	{ .compatible = "ti,adc128s052", .data = (void*)0L, },
+	{ .compatible = "ti,adc122s021", .data = (void*)1L, },
+	{ .compatible = "ti,adc122s051", .data = (void*)1L, },
+	{ .compatible = "ti,adc122s101", .data = (void*)1L, },
+	{ .compatible = "ti,adc124s021", .data = (void*)2L, },
+	{ .compatible = "ti,adc124s051", .data = (void*)2L, },
+	{ .compatible = "ti,adc124s101", .data = (void*)2L, },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, adc128_of_match);