| Message ID | 20240606-spi-match-data-v1-1-320b291ee1fe@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: simplify with spi_get_device_match_data() | expand |
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á
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
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 >
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]);
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(-)