Replace the int irq_line field in struct ad_sigma_delta_info with a
bool get_irq_by_name flag. (The field is reordered for better struct
packing.)

This struct is intended to be used as static const data. Currently, in
the ad7173 driver it is static, but not const and so each driver probe
will write over the struct, which is a problem if there is more than
one driver instance probing at the same time.

Instead of storing the actual IRQ number in the struct, a flag is added
to indicate that we need to get the IRQ number by name. Then the code
is modified to check this flag when setting sigma_delta->irq_line in
ad_sd_init().

fwnode_irq_get_byname() is moved to ad_sd_init() to be able to handle
this change with the bonus that it can be shared with other drivers in
the future.

static struct ad_sigma_delta_info ad7173_sigma_delta_info can't be
changed to const yet in this patch because there is still another bug
where another field is being written to in the probe function.

Fixes: 76a1e6a42802 ("iio: adc: ad7173: add AD7173 driver")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7173.c               |  7 +------
 drivers/iio/adc/ad_sigma_delta.c       | 14 +++++++++++---
 include/linux/iio/adc/ad_sigma_delta.h |  5 +++--
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 29ff9c7036c0..5215584438bf 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -758,6 +758,7 @@ static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
 	.disable_all = ad7173_disable_all,
 	.disable_one = ad7173_disable_one,
 	.set_mode = ad7173_set_mode,
+	.get_irq_by_name = true,
 	.has_registers = true,
 	.addr_shift = 0,
 	.read_mask = BIT(6),
@@ -1397,12 +1398,6 @@ static int ad7173_fw_parse_device_config(struct iio_dev *indio_dev)
 			return ret;
 	}
 
-	ret = fwnode_irq_get_byname(dev_fwnode(dev), "rdy");
-	if (ret < 0)
-		return dev_err_probe(dev, ret, "Interrupt 'rdy' is required\n");
-
-	ad7173_sigma_delta_info.irq_line = ret;
-
 	return ad7173_fw_parse_channel_config(indio_dev);
 }
 
diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 2f3b61765055..af982f21adfa 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -653,6 +653,8 @@ EXPORT_SYMBOL_NS_GPL(devm_ad_sd_setup_buffer_and_trigger, IIO_AD_SIGMA_DELTA);
 int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
 	struct spi_device *spi, const struct ad_sigma_delta_info *info)
 {
+	int ret;
+
 	sigma_delta->spi = spi;
 	sigma_delta->info = info;
 
@@ -674,10 +676,16 @@ int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
 		}
 	}
 
-	if (info->irq_line)
-		sigma_delta->irq_line = info->irq_line;
-	else
+	if (info->get_irq_by_name) {
+		ret = fwnode_irq_get_byname(dev_fwnode(&spi->dev), "rdy");
+		if (ret < 0)
+			return dev_err_probe(&spi->dev, ret,
+					     "Interrupt 'rdy' is required\n");
+
+		sigma_delta->irq_line = ret;
+	} else {
 		sigma_delta->irq_line = spi->irq;
+	}
 
 	iio_device_set_drvdata(indio_dev, sigma_delta);
 
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index f8c1d2505940..2d8bc5de8332 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -43,6 +43,8 @@ struct iio_dev;
  *		the value required for the driver to identify the channel.
  * @postprocess_sample: Is called for each sampled data word, can be used to
  *		modify or drop the sample data, it, may be NULL.
+ * @get_irq_by_name: Usually, the RDY IRQ is the first one and therefore ==
+ *		spi->irq. If not, set this to true to get the IRQ by name.
  * @has_registers: true if the device has writable and readable registers, false
  *		if there is just one read-only sample data shift register.
  * @addr_shift: Shift of the register address in the communications register.
@@ -52,7 +54,6 @@ struct iio_dev;
  *   be used.
  * @irq_flags: flags for the interrupt used by the triggered buffer
  * @num_slots: Number of sequencer slots
- * @irq_line: IRQ for reading conversions. If 0, spi->irq will be used
  */
 struct ad_sigma_delta_info {
 	int (*set_channel)(struct ad_sigma_delta *, unsigned int channel);
@@ -61,6 +62,7 @@ struct ad_sigma_delta_info {
 	int (*disable_all)(struct ad_sigma_delta *);
 	int (*disable_one)(struct ad_sigma_delta *, unsigned int chan);
 	int (*postprocess_sample)(struct ad_sigma_delta *, unsigned int raw_sample);
+	bool get_irq_by_name;
 	bool has_registers;
 	unsigned int addr_shift;
 	unsigned int read_mask;
@@ -68,7 +70,6 @@ struct ad_sigma_delta_info {
 	unsigned int data_reg;
 	unsigned long irq_flags;
 	unsigned int num_slots;
-	int irq_line;
 };
 
 /**

-- 
2.43.0

Hello,

first of all thanks for picking up my report.

$Subject ~= s/ad7313/ad7173/

I wonder if it would make sense to update the ad7173 binding to also
allow specifying the irq as the other ADCs do it and just
unconditionally fall back to rdy-interrupt (or the other way round)?
There is no good reason for ad7173 being special, is there?

Best regards
Uwe

On 11/25/24 2:59 AM, Uwe Kleine-König wrote:
> Hello,
> 
> first of all thanks for picking up my report.
> 
> $Subject ~= s/ad7313/ad7173/
> 
> I wonder if it would make sense to update the ad7173 binding to also
> allow specifying the irq as the other ADCs do it and just
> unconditionally fall back to rdy-interrupt (or the other way round)?
> There is no good reason for ad7173 being special, is there?
> 
> Best regards
> Uwe

That is a a good point. We actually don't have to change the DT
bindings, the "rdy" interrupt is already specified to be the first
interrupt, so spi->irq should already be the "rdy" interrupt
because is is always getting the interrupt at index 0. So we should
be able to just drop the special handling altogether.