public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling
@ 2025-04-18 20:47 Gabriel Shahrouzi
  2025-04-18 20:47 ` [PATCH v3 1/5] staging: iio: adc: ad7816: Allow channel 7 for all devices Gabriel Shahrouzi
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Gabriel Shahrouzi @ 2025-04-18 20:47 UTC (permalink / raw)
  To: gregkh, jic23, lars, linux-iio, linux-kernel, linux-staging,
	Michael.Hennerich, sonic.zhang, vapier
  Cc: gshahrouzi, skhan, linux-kernel-mentees

The original patch combined a functional fix (allowing channel 7) with
several refactoring steps (introducing chip_info, renaming structs,
improving validation). As requested, these have now been separated.

The series proceeds as follows:
1. Fix: Allow diagnostic channel 7 for all device variants.
2. Refactor: Rename the main state structure for clarity before introducing
   the new chip_info struct.
3. Refactor: Introduce struct ad7816_chip_info to hold static per-variant
   data, update ID tables to store pointers, and switch to using
   device_get_match_data() for firmware-independent identification.
   This removes the old enum/id mechanism.
4. Refactor: Add has_busy_pin to chip_info and use this flag to
   determine BUSY pin handling, replacing pointer comparisons.
5. Refactor: Simplify channel validation logic using 
   chip_info->max_channels, removing strcmp() checks.

Regarding the 'fixes' tag: I've applied it only to the first commit
containing the core fix, primarily to make backporting easier. Is this
the standard practice, or should the tag typically be applied to
subsequent commits that build upon or are related to the fix as well?

Chainges in v3:
	- Split the patch into smaller patches. Make the fix first
	  followed by clean up.
	- Include missing channel for channel selection.
	- Address specific feedback regarding enums vs. chip_info data.
	- Use device_get_match_data() for device identification.
	- Move BUSY pin capability check into chip_info data.
	- Simplify channel validation using chip_info data.
Changes in v2:
        - Refactor by adding chip_info struct which simplifies
          conditional logic.

Gabriel Shahrouzi (5):
  staging: iio: adc: ad7816: Allow channel 7 for all devices
  staging: iio: adc: ad7816: Rename state structure
  staging: iio: adc: ad7816: Introduce chip_info and use pointer
    matching
  staging: iio: adc: ad7816: Use chip_info for device capabilities
  staging: iio: adc: ad7816: Simplify channel validation using chip_info

 drivers/staging/iio/adc/ad7816.c | 94 ++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 40 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/5] staging: iio: adc: ad7816: Allow channel 7 for all devices
  2025-04-18 20:47 [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Gabriel Shahrouzi
@ 2025-04-18 20:47 ` Gabriel Shahrouzi
  2025-04-18 20:47 ` [PATCH v3 2/5] staging: iio: adc: ad7816: Rename state structure Gabriel Shahrouzi
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Gabriel Shahrouzi @ 2025-04-18 20:47 UTC (permalink / raw)
  To: gregkh, jic23, lars, linux-iio, linux-kernel, linux-staging,
	Michael.Hennerich, sonic.zhang, vapier
  Cc: gshahrouzi, skhan, linux-kernel-mentees, stable

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.

Modify the channel validation logic to permit channel 7 for all
supported device types.

Fixes: 7924425db04a ("staging: iio: adc: new driver for AD7816 devices")
Cc: stable@vger.kernel.org
Signed-off-by: Gabriel Shahrouzi <gshahrouzi@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 6c14d7bcdd675..a44b0c8c82b12 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -190,11 +190,11 @@ static ssize_t ad7816_store_channel(struct device *dev,
 		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) {
+	} else if (strcmp(indio_dev->name, "ad7818") == 0 && data > 1 && 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) {
+	} else if (strcmp(indio_dev->name, "ad7816") == 0 && data > 0 && data != AD7816_CS_MASK) {
 		dev_err(&chip->spi_dev->dev,
 			"Invalid channel id %lu for ad7816.\n", data);
 		return -EINVAL;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/5] staging: iio: adc: ad7816: Rename state structure
  2025-04-18 20:47 [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Gabriel Shahrouzi
  2025-04-18 20:47 ` [PATCH v3 1/5] staging: iio: adc: ad7816: Allow channel 7 for all devices Gabriel Shahrouzi
@ 2025-04-18 20:47 ` Gabriel Shahrouzi
  2025-04-18 20:47 ` [PATCH v3 3/5] staging: iio: adc: ad7816: Introduce chip_info and use pointer matching Gabriel Shahrouzi
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Gabriel Shahrouzi @ 2025-04-18 20:47 UTC (permalink / raw)
  To: gregkh, jic23, lars, linux-iio, linux-kernel, linux-staging,
	Michael.Hennerich, sonic.zhang, vapier
  Cc: gshahrouzi, skhan, linux-kernel-mentees

Rename the main driver state structure from 'ad7816_chip_info' to
'ad7816_state' to avoid confusion with the upcoming structure that will
hold static, per-device-type information.

This is purely a mechanical rename with no functional changes.

Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com>
---
 drivers/staging/iio/adc/ad7816.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index a44b0c8c82b12..cad2e55aff3f9 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -38,10 +38,10 @@
 #define AD7816_TEMP_FLOAT_MASK		0x3
 
 /*
- * struct ad7816_chip_info - chip specific information
+ * struct ad7816_state - chip specific information
  */
 
-struct ad7816_chip_info {
+struct ad7816_state {
 	kernel_ulong_t id;
 	struct spi_device *spi_dev;
 	struct gpio_desc *rdwr_pin;
@@ -61,7 +61,7 @@ enum ad7816_type {
 /*
  * 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;
@@ -102,7 +102,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 +121,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 +134,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 +167,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 +178,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;
 
@@ -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;
@@ -351,7 +351,7 @@ static const struct iio_info ad7816_info = {
 
 static int ad7816_probe(struct spi_device *spi_dev)
 {
-	struct ad7816_chip_info *chip;
+	struct ad7816_state *chip;
 	struct iio_dev *indio_dev;
 	int i, ret;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/5] staging: iio: adc: ad7816: Introduce chip_info and use pointer matching
  2025-04-18 20:47 [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Gabriel Shahrouzi
  2025-04-18 20:47 ` [PATCH v3 1/5] staging: iio: adc: ad7816: Allow channel 7 for all devices Gabriel Shahrouzi
  2025-04-18 20:47 ` [PATCH v3 2/5] staging: iio: adc: ad7816: Rename state structure Gabriel Shahrouzi
@ 2025-04-18 20:47 ` Gabriel Shahrouzi
  2025-04-18 20:47 ` [PATCH v3 4/5] staging: iio: adc: ad7816: Use chip_info for device capabilities Gabriel Shahrouzi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Gabriel Shahrouzi @ 2025-04-18 20:47 UTC (permalink / raw)
  To: gregkh, jic23, lars, linux-iio, linux-kernel, linux-staging,
	Michael.Hennerich, sonic.zhang, vapier
  Cc: gshahrouzi, skhan, linux-kernel-mentees

Introduce struct ad7816_chip_info to centralize static properties (e.g.
name, max channels) that differ between chip variants (AD7816/7/8) but
are constant for any specific type.

Store pointers to these instances in the of_device_id (.data) and
spi_device_id (driver_data) tables. Retrieve the pointer in probe()
using the firmware-independent device_get_match_data() and store it in
the ad7816_state struct.

Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com>
---
 drivers/staging/iio/adc/ad7816.c | 55 +++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index cad2e55aff3f9..39310ade770d0 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -41,8 +41,28 @@
  * struct ad7816_state - chip specific information
  */
 
+struct ad7816_chip_info {
+	const char *name;
+	u8 max_channels;
+};
+
+static const struct ad7816_chip_info ad7816_info_ad7816 = {
+	.name = "ad7816",
+	.max_channels = 0,
+};
+
+static const struct ad7816_chip_info ad7817_info_ad7817 = {
+	.name = "ad7817",
+	.max_channels = 3,
+};
+
+static const struct ad7816_chip_info ad7818_info_ad7818 = {
+	.name = "ad7818",
+	.max_channels = 1,
+};
+
 struct ad7816_state {
-	kernel_ulong_t id;
+	const struct ad7816_chip_info *chip_info;
 	struct spi_device *spi_dev;
 	struct gpio_desc *rdwr_pin;
 	struct gpio_desc *convert_pin;
@@ -52,12 +72,6 @@ struct ad7816_state {
 	u8  mode;
 };
 
-enum ad7816_type {
-	ID_AD7816,
-	ID_AD7817,
-	ID_AD7818,
-};
-
 /*
  * ad7816 data access by SPI
  */
@@ -84,7 +98,7 @@ static int ad7816_spi_read(struct ad7816_state *chip, u16 *data)
 		gpiod_set_value(chip->convert_pin, 1);
 	}
 
-	if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
+	if (chip->chip_info == &ad7816_info_ad7816 || chip->chip_info == &ad7817_info_ad7817) {
 		while (gpiod_get_value(chip->busy_pin))
 			cpu_relax();
 	}
@@ -353,6 +367,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
 {
 	struct ad7816_state *chip;
 	struct iio_dev *indio_dev;
+	const struct ad7816_chip_info *info;
 	int i, ret;
 
 	indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
@@ -362,11 +377,15 @@ static int ad7816_probe(struct spi_device *spi_dev)
 	/* this is only used for device removal purposes */
 	dev_set_drvdata(&spi_dev->dev, indio_dev);
 
+	info = device_get_match_data(&spi_dev->dev);
+	if (!info)
+		return -ENODEV;
+	chip->chip_info = info;
+
 	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 +401,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 == &ad7816_info_ad7816 || chip->chip_info == &ad7817_info_ad7817) {
 		chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy",
 						GPIOD_IN);
 		if (IS_ERR(chip->busy_pin)) {
@@ -393,7 +412,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;
 
@@ -420,18 +439,18 @@ static int ad7816_probe(struct spi_device *spi_dev)
 }
 
 static const struct of_device_id ad7816_of_match[] = {
-	{ .compatible = "adi,ad7816", },
-	{ .compatible = "adi,ad7817", },
-	{ .compatible = "adi,ad7818", },
+	{ .compatible = "adi,ad7816", .data = &ad7816_info_ad7816 },
+	{ .compatible = "adi,ad7817", .data = &ad7817_info_ad7817 },
+	{ .compatible = "adi,ad7818", .data = &ad7818_info_ad7818 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad7816_of_match);
 
 static const struct spi_device_id ad7816_id[] = {
-	{ "ad7816", ID_AD7816 },
-	{ "ad7817", ID_AD7817 },
-	{ "ad7818", ID_AD7818 },
-	{}
+	{ "ad7816", (kernel_ulong_t)&ad7816_info_ad7816 },
+	{ "ad7817", (kernel_ulong_t)&ad7817_info_ad7817 },
+	{ "ad7818", (kernel_ulong_t)&ad7818_info_ad7818 },
+	{ }
 };
 
 MODULE_DEVICE_TABLE(spi, ad7816_id);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 4/5] staging: iio: adc: ad7816: Use chip_info for device capabilities
  2025-04-18 20:47 [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Gabriel Shahrouzi
                   ` (2 preceding siblings ...)
  2025-04-18 20:47 ` [PATCH v3 3/5] staging: iio: adc: ad7816: Introduce chip_info and use pointer matching Gabriel Shahrouzi
@ 2025-04-18 20:47 ` Gabriel Shahrouzi
  2025-04-18 20:47 ` [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info Gabriel Shahrouzi
  2025-04-21 11:29 ` [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Jonathan Cameron
  5 siblings, 0 replies; 10+ messages in thread
From: Gabriel Shahrouzi @ 2025-04-18 20:47 UTC (permalink / raw)
  To: gregkh, jic23, lars, linux-iio, linux-kernel, linux-staging,
	Michael.Hennerich, sonic.zhang, vapier
  Cc: gshahrouzi, skhan, linux-kernel-mentees

Move device-specific capability information, like the presence of a
BUSY pin, into the ad7816_chip_info structure.

Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com>
---
 drivers/staging/iio/adc/ad7816.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 39310ade770d0..ab7520a8a3da9 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -44,21 +44,25 @@
 struct ad7816_chip_info {
 	const char *name;
 	u8 max_channels;
+	bool has_busy_pin;
 };
 
 static const struct ad7816_chip_info ad7816_info_ad7816 = {
 	.name = "ad7816",
 	.max_channels = 0,
+	.has_busy_pin = true,
 };
 
 static const struct ad7816_chip_info ad7817_info_ad7817 = {
 	.name = "ad7817",
 	.max_channels = 3,
+	.has_busy_pin = true,
 };
 
 static const struct ad7816_chip_info ad7818_info_ad7818 = {
 	.name = "ad7818",
 	.max_channels = 1,
+	.has_busy_pin = false,
 };
 
 struct ad7816_state {
@@ -98,7 +102,7 @@ static int ad7816_spi_read(struct ad7816_state *chip, u16 *data)
 		gpiod_set_value(chip->convert_pin, 1);
 	}
 
-	if (chip->chip_info == &ad7816_info_ad7816 || chip->chip_info == &ad7817_info_ad7817) {
+	if (chip->chip_info->has_busy_pin) {
 		while (gpiod_get_value(chip->busy_pin))
 			cpu_relax();
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info
  2025-04-18 20:47 [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Gabriel Shahrouzi
                   ` (3 preceding siblings ...)
  2025-04-18 20:47 ` [PATCH v3 4/5] staging: iio: adc: ad7816: Use chip_info for device capabilities Gabriel Shahrouzi
@ 2025-04-18 20:47 ` Gabriel Shahrouzi
  2025-04-19 12:19   ` kernel test robot
  2025-04-19 13:11   ` kernel test robot
  2025-04-21 11:29 ` [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Jonathan Cameron
  5 siblings, 2 replies; 10+ messages in thread
From: Gabriel Shahrouzi @ 2025-04-18 20:47 UTC (permalink / raw)
  To: gregkh, jic23, lars, linux-iio, linux-kernel, linux-staging,
	Michael.Hennerich, sonic.zhang, vapier
  Cc: gshahrouzi, skhan, linux-kernel-mentees

Refactor the channel validation logic within ad7816_store_channel() to
leverage the max_channels field previously introduced in the
ad7816_chip_info structure.

Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com>
---
 drivers/staging/iio/adc/ad7816.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index ab7520a8a3da9..b87934aaae581 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -204,19 +204,10 @@ static ssize_t ad7816_store_channel(struct device *dev,
 	if (ret)
 		return ret;
 
-	if (data > AD7816_CS_MAX && data != AD7816_CS_MASK) {
+	if (data > chip->chip_info->max_channels && data != AD7816_CS_MASK) {
 		dev_err(&chip->spi_dev->dev, "Invalid channel id %lu for %s.\n",
-			data, indio_dev->name);
+			data, chip->chip_info->name);
 		return -EINVAL;
-	} else if (strcmp(indio_dev->name, "ad7818") == 0 && data > 1 && 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 && data != AD7816_CS_MASK) {
-		dev_err(&chip->spi_dev->dev,
-			"Invalid channel id %lu for ad7816.\n", data);
-		return -EINVAL;
-	}
 
 	chip->channel_id = data;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info
  2025-04-18 20:47 ` [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info Gabriel Shahrouzi
@ 2025-04-19 12:19   ` kernel test robot
  2025-04-19 13:11   ` kernel test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2025-04-19 12:19 UTC (permalink / raw)
  To: Gabriel Shahrouzi, gregkh, jic23, lars, linux-iio, linux-kernel,
	linux-staging, Michael.Hennerich, sonic.zhang, vapier
  Cc: oe-kbuild-all, gshahrouzi, skhan, linux-kernel-mentees

Hi Gabriel,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Gabriel-Shahrouzi/staging-iio-adc-ad7816-Allow-channel-7-for-all-devices/20250419-045531
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/fad83a7efb12c0f40dc2660cf9dd4c57422ecff9.1745007964.git.gshahrouzi%40gmail.com
patch subject: [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info
config: x86_64-buildonly-randconfig-003-20250419 (https://download.01.org/0day-ci/archive/20250419/202504192012.ATXH4XfM-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250419/202504192012.ATXH4XfM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504192012.ATXH4XfM-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   drivers/staging/iio/adc/ad7816.c: In function 'ad7816_store_channel':
>> drivers/staging/iio/adc/ad7816.c:222:16: error: invalid storage class for function 'ad7816_show_value'
     222 | static ssize_t ad7816_show_value(struct device *dev,
         |                ^~~~~~~~~~~~~~~~~
   In file included from include/linux/kobject.h:20,
                    from include/linux/energy_model.h:7,
                    from include/linux/device.h:16,
                    from drivers/staging/iio/adc/ad7816.c:10:
>> drivers/staging/iio/adc/ad7816.c:248:37: error: initializer element is not constant
     248 | static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
         |                                     ^~~~~~~~~~~~~~~~~
   include/linux/sysfs.h:233:19: note: in definition of macro '__ATTR'
     233 |         .show   = _show,                                                \
         |                   ^~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   drivers/staging/iio/adc/ad7816.c:248:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     248 | static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
         |        ^~~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:248:37: note: (near initialization for 'iio_dev_attr_value.dev_attr.show')
     248 | static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
         |                                     ^~~~~~~~~~~~~~~~~
   include/linux/sysfs.h:233:19: note: in definition of macro '__ATTR'
     233 |         .show   = _show,                                                \
         |                   ^~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   drivers/staging/iio/adc/ad7816.c:248:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     248 | static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
         |        ^~~~~~~~~~~~~~~
>> drivers/staging/iio/adc/ad7816.c:271:20: error: invalid storage class for function 'ad7816_event_handler'
     271 | static irqreturn_t ad7816_event_handler(int irq, void *private)
         |                    ^~~~~~~~~~~~~~~~~~~~
>> drivers/staging/iio/adc/ad7816.c:278:16: error: invalid storage class for function 'ad7816_show_oti'
     278 | static ssize_t ad7816_show_oti(struct device *dev,
         |                ^~~~~~~~~~~~~~~
>> drivers/staging/iio/adc/ad7816.c:298:23: error: invalid storage class for function 'ad7816_set_oti'
     298 | static inline ssize_t ad7816_set_oti(struct device *dev,
         |                       ^~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:340:24: error: initializer element is not constant
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                        ^~~~~~~~~~~~~~~
   include/linux/sysfs.h:233:19: note: in definition of macro '__ATTR'
     233 |         .show   = _show,                                                \
         |                   ^~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   drivers/staging/iio/adc/ad7816.c:339:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     339 | static IIO_DEVICE_ATTR(oti, 0644,
         |        ^~~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:340:24: note: (near initialization for 'iio_dev_attr_oti.dev_attr.show')
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                        ^~~~~~~~~~~~~~~
   include/linux/sysfs.h:233:19: note: in definition of macro '__ATTR'
     233 |         .show   = _show,                                                \
         |                   ^~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   drivers/staging/iio/adc/ad7816.c:339:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     339 | static IIO_DEVICE_ATTR(oti, 0644,
         |        ^~~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:340:41: error: initializer element is not constant
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                                         ^~~~~~~~~~~~~~
   include/linux/sysfs.h:234:19: note: in definition of macro '__ATTR'
     234 |         .store  = _store,                                               \
         |                   ^~~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   drivers/staging/iio/adc/ad7816.c:339:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     339 | static IIO_DEVICE_ATTR(oti, 0644,
         |        ^~~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:340:41: note: (near initialization for 'iio_dev_attr_oti.dev_attr.store')
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                                         ^~~~~~~~~~~~~~
   include/linux/sysfs.h:234:19: note: in definition of macro '__ATTR'
     234 |         .store  = _store,                                               \
         |                   ^~~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   drivers/staging/iio/adc/ad7816.c:339:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     339 | static IIO_DEVICE_ATTR(oti, 0644,
         |        ^~~~~~~~~~~~~~~
>> drivers/staging/iio/adc/ad7816.c:361:12: error: invalid storage class for function 'ad7816_probe'
     361 | static int ad7816_probe(struct spi_device *spi_dev)
         |            ^~~~~~~~~~~~
>> drivers/staging/iio/adc/ad7816.c:442:1: warning: 'alias' attribute ignored [-Wattributes]
     442 | MODULE_DEVICE_TABLE(of, ad7816_of_match);
         | ^~~~~~~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:451:1: warning: 'alias' attribute ignored [-Wattributes]
     451 | MODULE_DEVICE_TABLE(spi, ad7816_id);
         | ^~~~~~~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:458:18: error: initializer element is not constant
     458 |         .probe = ad7816_probe,
         |                  ^~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:458:18: note: (near initialization for 'ad7816_driver.probe')
   In file included from include/linux/device.h:32:
>> drivers/staging/iio/adc/ad7816.c:461:19: error: invalid storage class for function 'ad7816_driver_init'
     461 | module_spi_driver(ad7816_driver);
         |                   ^~~~~~~~~~~~~
   include/linux/device/driver.h:258:19: note: in definition of macro 'module_driver'
     258 | static int __init __driver##_init(void) \
         |                   ^~~~~~~~
   drivers/staging/iio/adc/ad7816.c:461:1: note: in expansion of macro 'module_spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~
   In file included from include/linux/device/driver.h:21:
>> include/linux/module.h:131:49: error: invalid storage class for function '__inittest'
     131 |         static inline initcall_t __maybe_unused __inittest(void)                \
         |                                                 ^~~~~~~~~~
   include/linux/device/driver.h:262:1: note: in expansion of macro 'module_init'
     262 | module_init(__driver##_init); \
         | ^~~~~~~~~~~
   include/linux/spi/spi.h:387:9: note: in expansion of macro 'module_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:461:1: note: in expansion of macro 'module_spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:461:1: warning: 'alias' attribute ignored [-Wattributes]
>> drivers/staging/iio/adc/ad7816.c:461:19: error: invalid storage class for function 'ad7816_driver_exit'
     461 | module_spi_driver(ad7816_driver);
         |                   ^~~~~~~~~~~~~
   include/linux/device/driver.h:263:20: note: in definition of macro 'module_driver'
     263 | static void __exit __driver##_exit(void) \
         |                    ^~~~~~~~
   drivers/staging/iio/adc/ad7816.c:461:1: note: in expansion of macro 'module_spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~
>> include/linux/module.h:139:49: error: invalid storage class for function '__exittest'
     139 |         static inline exitcall_t __maybe_unused __exittest(void)                \
         |                                                 ^~~~~~~~~~
   include/linux/device/driver.h:267:1: note: in expansion of macro 'module_exit'
     267 | module_exit(__driver##_exit);
         | ^~~~~~~~~~~
   include/linux/spi/spi.h:387:9: note: in expansion of macro 'module_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:461:1: note: in expansion of macro 'module_spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:461:1: warning: 'alias' attribute ignored [-Wattributes]
>> drivers/staging/iio/adc/ad7816.c:465:1: error: expected declaration or statement at end of input
     465 | MODULE_LICENSE("GPL v2");
         | ^~~~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c: At top level:
>> drivers/staging/iio/adc/ad7816.c:193:16: warning: 'ad7816_store_channel' defined but not used [-Wunused-function]
     193 | static ssize_t ad7816_store_channel(struct device *dev,
         |                ^~~~~~~~~~~~~~~~~~~~
--
         |                   ^~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   ad7816.c:248:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     248 | static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
         |        ^~~~~~~~~~~~~~~
   ad7816.c:248:37: note: (near initialization for 'iio_dev_attr_value.dev_attr.show')
     248 | static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
         |                                     ^~~~~~~~~~~~~~~~~
   include/linux/sysfs.h:233:19: note: in definition of macro '__ATTR'
     233 |         .show   = _show,                                                \
         |                   ^~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   ad7816.c:248:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     248 | static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
         |        ^~~~~~~~~~~~~~~
   ad7816.c:271:20: error: invalid storage class for function 'ad7816_event_handler'
     271 | static irqreturn_t ad7816_event_handler(int irq, void *private)
         |                    ^~~~~~~~~~~~~~~~~~~~
   ad7816.c:278:16: error: invalid storage class for function 'ad7816_show_oti'
     278 | static ssize_t ad7816_show_oti(struct device *dev,
         |                ^~~~~~~~~~~~~~~
   ad7816.c:298:23: error: invalid storage class for function 'ad7816_set_oti'
     298 | static inline ssize_t ad7816_set_oti(struct device *dev,
         |                       ^~~~~~~~~~~~~~
   ad7816.c:340:24: error: initializer element is not constant
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                        ^~~~~~~~~~~~~~~
   include/linux/sysfs.h:233:19: note: in definition of macro '__ATTR'
     233 |         .show   = _show,                                                \
         |                   ^~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   ad7816.c:339:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     339 | static IIO_DEVICE_ATTR(oti, 0644,
         |        ^~~~~~~~~~~~~~~
   ad7816.c:340:24: note: (near initialization for 'iio_dev_attr_oti.dev_attr.show')
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                        ^~~~~~~~~~~~~~~
   include/linux/sysfs.h:233:19: note: in definition of macro '__ATTR'
     233 |         .show   = _show,                                                \
         |                   ^~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   ad7816.c:339:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     339 | static IIO_DEVICE_ATTR(oti, 0644,
         |        ^~~~~~~~~~~~~~~
   ad7816.c:340:41: error: initializer element is not constant
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                                         ^~~~~~~~~~~~~~
   include/linux/sysfs.h:234:19: note: in definition of macro '__ATTR'
     234 |         .store  = _store,                                               \
         |                   ^~~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   ad7816.c:339:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     339 | static IIO_DEVICE_ATTR(oti, 0644,
         |        ^~~~~~~~~~~~~~~
   ad7816.c:340:41: note: (near initialization for 'iio_dev_attr_oti.dev_attr.store')
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                                         ^~~~~~~~~~~~~~
   include/linux/sysfs.h:234:19: note: in definition of macro '__ATTR'
     234 |         .store  = _store,                                               \
         |                   ^~~~~~
   include/linux/iio/sysfs.h:72:11: note: in expansion of macro 'IIO_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |           ^~~~~~~~
   ad7816.c:339:8: note: in expansion of macro 'IIO_DEVICE_ATTR'
     339 | static IIO_DEVICE_ATTR(oti, 0644,
         |        ^~~~~~~~~~~~~~~
   ad7816.c:361:12: error: invalid storage class for function 'ad7816_probe'
     361 | static int ad7816_probe(struct spi_device *spi_dev)
         |            ^~~~~~~~~~~~
   ad7816.c:442:1: warning: 'alias' attribute ignored [-Wattributes]
     442 | MODULE_DEVICE_TABLE(of, ad7816_of_match);
         | ^~~~~~~~~~~~~~~~~~~
   ad7816.c:451:1: warning: 'alias' attribute ignored [-Wattributes]
     451 | MODULE_DEVICE_TABLE(spi, ad7816_id);
         | ^~~~~~~~~~~~~~~~~~~
   ad7816.c:458:18: error: initializer element is not constant
     458 |         .probe = ad7816_probe,
         |                  ^~~~~~~~~~~~
   ad7816.c:458:18: note: (near initialization for 'ad7816_driver.probe')
   In file included from include/linux/device.h:32:
   ad7816.c:461:19: error: invalid storage class for function 'ad7816_driver_init'
     461 | module_spi_driver(ad7816_driver);
         |                   ^~~~~~~~~~~~~
   include/linux/device/driver.h:258:19: note: in definition of macro 'module_driver'
     258 | static int __init __driver##_init(void) \
         |                   ^~~~~~~~
   ad7816.c:461:1: note: in expansion of macro 'module_spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~
   In file included from include/linux/device/driver.h:21:
>> include/linux/module.h:131:49: error: invalid storage class for function '__inittest'
     131 |         static inline initcall_t __maybe_unused __inittest(void)                \
         |                                                 ^~~~~~~~~~
   include/linux/device/driver.h:262:1: note: in expansion of macro 'module_init'
     262 | module_init(__driver##_init); \
         | ^~~~~~~~~~~
   include/linux/spi/spi.h:387:9: note: in expansion of macro 'module_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^~~~~~~~~~~~~
   ad7816.c:461:1: note: in expansion of macro 'module_spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~
   ad7816.c:461:1: warning: 'alias' attribute ignored [-Wattributes]
   ad7816.c:461:19: error: invalid storage class for function 'ad7816_driver_exit'
     461 | module_spi_driver(ad7816_driver);
         |                   ^~~~~~~~~~~~~
   include/linux/device/driver.h:263:20: note: in definition of macro 'module_driver'
     263 | static void __exit __driver##_exit(void) \
         |                    ^~~~~~~~
   ad7816.c:461:1: note: in expansion of macro 'module_spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~
>> include/linux/module.h:139:49: error: invalid storage class for function '__exittest'
     139 |         static inline exitcall_t __maybe_unused __exittest(void)                \
         |                                                 ^~~~~~~~~~
   include/linux/device/driver.h:267:1: note: in expansion of macro 'module_exit'
     267 | module_exit(__driver##_exit);
         | ^~~~~~~~~~~
   include/linux/spi/spi.h:387:9: note: in expansion of macro 'module_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^~~~~~~~~~~~~
   ad7816.c:461:1: note: in expansion of macro 'module_spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~
   ad7816.c:461:1: warning: 'alias' attribute ignored [-Wattributes]
   ad7816.c:465:1: error: expected declaration or statement at end of input
     465 | MODULE_LICENSE("GPL v2");
         | ^~~~~~~~~~~~~~
   ad7816.c: At top level:
   ad7816.c:193:16: warning: 'ad7816_store_channel' defined but not used [-Wunused-function]
     193 | static ssize_t ad7816_store_channel(struct device *dev,
         |                ^~~~~~~~~~~~~~~~~~~~


vim +/ad7816_show_value +222 drivers/staging/iio/adc/ad7816.c

7924425db04a61 Sonic Zhang          2010-10-27  192  
7924425db04a61 Sonic Zhang          2010-10-27 @193  static ssize_t ad7816_store_channel(struct device *dev,
7924425db04a61 Sonic Zhang          2010-10-27  194  				    struct device_attribute *attr,
7924425db04a61 Sonic Zhang          2010-10-27  195  				    const char *buf,
7924425db04a61 Sonic Zhang          2010-10-27  196  				    size_t len)
7924425db04a61 Sonic Zhang          2010-10-27  197  {
62c5183971428a Lars-Peter Clausen   2012-05-12  198  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
c746f5c9c52e03 Gabriel Shahrouzi    2025-04-18  199  	struct ad7816_state *chip = iio_priv(indio_dev);
7924425db04a61 Sonic Zhang          2010-10-27  200  	unsigned long data;
7924425db04a61 Sonic Zhang          2010-10-27  201  	int ret;
7924425db04a61 Sonic Zhang          2010-10-27  202  
f86f83622fe2c4 Aida Mynzhasova      2013-05-07  203  	ret = kstrtoul(buf, 10, &data);
7924425db04a61 Sonic Zhang          2010-10-27  204  	if (ret)
f86f83622fe2c4 Aida Mynzhasova      2013-05-07  205  		return ret;
7924425db04a61 Sonic Zhang          2010-10-27  206  
563a61a6448851 Gabriel Shahrouzi    2025-04-18  207  	if (data > chip->chip_info->max_channels && data != AD7816_CS_MASK) {
7924425db04a61 Sonic Zhang          2010-10-27  208  		dev_err(&chip->spi_dev->dev, "Invalid channel id %lu for %s.\n",
563a61a6448851 Gabriel Shahrouzi    2025-04-18  209  			data, chip->chip_info->name);
7924425db04a61 Sonic Zhang          2010-10-27  210  		return -EINVAL;
7924425db04a61 Sonic Zhang          2010-10-27  211  
7924425db04a61 Sonic Zhang          2010-10-27  212  	chip->channel_id = data;
7924425db04a61 Sonic Zhang          2010-10-27  213  
7924425db04a61 Sonic Zhang          2010-10-27  214  	return len;
7924425db04a61 Sonic Zhang          2010-10-27  215  }
7924425db04a61 Sonic Zhang          2010-10-27  216  
7f47d56c5b0500 Julián de Gortari    2017-01-23  217  static IIO_DEVICE_ATTR(channel, 0644,
7924425db04a61 Sonic Zhang          2010-10-27  218  		ad7816_show_channel,
7924425db04a61 Sonic Zhang          2010-10-27  219  		ad7816_store_channel,
7924425db04a61 Sonic Zhang          2010-10-27  220  		0);
7924425db04a61 Sonic Zhang          2010-10-27  221  
7924425db04a61 Sonic Zhang          2010-10-27 @222  static ssize_t ad7816_show_value(struct device *dev,
7924425db04a61 Sonic Zhang          2010-10-27  223  				 struct device_attribute *attr,
7924425db04a61 Sonic Zhang          2010-10-27  224  				 char *buf)
7924425db04a61 Sonic Zhang          2010-10-27  225  {
62c5183971428a Lars-Peter Clausen   2012-05-12  226  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
c746f5c9c52e03 Gabriel Shahrouzi    2025-04-18  227  	struct ad7816_state *chip = iio_priv(indio_dev);
7924425db04a61 Sonic Zhang          2010-10-27  228  	u16 data;
7924425db04a61 Sonic Zhang          2010-10-27  229  	s8 value;
7924425db04a61 Sonic Zhang          2010-10-27  230  	int ret;
7924425db04a61 Sonic Zhang          2010-10-27  231  
7924425db04a61 Sonic Zhang          2010-10-27  232  	ret = ad7816_spi_read(chip, &data);
7924425db04a61 Sonic Zhang          2010-10-27  233  	if (ret)
7924425db04a61 Sonic Zhang          2010-10-27  234  		return -EIO;
7924425db04a61 Sonic Zhang          2010-10-27  235  
7924425db04a61 Sonic Zhang          2010-10-27  236  	data >>= AD7816_VALUE_OFFSET;
7924425db04a61 Sonic Zhang          2010-10-27  237  
7924425db04a61 Sonic Zhang          2010-10-27  238  	if (chip->channel_id == 0) {
7924425db04a61 Sonic Zhang          2010-10-27  239  		value = (s8)((data >> AD7816_TEMP_FLOAT_OFFSET) - 103);
7924425db04a61 Sonic Zhang          2010-10-27  240  		data &= AD7816_TEMP_FLOAT_MASK;
7924425db04a61 Sonic Zhang          2010-10-27  241  		if (value < 0)
e7c3d05459673d Payal Kshirsagar     2019-04-02  242  			data = BIT(AD7816_TEMP_FLOAT_OFFSET) - data;
7924425db04a61 Sonic Zhang          2010-10-27  243  		return sprintf(buf, "%d.%.2d\n", value, data * 25);
da96aecdc59d08 Vaishali Thakkar     2014-09-25  244  	}
7924425db04a61 Sonic Zhang          2010-10-27  245  	return sprintf(buf, "%u\n", data);
7924425db04a61 Sonic Zhang          2010-10-27  246  }
7924425db04a61 Sonic Zhang          2010-10-27  247  
7f47d56c5b0500 Julián de Gortari    2017-01-23 @248  static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
7924425db04a61 Sonic Zhang          2010-10-27  249  
7924425db04a61 Sonic Zhang          2010-10-27  250  static struct attribute *ad7816_attributes[] = {
7924425db04a61 Sonic Zhang          2010-10-27  251  	&iio_dev_attr_available_modes.dev_attr.attr,
7924425db04a61 Sonic Zhang          2010-10-27  252  	&iio_dev_attr_mode.dev_attr.attr,
7924425db04a61 Sonic Zhang          2010-10-27  253  	&iio_dev_attr_channel.dev_attr.attr,
7924425db04a61 Sonic Zhang          2010-10-27  254  	&iio_dev_attr_value.dev_attr.attr,
7924425db04a61 Sonic Zhang          2010-10-27  255  	NULL,
7924425db04a61 Sonic Zhang          2010-10-27  256  };
7924425db04a61 Sonic Zhang          2010-10-27  257  
7924425db04a61 Sonic Zhang          2010-10-27  258  static const struct attribute_group ad7816_attribute_group = {
7924425db04a61 Sonic Zhang          2010-10-27  259  	.attrs = ad7816_attributes,
7924425db04a61 Sonic Zhang          2010-10-27  260  };
7924425db04a61 Sonic Zhang          2010-10-27  261  
7924425db04a61 Sonic Zhang          2010-10-27  262  /*
7924425db04a61 Sonic Zhang          2010-10-27  263   * temperature bound events
7924425db04a61 Sonic Zhang          2010-10-27  264   */
7924425db04a61 Sonic Zhang          2010-10-27  265  
c4b14d99bbc93c Jonathan Cameron     2011-08-12  266  #define IIO_EVENT_CODE_AD7816_OTI IIO_UNMOD_EVENT_CODE(IIO_TEMP,	\
0bb8be643161ae Jonathan Cameron     2011-05-18  267  						       0,		\
0bb8be643161ae Jonathan Cameron     2011-05-18  268  						       IIO_EV_TYPE_THRESH, \
0bb8be643161ae Jonathan Cameron     2011-05-18  269  						       IIO_EV_DIR_FALLING)
7924425db04a61 Sonic Zhang          2010-10-27  270  
db9afe2fc0c59f Jonathan Cameron     2011-05-18 @271  static irqreturn_t ad7816_event_handler(int irq, void *private)
7924425db04a61 Sonic Zhang          2010-10-27  272  {
bc2b7dab629a51 Gregor Boirie        2016-03-09  273  	iio_push_event(private, IIO_EVENT_CODE_AD7816_OTI,
bd28425a307417 Arushi Singhal       2018-03-07  274  		       iio_get_time_ns(private));
db9afe2fc0c59f Jonathan Cameron     2011-05-18  275  	return IRQ_HANDLED;
7924425db04a61 Sonic Zhang          2010-10-27  276  }
7924425db04a61 Sonic Zhang          2010-10-27  277  
7924425db04a61 Sonic Zhang          2010-10-27 @278  static ssize_t ad7816_show_oti(struct device *dev,
7924425db04a61 Sonic Zhang          2010-10-27  279  			       struct device_attribute *attr,
7924425db04a61 Sonic Zhang          2010-10-27  280  			       char *buf)
7924425db04a61 Sonic Zhang          2010-10-27  281  {
62c5183971428a Lars-Peter Clausen   2012-05-12  282  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
c746f5c9c52e03 Gabriel Shahrouzi    2025-04-18  283  	struct ad7816_state *chip = iio_priv(indio_dev);
7924425db04a61 Sonic Zhang          2010-10-27  284  	int value;
7924425db04a61 Sonic Zhang          2010-10-27  285  
7924425db04a61 Sonic Zhang          2010-10-27  286  	if (chip->channel_id > AD7816_CS_MAX) {
7924425db04a61 Sonic Zhang          2010-10-27  287  		dev_err(dev, "Invalid oti channel id %d.\n", chip->channel_id);
7924425db04a61 Sonic Zhang          2010-10-27  288  		return -EINVAL;
7924425db04a61 Sonic Zhang          2010-10-27  289  	} else if (chip->channel_id == 0) {
7924425db04a61 Sonic Zhang          2010-10-27  290  		value = AD7816_BOUND_VALUE_MIN +
7924425db04a61 Sonic Zhang          2010-10-27  291  			(chip->oti_data[chip->channel_id] -
7924425db04a61 Sonic Zhang          2010-10-27  292  			AD7816_BOUND_VALUE_BASE);
7924425db04a61 Sonic Zhang          2010-10-27  293  		return sprintf(buf, "%d\n", value);
da96aecdc59d08 Vaishali Thakkar     2014-09-25  294  	}
7924425db04a61 Sonic Zhang          2010-10-27  295  	return sprintf(buf, "%u\n", chip->oti_data[chip->channel_id]);
7924425db04a61 Sonic Zhang          2010-10-27  296  }
7924425db04a61 Sonic Zhang          2010-10-27  297  
7924425db04a61 Sonic Zhang          2010-10-27 @298  static inline ssize_t ad7816_set_oti(struct device *dev,
7924425db04a61 Sonic Zhang          2010-10-27  299  				     struct device_attribute *attr,
7924425db04a61 Sonic Zhang          2010-10-27  300  				     const char *buf,
7924425db04a61 Sonic Zhang          2010-10-27  301  				     size_t len)
7924425db04a61 Sonic Zhang          2010-10-27  302  {
62c5183971428a Lars-Peter Clausen   2012-05-12  303  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
c746f5c9c52e03 Gabriel Shahrouzi    2025-04-18  304  	struct ad7816_state *chip = iio_priv(indio_dev);
7924425db04a61 Sonic Zhang          2010-10-27  305  	long value;
7924425db04a61 Sonic Zhang          2010-10-27  306  	u8 data;
7924425db04a61 Sonic Zhang          2010-10-27  307  	int ret;
7924425db04a61 Sonic Zhang          2010-10-27  308  
f86f83622fe2c4 Aida Mynzhasova      2013-05-07  309  	ret = kstrtol(buf, 10, &value);
f86f83622fe2c4 Aida Mynzhasova      2013-05-07  310  	if (ret)
f86f83622fe2c4 Aida Mynzhasova      2013-05-07  311  		return ret;
7924425db04a61 Sonic Zhang          2010-10-27  312  
7924425db04a61 Sonic Zhang          2010-10-27  313  	if (chip->channel_id > AD7816_CS_MAX) {
7924425db04a61 Sonic Zhang          2010-10-27  314  		dev_err(dev, "Invalid oti channel id %d.\n", chip->channel_id);
7924425db04a61 Sonic Zhang          2010-10-27  315  		return -EINVAL;
7924425db04a61 Sonic Zhang          2010-10-27  316  	} else if (chip->channel_id == 0) {
0fd736f9f4d2ae Amitoj Kaur Chawla   2016-02-16  317  		if (value < AD7816_BOUND_VALUE_MIN ||
7924425db04a61 Sonic Zhang          2010-10-27  318  		    value > AD7816_BOUND_VALUE_MAX)
7924425db04a61 Sonic Zhang          2010-10-27  319  			return -EINVAL;
7924425db04a61 Sonic Zhang          2010-10-27  320  
7924425db04a61 Sonic Zhang          2010-10-27  321  		data = (u8)(value - AD7816_BOUND_VALUE_MIN +
7924425db04a61 Sonic Zhang          2010-10-27  322  			AD7816_BOUND_VALUE_BASE);
7924425db04a61 Sonic Zhang          2010-10-27  323  	} else {
0fd736f9f4d2ae Amitoj Kaur Chawla   2016-02-16  324  		if (value < AD7816_BOUND_VALUE_BASE || value > 255)
7924425db04a61 Sonic Zhang          2010-10-27  325  			return -EINVAL;
7924425db04a61 Sonic Zhang          2010-10-27  326  
7924425db04a61 Sonic Zhang          2010-10-27  327  		data = (u8)value;
7924425db04a61 Sonic Zhang          2010-10-27  328  	}
7924425db04a61 Sonic Zhang          2010-10-27  329  
7924425db04a61 Sonic Zhang          2010-10-27  330  	ret = ad7816_spi_write(chip, data);
7924425db04a61 Sonic Zhang          2010-10-27  331  	if (ret)
7924425db04a61 Sonic Zhang          2010-10-27  332  		return -EIO;
7924425db04a61 Sonic Zhang          2010-10-27  333  
7924425db04a61 Sonic Zhang          2010-10-27  334  	chip->oti_data[chip->channel_id] = data;
7924425db04a61 Sonic Zhang          2010-10-27  335  
7924425db04a61 Sonic Zhang          2010-10-27  336  	return len;
7924425db04a61 Sonic Zhang          2010-10-27  337  }
7924425db04a61 Sonic Zhang          2010-10-27  338  
7f47d56c5b0500 Julián de Gortari    2017-01-23 @339  static IIO_DEVICE_ATTR(oti, 0644,
7924425db04a61 Sonic Zhang          2010-10-27  340  		       ad7816_show_oti, ad7816_set_oti, 0);
7924425db04a61 Sonic Zhang          2010-10-27  341  
7924425db04a61 Sonic Zhang          2010-10-27  342  static struct attribute *ad7816_event_attributes[] = {
db9afe2fc0c59f Jonathan Cameron     2011-05-18  343  	&iio_dev_attr_oti.dev_attr.attr,
7924425db04a61 Sonic Zhang          2010-10-27  344  	NULL,
7924425db04a61 Sonic Zhang          2010-10-27  345  };
7924425db04a61 Sonic Zhang          2010-10-27  346  
0fa90023f23ced Bhumika Goyal        2016-10-02  347  static const struct attribute_group ad7816_event_attribute_group = {
7924425db04a61 Sonic Zhang          2010-10-27  348  	.attrs = ad7816_event_attributes,
8e7d967244a8ee Jonathan Cameron     2011-08-30  349  	.name = "events",
7924425db04a61 Sonic Zhang          2010-10-27  350  };
7924425db04a61 Sonic Zhang          2010-10-27  351  
6fe8135fccd66a Jonathan Cameron     2011-05-18  352  static const struct iio_info ad7816_info = {
6fe8135fccd66a Jonathan Cameron     2011-05-18  353  	.attrs = &ad7816_attribute_group,
6fe8135fccd66a Jonathan Cameron     2011-05-18  354  	.event_attrs = &ad7816_event_attribute_group,
6fe8135fccd66a Jonathan Cameron     2011-05-18  355  };
6fe8135fccd66a Jonathan Cameron     2011-05-18  356  
7924425db04a61 Sonic Zhang          2010-10-27  357  /*
7924425db04a61 Sonic Zhang          2010-10-27  358   * device probe and remove
7924425db04a61 Sonic Zhang          2010-10-27  359   */
7924425db04a61 Sonic Zhang          2010-10-27  360  
4ae1c61ff2ba4f Bill Pemberton       2012-11-19 @361  static int ad7816_probe(struct spi_device *spi_dev)
7924425db04a61 Sonic Zhang          2010-10-27  362  {
c746f5c9c52e03 Gabriel Shahrouzi    2025-04-18  363  	struct ad7816_state *chip;
b0011d6dbae18a Jonathan Cameron     2011-06-27  364  	struct iio_dev *indio_dev;
fc79e5b62248ee Gabriel Shahrouzi    2025-04-18  365  	const struct ad7816_chip_info *info;
f1b753a0f866a8 Hardik Singh Rathore 2018-12-12  366  	int i, ret;
7924425db04a61 Sonic Zhang          2010-10-27  367  
e5bf4f5b7d95ff Sachin Kamat         2013-08-31  368  	indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
e5bf4f5b7d95ff Sachin Kamat         2013-08-31  369  	if (!indio_dev)
e5bf4f5b7d95ff Sachin Kamat         2013-08-31  370  		return -ENOMEM;
b0011d6dbae18a Jonathan Cameron     2011-06-27  371  	chip = iio_priv(indio_dev);
7924425db04a61 Sonic Zhang          2010-10-27  372  	/* this is only used for device removal purposes */
b0011d6dbae18a Jonathan Cameron     2011-06-27  373  	dev_set_drvdata(&spi_dev->dev, indio_dev);
7924425db04a61 Sonic Zhang          2010-10-27  374  
fc79e5b62248ee Gabriel Shahrouzi    2025-04-18  375  	info = device_get_match_data(&spi_dev->dev);
fc79e5b62248ee Gabriel Shahrouzi    2025-04-18  376  	if (!info)
fc79e5b62248ee Gabriel Shahrouzi    2025-04-18  377  		return -ENODEV;
fc79e5b62248ee Gabriel Shahrouzi    2025-04-18  378  	chip->chip_info = info;
fc79e5b62248ee Gabriel Shahrouzi    2025-04-18  379  
7924425db04a61 Sonic Zhang          2010-10-27  380  	chip->spi_dev = spi_dev;
7924425db04a61 Sonic Zhang          2010-10-27  381  	for (i = 0; i <= AD7816_CS_MAX; i++)
7924425db04a61 Sonic Zhang          2010-10-27  382  		chip->oti_data[i] = 203;
073a391ca0352d Nishad Kamdar        2018-10-17  383  
72e3a5248da904 Nishad Kamdar        2018-11-09  384  	chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_OUT_HIGH);
073a391ca0352d Nishad Kamdar        2018-10-17  385  	if (IS_ERR(chip->rdwr_pin)) {
073a391ca0352d Nishad Kamdar        2018-10-17  386  		ret = PTR_ERR(chip->rdwr_pin);
073a391ca0352d Nishad Kamdar        2018-10-17  387  		dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
073a391ca0352d Nishad Kamdar        2018-10-17  388  			ret);
e5bf4f5b7d95ff Sachin Kamat         2013-08-31  389  		return ret;
7924425db04a61 Sonic Zhang          2010-10-27  390  	}
72e3a5248da904 Nishad Kamdar        2018-11-09  391  	chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert",
72e3a5248da904 Nishad Kamdar        2018-11-09  392  					   GPIOD_OUT_HIGH);
073a391ca0352d Nishad Kamdar        2018-10-17  393  	if (IS_ERR(chip->convert_pin)) {
073a391ca0352d Nishad Kamdar        2018-10-17  394  		ret = PTR_ERR(chip->convert_pin);
073a391ca0352d Nishad Kamdar        2018-10-17  395  		dev_err(&spi_dev->dev, "Failed to request convert GPIO: %d\n",
073a391ca0352d Nishad Kamdar        2018-10-17  396  			ret);
e5bf4f5b7d95ff Sachin Kamat         2013-08-31  397  		return ret;
7924425db04a61 Sonic Zhang          2010-10-27  398  	}
fc79e5b62248ee Gabriel Shahrouzi    2025-04-18  399  	if (chip->chip_info == &ad7816_info_ad7816 || chip->chip_info == &ad7817_info_ad7817) {
06c77f564ddb6a Nishad Kamdar        2018-11-09  400  		chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy",
06c77f564ddb6a Nishad Kamdar        2018-11-09  401  						GPIOD_IN);
073a391ca0352d Nishad Kamdar        2018-10-17  402  		if (IS_ERR(chip->busy_pin)) {
073a391ca0352d Nishad Kamdar        2018-10-17  403  			ret = PTR_ERR(chip->busy_pin);
073a391ca0352d Nishad Kamdar        2018-10-17  404  			dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
073a391ca0352d Nishad Kamdar        2018-10-17  405  				ret);
e5bf4f5b7d95ff Sachin Kamat         2013-08-31  406  			return ret;
7924425db04a61 Sonic Zhang          2010-10-27  407  		}
06c77f564ddb6a Nishad Kamdar        2018-11-09  408  	}
7924425db04a61 Sonic Zhang          2010-10-27  409  
fc79e5b62248ee Gabriel Shahrouzi    2025-04-18  410  	indio_dev->name = chip->chip_info->name;
b0011d6dbae18a Jonathan Cameron     2011-06-27  411  	indio_dev->info = &ad7816_info;
b0011d6dbae18a Jonathan Cameron     2011-06-27  412  	indio_dev->modes = INDIO_DIRECT_MODE;
7924425db04a61 Sonic Zhang          2010-10-27  413  
7924425db04a61 Sonic Zhang          2010-10-27  414  	if (spi_dev->irq) {
7924425db04a61 Sonic Zhang          2010-10-27  415  		/* Only low trigger is supported in ad7816/7/8 */
e5bf4f5b7d95ff Sachin Kamat         2013-08-31  416  		ret = devm_request_threaded_irq(&spi_dev->dev, spi_dev->irq,
db9afe2fc0c59f Jonathan Cameron     2011-05-18  417  						NULL,
db9afe2fc0c59f Jonathan Cameron     2011-05-18  418  						&ad7816_event_handler,
a91aff1c09fc41 Lars-Peter Clausen   2012-07-02  419  						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
b0011d6dbae18a Jonathan Cameron     2011-06-27  420  						indio_dev->name,
b0011d6dbae18a Jonathan Cameron     2011-06-27  421  						indio_dev);
7924425db04a61 Sonic Zhang          2010-10-27  422  		if (ret)
e5bf4f5b7d95ff Sachin Kamat         2013-08-31  423  			return ret;
7924425db04a61 Sonic Zhang          2010-10-27  424  	}
7924425db04a61 Sonic Zhang          2010-10-27  425  
5404dc77266e31 Sachin Kamat         2013-10-29  426  	ret = devm_iio_device_register(&spi_dev->dev, indio_dev);
26d25ae3f0d8ff Jonathan Cameron     2011-09-02  427  	if (ret)
e5bf4f5b7d95ff Sachin Kamat         2013-08-31  428  		return ret;
26d25ae3f0d8ff Jonathan Cameron     2011-09-02  429  
7924425db04a61 Sonic Zhang          2010-10-27  430  	dev_info(&spi_dev->dev, "%s temperature sensor and ADC registered.\n",
b0011d6dbae18a Jonathan Cameron     2011-06-27  431  		 indio_dev->name);
7924425db04a61 Sonic Zhang          2010-10-27  432  
7924425db04a61 Sonic Zhang          2010-10-27  433  	return 0;
7924425db04a61 Sonic Zhang          2010-10-27  434  }
7924425db04a61 Sonic Zhang          2010-10-27  435  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info
  2025-04-18 20:47 ` [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info Gabriel Shahrouzi
  2025-04-19 12:19   ` kernel test robot
@ 2025-04-19 13:11   ` kernel test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2025-04-19 13:11 UTC (permalink / raw)
  To: Gabriel Shahrouzi, gregkh, jic23, lars, linux-iio, linux-kernel,
	linux-staging, Michael.Hennerich, sonic.zhang, vapier
  Cc: llvm, oe-kbuild-all, gshahrouzi, skhan, linux-kernel-mentees

Hi Gabriel,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Gabriel-Shahrouzi/staging-iio-adc-ad7816-Allow-channel-7-for-all-devices/20250419-045531
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/fad83a7efb12c0f40dc2660cf9dd4c57422ecff9.1745007964.git.gshahrouzi%40gmail.com
patch subject: [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info
config: powerpc64-randconfig-003-20250419 (https://download.01.org/0day-ci/archive/20250419/202504192054.5XE1L0Lo-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250419/202504192054.5XE1L0Lo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504192054.5XE1L0Lo-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/staging/iio/adc/ad7816.c:225:1: error: function definition is not allowed here
     225 | {
         | ^
>> drivers/staging/iio/adc/ad7816.c:248:37: error: use of undeclared identifier 'ad7816_show_value'; did you mean 'ad7816_show_mode'?
     248 | static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
         |                                     ^~~~~~~~~~~~~~~~~
         |                                     ad7816_show_mode
   include/linux/iio/sysfs.h:72:27: note: expanded from macro 'IIO_DEVICE_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |                                  ^
   include/linux/iio/sysfs.h:55:37: note: expanded from macro 'IIO_ATTR'
      55 |         { .dev_attr = __ATTR(_name, _mode, _show, _store),      \
         |                                            ^
   include/linux/sysfs.h:233:10: note: expanded from macro '__ATTR'
     233 |         .show   = _show,                                                \
         |                   ^
   drivers/staging/iio/adc/ad7816.c:137:16: note: 'ad7816_show_mode' declared here
     137 | static ssize_t ad7816_show_mode(struct device *dev,
         |                ^
   drivers/staging/iio/adc/ad7816.c:272:1: error: function definition is not allowed here
     272 | {
         | ^
   drivers/staging/iio/adc/ad7816.c:281:1: error: function definition is not allowed here
     281 | {
         | ^
   drivers/staging/iio/adc/ad7816.c:302:1: error: function definition is not allowed here
     302 | {
         | ^
>> drivers/staging/iio/adc/ad7816.c:340:10: error: use of undeclared identifier 'ad7816_show_oti'; did you mean 'ad7816_show_mode'?
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                        ^~~~~~~~~~~~~~~
         |                        ad7816_show_mode
   include/linux/iio/sysfs.h:72:27: note: expanded from macro 'IIO_DEVICE_ATTR'
      72 |         = IIO_ATTR(_name, _mode, _show, _store, _addr)
         |                                  ^
   include/linux/iio/sysfs.h:55:37: note: expanded from macro 'IIO_ATTR'
      55 |         { .dev_attr = __ATTR(_name, _mode, _show, _store),      \
         |                                            ^
   include/linux/sysfs.h:233:10: note: expanded from macro '__ATTR'
     233 |         .show   = _show,                                                \
         |                   ^
   drivers/staging/iio/adc/ad7816.c:137:16: note: 'ad7816_show_mode' declared here
     137 | static ssize_t ad7816_show_mode(struct device *dev,
         |                ^
>> drivers/staging/iio/adc/ad7816.c:340:27: error: use of undeclared identifier 'ad7816_set_oti'
     340 |                        ad7816_show_oti, ad7816_set_oti, 0);
         |                                         ^
   drivers/staging/iio/adc/ad7816.c:362:1: error: function definition is not allowed here
     362 | {
         | ^
>> drivers/staging/iio/adc/ad7816.c:458:11: error: use of undeclared identifier 'ad7816_probe'; did you mean 'ad7816_driver'?
     458 |         .probe = ad7816_probe,
         |                  ^~~~~~~~~~~~
         |                  ad7816_driver
   drivers/staging/iio/adc/ad7816.c:453:26: note: 'ad7816_driver' declared here
     453 | static struct spi_driver ad7816_driver = {
         |                          ^
>> drivers/staging/iio/adc/ad7816.c:458:11: error: initializing 'int (*)(struct spi_device *)' with an expression of incompatible type 'struct spi_driver'
     458 |         .probe = ad7816_probe,
         |                  ^~~~~~~~~~~~
   drivers/staging/iio/adc/ad7816.c:461:1: error: function definition is not allowed here
     461 | module_spi_driver(ad7816_driver);
         | ^
   include/linux/spi/spi.h:387:2: note: expanded from macro 'module_spi_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^
   include/linux/device/driver.h:258:41: note: expanded from macro 'module_driver'
     258 | static int __init __driver##_init(void) \
         |                                         ^
>> drivers/staging/iio/adc/ad7816.c:461:1: error: use of undeclared identifier 'ad7816_driver_init'; did you mean 'ad7816_driver'?
   include/linux/spi/spi.h:387:2: note: expanded from macro 'module_spi_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^
   include/linux/device/driver.h:262:13: note: expanded from macro 'module_driver'
     262 | module_init(__driver##_init); \
         |             ^
   <scratch space>:91:1: note: expanded from here
      91 | ad7816_driver_init
         | ^
   drivers/staging/iio/adc/ad7816.c:453:26: note: 'ad7816_driver' declared here
     453 | static struct spi_driver ad7816_driver = {
         |                          ^
>> drivers/staging/iio/adc/ad7816.c:461:1: error: initializing 'initcall_t' (aka 'int (*)(void)') with an expression of incompatible type 'struct spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/spi/spi.h:387:2: note: expanded from macro 'module_spi_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     388 |                         spi_unregister_driver)
         |                         ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/device/driver.h:261:3: note: expanded from macro 'module_driver'
     261 | } \
         |   ^
     262 | module_init(__driver##_init); \
         |             ~~~~~~~~~~~~~~~
   include/linux/module.h:88:24: note: expanded from macro '\
   module_init'
      88 | #define module_init(x)  __initcall(x);
         |                         ^          ~
   note: (skipping 7 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   <scratch space>:104:1: note: expanded from here
     104 | __initcall__kmod_ad7816__326_461_ad7816_driver_init6
         | ^
   include/linux/init.h:269:20: note: expanded from macro '____define_initcall'
     269 |         static initcall_t __name __used                         \
         |                           ^
     270 |                 __attribute__((__section__(__sec))) = fn;
         |                                                       ~~
   drivers/staging/iio/adc/ad7816.c:461:1: error: function definition is not allowed here
   include/linux/spi/spi.h:387:2: note: expanded from macro 'module_spi_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^
   include/linux/device/driver.h:263:42: note: expanded from macro 'module_driver'
     263 | static void __exit __driver##_exit(void) \
         |                                          ^
>> drivers/staging/iio/adc/ad7816.c:461:1: error: use of undeclared identifier 'ad7816_driver_exit'; did you mean 'ad7816_driver'?
   include/linux/spi/spi.h:387:2: note: expanded from macro 'module_spi_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^
   include/linux/device/driver.h:267:13: note: expanded from macro 'module_driver'
     267 | module_exit(__driver##_exit);
         |             ^
   <scratch space>:107:1: note: expanded from here
     107 | ad7816_driver_exit
         | ^
   drivers/staging/iio/adc/ad7816.c:453:26: note: 'ad7816_driver' declared here
     453 | static struct spi_driver ad7816_driver = {
         |                          ^
>> drivers/staging/iio/adc/ad7816.c:461:1: error: initializing 'exitcall_t' (aka 'void (*)(void)') with an expression of incompatible type 'struct spi_driver'
     461 | module_spi_driver(ad7816_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/spi/spi.h:387:2: note: expanded from macro 'module_spi_driver'
     387 |         module_driver(__spi_driver, spi_register_driver, \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     388 |                         spi_unregister_driver)
         |                         ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/device/driver.h:266:3: note: expanded from macro 'module_driver'
     266 | } \
         |   ^
     267 | module_exit(__driver##_exit);
         |             ~~~~~~~~~~~~~~~
   include/linux/module.h:100:24: note: expanded from macro '\
   module_exit'
     100 | #define module_exit(x)  __exitcall(x);
         |                         ^          ~
   include/linux/init.h:319:20: note: expanded from macro '__exitcall'
     319 |         static exitcall_t __exitcall_##fn __exit_call = fn
         |                           ^                             ~~
   <scratch space>:108:1: note: expanded from here
     108 | __exitcall_ad7816_driver_exit
         | ^
>> drivers/staging/iio/adc/ad7816.c:465:26: error: expected '}'
     465 | MODULE_LICENSE("GPL v2");
         |                          ^
   drivers/staging/iio/adc/ad7816.c:197:1: note: to match this '{'
     197 | {
         | ^
   17 errors generated.


vim +225 drivers/staging/iio/adc/ad7816.c

7924425db04a61 Sonic Zhang        2010-10-27  216  
7f47d56c5b0500 Julián de Gortari  2017-01-23  217  static IIO_DEVICE_ATTR(channel, 0644,
7924425db04a61 Sonic Zhang        2010-10-27  218  		ad7816_show_channel,
7924425db04a61 Sonic Zhang        2010-10-27  219  		ad7816_store_channel,
7924425db04a61 Sonic Zhang        2010-10-27  220  		0);
7924425db04a61 Sonic Zhang        2010-10-27  221  
7924425db04a61 Sonic Zhang        2010-10-27  222  static ssize_t ad7816_show_value(struct device *dev,
7924425db04a61 Sonic Zhang        2010-10-27  223  				 struct device_attribute *attr,
7924425db04a61 Sonic Zhang        2010-10-27  224  				 char *buf)
7924425db04a61 Sonic Zhang        2010-10-27 @225  {
62c5183971428a Lars-Peter Clausen 2012-05-12  226  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
c746f5c9c52e03 Gabriel Shahrouzi  2025-04-18  227  	struct ad7816_state *chip = iio_priv(indio_dev);
7924425db04a61 Sonic Zhang        2010-10-27  228  	u16 data;
7924425db04a61 Sonic Zhang        2010-10-27  229  	s8 value;
7924425db04a61 Sonic Zhang        2010-10-27  230  	int ret;
7924425db04a61 Sonic Zhang        2010-10-27  231  
7924425db04a61 Sonic Zhang        2010-10-27  232  	ret = ad7816_spi_read(chip, &data);
7924425db04a61 Sonic Zhang        2010-10-27  233  	if (ret)
7924425db04a61 Sonic Zhang        2010-10-27  234  		return -EIO;
7924425db04a61 Sonic Zhang        2010-10-27  235  
7924425db04a61 Sonic Zhang        2010-10-27  236  	data >>= AD7816_VALUE_OFFSET;
7924425db04a61 Sonic Zhang        2010-10-27  237  
7924425db04a61 Sonic Zhang        2010-10-27  238  	if (chip->channel_id == 0) {
7924425db04a61 Sonic Zhang        2010-10-27  239  		value = (s8)((data >> AD7816_TEMP_FLOAT_OFFSET) - 103);
7924425db04a61 Sonic Zhang        2010-10-27  240  		data &= AD7816_TEMP_FLOAT_MASK;
7924425db04a61 Sonic Zhang        2010-10-27  241  		if (value < 0)
e7c3d05459673d Payal Kshirsagar   2019-04-02  242  			data = BIT(AD7816_TEMP_FLOAT_OFFSET) - data;
7924425db04a61 Sonic Zhang        2010-10-27  243  		return sprintf(buf, "%d.%.2d\n", value, data * 25);
da96aecdc59d08 Vaishali Thakkar   2014-09-25  244  	}
7924425db04a61 Sonic Zhang        2010-10-27  245  	return sprintf(buf, "%u\n", data);
7924425db04a61 Sonic Zhang        2010-10-27  246  }
7924425db04a61 Sonic Zhang        2010-10-27  247  
7f47d56c5b0500 Julián de Gortari  2017-01-23 @248  static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
7924425db04a61 Sonic Zhang        2010-10-27  249  
7924425db04a61 Sonic Zhang        2010-10-27  250  static struct attribute *ad7816_attributes[] = {
7924425db04a61 Sonic Zhang        2010-10-27  251  	&iio_dev_attr_available_modes.dev_attr.attr,
7924425db04a61 Sonic Zhang        2010-10-27  252  	&iio_dev_attr_mode.dev_attr.attr,
7924425db04a61 Sonic Zhang        2010-10-27  253  	&iio_dev_attr_channel.dev_attr.attr,
7924425db04a61 Sonic Zhang        2010-10-27  254  	&iio_dev_attr_value.dev_attr.attr,
7924425db04a61 Sonic Zhang        2010-10-27  255  	NULL,
7924425db04a61 Sonic Zhang        2010-10-27  256  };
7924425db04a61 Sonic Zhang        2010-10-27  257  
7924425db04a61 Sonic Zhang        2010-10-27  258  static const struct attribute_group ad7816_attribute_group = {
7924425db04a61 Sonic Zhang        2010-10-27  259  	.attrs = ad7816_attributes,
7924425db04a61 Sonic Zhang        2010-10-27  260  };
7924425db04a61 Sonic Zhang        2010-10-27  261  
7924425db04a61 Sonic Zhang        2010-10-27  262  /*
7924425db04a61 Sonic Zhang        2010-10-27  263   * temperature bound events
7924425db04a61 Sonic Zhang        2010-10-27  264   */
7924425db04a61 Sonic Zhang        2010-10-27  265  
c4b14d99bbc93c Jonathan Cameron   2011-08-12  266  #define IIO_EVENT_CODE_AD7816_OTI IIO_UNMOD_EVENT_CODE(IIO_TEMP,	\
0bb8be643161ae Jonathan Cameron   2011-05-18  267  						       0,		\
0bb8be643161ae Jonathan Cameron   2011-05-18  268  						       IIO_EV_TYPE_THRESH, \
0bb8be643161ae Jonathan Cameron   2011-05-18  269  						       IIO_EV_DIR_FALLING)
7924425db04a61 Sonic Zhang        2010-10-27  270  
db9afe2fc0c59f Jonathan Cameron   2011-05-18  271  static irqreturn_t ad7816_event_handler(int irq, void *private)
7924425db04a61 Sonic Zhang        2010-10-27  272  {
bc2b7dab629a51 Gregor Boirie      2016-03-09  273  	iio_push_event(private, IIO_EVENT_CODE_AD7816_OTI,
bd28425a307417 Arushi Singhal     2018-03-07  274  		       iio_get_time_ns(private));
db9afe2fc0c59f Jonathan Cameron   2011-05-18  275  	return IRQ_HANDLED;
7924425db04a61 Sonic Zhang        2010-10-27  276  }
7924425db04a61 Sonic Zhang        2010-10-27  277  
7924425db04a61 Sonic Zhang        2010-10-27  278  static ssize_t ad7816_show_oti(struct device *dev,
7924425db04a61 Sonic Zhang        2010-10-27  279  			       struct device_attribute *attr,
7924425db04a61 Sonic Zhang        2010-10-27  280  			       char *buf)
7924425db04a61 Sonic Zhang        2010-10-27  281  {
62c5183971428a Lars-Peter Clausen 2012-05-12  282  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
c746f5c9c52e03 Gabriel Shahrouzi  2025-04-18  283  	struct ad7816_state *chip = iio_priv(indio_dev);
7924425db04a61 Sonic Zhang        2010-10-27  284  	int value;
7924425db04a61 Sonic Zhang        2010-10-27  285  
7924425db04a61 Sonic Zhang        2010-10-27  286  	if (chip->channel_id > AD7816_CS_MAX) {
7924425db04a61 Sonic Zhang        2010-10-27  287  		dev_err(dev, "Invalid oti channel id %d.\n", chip->channel_id);
7924425db04a61 Sonic Zhang        2010-10-27  288  		return -EINVAL;
7924425db04a61 Sonic Zhang        2010-10-27  289  	} else if (chip->channel_id == 0) {
7924425db04a61 Sonic Zhang        2010-10-27  290  		value = AD7816_BOUND_VALUE_MIN +
7924425db04a61 Sonic Zhang        2010-10-27  291  			(chip->oti_data[chip->channel_id] -
7924425db04a61 Sonic Zhang        2010-10-27  292  			AD7816_BOUND_VALUE_BASE);
7924425db04a61 Sonic Zhang        2010-10-27  293  		return sprintf(buf, "%d\n", value);
da96aecdc59d08 Vaishali Thakkar   2014-09-25  294  	}
7924425db04a61 Sonic Zhang        2010-10-27  295  	return sprintf(buf, "%u\n", chip->oti_data[chip->channel_id]);
7924425db04a61 Sonic Zhang        2010-10-27  296  }
7924425db04a61 Sonic Zhang        2010-10-27  297  
7924425db04a61 Sonic Zhang        2010-10-27  298  static inline ssize_t ad7816_set_oti(struct device *dev,
7924425db04a61 Sonic Zhang        2010-10-27  299  				     struct device_attribute *attr,
7924425db04a61 Sonic Zhang        2010-10-27  300  				     const char *buf,
7924425db04a61 Sonic Zhang        2010-10-27  301  				     size_t len)
7924425db04a61 Sonic Zhang        2010-10-27  302  {
62c5183971428a Lars-Peter Clausen 2012-05-12  303  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
c746f5c9c52e03 Gabriel Shahrouzi  2025-04-18  304  	struct ad7816_state *chip = iio_priv(indio_dev);
7924425db04a61 Sonic Zhang        2010-10-27  305  	long value;
7924425db04a61 Sonic Zhang        2010-10-27  306  	u8 data;
7924425db04a61 Sonic Zhang        2010-10-27  307  	int ret;
7924425db04a61 Sonic Zhang        2010-10-27  308  
f86f83622fe2c4 Aida Mynzhasova    2013-05-07  309  	ret = kstrtol(buf, 10, &value);
f86f83622fe2c4 Aida Mynzhasova    2013-05-07  310  	if (ret)
f86f83622fe2c4 Aida Mynzhasova    2013-05-07  311  		return ret;
7924425db04a61 Sonic Zhang        2010-10-27  312  
7924425db04a61 Sonic Zhang        2010-10-27  313  	if (chip->channel_id > AD7816_CS_MAX) {
7924425db04a61 Sonic Zhang        2010-10-27  314  		dev_err(dev, "Invalid oti channel id %d.\n", chip->channel_id);
7924425db04a61 Sonic Zhang        2010-10-27  315  		return -EINVAL;
7924425db04a61 Sonic Zhang        2010-10-27  316  	} else if (chip->channel_id == 0) {
0fd736f9f4d2ae Amitoj Kaur Chawla 2016-02-16  317  		if (value < AD7816_BOUND_VALUE_MIN ||
7924425db04a61 Sonic Zhang        2010-10-27  318  		    value > AD7816_BOUND_VALUE_MAX)
7924425db04a61 Sonic Zhang        2010-10-27  319  			return -EINVAL;
7924425db04a61 Sonic Zhang        2010-10-27  320  
7924425db04a61 Sonic Zhang        2010-10-27  321  		data = (u8)(value - AD7816_BOUND_VALUE_MIN +
7924425db04a61 Sonic Zhang        2010-10-27  322  			AD7816_BOUND_VALUE_BASE);
7924425db04a61 Sonic Zhang        2010-10-27  323  	} else {
0fd736f9f4d2ae Amitoj Kaur Chawla 2016-02-16  324  		if (value < AD7816_BOUND_VALUE_BASE || value > 255)
7924425db04a61 Sonic Zhang        2010-10-27  325  			return -EINVAL;
7924425db04a61 Sonic Zhang        2010-10-27  326  
7924425db04a61 Sonic Zhang        2010-10-27  327  		data = (u8)value;
7924425db04a61 Sonic Zhang        2010-10-27  328  	}
7924425db04a61 Sonic Zhang        2010-10-27  329  
7924425db04a61 Sonic Zhang        2010-10-27  330  	ret = ad7816_spi_write(chip, data);
7924425db04a61 Sonic Zhang        2010-10-27  331  	if (ret)
7924425db04a61 Sonic Zhang        2010-10-27  332  		return -EIO;
7924425db04a61 Sonic Zhang        2010-10-27  333  
7924425db04a61 Sonic Zhang        2010-10-27  334  	chip->oti_data[chip->channel_id] = data;
7924425db04a61 Sonic Zhang        2010-10-27  335  
7924425db04a61 Sonic Zhang        2010-10-27  336  	return len;
7924425db04a61 Sonic Zhang        2010-10-27  337  }
7924425db04a61 Sonic Zhang        2010-10-27  338  
7f47d56c5b0500 Julián de Gortari  2017-01-23  339  static IIO_DEVICE_ATTR(oti, 0644,
7924425db04a61 Sonic Zhang        2010-10-27 @340  		       ad7816_show_oti, ad7816_set_oti, 0);
7924425db04a61 Sonic Zhang        2010-10-27  341  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling
  2025-04-18 20:47 [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Gabriel Shahrouzi
                   ` (4 preceding siblings ...)
  2025-04-18 20:47 ` [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info Gabriel Shahrouzi
@ 2025-04-21 11:29 ` Jonathan Cameron
  2025-04-21 13:44   ` Gabriel Shahrouzi
  5 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2025-04-21 11:29 UTC (permalink / raw)
  To: Gabriel Shahrouzi
  Cc: gregkh, lars, linux-iio, linux-kernel, linux-staging,
	Michael.Hennerich, sonic.zhang, vapier, skhan,
	linux-kernel-mentees

On Fri, 18 Apr 2025 16:47:34 -0400
Gabriel Shahrouzi <gshahrouzi@gmail.com> wrote:

> The original patch combined a functional fix (allowing channel 7) with
> several refactoring steps (introducing chip_info, renaming structs,
> improving validation). As requested, these have now been separated.
> 
> The series proceeds as follows:
> 1. Fix: Allow diagnostic channel 7 for all device variants.
> 2. Refactor: Rename the main state structure for clarity before introducing
>    the new chip_info struct.
> 3. Refactor: Introduce struct ad7816_chip_info to hold static per-variant
>    data, update ID tables to store pointers, and switch to using
>    device_get_match_data() for firmware-independent identification.
>    This removes the old enum/id mechanism.
> 4. Refactor: Add has_busy_pin to chip_info and use this flag to
>    determine BUSY pin handling, replacing pointer comparisons.
> 5. Refactor: Simplify channel validation logic using 
>    chip_info->max_channels, removing strcmp() checks.
> 
> Regarding the 'fixes' tag: I've applied it only to the first commit
> containing the core fix, primarily to make backporting easier. Is this
> the standard practice, or should the tag typically be applied to
> subsequent commits that build upon or are related to the fix as well?
> 
> Chainges in v3:
> 	- Split the patch into smaller patches. Make the fix first
> 	  followed by clean up.
> 	- Include missing channel for channel selection.
> 	- Address specific feedback regarding enums vs. chip_info data.
> 	- Use device_get_match_data() for device identification.
> 	- Move BUSY pin capability check into chip_info data.
> 	- Simplify channel validation using chip_info data.
> Changes in v2:
>         - Refactor by adding chip_info struct which simplifies
>           conditional logic.

Hi Gabriel,

Whilst I appreciate the enthusiasm. Generally slow down a little!
If there is a fundamental issue like an accidental sending of the wrong
version then please reply to the thread cover letter to say that.
Otherwise, even in the presence of build bot reports, it is good
for any non trivial series to wait a little for reviews to come in.
Ideally a week, but a few days can be fine if you already have a lot
of feedback from reviewers.

IIO is moderately high traffic and whilst we are good at hitting
the button to mark a thread read, it still takes some time!

Jonathan

> 
> Gabriel Shahrouzi (5):
>   staging: iio: adc: ad7816: Allow channel 7 for all devices
>   staging: iio: adc: ad7816: Rename state structure
>   staging: iio: adc: ad7816: Introduce chip_info and use pointer
>     matching
>   staging: iio: adc: ad7816: Use chip_info for device capabilities
>   staging: iio: adc: ad7816: Simplify channel validation using chip_info
> 
>  drivers/staging/iio/adc/ad7816.c | 94 ++++++++++++++++++--------------
>  1 file changed, 54 insertions(+), 40 deletions(-)
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling
  2025-04-21 11:29 ` [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Jonathan Cameron
@ 2025-04-21 13:44   ` Gabriel Shahrouzi
  0 siblings, 0 replies; 10+ messages in thread
From: Gabriel Shahrouzi @ 2025-04-21 13:44 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: gregkh, lars, linux-iio, linux-kernel, linux-staging,
	Michael.Hennerich, sonic.zhang, vapier, skhan,
	linux-kernel-mentees

On Mon, Apr 21, 2025 at 7:29 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 18 Apr 2025 16:47:34 -0400
> Gabriel Shahrouzi <gshahrouzi@gmail.com> wrote:
>
> > The original patch combined a functional fix (allowing channel 7) with
> > several refactoring steps (introducing chip_info, renaming structs,
> > improving validation). As requested, these have now been separated.
> >
> > The series proceeds as follows:
> > 1. Fix: Allow diagnostic channel 7 for all device variants.
> > 2. Refactor: Rename the main state structure for clarity before introducing
> >    the new chip_info struct.
> > 3. Refactor: Introduce struct ad7816_chip_info to hold static per-variant
> >    data, update ID tables to store pointers, and switch to using
> >    device_get_match_data() for firmware-independent identification.
> >    This removes the old enum/id mechanism.
> > 4. Refactor: Add has_busy_pin to chip_info and use this flag to
> >    determine BUSY pin handling, replacing pointer comparisons.
> > 5. Refactor: Simplify channel validation logic using
> >    chip_info->max_channels, removing strcmp() checks.
> >
> > Regarding the 'fixes' tag: I've applied it only to the first commit
> > containing the core fix, primarily to make backporting easier. Is this
> > the standard practice, or should the tag typically be applied to
> > subsequent commits that build upon or are related to the fix as well?
> >
> > Chainges in v3:
> >       - Split the patch into smaller patches. Make the fix first
> >         followed by clean up.
> >       - Include missing channel for channel selection.
> >       - Address specific feedback regarding enums vs. chip_info data.
> >       - Use device_get_match_data() for device identification.
> >       - Move BUSY pin capability check into chip_info data.
> >       - Simplify channel validation using chip_info data.
> > Changes in v2:
> >         - Refactor by adding chip_info struct which simplifies
> >           conditional logic.
>
> Hi Gabriel,
>
> Whilst I appreciate the enthusiasm. Generally slow down a little!
> If there is a fundamental issue like an accidental sending of the wrong
> version then please reply to the thread cover letter to say that.
> Otherwise, even in the presence of build bot reports, it is good
> for any non trivial series to wait a little for reviews to come in.
> Ideally a week, but a few days can be fine if you already have a lot
> of feedback from reviewers.
Got it, makes more sense to respond to a fundamental error like that
in the cover letter than send an entire new patch series out which
bloats the mailing list.
>
> IIO is moderately high traffic and whilst we are good at hitting
> the button to mark a thread read, it still takes some time!
Got it.
>
> Jonathan
>
> >
> > Gabriel Shahrouzi (5):
> >   staging: iio: adc: ad7816: Allow channel 7 for all devices
> >   staging: iio: adc: ad7816: Rename state structure
> >   staging: iio: adc: ad7816: Introduce chip_info and use pointer
> >     matching
> >   staging: iio: adc: ad7816: Use chip_info for device capabilities
> >   staging: iio: adc: ad7816: Simplify channel validation using chip_info
> >
> >  drivers/staging/iio/adc/ad7816.c | 94 ++++++++++++++++++--------------
> >  1 file changed, 54 insertions(+), 40 deletions(-)
> >
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-04-21 13:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-18 20:47 [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Gabriel Shahrouzi
2025-04-18 20:47 ` [PATCH v3 1/5] staging: iio: adc: ad7816: Allow channel 7 for all devices Gabriel Shahrouzi
2025-04-18 20:47 ` [PATCH v3 2/5] staging: iio: adc: ad7816: Rename state structure Gabriel Shahrouzi
2025-04-18 20:47 ` [PATCH v3 3/5] staging: iio: adc: ad7816: Introduce chip_info and use pointer matching Gabriel Shahrouzi
2025-04-18 20:47 ` [PATCH v3 4/5] staging: iio: adc: ad7816: Use chip_info for device capabilities Gabriel Shahrouzi
2025-04-18 20:47 ` [PATCH v3 5/5] staging: iio: adc: ad7816: Simplify channel validation using chip_info Gabriel Shahrouzi
2025-04-19 12:19   ` kernel test robot
2025-04-19 13:11   ` kernel test robot
2025-04-21 11:29 ` [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling Jonathan Cameron
2025-04-21 13:44   ` Gabriel Shahrouzi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox