[PATCH v3 0/3] iio: adc: add new ad7380 driver

[PATCH v3 0/3] iio: adc: add new ad7380 driver

[David Lechner]

This series is adding a new driver for the Analog Devices Inc. AD7380,
AD7381, AD7383, and AD7384 ADCs. These chips are part of a family of
simultaneous sampling SAR ADCs.

One quirk of these chips is that since they are simultaneous sampling,
they have multiple SPI data output lines that allow transferring two
data words (one for each input channel) at the same time. So a new
generic devicetree binding is added to describe this sort of SPI bus
configuration.

To keep things simple, the initial driver implementation only supports
the 2-channel differential chips listed above. There are also 4-channel
differential chips and 4-channel single-ended chips in the family that
can be added later.

Furthermore, the driver is just implementing basic support for capturing
data. Additional features like interrupts, CRC, etc. can be added later.

Also, FYI, this driver will also be used as the base for an upcoming
series adding AXI SPI Engine offload support for these chips along with
[1].

This work is being done by BayLibre and on behalf of Analog Devices Inc.
hence the maintainers are @analog.com.

[1]: https://lore.kernel.org/linux-spi/20231204-axi-spi-engine-series-2-v1-0-063672323fce@baylibre.com/

---
Changes in v3:
- dt-bindings:
    - Picked up Conor's Reviewed-By on the adi,ad7380 bindings
- driver:
    - Removed extra indent in DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL macro
    - Removed scan mask that included timestamp channel
    - Removed parent device assignment
    - Picked up Nuno's Reviewed-by
- Link to v2: https://lore.kernel.org/r/20231213-ad7380-mainline-v2-0-cd32150d84a3@baylibre.com

Changes in v2:
- dt-bindings:
    - Added new patch with generic spi-rx-bus-channels property
    - Added maxItems to reg property
    - Replaced adi,sdo-mode property with spi-rx-bus-channels
    - Made spi-rx-bus-channels property optional with default value of 1
      (this made the if: check more complex)
    - Changed example to use gpio for interrupt
- driver:
    - Fixed CONFIG_AD7380 in Makefile
    - rx_buf = st->scan_data.raw instead of rx_buf = &st->scan_data
    - Moved iio_push_to_buffers_with_timestamp() outside of if statement
    - Removed extra blank lines
    - Renamed regulator disable function
    - Dropped checking of adi,sdo-mode property (regardless of the actual
      wiring, we can always use 1-wire mode)
    - Added available_scan_masks
    - Added check for missing driver match data
- Link to v1: https://lore.kernel.org/r/20231208-ad7380-mainline-v1-0-2b33fe2f44ae@baylibre.com

---
David Lechner (3):
      dt-bindings: spi: add spi-rx-bus-channels peripheral property
      dt-bindings: iio: adc: Add binding for AD7380 ADCs
      iio: adc: ad7380: new driver for AD7380 ADCs

 .../devicetree/bindings/iio/adc/adi,ad7380.yaml    | 107 +++++
 .../bindings/spi/spi-peripheral-props.yaml         |  12 +
 MAINTAINERS                                        |  10 +
 drivers/iio/adc/Kconfig                            |  16 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/ad7380.c                           | 462 +++++++++++++++++++++
 6 files changed, 608 insertions(+)
---
base-commit: 18f78b5e609b19b56237f0dae47068d44b8b0ecd
change-id: 20231208-ad7380-mainline-e6c4fa7dbedd

[PATCH v3 1/3] dt-bindings: spi: add spi-rx-bus-channels peripheral property

[David Lechner]

This adds a new spi-rx-bus-channels property to the generic spi
peripheral property bindings. This property is used to describe
devices that have parallel data output channels.

This property is different from spi-rx-bus-width in that the latter
means that we are reading multiple bits of a single word at one time
while the former means that we are reading single bits of multiple words
at the same time.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---

The rest of this series is ready to merge, so just looking for an ack from
Mark on this one.

 .../devicetree/bindings/spi/spi-peripheral-props.yaml        | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
index 15938f81fdce..1c8e71c18234 100644
--- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
@@ -67,6 +67,18 @@ properties:
     enum: [0, 1, 2, 4, 8]
     default: 1
 
+  spi-rx-bus-channels:
+    description:
+      The number of parallel channels for read transfers. The difference between
+      this and spi-rx-bus-width is that a value N for spi-rx-bus-channels means
+      the SPI bus is receiving one bit each of N different words at the same
+      time whereas a value M for spi-rx-bus-width means that the bus is
+      receiving M bits of a single word at the same time. It is also possible to
+      use both properties at the same time, meaning the bus is receiving M bits
+      of N different words at the same time.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 1
+
   spi-rx-delay-us:
     description:
       Delay, in microseconds, after a read transfer.

-- 
2.34.1

[PATCH v3 2/3] dt-bindings: iio: adc: Add binding for AD7380 ADCs

[David Lechner]

This adds a binding specification for the Analog Devices Inc. AD7380
family of ADCs.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 .../devicetree/bindings/iio/adc/adi,ad7380.yaml    | 107 +++++++++++++++++++++
 MAINTAINERS                                        |   9 ++
 2 files changed, 116 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
new file mode 100644
index 000000000000..43d58c52f7dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
@@ -0,0 +1,107 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7380.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices Simultaneous Sampling Analog to Digital Converters
+
+maintainers:
+  - Michael Hennerich <Michael.Hennerich@analog.com>
+  - Nuno Sá <nuno.sa@analog.com>
+
+description: |
+  * https://www.analog.com/en/products/ad7380.html
+  * https://www.analog.com/en/products/ad7381.html
+  * https://www.analog.com/en/products/ad7383.html
+  * https://www.analog.com/en/products/ad7384.html
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7380
+      - adi,ad7381
+      - adi,ad7383
+      - adi,ad7384
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 80000000
+  spi-cpol: true
+  spi-cpha: true
+
+  spi-rx-bus-channels:
+    description:
+      In 1-wire mode, the SDOA pin acts as the sole data line and the SDOB/ALERT
+      pin acts as the ALERT interrupt signal. In 2-wire mode, data for input A
+      is read from SDOA and data for input B is read from SDOB/ALERT (and the
+      ALERT interrupt signal is not available).
+    enum: [1, 2]
+
+  vcc-supply:
+    description: A 3V to 3.6V supply that powers the chip.
+
+  vlogic-supply:
+    description:
+      A 1.65V to 3.6V supply for the logic pins.
+
+  refio-supply:
+    description:
+      A 2.5V to 3.3V supply for the external reference voltage. When omitted,
+      the internal 2.5V reference is used.
+
+  interrupts:
+    description:
+      When the device is using 1-wire mode, this property is used to optionally
+      specify the ALERT interrupt.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - vcc-supply
+  - vlogic-supply
+
+allOf:
+  - if:
+      required:
+        - spi-rx-bus-channels
+    then:
+      if:
+        properties:
+          spi-rx-bus-channels:
+            const: 2
+      then:
+        properties:
+          interrupts: false
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad7380";
+            reg = <0>;
+
+            spi-cpol;
+            spi-cpha;
+            spi-max-frequency = <80000000>;
+
+            interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-parent = <&gpio0>;
+
+            vcc-supply = <&supply_3_3V>;
+            vlogic-supply = <&supply_3_3V>;
+            refio-supply = <&supply_2_5V>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index fe1f6f97f96a..e2a998be5879 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -430,6 +430,15 @@ W:	http://wiki.analog.com/AD7142
 W:	https://ez.analog.com/linux-software-drivers
 F:	drivers/input/misc/ad714x.c
 
+AD738X ADC DRIVER (AD7380/1/2/4)
+M:	Michael Hennerich <michael.hennerich@analog.com>
+M:	Nuno Sá <nuno.sa@analog.com>
+R:	David Lechner <dlechner@baylibre.com>
+S:	Supported
+W:	https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
+
 AD7877 TOUCHSCREEN DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
 S:	Supported

-- 
2.34.1

[PATCH v3 3/3] iio: adc: ad7380: new driver for AD7380 ADCs

[David Lechner]

This adds a new driver for the AD7380 family ADCs.

The driver currently implements basic support for the AD7380, AD7381,
AD7383, and AD7384 2-channel differential ADCs. Support for additional
single-ended and 4-channel chips that use the same register map as well
as additional features of the chip will be added in future patches.

Co-developed-by: Stefan Popa <stefan.popa@analog.com>
Signed-off-by: Stefan Popa <stefan.popa@analog.com>
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  16 ++
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad7380.c | 462 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 480 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index e2a998be5879..5a54620a31b8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -438,6 +438,7 @@ S:	Supported
 W:	https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
+F:	drivers/iio/adc/ad7380.c
 
 AD7877 TOUCHSCREEN DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 35f9867da12c..cbfd626712e3 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -122,6 +122,22 @@ config AD7298
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7298.
 
+config AD7380
+	tristate "Analog Devices AD7380 ADC driver"
+	depends on SPI_MASTER
+	select IIO_BUFFER
+	select IIO_TRIGGER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  AD7380 is a family of simultaneous sampling ADCs that share the same
+	  SPI register map and have similar pinouts.
+
+	  Say yes here to build support for Analog Devices AD7380 ADC and
+	  similar chips.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad7380.
+
 config AD7476
 	tristate "Analog Devices AD7476 1-channel ADCs driver and other similar devices from AD and TI"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index bee11d442af4..9c921c497655 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_AD7291) += ad7291.o
 obj-$(CONFIG_AD7292) += ad7292.o
 obj-$(CONFIG_AD7298) += ad7298.o
 obj-$(CONFIG_AD7923) += ad7923.o
+obj-$(CONFIG_AD7380) += ad7380.o
 obj-$(CONFIG_AD7476) += ad7476.o
 obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
 obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
new file mode 100644
index 000000000000..80712aaa9548
--- /dev/null
+++ b/drivers/iio/adc/ad7380.c
@@ -0,0 +1,462 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD738x Simultaneous Sampling SAR ADCs
+ *
+ * Copyright 2017 Analog Devices Inc.
+ * Copyright 2023 BayLibre, SAS
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+/* 2.5V internal reference voltage */
+#define AD7380_INTERNAL_REF_MV		2500
+
+/* reading and writing registers is more reliable at lower than max speed */
+#define AD7380_REG_WR_SPEED_HZ		10000000
+
+#define AD7380_REG_WR			BIT(15)
+#define AD7380_REG_REGADDR		GENMASK(14, 12)
+#define AD7380_REG_DATA			GENMASK(11, 0)
+
+#define AD7380_REG_ADDR_NOP		0x0
+#define AD7380_REG_ADDR_CONFIG1		0x1
+#define AD7380_REG_ADDR_CONFIG2		0x2
+#define AD7380_REG_ADDR_ALERT		0x3
+#define AD7380_REG_ADDR_ALERT_LOW_TH	0x4
+#define AD7380_REG_ADDR_ALERT_HIGH_TH	0x5
+
+#define AD7380_CONFIG1_OS_MODE		BIT(9)
+#define AD7380_CONFIG1_OSR		GENMASK(8, 6)
+#define AD7380_CONFIG1_CRC_W		BIT(5)
+#define AD7380_CONFIG1_CRC_R		BIT(4)
+#define AD7380_CONFIG1_ALERTEN		BIT(3)
+#define AD7380_CONFIG1_RES		BIT(2)
+#define AD7380_CONFIG1_REFSEL		BIT(1)
+#define AD7380_CONFIG1_PMODE		BIT(0)
+
+#define AD7380_CONFIG2_SDO2		GENMASK(9, 8)
+#define AD7380_CONFIG2_SDO		BIT(8)
+#define AD7380_CONFIG2_RESET		GENMASK(7, 0)
+
+#define AD7380_CONFIG2_RESET_SOFT	0x3C
+#define AD7380_CONFIG2_RESET_HARD	0xFF
+
+#define AD7380_ALERT_LOW_TH		GENMASK(11, 0)
+#define AD7380_ALERT_HIGH_TH		GENMASK(11, 0)
+
+struct ad7380_chip_info {
+	const char *name;
+	const struct iio_chan_spec *channels;
+	unsigned int num_channels;
+};
+
+#define AD7380_DIFFERENTIAL_CHANNEL(index, bits) {		\
+	.type = IIO_VOLTAGE,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+	.indexed = 1,						\
+	.differential = 1,					\
+	.channel = 2 * (index),					\
+	.channel2 = 2 * (index) + 1,				\
+	.scan_index = (index),					\
+	.scan_type = {						\
+		.sign = 's',					\
+		.realbits = (bits),				\
+		.storagebits = 16,				\
+		.endianness = IIO_CPU,				\
+	},							\
+}
+
+#define DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(name, bits)	\
+static const struct iio_chan_spec name[] = {			\
+	AD7380_DIFFERENTIAL_CHANNEL(0, bits),			\
+	AD7380_DIFFERENTIAL_CHANNEL(1, bits),			\
+	IIO_CHAN_SOFT_TIMESTAMP(2),				\
+}
+
+/* fully differential */
+DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7380_channels, 16);
+DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7381_channels, 14);
+/* pseudo differential */
+DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7383_channels, 16);
+DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7384_channels, 14);
+
+/* Since this is simultaneous sampling, we don't allow individual channels. */
+static const unsigned long ad7380_2_channel_scan_masks[] = {
+	GENMASK(1, 0),
+	0
+};
+
+static const struct ad7380_chip_info ad7380_chip_info = {
+	.name = "ad7380",
+	.channels = ad7380_channels,
+	.num_channels = ARRAY_SIZE(ad7380_channels),
+};
+
+static const struct ad7380_chip_info ad7381_chip_info = {
+	.name = "ad7381",
+	.channels = ad7381_channels,
+	.num_channels = ARRAY_SIZE(ad7381_channels),
+};
+
+static const struct ad7380_chip_info ad7383_chip_info = {
+	.name = "ad7383",
+	.channels = ad7383_channels,
+	.num_channels = ARRAY_SIZE(ad7383_channels),
+};
+
+static const struct ad7380_chip_info ad7384_chip_info = {
+	.name = "ad7384",
+	.channels = ad7384_channels,
+	.num_channels = ARRAY_SIZE(ad7384_channels),
+};
+
+struct ad7380_state {
+	const struct ad7380_chip_info *chip_info;
+	struct spi_device *spi;
+	struct regulator *vref;
+	struct regmap *regmap;
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 * Make the buffer large enough for 2 16-bit samples and one 64-bit
+	 * aligned 64 bit timestamp.
+	 */
+	struct {
+		u16 raw[2];
+		s64 ts __aligned(8);
+	} scan_data __aligned(IIO_DMA_MINALIGN);
+	u16 tx[2];
+	u16 rx[2];
+};
+
+static int ad7380_regmap_reg_write(void *context, unsigned int reg,
+				   unsigned int val)
+{
+	struct ad7380_state *st = context;
+	struct spi_transfer xfer = {
+		.speed_hz = AD7380_REG_WR_SPEED_HZ,
+		.bits_per_word = 16,
+		.len = 2,
+		.tx_buf = &st->tx[0],
+	};
+
+	st->tx[0] = FIELD_PREP(AD7380_REG_WR, 1) |
+		    FIELD_PREP(AD7380_REG_REGADDR, reg) |
+		    FIELD_PREP(AD7380_REG_DATA, val);
+
+	return spi_sync_transfer(st->spi, &xfer, 1);
+}
+
+static int ad7380_regmap_reg_read(void *context, unsigned int reg,
+				  unsigned int *val)
+{
+	struct ad7380_state *st = context;
+	struct spi_transfer xfers[] = {
+		{
+			.speed_hz = AD7380_REG_WR_SPEED_HZ,
+			.bits_per_word = 16,
+			.len = 2,
+			.tx_buf = &st->tx[0],
+			.cs_change = 1,
+			.cs_change_delay = {
+				.value = 10, /* t[CSH] */
+				.unit = SPI_DELAY_UNIT_NSECS,
+			},
+		}, {
+			.speed_hz = AD7380_REG_WR_SPEED_HZ,
+			.bits_per_word = 16,
+			.len = 2,
+			.rx_buf = &st->rx[0],
+		},
+	};
+	int ret;
+
+	st->tx[0] = FIELD_PREP(AD7380_REG_WR, 0) |
+		    FIELD_PREP(AD7380_REG_REGADDR, reg) |
+		    FIELD_PREP(AD7380_REG_DATA, 0);
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+	if (ret < 0)
+		return ret;
+
+	*val = FIELD_GET(AD7380_REG_DATA, st->rx[0]);
+
+	return 0;
+}
+
+const struct regmap_config ad7380_regmap_config = {
+	.reg_bits = 3,
+	.val_bits = 12,
+	.reg_read = ad7380_regmap_reg_read,
+	.reg_write = ad7380_regmap_reg_write,
+	.max_register = AD7380_REG_ADDR_ALERT_HIGH_TH,
+	.can_sleep = true,
+};
+
+static int ad7380_debugfs_reg_access(struct iio_dev *indio_dev, u32 reg,
+				     u32 writeval, u32 *readval)
+{
+	struct ad7380_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret)
+		return ret;
+
+	if (readval)
+		ret = regmap_read(st->regmap, reg, readval);
+	else
+		ret = regmap_write(st->regmap, reg, writeval);
+
+	iio_device_release_direct_mode(indio_dev);
+
+	return ret;
+}
+
+static irqreturn_t ad7380_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ad7380_state *st = iio_priv(indio_dev);
+	struct spi_transfer xfer = {
+		.bits_per_word = st->chip_info->channels[0].scan_type.realbits,
+		.len = 4,
+		.rx_buf = st->scan_data.raw,
+	};
+	int ret;
+
+	ret = spi_sync_transfer(st->spi, &xfer, 1);
+	if (ret)
+		goto out;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &st->scan_data,
+					   pf->timestamp);
+
+out:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int ad7380_read_direct(struct ad7380_state *st,
+			      struct iio_chan_spec const *chan, int *val)
+{
+	struct spi_transfer xfers[] = {
+		/* toggle CS (no data xfer) to trigger a conversion */
+		{
+			.speed_hz = AD7380_REG_WR_SPEED_HZ,
+			.bits_per_word = chan->scan_type.realbits,
+			.delay = {
+				.value = 190, /* t[CONVERT] */
+				.unit = SPI_DELAY_UNIT_NSECS,
+			},
+			.cs_change = 1,
+			.cs_change_delay = {
+				.value = 10, /* t[CSH] */
+				.unit = SPI_DELAY_UNIT_NSECS,
+			},
+		},
+		/* then read both channels */
+		{
+			.speed_hz = AD7380_REG_WR_SPEED_HZ,
+			.bits_per_word = chan->scan_type.realbits,
+			.rx_buf = &st->rx[0],
+			.len = 4,
+		},
+	};
+	int ret;
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+	if (ret < 0)
+		return ret;
+
+	*val = sign_extend32(st->rx[chan->scan_index],
+			     chan->scan_type.realbits - 1);
+
+	return IIO_VAL_INT;
+}
+
+static int ad7380_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long info)
+{
+	struct ad7380_state *st = 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 = ad7380_read_direct(st, chan, val);
+		iio_device_release_direct_mode(indio_dev);
+
+		return ret;
+	case IIO_CHAN_INFO_SCALE:
+		if (st->vref) {
+			ret = regulator_get_voltage(st->vref);
+			if (ret < 0)
+				return ret;
+
+			*val = ret / 1000;
+		} else {
+			*val = AD7380_INTERNAL_REF_MV;
+		}
+
+		*val2 = chan->scan_type.realbits;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info ad7380_info = {
+	.read_raw = &ad7380_read_raw,
+	.debugfs_reg_access = &ad7380_debugfs_reg_access,
+};
+
+static int ad7380_init(struct ad7380_state *st)
+{
+	int ret;
+
+	/* perform hard reset */
+	ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2,
+				 AD7380_CONFIG2_RESET,
+				 FIELD_PREP(AD7380_CONFIG2_RESET,
+					    AD7380_CONFIG2_RESET_HARD));
+	if (ret < 0)
+		return ret;
+
+	/* select internal or external reference voltage */
+	ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1,
+				 AD7380_CONFIG1_REFSEL,
+				 FIELD_PREP(AD7380_CONFIG1_REFSEL, !!st->vref));
+	if (ret < 0)
+		return ret;
+
+	/* SPI 1-wire mode */
+	return regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2,
+				  AD7380_CONFIG2_SDO,
+				  FIELD_PREP(AD7380_CONFIG2_SDO, 1));
+}
+
+static void ad7380_regulator_disable(void *p)
+{
+	regulator_disable(p);
+}
+
+static int ad7380_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct ad7380_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->chip_info = spi_get_device_match_data(spi);
+	if (!st->chip_info)
+		return dev_err_probe(&spi->dev, -EINVAL, "missing match data\n");
+
+	st->vref = devm_regulator_get_optional(&spi->dev, "refio");
+	if (IS_ERR(st->vref)) {
+		/*
+		 * If there is no REFIO supply, then it means that we are using
+		 * the internal 2.5V reference.
+		 */
+		if (PTR_ERR(st->vref) == -ENODEV)
+			st->vref = NULL;
+		else
+			return dev_err_probe(&spi->dev, PTR_ERR(st->vref),
+					     "Failed to get refio regulator\n");
+	}
+
+	if (st->vref) {
+		ret = regulator_enable(st->vref);
+		if (ret)
+			return ret;
+
+		ret = devm_add_action_or_reset(&spi->dev, ad7380_regulator_disable,
+					       st->vref);
+		if (ret)
+			return ret;
+	}
+
+	st->regmap = devm_regmap_init(&spi->dev, NULL, st, &ad7380_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->regmap),
+				     "failed to allocate register map\n");
+
+	indio_dev->channels = st->chip_info->channels;
+	indio_dev->num_channels = st->chip_info->num_channels;
+	indio_dev->name = st->chip_info->name;
+	indio_dev->info = &ad7380_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->available_scan_masks = ad7380_2_channel_scan_masks;
+
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+					      iio_pollfunc_store_time,
+					      ad7380_trigger_handler, NULL);
+	if (ret)
+		return ret;
+
+	ret = ad7380_init(st);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id ad7380_of_match_table[] = {
+	{ .compatible = "adi,ad7380", .data = &ad7380_chip_info },
+	{ .compatible = "adi,ad7381", .data = &ad7381_chip_info },
+	{ .compatible = "adi,ad7383", .data = &ad7383_chip_info },
+	{ .compatible = "adi,ad7384", .data = &ad7384_chip_info },
+	{ }
+};
+
+static const struct spi_device_id ad7380_id_table[] = {
+	{ "ad7380", (kernel_ulong_t)&ad7380_chip_info },
+	{ "ad7381", (kernel_ulong_t)&ad7381_chip_info },
+	{ "ad7383", (kernel_ulong_t)&ad7383_chip_info },
+	{ "ad7384", (kernel_ulong_t)&ad7384_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad7380_id_table);
+
+static struct spi_driver ad7380_driver = {
+	.driver = {
+		.name = "ad7380",
+		.of_match_table = ad7380_of_match_table,
+	},
+	.probe = ad7380_probe,
+	.id_table = ad7380_id_table,
+};
+module_spi_driver(ad7380_driver);
+
+MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD738x ADC driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v3 3/3] iio: adc: ad7380: new driver for AD7380 ADCs

[Christophe JAILLET]

Le 15/12/2023 à 11:32, David Lechner a écrit :
> This adds a new driver for the AD7380 family ADCs.
> 
> The driver currently implements basic support for the AD7380, AD7381,
> AD7383, and AD7384 2-channel differential ADCs. Support for additional
> single-ended and 4-channel chips that use the same register map as well
> as additional features of the chip will be added in future patches.
> 
> Co-developed-by: Stefan Popa <stefan.popa-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Stefan Popa <stefan.popa-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Nuno Sa <nuno.sa-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: David Lechner <dlechner-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---

...

> +static void ad7380_regulator_disable(void *p)
> +{
> +	regulator_disable(p);
> +}
> +
> +static int ad7380_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ad7380_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	st->chip_info = spi_get_device_match_data(spi);
> +	if (!st->chip_info)
> +		return dev_err_probe(&spi->dev, -EINVAL, "missing match data\n");
> +
> +	st->vref = devm_regulator_get_optional(&spi->dev, "refio");

Hi,

devm_regulator_get_enable_optional()?
to save some LoC below and ad7380_regulator_disable()

CJ

> +	if (IS_ERR(st->vref)) {
> +		/*
> +		 * If there is no REFIO supply, then it means that we are using
> +		 * the internal 2.5V reference.
> +		 */
> +		if (PTR_ERR(st->vref) == -ENODEV)
> +			st->vref = NULL;
> +		else
> +			return dev_err_probe(&spi->dev, PTR_ERR(st->vref),
> +					     "Failed to get refio regulator\n");
> +	}
> +
> +	if (st->vref) {
> +		ret = regulator_enable(st->vref);
> +		if (ret)
> +			return ret;
> +
> +		ret = devm_add_action_or_reset(&spi->dev, ad7380_regulator_disable,
> +					       st->vref);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	st->regmap = devm_regmap_init(&spi->dev, NULL, st, &ad7380_regmap_config);
> +	if (IS_ERR(st->regmap))
> +		return dev_err_probe(&spi->dev, PTR_ERR(st->regmap),
> +				     "failed to allocate register map\n");
> +
> +	indio_dev->channels = st->chip_info->channels;
> +	indio_dev->num_channels = st->chip_info->num_channels;
> +	indio_dev->name = st->chip_info->name;
> +	indio_dev->info = &ad7380_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->available_scan_masks = ad7380_2_channel_scan_masks;
> +
> +	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> +					      iio_pollfunc_store_time,
> +					      ad7380_trigger_handler, NULL);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad7380_init(st);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}

...

Re: [PATCH v3 3/3] iio: adc: ad7380: new driver for AD7380 ADCs

[David Lechner]

On Fri, Dec 15, 2023 at 5:53 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 15/12/2023 à 11:32, David Lechner a écrit :
> > This adds a new driver for the AD7380 family ADCs.
> >
> > The driver currently implements basic support for the AD7380, AD7381,
> > AD7383, and AD7384 2-channel differential ADCs. Support for additional
> > single-ended and 4-channel chips that use the same register map as well
> > as additional features of the chip will be added in future patches.
> >
> > Co-developed-by: Stefan Popa <stefan.popa-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Stefan Popa <stefan.popa-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Nuno Sa <nuno.sa-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: David Lechner <dlechner-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> > ---
>
> ...
>
> > +static void ad7380_regulator_disable(void *p)
> > +{
> > +     regulator_disable(p);
> > +}
> > +
> > +static int ad7380_probe(struct spi_device *spi)
> > +{
> > +     struct iio_dev *indio_dev;
> > +     struct ad7380_state *st;
> > +     int ret;
> > +
> > +     indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > +     if (!indio_dev)
> > +             return -ENOMEM;
> > +
> > +     st = iio_priv(indio_dev);
> > +     st->spi = spi;
> > +     st->chip_info = spi_get_device_match_data(spi);
> > +     if (!st->chip_info)
> > +             return dev_err_probe(&spi->dev, -EINVAL, "missing match data\n");
> > +
> > +     st->vref = devm_regulator_get_optional(&spi->dev, "refio");
>
> Hi,
>
> devm_regulator_get_enable_optional()?
> to save some LoC below and ad7380_regulator_disable()

It would be nice if we could, but we need the pointer to the regulator
to read the voltage of the regulator (it is the reference voltage for
an ADC). So we can't use devm_regulator_get_enable_optional() because
it only an int and not the pointer to the regulator.

>
> CJ
>
> > +     if (IS_ERR(st->vref)) {
> > +             /*
> > +              * If there is no REFIO supply, then it means that we are using
> > +              * the internal 2.5V reference.
> > +              */
> > +             if (PTR_ERR(st->vref) == -ENODEV)
> > +                     st->vref = NULL;
> > +             else
> > +                     return dev_err_probe(&spi->dev, PTR_ERR(st->vref),
> > +                                          "Failed to get refio regulator\n");
> > +     }
> > +
> > +     if (st->vref) {
> > +             ret = regulator_enable(st->vref);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             ret = devm_add_action_or_reset(&spi->dev, ad7380_regulator_disable,
> > +                                            st->vref);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     st->regmap = devm_regmap_init(&spi->dev, NULL, st, &ad7380_regmap_config);
> > +     if (IS_ERR(st->regmap))
> > +             return dev_err_probe(&spi->dev, PTR_ERR(st->regmap),
> > +                                  "failed to allocate register map\n");
> > +
> > +     indio_dev->channels = st->chip_info->channels;
> > +     indio_dev->num_channels = st->chip_info->num_channels;
> > +     indio_dev->name = st->chip_info->name;
> > +     indio_dev->info = &ad7380_info;
> > +     indio_dev->modes = INDIO_DIRECT_MODE;
> > +     indio_dev->available_scan_masks = ad7380_2_channel_scan_masks;
> > +
> > +     ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> > +                                           iio_pollfunc_store_time,
> > +                                           ad7380_trigger_handler, NULL);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = ad7380_init(st);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return devm_iio_device_register(&spi->dev, indio_dev);
> > +}
>
> ...
>

Re: [PATCH v3 3/3] iio: adc: ad7380: new driver for AD7380 ADCs

[David Lechner]

On Fri, Dec 15, 2023 at 6:31 PM David Lechner <dlechner@baylibre.com> wrote:
> it only an int and not the pointer to the regulator.

I missed a word, so just it case it wasn't clear:

it only *returns* an int and not the pointer to the regulator.

Re: [PATCH v3 1/3] dt-bindings: spi: add spi-rx-bus-channels peripheral property

[Rob Herring]

On Fri, 15 Dec 2023 04:32:02 -0600, David Lechner wrote:
> This adds a new spi-rx-bus-channels property to the generic spi
> peripheral property bindings. This property is used to describe
> devices that have parallel data output channels.
> 
> This property is different from spi-rx-bus-width in that the latter
> means that we are reading multiple bits of a single word at one time
> while the former means that we are reading single bits of multiple words
> at the same time.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> 
> The rest of this series is ready to merge, so just looking for an ack from
> Mark on this one.
> 
>  .../devicetree/bindings/spi/spi-peripheral-props.yaml        | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 3/3] iio: adc: ad7380: new driver for AD7380 ADCs

[Jonathan Cameron]

On Fri, 15 Dec 2023 04:32:04 -0600
David Lechner <dlechner@baylibre.com> wrote:

> This adds a new driver for the AD7380 family ADCs.
> 
> The driver currently implements basic support for the AD7380, AD7381,
> AD7383, and AD7384 2-channel differential ADCs. Support for additional
> single-ended and 4-channel chips that use the same register map as well
> as additional features of the chip will be added in future patches.
> 
> Co-developed-by: Stefan Popa <stefan.popa@analog.com>
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
LGTM.  Just the Ack from Mark on patch 1 needed now I think
(unless other reviews come in of course!)

Jonathan
> +

Re: [PATCH v3 1/3] dt-bindings: spi: add spi-rx-bus-channels peripheral property

[Jonathan Cameron]

On Fri, 15 Dec 2023 04:32:02 -0600
David Lechner <dlechner@baylibre.com> wrote:

> This adds a new spi-rx-bus-channels property to the generic spi
> peripheral property bindings. This property is used to describe
> devices that have parallel data output channels.
> 
> This property is different from spi-rx-bus-width in that the latter
> means that we are reading multiple bits of a single word at one time
> while the former means that we are reading single bits of multiple words
> at the same time.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> 
> The rest of this series is ready to merge, so just looking for an ack from
> Mark on this one.

Mark, could you take a look at this SPI binding change when you have time?

I don't want to apply it without your view on whether this makes sense
from a general SPI point of view as we all hate maintaining bindings
if they turn out to not be sufficiently future looking etc and we need
to deprecate them in favour of something else.

Thanks,

Jonathan

> 
>  .../devicetree/bindings/spi/spi-peripheral-props.yaml        | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index 15938f81fdce..1c8e71c18234 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -67,6 +67,18 @@ properties:
>      enum: [0, 1, 2, 4, 8]
>      default: 1
>  
> +  spi-rx-bus-channels:
> +    description:
> +      The number of parallel channels for read transfers. The difference between
> +      this and spi-rx-bus-width is that a value N for spi-rx-bus-channels means
> +      the SPI bus is receiving one bit each of N different words at the same
> +      time whereas a value M for spi-rx-bus-width means that the bus is
> +      receiving M bits of a single word at the same time. It is also possible to
> +      use both properties at the same time, meaning the bus is receiving M bits
> +      of N different words at the same time.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 1
> +
>    spi-rx-delay-us:
>      description:
>        Delay, in microseconds, after a read transfer.
> 

Re: [PATCH v3 1/3] dt-bindings: spi: add spi-rx-bus-channels peripheral property

[Mark Brown]

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

On Sun, Jan 07, 2024 at 04:43:56PM +0000, Jonathan Cameron wrote:
> David Lechner <dlechner@baylibre.com> wrote:

> > This adds a new spi-rx-bus-channels property to the generic spi
> > peripheral property bindings. This property is used to describe
> > devices that have parallel data output channels.

> > This property is different from spi-rx-bus-width in that the latter
> > means that we are reading multiple bits of a single word at one time
> > while the former means that we are reading single bits of multiple words
> > at the same time.

> Mark, could you take a look at this SPI binding change when you have time?

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

> I don't want to apply it without your view on whether this makes sense
> from a general SPI point of view as we all hate maintaining bindings
> if they turn out to not be sufficiently future looking etc and we need
> to deprecate them in favour of something else.

This makes no sense to me without a corresponding change in the SPI core
and possibly controller support, though I guess you could do data
manging to rewrite from a normal parallel SPI to this for a pure
software implementation.  I also see nothing in the driver that even
attempts to parse this so I can't see how it could possibly work.

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

Re: [PATCH v3 1/3] dt-bindings: spi: add spi-rx-bus-channels peripheral property

[David Lechner]

On Sun, Jan 7, 2024 at 3:27 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Sun, Jan 07, 2024 at 04:43:56PM +0000, Jonathan Cameron wrote:
> > David Lechner <dlechner@baylibre.com> wrote:
>
> > > This adds a new spi-rx-bus-channels property to the generic spi
> > > peripheral property bindings. This property is used to describe
> > > devices that have parallel data output channels.
>
> > > This property is different from spi-rx-bus-width in that the latter
> > > means that we are reading multiple bits of a single word at one time
> > > while the former means that we are reading single bits of multiple words
> > > at the same time.
>
> > Mark, could you take a look at this SPI binding change when you have time?
>
> Please submit patches using subject lines reflecting the style for the
> subsystem, this makes it easier for people to identify relevant patches.
> Look at what existing commits in the area you're changing are doing and
> make sure your subject lines visually resemble what they're doing.
> There's no need to resubmit to fix this alone.

Are you saying that `spi: dt-bindings:` should be preferred over
`dt-bindings: spi:`?

I thought I was doing it right since I was following the guidelines of
[1] which says:

> The preferred subject prefix for binding patches is:
>     "dt-bindings: <binding dir>: ..."

[1]: https://www.kernel.org/doc/html//v6.7/devicetree/bindings/submitting-patches.html

>
> > I don't want to apply it without your view on whether this makes sense
> > from a general SPI point of view as we all hate maintaining bindings
> > if they turn out to not be sufficiently future looking etc and we need
> > to deprecate them in favour of something else.
>
> This makes no sense to me without a corresponding change in the SPI core
> and possibly controller support, though I guess you could do data
> manging to rewrite from a normal parallel SPI to this for a pure
> software implementation.  I also see nothing in the driver that even
> attempts to parse this so I can't see how it could possibly work.

We currently don't have a controller that supports this. This is just
an attempt to make a complete binding for a peripheral according to
[2] which says:

> DO attempt to make bindings complete even if a driver doesn't support some features

[2]: https://www.kernel.org/doc/html//v6.7/devicetree/bindings/writing-bindings.html

So, will DT maintainers accept an incomplete binding for the
peripheral? Or will you reconsider this without SPI core support if I
can explain it better? It doesn't seem like a reasonable request to
expect us to spend time developing software that we don't need at this
time just to get a complete DT binding accepted for a feature that
isn't being used.

In the SPI core, I would expect this property to correspond to new
flags `SPI_RX_2_CH`, `SPI_RX_4_CH`, `SPI_RX_8_CH` and it would have
checks similar to other flags to make sure controller supports the
flag if the peripheral requires it. Likewise, struct spi_transfer
would probably need a rx_n_ch field similar to rx_nbits to specify if
individual xfers use the feature. But beyond that, yes I agree it
would be difficult to say how it should work without implementing it
on actual hardware.

Re: [PATCH v3 1/3] dt-bindings: spi: add spi-rx-bus-channels peripheral property

[Krzysztof Kozlowski]

On 08/01/2024 00:02, David Lechner wrote:
> On Sun, Jan 7, 2024 at 3:27 PM Mark Brown <broonie@kernel.org> wrote:
>>
>> On Sun, Jan 07, 2024 at 04:43:56PM +0000, Jonathan Cameron wrote:
>>> David Lechner <dlechner@baylibre.com> wrote:
>>
>>>> This adds a new spi-rx-bus-channels property to the generic spi
>>>> peripheral property bindings. This property is used to describe
>>>> devices that have parallel data output channels.
>>
>>>> This property is different from spi-rx-bus-width in that the latter
>>>> means that we are reading multiple bits of a single word at one time
>>>> while the former means that we are reading single bits of multiple words
>>>> at the same time.
>>
>>> Mark, could you take a look at this SPI binding change when you have time?
>>
>> Please submit patches using subject lines reflecting the style for the
>> subsystem, this makes it easier for people to identify relevant patches.
>> Look at what existing commits in the area you're changing are doing and
>> make sure your subject lines visually resemble what they're doing.
>> There's no need to resubmit to fix this alone.
> 
> Are you saying that `spi: dt-bindings:` should be preferred over
> `dt-bindings: spi:`?
> 
> I thought I was doing it right since I was following the guidelines of
> [1] which says:
> 
>> The preferred subject prefix for binding patches is:
>>     "dt-bindings: <binding dir>: ..."
> 
> [1]: https://www.kernel.org/doc/html//v6.7/devicetree/bindings/submitting-patches.html

There are exceptions. I documented them now:

https://lore.kernel.org/linux-devicetree/20240108083750.16350-2-krzysztof.kozlowski@linaro.org/T/#u

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: spi: add spi-rx-bus-channels peripheral property

[Mark Brown]

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

On Sun, Jan 07, 2024 at 05:02:56PM -0600, David Lechner wrote:
> On Sun, Jan 7, 2024 at 3:27 PM Mark Brown <broonie@kernel.org> wrote:

> > This makes no sense to me without a corresponding change in the SPI core
> > and possibly controller support, though I guess you could do data
> > manging to rewrite from a normal parallel SPI to this for a pure
> > software implementation.  I also see nothing in the driver that even
> > attempts to parse this so I can't see how it could possibly work.

> We currently don't have a controller that supports this. This is just
> an attempt to make a complete binding for a peripheral according to
> [2] which says:

...

> So, will DT maintainers accept an incomplete binding for the
> peripheral? Or will you reconsider this without SPI core support if I
> can explain it better? It doesn't seem like a reasonable request to
> expect us to spend time developing software that we don't need at this
> time just to get a complete DT binding accepted for a feature that
> isn't being used.

I don't think it's sensible to try to make a binding for this without
having actually tried to put a system together that uses it and made
sure that everything is joined up properly, the thing about complete
bindings is more for things that are handle turning than for things that
are substantial new features in subsystems.

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

Re: [PATCH v3 1/3] dt-bindings: spi: add spi-rx-bus-channels peripheral property

[David Lechner]

On Mon, Jan 8, 2024 at 10:39 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Sun, Jan 07, 2024 at 05:02:56PM -0600, David Lechner wrote:
> > On Sun, Jan 7, 2024 at 3:27 PM Mark Brown <broonie@kernel.org> wrote:
>
> > > This makes no sense to me without a corresponding change in the SPI core
> > > and possibly controller support, though I guess you could do data
> > > manging to rewrite from a normal parallel SPI to this for a pure
> > > software implementation.  I also see nothing in the driver that even
> > > attempts to parse this so I can't see how it could possibly work.
>
> > We currently don't have a controller that supports this. This is just
> > an attempt to make a complete binding for a peripheral according to
> > [2] which says:
>
> ...
>
> > So, will DT maintainers accept an incomplete binding for the
> > peripheral? Or will you reconsider this without SPI core support if I
> > can explain it better? It doesn't seem like a reasonable request to
> > expect us to spend time developing software that we don't need at this
> > time just to get a complete DT binding accepted for a feature that
> > isn't being used.
>
> I don't think it's sensible to try to make a binding for this without
> having actually tried to put a system together that uses it and made
> sure that everything is joined up properly, the thing about complete
> bindings is more for things that are handle turning than for things that
> are substantial new features in subsystems.

We do have plans to eventually implement such a feature in an
FPGA-based SPI controller, so if we need to wait until then for the
binding, then we can do that. But it would be really nice if we could
find a way forward for the IIO driver in this series without having to
wait for the resolution of new SPI controller feature for the complete
DT bindings.

DT/IIO maintainers, if I resubmit this series with the
`spi-rx-bus-channels` parts removed from the iio/adc/adi,ad7380.yaml
bindings, would that be acceptable? (Also resubmitting without this
patch of course.)

Re: [PATCH v3 1/3] dt-bindings: spi: add spi-rx-bus-channels peripheral property

[Jonathan Cameron]

On Mon, 8 Jan 2024 11:15:31 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On Mon, Jan 8, 2024 at 10:39 AM Mark Brown <broonie@kernel.org> wrote:
> >
> > On Sun, Jan 07, 2024 at 05:02:56PM -0600, David Lechner wrote:  
> > > On Sun, Jan 7, 2024 at 3:27 PM Mark Brown <broonie@kernel.org> wrote:  
> >  
> > > > This makes no sense to me without a corresponding change in the SPI core
> > > > and possibly controller support, though I guess you could do data
> > > > manging to rewrite from a normal parallel SPI to this for a pure
> > > > software implementation.  I also see nothing in the driver that even
> > > > attempts to parse this so I can't see how it could possibly work.  
> >  
> > > We currently don't have a controller that supports this. This is just
> > > an attempt to make a complete binding for a peripheral according to
> > > [2] which says:  
> >
> > ...
> >  
> > > So, will DT maintainers accept an incomplete binding for the
> > > peripheral? Or will you reconsider this without SPI core support if I
> > > can explain it better? It doesn't seem like a reasonable request to
> > > expect us to spend time developing software that we don't need at this
> > > time just to get a complete DT binding accepted for a feature that
> > > isn't being used.  
> >
> > I don't think it's sensible to try to make a binding for this without
> > having actually tried to put a system together that uses it and made
> > sure that everything is joined up properly, the thing about complete
> > bindings is more for things that are handle turning than for things that
> > are substantial new features in subsystems.  
> 
> We do have plans to eventually implement such a feature in an
> FPGA-based SPI controller, so if we need to wait until then for the
> binding, then we can do that. But it would be really nice if we could
> find a way forward for the IIO driver in this series without having to
> wait for the resolution of new SPI controller feature for the complete
> DT bindings.
> 
> DT/IIO maintainers, if I resubmit this series with the
> `spi-rx-bus-channels` parts removed from the iio/adc/adi,ad7380.yaml
> bindings, would that be acceptable? (Also resubmitting without this
> patch of course.)
> 
From IIO side of things that's fine with me.

Jonathan