[PATCH v7 1/2] dt-bindings: adc: add AD7173

[PATCH v7 1/2] dt-bindings: adc: add AD7173

[Dumitru Ceclan]

The AD7173 family offer a complete integrated Sigma-Delta ADC solution
which can be used in high precision, low noise single channel applications
or higher speed multiplexed applications. The Sigma-Delta ADC is intended
primarily for measurement of signals close to DC but also delivers
outstanding performance with input bandwidths out to ~10kHz.

Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com>
---
 .../bindings/iio/adc/adi,ad7173.yaml          | 170 ++++++++++++++++++
 1 file changed, 170 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
new file mode 100644
index 000000000000..087820a0cf48
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
@@ -0,0 +1,170 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7173 ADC
+
+maintainers:
+  - Ceclan Dumitru <dumitru.ceclan@analog.com>
+
+description: |
+  Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7172-2
+      - adi,ad7173-8
+      - adi,ad7175-2
+      - adi,ad7176-2
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  spi-max-frequency:
+    maximum: 20000000
+
+  refin-supply:
+    description: external reference supply, can be used as reference for conversion.
+
+  refin2-supply:
+    description: external reference supply, can be used as reference for conversion.
+
+  avdd-supply:
+    description: avdd supply, can be used as reference for conversion.
+
+  required:
+    - compatible
+    - reg
+    - interrupts
+
+patternProperties:
+  "^channel@[0-9a-f]$":
+    type: object
+    $ref: adc.yaml
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        minimum: 0
+        maximum: 15
+
+      diff-channels:
+        items:
+          minimum: 0
+          maximum: 31
+
+      adi,reference-select:
+        description: |
+          Select the reference source to use when converting on
+          the specific channel. Valid values are:
+          refin      : REFIN(+)/REFIN(−).
+          refin2     : REFIN2(+)/REFIN2(−)
+          refout-avss: REFOUT/AVSS (Internal reference)
+          avdd       : AVDD
+
+          External reference refin2 only available on ad7173-8.
+          If not specified, internal reference used.
+        enum:
+          - refin
+          - refin2
+          - refout-avss
+          - avdd
+        default: refout-avss
+
+    required:
+      - reg
+      - diff-channels
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+  - if:
+      properties:
+        compatible:
+          not:
+            contains:
+              const: adi,ad7173-8
+    then:
+      properties:
+        refin2-supply: false
+      patternProperties:
+        "^channel@[0-9a-f]$":
+          properties:
+            adi,reference-select:
+              enum:
+                - refin
+                - refout-avss
+                - avdd
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      adc@0 {
+        compatible = "adi,ad7173-8";
+        reg = <0>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+        interrupt-parent = <&gpio>;
+        spi-max-frequency = <5000000>;
+
+        refin-supply = <&dummy_regulator>;
+
+        channel@0 {
+          reg = <0>;
+          bipolar;
+          diff-channels = <0 1>;
+          adi,reference-select = "refin";
+        };
+
+        channel@1 {
+          reg = <1>;
+          diff-channels = <2 3>;
+        };
+
+        channel@2 {
+          reg = <2>;
+          bipolar;
+          diff-channels = <4 5>;
+        };
+
+        channel@3 {
+          reg = <3>;
+          bipolar;
+          diff-channels = <6 7>;
+        };
+
+        channel@4 {
+          reg = <4>;
+          diff-channels = <8 9>;
+          adi,reference-select = "avdd";
+        };
+      };
+    };
-- 
2.42.0

[PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver

[Dumitru Ceclan]

The AD7173 family offer a complete integrated Sigma-Delta ADC solution
which can be used in high precision, low noise single channel
applications or higher speed multiplexed applications. The Sigma-Delta
ADC is intended primarily for measurement of signals close to DC but also
delivers outstanding performance with input bandwidths out to ~10kHz.

Reviewed-by: Michael Walle <michael@walle.cc> # for gpio-regmap
Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com>
---
 drivers/iio/adc/Kconfig  |  17 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad7173.c | 967 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 985 insertions(+)
 create mode 100644 drivers/iio/adc/ad7173.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 35f9867da12c..ef5d5a825b5a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -54,6 +54,23 @@ config AD7124
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad7124.
 
+config AD7173
+	tristate "Analog Devices AD7173 driver"
+	depends on SPI_MASTER
+	select AD_SIGMA_DELTA
+	select GPIO_REGMAP if GPIOLIB
+	select REGMAP_SPI if GPIOLIB
+	help
+	  Say yes here to build support for Analog Devices AD7173 and similar ADC
+	  Currently supported models:
+	   - AD7172-2,
+	   - AD7173-8,
+	   - AD7175-2,
+	   - AD7176-2
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad7173.
+
 config AD7192
 	tristate "Analog Devices AD7190 AD7192 AD7193 AD7195 ADC driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index bee11d442af4..9f1f8f263340 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
 obj-$(CONFIG_AD4130) += ad4130.o
 obj-$(CONFIG_AD7091R5) += ad7091r5.o ad7091r-base.o
 obj-$(CONFIG_AD7124) += ad7124.o
+obj-$(CONFIG_AD7173) += ad7173.o
 obj-$(CONFIG_AD7192) += ad7192.o
 obj-$(CONFIG_AD7266) += ad7266.o
 obj-$(CONFIG_AD7280) += ad7280a.o
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
new file mode 100644
index 000000000000..e8c6c7666bd4
--- /dev/null
+++ b/drivers/iio/adc/ad7173.c
@@ -0,0 +1,967 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD7172-2/AD7173-8/AD7175-2/AD7176-2 SPI ADC driver
+ * Copyright (C) 2015, 2023 Analog Devices, Inc.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitmap.h>
+#include <linux/container_of.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/idr.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/gpio/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#include <linux/iio/adc/ad_sigma_delta.h>
+
+#define AD7173_REG_COMMS		0x00
+#define AD7173_REG_ADC_MODE		0x01
+#define AD7173_REG_INTERFACE_MODE	0x02
+#define AD7173_REG_CRC			0x03
+#define AD7173_REG_DATA			0x04
+#define AD7173_REG_GPIO			0x06
+#define AD7173_REG_ID			0x07
+#define AD7173_REG_CH(x)		(0x10 + (x))
+#define AD7173_REG_SETUP(x)		(0x20 + (x))
+#define AD7173_REG_FILTER(x)		(0x28 + (x))
+#define AD7173_REG_OFFSET(x)		(0x30 + (x))
+#define AD7173_REG_GAIN(x)		(0x38 + (x))
+
+#define AD7173_RESET_LENGTH		BITS_TO_BYTES(64)
+
+#define AD7173_CH_ENABLE		BIT(15)
+#define AD7173_CH_SETUP_SEL_MASK	GENMASK(14, 12)
+#define AD7173_CH_SETUP_AINPOS_MASK	GENMASK(9, 5)
+#define AD7173_CH_SETUP_AINNEG_MASK	GENMASK(4, 0)
+
+#define AD7173_CH_ADDRESS(pos, neg) \
+	(FIELD_PREP(AD7173_CH_SETUP_AINPOS_MASK, pos) | \
+	 FIELD_PREP(AD7173_CH_SETUP_AINNEG_MASK, neg))
+#define AD7173_AIN_TEMP_POS	17
+#define AD7173_AIN_TEMP_NEG	18
+
+#define AD7172_ID			0x00d0
+#define AD7173_ID			0x30d0
+#define AD7175_ID			0x0cd0
+#define AD7176_ID			0x0c90
+#define AD7173_ID_MASK			GENMASK(15, 4)
+
+#define AD7173_ADC_MODE_REF_EN		BIT(15)
+#define AD7173_ADC_MODE_SING_CYC	BIT(13)
+#define AD7173_ADC_MODE_MODE_MASK	GENMASK(6, 4)
+#define AD7173_ADC_MODE_CLOCKSEL_MASK	GENMASK(3, 2)
+
+#define AD7173_GPIO_PDSW	BIT(14)
+#define AD7173_GPIO_OP_EN2_3	BIT(13)
+#define AD7173_GPIO_MUX_IO	BIT(12)
+#define AD7173_GPIO_SYNC_EN	BIT(11)
+#define AD7173_GPIO_ERR_EN	BIT(10)
+#define AD7173_GPIO_ERR_DAT	BIT(9)
+#define AD7173_GPIO_GP_DATA3	BIT(7)
+#define AD7173_GPIO_GP_DATA2	BIT(6)
+#define AD7173_GPIO_IP_EN1	BIT(5)
+#define AD7173_GPIO_IP_EN0	BIT(4)
+#define AD7173_GPIO_OP_EN1	BIT(3)
+#define AD7173_GPIO_OP_EN0	BIT(2)
+#define AD7173_GPIO_GP_DATA1	BIT(1)
+#define AD7173_GPIO_GP_DATA0	BIT(0)
+
+#define AD7173_GPO12_DATA(x)	BIT(x)
+#define AD7173_GPO23_DATA(x)	BIT(x + 4)
+#define AD7173_GPO_DATA(x)	(x < 2 ? AD7173_GPO12_DATA(x) : AD7173_GPO23_DATA(x))
+
+#define AD7173_INTERFACE_DATA_STAT	BIT(6)
+#define AD7173_INTERFACE_DATA_STAT_EN(x) \
+	FIELD_PREP(AD7173_INTERFACE_DATA_STAT, x)
+
+#define AD7173_SETUP_BIPOLAR		BIT(12)
+#define AD7173_SETUP_AREF_BUF_MASK	GENMASK(11, 10)
+#define AD7173_SETUP_AIN_BUF_MASK	GENMASK(9, 8)
+
+#define AD7173_SETUP_REF_SEL_MASK	GENMASK(5, 4)
+#define AD7173_SETUP_REF_SEL_AVDD1_AVSS	0x3
+#define AD7173_SETUP_REF_SEL_INT_REF	0x2
+#define AD7173_SETUP_REF_SEL_EXT_REF2	0x1
+#define AD7173_SETUP_REF_SEL_EXT_REF	0x0
+#define AD7173_VOLTAGE_INT_REF_uV	2500000
+
+#define AD7173_FILTER_ODR0_MASK		GENMASK(5, 0)
+#define AD7173_MAX_CONFIGS		8
+
+enum ad7173_ids {
+	ID_AD7172_2,
+	ID_AD7173_8,
+	ID_AD7175_2,
+	ID_AD7176_2,
+};
+
+struct ad7173_device_info {
+	const unsigned int *sinc5_data_rates;
+	unsigned int num_sinc5_data_rates;
+	unsigned int num_channels;
+	unsigned int num_configs;
+	unsigned int num_inputs;
+	unsigned int clock;
+	unsigned int id;
+	char *name;
+	bool has_temp;
+	u8 num_gpios;
+};
+
+struct ad7173_channel_config {
+	u8 cfg_slot;
+	bool live;
+
+	/* Following fields are used to compare equality. */
+	struct_group(config_props,
+		bool bipolar;
+		bool input_buf;
+		u8 odr;
+		u8 ref_sel;
+	);
+};
+
+struct ad7173_channel {
+	unsigned int chan_reg;
+	unsigned int ain;
+	struct ad7173_channel_config cfg;
+};
+
+struct ad7173_state {
+	struct ad_sigma_delta sd;
+	const struct ad7173_device_info *info;
+	struct ad7173_channel *channels;
+	struct regulator_bulk_data regulators[3];
+	unsigned int adc_mode;
+	unsigned int interface_mode;
+	unsigned int num_channels;
+	struct ida cfg_slots_status;
+	unsigned long long config_usage_counter;
+	unsigned long long *config_cnts;
+#if IS_ENABLED(CONFIG_GPIOLIB)
+	struct regmap *reg_gpiocon_regmap;
+	struct gpio_regmap *gpio_regmap;
+#endif
+};
+
+static const unsigned int ad7173_sinc5_data_rates[] = {
+	6211000, 6211000, 6211000, 6211000, 6211000, 6211000, 5181000, 4444000,
+	3115000, 2597000, 1007000, 503800,  381000,  200300,  100500,  59520,
+	49680,	 20010,	  16333,   10000,   5000,    2500,    1250,
+};
+
+static const unsigned int ad7175_sinc5_data_rates[] = {
+	50000000, 41667000, 31250000, 27778000, 20833000, 17857000, 12500000,
+	10000000, 5000000,  2500000,  1000000,	500000,	  397500,   200000,
+	100000,	  59920,    49960,    20000,	16666,	  10000,    5000,
+};
+
+static const struct ad7173_device_info ad7173_device_info[] = {
+	[ID_AD7172_2] = {
+		.name = "ad7172-2",
+		.id = AD7172_ID,
+		.num_inputs = 5,
+		.num_channels = 4,
+		.num_configs = 4,
+		.num_gpios = 2,
+		.has_temp = true,
+		.clock = 2 * HZ_PER_MHZ,
+		.sinc5_data_rates = ad7173_sinc5_data_rates,
+		.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+	},
+	[ID_AD7173_8] = {
+		.name = "ad7173-8",
+		.id = AD7173_ID,
+		.num_inputs = 17,
+		.num_channels = 16,
+		.num_configs = 8,
+		.num_gpios = 4,
+		.has_temp = true,
+		.clock = 2 * HZ_PER_MHZ,
+		.sinc5_data_rates = ad7173_sinc5_data_rates,
+		.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+	},
+	[ID_AD7175_2] = {
+		.name = "ad7175-2",
+		.id = AD7175_ID,
+		.num_inputs = 5,
+		.num_channels = 4,
+		.num_configs = 4,
+		.num_gpios = 2,
+		.has_temp = true,
+		.clock = 16 * HZ_PER_MHZ,
+		.sinc5_data_rates = ad7175_sinc5_data_rates,
+		.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
+	},
+	[ID_AD7176_2] = {
+		.name = "ad7176-2",
+		.id = AD7176_ID,
+		.num_inputs = 5,
+		.num_channels = 4,
+		.num_configs = 4,
+		.num_gpios = 2,
+		.has_temp = false,
+		.clock = 16 * HZ_PER_MHZ,
+		.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]    = "refin",
+	[AD7173_SETUP_REF_SEL_EXT_REF2]   = "refin2",
+	[AD7173_SETUP_REF_SEL_INT_REF]    = "refout-avss",
+	[AD7173_SETUP_REF_SEL_AVDD1_AVSS] = "avdd",
+};
+
+#if IS_ENABLED(CONFIG_GPIOLIB)
+
+static const struct regmap_range ad7173_range_gpio[] = {
+	regmap_reg_range(AD7173_REG_GPIO, AD7173_REG_GPIO),
+};
+
+static const struct regmap_access_table ad7173_access_table = {
+	.yes_ranges = ad7173_range_gpio,
+	.n_yes_ranges = ARRAY_SIZE(ad7173_range_gpio),
+};
+
+static const struct regmap_config ad7173_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.rd_table = &ad7173_access_table,
+	.wr_table = &ad7173_access_table,
+	.read_flag_mask = BIT(6),
+};
+
+static int ad7173_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
+			     unsigned int offset, unsigned int *reg,
+			     unsigned int *mask)
+{
+	*mask = AD7173_GPO_DATA(offset);
+	*reg = base;
+	return 0;
+}
+
+static void ad7173_gpio_disable(void *data)
+{
+	struct ad7173_state *st = data;
+	unsigned int mask;
+
+	mask = AD7173_GPIO_OP_EN0 | AD7173_GPIO_OP_EN1 | AD7173_GPIO_OP_EN2_3;
+	regmap_update_bits(st->reg_gpiocon_regmap, AD7173_REG_GPIO, mask, ~mask);
+}
+
+static int ad7173_gpio_init(struct ad7173_state *st)
+{
+	struct gpio_regmap_config gpio_regmap = {};
+	struct device *dev = &st->sd.spi->dev;
+	unsigned int mask;
+	int ret;
+
+	st->reg_gpiocon_regmap = devm_regmap_init_spi(st->sd.spi, &ad7173_regmap_config);
+	ret = PTR_ERR_OR_ZERO(st->reg_gpiocon_regmap);
+	if (ret)
+		return dev_err_probe(dev, ret, "Unable to init regmap\n");
+
+	mask = AD7173_GPIO_OP_EN0 | AD7173_GPIO_OP_EN1 | AD7173_GPIO_OP_EN2_3;
+	regmap_update_bits(st->reg_gpiocon_regmap, AD7173_REG_GPIO, mask, mask);
+
+	ret = devm_add_action_or_reset(dev, ad7173_gpio_disable, st);
+	if (ret)
+		return ret;
+
+	gpio_regmap.parent = dev;
+	gpio_regmap.regmap = st->reg_gpiocon_regmap;
+	gpio_regmap.ngpio = st->info->num_gpios;
+	gpio_regmap.reg_set_base = AD7173_REG_GPIO;
+	gpio_regmap.reg_mask_xlate = ad7173_mask_xlate;
+
+	st->gpio_regmap = devm_gpio_regmap_register(dev, &gpio_regmap);
+	ret = PTR_ERR_OR_ZERO(st->gpio_regmap);
+	if (ret)
+		return dev_err_probe(dev, ret, "Unable to init gpio-regmap\n");
+
+	return 0;
+}
+
+#endif /* CONFIG_GPIOLIB */
+
+static struct ad7173_state *ad_sigma_delta_to_ad7173(struct ad_sigma_delta *sd)
+{
+	return container_of(sd, struct ad7173_state, sd);
+}
+
+static void ad7173_ida_destroy(void *data)
+{
+	struct ad7173_state *st = data;
+
+	ida_destroy(&st->cfg_slots_status);
+}
+
+static void ad7173_reset_usage_cnts(struct ad7173_state *st)
+{
+	memset64(st->config_cnts, 0, st->info->num_configs);
+	st->config_usage_counter = 0;
+}
+
+static struct ad7173_channel_config *
+ad7173_find_live_config(struct ad7173_state *st, struct ad7173_channel_config *cfg)
+{
+	struct ad7173_channel_config *cfg_aux;
+	ptrdiff_t cmp_size;
+	int i;
+
+	cmp_size = sizeof_field(struct ad7173_channel_config, config_props);
+	for (i = 0; i < st->num_channels; i++) {
+		cfg_aux = &st->channels[i].cfg;
+
+		if (cfg_aux->live &&
+		    !memcmp(&cfg->config_props, &cfg_aux->config_props, cmp_size))
+			return cfg_aux;
+	}
+	return NULL;
+}
+
+/* Could be replaced with a generic LRU implementation */
+static int ad7173_free_config_slot_lru(struct ad7173_state *st)
+{
+	int i, lru_position = 0;
+
+	for (i = 1; i < st->info->num_configs; i++)
+		if (st->config_cnts[i] < st->config_cnts[lru_position])
+			lru_position = i;
+
+	for (i = 0; i < st->num_channels; i++)
+		if (st->channels[i].cfg.cfg_slot == lru_position)
+			st->channels[i].cfg.live = false;
+
+	ida_free(&st->cfg_slots_status, lru_position);
+	return ida_alloc(&st->cfg_slots_status, GFP_KERNEL);
+}
+
+/* Could be replaced with a generic LRU implementation */
+static int ad7173_load_config(struct ad7173_state *st,
+			      struct ad7173_channel_config *cfg)
+{
+	unsigned int config;
+	int free_cfg_slot, ret;
+
+	free_cfg_slot = ida_alloc_range(&st->cfg_slots_status, 0,
+					st->info->num_configs - 1, GFP_KERNEL);
+	if (free_cfg_slot < 0)
+		free_cfg_slot = ad7173_free_config_slot_lru(st);
+
+	cfg->cfg_slot = free_cfg_slot;
+	config = FIELD_PREP(AD7173_SETUP_REF_SEL_MASK, cfg->ref_sel);
+
+	if (cfg->bipolar)
+		config |= AD7173_SETUP_BIPOLAR;
+
+	if (cfg->input_buf)
+		config |= AD7173_SETUP_AIN_BUF_MASK;
+
+	ret = ad_sd_write_reg(&st->sd, AD7173_REG_SETUP(free_cfg_slot), 2, config);
+	if (ret)
+		return ret;
+
+	return ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(free_cfg_slot), 2,
+			       AD7173_FILTER_ODR0_MASK & cfg->odr);
+}
+
+static int ad7173_config_channel(struct ad7173_state *st, int addr)
+{
+	struct ad7173_channel_config *cfg = &st->channels[addr].cfg;
+	struct ad7173_channel_config *live_cfg;
+	int ret;
+
+	if (!cfg->live) {
+		live_cfg = ad7173_find_live_config(st, cfg);
+		if (live_cfg) {
+			cfg->cfg_slot = live_cfg->cfg_slot;
+		} else {
+			ret = ad7173_load_config(st, cfg);
+			if (ret)
+				return ret;
+			cfg->live = true;
+		}
+	}
+
+	if (st->config_usage_counter == U64_MAX)
+		ad7173_reset_usage_cnts(st);
+
+	st->config_usage_counter++;
+	st->config_cnts[cfg->cfg_slot] = st->config_usage_counter;
+
+	return 0;
+}
+
+static int ad7173_set_channel(struct ad_sigma_delta *sd, unsigned int channel)
+{
+	struct ad7173_state *st = ad_sigma_delta_to_ad7173(sd);
+	unsigned int val;
+	int ret;
+
+	ret = ad7173_config_channel(st, channel);
+	if (ret)
+		return ret;
+
+	val = AD7173_CH_ENABLE |
+	      FIELD_PREP(AD7173_CH_SETUP_SEL_MASK, st->channels[channel].cfg.cfg_slot) |
+	      st->channels[channel].ain;
+
+	return ad_sd_write_reg(&st->sd, AD7173_REG_CH(channel), 2, val);
+}
+
+static int ad7173_set_mode(struct ad_sigma_delta *sd,
+			   enum ad_sigma_delta_mode mode)
+{
+	struct ad7173_state *st = ad_sigma_delta_to_ad7173(sd);
+
+	st->adc_mode &= ~AD7173_ADC_MODE_MODE_MASK;
+	st->adc_mode |= FIELD_PREP(AD7173_ADC_MODE_MODE_MASK, mode);
+
+	return ad_sd_write_reg(&st->sd, AD7173_REG_ADC_MODE, 2, st->adc_mode);
+}
+
+static int ad7173_append_status(struct ad_sigma_delta *sd, bool append)
+{
+	struct ad7173_state *st = ad_sigma_delta_to_ad7173(sd);
+	unsigned int interface_mode = st->interface_mode;
+	int ret;
+
+	interface_mode |= AD7173_INTERFACE_DATA_STAT_EN(append);
+	ret = ad_sd_write_reg(&st->sd, AD7173_REG_INTERFACE_MODE, 2, interface_mode);
+	if (ret)
+		return ret;
+
+	st->interface_mode = interface_mode;
+
+	return 0;
+}
+
+static int ad7173_disable_all(struct ad_sigma_delta *sd)
+{
+	struct ad7173_state *st = ad_sigma_delta_to_ad7173(sd);
+	int ret;
+	int i;
+
+	for (i = 0; i < st->num_channels; i++) {
+		ret = ad_sd_write_reg(sd, AD7173_REG_CH(i), 2, 0);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
+	.set_channel = ad7173_set_channel,
+	.append_status = ad7173_append_status,
+	.disable_all = ad7173_disable_all,
+	.set_mode = ad7173_set_mode,
+	.has_registers = true,
+	.addr_shift = 0,
+	.read_mask = BIT(6),
+	.status_ch_mask = GENMASK(3, 0),
+	.data_reg = AD7173_REG_DATA,
+	.irq_flags = IRQF_TRIGGER_FALLING,
+};
+
+static int ad7173_setup(struct iio_dev *indio_dev)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+	u8 buf[AD7173_RESET_LENGTH];
+	unsigned int id;
+	int ret;
+
+	/* reset the serial interface */
+	memset(buf, 0xff, AD7173_RESET_LENGTH);
+	ret = spi_write_then_read(st->sd.spi, buf, sizeof(buf), NULL, 0);
+	if (ret < 0)
+		return ret;
+
+	/* datasheet recommends a delay of at least 500us after reset */
+	fsleep(500);
+
+	ret = ad_sd_read_reg(&st->sd, AD7173_REG_ID, 2, &id);
+	if (ret)
+		return ret;
+
+	id &= AD7173_ID_MASK;
+	if (id != st->info->id)
+		dev_warn(&st->sd.spi->dev,
+			 "Unexpected device id: 0x%04X, expected: 0x%04X\n",
+			 id, st->info->id);
+
+	st->adc_mode |= AD7173_ADC_MODE_SING_CYC;
+	st->interface_mode = 0x0;
+
+	st->config_usage_counter = 0;
+	st->config_cnts = devm_kcalloc(indio_dev->dev.parent,
+				       st->info->num_configs, sizeof(u64),
+				       GFP_KERNEL);
+	if (!st->config_cnts)
+		return -ENOMEM;
+
+	/* All channels are enabled by default after a reset */
+	return ad7173_disable_all(&st->sd);
+}
+
+static unsigned int ad7173_get_ref_voltage_milli(struct ad7173_state *st,
+						 u8 reference_select)
+{
+	int vref;
+
+	switch (reference_select) {
+	case AD7173_SETUP_REF_SEL_EXT_REF:
+		vref = regulator_get_voltage(st->regulators[0].consumer);
+		break;
+
+	case AD7173_SETUP_REF_SEL_EXT_REF2:
+		vref = regulator_get_voltage(st->regulators[1].consumer);
+		break;
+
+	case AD7173_SETUP_REF_SEL_INT_REF:
+		vref = AD7173_VOLTAGE_INT_REF_uV;
+		break;
+
+	case AD7173_SETUP_REF_SEL_AVDD1_AVSS:
+		vref = regulator_get_voltage(st->regulators[2].consumer);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	if (vref < 0)
+		return vref;
+
+	return vref / (MICRO / MILLI);
+}
+
+static int ad7173_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long info)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+	struct ad7173_channel *ch = &st->channels[chan->address];
+	unsigned int reg;
+	int ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		ret = ad_sigma_delta_single_conversion(indio_dev, chan, val);
+		if (ret < 0)
+			return ret;
+
+		/* disable channel after single conversion */
+		ret = ad_sd_write_reg(&st->sd, AD7173_REG_CH(chan->address), 2, 0);
+		if (ret < 0)
+			return ret;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type == IIO_TEMP) {
+			*val = 250000000;
+			*val2 = 800273203; /* (2^24 * 477) / 10 */
+			return IIO_VAL_FRACTIONAL;
+		} else {
+			*val = ad7173_get_ref_voltage_milli(st, ch->cfg.ref_sel);
+			*val2 = chan->scan_type.realbits - !!(ch->cfg.bipolar);
+			return IIO_VAL_FRACTIONAL_LOG2;
+		}
+	case IIO_CHAN_INFO_OFFSET:
+		if (chan->type == IIO_TEMP)
+			*val = -874379;
+		else
+			*val = -BIT(chan->scan_type.realbits - 1);
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		reg = st->channels[chan->address].cfg.odr;
+
+		*val = st->info->sinc5_data_rates[reg] / MILLI;
+		*val2 = (st->info->sinc5_data_rates[reg] % MILLI) * (MICRO / MILLI);
+
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad7173_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long info)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+	struct ad7173_channel_config *cfg;
+	unsigned int freq, i, reg;
+	int ret;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		freq = val * MILLI + val2 / MILLI;
+		for (i = 0; i < st->info->num_sinc5_data_rates - 1; i++)
+			if (freq >= st->info->sinc5_data_rates[i])
+				break;
+
+		cfg = &st->channels[chan->address].cfg;
+		cfg->odr = i;
+
+		if (!cfg->live)
+			break;
+
+		ret = ad_sd_read_reg(&st->sd, AD7173_REG_FILTER(cfg->cfg_slot), 2, &reg);
+		if (ret)
+			break;
+		reg &= ~AD7173_FILTER_ODR0_MASK;
+		reg |= FIELD_PREP(AD7173_FILTER_ODR0_MASK, i);
+		ret = ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(cfg->cfg_slot), 2, reg);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	iio_device_release_direct_mode(indio_dev);
+	return ret;
+}
+
+static int ad7173_update_scan_mode(struct iio_dev *indio_dev,
+				   const unsigned long *scan_mask)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+	int i, ret;
+
+	for (i = 0; i < indio_dev->num_channels; i++) {
+		if (test_bit(i, scan_mask))
+			ret = ad7173_set_channel(&st->sd, i);
+		else
+			ret = ad_sd_write_reg(&st->sd, AD7173_REG_CH(i), 2, 0);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ad7173_debug_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+				   unsigned int writeval, unsigned int *readval)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+	u8 reg_size;
+
+	if (reg == 0)
+		reg_size = 1;
+	else if (reg == AD7173_REG_CRC || reg == AD7173_REG_DATA ||
+		 reg >= AD7173_REG_OFFSET(0))
+		reg_size = 3;
+	else
+		reg_size = 2;
+
+	if (readval)
+		return ad_sd_read_reg(&st->sd, reg, reg_size, readval);
+
+	return ad_sd_write_reg(&st->sd, reg, reg_size, writeval);
+}
+
+static const struct iio_info ad7173_info = {
+	.read_raw = &ad7173_read_raw,
+	.write_raw = &ad7173_write_raw,
+	.debugfs_reg_access = &ad7173_debug_reg_access,
+	.validate_trigger = ad_sd_validate_trigger,
+	.update_scan_mode = ad7173_update_scan_mode,
+};
+
+static const struct iio_chan_spec ad7173_channel_template = {
+	.type = IIO_VOLTAGE,
+	.indexed = 1,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+		BIT(IIO_CHAN_INFO_SCALE),
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+	.scan_type = {
+		.sign = 'u',
+		.realbits = 24,
+		.storagebits = 32,
+		.endianness = IIO_BE,
+	},
+};
+
+static const struct iio_chan_spec ad7173_temp_iio_channel_template = {
+	.type = IIO_TEMP,
+	.indexed = 1,
+	.channel = AD7173_AIN_TEMP_POS,
+	.channel2 = AD7173_AIN_TEMP_NEG,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+	.scan_type = {
+		.sign = 'u',
+		.realbits = 24,
+		.storagebits = 32,
+		.endianness = IIO_BE,
+	},
+};
+
+static void ad7173_disable_regulators(void *data)
+{
+	struct ad7173_state *st = data;
+
+	regulator_bulk_disable(ARRAY_SIZE(st->regulators), st->regulators);
+}
+
+static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
+{
+	struct ad7173_channel *channels_st_priv_arr, *chan_st_priv;
+	struct ad7173_state *st = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
+	struct iio_chan_spec *chan_arr, *chan;
+	struct fwnode_handle *child;
+	unsigned int ain[2], chan_index = 0;
+	unsigned int num_channels;
+	const char *ref_label;
+	u32 ref_sel;
+	int ret;
+
+	st->regulators[0].supply = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_EXT_REF];
+	st->regulators[1].supply = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_EXT_REF2];
+	st->regulators[2].supply = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_AVDD1_AVSS];
+
+	/* If a regulator is not available, it will be set to a dummy regulator.
+	 * Each channel reference is checked with regulator_get_voltage() before
+	 *  setting attributes so if any channel uses a dummy supply the driver
+	 *  probe will fail.
+	 */
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(st->regulators),
+				      st->regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(st->regulators), st->regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable regulators\n");
+
+	ret = devm_add_action_or_reset(dev, ad7173_disable_regulators, st);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to add regulators disable action\n");
+
+	num_channels = device_get_child_node_count(dev);
+
+	if (st->info->has_temp)
+		num_channels++;
+
+	if (num_channels == 0)
+		return dev_err_probe(dev, -EINVAL, "No channels specified\n");
+	st->num_channels = num_channels;
+
+	chan_arr = devm_kcalloc(dev, sizeof(*chan_arr), num_channels, GFP_KERNEL);
+	if (!chan_arr)
+		return -ENOMEM;
+
+	channels_st_priv_arr = devm_kcalloc(dev, num_channels,
+					    sizeof(*channels_st_priv_arr),
+					    GFP_KERNEL);
+	if (!channels_st_priv_arr)
+		return -ENOMEM;
+
+	indio_dev->channels = chan_arr;
+	indio_dev->num_channels = num_channels;
+	st->channels = channels_st_priv_arr;
+
+	if (st->info->has_temp) {
+		chan_arr[chan_index] = ad7173_temp_iio_channel_template;
+		chan_st_priv = &channels_st_priv_arr[chan_index];
+		chan_st_priv->ain =
+			AD7173_CH_ADDRESS(chan_arr[chan_index].channel, chan_arr[chan_index].channel2);
+		chan_st_priv->cfg.bipolar = false;
+		chan_st_priv->cfg.input_buf = true;
+		chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
+		st->adc_mode |= AD7173_ADC_MODE_REF_EN;
+
+		chan_index++;
+	}
+
+	device_for_each_child_node(dev, child) {
+		chan = &chan_arr[chan_index];
+		chan_st_priv = &channels_st_priv_arr[chan_index];
+		ret = fwnode_property_read_u32_array(child, "diff-channels",
+						     ain, ARRAY_SIZE(ain));
+		if (ret) {
+			fwnode_handle_put(child);
+			return ret;
+		}
+
+		if (ain[0] >= st->info->num_inputs ||
+		    ain[1] >= st->info->num_inputs) {
+			fwnode_handle_put(child);
+			return dev_err_probe(dev, -EINVAL,
+					     "Input pin number out of range for pair (%d %d).\n",
+					     ain[0], ain[1]);
+		}
+
+		ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
+		ref_label = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_INT_REF];
+
+		fwnode_property_read_string(child, "adi,reference-select",
+					    &ref_label);
+		ref_sel = match_string(ad7173_ref_sel_str,
+				       ARRAY_SIZE(ad7173_ref_sel_str), ref_label);
+		if (ref_sel < 0) {
+			fwnode_handle_put(child);
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid channel reference name %s\n",
+					     ref_label);
+		}
+
+		if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 &&
+		    st->info->id != AD7173_ID) {
+			fwnode_handle_put(child);
+			return dev_err_probe(dev, -EINVAL, "External reference 2 is only available on ad7173-8\n");
+		}
+
+		ret = ad7173_get_ref_voltage_milli(st, ref_sel);
+		if (ret < 0) {
+			fwnode_handle_put(child);
+			return dev_err_probe(dev, ret,
+					     "Cannot use reference %u\n", ref_sel);
+		}
+		if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF)
+			st->adc_mode |= AD7173_ADC_MODE_REF_EN;
+		chan_st_priv->cfg.ref_sel = ref_sel;
+
+		*chan = ad7173_channel_template;
+		chan->address = chan_index;
+		chan->scan_index = chan_index;
+		chan->channel = ain[0];
+		chan->channel2 = ain[1];
+		chan->differential = true;
+
+		chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
+		chan_st_priv->chan_reg = chan_index;
+		chan_st_priv->cfg.input_buf = true;
+		chan_st_priv->cfg.odr = 0;
+
+		chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
+		if (chan_st_priv->cfg.bipolar)
+			chan->info_mask_separate |= BIT(IIO_CHAN_INFO_OFFSET);
+
+		chan_index++;
+	}
+
+	return 0;
+}
+
+static int ad7173_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct ad7173_state *st;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->info = device_get_match_data(dev);
+	if (!st->info)
+		return -ENODEV;
+
+	ida_init(&st->cfg_slots_status);
+	ret = devm_add_action_or_reset(dev, ad7173_ida_destroy, st);
+	if (ret)
+		return ret;
+
+	indio_dev->name = st->info->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &ad7173_info;
+
+	spi->mode = SPI_MODE_3;
+
+	ad7173_sigma_delta_info.num_slots = st->info->num_configs;
+	ret = ad_sd_init(&st->sd, indio_dev, spi, &ad7173_sigma_delta_info);
+	if (ret)
+		return ret;
+
+	ret = ad7173_fw_parse_channel_config(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_ad_sd_setup_buffer_and_trigger(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	ret = ad7173_setup(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	if (IS_ENABLED(CONFIG_GPIOLIB))
+		return ad7173_gpio_init(st);
+
+	return 0;
+}
+
+static const struct of_device_id ad7173_of_match[] = {
+	{ .compatible = "adi,ad7172-2",
+	  .data = &ad7173_device_info[ID_AD7172_2]},
+	{ .compatible = "adi,ad7173-8",
+	  .data = &ad7173_device_info[ID_AD7173_8]},
+	{ .compatible = "adi,ad7175-2",
+	  .data = &ad7173_device_info[ID_AD7175_2]},
+	{ .compatible = "adi,ad7176-2",
+	  .data = &ad7173_device_info[ID_AD7176_2]},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad7173_of_match);
+
+static const struct spi_device_id ad7173_id_table[] = {
+	{ "ad7172-2", (kernel_ulong_t)&ad7173_device_info[ID_AD7172_2]},
+	{ "ad7173-8", (kernel_ulong_t)&ad7173_device_info[ID_AD7173_8]},
+	{ "ad7175-2", (kernel_ulong_t)&ad7173_device_info[ID_AD7175_2]},
+	{ "ad7176-2", (kernel_ulong_t)&ad7173_device_info[ID_AD7176_2]},
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad7173_id_table);
+
+static struct spi_driver ad7173_driver = {
+	.driver = {
+		.name	= "ad7173",
+		.of_match_table = ad7173_of_match,
+	},
+	.probe		= ad7173_probe,
+	.id_table	= ad7173_id_table,
+};
+module_spi_driver(ad7173_driver);
+
+MODULE_AUTHOR("Lars-Peter Clausen <lars@metafo.de>");
+MODULE_AUTHOR("Dumitru Ceclan <dumitru.ceclan@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD7172/AD7173/AD7175/AD7176 ADC driver");
+MODULE_LICENSE("GPL");
-- 
2.42.0

Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver

[Ceclan Dumitru]

On 12/5/23 15:42, Dumitru Ceclan wrote:
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel
> applications or higher speed multiplexed applications. The Sigma-Delta
> ADC is intended primarily for measurement of signals close to DC but also
> delivers outstanding performance with input bandwidths out to ~10kHz.
> 


V6 -> V7

 - format Kconfig supported models using '-'

 - include types.h instead of stddef.h

 - change device_info->num_gpios type to u8

 - reorder fields in ad7173_state, place ad_sigma_delta

 - reorder fields in ad7173_device_info, group by type

 - add default case in read_raw()

 - rename ad7173_debug() to ad7173_debug_reg_access()

 - remove explicit u8 cast

 - remove return 0; for offset in read_raw()

 - add '\n' to error messages

 - reorder probe variables -> reversed xmas tree

 - remove redundant inner commas in of_match and id_table

 - simplify selected reference code by setting default value and
ignoring fwnode_property_read_string() error

 - use match_string() for finding selected reference

 - err on no channels specified

 - add missing fwnode_handle_put(child) on err return branches

 - remove spi_set_drvdata() from probe

 - remove offset variable from find_live_config(), not used

 - add comment showing a generic LRU implementation might be used if one
will exist



 - MISC:  add blank line to chanel_config struct, trailing comma
ref_sel_str[],
 remove blank line in update_scan_mode(), add spaces around '/'

Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver

[Andy Shevchenko]

On Tue, Dec 5, 2023 at 3:46 PM Dumitru Ceclan <mitrutzceclan@gmail.com> wrote:
>
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel
> applications or higher speed multiplexed applications. The Sigma-Delta
> ADC is intended primarily for measurement of signals close to DC but also
> delivers outstanding performance with input bandwidths out to ~10kHz.

...

> +config AD7173
> +       tristate "Analog Devices AD7173 driver"
> +       depends on SPI_MASTER
> +       select AD_SIGMA_DELTA
> +       select GPIO_REGMAP if GPIOLIB
> +       select REGMAP_SPI if GPIOLIB
> +       help
> +         Say yes here to build support for Analog Devices AD7173 and similar ADC
> +         Currently supported models:
> +          - AD7172-2,
> +          - AD7173-8,
> +          - AD7175-2,
> +          - AD7176-2

Drop commas (sorry if I was not clear enough).


> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> +#include <linux/container_of.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>

> +#include <linux/gpio/driver.h>

This...

> +#include <linux/idr.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>

> +#include <linux/gpio/regmap.h>

...and this are too far from each other. I believe you should count on G order.

> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +#include <linux/units.h>

...

> +#define AD7173_GPO12_DATA(x)   BIT(x)

x + 0 ?

> +#define AD7173_GPO23_DATA(x)   BIT(x + 4)
> +#define AD7173_GPO_DATA(x)     (x < 2 ? AD7173_GPO12_DATA(x) : AD7173_GPO23_DATA(x))

...

> +               dev_warn(&st->sd.spi->dev,

Here...

> +                        "Unexpected device id: 0x%04X, expected: 0x%04X\n",
> +                        id, st->info->id);
> +
> +       st->adc_mode |= AD7173_ADC_MODE_SING_CYC;
> +       st->interface_mode = 0x0;
> +
> +       st->config_usage_counter = 0;
> +       st->config_cnts = devm_kcalloc(indio_dev->dev.parent,

...and here are different pointers in use, can you unify to use the
physical device pointer (as per above) here as well?

> +                                      st->info->num_configs, sizeof(u64),
> +                                      GFP_KERNEL);
> +       if (!st->config_cnts)
> +               return -ENOMEM;

...

> +       case IIO_CHAN_INFO_SCALE:
> +               if (chan->type == IIO_TEMP) {
> +                       *val = 250000000;

MEGA?

> +                       *val2 = 800273203; /* (2^24 * 477) / 10 */

Why not write it as is:

                       *val2 = (BIT(24) * 477) / 10;

?

It will be more robust as we don't expect compiler to give different
results here

> +                       return IIO_VAL_FRACTIONAL;
> +               } else {
> +                       *val = ad7173_get_ref_voltage_milli(st, ch->cfg.ref_sel);
> +                       *val2 = chan->scan_type.realbits - !!(ch->cfg.bipolar);
> +                       return IIO_VAL_FRACTIONAL_LOG2;
> +               }

...

> +       /* If a regulator is not available, it will be set to a dummy regulator.
> +        * Each channel reference is checked with regulator_get_voltage() before
> +        *  setting attributes so if any channel uses a dummy supply the driver
> +        *  probe will fail.
> +        */

/*
 * Multi-line comments should follow this style. Find the
 * difference.
 */

...

> +               ref_label = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_INT_REF];
> +
> +               fwnode_property_read_string(child, "adi,reference-select",
> +                                           &ref_label);
> +               ref_sel = match_string(ad7173_ref_sel_str,
> +                                      ARRAY_SIZE(ad7173_ref_sel_str), ref_label);
> +               if (ref_sel < 0) {

Can we use fwnode_property_match_property_string()?

> +                       fwnode_handle_put(child);
> +                       return dev_err_probe(dev, -EINVAL,
> +                                            "Invalid channel reference name %s\n",
> +                                            ref_label);
> +               }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver

[Andy Shevchenko]

On Tue, Dec 05, 2023 at 05:28:34PM +0200, Andy Shevchenko wrote:
> On Tue, Dec 5, 2023 at 3:46 PM Dumitru Ceclan <mitrutzceclan@gmail.com> wrote:

...

> > +       case IIO_CHAN_INFO_SCALE:
> > +               if (chan->type == IIO_TEMP) {
> > +                       *val = 250000000;
> 
> MEGA?

Or MICRO?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver

[Ceclan Dumitru]

On 12/5/23 17:28, Andy Shevchenko wrote:
>> +               ref_label = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_INT_REF];
>> +
>> +               fwnode_property_read_string(child, "adi,reference-select",
>> +                                           &ref_label);
>> +               ref_sel = match_string(ad7173_ref_sel_str,
>> +                                      ARRAY_SIZE(ad7173_ref_sel_str), ref_label);
>> +               if (ref_sel < 0) {
> Can we use fwnode_property_match_property_string()?

fwnode_property_match_string() searches a given string in a device-tree
string array and returns the index. I do not think that this function
fits here as the DT attribute is a single string.

Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver

[Andy Shevchenko]

On Tue, Dec 05, 2023 at 06:12:18PM +0200, Ceclan Dumitru wrote:
> On 12/5/23 17:28, Andy Shevchenko wrote:
> >> +               ref_label = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_INT_REF];
> >> +
> >> +               fwnode_property_read_string(child, "adi,reference-select",
> >> +                                           &ref_label);
> >> +               ref_sel = match_string(ad7173_ref_sel_str,
> >> +                                      ARRAY_SIZE(ad7173_ref_sel_str), ref_label);
> >> +               if (ref_sel < 0) {
> > Can we use fwnode_property_match_property_string()?
> 
> fwnode_property_match_string() searches a given string in a device-tree
> string array and returns the index. I do not think that this function
> fits here as the DT attribute is a single string.

I'm not talking about that. I mentioned different API call.

/**
 * fwnode_property_match_property_string - find a property string value in an array and return index
 * @fwnode: Firmware node to get the property of
 * @propname: Name of the property holding the string value
 * @array: String array to search in
 * @n: Size of the @array
 *
 * Find a property string value in a given @array and if it is found return
 * the index back.
 *
 * Return: index, starting from %0, if the string value was found in the @array (success),
 *         %-ENOENT when the string value was not found in the @array,
 *         %-EINVAL if given arguments are not valid,
 *         %-ENODATA if the property does not have a value,
 *         %-EPROTO or %-EILSEQ if the property is not a string,
 *         %-ENXIO if no suitable firmware interface is present.
 */

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver

[Ceclan Dumitru]

On 12/5/23 19:18, Andy Shevchenko wrote:
> On Tue, Dec 05, 2023 at 06:12:18PM +0200, Ceclan Dumitru wrote:
>> On 12/5/23 17:28, Andy Shevchenko wrote:
>>>> +               ref_label = 
>>>> + ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_INT_REF];
>>>> +
>>>> +               fwnode_property_read_string(child, "adi,reference-select",
>>>> +                                           &ref_label);
>>>> +               ref_sel = match_string(ad7173_ref_sel_str,
>>>> +                                      ARRAY_SIZE(ad7173_ref_sel_str), ref_label);
>>>> +               if (ref_sel < 0) {
>>> Can we use fwnode_property_match_property_string()?
>>
>> fwnode_property_match_string() searches a given string in a 
>> device-tree string array and returns the index. I do not think that 
>> this function fits here as the DT attribute is a single string.
> 
> I'm not talking about that. I mentioned different API call.
> 
> /**
>  * fwnode_property_match_property_string - find a property string value in an array and return index
>  * @fwnode: Firmware node to get the property of
>  * @propname: Name of the property holding the string value
>  * @array: String array to search in
>  * @n: Size of the @array
>  *
>  * Find a property string value in a given @array and if it is found return
>  * the index back.
>  *
>  * Return: index, starting from %0, if the string value was found in the @array (success),
>  *         %-ENOENT when the string value was not found in the @array,
>  *         %-EINVAL if given arguments are not valid,
>  *         %-ENODATA if the property does not have a value,
>  *         %-EPROTO or %-EILSEQ if the property is not a string,
>  *         %-ENXIO if no suitable firmware interface is present.
>  */
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

Yep, it can be used. I cannot seem to find it upstream but found the
thread, when the patch containing that function will be applied I'll
change to that.

Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver

[Jonathan Cameron]

On Tue, 5 Dec 2023 19:18:12 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Dec 05, 2023 at 06:12:18PM +0200, Ceclan Dumitru wrote:
> > On 12/5/23 17:28, Andy Shevchenko wrote:  
> > >> +               ref_label = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_INT_REF];
> > >> +
> > >> +               fwnode_property_read_string(child, "adi,reference-select",
> > >> +                                           &ref_label);
> > >> +               ref_sel = match_string(ad7173_ref_sel_str,
> > >> +                                      ARRAY_SIZE(ad7173_ref_sel_str), ref_label);
> > >> +               if (ref_sel < 0) {  
> > > Can we use fwnode_property_match_property_string()?  
> > 
> > fwnode_property_match_string() searches a given string in a device-tree
> > string array and returns the index. I do not think that this function
> > fits here as the DT attribute is a single string.  
> 
> I'm not talking about that. I mentioned different API call.
> 
> /**
>  * fwnode_property_match_property_string - find a property string value in an array and return index
>  * @fwnode: Firmware node to get the property of
>  * @propname: Name of the property holding the string value
>  * @array: String array to search in
>  * @n: Size of the @array
>  *
>  * Find a property string value in a given @array and if it is found return
>  * the index back.
>  *
>  * Return: index, starting from %0, if the string value was found in the @array (success),
>  *         %-ENOENT when the string value was not found in the @array,
>  *         %-EINVAL if given arguments are not valid,
>  *         %-ENODATA if the property does not have a value,
>  *         %-EPROTO or %-EILSEQ if the property is not a string,
>  *         %-ENXIO if no suitable firmware interface is present.
>  */
> 
Which is in the togreg branch of iio.git (was a patch from Andy that I've queued up)

Jonathan

Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver

[kernel test robot]

Hi Dumitru,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.7-rc4 next-20231206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dumitru-Ceclan/iio-adc-ad7173-add-AD7173-driver/20231205-214833
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20231205134223.17335-2-mitrutzceclan%40gmail.com
patch subject: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
config: m68k-randconfig-r071-20231207 (https://download.01.org/0day-ci/archive/20231207/202312070921.XWcr7wUd-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231207/202312070921.XWcr7wUd-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/202312070921.XWcr7wUd-lkp@intel.com/

smatch warnings:
drivers/iio/adc/ad7173.c:833 ad7173_fw_parse_channel_config() warn: unsigned 'ref_sel' is never less than zero.

vim +/ref_sel +833 drivers/iio/adc/ad7173.c

   735	
   736	static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
   737	{
   738		struct ad7173_channel *channels_st_priv_arr, *chan_st_priv;
   739		struct ad7173_state *st = iio_priv(indio_dev);
   740		struct device *dev = indio_dev->dev.parent;
   741		struct iio_chan_spec *chan_arr, *chan;
   742		struct fwnode_handle *child;
   743		unsigned int ain[2], chan_index = 0;
   744		unsigned int num_channels;
   745		const char *ref_label;
   746		u32 ref_sel;
   747		int ret;
   748	
   749		st->regulators[0].supply = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_EXT_REF];
   750		st->regulators[1].supply = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_EXT_REF2];
   751		st->regulators[2].supply = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_AVDD1_AVSS];
   752	
   753		/* If a regulator is not available, it will be set to a dummy regulator.
   754		 * Each channel reference is checked with regulator_get_voltage() before
   755		 *  setting attributes so if any channel uses a dummy supply the driver
   756		 *  probe will fail.
   757		 */
   758		ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(st->regulators),
   759					      st->regulators);
   760		if (ret)
   761			return dev_err_probe(dev, ret, "Failed to get regulators\n");
   762	
   763		ret = regulator_bulk_enable(ARRAY_SIZE(st->regulators), st->regulators);
   764		if (ret)
   765			return dev_err_probe(dev, ret, "Failed to enable regulators\n");
   766	
   767		ret = devm_add_action_or_reset(dev, ad7173_disable_regulators, st);
   768		if (ret)
   769			return dev_err_probe(dev, ret,
   770					     "Failed to add regulators disable action\n");
   771	
   772		num_channels = device_get_child_node_count(dev);
   773	
   774		if (st->info->has_temp)
   775			num_channels++;
   776	
   777		if (num_channels == 0)
   778			return dev_err_probe(dev, -EINVAL, "No channels specified\n");
   779		st->num_channels = num_channels;
   780	
   781		chan_arr = devm_kcalloc(dev, sizeof(*chan_arr), num_channels, GFP_KERNEL);
   782		if (!chan_arr)
   783			return -ENOMEM;
   784	
   785		channels_st_priv_arr = devm_kcalloc(dev, num_channels,
   786						    sizeof(*channels_st_priv_arr),
   787						    GFP_KERNEL);
   788		if (!channels_st_priv_arr)
   789			return -ENOMEM;
   790	
   791		indio_dev->channels = chan_arr;
   792		indio_dev->num_channels = num_channels;
   793		st->channels = channels_st_priv_arr;
   794	
   795		if (st->info->has_temp) {
   796			chan_arr[chan_index] = ad7173_temp_iio_channel_template;
   797			chan_st_priv = &channels_st_priv_arr[chan_index];
   798			chan_st_priv->ain =
   799				AD7173_CH_ADDRESS(chan_arr[chan_index].channel, chan_arr[chan_index].channel2);
   800			chan_st_priv->cfg.bipolar = false;
   801			chan_st_priv->cfg.input_buf = true;
   802			chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
   803			st->adc_mode |= AD7173_ADC_MODE_REF_EN;
   804	
   805			chan_index++;
   806		}
   807	
   808		device_for_each_child_node(dev, child) {
   809			chan = &chan_arr[chan_index];
   810			chan_st_priv = &channels_st_priv_arr[chan_index];
   811			ret = fwnode_property_read_u32_array(child, "diff-channels",
   812							     ain, ARRAY_SIZE(ain));
   813			if (ret) {
   814				fwnode_handle_put(child);
   815				return ret;
   816			}
   817	
   818			if (ain[0] >= st->info->num_inputs ||
   819			    ain[1] >= st->info->num_inputs) {
   820				fwnode_handle_put(child);
   821				return dev_err_probe(dev, -EINVAL,
   822						     "Input pin number out of range for pair (%d %d).\n",
   823						     ain[0], ain[1]);
   824			}
   825	
   826			ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
   827			ref_label = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_INT_REF];
   828	
   829			fwnode_property_read_string(child, "adi,reference-select",
   830						    &ref_label);
   831			ref_sel = match_string(ad7173_ref_sel_str,
   832					       ARRAY_SIZE(ad7173_ref_sel_str), ref_label);
 > 833			if (ref_sel < 0) {
   834				fwnode_handle_put(child);
   835				return dev_err_probe(dev, -EINVAL,
   836						     "Invalid channel reference name %s\n",
   837						     ref_label);
   838			}
   839	
   840			if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 &&
   841			    st->info->id != AD7173_ID) {
   842				fwnode_handle_put(child);
   843				return dev_err_probe(dev, -EINVAL, "External reference 2 is only available on ad7173-8\n");
   844			}
   845	
   846			ret = ad7173_get_ref_voltage_milli(st, ref_sel);
   847			if (ret < 0) {
   848				fwnode_handle_put(child);
   849				return dev_err_probe(dev, ret,
   850						     "Cannot use reference %u\n", ref_sel);
   851			}
   852			if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF)
   853				st->adc_mode |= AD7173_ADC_MODE_REF_EN;
   854			chan_st_priv->cfg.ref_sel = ref_sel;
   855	
   856			*chan = ad7173_channel_template;
   857			chan->address = chan_index;
   858			chan->scan_index = chan_index;
   859			chan->channel = ain[0];
   860			chan->channel2 = ain[1];
   861			chan->differential = true;
   862	
   863			chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
   864			chan_st_priv->chan_reg = chan_index;
   865			chan_st_priv->cfg.input_buf = true;
   866			chan_st_priv->cfg.odr = 0;
   867	
   868			chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
   869			if (chan_st_priv->cfg.bipolar)
   870				chan->info_mask_separate |= BIT(IIO_CHAN_INFO_OFFSET);
   871	
   872			chan_index++;
   873		}
   874	
   875		return 0;
   876	}
   877	

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

Re: [PATCH v7 1/2] dt-bindings: adc: add AD7173

[Ceclan Dumitru]

On 12/5/23 15:42, Dumitru Ceclan wrote:
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel applications
> or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
> 

Bindings V6 -> V7
 - remove redundant bipolar attribute
 - add reference-select example

Re: [PATCH v7 1/2] dt-bindings: adc: add AD7173

[Rob Herring]

On Tue, 05 Dec 2023 15:42:20 +0200, Dumitru Ceclan wrote:
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel applications
> or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
> 
> Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com>
> ---
>  .../bindings/iio/adc/adi,ad7173.yaml          | 170 ++++++++++++++++++
>  1 file changed, 170 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:required: ['compatible', 'reg', 'interrupts'] is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties: 'required' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
	hint: A json-schema keyword was found instead of a DT property name.
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:required: ['compatible', 'reg', 'interrupts'] is not of type 'object', 'boolean'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231205134223.17335-1-mitrutzceclan@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v7 1/2] dt-bindings: adc: add AD7173

[Conor Dooley]

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

On Tue, Dec 05, 2023 at 03:42:20PM +0200, Dumitru Ceclan wrote:
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel applications
> or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
> 
> Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com>
> ---
>  .../bindings/iio/adc/adi,ad7173.yaml          | 170 ++++++++++++++++++
>  1 file changed, 170 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> new file mode 100644
> index 000000000000..087820a0cf48
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> @@ -0,0 +1,170 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7173 ADC
> +
> +maintainers:
> +  - Ceclan Dumitru <dumitru.ceclan@analog.com>
> +
> +description: |
> +  Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips:
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7172-2
> +      - adi,ad7173-8
> +      - adi,ad7175-2
> +      - adi,ad7176-2
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  spi-max-frequency:
> +    maximum: 20000000
> +
> +  refin-supply:
> +    description: external reference supply, can be used as reference for conversion.
> +
> +  refin2-supply:
> +    description: external reference supply, can be used as reference for conversion.
> +
> +  avdd-supply:
> +    description: avdd supply, can be used as reference for conversion.
> +

> +  required:
> +    - compatible
> +    - reg
> +    - interrupts

This is at the wrong level of indent (as Rob's bot pointed out) and
should come after patternProperties too.

Otherwise, this looks okay to me.
Thanks,
Conor.

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

Re: [PATCH v7 1/2] dt-bindings: adc: add AD7173

[Ceclan Dumitru]

On 12/5/23 18:25, Conor Dooley wrote:
> On Tue, Dec 05, 2023 at 03:42:20PM +0200, Dumitru Ceclan wrote:
>> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
>> which can be used in high precision, low noise single channel applications
>> or higher speed multiplexed applications. The Sigma-Delta ADC is intended
>> primarily for measurement of signals close to DC but also delivers
>> outstanding performance with input bandwidths out to ~10kHz.
>>
...
>> +  required:
>> +    - compatible
>> +    - reg
>> +    - interrupts
> 
> This is at the wrong level of indent (as Rob's bot pointed out) and
> should come after patternProperties too.
> 

Yep, it was fixed in V6, forgot to include it