linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] iio: adc: ad7944: new driver
@ 2024-02-16 19:46 David Lechner
  2024-02-16 19:46 ` [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs David Lechner
  2024-02-16 19:46 ` [PATCH v2 2/2] iio: adc: ad7944: add driver for AD7944/AD7985/AD7986 David Lechner
  0 siblings, 2 replies; 10+ messages in thread
From: David Lechner @ 2024-02-16 19:46 UTC (permalink / raw)
  To: linux-iio
  Cc: David Lechner, Michael Hennerich, Jonathan Cameron, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

This is a new driver for the Analog Devices AD7944/AD7985/AD7986 family
of ADCs. These are fairly simple chips (e.g. no configuration registers).
The initial driver is only supporting the 'multi' (4-wire) SPI mode. We
plan to follow up with support for the 'single' (3-wire) SPI mode.

This work is done on behalf of Analog Devices, Inc., hence the
MAINTAINERS are @analog.com folks.

---
Changes in v2:
- Added limit to spi-max-frequency for chain mode in DT bindings
- Added spi-cpol property to DT bindings
- Renamed '3-wire' mode to 'single' mode (to avoid confusion with spi-3wire)
- Renamed '4-wire' mode to 'multi' mode
- Dropped adi,reference property - now using only ref-supply and 
  refin-supply to determine the reference voltage source
- Fixed spelling of TURBO
- Renamed t_cnv to t_conv to match datasheet name and fixed comment
- Fixed wrong timestamp pushed to buffer
- Fixed scaling for chips with signed data
- Make use of sysfs_match_string() function
- Link to v1: https://lore.kernel.org/r/20240206-ad7944-mainline-v1-0-bf115fa9474f@baylibre.com

---
David Lechner (2):
      dt-bindings: iio: adc: add ad7944 ADCs
      iio: adc: ad7944: add driver for AD7944/AD7985/AD7986

 .../devicetree/bindings/iio/adc/adi,ad7944.yaml    | 204 ++++++++++
 MAINTAINERS                                        |   9 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/ad7944.c                           | 427 +++++++++++++++++++++
 5 files changed, 651 insertions(+)
---
base-commit: bd2f1ed8873d4bbb2798151bbe28c86565251cfb
change-id: 20240206-ad7944-mainline-17c968aa0967

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

* [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs
  2024-02-16 19:46 [PATCH v2 0/2] iio: adc: ad7944: new driver David Lechner
@ 2024-02-16 19:46 ` David Lechner
  2024-02-20 19:47   ` Conor Dooley
  2024-02-21 15:22   ` Rob Herring
  2024-02-16 19:46 ` [PATCH v2 2/2] iio: adc: ad7944: add driver for AD7944/AD7985/AD7986 David Lechner
  1 sibling, 2 replies; 10+ messages in thread
From: David Lechner @ 2024-02-16 19:46 UTC (permalink / raw)
  To: linux-iio
  Cc: David Lechner, Michael Hennerich, Jonathan Cameron, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

This adds a new binding for the Analog Devices, Inc. AD7944, AD7985, and
AD7986 ADCs.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 .../devicetree/bindings/iio/adc/adi,ad7944.yaml    | 204 +++++++++++++++++++++
 MAINTAINERS                                        |   8 +
 2 files changed, 212 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml
new file mode 100644
index 000000000000..61ee81326660
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml
@@ -0,0 +1,204 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7944.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices PulSAR LFCSP Analog to Digital Converters
+
+maintainers:
+  - Michael Hennerich <Michael.Hennerich@analog.com>
+  - Nuno Sá <nuno.sa@analog.com>
+
+description: |
+  A family of pin-compatible single channel differential analog to digital
+  converters with SPI support in a LFCSP package.
+
+  * https://www.analog.com/en/products/ad7944.html
+  * https://www.analog.com/en/products/ad7985.html
+  * https://www.analog.com/en/products/ad7986.html
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7944
+      - adi,ad7985
+      - adi,ad7986
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 111111111
+
+  spi-cpol: true
+  spi-cpha: true
+
+  adi,spi-mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [ single, multi, chain ]
+    default: multi
+    description: |
+      * single: The datasheet calls this "3-wire mode". It is often used when
+        the ADC is the only device on the bus. In this mode, SDI is tied to VIO,
+        and the CNV line can be connected to the CS line of the SPI controller
+        or to a GPIO, in which case the CS line of the controller is unused.
+      * multi: The datasheet calls this "4-wire mode". This is the convential
+        SPI mode used when there are multiple devices on the same bus. In this
+        mode, the CNV line is used to initiate the conversion and the SDI line
+        is connected to CS on the SPI controller.
+      * chain: The datasheet calls this "chain mode". This mode is used to save
+        on wiring when multiple ADCs are used. In this mode, the SDI line of
+        one chip is tied to the SDO of the next chip in the chain and the SDI of
+        the last chip in the chain is tied to GND. Only the first chip in the
+        chain is connected to the SPI bus. The CNV line of all chips are tied
+        together. The CS line of the SPI controller is unused.
+
+  avdd-supply:
+    description: A 2.5V supply that powers the analog circuitry.
+
+  dvdd-supply:
+    description: A 2.5V supply that powers the digital circuitry.
+
+  vio-supply:
+    description:
+      A 1.8V to 2.7V supply for the digital inputs and outputs.
+
+  bvdd-supply:
+    description:
+      A voltage supply for the buffered power. When using an external reference
+      without an internal buffer (PDREF high, REFIN low), this should be
+      connected to the same supply as ref-supply. Otherwise, when using an
+      internal reference or an external reference with an internal buffer, this
+      is connected to a 5V supply.
+
+  ref-supply:
+    description:
+      Voltage regulator for the external reference voltage (REF). This property
+      is omitted when using an internal reference.
+
+  refin-supply:
+    description:
+      Voltage regulator for the reference buffer input (REFIN). When using an
+      external buffer with internal reference, this should be connected to a
+      1.2V external reference voltage supply. Otherwise, this property is
+      omitted.
+
+  cnv-gpios:
+    description:
+      The Convert Input (CNV). This input has multiple functions. It initiates
+      the conversions and selects the SPI mode of the device (chain or CS). In
+      'single' mode, this property is omitted if the CNV pin is connected to the
+      CS line of the SPI controller.
+    maxItems: 1
+
+  turbo-gpios:
+    description:
+      GPIO connected to the TURBO line. If omitted, it is assumed that the TURBO
+      line is hard-wired and the state is determined by the adi,always-turbo
+      property.
+    maxItems: 1
+
+  adi,always-turbo:
+    type: boolean
+    description:
+      When present, this property indicates that the TURBO line is hard-wired
+      and the state is always high. If neither this property nor turbo-gpios is
+      present, the TURBO line is assumed to be hard-wired and the state is
+      always low.
+
+  interrupts:
+    description:
+      The SDO pin can also function as a busy indicator. This node should be
+      connected to an interrupt that is triggered when the SDO line goes low
+      while the SDI line is high and the CNV line is low ('single' mode) or the
+      SDI line is low and the CNV line is high ('multi' mode); or when the SDO
+      line goes high while the SDI and CNV lines are high (chain mode),
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - avdd-supply
+  - dvdd-supply
+  - vio-supply
+  - bvdd-supply
+
+allOf:
+  # ref-supply and refin-supply are mutually exclusive (neither is also valid)
+  - if:
+      required:
+        - ref-supply
+    then:
+      properties:
+        refin-supply: false
+  - if:
+      required:
+        - refin-supply
+    then:
+      properties:
+        ref-supply: false
+  # in 'single' mode, cnv-gpios is optional, for other modes it is required
+  - if:
+      required:
+        - adi,spi-mode
+    then:
+      if:
+        properties:
+          adi,spi-mode:
+            enum: [ multi, chain ]
+      then:
+        required:
+          - cnv-gpios
+    else:
+      required:
+        - cnv-gpios
+  # chain mode has lower SCLK max rate and doesn't work when TURBO is enabled
+  - if:
+      required:
+        - adi,spi-mode
+      properties:
+        adi,spi-mode:
+          const: chain
+    then:
+      properties:
+        spi-max-frequency:
+          maximum: 90909090
+        adi,always-turbo: false
+  # turbo-gpios and adi,always-turbo are mutually exclusive
+  - if:
+      required:
+        - turbo-gpios
+    then:
+      properties:
+        adi,always-turbo: false
+  - if:
+      required:
+        - adi,always-turbo
+    then:
+      properties:
+        turbo-gpios: false
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        adc@0 {
+            compatible = "adi,ad7944";
+            reg = <0>;
+            spi-cpha;
+            spi-max-frequency = <111111111>;
+            avdd-supply = <&supply_2_5V>;
+            dvdd-supply = <&supply_2_5V>;
+            vio-supply = <&supply_1_8V>;
+            bvdd-supply = <&supply_5V>;
+            cnv-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
+            turbo-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 00d354af10f5..4f1e658e1e0d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -451,6 +451,14 @@ W:	http://wiki.analog.com/AD7879
 W:	https://ez.analog.com/linux-software-drivers
 F:	drivers/input/touchscreen/ad7879.c
 
+AD7944 ADC DRIVER (AD7944/AD7985/AD7986)
+M:	Michael Hennerich <michael.hennerich@analog.com>
+M:	Nuno Sá <nuno.sa@analog.com>
+R:	David Lechner <dlechner@baylibre.com>
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml
+
 ADAFRUIT MINI I2C GAMEPAD
 M:	Anshul Dalal <anshulusr@gmail.com>
 L:	linux-input@vger.kernel.org

-- 
2.43.2


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

* [PATCH v2 2/2] iio: adc: ad7944: add driver for AD7944/AD7985/AD7986
  2024-02-16 19:46 [PATCH v2 0/2] iio: adc: ad7944: new driver David Lechner
  2024-02-16 19:46 ` [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs David Lechner
@ 2024-02-16 19:46 ` David Lechner
  2024-02-19 19:13   ` David Lechner
  1 sibling, 1 reply; 10+ messages in thread
From: David Lechner @ 2024-02-16 19:46 UTC (permalink / raw)
  To: linux-iio
  Cc: David Lechner, Michael Hennerich, Jonathan Cameron, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

This adds a driver for the Analog Devices Inc. AD7944, AD7985, and
AD7986 ADCs. These are a family of pin-compatible ADCs that can sample
at rates up to 2.5 MSPS.

The initial driver adds support for sampling at lower rates using the
usual IIO triggered buffer and can handle all 3 possible reference
voltage configurations.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  10 ++
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad7944.c | 427 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 439 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4f1e658e1e0d..83d8367595f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -458,6 +458,7 @@ R:	David Lechner <dlechner@baylibre.com>
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml
+F:	drivers/iio/adc/ad7944.c
 
 ADAFRUIT MINI I2C GAMEPAD
 M:	Anshul Dalal <anshulusr@gmail.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 59ae1d17b50d..93fbe6f8e306 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -280,6 +280,16 @@ config AD7923
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7923.
 
+config AD7944
+	tristate "Analog Devices AD7944 and similar ADCs driver"
+	depends on SPI
+	help
+	  Say yes here to build support for Analog Devices
+	  AD7944, AD7985, AD7986 ADCs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad7944
+
 config AD7949
 	tristate "Analog Devices AD7949 and similar ADCs driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 5a26ab6f1109..52d803b92cd7 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_AD7780) += ad7780.o
 obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7793) += ad7793.o
 obj-$(CONFIG_AD7887) += ad7887.o
+obj-$(CONFIG_AD7944) += ad7944.o
 obj-$(CONFIG_AD7949) += ad7949.o
 obj-$(CONFIG_AD799X) += ad799x.o
 obj-$(CONFIG_AD9467) += ad9467.o
diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c
new file mode 100644
index 000000000000..d1964ef02fb5
--- /dev/null
+++ b/drivers/iio/adc/ad7944.c
@@ -0,0 +1,427 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD7944/85/86 PulSAR ADC family driver.
+ *
+ * Copyright 2024 Analog Devices, Inc.
+ * Copyright 2024 BayLibre, SAS
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/string_helpers.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#define AD7944_INTERNAL_REF_MV		4096
+
+enum ad7944_spi_mode {
+	/* datasheet calls this 3-wire mode */
+	AD7944_SPI_MODE_SINGLE,
+	/* datasheet calls this 4-wire mode */
+	AD7944_SPI_MODE_MULTI,
+	/* datasheet calls this chain mode */
+	AD7944_SPI_MODE_CHAIN,
+};
+
+struct ad7944_timing_spec {
+	/* Normal mode max conversion time (t_{CONV}) in nanoseconds. */
+	unsigned int conv_ns;
+	/* TURBO mode max conversion time (t_{CONV}) in nanoseconds. */
+	unsigned int turbo_conv_ns;
+};
+
+struct ad7944_adc {
+	struct spi_device *spi;
+	/* Chip-specific timing specifications. */
+	const struct ad7944_timing_spec *t;
+	/* GPIO connected to CNV pin. */
+	struct gpio_desc *cnv;
+	/* Optional GPIO to enable turbo mode. */
+	struct gpio_desc *turbo;
+	/* Indicates TURBO is hard-wired to be always enabled. */
+	bool always_turbo;
+	/* Reference voltage (millivolts). */
+	unsigned int ref_mv;
+
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	struct {
+		union {
+			u16 u16;
+			u32 u32;
+		} raw;
+		u64 timestamp __aligned(8);
+	 } sample __aligned(IIO_DMA_MINALIGN);
+};
+
+static const struct ad7944_timing_spec ad7944_timing_spec = {
+	.conv_ns = 420,
+	.turbo_conv_ns = 320,
+};
+
+static const struct ad7944_timing_spec ad7986_timing_spec = {
+	.conv_ns = 500,
+	.turbo_conv_ns = 400,
+};
+
+struct ad7944_chip_info {
+	const char *name;
+	const struct ad7944_timing_spec *t;
+	const struct iio_chan_spec channels[2];
+};
+
+#define AD7944_DEFINE_CHIP_INFO(_name, _t, _bits, _sign)		\
+static const struct ad7944_chip_info _name##_chip_info = {		\
+	.name = #_name,							\
+	.t = &_t##_timing_spec,						\
+	.channels = {							\
+		{							\
+			.type = IIO_VOLTAGE,				\
+			.indexed = 1,					\
+			.differential = 1,				\
+			.channel = 0,					\
+			.channel2 = 1,					\
+			.scan_index = 0,				\
+			.scan_type.sign = _sign,			\
+			.scan_type.realbits = _bits,			\
+			.scan_type.storagebits = _bits > 16 ? 32 : 16,	\
+			.scan_type.endianness = IIO_CPU,		\
+			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)	\
+					| BIT(IIO_CHAN_INFO_SCALE),	\
+		},							\
+		IIO_CHAN_SOFT_TIMESTAMP(1),				\
+	},								\
+}
+
+AD7944_DEFINE_CHIP_INFO(ad7944, ad7944, 14, 'u');
+AD7944_DEFINE_CHIP_INFO(ad7985, ad7944, 16, 'u');
+AD7944_DEFINE_CHIP_INFO(ad7986, ad7986, 18, 's');
+
+static const char * const ad7944_spi_modes[] = {
+	[AD7944_SPI_MODE_SINGLE] = "single",
+	[AD7944_SPI_MODE_MULTI] = "multi",
+	[AD7944_SPI_MODE_CHAIN] = "chain",
+};
+
+/**
+ * ad7944_multi_mode_conversion - Perform a multi (4-wire) mode conversion and
+ *                                acquisition
+ * @adc: The ADC device structure
+ * @chan: The channel specification
+ * Return: 0 on success, a negative error code on failure
+ *
+ * Upon successful return adc->sample.raw will contain the conversion result.
+ */
+static int ad7944_multi_mode_conversion(struct ad7944_adc *adc,
+					 const struct iio_chan_spec *chan)
+{
+	unsigned int t_conv_ns = adc->always_turbo ? adc->t->turbo_conv_ns
+						   : adc->t->conv_ns;
+	struct spi_transfer xfers[] = {
+		{
+			/*
+			 * NB: can get better performance from some SPI
+			 * controllers if we use the same bits_per_word
+			 * in every transfer.
+			 */
+			.bits_per_word = chan->scan_type.realbits,
+			/*
+			 * CS has to be high for full conversion time to avoid
+			 * triggering the busy indication.
+			 */
+			.cs_off = 1,
+			.delay = {
+				.value = t_conv_ns,
+				.unit = SPI_DELAY_UNIT_NSECS,
+			},
+
+		},
+		{
+			.rx_buf = &adc->sample.raw,
+			.len = BITS_TO_BYTES(chan->scan_type.storagebits),
+			.bits_per_word = chan->scan_type.realbits,
+		},
+	};
+	int ret;
+
+	/*
+	 * In 4-wire mode, the CNV line is held high for the entire conversion
+	 * and acquisition process.
+	 */
+	gpiod_set_value_cansleep(adc->cnv, 1);
+
+	ret = spi_sync_transfer(adc->spi, xfers, ARRAY_SIZE(xfers));
+	if (ret)
+		return ret;
+
+	gpiod_set_value_cansleep(adc->cnv, 0);
+
+	return 0;
+}
+
+static int ad7944_single_conversion(struct ad7944_adc *adc,
+				    const struct iio_chan_spec *chan,
+				    int *val)
+{
+	int ret;
+
+	ret = ad7944_multi_mode_conversion(adc, chan);
+	if (ret)
+		return ret;
+
+	if (chan->scan_type.storagebits > 16)
+		*val = adc->sample.raw.u32;
+	else
+		*val = adc->sample.raw.u16;
+
+	if (chan->scan_type.sign == 's')
+		*val = sign_extend32(*val, chan->scan_type.realbits - 1);
+
+	return IIO_VAL_INT;
+}
+
+static int ad7944_read_raw(struct iio_dev *indio_dev,
+			   const struct iio_chan_spec *chan,
+			   int *val, int *val2, long info)
+{
+	struct ad7944_adc *adc = iio_priv(indio_dev);
+	int ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			return ret;
+
+		ret = ad7944_single_conversion(adc, chan, val);
+		iio_device_release_direct_mode(indio_dev);
+		return ret;
+
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			*val = adc->ref_mv;
+			if (chan->scan_type.sign == 's')
+				*val2 = chan->scan_type.realbits - 1;
+			else
+				*val2 = chan->scan_type.realbits;
+
+			return IIO_VAL_FRACTIONAL_LOG2;
+		default:
+			return -EINVAL;
+		}
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info ad7944_iio_info = {
+	.read_raw = &ad7944_read_raw,
+};
+
+static irqreturn_t ad7944_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ad7944_adc *adc = iio_priv(indio_dev);
+	int ret;
+
+	ret = ad7944_multi_mode_conversion(adc, &indio_dev->channels[0]);
+	if (ret)
+		goto out;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &adc->sample.raw,
+					   pf->timestamp);
+
+out:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static const char * const ad7944_power_supplies[] = {
+	"avdd",	"dvdd",	"bvdd", "vio"
+};
+
+static void ad7944_ref_disable(void *ref)
+{
+	regulator_disable(ref);
+}
+
+static int ad7944_probe(struct spi_device *spi)
+{
+	const struct ad7944_chip_info *chip_info;
+	enum ad7944_spi_mode spi_mode;
+	struct iio_dev *indio_dev;
+	struct ad7944_adc *adc;
+	bool have_refin = false;
+	struct regulator *ref;
+	const char *str_val;
+	int ret;
+
+	/* adi,spi-mode property defaults to "multi" if not present */
+	if (device_property_read_string(&spi->dev, "adi,spi-mode", &str_val) < 0)
+		str_val = "multi";
+
+	spi_mode = sysfs_match_string(ad7944_spi_modes, str_val);
+
+	if (spi_mode != AD7944_SPI_MODE_MULTI)
+		return dev_err_probe(&spi->dev, -EINVAL,
+			"only adi,spi-mode = \"multi\" is currently supported\n");
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	adc = iio_priv(indio_dev);
+	adc->spi = spi;
+
+	chip_info = spi_get_device_match_data(spi);
+	if (!chip_info)
+		return dev_err_probe(&spi->dev, -EINVAL, "no chip info\n");
+
+	adc->t = chip_info->t;
+
+	/*
+	 * Some chips use unusual word sizes, so check now instead of waiting
+	 * for the first xfer.
+	 */
+	if (!spi_is_bpw_supported(spi, chip_info->channels[0].scan_type.realbits))
+		return dev_err_probe(&spi->dev, -EINVAL,
+				"SPI host does not support %d bits per word\n",
+				chip_info->channels[0].scan_type.realbits);
+
+	ret = devm_regulator_bulk_get_enable(&spi->dev,
+					     ARRAY_SIZE(ad7944_power_supplies),
+					     ad7944_power_supplies);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "failed to get and enable supplies\n");
+
+	/*
+	 * Sort out what is being used for the reference voltage. Options are:
+	 * - internal reference: neither REF or REFIN is connected
+	 * - internal reference with external buffer: REF not connected, REFIN
+	 *   is connected
+	 * - external reference: REF is connected, REFIN is not connected
+	 */
+
+	ref = devm_regulator_get_optional(&spi->dev, "ref");
+	if (IS_ERR(ref)) {
+		if (PTR_ERR(ref) != -ENODEV)
+			ref = NULL;
+		else
+			return dev_err_probe(&spi->dev, PTR_ERR(ref),
+					     "failed to get REF supply\n");
+	}
+
+	ret = devm_regulator_get_enable_optional(&spi->dev, "refin");
+	if (ret == 0)
+		have_refin = true;
+	else if (ret != -ENODEV)
+		return dev_err_probe(&spi->dev, ret,
+				     "failed to get and enable REFIN supply\n");
+
+	if (have_refin && ref)
+		return dev_err_probe(&spi->dev, -EINVAL,
+				     "cannot have both refin and ref supplies\n");
+
+	if (ref) {
+		ret = regulator_enable(ref);
+		if (ret)
+			return dev_err_probe(&spi->dev, ret,
+					     "failed to enable REF supply\n");
+
+		ret = devm_add_action_or_reset(&spi->dev,
+						ad7944_ref_disable, ref);
+		if (ret)
+			return ret;
+
+		ret = regulator_get_voltage(ref);
+		if (ret < 0)
+			return dev_err_probe(&spi->dev, ret,
+					     "failed to get REF voltage\n");
+
+		/* external reference */
+		adc->ref_mv = ret / 1000;
+	} else {
+		/* internal reference */
+		adc->ref_mv = AD7944_INTERNAL_REF_MV;
+	}
+
+	adc->cnv = devm_gpiod_get(&spi->dev, "cnv", GPIOD_OUT_LOW);
+	if (IS_ERR(adc->cnv))
+		return dev_err_probe(&spi->dev, PTR_ERR(adc->cnv),
+				     "failed to get CNV GPIO\n");
+
+	adc->turbo = devm_gpiod_get_optional(&spi->dev, "turbo", GPIOD_OUT_LOW);
+	if (IS_ERR(adc->turbo))
+		return dev_err_probe(&spi->dev, PTR_ERR(adc->turbo),
+				     "failed to get TURBO GPIO\n");
+
+	if (device_property_present(&spi->dev, "adi,always-turbo"))
+		adc->always_turbo = true;
+
+	if (adc->turbo && adc->always_turbo)
+		return dev_err_probe(&spi->dev, -EINVAL,
+			"cannot have both turbo-gpios and adi,always-turbo\n");
+
+	indio_dev->name = chip_info->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &ad7944_iio_info;
+	indio_dev->channels = chip_info->channels;
+	indio_dev->num_channels = ARRAY_SIZE(chip_info->channels);
+
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+					      iio_pollfunc_store_time,
+					      ad7944_trigger_handler, NULL);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id ad7944_of_match[] = {
+	{ .compatible = "adi,ad7944", .data = &ad7944_chip_info },
+	{ .compatible = "adi,ad7985", .data = &ad7985_chip_info },
+	{ .compatible = "adi,ad7986", .data = &ad7986_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad7944_of_match);
+
+static const struct spi_device_id ad7944_spi_id[] = {
+	{ "ad7944", (kernel_ulong_t)&ad7944_chip_info },
+	{ "ad7985", (kernel_ulong_t)&ad7985_chip_info },
+	{ "ad7986", (kernel_ulong_t)&ad7986_chip_info },
+	{ }
+
+};
+MODULE_DEVICE_TABLE(spi, ad7944_spi_id);
+
+static struct spi_driver ad7944_driver = {
+	.driver = {
+		.name = "ad7944",
+		.of_match_table = ad7944_of_match,
+	},
+	.probe = ad7944_probe,
+	.id_table = ad7944_spi_id,
+};
+module_spi_driver(ad7944_driver);
+
+MODULE_AUTHOR("David Lechner <dlechner@baylibre.com>");
+MODULE_DESCRIPTION("Analog Devices AD7944 PulSAR ADC family driver");
+MODULE_LICENSE("GPL");

-- 
2.43.2


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

* Re: [PATCH v2 2/2] iio: adc: ad7944: add driver for AD7944/AD7985/AD7986
  2024-02-16 19:46 ` [PATCH v2 2/2] iio: adc: ad7944: add driver for AD7944/AD7985/AD7986 David Lechner
@ 2024-02-19 19:13   ` David Lechner
  2024-02-19 19:46     ` Jonathan Cameron
  0 siblings, 1 reply; 10+ messages in thread
From: David Lechner @ 2024-02-19 19:13 UTC (permalink / raw)
  To: linux-iio
  Cc: Michael Hennerich, Jonathan Cameron, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

On Fri, Feb 16, 2024 at 1:47 PM David Lechner <dlechner@baylibre.com> wrote:

...

> +
> +#define AD7944_DEFINE_CHIP_INFO(_name, _t, _bits, _sign)               \
> +static const struct ad7944_chip_info _name##_chip_info = {             \
> +       .name = #_name,                                                 \
> +       .t = &_t##_timing_spec,                                         \
> +       .channels = {                                                   \
> +               {                                                       \
> +                       .type = IIO_VOLTAGE,                            \
> +                       .indexed = 1,                                   \
> +                       .differential = 1,                              \
> +                       .channel = 0,                                   \
> +                       .channel2 = 1,                                  \
> +                       .scan_index = 0,                                \
> +                       .scan_type.sign = _sign,                        \
> +                       .scan_type.realbits = _bits,                    \
> +                       .scan_type.storagebits = _bits > 16 ? 32 : 16,  \
> +                       .scan_type.endianness = IIO_CPU,                \
> +                       .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)    \
> +                                       | BIT(IIO_CHAN_INFO_SCALE),     \
> +               },                                                      \
> +               IIO_CHAN_SOFT_TIMESTAMP(1),                             \
> +       },                                                              \
> +}
> +
> +AD7944_DEFINE_CHIP_INFO(ad7944, ad7944, 14, 'u');
> +AD7944_DEFINE_CHIP_INFO(ad7985, ad7944, 16, 'u');
> +AD7944_DEFINE_CHIP_INFO(ad7986, ad7986, 18, 's');

Now that I have been enlightened [1] about pseudo-differntial inputs,
I'm thinking that AD7944 and AD7985 should not have the .differential
= 1 flag set since they are pseudo-differential inputs with a ground
sense on the negative input (and no extra supply needed since it is
always ground). Does that sound right?

AD7986 is true differential though, so should be correct already.

[1]: https://lore.kernel.org/linux-iio/CAMknhBF5mAsN1c-194Qwa5oKmqKzef2khXnqA1cSdKpWHKWp0w@mail.gmail.com/

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

* Re: [PATCH v2 2/2] iio: adc: ad7944: add driver for AD7944/AD7985/AD7986
  2024-02-19 19:13   ` David Lechner
@ 2024-02-19 19:46     ` Jonathan Cameron
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2024-02-19 19:46 UTC (permalink / raw)
  To: David Lechner
  Cc: linux-iio, Michael Hennerich, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

On Mon, 19 Feb 2024 13:13:35 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On Fri, Feb 16, 2024 at 1:47 PM David Lechner <dlechner@baylibre.com> wrote:
> 
> ...
> 
> > +
> > +#define AD7944_DEFINE_CHIP_INFO(_name, _t, _bits, _sign)               \
> > +static const struct ad7944_chip_info _name##_chip_info = {             \
> > +       .name = #_name,                                                 \
> > +       .t = &_t##_timing_spec,                                         \
> > +       .channels = {                                                   \
> > +               {                                                       \
> > +                       .type = IIO_VOLTAGE,                            \
> > +                       .indexed = 1,                                   \
> > +                       .differential = 1,                              \
> > +                       .channel = 0,                                   \
> > +                       .channel2 = 1,                                  \
> > +                       .scan_index = 0,                                \
> > +                       .scan_type.sign = _sign,                        \
> > +                       .scan_type.realbits = _bits,                    \
> > +                       .scan_type.storagebits = _bits > 16 ? 32 : 16,  \
> > +                       .scan_type.endianness = IIO_CPU,                \
> > +                       .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)    \
> > +                                       | BIT(IIO_CHAN_INFO_SCALE),     \
> > +               },                                                      \
> > +               IIO_CHAN_SOFT_TIMESTAMP(1),                             \
> > +       },                                                              \
> > +}
> > +
> > +AD7944_DEFINE_CHIP_INFO(ad7944, ad7944, 14, 'u');
> > +AD7944_DEFINE_CHIP_INFO(ad7985, ad7944, 16, 'u');
> > +AD7944_DEFINE_CHIP_INFO(ad7986, ad7986, 18, 's');  
> 
> Now that I have been enlightened [1] about pseudo-differntial inputs,
> I'm thinking that AD7944 and AD7985 should not have the .differential
> = 1 flag set since they are pseudo-differential inputs with a ground
> sense on the negative input (and no extra supply needed since it is
> always ground). Does that sound right?

yes

> 
> AD7986 is true differential though, so should be correct already.
> 
> [1]: https://lore.kernel.org/linux-iio/CAMknhBF5mAsN1c-194Qwa5oKmqKzef2khXnqA1cSdKpWHKWp0w@mail.gmail.com/


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

* Re: [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs
  2024-02-16 19:46 ` [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs David Lechner
@ 2024-02-20 19:47   ` Conor Dooley
  2024-02-21 15:22   ` Rob Herring
  1 sibling, 0 replies; 10+ messages in thread
From: Conor Dooley @ 2024-02-20 19:47 UTC (permalink / raw)
  To: David Lechner
  Cc: linux-iio, Michael Hennerich, Jonathan Cameron, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

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

On Fri, Feb 16, 2024 at 01:46:18PM -0600, David Lechner wrote:
> This adds a new binding for the Analog Devices, Inc. AD7944, AD7985, and
> AD7986 ADCs.

I think this binding is overall pretty well written, especially
considering the interproperty dependencies.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.


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

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

* Re: [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs
  2024-02-16 19:46 ` [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs David Lechner
  2024-02-20 19:47   ` Conor Dooley
@ 2024-02-21 15:22   ` Rob Herring
  2024-02-21 15:44     ` David Lechner
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2024-02-21 15:22 UTC (permalink / raw)
  To: David Lechner
  Cc: linux-iio, Michael Hennerich, Jonathan Cameron,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

On Fri, Feb 16, 2024 at 01:46:18PM -0600, David Lechner wrote:
> This adds a new binding for the Analog Devices, Inc. AD7944, AD7985, and
> AD7986 ADCs.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad7944.yaml    | 204 +++++++++++++++++++++
>  MAINTAINERS                                        |   8 +
>  2 files changed, 212 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml
> new file mode 100644
> index 000000000000..61ee81326660
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml
> @@ -0,0 +1,204 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7944.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices PulSAR LFCSP Analog to Digital Converters
> +
> +maintainers:
> +  - Michael Hennerich <Michael.Hennerich@analog.com>
> +  - Nuno Sá <nuno.sa@analog.com>
> +
> +description: |
> +  A family of pin-compatible single channel differential analog to digital
> +  converters with SPI support in a LFCSP package.
> +
> +  * https://www.analog.com/en/products/ad7944.html
> +  * https://www.analog.com/en/products/ad7985.html
> +  * https://www.analog.com/en/products/ad7986.html
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7944
> +      - adi,ad7985
> +      - adi,ad7986
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 111111111
> +
> +  spi-cpol: true
> +  spi-cpha: true
> +
> +  adi,spi-mode:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [ single, multi, chain ]
> +    default: multi
> +    description: |
> +      * single: The datasheet calls this "3-wire mode". It is often used when
> +        the ADC is the only device on the bus. In this mode, SDI is tied to VIO,
> +        and the CNV line can be connected to the CS line of the SPI controller
> +        or to a GPIO, in which case the CS line of the controller is unused.

We have a standard property for this.

> +      * multi: The datasheet calls this "4-wire mode". This is the convential
> +        SPI mode used when there are multiple devices on the same bus. In this
> +        mode, the CNV line is used to initiate the conversion and the SDI line
> +        is connected to CS on the SPI controller.

That's "normal" mode.

> +      * chain: The datasheet calls this "chain mode". This mode is used to save
> +        on wiring when multiple ADCs are used. In this mode, the SDI line of
> +        one chip is tied to the SDO of the next chip in the chain and the SDI of
> +        the last chip in the chain is tied to GND. Only the first chip in the
> +        chain is connected to the SPI bus. The CNV line of all chips are tied
> +        together. The CS line of the SPI controller is unused.

Don't you need to know how many chips are chained? In any case, you just 
need a property for chain mode. There's some existing properties for 
chained devices I think. Standard logic shift register based GPIO IIRC.

CNV are tied together, but must be driven by something? I suppose 
cnv-gpios? But wouldn't that be the same as the SPI controller GPIO CS? 
Does a SPI controller CS line connected to CNV not work in this case?

Rob

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

* Re: [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs
  2024-02-21 15:22   ` Rob Herring
@ 2024-02-21 15:44     ` David Lechner
  2024-02-22 15:34       ` Rob Herring
  0 siblings, 1 reply; 10+ messages in thread
From: David Lechner @ 2024-02-21 15:44 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-iio, Michael Hennerich, Jonathan Cameron,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

On Wed, Feb 21, 2024 at 9:22 AM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Feb 16, 2024 at 01:46:18PM -0600, David Lechner wrote:

...

> > +  adi,spi-mode:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum: [ single, multi, chain ]
> > +    default: multi
> > +    description: |
> > +      * single: The datasheet calls this "3-wire mode". It is often used when
> > +        the ADC is the only device on the bus. In this mode, SDI is tied to VIO,
> > +        and the CNV line can be connected to the CS line of the SPI controller
> > +        or to a GPIO, in which case the CS line of the controller is unused.
>
> We have a standard property for this.

As discussed in v1 [1], the datasheet's definition of "3-wire mode" is
_not_ the same as the standard spi-3wire property. I can add that to
the description here to clarify (I hoped changing the enum name was
enough, but perhaps not). Or is there a different property you are
referring to?

[1]: https://lore.kernel.org/all/20240216140826.58b3318d@jic23-huawei/

>
> > +      * multi: The datasheet calls this "4-wire mode". This is the convential
> > +        SPI mode used when there are multiple devices on the same bus. In this
> > +        mode, the CNV line is used to initiate the conversion and the SDI line
> > +        is connected to CS on the SPI controller.
>
> That's "normal" mode.

That was my first choice, but the datasheet uses the term "normal
mode" to mean not TURBO mode which is something else unrelated to the
SPI mode.


>
> > +      * chain: The datasheet calls this "chain mode". This mode is used to save
> > +        on wiring when multiple ADCs are used. In this mode, the SDI line of
> > +        one chip is tied to the SDO of the next chip in the chain and the SDI of
> > +        the last chip in the chain is tied to GND. Only the first chip in the
> > +        chain is connected to the SPI bus. The CNV line of all chips are tied
> > +        together. The CS line of the SPI controller is unused.
>
> Don't you need to know how many chips are chained? In any case, you just
> need a property for chain mode. There's some existing properties for
> chained devices I think. Standard logic shift register based GPIO IIRC.

Thanks, I see #daisy-chained-devices now. I missed that before.

>
> CNV are tied together, but must be driven by something? I suppose
> cnv-gpios?

Yes.

> But wouldn't that be the same as the SPI controller GPIO CS?
> Does a SPI controller CS line connected to CNV not work in this case?

Maybe technically possible if CS is inverted on the bus since the line
has to be high to trigger the conversion and during the xfer.

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

* Re: [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs
  2024-02-21 15:44     ` David Lechner
@ 2024-02-22 15:34       ` Rob Herring
  2024-02-27 19:15         ` David Lechner
  0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2024-02-22 15:34 UTC (permalink / raw)
  To: David Lechner
  Cc: linux-iio, Michael Hennerich, Jonathan Cameron,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

On Wed, Feb 21, 2024 at 8:44 AM David Lechner <dlechner@baylibre.com> wrote:
>
> On Wed, Feb 21, 2024 at 9:22 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Feb 16, 2024 at 01:46:18PM -0600, David Lechner wrote:
>
> ...
>
> > > +  adi,spi-mode:
> > > +    $ref: /schemas/types.yaml#/definitions/string
> > > +    enum: [ single, multi, chain ]
> > > +    default: multi
> > > +    description: |
> > > +      * single: The datasheet calls this "3-wire mode". It is often used when
> > > +        the ADC is the only device on the bus. In this mode, SDI is tied to VIO,
> > > +        and the CNV line can be connected to the CS line of the SPI controller
> > > +        or to a GPIO, in which case the CS line of the controller is unused.
> >
> > We have a standard property for this.
>
> As discussed in v1 [1], the datasheet's definition of "3-wire mode" is
> _not_ the same as the standard spi-3wire property. I can add that to
> the description here to clarify (I hoped changing the enum name was
> enough, but perhaps not). Or is there a different property you are
> referring to?
>
> [1]: https://lore.kernel.org/all/20240216140826.58b3318d@jic23-huawei/
>
> >
> > > +      * multi: The datasheet calls this "4-wire mode". This is the convential

Also, typo.

> > > +        SPI mode used when there are multiple devices on the same bus. In this
> > > +        mode, the CNV line is used to initiate the conversion and the SDI line
> > > +        is connected to CS on the SPI controller.
> >
> > That's "normal" mode.
>
> That was my first choice, but the datasheet uses the term "normal
> mode" to mean not TURBO mode which is something else unrelated to the
> SPI mode.

What I mean is this should be conveyed by the absence of any property.
You don't need a property for "normal SPI mode".

> >
> > > +      * chain: The datasheet calls this "chain mode". This mode is used to save
> > > +        on wiring when multiple ADCs are used. In this mode, the SDI line of
> > > +        one chip is tied to the SDO of the next chip in the chain and the SDI of
> > > +        the last chip in the chain is tied to GND. Only the first chip in the
> > > +        chain is connected to the SPI bus. The CNV line of all chips are tied
> > > +        together. The CS line of the SPI controller is unused.
> >
> > Don't you need to know how many chips are chained? In any case, you just
> > need a property for chain mode. There's some existing properties for
> > chained devices I think. Standard logic shift register based GPIO IIRC.
>
> Thanks, I see #daisy-chained-devices now. I missed that before.
>
> >
> > CNV are tied together, but must be driven by something? I suppose
> > cnv-gpios?
>
> Yes.
>
> > But wouldn't that be the same as the SPI controller GPIO CS?
> > Does a SPI controller CS line connected to CNV not work in this case?
>
> Maybe technically possible if CS is inverted on the bus since the line
> has to be high to trigger the conversion and during the xfer.

That's supported by the binding. Seems like it would simplify the
driver if you went that route and better support other devices on the
SPI bus. Also, we require 'reg', so I don't know what you'd put in it
in the no CS case. Though, we probably already have that case with CS
tied active. Shrug.

Rob

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

* Re: [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs
  2024-02-22 15:34       ` Rob Herring
@ 2024-02-27 19:15         ` David Lechner
  0 siblings, 0 replies; 10+ messages in thread
From: David Lechner @ 2024-02-27 19:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-iio, Michael Hennerich, Jonathan Cameron,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel

On Thu, Feb 22, 2024 at 9:34 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Feb 21, 2024 at 8:44 AM David Lechner <dlechner@baylibre.com> wrote:
> >
> > On Wed, Feb 21, 2024 at 9:22 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Fri, Feb 16, 2024 at 01:46:18PM -0600, David Lechner wrote:
> >
> > ...
> >
> > > > +  adi,spi-mode:
> > > > +    $ref: /schemas/types.yaml#/definitions/string
> > > > +    enum: [ single, multi, chain ]
> > > > +    default: multi
> > > > +    description: |
> > > > +      * single: The datasheet calls this "3-wire mode". It is often used when
> > > > +        the ADC is the only device on the bus. In this mode, SDI is tied to VIO,
> > > > +        and the CNV line can be connected to the CS line of the SPI controller
> > > > +        or to a GPIO, in which case the CS line of the controller is unused.
> > >
> > > We have a standard property for this.
> >
> > As discussed in v1 [1], the datasheet's definition of "3-wire mode" is
> > _not_ the same as the standard spi-3wire property. I can add that to
> > the description here to clarify (I hoped changing the enum name was
> > enough, but perhaps not). Or is there a different property you are
> > referring to?
> >
> > [1]: https://lore.kernel.org/all/20240216140826.58b3318d@jic23-huawei/
> >
> > >
> > > > +      * multi: The datasheet calls this "4-wire mode". This is the convential
>
> Also, typo.
>
> > > > +        SPI mode used when there are multiple devices on the same bus. In this
> > > > +        mode, the CNV line is used to initiate the conversion and the SDI line
> > > > +        is connected to CS on the SPI controller.
> > >
> > > That's "normal" mode.
> >
> > That was my first choice, but the datasheet uses the term "normal
> > mode" to mean not TURBO mode which is something else unrelated to the
> > SPI mode.
>
> What I mean is this should be conveyed by the absence of any property.
> You don't need a property for "normal SPI mode".

The binding already has `default: multi` to cover this case. But I
suppose we can just leave out the option altogether if you prefer.

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

end of thread, other threads:[~2024-02-27 19:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-16 19:46 [PATCH v2 0/2] iio: adc: ad7944: new driver David Lechner
2024-02-16 19:46 ` [PATCH v2 1/2] dt-bindings: iio: adc: add ad7944 ADCs David Lechner
2024-02-20 19:47   ` Conor Dooley
2024-02-21 15:22   ` Rob Herring
2024-02-21 15:44     ` David Lechner
2024-02-22 15:34       ` Rob Herring
2024-02-27 19:15         ` David Lechner
2024-02-16 19:46 ` [PATCH v2 2/2] iio: adc: ad7944: add driver for AD7944/AD7985/AD7986 David Lechner
2024-02-19 19:13   ` David Lechner
2024-02-19 19:46     ` Jonathan Cameron

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).