[PATCH 0/8] iio: simplify with spi_get_device_match_data()

[PATCH 0/8] iio: simplify with spi_get_device_match_data()

[Krzysztof Kozlowski]

Not tested on hardware, but code looks equivalent and should not have
functional effect.

Best regards,
Krzysztof

---
Krzysztof Kozlowski (8):
      iio: accel: adxl313: simplify with spi_get_device_match_data()
      iio: accel: adxl355: simplify with spi_get_device_match_data()
      iio: adc: max11205: simplify with spi_get_device_match_data()
      iio: adc: ti-ads131e08: simplify with spi_get_device_match_data()
      iio: adc: ti-tsc2046: simplify with spi_get_device_match_data()
      iio: addac: ad74413r: simplify with spi_get_device_match_data()
      iio: dac: max5522: simplify with spi_get_device_match_data()
      iio: adc: mcp3564: drop redundant open-coded spi_get_device_match_data()

 drivers/iio/accel/adxl313_spi.c |  8 +-------
 drivers/iio/accel/adxl355_spi.c | 10 +++-------
 drivers/iio/adc/max11205.c      |  5 +----
 drivers/iio/adc/mcp3564.c       |  6 ------
 drivers/iio/adc/ti-ads131e08.c  |  4 +---
 drivers/iio/adc/ti-tsc2046.c    |  7 +------
 drivers/iio/addac/ad74413r.c    | 13 +++----------
 drivers/iio/dac/max5522.c       | 11 +++--------
 8 files changed, 13 insertions(+), 51 deletions(-)
---
base-commit: ed3dab9323648a17a85908c574787be12d4cc871
change-id: 20240606-spi-match-data-6bf7c51ba913

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

[PATCH 1/8] iio: accel: adxl313: simplify with spi_get_device_match_data()

[Krzysztof Kozlowski]

Use spi_get_device_match_data() helper to simplify a bit the driver.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/accel/adxl313_spi.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c
index b7cc15678a2b..6f8d73f6e5a9 100644
--- a/drivers/iio/accel/adxl313_spi.c
+++ b/drivers/iio/accel/adxl313_spi.c
@@ -72,13 +72,7 @@ static int adxl313_spi_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	/*
-	 * Retrieves device specific data as a pointer to a
-	 * adxl313_chip_info structure
-	 */
-	chip_data = device_get_match_data(&spi->dev);
-	if (!chip_data)
-		chip_data = (const struct adxl313_chip_info *)spi_get_device_id(spi)->driver_data;
+	chip_data = spi_get_device_match_data(spi);
 
 	regmap = devm_regmap_init_spi(spi,
 				      &adxl31x_spi_regmap_config[chip_data->type]);

-- 
2.43.0

Re: [PATCH 1/8] iio: accel: adxl313: simplify with spi_get_device_match_data()

[Nuno Sá]

On Thu, 2024-06-06 at 16:26 +0200, Krzysztof Kozlowski wrote:
> Use spi_get_device_match_data() helper to simplify a bit the driver.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/iio/accel/adxl313_spi.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c
> index b7cc15678a2b..6f8d73f6e5a9 100644
> --- a/drivers/iio/accel/adxl313_spi.c
> +++ b/drivers/iio/accel/adxl313_spi.c
> @@ -72,13 +72,7 @@ static int adxl313_spi_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	/*
> -	 * Retrieves device specific data as a pointer to a
> -	 * adxl313_chip_info structure
> -	 */
> -	chip_data = device_get_match_data(&spi->dev);
> -	if (!chip_data)
> -		chip_data = (const struct adxl313_chip_info
> *)spi_get_device_id(spi)->driver_data;
> +	chip_data = spi_get_device_match_data(spi);
>  

I understand you're sticking with the original code but since you're doing this,
could we maybe add proper error checking for the call? Maybe Jonathan can even
tweak that while applying...

(same comment for patch 3)

- Nuno Sá

Re: [PATCH 1/8] iio: accel: adxl313: simplify with spi_get_device_match_data()

[Krzysztof Kozlowski]

On 07/06/2024 10:57, Nuno Sá wrote:
> On Thu, 2024-06-06 at 16:26 +0200, Krzysztof Kozlowski wrote:
>> Use spi_get_device_match_data() helper to simplify a bit the driver.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  drivers/iio/accel/adxl313_spi.c | 8 +-------
>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c
>> index b7cc15678a2b..6f8d73f6e5a9 100644
>> --- a/drivers/iio/accel/adxl313_spi.c
>> +++ b/drivers/iio/accel/adxl313_spi.c
>> @@ -72,13 +72,7 @@ static int adxl313_spi_probe(struct spi_device *spi)
>>  	if (ret)
>>  		return ret;
>>  
>> -	/*
>> -	 * Retrieves device specific data as a pointer to a
>> -	 * adxl313_chip_info structure
>> -	 */
>> -	chip_data = device_get_match_data(&spi->dev);
>> -	if (!chip_data)
>> -		chip_data = (const struct adxl313_chip_info
>> *)spi_get_device_id(spi)->driver_data;
>> +	chip_data = spi_get_device_match_data(spi);
>>  
> 
> I understand you're sticking with the original code but since you're doing this,
> could we maybe add proper error checking for the call? Maybe Jonathan can even
> tweak that while applying...
> 
> (same comment for patch 3)

I consider that a separate patch/work, because it would have functional
impact.

Best regards,
Krzysztof

Re: [PATCH 1/8] iio: accel: adxl313: simplify with spi_get_device_match_data()

[Jonathan Cameron]

On Fri, 7 Jun 2024 11:18:54 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 07/06/2024 10:57, Nuno Sá wrote:
> > On Thu, 2024-06-06 at 16:26 +0200, Krzysztof Kozlowski wrote:  
> >> Use spi_get_device_match_data() helper to simplify a bit the driver.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >>  drivers/iio/accel/adxl313_spi.c | 8 +-------
> >>  1 file changed, 1 insertion(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c
> >> index b7cc15678a2b..6f8d73f6e5a9 100644
> >> --- a/drivers/iio/accel/adxl313_spi.c
> >> +++ b/drivers/iio/accel/adxl313_spi.c
> >> @@ -72,13 +72,7 @@ static int adxl313_spi_probe(struct spi_device *spi)
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> -	/*
> >> -	 * Retrieves device specific data as a pointer to a
> >> -	 * adxl313_chip_info structure
> >> -	 */
> >> -	chip_data = device_get_match_data(&spi->dev);
> >> -	if (!chip_data)
> >> -		chip_data = (const struct adxl313_chip_info
> >> *)spi_get_device_id(spi)->driver_data;
> >> +	chip_data = spi_get_device_match_data(spi);
> >>    
> > 
> > I understand you're sticking with the original code but since you're doing this,
> > could we maybe add proper error checking for the call? Maybe Jonathan can even
> > tweak that while applying...
> > 
> > (same comment for patch 3)  
> 
> I consider that a separate patch/work, because it would have functional
> impact.

Agreed. Though error checking on these is normally paranoia / readability thing
as we probed from some firmware match and all those entries are present, so
it should just work.


> 
> Best regards,
> Krzysztof
> 

[PATCH 2/8] iio: accel: adxl355: simplify with spi_get_device_match_data()

[Krzysztof Kozlowski]

Use spi_get_device_match_data() helper to simplify a bit the driver.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/accel/adxl355_spi.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/adxl355_spi.c b/drivers/iio/accel/adxl355_spi.c
index fc99534d91ff..5153ac815e4b 100644
--- a/drivers/iio/accel/adxl355_spi.c
+++ b/drivers/iio/accel/adxl355_spi.c
@@ -28,13 +28,9 @@ static int adxl355_spi_probe(struct spi_device *spi)
 	const struct adxl355_chip_info *chip_data;
 	struct regmap *regmap;
 
-	chip_data = device_get_match_data(&spi->dev);
-	if (!chip_data) {
-		chip_data = (void *)spi_get_device_id(spi)->driver_data;
-
-		if (!chip_data)
-			return -EINVAL;
-	}
+	chip_data = spi_get_device_match_data(spi);
+	if (!chip_data)
+		return -EINVAL;
 
 	regmap = devm_regmap_init_spi(spi, &adxl355_spi_regmap_config);
 	if (IS_ERR(regmap)) {

-- 
2.43.0

[PATCH 3/8] iio: adc: max11205: simplify with spi_get_device_match_data()

[Krzysztof Kozlowski]

Use spi_get_device_match_data() helper to simplify a bit the driver.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/adc/max11205.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iio/adc/max11205.c b/drivers/iio/adc/max11205.c
index 65fc32971ba5..9d8bc0b154dd 100644
--- a/drivers/iio/adc/max11205.c
+++ b/drivers/iio/adc/max11205.c
@@ -116,10 +116,7 @@ static int max11205_probe(struct spi_device *spi)
 
 	ad_sd_init(&st->sd, indio_dev, spi, &max11205_sigma_delta_info);
 
-	st->chip_info = device_get_match_data(&spi->dev);
-	if (!st->chip_info)
-		st->chip_info =
-			(const struct max11205_chip_info *)spi_get_device_id(spi)->driver_data;
+	st->chip_info = spi_get_device_match_data(spi);
 
 	indio_dev->name = st->chip_info->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;

-- 
2.43.0

[PATCH 4/8] iio: adc: ti-ads131e08: simplify with spi_get_device_match_data()

[Krzysztof Kozlowski]

Use spi_get_device_match_data() helper to simplify a bit the driver.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/adc/ti-ads131e08.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ti-ads131e08.c b/drivers/iio/adc/ti-ads131e08.c
index cb04a29b3dba..96dd9366f8ff 100644
--- a/drivers/iio/adc/ti-ads131e08.c
+++ b/drivers/iio/adc/ti-ads131e08.c
@@ -802,9 +802,7 @@ static int ads131e08_probe(struct spi_device *spi)
 	unsigned long adc_clk_ns;
 	int ret;
 
-	info = device_get_match_data(&spi->dev);
-	if (!info)
-		info = (void *)spi_get_device_id(spi)->driver_data;
+	info = spi_get_device_match_data(spi);
 	if (!info) {
 		dev_err(&spi->dev, "failed to get match data\n");
 		return -ENODEV;

-- 
2.43.0

[PATCH 5/8] iio: adc: ti-tsc2046: simplify with spi_get_device_match_data()

[Krzysztof Kozlowski]

Use spi_get_device_match_data() helper to simplify a bit the driver.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/adc/ti-tsc2046.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
index 1bbb51a6683c..edcef8f11522 100644
--- a/drivers/iio/adc/ti-tsc2046.c
+++ b/drivers/iio/adc/ti-tsc2046.c
@@ -804,12 +804,7 @@ static int tsc2046_adc_probe(struct spi_device *spi)
 		return -EINVAL;
 	}
 
-	dcfg = device_get_match_data(dev);
-	if (!dcfg) {
-		const struct spi_device_id *id = spi_get_device_id(spi);
-
-		dcfg = (const struct tsc2046_adc_dcfg *)id->driver_data;
-	}
+	dcfg = spi_get_device_match_data(spi);
 	if (!dcfg)
 		return -EINVAL;
 

-- 
2.43.0

[PATCH 6/8] iio: addac: ad74413r: simplify with spi_get_device_match_data()

[Krzysztof Kozlowski]

Use spi_get_device_match_data() helper to simplify a bit the driver.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/addac/ad74413r.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index cd26a16dc0ff..2410d72da49b 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -1365,16 +1365,9 @@ static int ad74413r_probe(struct spi_device *spi)
 
 	st->spi = spi;
 	st->dev = &spi->dev;
-	st->chip_info = device_get_match_data(&spi->dev);
-	if (!st->chip_info) {
-		const struct spi_device_id *id = spi_get_device_id(spi);
-
-		if (id)
-			st->chip_info =
-				(struct ad74413r_chip_info *)id->driver_data;
-		if (!st->chip_info)
-			return -EINVAL;
-	}
+	st->chip_info = spi_get_device_match_data(spi);
+	if (!st->chip_info)
+		return -EINVAL;
 
 	mutex_init(&st->lock);
 	init_completion(&st->adc_data_completion);

-- 
2.43.0

[PATCH 7/8] iio: dac: max5522: simplify with spi_get_device_match_data()

[Krzysztof Kozlowski]

Use spi_get_device_match_data() helper to simplify a bit the driver.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/dac/max5522.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/dac/max5522.c b/drivers/iio/dac/max5522.c
index 05034a306597..9f72155dcbc7 100644
--- a/drivers/iio/dac/max5522.c
+++ b/drivers/iio/dac/max5522.c
@@ -132,7 +132,6 @@ static const struct regmap_config max5522_regmap_config = {
 
 static int max5522_spi_probe(struct spi_device *spi)
 {
-	const struct spi_device_id *id = spi_get_device_id(spi);
 	struct iio_dev *indio_dev;
 	struct max5522_state *state;
 	int ret;
@@ -144,13 +143,9 @@ static int max5522_spi_probe(struct spi_device *spi)
 	}
 
 	state = iio_priv(indio_dev);
-	state->chip_info = device_get_match_data(&spi->dev);
-	if (!state->chip_info) {
-		state->chip_info =
-			(struct max5522_chip_info *)(id->driver_data);
-		if (!state->chip_info)
-			return -EINVAL;
-	}
+	state->chip_info = spi_get_device_match_data(spi);
+	if (!state->chip_info)
+		return -EINVAL;
 
 	state->vrefin_reg = devm_regulator_get(&spi->dev, "vrefin");
 	if (IS_ERR(state->vrefin_reg))

-- 
2.43.0

[PATCH 8/8] iio: adc: mcp3564: drop redundant open-coded spi_get_device_match_data()

[Krzysztof Kozlowski]

The driver calls spi_get_device_match_data() and on its failure calls
again parts of it: spi_get_device_id() and getting driver data.  This is
entirely redundant, because it is part of spi_get_device_match_data().

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/adc/mcp3564.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/iio/adc/mcp3564.c b/drivers/iio/adc/mcp3564.c
index e2ae13f1e842..d83bed0e63d2 100644
--- a/drivers/iio/adc/mcp3564.c
+++ b/drivers/iio/adc/mcp3564.c
@@ -1114,7 +1114,6 @@ static int mcp3564_config(struct iio_dev *indio_dev)
 {
 	struct mcp3564_state *adc = iio_priv(indio_dev);
 	struct device *dev = &adc->spi->dev;
-	const struct spi_device_id *dev_id;
 	u8 tmp_reg;
 	u16 tmp_u16;
 	enum mcp3564_ids ids;
@@ -1212,11 +1211,6 @@ static int mcp3564_config(struct iio_dev *indio_dev)
 		 * try using fallback compatible in device tree to deal with some newer part number.
 		 */
 		adc->chip_info = spi_get_device_match_data(adc->spi);
-		if (!adc->chip_info) {
-			dev_id = spi_get_device_id(adc->spi);
-			adc->chip_info = (const struct mcp3564_chip_info *)dev_id->driver_data;
-		}
-
 		adc->have_vref = adc->chip_info->have_vref;
 	} else {
 		adc->chip_info = &mcp3564_chip_infos_tbl[ids];

-- 
2.43.0

Re: [PATCH 0/8] iio: simplify with spi_get_device_match_data()

[Nuno Sá]

On Thu, 2024-06-06 at 16:26 +0200, Krzysztof Kozlowski wrote:
> Not tested on hardware, but code looks equivalent and should not have
> functional effect.
> 
> Best regards,
> Krzysztof
> 
> ---
> Krzysztof Kozlowski (8):
>       iio: accel: adxl313: simplify with spi_get_device_match_data()
>       iio: accel: adxl355: simplify with spi_get_device_match_data()
>       iio: adc: max11205: simplify with spi_get_device_match_data()
>       iio: adc: ti-ads131e08: simplify with spi_get_device_match_data()
>       iio: adc: ti-tsc2046: simplify with spi_get_device_match_data()
>       iio: addac: ad74413r: simplify with spi_get_device_match_data()
>       iio: dac: max5522: simplify with spi_get_device_match_data()
>       iio: adc: mcp3564: drop redundant open-coded spi_get_device_match_data()
> 
>  drivers/iio/accel/adxl313_spi.c |  8 +-------
>  drivers/iio/accel/adxl355_spi.c | 10 +++-------
>  drivers/iio/adc/max11205.c      |  5 +----
>  drivers/iio/adc/mcp3564.c       |  6 ------
>  drivers/iio/adc/ti-ads131e08.c  |  4 +---
>  drivers/iio/adc/ti-tsc2046.c    |  7 +------
>  drivers/iio/addac/ad74413r.c    | 13 +++----------
>  drivers/iio/dac/max5522.c       | 11 +++--------
>  8 files changed, 13 insertions(+), 51 deletions(-)
> ---

LGTM, just some minor comments/asks that don't really stop me from:

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

Re: [PATCH 0/8] iio: simplify with spi_get_device_match_data()

[Jonathan Cameron]

On Fri, 07 Jun 2024 10:55:44 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, 2024-06-06 at 16:26 +0200, Krzysztof Kozlowski wrote:
> > Not tested on hardware, but code looks equivalent and should not have
> > functional effect.
> > 
> > Best regards,
> > Krzysztof
> > 
> > ---
> > Krzysztof Kozlowski (8):
> >       iio: accel: adxl313: simplify with spi_get_device_match_data()
> >       iio: accel: adxl355: simplify with spi_get_device_match_data()
> >       iio: adc: max11205: simplify with spi_get_device_match_data()
> >       iio: adc: ti-ads131e08: simplify with spi_get_device_match_data()
> >       iio: adc: ti-tsc2046: simplify with spi_get_device_match_data()
> >       iio: addac: ad74413r: simplify with spi_get_device_match_data()
> >       iio: dac: max5522: simplify with spi_get_device_match_data()
> >       iio: adc: mcp3564: drop redundant open-coded spi_get_device_match_data()
> > 
> >  drivers/iio/accel/adxl313_spi.c |  8 +-------
> >  drivers/iio/accel/adxl355_spi.c | 10 +++-------
> >  drivers/iio/adc/max11205.c      |  5 +----
> >  drivers/iio/adc/mcp3564.c       |  6 ------
> >  drivers/iio/adc/ti-ads131e08.c  |  4 +---
> >  drivers/iio/adc/ti-tsc2046.c    |  7 +------
> >  drivers/iio/addac/ad74413r.c    | 13 +++----------
> >  drivers/iio/dac/max5522.c       | 11 +++--------
> >  8 files changed, 13 insertions(+), 51 deletions(-)
> > ---  
> 
> LGTM, just some minor comments/asks that don't really stop me from:
> 
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> 
> 

Applied.

Thanks for tidying these up.

Jonathan