linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 1/2] dt-bindings: adc: add AD7173
@ 2023-12-05 13:42 Dumitru Ceclan
  2023-12-05 13:42 ` [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver Dumitru Ceclan
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Dumitru Ceclan @ 2023-12-05 13:42 UTC (permalink / raw)
  Cc: linus.walleij, brgl, andy, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Andy Shevchenko, Arnd Bergmann, ChiaEn Wu,
	Niklas Schnelle, Leonard Göhrs, Mike Looijmans, Haibo Chen,
	Hugo Villeneuve, Ceclan Dumitru, linux-iio, devicetree,
	linux-kernel, 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


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

* [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
  2023-12-05 13:42 [PATCH v7 1/2] dt-bindings: adc: add AD7173 Dumitru Ceclan
@ 2023-12-05 13:42 ` Dumitru Ceclan
  2023-12-05 13:49   ` Ceclan Dumitru
                     ` (2 more replies)
  2023-12-05 13:50 ` [PATCH v7 1/2] dt-bindings: adc: add AD7173 Ceclan Dumitru
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 14+ messages in thread
From: Dumitru Ceclan @ 2023-12-05 13:42 UTC (permalink / raw)
  Cc: linus.walleij, brgl, andy, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Andy Shevchenko, Arnd Bergmann, ChiaEn Wu,
	Niklas Schnelle, Leonard Göhrs, Mike Looijmans, Haibo Chen,
	Hugo Villeneuve, Ceclan Dumitru, linux-iio, devicetree,
	linux-kernel, 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


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

* Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
  2023-12-05 13:42 ` [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver Dumitru Ceclan
@ 2023-12-05 13:49   ` Ceclan Dumitru
  2023-12-05 15:28   ` Andy Shevchenko
  2023-12-07  1:22   ` kernel test robot
  2 siblings, 0 replies; 14+ messages in thread
From: Ceclan Dumitru @ 2023-12-05 13:49 UTC (permalink / raw)
  Cc: linus.walleij, brgl, andy, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Andy Shevchenko, Arnd Bergmann, ChiaEn Wu,
	Niklas Schnelle, Leonard Göhrs, Mike Looijmans, Haibo Chen,
	Hugo Villeneuve, Ceclan Dumitru, linux-iio, devicetree,
	linux-kernel



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 '/'

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

* Re: [PATCH v7 1/2] dt-bindings: adc: add AD7173
  2023-12-05 13:42 [PATCH v7 1/2] dt-bindings: adc: add AD7173 Dumitru Ceclan
  2023-12-05 13:42 ` [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver Dumitru Ceclan
@ 2023-12-05 13:50 ` Ceclan Dumitru
  2023-12-05 15:16 ` Rob Herring
  2023-12-05 16:25 ` Conor Dooley
  3 siblings, 0 replies; 14+ messages in thread
From: Ceclan Dumitru @ 2023-12-05 13:50 UTC (permalink / raw)
  Cc: linus.walleij, brgl, andy, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Andy Shevchenko, Arnd Bergmann, ChiaEn Wu,
	Niklas Schnelle, Leonard Göhrs, Mike Looijmans, Haibo Chen,
	Hugo Villeneuve, Ceclan Dumitru, linux-iio, devicetree,
	linux-kernel



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

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

* Re: [PATCH v7 1/2] dt-bindings: adc: add AD7173
  2023-12-05 13:42 [PATCH v7 1/2] dt-bindings: adc: add AD7173 Dumitru Ceclan
  2023-12-05 13:42 ` [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver Dumitru Ceclan
  2023-12-05 13:50 ` [PATCH v7 1/2] dt-bindings: adc: add AD7173 Ceclan Dumitru
@ 2023-12-05 15:16 ` Rob Herring
  2023-12-05 16:25 ` Conor Dooley
  3 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2023-12-05 15:16 UTC (permalink / raw)
  To: Dumitru Ceclan
  Cc: devicetree, ChiaEn Wu, linux-kernel, Ceclan Dumitru,
	Jonathan Cameron, andy, linux-iio, Andy Shevchenko, Conor Dooley,
	linux-gpio, Mike Looijmans, linus.walleij, Niklas Schnelle,
	Hugo Villeneuve, Rob Herring, Lars-Peter Clausen, brgl,
	Krzysztof Kozlowski, Leonard Göhrs, Arnd Bergmann,
	Haibo Chen, Michael Walle


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.


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

* Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
  2023-12-05 13:42 ` [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver Dumitru Ceclan
  2023-12-05 13:49   ` Ceclan Dumitru
@ 2023-12-05 15:28   ` Andy Shevchenko
  2023-12-05 15:59     ` Andy Shevchenko
  2023-12-05 16:12     ` Ceclan Dumitru
  2023-12-07  1:22   ` kernel test robot
  2 siblings, 2 replies; 14+ messages in thread
From: Andy Shevchenko @ 2023-12-05 15:28 UTC (permalink / raw)
  To: Dumitru Ceclan
  Cc: linus.walleij, brgl, andy, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Arnd Bergmann, ChiaEn Wu, Niklas Schnelle,
	Leonard Göhrs, Mike Looijmans, Haibo Chen, Hugo Villeneuve,
	Ceclan Dumitru, linux-iio, devicetree, linux-kernel

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

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

* Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
  2023-12-05 15:28   ` Andy Shevchenko
@ 2023-12-05 15:59     ` Andy Shevchenko
  2023-12-05 16:12     ` Ceclan Dumitru
  1 sibling, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2023-12-05 15:59 UTC (permalink / raw)
  To: Dumitru Ceclan
  Cc: linus.walleij, brgl, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Arnd Bergmann, ChiaEn Wu, Niklas Schnelle,
	Leonard Göhrs, Mike Looijmans, Haibo Chen, Hugo Villeneuve,
	Ceclan Dumitru, linux-iio, devicetree, linux-kernel

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



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

* Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
  2023-12-05 15:28   ` Andy Shevchenko
  2023-12-05 15:59     ` Andy Shevchenko
@ 2023-12-05 16:12     ` Ceclan Dumitru
  2023-12-05 17:18       ` Andy Shevchenko
  1 sibling, 1 reply; 14+ messages in thread
From: Ceclan Dumitru @ 2023-12-05 16:12 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linus.walleij, brgl, andy, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Arnd Bergmann, ChiaEn Wu, Niklas Schnelle,
	Leonard Göhrs, Mike Looijmans, Haibo Chen, Hugo Villeneuve,
	Ceclan Dumitru, linux-iio, devicetree, linux-kernel



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.

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

* Re: [PATCH v7 1/2] dt-bindings: adc: add AD7173
  2023-12-05 13:42 [PATCH v7 1/2] dt-bindings: adc: add AD7173 Dumitru Ceclan
                   ` (2 preceding siblings ...)
  2023-12-05 15:16 ` Rob Herring
@ 2023-12-05 16:25 ` Conor Dooley
  2023-12-05 16:33   ` Ceclan Dumitru
  3 siblings, 1 reply; 14+ messages in thread
From: Conor Dooley @ 2023-12-05 16:25 UTC (permalink / raw)
  To: Dumitru Ceclan
  Cc: linus.walleij, brgl, andy, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Andy Shevchenko, Arnd Bergmann, ChiaEn Wu,
	Niklas Schnelle, Leonard Göhrs, Mike Looijmans, Haibo Chen,
	Hugo Villeneuve, Ceclan Dumitru, linux-iio, devicetree,
	linux-kernel

[-- 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 --]

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

* Re: [PATCH v7 1/2] dt-bindings: adc: add AD7173
  2023-12-05 16:25 ` Conor Dooley
@ 2023-12-05 16:33   ` Ceclan Dumitru
  0 siblings, 0 replies; 14+ messages in thread
From: Ceclan Dumitru @ 2023-12-05 16:33 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linus.walleij, brgl, andy, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Andy Shevchenko, Arnd Bergmann, ChiaEn Wu,
	Niklas Schnelle, Leonard Göhrs, Mike Looijmans, Haibo Chen,
	Hugo Villeneuve, Ceclan Dumitru, linux-iio, devicetree,
	linux-kernel



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

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

* Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
  2023-12-05 16:12     ` Ceclan Dumitru
@ 2023-12-05 17:18       ` Andy Shevchenko
       [not found]         ` <e03e40b55f834d5cafb67bf728599688@analog.com>
  2023-12-06 17:48         ` Jonathan Cameron
  0 siblings, 2 replies; 14+ messages in thread
From: Andy Shevchenko @ 2023-12-05 17:18 UTC (permalink / raw)
  To: Ceclan Dumitru
  Cc: linus.walleij, brgl, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Arnd Bergmann, ChiaEn Wu, Niklas Schnelle,
	Leonard Göhrs, Mike Looijmans, Haibo Chen, Hugo Villeneuve,
	Ceclan Dumitru, linux-iio, devicetree, linux-kernel

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



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

* Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
       [not found]         ` <e03e40b55f834d5cafb67bf728599688@analog.com>
@ 2023-12-06 17:31           ` Ceclan Dumitru
  0 siblings, 0 replies; 14+ messages in thread
From: Ceclan Dumitru @ 2023-12-06 17:31 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linus.walleij, brgl, andy, linux-gpio, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Walle, Arnd Bergmann, ChiaEn Wu, Niklas Schnelle,
	Leonard Göhrs, Mike Looijmans, Haibo Chen, Hugo Villeneuve,
	Ceclan Dumitru, linux-iio, devicetree, linux-kernel


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.

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

* Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
  2023-12-05 17:18       ` Andy Shevchenko
       [not found]         ` <e03e40b55f834d5cafb67bf728599688@analog.com>
@ 2023-12-06 17:48         ` Jonathan Cameron
  1 sibling, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2023-12-06 17:48 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Ceclan Dumitru, linus.walleij, brgl, linux-gpio,
	Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Michael Walle, Arnd Bergmann, ChiaEn Wu,
	Niklas Schnelle, Leonard Göhrs, Mike Looijmans, Haibo Chen,
	Hugo Villeneuve, Ceclan Dumitru, linux-iio, devicetree,
	linux-kernel

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



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

* Re: [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver
  2023-12-05 13:42 ` [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver Dumitru Ceclan
  2023-12-05 13:49   ` Ceclan Dumitru
  2023-12-05 15:28   ` Andy Shevchenko
@ 2023-12-07  1:22   ` kernel test robot
  2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-12-07  1:22 UTC (permalink / raw)
  To: Dumitru Ceclan
  Cc: oe-kbuild-all, linus.walleij, brgl, andy, linux-gpio,
	Lars-Peter Clausen, Jonathan Cameron, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Walle, Andy Shevchenko,
	Arnd Bergmann, ChiaEn Wu, Niklas Schnelle, Leonard Göhrs,
	Mike Looijmans, Haibo Chen, Hugo Villeneuve, Ceclan Dumitru,
	linux-iio, devicetree, linux-kernel, Dumitru Ceclan

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

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

end of thread, other threads:[~2023-12-07  1:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-05 13:42 [PATCH v7 1/2] dt-bindings: adc: add AD7173 Dumitru Ceclan
2023-12-05 13:42 ` [PATCH v7 2/2] iio: adc: ad7173: add AD7173 driver Dumitru Ceclan
2023-12-05 13:49   ` Ceclan Dumitru
2023-12-05 15:28   ` Andy Shevchenko
2023-12-05 15:59     ` Andy Shevchenko
2023-12-05 16:12     ` Ceclan Dumitru
2023-12-05 17:18       ` Andy Shevchenko
     [not found]         ` <e03e40b55f834d5cafb67bf728599688@analog.com>
2023-12-06 17:31           ` Ceclan Dumitru
2023-12-06 17:48         ` Jonathan Cameron
2023-12-07  1:22   ` kernel test robot
2023-12-05 13:50 ` [PATCH v7 1/2] dt-bindings: adc: add AD7173 Ceclan Dumitru
2023-12-05 15:16 ` Rob Herring
2023-12-05 16:25 ` Conor Dooley
2023-12-05 16:33   ` Ceclan Dumitru

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).