iio: simplify with spi_get_device_match_data()

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

Posted by Krzysztof Kozlowski 1 year, 8 months ago

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()

Posted by Nuno Sá 1 year, 8 months ago

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()

Posted by Krzysztof Kozlowski 1 year, 8 months ago

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()

Posted by Jonathan Cameron 1 year, 8 months ago

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
>