[PATCH v3 0/4] staging: iio: adc: ad7816: code cleanups and modernization

[PATCH v3 0/4] staging: iio: adc: ad7816: code cleanups and modernization

[Md. Mahmudul Hasan Mabud]

Changes in v3:
- Added missing Signed-off-by tags and detailed patch descriptions to
  all commits as requested by the automated patch-bot.
- No functional changes since v2.

Changes in v2:
- Renamed sysfs callback functions (e.g., ad7816_show_mode to mode_show)
  to satisfy the requirements of modern IIO attribute macros.
- Fixed usage of sysfs_match_string() by removing the unnecessary 
  ARRAY_SIZE parameter.
- Fixed vertical alignment of function parameters to match open parenthesis.
- Replaced sprintf() with sysfs_emit() across all show attributes.

Md. Mahmudul Hasan Mabud (4):
  staging: iio: adc: ad7816: redefine mode constants to start from 0
  staging: iio: adc: ad7816: use sysfs_match_string() for mode selection
  staging: iio: adc: ad7816: replace sprintf() with sysfs_emit()
  staging: iio: adc: ad7816: use modern IIO attribute macros

 drivers/staging/iio/adc/ad7816.c | 111 +++++++++++++++----------------
 1 file changed, 53 insertions(+), 58 deletions(-)

-- 
2.34.1

[PATCH v3 1/4] staging: iio: adc: ad7816: redefine mode constants to start from 0

[Md. Mahmudul Hasan Mabud]

Redefine AD7816_FULL and AD7816_PD constants to start from 0 instead
of 1. This change is necessary to use these constants as direct
indices for the mode string array and to align with standard
indexing in modern sysfs helpers.

Signed-off-by: Md. Mahmudul Hasan Mabud <mdmahmudulhasan1511@gmail.com>
---
 drivers/staging/iio/adc/ad7816.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 172acf135..5acfceb90 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -22,8 +22,8 @@
 /*
  * AD7816 config masks
  */
-#define AD7816_FULL			0x1
-#define AD7816_PD			0x2
+#define AD7816_FULL			0
+#define AD7816_PD			1
 #define AD7816_CS_MASK			0x7
 #define AD7816_CS_MAX			0x4
 
-- 
2.34.1

[PATCH v3 2/4] staging: iio: adc: ad7816: use sysfs_match_string() for mode selection

[Md. Mahmudul Hasan Mabud]

Replace manual strcmp() logic with sysfs_match_string() to simplify
the mode selection process. This reduces boilerplate code and
leverages standard kernel helper functions for string matching.

Signed-off-by: Md. Mahmudul Hasan Mabud <mdmahmudulhasan1511@gmail.com>
---
 drivers/staging/iio/adc/ad7816.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 5acfceb90..f9a0a5a69 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -58,6 +58,11 @@ enum ad7816_type {
 	ID_AD7818,
 };
 
+static const char * const ad7816_modes[] = {
+	[AD7816_FULL] = "full",
+	[AD7816_PD] = "power-save",
+};
+
 /*
  * ad7816 data access by SPI
  */
@@ -135,14 +140,14 @@ static ssize_t ad7816_store_mode(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
+	int ret;
 
-	if (strcmp(buf, "full") == 0) {
-		gpiod_set_value(chip->rdwr_pin, 1);
-		chip->mode = AD7816_FULL;
-	} else {
-		gpiod_set_value(chip->rdwr_pin, 0);
-		chip->mode = AD7816_PD;
-	}
+	ret = sysfs_match_string(ad7816_modes, buf);
+	if (ret < 0)
+		return ret;
+
+	chip->mode = ret;
+	gpiod_set_value(chip->rdwr_pin, ret == AD7816_FULL);
 
 	return len;
 }
-- 
2.34.1

[PATCH v3 3/4] staging: iio: adc: ad7816: replace sprintf() with sysfs_emit()

[Md. Mahmudul Hasan Mabud]

Replace sprintf() with sysfs_emit() in all show attributes.
sysfs_emit() is the preferred way to return values in sysfs
callbacks as it is safer and handles page boundary checks
automatically.

Signed-off-by: Md. Mahmudul Hasan Mabud <mdmahmudulhasan1511@gmail.com>
---
 drivers/staging/iio/adc/ad7816.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index f9a0a5a69..f2486cb29 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -128,9 +128,7 @@ static ssize_t ad7816_show_mode(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
 
-	if (chip->mode)
-		return sprintf(buf, "power-save\n");
-	return sprintf(buf, "full\n");
+	return sysfs_emit(buf, "%s\n", ad7816_modes[chip->mode]);
 }
 
 static ssize_t ad7816_store_mode(struct device *dev,
@@ -161,7 +159,7 @@ static ssize_t ad7816_show_available_modes(struct device *dev,
 					   struct device_attribute *attr,
 					   char *buf)
 {
-	return sprintf(buf, "full\npower-save\n");
+	return sysfs_emit(buf, "full\npower-save\n");
 }
 
 static IIO_DEVICE_ATTR(available_modes, 0444, ad7816_show_available_modes,
@@ -174,7 +172,7 @@ static ssize_t ad7816_show_channel(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", chip->channel_id);
+	return sysfs_emit(buf, "%d\n", chip->channel_id);
 }
 
 static ssize_t ad7816_store_channel(struct device *dev,
@@ -236,9 +234,9 @@ static ssize_t ad7816_show_value(struct device *dev,
 		data &= AD7816_TEMP_FLOAT_MASK;
 		if (value < 0)
 			data = BIT(AD7816_TEMP_FLOAT_OFFSET) - data;
-		return sprintf(buf, "%d.%.2d\n", value, data * 25);
+		return sysfs_emit(buf, "%d.%.2d\n", value, data * 25);
 	}
-	return sprintf(buf, "%u\n", data);
+	return sysfs_emit(buf, "%u\n", data);
 }
 
 static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
@@ -286,9 +284,9 @@ static ssize_t ad7816_show_oti(struct device *dev,
 		value = AD7816_BOUND_VALUE_MIN +
 			(chip->oti_data[chip->channel_id] -
 			AD7816_BOUND_VALUE_BASE);
-		return sprintf(buf, "%d\n", value);
+		return sysfs_emit(buf, "%d\n", value);
 	}
-	return sprintf(buf, "%u\n", chip->oti_data[chip->channel_id]);
+	return sysfs_emit(buf, "%u\n", chip->oti_data[chip->channel_id]);
 }
 
 static inline ssize_t ad7816_set_oti(struct device *dev,
-- 
2.34.1

[PATCH v3 4/4] staging: iio: adc: ad7816: use modern IIO attribute macros

[Md. Mahmudul Hasan Mabud]

Modernize the driver by using IIO_DEVICE_ATTR_RW and
IIO_DEVICE_ATTR_RO macros. This involves renaming the callback
functions to standard <name>_show and <name>_store formats, making
the code cleaner and more maintainable.

Signed-off-by: Md. Mahmudul Hasan Mabud <mdmahmudulhasan1511@gmail.com>
---
 drivers/staging/iio/adc/ad7816.c | 72 ++++++++++++++------------------
 1 file changed, 32 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index f2486cb29..80f84e2bb 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -121,9 +121,9 @@ static int ad7816_spi_write(struct ad7816_chip_info *chip, u8 data)
 	return ret;
 }
 
-static ssize_t ad7816_show_mode(struct device *dev,
-				struct device_attribute *attr,
-				char *buf)
+static ssize_t mode_show(struct device *dev,
+			 struct device_attribute *attr,
+			 char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
@@ -131,10 +131,10 @@ static ssize_t ad7816_show_mode(struct device *dev,
 	return sysfs_emit(buf, "%s\n", ad7816_modes[chip->mode]);
 }
 
-static ssize_t ad7816_store_mode(struct device *dev,
-				 struct device_attribute *attr,
-				 const char *buf,
-				 size_t len)
+static ssize_t mode_store(struct device *dev,
+			  struct device_attribute *attr,
+			  const char *buf,
+			  size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
@@ -150,24 +150,20 @@ static ssize_t ad7816_store_mode(struct device *dev,
 	return len;
 }
 
-static IIO_DEVICE_ATTR(mode, 0644,
-		ad7816_show_mode,
-		ad7816_store_mode,
-		0);
+static IIO_DEVICE_ATTR_RW(mode, 0);
 
-static ssize_t ad7816_show_available_modes(struct device *dev,
-					   struct device_attribute *attr,
-					   char *buf)
+static ssize_t available_modes_show(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
 {
 	return sysfs_emit(buf, "full\npower-save\n");
 }
 
-static IIO_DEVICE_ATTR(available_modes, 0444, ad7816_show_available_modes,
-			NULL, 0);
+static IIO_DEVICE_ATTR_RO(available_modes, 0);
 
-static ssize_t ad7816_show_channel(struct device *dev,
-				   struct device_attribute *attr,
-				   char *buf)
+static ssize_t channel_show(struct device *dev,
+			    struct device_attribute *attr,
+			    char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
@@ -175,10 +171,10 @@ static ssize_t ad7816_show_channel(struct device *dev,
 	return sysfs_emit(buf, "%d\n", chip->channel_id);
 }
 
-static ssize_t ad7816_store_channel(struct device *dev,
-				    struct device_attribute *attr,
-				    const char *buf,
-				    size_t len)
+static ssize_t channel_store(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf,
+			     size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
@@ -208,14 +204,11 @@ static ssize_t ad7816_store_channel(struct device *dev,
 	return len;
 }
 
-static IIO_DEVICE_ATTR(channel, 0644,
-		ad7816_show_channel,
-		ad7816_store_channel,
-		0);
+static IIO_DEVICE_ATTR_RW(channel, 0);
 
-static ssize_t ad7816_show_value(struct device *dev,
-				 struct device_attribute *attr,
-				 char *buf)
+static ssize_t value_show(struct device *dev,
+			  struct device_attribute *attr,
+			  char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
@@ -239,7 +232,7 @@ static ssize_t ad7816_show_value(struct device *dev,
 	return sysfs_emit(buf, "%u\n", data);
 }
 
-static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
+static IIO_DEVICE_ATTR_RO(value, 0);
 
 static struct attribute *ad7816_attributes[] = {
 	&iio_dev_attr_available_modes.dev_attr.attr,
@@ -269,9 +262,9 @@ static irqreturn_t ad7816_event_handler(int irq, void *private)
 	return IRQ_HANDLED;
 }
 
-static ssize_t ad7816_show_oti(struct device *dev,
-			       struct device_attribute *attr,
-			       char *buf)
+static ssize_t oti_show(struct device *dev,
+			struct device_attribute *attr,
+			char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
@@ -289,10 +282,10 @@ static ssize_t ad7816_show_oti(struct device *dev,
 	return sysfs_emit(buf, "%u\n", chip->oti_data[chip->channel_id]);
 }
 
-static inline ssize_t ad7816_set_oti(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf,
-				     size_t len)
+static inline ssize_t oti_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf,
+				size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
@@ -330,8 +323,7 @@ static inline ssize_t ad7816_set_oti(struct device *dev,
 	return len;
 }
 
-static IIO_DEVICE_ATTR(oti, 0644,
-		       ad7816_show_oti, ad7816_set_oti, 0);
+static IIO_DEVICE_ATTR_RW(oti, 0);
 
 static struct attribute *ad7816_event_attributes[] = {
 	&iio_dev_attr_oti.dev_attr.attr,
-- 
2.34.1

Re: [PATCH v3 1/4] staging: iio: adc: ad7816: redefine mode constants to start from 0

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 12:17:07AM +0600, Md. Mahmudul Hasan Mabud wrote:
> Redefine AD7816_FULL and AD7816_PD constants to start from 0 instead
> of 1. This change is necessary to use these constants as direct
> indices for the mode string array and to align with standard
> indexing in modern sysfs helpers.

...

> -#define AD7816_FULL			0x1
> -#define AD7816_PD			0x2
> +#define AD7816_FULL			0
> +#define AD7816_PD			1

+ blank line here.

>  #define AD7816_CS_MASK			0x7
>  #define AD7816_CS_MAX			0x4

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/4] staging: iio: adc: ad7816: replace sprintf() with sysfs_emit()

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 12:17:09AM +0600, Md. Mahmudul Hasan Mabud wrote:
> Replace sprintf() with sysfs_emit() in all show attributes.
> sysfs_emit() is the preferred way to return values in sysfs
> callbacks as it is safer and handles page boundary checks
> automatically.

...

> static ssize_t ad7816_show_available_modes(struct device *dev,
>  					   struct device_attribute *attr,
>  					   char *buf)
>  {
> -	return sprintf(buf, "full\npower-save\n");
> +	return sysfs_emit(buf, "full\npower-save\n");

This should be replaced with a for-loop, printing all from that array.

>  }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/4] staging: iio: adc: ad7816: use modern IIO attribute macros

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 12:17:10AM +0600, Md. Mahmudul Hasan Mabud wrote:
> Modernize the driver by using IIO_DEVICE_ATTR_RW and

IIO_DEVICE_ATTR_RW()

> IIO_DEVICE_ATTR_RO macros. This involves renaming the callback

IIO_DEVICE_ATTR_RO()

> functions to standard <name>_show and <name>_store formats, making

"...<name>_show() and <name>_store()..."

> the code cleaner and more maintainable.

...

> -static ssize_t ad7816_show_mode(struct device *dev,
> -				struct device_attribute *attr,
> -				char *buf)
> +static ssize_t mode_show(struct device *dev,
> +			 struct device_attribute *attr,
> +			 char *buf)

You can use room on the previous line(s)

static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
			 char *buf)

...

> -static ssize_t ad7816_store_mode(struct device *dev,
> -				 struct device_attribute *attr,
> -				 const char *buf,
> -				 size_t len)
> +static ssize_t mode_store(struct device *dev,
> +			  struct device_attribute *attr,
> +			  const char *buf,
> +			  size_t len)

static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
			  const char *buf, size_t len)

And so on...

...

> -static ssize_t ad7816_show_available_modes(struct device *dev,
> -					   struct device_attribute *attr,
> -					   char *buf)
> +static ssize_t available_modes_show(struct device *dev,
> +				    struct device_attribute *attr,
> +				    char *buf)

This is okay to leave as is, as it might be too long (over 80).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/4] staging: iio: adc: ad7816: code cleanups and modernization

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 12:17:06AM +0600, Md. Mahmudul Hasan Mabud wrote:

Please, also put a small description of what is done in the series as a whole.
Nevertheless, this version is much better now, but some small tweaks are
required. I believe the next one will be good enough.

> Changes in v3:
> - Added missing Signed-off-by tags and detailed patch descriptions to
>   all commits as requested by the automated patch-bot.
> - No functional changes since v2.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/4] staging: iio: adc: ad7816: redefine mode constants to start from 0

[Dan Carpenter]

On Tue, Mar 31, 2026 at 12:17:07AM +0600, Md. Mahmudul Hasan Mabud wrote:
> Redefine AD7816_FULL and AD7816_PD constants to start from 0 instead
> of 1. This change is necessary to use these constants as direct
> indices for the mode string array and to align with standard
> indexing in modern sysfs helpers.
> 
> Signed-off-by: Md. Mahmudul Hasan Mabud <mdmahmudulhasan1511@gmail.com>
> ---
>  drivers/staging/iio/adc/ad7816.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index 172acf135..5acfceb90 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -22,8 +22,8 @@
>  /*
>   * AD7816 config masks
>   */
> -#define AD7816_FULL			0x1
> -#define AD7816_PD			0x2
> +#define AD7816_FULL			0
> +#define AD7816_PD			1

I don't love this.  AD7816_PD used to be related to the spec but now
it's just a random number which tricks you into thinking it has
meaning but it doesn't.

if (chip->mode == AD7816_PD) { /* operating mode 2 */

regards,
dan carpenter

Re: [PATCH v3 1/4] staging: iio: adc: ad7816: redefine mode constants to start from 0

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 01:53:29PM +0300, Dan Carpenter wrote:
> On Tue, Mar 31, 2026 at 12:17:07AM +0600, Md. Mahmudul Hasan Mabud wrote:

...

> >  /*
> >   * AD7816 config masks
> >   */
> > -#define AD7816_FULL			0x1
> > -#define AD7816_PD			0x2
> > +#define AD7816_FULL			0
> > +#define AD7816_PD			1
> 
> I don't love this.  AD7816_PD used to be related to the spec but now
> it's just a random number which tricks you into thinking it has
> meaning but it doesn't.
> 
> if (chip->mode == AD7816_PD) { /* operating mode 2 */

We can leave that and add a new member to the string array

	[_DEFAULT] = "default";

and associate it with the default choice which seems to be _FULL as per
kzalloc() in the IIO core.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/4] staging: iio: adc: ad7816: redefine mode constants to start from 0

[Jonathan Cameron]

On Tue, 31 Mar 2026 14:06:04 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Mar 31, 2026 at 01:53:29PM +0300, Dan Carpenter wrote:
> > On Tue, Mar 31, 2026 at 12:17:07AM +0600, Md. Mahmudul Hasan Mabud wrote:  
> 
> ...
> 
> > >  /*
> > >   * AD7816 config masks
> > >   */
> > > -#define AD7816_FULL			0x1
> > > -#define AD7816_PD			0x2
> > > +#define AD7816_FULL			0
> > > +#define AD7816_PD			1  
> > 
> > I don't love this.  AD7816_PD used to be related to the spec but now
> > it's just a random number which tricks you into thinking it has
> > meaning but it doesn't.
> > 
> > if (chip->mode == AD7816_PD) { /* operating mode 2 */  
> 
> We can leave that and add a new member to the string array
> 
> 	[_DEFAULT] = "default";
> 
> and associate it with the default choice which seems to be _FULL as per
> kzalloc() in the IIO core.

Hmm. This ABI is pretty bad anyway, good not to introduce more values that
don't actually occur.  Like pretty much all IIO drivers, I'd suggest
this shouldn't ultimately be exposed to userspace at all. This is
what we have runtime PM for which removes the need for userspace to
do anything specific to bring the device out of power down mode.

The only place we allow these is when it's an output channel an
powering down is entering a specific state (typically a pull high
or something like that).

Jonathan

> 
>