Linux IIO development
 help / color / mirror / Atom feed
* [PATCH 0/2] iio: adc: ad7313: fix non-const info struct
@ 2024-11-22 17:39 David Lechner
  2024-11-22 17:39 ` [PATCH 1/2] iio: adc: ad7313: fix irq number stored in static " David Lechner
  2024-11-22 17:39 ` [PATCH 2/2] iio: adc: ad7173: make struct ad_sigma_delta_info ad7173_sigma_delta_info const David Lechner
  0 siblings, 2 replies; 6+ messages in thread
From: David Lechner @ 2024-11-22 17:39 UTC (permalink / raw)
  To: Jonathan Cameron, Dumitru Ceclan
  Cc: Michael Hennerich, Nuno Sa, Michael Walle, Andy Shevchenko,
	linux-iio, linux-kernel, Guillaume Ranquet, Uwe Kleine-König,
	David Lechner

While working ad7124, Uwe pointed out a bug in the ad7313 driver.
static struct ad_sigma_delta_info ad7173_sigma_delta_info was not const
and was being modified during driver probe, which could lead to race
conditions if two instances of the driver were probed at the same time.

I've made an attempt to fix it, but it isn't exactly trivial and I have
only compile tested it. Guillaume has access to ad4111 hardware, so it
would be good to get a Tested-by from him to make sure this doesn't
break anything.

Reported-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
David Lechner (2):
      iio: adc: ad7313: fix irq number stored in static info struct
      iio: adc: ad7173: make struct ad_sigma_delta_info ad7173_sigma_delta_info const

 drivers/iio/adc/ad7173.c               | 476 +++++++++++++++++----------------
 drivers/iio/adc/ad_sigma_delta.c       |  14 +-
 include/linux/iio/adc/ad_sigma_delta.h |   5 +-
 3 files changed, 264 insertions(+), 231 deletions(-)
---
base-commit: 9dd2270ca0b38ee16094817f4a53e7ba78e31567
change-id: 20241122-iio-adc-ad7313-fix-non-const-info-struct-92e59b91ee2e

Best regards,
-- 
David Lechner <dlechner@baylibre.com>


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

* [PATCH 1/2] iio: adc: ad7313: fix irq number stored in static info struct
  2024-11-22 17:39 [PATCH 0/2] iio: adc: ad7313: fix non-const info struct David Lechner
@ 2024-11-22 17:39 ` David Lechner
  2024-11-25  8:59   ` Uwe Kleine-König
  2024-11-22 17:39 ` [PATCH 2/2] iio: adc: ad7173: make struct ad_sigma_delta_info ad7173_sigma_delta_info const David Lechner
  1 sibling, 1 reply; 6+ messages in thread
From: David Lechner @ 2024-11-22 17:39 UTC (permalink / raw)
  To: Jonathan Cameron, Dumitru Ceclan
  Cc: Michael Hennerich, Nuno Sa, Michael Walle, Andy Shevchenko,
	linux-iio, linux-kernel, Guillaume Ranquet, Uwe Kleine-König,
	David Lechner

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


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

* [PATCH 2/2] iio: adc: ad7173: make struct ad_sigma_delta_info ad7173_sigma_delta_info const
  2024-11-22 17:39 [PATCH 0/2] iio: adc: ad7313: fix non-const info struct David Lechner
  2024-11-22 17:39 ` [PATCH 1/2] iio: adc: ad7313: fix irq number stored in static " David Lechner
@ 2024-11-22 17:39 ` David Lechner
  2024-11-24 17:48   ` Jonathan Cameron
  1 sibling, 1 reply; 6+ messages in thread
From: David Lechner @ 2024-11-22 17:39 UTC (permalink / raw)
  To: Jonathan Cameron, Dumitru Ceclan
  Cc: Michael Hennerich, Nuno Sa, Michael Walle, Andy Shevchenko,
	linux-iio, linux-kernel, Guillaume Ranquet, Uwe Kleine-König,
	David Lechner

Make struct ad_sigma_delta_info ad7173_sigma_delta_info static const.
This structure is shared by all instances of the driver, so it can't be
safely modified by one instance without affecting all other instances.

The num_slots field was being modified, so we need to make two copies of
the structure, one for each possible value of num_slots. Then we add a
field to the chip-specific info struct to point to the correct copy of
the struct ad_sigma_delta_info depending on the chip's capabilities.

In order to do this, all of the chip-specific info structs have to be
moved after the struct ad_sigma_delta_info definitions.

Fixes: 76a1e6a42802 ("iio: adc: ad7173: add AD7173 driver")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7173.c | 469 +++++++++++++++++++++++++----------------------
 1 file changed, 249 insertions(+), 220 deletions(-)

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 5215584438bf..ab2a7a16c477 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -166,6 +166,7 @@ struct ad7173_device_info {
 	unsigned int clock;
 	unsigned int id;
 	char *name;
+	const struct ad_sigma_delta_info *sd_info;
 	bool has_current_inputs;
 	bool has_vincom_input;
 	bool has_temp;
@@ -257,223 +258,6 @@ static unsigned int ad4111_current_channel_config[] = {
 	0x18B, /* 12:IIN3+   11:IIN3− */
 };
 
-static const struct ad7173_device_info ad4111_device_info = {
-	.name = "ad4111",
-	.id = AD4111_ID,
-	.num_voltage_in_div = 8,
-	.num_channels = 16,
-	.num_configs = 8,
-	.num_voltage_in = 8,
-	.num_gpios = 2,
-	.higher_gpio_bits = true,
-	.has_temp = true,
-	.has_vincom_input = true,
-	.has_input_buf = true,
-	.has_current_inputs = true,
-	.has_int_ref = true,
-	.clock = 2 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7173_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad4112_device_info = {
-	.name = "ad4112",
-	.id = AD4112_ID,
-	.num_voltage_in_div = 8,
-	.num_channels = 16,
-	.num_configs = 8,
-	.num_voltage_in = 8,
-	.num_gpios = 2,
-	.higher_gpio_bits = true,
-	.has_vincom_input = true,
-	.has_temp = true,
-	.has_input_buf = true,
-	.has_current_inputs = true,
-	.has_int_ref = true,
-	.clock = 2 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7173_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad4113_device_info = {
-	.name = "ad4113",
-	.id = AD4113_ID,
-	.num_voltage_in_div = 8,
-	.num_channels = 16,
-	.num_configs = 8,
-	.num_voltage_in = 8,
-	.num_gpios = 2,
-	.data_reg_only_16bit = true,
-	.higher_gpio_bits = true,
-	.has_vincom_input = true,
-	.has_input_buf = true,
-	.has_int_ref = true,
-	.clock = 2 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7173_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad4114_device_info = {
-	.name = "ad4114",
-	.id = AD4114_ID,
-	.num_voltage_in_div = 16,
-	.num_channels = 16,
-	.num_configs = 8,
-	.num_voltage_in = 16,
-	.num_gpios = 4,
-	.has_vincom_input = true,
-	.has_temp = true,
-	.has_input_buf = true,
-	.has_int_ref = true,
-	.clock = 2 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7173_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad4115_device_info = {
-	.name = "ad4115",
-	.id = AD4115_ID,
-	.num_voltage_in_div = 16,
-	.num_channels = 16,
-	.num_configs = 8,
-	.num_voltage_in = 16,
-	.num_gpios = 4,
-	.has_vincom_input = true,
-	.has_temp = true,
-	.has_input_buf = true,
-	.has_int_ref = true,
-	.clock = 8 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad4115_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad4115_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad4116_device_info = {
-	.name = "ad4116",
-	.id = AD4116_ID,
-	.num_voltage_in_div = 11,
-	.num_channels = 16,
-	.num_configs = 8,
-	.num_voltage_in = 16,
-	.num_gpios = 4,
-	.has_vincom_input = true,
-	.has_temp = true,
-	.has_input_buf = true,
-	.has_int_ref = true,
-	.clock = 4 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad4116_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad4116_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad7172_2_device_info = {
-	.name = "ad7172-2",
-	.id = AD7172_2_ID,
-	.num_voltage_in = 5,
-	.num_channels = 4,
-	.num_configs = 4,
-	.num_gpios = 2,
-	.has_temp = true,
-	.has_input_buf = true,
-	.has_int_ref = true,
-	.has_pow_supply_monitoring = true,
-	.clock = 2 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7173_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad7172_4_device_info = {
-	.name = "ad7172-4",
-	.id = AD7172_4_ID,
-	.num_voltage_in = 9,
-	.num_channels = 8,
-	.num_configs = 8,
-	.num_gpios = 4,
-	.has_input_buf = true,
-	.has_ref2 = true,
-	.has_pow_supply_monitoring = true,
-	.clock = 2 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7173_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad7173_8_device_info = {
-	.name = "ad7173-8",
-	.id = AD7173_ID,
-	.num_voltage_in = 17,
-	.num_channels = 16,
-	.num_configs = 8,
-	.num_gpios = 4,
-	.has_temp = true,
-	.has_input_buf = true,
-	.has_int_ref = true,
-	.has_ref2 = true,
-	.clock = 2 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7173_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad7175_2_device_info = {
-	.name = "ad7175-2",
-	.id = AD7175_2_ID,
-	.num_voltage_in = 5,
-	.num_channels = 4,
-	.num_configs = 4,
-	.num_gpios = 2,
-	.has_temp = true,
-	.has_input_buf = true,
-	.has_int_ref = true,
-	.has_pow_supply_monitoring = true,
-	.clock = 16 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7175_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad7175_8_device_info = {
-	.name = "ad7175-8",
-	.id = AD7175_8_ID,
-	.num_voltage_in = 17,
-	.num_channels = 16,
-	.num_configs = 8,
-	.num_gpios = 4,
-	.has_temp = true,
-	.has_input_buf = true,
-	.has_int_ref = true,
-	.has_ref2 = true,
-	.has_pow_supply_monitoring = true,
-	.clock = 16 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7175_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad7176_2_device_info = {
-	.name = "ad7176-2",
-	.id = AD7176_ID,
-	.num_voltage_in = 5,
-	.num_channels = 4,
-	.num_configs = 4,
-	.num_gpios = 2,
-	.has_int_ref = true,
-	.clock = 16 * HZ_PER_MHZ,
-	.sinc5_data_rates = ad7175_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
-};
-
-static const struct ad7173_device_info ad7177_2_device_info = {
-	.name = "ad7177-2",
-	.id = AD7177_ID,
-	.num_voltage_in = 5,
-	.num_channels = 4,
-	.num_configs = 4,
-	.num_gpios = 2,
-	.has_temp = true,
-	.has_input_buf = true,
-	.has_int_ref = true,
-	.has_pow_supply_monitoring = true,
-	.clock = 16 * HZ_PER_MHZ,
-	.odr_start_value = AD7177_ODR_START_VALUE,
-	.sinc5_data_rates = ad7175_sinc5_data_rates,
-	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
-};
-
 static const char *const ad7173_ref_sel_str[] = {
 	[AD7173_SETUP_REF_SEL_EXT_REF]    = "vref",
 	[AD7173_SETUP_REF_SEL_EXT_REF2]   = "vref2",
@@ -752,7 +536,7 @@ static int ad7173_disable_one(struct ad_sigma_delta *sd, unsigned int chan)
 	return ad_sd_write_reg(sd, AD7173_REG_CH(chan), 2, 0);
 }
 
-static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
+static const struct ad_sigma_delta_info ad7173_sigma_delta_info_4_slots = {
 	.set_channel = ad7173_set_channel,
 	.append_status = ad7173_append_status,
 	.disable_all = ad7173_disable_all,
@@ -764,6 +548,252 @@ static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
 	.read_mask = BIT(6),
 	.status_ch_mask = GENMASK(3, 0),
 	.data_reg = AD7173_REG_DATA,
+	.num_slots = 4,
+};
+
+static const struct ad_sigma_delta_info ad7173_sigma_delta_info_8_slots = {
+	.set_channel = ad7173_set_channel,
+	.append_status = ad7173_append_status,
+	.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),
+	.status_ch_mask = GENMASK(3, 0),
+	.data_reg = AD7173_REG_DATA,
+	.num_slots = 8,
+};
+
+static const struct ad7173_device_info ad4111_device_info = {
+	.name = "ad4111",
+	.id = AD4111_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in_div = 8,
+	.num_channels = 16,
+	.num_configs = 8,
+	.num_voltage_in = 8,
+	.num_gpios = 2,
+	.higher_gpio_bits = true,
+	.has_temp = true,
+	.has_vincom_input = true,
+	.has_input_buf = true,
+	.has_current_inputs = true,
+	.has_int_ref = true,
+	.clock = 2 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7173_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad4112_device_info = {
+	.name = "ad4112",
+	.id = AD4112_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in_div = 8,
+	.num_channels = 16,
+	.num_configs = 8,
+	.num_voltage_in = 8,
+	.num_gpios = 2,
+	.higher_gpio_bits = true,
+	.has_vincom_input = true,
+	.has_temp = true,
+	.has_input_buf = true,
+	.has_current_inputs = true,
+	.has_int_ref = true,
+	.clock = 2 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7173_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad4113_device_info = {
+	.name = "ad4113",
+	.id = AD4113_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in_div = 8,
+	.num_channels = 16,
+	.num_configs = 8,
+	.num_voltage_in = 8,
+	.num_gpios = 2,
+	.data_reg_only_16bit = true,
+	.higher_gpio_bits = true,
+	.has_vincom_input = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.clock = 2 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7173_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad4114_device_info = {
+	.name = "ad4114",
+	.id = AD4114_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in_div = 16,
+	.num_channels = 16,
+	.num_configs = 8,
+	.num_voltage_in = 16,
+	.num_gpios = 4,
+	.has_vincom_input = true,
+	.has_temp = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.clock = 2 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7173_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad4115_device_info = {
+	.name = "ad4115",
+	.id = AD4115_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in_div = 16,
+	.num_channels = 16,
+	.num_configs = 8,
+	.num_voltage_in = 16,
+	.num_gpios = 4,
+	.has_vincom_input = true,
+	.has_temp = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.clock = 8 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad4115_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad4115_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad4116_device_info = {
+	.name = "ad4116",
+	.id = AD4116_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in_div = 11,
+	.num_channels = 16,
+	.num_configs = 8,
+	.num_voltage_in = 16,
+	.num_gpios = 4,
+	.has_vincom_input = true,
+	.has_temp = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.clock = 4 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad4116_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad4116_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad7172_2_device_info = {
+	.name = "ad7172-2",
+	.id = AD7172_2_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in = 5,
+	.num_channels = 4,
+	.num_configs = 4,
+	.num_gpios = 2,
+	.has_temp = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.has_pow_supply_monitoring = true,
+	.clock = 2 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7173_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad7172_4_device_info = {
+	.name = "ad7172-4",
+	.id = AD7172_4_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in = 9,
+	.num_channels = 8,
+	.num_configs = 8,
+	.num_gpios = 4,
+	.has_input_buf = true,
+	.has_ref2 = true,
+	.has_pow_supply_monitoring = true,
+	.clock = 2 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7173_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad7173_8_device_info = {
+	.name = "ad7173-8",
+	.id = AD7173_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in = 17,
+	.num_channels = 16,
+	.num_configs = 8,
+	.num_gpios = 4,
+	.has_temp = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.has_ref2 = true,
+	.clock = 2 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7173_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad7175_2_device_info = {
+	.name = "ad7175-2",
+	.id = AD7175_2_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in = 5,
+	.num_channels = 4,
+	.num_configs = 4,
+	.num_gpios = 2,
+	.has_temp = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.has_pow_supply_monitoring = true,
+	.clock = 16 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7175_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad7175_8_device_info = {
+	.name = "ad7175-8",
+	.id = AD7175_8_ID,
+	.sd_info = &ad7173_sigma_delta_info_8_slots,
+	.num_voltage_in = 17,
+	.num_channels = 16,
+	.num_configs = 8,
+	.num_gpios = 4,
+	.has_temp = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.has_ref2 = true,
+	.has_pow_supply_monitoring = true,
+	.clock = 16 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7175_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad7176_2_device_info = {
+	.name = "ad7176-2",
+	.id = AD7176_ID,
+	.sd_info = &ad7173_sigma_delta_info_4_slots,
+	.num_voltage_in = 5,
+	.num_channels = 4,
+	.num_configs = 4,
+	.num_gpios = 2,
+	.has_int_ref = true,
+	.clock = 16 * HZ_PER_MHZ,
+	.sinc5_data_rates = ad7175_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad7177_2_device_info = {
+	.name = "ad7177-2",
+	.id = AD7177_ID,
+	.sd_info = &ad7173_sigma_delta_info_4_slots,
+	.num_voltage_in = 5,
+	.num_channels = 4,
+	.num_configs = 4,
+	.num_gpios = 2,
+	.has_temp = true,
+	.has_input_buf = true,
+	.has_int_ref = true,
+	.has_pow_supply_monitoring = true,
+	.clock = 16 * HZ_PER_MHZ,
+	.odr_start_value = AD7177_ODR_START_VALUE,
+	.sinc5_data_rates = ad7175_sinc5_data_rates,
+	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
 };
 
 static int ad7173_setup(struct iio_dev *indio_dev)
@@ -1429,8 +1459,7 @@ static int ad7173_probe(struct spi_device *spi)
 	spi->mode = SPI_MODE_3;
 	spi_setup(spi);
 
-	ad7173_sigma_delta_info.num_slots = st->info->num_configs;
-	ret = ad_sd_init(&st->sd, indio_dev, spi, &ad7173_sigma_delta_info);
+	ret = ad_sd_init(&st->sd, indio_dev, spi, st->info->sd_info);
 	if (ret)
 		return ret;
 

-- 
2.43.0


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

* Re: [PATCH 2/2] iio: adc: ad7173: make struct ad_sigma_delta_info ad7173_sigma_delta_info const
  2024-11-22 17:39 ` [PATCH 2/2] iio: adc: ad7173: make struct ad_sigma_delta_info ad7173_sigma_delta_info const David Lechner
@ 2024-11-24 17:48   ` Jonathan Cameron
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2024-11-24 17:48 UTC (permalink / raw)
  To: David Lechner
  Cc: Dumitru Ceclan, Michael Hennerich, Nuno Sa, Michael Walle,
	Andy Shevchenko, linux-iio, linux-kernel, Guillaume Ranquet,
	Uwe Kleine-König

On Fri, 22 Nov 2024 11:39:53 -0600
David Lechner <dlechner@baylibre.com> wrote:

> Make struct ad_sigma_delta_info ad7173_sigma_delta_info static const.
> This structure is shared by all instances of the driver, so it can't be
> safely modified by one instance without affecting all other instances.
> 
> The num_slots field was being modified, so we need to make two copies of
> the structure, one for each possible value of num_slots. Then we add a
> field to the chip-specific info struct to point to the correct copy of
> the struct ad_sigma_delta_info depending on the chip's capabilities.
> 
> In order to do this, all of the chip-specific info structs have to be
> moved after the struct ad_sigma_delta_info definitions.
> 
> Fixes: 76a1e6a42802 ("iio: adc: ad7173: add AD7173 driver")
> Signed-off-by: David Lechner <dlechner@baylibre.com>
Too big to be suitable for backporting.

How about just duplicating the structure before modifying?
	
Then continue as before though it will need to passed into a few places
where the global variable is used directly.

Might be sensible to first do a  minimal fix with duplication then
follow up with a refactor so it is picking between static const
structures as here.


Jonathan

> ---
>  drivers/iio/adc/ad7173.c | 469 +++++++++++++++++++++++++----------------------
>  1 file changed, 249 insertions(+), 220 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index 5215584438bf..ab2a7a16c477 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -166,6 +166,7 @@ struct ad7173_device_info {
>  	unsigned int clock;
>  	unsigned int id;
>  	char *name;
> +	const struct ad_sigma_delta_info *sd_info;
>  	bool has_current_inputs;
>  	bool has_vincom_input;
>  	bool has_temp;
> @@ -257,223 +258,6 @@ static unsigned int ad4111_current_channel_config[] = {
>  	0x18B, /* 12:IIN3+   11:IIN3− */
>  };
>  
> -static const struct ad7173_device_info ad4111_device_info = {
> -	.name = "ad4111",
> -	.id = AD4111_ID,
> -	.num_voltage_in_div = 8,
> -	.num_channels = 16,
> -	.num_configs = 8,
> -	.num_voltage_in = 8,
> -	.num_gpios = 2,
> -	.higher_gpio_bits = true,
> -	.has_temp = true,
> -	.has_vincom_input = true,
> -	.has_input_buf = true,
> -	.has_current_inputs = true,
> -	.has_int_ref = true,
> -	.clock = 2 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7173_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4112_device_info = {
> -	.name = "ad4112",
> -	.id = AD4112_ID,
> -	.num_voltage_in_div = 8,
> -	.num_channels = 16,
> -	.num_configs = 8,
> -	.num_voltage_in = 8,
> -	.num_gpios = 2,
> -	.higher_gpio_bits = true,
> -	.has_vincom_input = true,
> -	.has_temp = true,
> -	.has_input_buf = true,
> -	.has_current_inputs = true,
> -	.has_int_ref = true,
> -	.clock = 2 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7173_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4113_device_info = {
> -	.name = "ad4113",
> -	.id = AD4113_ID,
> -	.num_voltage_in_div = 8,
> -	.num_channels = 16,
> -	.num_configs = 8,
> -	.num_voltage_in = 8,
> -	.num_gpios = 2,
> -	.data_reg_only_16bit = true,
> -	.higher_gpio_bits = true,
> -	.has_vincom_input = true,
> -	.has_input_buf = true,
> -	.has_int_ref = true,
> -	.clock = 2 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7173_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4114_device_info = {
> -	.name = "ad4114",
> -	.id = AD4114_ID,
> -	.num_voltage_in_div = 16,
> -	.num_channels = 16,
> -	.num_configs = 8,
> -	.num_voltage_in = 16,
> -	.num_gpios = 4,
> -	.has_vincom_input = true,
> -	.has_temp = true,
> -	.has_input_buf = true,
> -	.has_int_ref = true,
> -	.clock = 2 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7173_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4115_device_info = {
> -	.name = "ad4115",
> -	.id = AD4115_ID,
> -	.num_voltage_in_div = 16,
> -	.num_channels = 16,
> -	.num_configs = 8,
> -	.num_voltage_in = 16,
> -	.num_gpios = 4,
> -	.has_vincom_input = true,
> -	.has_temp = true,
> -	.has_input_buf = true,
> -	.has_int_ref = true,
> -	.clock = 8 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad4115_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad4115_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4116_device_info = {
> -	.name = "ad4116",
> -	.id = AD4116_ID,
> -	.num_voltage_in_div = 11,
> -	.num_channels = 16,
> -	.num_configs = 8,
> -	.num_voltage_in = 16,
> -	.num_gpios = 4,
> -	.has_vincom_input = true,
> -	.has_temp = true,
> -	.has_input_buf = true,
> -	.has_int_ref = true,
> -	.clock = 4 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad4116_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad4116_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7172_2_device_info = {
> -	.name = "ad7172-2",
> -	.id = AD7172_2_ID,
> -	.num_voltage_in = 5,
> -	.num_channels = 4,
> -	.num_configs = 4,
> -	.num_gpios = 2,
> -	.has_temp = true,
> -	.has_input_buf = true,
> -	.has_int_ref = true,
> -	.has_pow_supply_monitoring = true,
> -	.clock = 2 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7173_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7172_4_device_info = {
> -	.name = "ad7172-4",
> -	.id = AD7172_4_ID,
> -	.num_voltage_in = 9,
> -	.num_channels = 8,
> -	.num_configs = 8,
> -	.num_gpios = 4,
> -	.has_input_buf = true,
> -	.has_ref2 = true,
> -	.has_pow_supply_monitoring = true,
> -	.clock = 2 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7173_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7173_8_device_info = {
> -	.name = "ad7173-8",
> -	.id = AD7173_ID,
> -	.num_voltage_in = 17,
> -	.num_channels = 16,
> -	.num_configs = 8,
> -	.num_gpios = 4,
> -	.has_temp = true,
> -	.has_input_buf = true,
> -	.has_int_ref = true,
> -	.has_ref2 = true,
> -	.clock = 2 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7173_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7175_2_device_info = {
> -	.name = "ad7175-2",
> -	.id = AD7175_2_ID,
> -	.num_voltage_in = 5,
> -	.num_channels = 4,
> -	.num_configs = 4,
> -	.num_gpios = 2,
> -	.has_temp = true,
> -	.has_input_buf = true,
> -	.has_int_ref = true,
> -	.has_pow_supply_monitoring = true,
> -	.clock = 16 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7175_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7175_8_device_info = {
> -	.name = "ad7175-8",
> -	.id = AD7175_8_ID,
> -	.num_voltage_in = 17,
> -	.num_channels = 16,
> -	.num_configs = 8,
> -	.num_gpios = 4,
> -	.has_temp = true,
> -	.has_input_buf = true,
> -	.has_int_ref = true,
> -	.has_ref2 = true,
> -	.has_pow_supply_monitoring = true,
> -	.clock = 16 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7175_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7176_2_device_info = {
> -	.name = "ad7176-2",
> -	.id = AD7176_ID,
> -	.num_voltage_in = 5,
> -	.num_channels = 4,
> -	.num_configs = 4,
> -	.num_gpios = 2,
> -	.has_int_ref = true,
> -	.clock = 16 * HZ_PER_MHZ,
> -	.sinc5_data_rates = ad7175_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7177_2_device_info = {
> -	.name = "ad7177-2",
> -	.id = AD7177_ID,
> -	.num_voltage_in = 5,
> -	.num_channels = 4,
> -	.num_configs = 4,
> -	.num_gpios = 2,
> -	.has_temp = true,
> -	.has_input_buf = true,
> -	.has_int_ref = true,
> -	.has_pow_supply_monitoring = true,
> -	.clock = 16 * HZ_PER_MHZ,
> -	.odr_start_value = AD7177_ODR_START_VALUE,
> -	.sinc5_data_rates = ad7175_sinc5_data_rates,
> -	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> -};
> -
>  static const char *const ad7173_ref_sel_str[] = {
>  	[AD7173_SETUP_REF_SEL_EXT_REF]    = "vref",
>  	[AD7173_SETUP_REF_SEL_EXT_REF2]   = "vref2",
> @@ -752,7 +536,7 @@ static int ad7173_disable_one(struct ad_sigma_delta *sd, unsigned int chan)
>  	return ad_sd_write_reg(sd, AD7173_REG_CH(chan), 2, 0);
>  }
>  
> -static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
> +static const struct ad_sigma_delta_info ad7173_sigma_delta_info_4_slots = {
>  	.set_channel = ad7173_set_channel,
>  	.append_status = ad7173_append_status,
>  	.disable_all = ad7173_disable_all,
> @@ -764,6 +548,252 @@ static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
>  	.read_mask = BIT(6),
>  	.status_ch_mask = GENMASK(3, 0),
>  	.data_reg = AD7173_REG_DATA,
> +	.num_slots = 4,
> +};
> +
> +static const struct ad_sigma_delta_info ad7173_sigma_delta_info_8_slots = {
> +	.set_channel = ad7173_set_channel,
> +	.append_status = ad7173_append_status,
> +	.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),
> +	.status_ch_mask = GENMASK(3, 0),
> +	.data_reg = AD7173_REG_DATA,
> +	.num_slots = 8,
> +};
> +
> +static const struct ad7173_device_info ad4111_device_info = {
> +	.name = "ad4111",
> +	.id = AD4111_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in_div = 8,
> +	.num_channels = 16,
> +	.num_configs = 8,
> +	.num_voltage_in = 8,
> +	.num_gpios = 2,
> +	.higher_gpio_bits = true,
> +	.has_temp = true,
> +	.has_vincom_input = true,
> +	.has_input_buf = true,
> +	.has_current_inputs = true,
> +	.has_int_ref = true,
> +	.clock = 2 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7173_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4112_device_info = {
> +	.name = "ad4112",
> +	.id = AD4112_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in_div = 8,
> +	.num_channels = 16,
> +	.num_configs = 8,
> +	.num_voltage_in = 8,
> +	.num_gpios = 2,
> +	.higher_gpio_bits = true,
> +	.has_vincom_input = true,
> +	.has_temp = true,
> +	.has_input_buf = true,
> +	.has_current_inputs = true,
> +	.has_int_ref = true,
> +	.clock = 2 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7173_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4113_device_info = {
> +	.name = "ad4113",
> +	.id = AD4113_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in_div = 8,
> +	.num_channels = 16,
> +	.num_configs = 8,
> +	.num_voltage_in = 8,
> +	.num_gpios = 2,
> +	.data_reg_only_16bit = true,
> +	.higher_gpio_bits = true,
> +	.has_vincom_input = true,
> +	.has_input_buf = true,
> +	.has_int_ref = true,
> +	.clock = 2 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7173_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4114_device_info = {
> +	.name = "ad4114",
> +	.id = AD4114_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in_div = 16,
> +	.num_channels = 16,
> +	.num_configs = 8,
> +	.num_voltage_in = 16,
> +	.num_gpios = 4,
> +	.has_vincom_input = true,
> +	.has_temp = true,
> +	.has_input_buf = true,
> +	.has_int_ref = true,
> +	.clock = 2 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7173_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4115_device_info = {
> +	.name = "ad4115",
> +	.id = AD4115_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in_div = 16,
> +	.num_channels = 16,
> +	.num_configs = 8,
> +	.num_voltage_in = 16,
> +	.num_gpios = 4,
> +	.has_vincom_input = true,
> +	.has_temp = true,
> +	.has_input_buf = true,
> +	.has_int_ref = true,
> +	.clock = 8 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad4115_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad4115_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4116_device_info = {
> +	.name = "ad4116",
> +	.id = AD4116_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in_div = 11,
> +	.num_channels = 16,
> +	.num_configs = 8,
> +	.num_voltage_in = 16,
> +	.num_gpios = 4,
> +	.has_vincom_input = true,
> +	.has_temp = true,
> +	.has_input_buf = true,
> +	.has_int_ref = true,
> +	.clock = 4 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad4116_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad4116_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7172_2_device_info = {
> +	.name = "ad7172-2",
> +	.id = AD7172_2_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in = 5,
> +	.num_channels = 4,
> +	.num_configs = 4,
> +	.num_gpios = 2,
> +	.has_temp = true,
> +	.has_input_buf = true,
> +	.has_int_ref = true,
> +	.has_pow_supply_monitoring = true,
> +	.clock = 2 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7173_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7172_4_device_info = {
> +	.name = "ad7172-4",
> +	.id = AD7172_4_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in = 9,
> +	.num_channels = 8,
> +	.num_configs = 8,
> +	.num_gpios = 4,
> +	.has_input_buf = true,
> +	.has_ref2 = true,
> +	.has_pow_supply_monitoring = true,
> +	.clock = 2 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7173_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7173_8_device_info = {
> +	.name = "ad7173-8",
> +	.id = AD7173_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in = 17,
> +	.num_channels = 16,
> +	.num_configs = 8,
> +	.num_gpios = 4,
> +	.has_temp = true,
> +	.has_input_buf = true,
> +	.has_int_ref = true,
> +	.has_ref2 = true,
> +	.clock = 2 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7173_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7175_2_device_info = {
> +	.name = "ad7175-2",
> +	.id = AD7175_2_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in = 5,
> +	.num_channels = 4,
> +	.num_configs = 4,
> +	.num_gpios = 2,
> +	.has_temp = true,
> +	.has_input_buf = true,
> +	.has_int_ref = true,
> +	.has_pow_supply_monitoring = true,
> +	.clock = 16 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7175_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7175_8_device_info = {
> +	.name = "ad7175-8",
> +	.id = AD7175_8_ID,
> +	.sd_info = &ad7173_sigma_delta_info_8_slots,
> +	.num_voltage_in = 17,
> +	.num_channels = 16,
> +	.num_configs = 8,
> +	.num_gpios = 4,
> +	.has_temp = true,
> +	.has_input_buf = true,
> +	.has_int_ref = true,
> +	.has_ref2 = true,
> +	.has_pow_supply_monitoring = true,
> +	.clock = 16 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7175_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7176_2_device_info = {
> +	.name = "ad7176-2",
> +	.id = AD7176_ID,
> +	.sd_info = &ad7173_sigma_delta_info_4_slots,
> +	.num_voltage_in = 5,
> +	.num_channels = 4,
> +	.num_configs = 4,
> +	.num_gpios = 2,
> +	.has_int_ref = true,
> +	.clock = 16 * HZ_PER_MHZ,
> +	.sinc5_data_rates = ad7175_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7177_2_device_info = {
> +	.name = "ad7177-2",
> +	.id = AD7177_ID,
> +	.sd_info = &ad7173_sigma_delta_info_4_slots,
> +	.num_voltage_in = 5,
> +	.num_channels = 4,
> +	.num_configs = 4,
> +	.num_gpios = 2,
> +	.has_temp = true,
> +	.has_input_buf = true,
> +	.has_int_ref = true,
> +	.has_pow_supply_monitoring = true,
> +	.clock = 16 * HZ_PER_MHZ,
> +	.odr_start_value = AD7177_ODR_START_VALUE,
> +	.sinc5_data_rates = ad7175_sinc5_data_rates,
> +	.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
>  };
>  
>  static int ad7173_setup(struct iio_dev *indio_dev)
> @@ -1429,8 +1459,7 @@ static int ad7173_probe(struct spi_device *spi)
>  	spi->mode = SPI_MODE_3;
>  	spi_setup(spi);
>  
> -	ad7173_sigma_delta_info.num_slots = st->info->num_configs;
> -	ret = ad_sd_init(&st->sd, indio_dev, spi, &ad7173_sigma_delta_info);
> +	ret = ad_sd_init(&st->sd, indio_dev, spi, st->info->sd_info);
>  	if (ret)
>  		return ret;
>  
> 


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

* Re: [PATCH 1/2] iio: adc: ad7313: fix irq number stored in static info struct
  2024-11-22 17:39 ` [PATCH 1/2] iio: adc: ad7313: fix irq number stored in static " David Lechner
@ 2024-11-25  8:59   ` Uwe Kleine-König
  2024-11-25 14:57     ` David Lechner
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2024-11-25  8:59 UTC (permalink / raw)
  To: David Lechner
  Cc: Jonathan Cameron, Dumitru Ceclan, Michael Hennerich, Nuno Sa,
	Michael Walle, Andy Shevchenko, linux-iio, linux-kernel,
	Guillaume Ranquet

[-- Attachment #1: Type: text/plain, Size: 359 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 1/2] iio: adc: ad7313: fix irq number stored in static info struct
  2024-11-25  8:59   ` Uwe Kleine-König
@ 2024-11-25 14:57     ` David Lechner
  0 siblings, 0 replies; 6+ messages in thread
From: David Lechner @ 2024-11-25 14:57 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jonathan Cameron, Dumitru Ceclan, Michael Hennerich, Nuno Sa,
	Michael Walle, Andy Shevchenko, linux-iio, linux-kernel,
	Guillaume Ranquet

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.



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

end of thread, other threads:[~2024-11-25 14:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-22 17:39 [PATCH 0/2] iio: adc: ad7313: fix non-const info struct David Lechner
2024-11-22 17:39 ` [PATCH 1/2] iio: adc: ad7313: fix irq number stored in static " David Lechner
2024-11-25  8:59   ` Uwe Kleine-König
2024-11-25 14:57     ` David Lechner
2024-11-22 17:39 ` [PATCH 2/2] iio: adc: ad7173: make struct ad_sigma_delta_info ad7173_sigma_delta_info const David Lechner
2024-11-24 17:48   ` Jonathan Cameron

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