[PATCH v2 05/10] iio: adc: ad7476: Limit the scope of the chip_info

Matti Vaittinen posted 10 patches 4 months, 1 week ago
There is a newer version of this series
[PATCH v2 05/10] iio: adc: ad7476: Limit the scope of the chip_info
Posted by Matti Vaittinen 4 months, 1 week ago
The chip_info structure is not required to be accessed after probe.

Remove the chip_info pointer from the driver data to reduce the scope
and to make driver clearer.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
 v1 => v2:
 - New patch
---
 drivers/iio/adc/ad7476.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index fc701267358e..e97742912b8e 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -38,7 +38,6 @@ struct ad7476_chip_info {
 
 struct ad7476_state {
 	struct spi_device		*spi;
-	const struct ad7476_chip_info	*chip_info;
 	struct gpio_desc		*convst_gpio;
 	struct spi_transfer		xfer;
 	struct spi_message		msg;
@@ -280,6 +279,7 @@ static const struct iio_info ad7476_info = {
 
 static int ad7476_probe(struct spi_device *spi)
 {
+	const struct ad7476_chip_info *chip_info;
 	struct ad7476_state *st;
 	struct iio_dev *indio_dev;
 	int ret;
@@ -290,12 +290,12 @@ static int ad7476_probe(struct spi_device *spi)
 
 	st = iio_priv(indio_dev);
 
-	st->chip_info = spi_get_device_match_data(spi);
-	if (!st->chip_info)
+	chip_info = spi_get_device_match_data(spi);
+	if (!chip_info)
 		return -ENODEV;
 
 	/* Use VCC for reference voltage if vref / internal vref aren't used */
-	if (!st->chip_info->int_vref_mv && !st->chip_info->has_vref) {
+	if (!chip_info->int_vref_mv && !chip_info->has_vref) {
 		ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vcc");
 		if (ret < 0)
 			return ret;
@@ -306,11 +306,11 @@ static int ad7476_probe(struct spi_device *spi)
 			return ret;
 	}
 
-	if (st->chip_info->has_vref) {
+	if (chip_info->has_vref) {
 		ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
 		if (ret < 0) {
 			/* Vref is optional if a device has an internal reference */
-			if (!st->chip_info->int_vref_mv || ret != -ENODEV)
+			if (!chip_info->int_vref_mv || ret != -ENODEV)
 				return ret;
 		} else {
 			st->scale_mv = ret / 1000;
@@ -318,9 +318,9 @@ static int ad7476_probe(struct spi_device *spi)
 	}
 
 	if (!st->scale_mv)
-		st->scale_mv = st->chip_info->int_vref_mv;
+		st->scale_mv = chip_info->int_vref_mv;
 
-	if (st->chip_info->has_vdrive) {
+	if (chip_info->has_vdrive) {
 		ret = devm_regulator_get_enable(&spi->dev, "vdrive");
 		if (ret)
 			return ret;
@@ -336,12 +336,12 @@ static int ad7476_probe(struct spi_device *spi)
 
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = st->chip_info->channel;
+	indio_dev->channels = chip_info->channel;
 	indio_dev->num_channels = 2;
 	indio_dev->info = &ad7476_info;
 
 	if (st->convst_gpio)
-		indio_dev->channels = st->chip_info->convst_channel;
+		indio_dev->channels = chip_info->convst_channel;
 	/* Setup default message */
 
 	st->xfer.rx_buf = &st->data;
@@ -355,8 +355,8 @@ static int ad7476_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	if (st->chip_info->reset)
-		st->chip_info->reset(st);
+	if (chip_info->reset)
+		chip_info->reset(st);
 
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
-- 
2.50.1

Re: [PATCH v2 05/10] iio: adc: ad7476: Limit the scope of the chip_info
Posted by Andy Shevchenko 4 months, 1 week ago
On Thu, Aug 7, 2025 at 11:34 AM Matti Vaittinen
<mazziesaccount@gmail.com> wrote:
>
> The chip_info structure is not required to be accessed after probe.
>
> Remove the chip_info pointer from the driver data to reduce the scope
> and to make driver clearer.

the driver

clearer or cleaner? I think you want the latter...

...

Not sure how the future of the development of this driver will look
like, but it might be this patch will be reverted if one wants
something else from chip_info to have a longer lifetime.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 05/10] iio: adc: ad7476: Limit the scope of the chip_info
Posted by Matti Vaittinen 4 months, 1 week ago
On 08/08/2025 00:12, Andy Shevchenko wrote:
> On Thu, Aug 7, 2025 at 11:34 AM Matti Vaittinen
> <mazziesaccount@gmail.com> wrote:
>>
>> The chip_info structure is not required to be accessed after probe.
>>
>> Remove the chip_info pointer from the driver data to reduce the scope
>> and to make driver clearer.
> 
> the driver
> 
> clearer or cleaner? I think you want the latter...
> 

I actually think both :)

> ...
> 
> Not sure how the future of the development of this driver will look
> like, but it might be this patch will be reverted if one wants
> something else from chip_info to have a longer lifetime.
> 

Nuno had the same comment. I kind of like the idea of only having those 
bits of chip_info that are used after probe, stored in the "state 
struct". Or, to reverse this, I don't like having the unused (after the 
probe) data stored in the state struct. For me it is both clearer, and 
cleaner.

But yes, as You and Nuno pointed out, this leads to some data 
duplication. If the opinions were "1 against 1", I would try discussing 
this - but meh, I'll drop this as you both suggested.

Thanks for the review Nuno & Andy.

Yours,
	-- Matti