[PATCH v2] iio: adc: Include valid channel for channel selection

[PATCH v2] iio: adc: Include valid channel for channel selection

[Gabriel Shahrouzi]

According to the datasheet on page 9 under the channel selection table,
all devices (AD7816/7/8) are able to use the channel marked as 7. This
channel is used for diagnostic purposes by routing the internal 1.23V
bandgap source through the MUX to the input of the ADC.

Replace checking for string equality with checking for the same chip ID
to reduce time complexity.

Group invalid channels for all devices together because they are
processed the same way.

Fixes: 7924425db04a ("staging: iio: adc: new driver for AD7816 devices")
Cc: stable@vger.kernel.org
Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com>
---
Changes since v2:
	- Refactor by adding chip_info struct which simplifies
	  condtional logic.
---
 drivers/staging/iio/adc/ad7816.c | 68 ++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 6c14d7bcdd675..ec955cbf06c17 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -41,8 +41,20 @@
  * struct ad7816_chip_info - chip specific information
  */
 
+enum ad7816_type {
+	ID_AD7816,
+	ID_AD7817,
+	ID_AD7818,
+};
+
 struct ad7816_chip_info {
-	kernel_ulong_t id;
+	const char *name;
+	enum ad7816_type type;
+	u8 max_channels;
+};
+
+struct ad7816_state {
+	const struct ad7816_chip_info *chip_info;
 	struct spi_device *spi_dev;
 	struct gpio_desc *rdwr_pin;
 	struct gpio_desc *convert_pin;
@@ -52,16 +64,11 @@ struct ad7816_chip_info {
 	u8  mode;
 };
 
-enum ad7816_type {
-	ID_AD7816,
-	ID_AD7817,
-	ID_AD7818,
-};
 
 /*
  * ad7816 data access by SPI
  */
-static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
+static int ad7816_spi_read(struct ad7816_state *chip, u16 *data)
 {
 	struct spi_device *spi_dev = chip->spi_dev;
 	int ret;
@@ -84,7 +91,7 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
 		gpiod_set_value(chip->convert_pin, 1);
 	}
 
-	if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
+	if (chip->chip_info->type == ID_AD7816 || chip->chip_info->type == ID_AD7817) {
 		while (gpiod_get_value(chip->busy_pin))
 			cpu_relax();
 	}
@@ -102,7 +109,7 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
 	return ret;
 }
 
-static int ad7816_spi_write(struct ad7816_chip_info *chip, u8 data)
+static int ad7816_spi_write(struct ad7816_state *chip, u8 data)
 {
 	struct spi_device *spi_dev = chip->spi_dev;
 	int ret;
@@ -121,7 +128,7 @@ static ssize_t ad7816_show_mode(struct device *dev,
 				char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7816_chip_info *chip = iio_priv(indio_dev);
+	struct ad7816_state *chip = iio_priv(indio_dev);
 
 	if (chip->mode)
 		return sprintf(buf, "power-save\n");
@@ -134,7 +141,7 @@ static ssize_t ad7816_store_mode(struct device *dev,
 				 size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7816_chip_info *chip = iio_priv(indio_dev);
+	struct ad7816_state *chip = iio_priv(indio_dev);
 
 	if (strcmp(buf, "full")) {
 		gpiod_set_value(chip->rdwr_pin, 1);
@@ -167,7 +174,7 @@ static ssize_t ad7816_show_channel(struct device *dev,
 				   char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7816_chip_info *chip = iio_priv(indio_dev);
+	struct ad7816_state *chip = iio_priv(indio_dev);
 
 	return sprintf(buf, "%d\n", chip->channel_id);
 }
@@ -178,7 +185,7 @@ static ssize_t ad7816_store_channel(struct device *dev,
 				    size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7816_chip_info *chip = iio_priv(indio_dev);
+	struct ad7816_state *chip = iio_priv(indio_dev);
 	unsigned long data;
 	int ret;
 
@@ -186,17 +193,10 @@ static ssize_t ad7816_store_channel(struct device *dev,
 	if (ret)
 		return ret;
 
-	if (data > AD7816_CS_MAX && data != AD7816_CS_MASK) {
-		dev_err(&chip->spi_dev->dev, "Invalid channel id %lu for %s.\n",
-			data, indio_dev->name);
-		return -EINVAL;
-	} else if (strcmp(indio_dev->name, "ad7818") == 0 && data > 1) {
+	if (data > chip->chip_info->max_channels && data != AD7816_CS_MASK) {
 		dev_err(&chip->spi_dev->dev,
-			"Invalid channel id %lu for ad7818.\n", data);
-		return -EINVAL;
-	} else if (strcmp(indio_dev->name, "ad7816") == 0 && data > 0) {
-		dev_err(&chip->spi_dev->dev,
-			"Invalid channel id %lu for ad7816.\n", data);
+			"Invalid channel id %lu for %s (max regular: %u).\n", data,
+			chip->chip_info->name, chip->chip_info->max_channels);
 		return -EINVAL;
 	}
 
@@ -215,7 +215,7 @@ static ssize_t ad7816_show_value(struct device *dev,
 				 char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7816_chip_info *chip = iio_priv(indio_dev);
+	struct ad7816_state *chip = iio_priv(indio_dev);
 	u16 data;
 	s8 value;
 	int ret;
@@ -271,7 +271,7 @@ static ssize_t ad7816_show_oti(struct device *dev,
 			       char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7816_chip_info *chip = iio_priv(indio_dev);
+	struct ad7816_state *chip = iio_priv(indio_dev);
 	int value;
 
 	if (chip->channel_id > AD7816_CS_MAX) {
@@ -292,7 +292,7 @@ static inline ssize_t ad7816_set_oti(struct device *dev,
 				     size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7816_chip_info *chip = iio_priv(indio_dev);
+	struct ad7816_state *chip = iio_priv(indio_dev);
 	long value;
 	u8 data;
 	int ret;
@@ -345,14 +345,22 @@ static const struct iio_info ad7816_info = {
 	.event_attrs = &ad7816_event_attribute_group,
 };
 
+static const struct ad7816_chip_info ad7816_chip_infos[] = {
+	[ID_AD7816] = { .name = "ad7816", .max_channels = 0, .type = ID_AD7816 },
+	[ID_AD7817] = { .name = "ad7817", .max_channels = 3, .type = ID_AD7817 },
+	[ID_AD7818] = { .name = "ad7818", .max_channels = 1, .type = ID_AD7818 },
+};
+
 /*
  * device probe and remove
  */
 
 static int ad7816_probe(struct spi_device *spi_dev)
 {
-	struct ad7816_chip_info *chip;
+	struct ad7816_state *chip;
 	struct iio_dev *indio_dev;
+	const struct spi_device_id *id = spi_get_device_id(spi_dev);
+	enum ad7816_type chip_type = (enum ad7816_type)id->driver_data;
 	int i, ret;
 
 	indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
@@ -361,12 +369,12 @@ static int ad7816_probe(struct spi_device *spi_dev)
 	chip = iio_priv(indio_dev);
 	/* this is only used for device removal purposes */
 	dev_set_drvdata(&spi_dev->dev, indio_dev);
+	chip->chip_info = &ad7816_chip_infos[chip_type];
 
 	chip->spi_dev = spi_dev;
 	for (i = 0; i <= AD7816_CS_MAX; i++)
 		chip->oti_data[i] = 203;
 
-	chip->id = spi_get_device_id(spi_dev)->driver_data;
 	chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_OUT_HIGH);
 	if (IS_ERR(chip->rdwr_pin)) {
 		ret = PTR_ERR(chip->rdwr_pin);
@@ -382,7 +390,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
 			ret);
 		return ret;
 	}
-	if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
+	if (chip->chip_info->type == ID_AD7816 || chip->chip_info->type == ID_AD7817) {
 		chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy",
 						GPIOD_IN);
 		if (IS_ERR(chip->busy_pin)) {
@@ -393,7 +401,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
 		}
 	}
 
-	indio_dev->name = spi_get_device_id(spi_dev)->name;
+	indio_dev->name = chip->chip_info->name;
 	indio_dev->info = &ad7816_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-- 
2.43.0

Re: [PATCH v2] iio: adc: Include valid channel for channel selection

[Jonathan Cameron]

On Thu, 17 Apr 2025 13:01:09 -0400
Gabriel Shahrouzi <gshahrouzi@gmail.com> wrote:

> According to the datasheet on page 9 under the channel selection table,
> all devices (AD7816/7/8) are able to use the channel marked as 7. This
> channel is used for diagnostic purposes by routing the internal 1.23V
> bandgap source through the MUX to the input of the ADC.
> 
> Replace checking for string equality with checking for the same chip ID
> to reduce time complexity.
> 
> Group invalid channels for all devices together because they are
> processed the same way.
> 
> Fixes: 7924425db04a ("staging: iio: adc: new driver for AD7816 devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com>

This is doing too many things for one patch.  The fix wants be
on it's own. Ideally before everything else but if that is tricky to do
then I don't mind that much.

> ---
> Changes since v2:
> 	- Refactor by adding chip_info struct which simplifies
> 	  condtional logic.
> ---
>  drivers/staging/iio/adc/ad7816.c | 68 ++++++++++++++++++--------------
>  1 file changed, 38 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index 6c14d7bcdd675..ec955cbf06c17 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -41,8 +41,20 @@
>   * struct ad7816_chip_info - chip specific information
>   */
>  
> +enum ad7816_type {
> +	ID_AD7816,
> +	ID_AD7817,
> +	ID_AD7818,
> +};
The aim of moving to a chip_info structure is typically
to get rid of this sort of enum in favour of specific data
in the chip_info structures.  Anyhow see below for more on that.

> +
>  struct ad7816_chip_info {
> -	kernel_ulong_t id;
> +	const char *name;
> +	enum ad7816_type type;
> +	u8 max_channels;
> +};
> +
> +struct ad7816_state {

This effective rename needs to be a patch on it's own before
you introduce the new struct ad7816_chip_info.  That lets
us clearly see that all instances are renamed and is
an easy patch to review as it's just a rename.

> +	const struct ad7816_chip_info *chip_info;
>  	struct spi_device *spi_dev;
>  	struct gpio_desc *rdwr_pin;
>  	struct gpio_desc *convert_pin;
> @@ -52,16 +64,11 @@ struct ad7816_chip_info {
>  	u8  mode;
>  };
>  
> -enum ad7816_type {
> -	ID_AD7816,
> -	ID_AD7817,
> -	ID_AD7818,
> -};
>
> @@ -215,7 +215,7 @@ static ssize_t ad7816_show_value(struct device *dev,
>  				 char *buf)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct ad7816_chip_info *chip = iio_priv(indio_dev);
> +	struct ad7816_state *chip = iio_priv(indio_dev);
>  	u16 data;
>  	s8 value;
>  	int ret;
> @@ -271,7 +271,7 @@ static ssize_t ad7816_show_oti(struct device *dev,
>  			       char *buf)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct ad7816_chip_info *chip = iio_priv(indio_dev);
> +	struct ad7816_state *chip = iio_priv(indio_dev);
>  	int value;
>  
>  	if (chip->channel_id > AD7816_CS_MAX) {
> @@ -292,7 +292,7 @@ static inline ssize_t ad7816_set_oti(struct device *dev,
>  				     size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct ad7816_chip_info *chip = iio_priv(indio_dev);
> +	struct ad7816_state *chip = iio_priv(indio_dev);
>  	long value;
>  	u8 data;
>  	int ret;
> @@ -345,14 +345,22 @@ static const struct iio_info ad7816_info = {
>  	.event_attrs = &ad7816_event_attribute_group,
>  };
>  
> +static const struct ad7816_chip_info ad7816_chip_infos[] = {
> +	[ID_AD7816] = { .name = "ad7816", .max_channels = 0, .type = ID_AD7816 },
Use separate named structures.

This approach with an enum and an array is something we used to do but it
actually obscures what is going on when compared with separate names structure
instances.

static const struct ad7816_chip_info ad7816_chip_info = {..};

static const struct ad7816_chip_info ad7817_chip_info = {..};

etc.
> +	[ID_AD7817] = { .name = "ad7817", .max_channels = 3, .type = ID_AD7817 },
> +	[ID_AD7818] = { .name = "ad7818", .max_channels = 1, .type = ID_AD7818 },
> +};
> +
>  /*
>   * device probe and remove
>   */
>  
>  static int ad7816_probe(struct spi_device *spi_dev)
>  {
> -	struct ad7816_chip_info *chip;
> +	struct ad7816_state *chip;
>  	struct iio_dev *indio_dev;
> +	const struct spi_device_id *id = spi_get_device_id(spi_dev);
> +	enum ad7816_type chip_type = (enum ad7816_type)id->driver_data;

Don't go via an enum.  Put pointers directly in the driver_data field.
+ if the driver has on in the of_match_id table as well.

Then use spi_get_device_match_data() to retrieve it in a firmware type
independent way.

Ideally by the end of the series you should no such enum

>  	int i, ret;
>  
>  	indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
> @@ -361,12 +369,12 @@ static int ad7816_probe(struct spi_device *spi_dev)
>  	chip = iio_priv(indio_dev);
>  	/* this is only used for device removal purposes */
>  	dev_set_drvdata(&spi_dev->dev, indio_dev);
> +	chip->chip_info = &ad7816_chip_infos[chip_type];
>  
>  	chip->spi_dev = spi_dev;
>  	for (i = 0; i <= AD7816_CS_MAX; i++)
>  		chip->oti_data[i] = 203;
>  
> -	chip->id = spi_get_device_id(spi_dev)->driver_data;
>  	chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_OUT_HIGH);
>  	if (IS_ERR(chip->rdwr_pin)) {
>  		ret = PTR_ERR(chip->rdwr_pin);
> @@ -382,7 +390,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
>  			ret);
>  		return ret;
>  	}
> -	if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
> +	if (chip->chip_info->type == ID_AD7816 || chip->chip_info->type == ID_AD7817) {

Given you now have a chip_info structure make this data rather than code.  That is
it should be something ilke

	if (chip->chip_info->has_busy_pin)

>  		chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy",
>  						GPIOD_IN);
>  		if (IS_ERR(chip->busy_pin)) {
> @@ -393,7 +401,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
>  		}
>  	}
>  
> -	indio_dev->name = spi_get_device_id(spi_dev)->name;
> +	indio_dev->name = chip->chip_info->name;
>  	indio_dev->info = &ad7816_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  

Re: [PATCH v2] iio: adc: Include valid channel for channel selection

[Gabriel Shahrouzi]

On Fri, Apr 18, 2025 at 11:35 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 17 Apr 2025 13:01:09 -0400
> Gabriel Shahrouzi <gshahrouzi@gmail.com> wrote:
>
> > According to the datasheet on page 9 under the channel selection table,
> > all devices (AD7816/7/8) are able to use the channel marked as 7. This
> > channel is used for diagnostic purposes by routing the internal 1.23V
> > bandgap source through the MUX to the input of the ADC.
> >
> > Replace checking for string equality with checking for the same chip ID
> > to reduce time complexity.
> >
> > Group invalid channels for all devices together because they are
> > processed the same way.
> >
> > Fixes: 7924425db04a ("staging: iio: adc: new driver for AD7816 devices")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com>
>
> This is doing too many things for one patch.  The fix wants be
> on it's own. Ideally before everything else but if that is tricky to do
> then I don't mind that much.
Got it - will split them up into modular changes.
>
> > ---
> > Changes since v2:
> >       - Refactor by adding chip_info struct which simplifies
> >         condtional logic.
> > ---
> >  drivers/staging/iio/adc/ad7816.c | 68 ++++++++++++++++++--------------
> >  1 file changed, 38 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> > index 6c14d7bcdd675..ec955cbf06c17 100644
> > --- a/drivers/staging/iio/adc/ad7816.c
> > +++ b/drivers/staging/iio/adc/ad7816.c
> > @@ -41,8 +41,20 @@
> >   * struct ad7816_chip_info - chip specific information
> >   */
> >
> > +enum ad7816_type {
> > +     ID_AD7816,
> > +     ID_AD7817,
> > +     ID_AD7818,
> > +};
> The aim of moving to a chip_info structure is typically
> to get rid of this sort of enum in favour of specific data
> in the chip_info structures.  Anyhow see below for more on that.
>
> > +
> >  struct ad7816_chip_info {
> > -     kernel_ulong_t id;
> > +     const char *name;
> > +     enum ad7816_type type;
> > +     u8 max_channels;
> > +};
> > +
> > +struct ad7816_state {
>
> This effective rename needs to be a patch on it's own before
> you introduce the new struct ad7816_chip_info.  That lets
> us clearly see that all instances are renamed and is
> an easy patch to review as it's just a rename.
Got it.
>
> > +     const struct ad7816_chip_info *chip_info;
> >       struct spi_device *spi_dev;
> >       struct gpio_desc *rdwr_pin;
> >       struct gpio_desc *convert_pin;
> > @@ -52,16 +64,11 @@ struct ad7816_chip_info {
> >       u8  mode;
> >  };
> >
> > -enum ad7816_type {
> > -     ID_AD7816,
> > -     ID_AD7817,
> > -     ID_AD7818,
> > -};
> >
> > @@ -215,7 +215,7 @@ static ssize_t ad7816_show_value(struct device *dev,
> >                                char *buf)
> >  {
> >       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > -     struct ad7816_chip_info *chip = iio_priv(indio_dev);
> > +     struct ad7816_state *chip = iio_priv(indio_dev);
> >       u16 data;
> >       s8 value;
> >       int ret;
> > @@ -271,7 +271,7 @@ static ssize_t ad7816_show_oti(struct device *dev,
> >                              char *buf)
> >  {
> >       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > -     struct ad7816_chip_info *chip = iio_priv(indio_dev);
> > +     struct ad7816_state *chip = iio_priv(indio_dev);
> >       int value;
> >
> >       if (chip->channel_id > AD7816_CS_MAX) {
> > @@ -292,7 +292,7 @@ static inline ssize_t ad7816_set_oti(struct device *dev,
> >                                    size_t len)
> >  {
> >       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > -     struct ad7816_chip_info *chip = iio_priv(indio_dev);
> > +     struct ad7816_state *chip = iio_priv(indio_dev);
> >       long value;
> >       u8 data;
> >       int ret;
> > @@ -345,14 +345,22 @@ static const struct iio_info ad7816_info = {
> >       .event_attrs = &ad7816_event_attribute_group,
> >  };
> >
> > +static const struct ad7816_chip_info ad7816_chip_infos[] = {
> > +     [ID_AD7816] = { .name = "ad7816", .max_channels = 0, .type = ID_AD7816 },
> Use separate named structures.
>
> This approach with an enum and an array is something we used to do but it
> actually obscures what is going on when compared with separate names structure
> instances.
>
> static const struct ad7816_chip_info ad7816_chip_info = {..};
>
> static const struct ad7816_chip_info ad7817_chip_info = {..};
>
> etc.
Got it.
> > +     [ID_AD7817] = { .name = "ad7817", .max_channels = 3, .type = ID_AD7817 },
> > +     [ID_AD7818] = { .name = "ad7818", .max_channels = 1, .type = ID_AD7818 },
> > +};
> > +
> >  /*
> >   * device probe and remove
> >   */
> >
> >  static int ad7816_probe(struct spi_device *spi_dev)
> >  {
> > -     struct ad7816_chip_info *chip;
> > +     struct ad7816_state *chip;
> >       struct iio_dev *indio_dev;
> > +     const struct spi_device_id *id = spi_get_device_id(spi_dev);
> > +     enum ad7816_type chip_type = (enum ad7816_type)id->driver_data;
>
> Don't go via an enum.  Put pointers directly in the driver_data field.
> + if the driver has on in the of_match_id table as well.
>
> Then use spi_get_device_match_data() to retrieve it in a firmware type
> independent way.
>
> Ideally by the end of the series you should no such enum
Sent the changes in v2 where no enum was used.
>
> >       int i, ret;
> >
> >       indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
> > @@ -361,12 +369,12 @@ static int ad7816_probe(struct spi_device *spi_dev)
> >       chip = iio_priv(indio_dev);
> >       /* this is only used for device removal purposes */
> >       dev_set_drvdata(&spi_dev->dev, indio_dev);
> > +     chip->chip_info = &ad7816_chip_infos[chip_type];
> >
> >       chip->spi_dev = spi_dev;
> >       for (i = 0; i <= AD7816_CS_MAX; i++)
> >               chip->oti_data[i] = 203;
> >
> > -     chip->id = spi_get_device_id(spi_dev)->driver_data;
> >       chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_OUT_HIGH);
> >       if (IS_ERR(chip->rdwr_pin)) {
> >               ret = PTR_ERR(chip->rdwr_pin);
> > @@ -382,7 +390,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
> >                       ret);
> >               return ret;
> >       }
> > -     if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
> > +     if (chip->chip_info->type == ID_AD7816 || chip->chip_info->type == ID_AD7817) {
>
> Given you now have a chip_info structure make this data rather than code.  That is
> it should be something ilke
>
>         if (chip->chip_info->has_busy_pin)
>
> >               chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy",
> >                                               GPIOD_IN);
> >               if (IS_ERR(chip->busy_pin)) {
> > @@ -393,7 +401,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
> >               }
> >       }
> >
> > -     indio_dev->name = spi_get_device_id(spi_dev)->name;
> > +     indio_dev->name = chip->chip_info->name;
> >       indio_dev->info = &ad7816_info;
> >       indio_dev->modes = INDIO_DIRECT_MODE;
> >
>