[PATCH v4 0/2] add AD8460 DAC driver

[PATCH v4 0/2] add AD8460 DAC driver

[Mariel Tinaco]

Apply comments for adding support to AD8460 Waveform Generator DAC

changes in v4

ad8460:
  * Fixed spacing issues and tab format of includes.
  * Changed data type of spi_tx_buffer to __le16. Then used le16_to_cpu and
    cpu_to_le16 functions for bulk read and write access.
  * Changed print format in sysfs_emit for unsigned int.
  * Added comments on powerdown write function to describe logic.
  * Omitted "_shared" parameter in AD8460_CHAN_EXT_INFO. 
  * Applied further changes to channel definition (endianness = IIO_CPU and
    AD8460_CURRENT_CHAN output = 1).
  * Reformatted section that reads REFIO_1P2V voltage.
  * Fixed upper and lower limits for checking of REFIO_1P2V.
  * Removed comment for arming the device in probe function.
  * Created a section for IIO related includes.
  * Replaced mutex_init with devm_mutex_init.
  * Added and enabled the other supplies using regulator bulk enable
  * Handled devm_iio_channel_get error -EPROBE_DEFER by returning
    -EPROBE_DEFER

Bindings:
  * Removed description on clocks
  * Reverted GPIO pin name to shutdown-reset-GPIO
  * Added minimum, maximum and default for setting voltage, current and
    temperature
  * Added required supplies
  * Changed additionalProperties to unevaluatedProperties

Mariel Tinaco (2):
  dt-bindings: iio: dac: add docs for ad8460
  iio: dac: support the ad8460 Waveform DAC

 .../bindings/iio/dac/adi,ad8460.yaml          | 164 +++
 MAINTAINERS                                   |   8 +
 drivers/iio/dac/Kconfig                       |  13 +
 drivers/iio/dac/Makefile                      |   1 +
 drivers/iio/dac/ad8460.c                      | 947 ++++++++++++++++++
 5 files changed, 1133 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml
 create mode 100644 drivers/iio/dac/ad8460.c


base-commit: fec496684388685647652ab4213454fbabdab099
-- 
2.34.1

[PATCH v4 1/2] dt-bindings: iio: dac: add docs for ad8460

[Mariel Tinaco]

This adds the bindings documentation for the 14-bit
High Voltage, High Current, Waveform Generator
Digital-to-Analog converter.

Signed-off-by: Mariel Tinaco <Mariel.Tinaco@analog.com>
---
 .../bindings/iio/dac/adi,ad8460.yaml          | 164 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 171 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml
new file mode 100644
index 000000000000..b65928024e12
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml
@@ -0,0 +1,164 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2024 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad8460.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD8460 DAC
+
+maintainers:
+  - Mariel Tinaco <mariel.tinaco@analog.com>
+
+description: |
+  Analog Devices AD8460 110 V High Voltage, 1 A High Current,
+  Arbitrary Waveform Generator with Integrated 14-Bit High Speed DAC
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad8460.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ad8460
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  dmas:
+    maxItems: 1
+
+  dma-names:
+    items:
+      - const: tx
+
+  spi-max-frequency:
+    maximum: 20000000
+
+  hvcc-supply:
+    description: Positive high voltage power supply line
+
+  hvee-supply:
+    description: Negative high voltage power supply line
+
+  vcc-5v-supply:
+    description: Low voltage power supply
+
+  vref-5v-supply:
+    description: Reference voltage for analog low voltage
+
+  dvdd-3p3v-supply:
+    description: Digital supply bypass
+
+  avdd-3p3v-supply:
+    description: Analog supply bypass
+
+  refio-1p2v-supply:
+    description: Drive voltage in the range of 1.2V maximum to as low as
+      low as 0.12V through the REF_IO pin to adjust full scale output span
+
+  adi,external-resistor-ohms:
+    description: Specify value of external resistor connected to FS_ADJ pin
+      to establish internal HVDAC's reference current I_REF
+    minimum: 2000
+    maximum: 20000
+    default: 2000
+
+  adi,range-microvolt:
+    description: Voltage output range specified as <minimum, maximum>
+    items:
+      - minimum: -55000000
+        maximum: 0
+        default: 0
+      - minimum: 0
+        maximum: 55000000
+        default: 0
+
+  adi,range-microamp:
+    description: Current output range specified as <minimum, maximum>
+    items:
+      - minimum: -1000000
+        maximum: 0
+        default: 0
+      - minimum: 0
+        maximum: 1000000
+        default: 0
+
+  adi,max-millicelsius:
+    description: Overtemperature threshold
+    minimum: 0
+    maximum: 150000
+    default: 0
+
+  shutdown-reset-gpios:
+    description: Corresponds to SDN_RESET pin. To exit shutdown
+      or sleep mode, pulse SDN_RESET HIGH, then leave LOW.
+    maxItems: 1
+
+  reset-gpios:
+    description: Manual Power On Reset (POR). Pull this GPIO pin
+      LOW and then HIGH to reset all digital registers to default
+    maxItems: 1
+
+  shutdown-gpios:
+    description: Corresponds to SDN_IO pin. Shutdown may be
+      initiated by the user, by pulsing SDN_IO high. To exit shutdown,
+      pulse SDN_IO low, then float.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - hvcc-supply
+  - hvee-supply
+  - vcc-5v-supply
+  - vref-5v-supply
+  - dvdd-3p3v-supply
+  - avdd-3p3v-supply
+  - refio-1p2v-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dac@0 {
+            compatible = "adi,ad8460";
+            reg = <0>;
+            spi-max-frequency = <8000000>;
+
+            dmas = <&tx_dma 0>;
+            dma-names = "tx";
+
+            shutdown-reset-gpios = <&gpio 86 GPIO_ACTIVE_HIGH>;
+            reset-gpios = <&gpio 91 GPIO_ACTIVE_LOW>;
+            shutdown-gpios = <&gpio 88 GPIO_ACTIVE_HIGH>;
+
+            clocks = <&sync_ext_clk>;
+
+            hvcc-supply = <&hvcc>;
+            hvee-supply = <&hvee>;
+            vcc-5v-supply = <&vcc_5>;
+            vref-5v-supply = <&vref_5>;
+            dvdd-3p3v-supply = <&dvdd_3_3>;
+            avdd-3p3v-supply = <&avdd_3_3>;
+            refio-1p2v-supply = <&refio_1_2>;
+
+            adi,external-resistor-ohms = <2000>;
+            adi,range-microvolt = <(-40000000) 40000000>;
+            adi,range-microamp = <0 50000>;
+            adi,max-millicelsius = <50000>;
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index a22f22369ad5..9f0acaea0749 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1320,6 +1320,13 @@ F:	Documentation/ABI/testing/debugfs-iio-ad9467
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
 F:	drivers/iio/adc/ad9467.c
 
+ANALOG DEVICES INC AD8460 DRIVER
+M:	Mariel Tinaco <Mariel.Tinaco@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml
+
 ANALOG DEVICES INC AD9739a DRIVER
 M:	Nuno Sa <nuno.sa@analog.com>
 M:	Dragos Bogdan <dragos.bogdan@analog.com>
-- 
2.34.1

[PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC

[Mariel Tinaco]

The AD8460 is a “bits in, power out” high voltage, high-power,
high-speed driver optimized for large output current (up to ±1 A)
and high slew rate (up to ±1800 V/μs) at high voltage (up to ±40 V)
into capacitive loads.

A digital engine implements user-configurable features: modes for
digital input, programmable supply current, and fault monitoring
and programmable protection settings for output current,
output voltage, and junction temperature. The AD8460 operates on
high voltage dual supplies up to ±55 V and a single low voltage
supply of 5 V.

Signed-off-by: Mariel Tinaco <Mariel.Tinaco@analog.com>
---
 MAINTAINERS              |   1 +
 drivers/iio/dac/Kconfig  |  13 +
 drivers/iio/dac/Makefile |   1 +
 drivers/iio/dac/ad8460.c | 947 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 962 insertions(+)
 create mode 100644 drivers/iio/dac/ad8460.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9f0acaea0749..b0d67a2229f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1326,6 +1326,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml
+F:	drivers/iio/dac/ad8460.c
 
 ANALOG DEVICES INC AD9739a DRIVER
 M:	Nuno Sa <nuno.sa@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 1cfd7e2a622f..fa091995d002 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -301,6 +301,19 @@ config AD7303
 	  To compile this driver as module choose M here: the module will be called
 	  ad7303.
 
+config AD8460
+	tristate "Analog Devices AD8460 DAC driver"
+	depends on SPI
+	select REGMAP_SPI
+	select IIO_BUFFER
+	select IIO_BUFFER_DMAENGINE
+	help
+	  Say yes here to build support for Analog Devices AD8460 Digital to
+	  Analog Converters (DAC).
+
+	  To compile this driver as a module choose M here: the module will be called
+	  ad8460.
+
 config AD8801
 	tristate "Analog Devices AD8801/AD8803 DAC driver"
 	depends on SPI_MASTER
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 2cf148f16306..621d553bd6e3 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_AD5686_SPI) += ad5686-spi.o
 obj-$(CONFIG_AD5696_I2C) += ad5696-i2c.o
 obj-$(CONFIG_AD7293) += ad7293.o
 obj-$(CONFIG_AD7303) += ad7303.o
+obj-$(CONFIG_AD8460) += ad8460.o
 obj-$(CONFIG_AD8801) += ad8801.o
 obj-$(CONFIG_AD9739A) += ad9739a.o
 obj-$(CONFIG_ADI_AXI_DAC) += adi-axi-dac.o
diff --git a/drivers/iio/dac/ad8460.c b/drivers/iio/dac/ad8460.c
new file mode 100644
index 000000000000..9ce3a0f288ba
--- /dev/null
+++ b/drivers/iio/dac/ad8460.c
@@ -0,0 +1,947 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD8460 Waveform generator DAC Driver
+ *
+ * Copyright (C) 2024 Analog Devices, Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/buffer-dma.h>
+#include <linux/iio/buffer-dmaengine.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+
+#define AD8460_CTRL_REG(x)			(x)
+#define AD8460_HVDAC_DATA_WORD(x)		(0x60 + (2 * (x)))
+
+#define AD8460_HV_RESET_MSK			BIT(7)
+#define AD8460_HV_SLEEP_MSK			BIT(4)
+#define AD8460_WAVE_GEN_MODE_MSK		BIT(0)
+
+#define AD8460_HVDAC_SLEEP_MSK			BIT(3)
+
+#define AD8460_FAULT_ARM_MSK			BIT(7)
+#define AD8460_FAULT_LIMIT_MSK			GENMASK(6, 0)
+
+#define AD8460_APG_MODE_ENABLE_MSK		BIT(5)
+#define AD8460_PATTERN_DEPTH_MSK		GENMASK(3, 0)
+
+#define AD8460_QUIESCENT_CURRENT_MSK		GENMASK(7, 0)
+
+#define AD8460_SHUTDOWN_FLAG_MSK		BIT(7)
+
+#define AD8460_DATA_BYTE_LOW_MSK		GENMASK(7, 0)
+#define AD8460_DATA_BYTE_HIGH_MSK		GENMASK(5, 0)
+#define AD8460_DATA_BYTE_FULL_MSK		GENMASK(13, 0)
+
+#define AD8460_DEFAULT_FAULT_PROTECT		0x00
+#define AD8460_DATA_BYTE_WORD_LENGTH		2
+#define AD8460_NUM_DATA_WORDS			16
+#define AD8460_NOMINAL_VOLTAGE_SPAN		80
+#define AD8460_MIN_EXT_RESISTOR_OHMS		2000
+#define AD8460_MAX_EXT_RESISTOR_OHMS		20000
+#define AD8460_MIN_VREFIO_UV			120000
+#define AD8460_MAX_VREFIO_UV			1200000
+#define AD8460_ABS_MAX_OVERVOLTAGE_UV		55000000
+#define AD8460_ABS_MAX_OVERCURRENT_UA		1000000
+#define AD8460_MAX_OVERTEMPERATURE_MC		150000
+#define AD8460_MIN_OVERTEMPERATURE_MC		20000
+#define AD8460_CURRENT_LIMIT_CONV(x)		((x) / 15625)
+#define AD8460_VOLTAGE_LIMIT_CONV(x)		((x) / 1953000)
+#define AD8460_TEMP_LIMIT_CONV(x)		(((x) + 266640) / 6510)
+
+enum ad8460_fault_type {
+	AD8460_OVERCURRENT_SRC,
+	AD8460_OVERCURRENT_SNK,
+	AD8460_OVERVOLTAGE_POS,
+	AD8460_OVERVOLTAGE_NEG,
+	AD8460_OVERTEMPERATURE,
+};
+
+struct ad8460_state {
+	struct spi_device *spi;
+	struct regmap *regmap;
+	struct iio_channel *tmp_adc_channel;
+	struct clk *sync_clk;
+	/* lock to protect against multiple access to the device and shared data */
+	struct mutex lock;
+	int refio_1p2v_mv;
+	u32 ext_resistor_ohms;
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	__le16 spi_tx_buf __aligned(IIO_DMA_MINALIGN);
+};
+
+static int ad8460_hv_reset(struct ad8460_state *state)
+{
+	int ret;
+
+	ret = regmap_set_bits(state->regmap, AD8460_CTRL_REG(0x00),
+			      AD8460_HV_RESET_MSK);
+	if (ret)
+		return ret;
+
+	fsleep(20);
+
+	return regmap_clear_bits(state->regmap, AD8460_CTRL_REG(0x00),
+				 AD8460_HV_RESET_MSK);
+}
+
+static int ad8460_reset(const struct ad8460_state *state)
+{
+	struct device *dev = &state->spi->dev;
+	struct gpio_desc *reset;
+
+	reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(reset))
+		return dev_err_probe(dev, PTR_ERR(reset),
+				     "Failed to get reset gpio");
+	if (reset) {
+		/* minimum duration of 10ns */
+		ndelay(10);
+		gpiod_set_value_cansleep(reset, 1);
+		return 0;
+	}
+
+	/* bring all registers to their default state */
+	return regmap_write(state->regmap, AD8460_CTRL_REG(0x03), 1);
+}
+
+static int ad8460_enable_apg_mode(struct ad8460_state *state, int val)
+{
+	int ret;
+
+	ret = regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x02),
+				 AD8460_APG_MODE_ENABLE_MSK,
+				 FIELD_PREP(AD8460_APG_MODE_ENABLE_MSK, val));
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x00),
+				  AD8460_WAVE_GEN_MODE_MSK,
+				  FIELD_PREP(AD8460_WAVE_GEN_MODE_MSK, val));
+}
+
+static int ad8460_read_shutdown_flag(struct ad8460_state *state, u64 *flag)
+{
+	int ret, val;
+
+	ret = regmap_read(state->regmap, AD8460_CTRL_REG(0x0E), &val);
+	if (ret)
+		return ret;
+
+	*flag = FIELD_GET(AD8460_SHUTDOWN_FLAG_MSK, val);
+	return 0;
+}
+
+static int ad8460_get_hvdac_word(struct ad8460_state *state, int index, int *val)
+{
+	int ret;
+
+	ret = regmap_bulk_read(state->regmap, AD8460_HVDAC_DATA_WORD(index),
+			       &state->spi_tx_buf, AD8460_DATA_BYTE_WORD_LENGTH);
+	if (ret)
+		return ret;
+
+	*val = le16_to_cpu(state->spi_tx_buf);
+
+	return ret;
+}
+
+static int ad8460_set_hvdac_word(struct ad8460_state *state, int index, int val)
+{
+	state->spi_tx_buf = cpu_to_le16(FIELD_PREP(AD8460_DATA_BYTE_FULL_MSK, val));
+
+	return regmap_bulk_write(state->regmap, AD8460_HVDAC_DATA_WORD(index),
+				 &state->spi_tx_buf, AD8460_DATA_BYTE_WORD_LENGTH);
+}
+
+static ssize_t ad8460_dac_input_read(struct iio_dev *indio_dev, uintptr_t private,
+				     const struct iio_chan_spec *chan, char *buf)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	unsigned int reg;
+	int ret;
+
+	ret = ad8460_get_hvdac_word(state, private, &reg);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%u\n", reg);
+}
+
+static ssize_t ad8460_dac_input_write(struct iio_dev *indio_dev, uintptr_t private,
+				      const struct iio_chan_spec *chan,
+				      const char *buf, size_t len)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	unsigned int reg;
+	int ret;
+
+	ret = kstrtou32(buf, 10, &reg);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&state->lock);
+
+	return ad8460_set_hvdac_word(state, private, reg);
+}
+
+static ssize_t ad8460_read_symbol(struct iio_dev *indio_dev, uintptr_t private,
+				  const struct iio_chan_spec *chan, char *buf)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(state->regmap, AD8460_CTRL_REG(0x02), &reg);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lu\n", FIELD_GET(AD8460_PATTERN_DEPTH_MSK, reg));
+}
+
+static ssize_t ad8460_write_symbol(struct iio_dev *indio_dev, uintptr_t private,
+				   const struct iio_chan_spec *chan,
+				   const char *buf, size_t len)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	uint16_t sym;
+	int ret;
+
+	ret = kstrtou16(buf, 10, &sym);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&state->lock);
+
+	return regmap_update_bits(state->regmap,
+				  AD8460_CTRL_REG(0x02),
+				  AD8460_PATTERN_DEPTH_MSK,
+				  FIELD_PREP(AD8460_PATTERN_DEPTH_MSK, sym));
+}
+
+static ssize_t ad8460_read_toggle_en(struct iio_dev *indio_dev, uintptr_t private,
+				     const struct iio_chan_spec *chan, char *buf)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(state->regmap, AD8460_CTRL_REG(0x02), &reg);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%ld\n", FIELD_GET(AD8460_APG_MODE_ENABLE_MSK, reg));
+}
+
+static ssize_t ad8460_write_toggle_en(struct iio_dev *indio_dev, uintptr_t private,
+				      const struct iio_chan_spec *chan,
+				      const char *buf, size_t len)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	bool toggle_en;
+	int ret;
+
+	ret = kstrtobool(buf, &toggle_en);
+	if (ret)
+		return ret;
+
+	iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
+		return ad8460_enable_apg_mode(state, toggle_en);
+	unreachable();
+}
+
+static ssize_t ad8460_read_powerdown(struct iio_dev *indio_dev, uintptr_t private,
+				     const struct iio_chan_spec *chan, char *buf)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(state->regmap, AD8460_CTRL_REG(0x01), &reg);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%ld\n", FIELD_GET(AD8460_HVDAC_SLEEP_MSK, reg));
+}
+
+static ssize_t ad8460_write_powerdown(struct iio_dev *indio_dev, uintptr_t private,
+				      const struct iio_chan_spec *chan,
+				      const char *buf, size_t len)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	bool pwr_down;
+	u64 sdn_flag;
+	int ret;
+
+	ret = kstrtobool(buf, &pwr_down);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&state->lock);
+
+	/*
+	 * If powerdown is set, HVDAC is enabled and the HV driver is
+	 * enabled via HV_RESET in case it is in shutdown mode,
+	 * If powerdown is cleared, HVDAC is set to shutdown state
+	 * as well as the HV driver. Quiescent current decreases and ouput is
+	 * floating (high impedance).
+	 */
+
+	ret = regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x01),
+				 AD8460_HVDAC_SLEEP_MSK,
+				 FIELD_PREP(AD8460_HVDAC_SLEEP_MSK, pwr_down));
+	if (ret)
+		return ret;
+
+	if (!pwr_down) {
+		ret = ad8460_read_shutdown_flag(state, &sdn_flag);
+		if (ret)
+			return ret;
+
+		if (sdn_flag) {
+			ret = ad8460_hv_reset(state);
+			if (ret)
+				return ret;
+		}
+	}
+
+	ret = regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x00),
+				 AD8460_HV_SLEEP_MSK,
+				 FIELD_PREP(AD8460_HV_SLEEP_MSK, !pwr_down));
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static const char * const ad8460_powerdown_modes[] = {
+	"three_state",
+};
+
+static int ad8460_get_powerdown_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan)
+{
+	return 0;
+}
+
+static int ad8460_set_powerdown_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     unsigned int type)
+{
+	return 0;
+}
+
+static int ad8460_set_sample(struct ad8460_state *state, int val)
+{
+	int ret;
+
+	ret = ad8460_enable_apg_mode(state, 1);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&state->lock);
+	ret = ad8460_set_hvdac_word(state, 0, val);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x02),
+				  AD8460_PATTERN_DEPTH_MSK,
+				  FIELD_PREP(AD8460_PATTERN_DEPTH_MSK, 0));
+}
+
+static int ad8460_set_fault_threshold(struct ad8460_state *state,
+				      enum ad8460_fault_type fault,
+				      unsigned int threshold)
+{
+	return regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x08 + fault),
+				  AD8460_FAULT_LIMIT_MSK,
+				  FIELD_PREP(AD8460_FAULT_LIMIT_MSK, threshold));
+}
+
+static int ad8460_get_fault_threshold(struct ad8460_state *state,
+				      enum ad8460_fault_type fault,
+				      unsigned int *threshold)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(state->regmap, AD8460_CTRL_REG(0x08 + fault), &val);
+	if (ret)
+		return ret;
+
+	*threshold = FIELD_GET(AD8460_FAULT_LIMIT_MSK, val);
+
+	return ret;
+}
+
+static int ad8460_set_fault_threshold_en(struct ad8460_state *state,
+					 enum ad8460_fault_type fault, bool en)
+{
+	return regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x08 + fault),
+				  AD8460_FAULT_ARM_MSK,
+				  FIELD_PREP(AD8460_FAULT_ARM_MSK, en));
+}
+
+static int ad8460_get_fault_threshold_en(struct ad8460_state *state,
+					 enum ad8460_fault_type fault, bool *en)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(state->regmap, AD8460_CTRL_REG(0x08 + fault), &val);
+	if (ret)
+		return ret;
+
+	*en = FIELD_GET(AD8460_FAULT_ARM_MSK, val);
+
+	return 0;
+}
+
+static int ad8460_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int val, int val2,
+			    long mask)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
+				return ad8460_set_sample(state, val);
+			unreachable();
+		case IIO_CURRENT:
+			return regmap_write(state->regmap, AD8460_CTRL_REG(0x04),
+					    FIELD_PREP(AD8460_QUIESCENT_CURRENT_MSK, val));
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad8460_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	int data, ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			scoped_guard(mutex, &state->lock) {
+				ret = ad8460_get_hvdac_word(state, 0, &data);
+				if (ret)
+					return ret;
+			}
+			*val = data;
+			return IIO_VAL_INT;
+		case IIO_CURRENT:
+			ret = regmap_read(state->regmap, AD8460_CTRL_REG(0x04),
+					  &data);
+			if (ret)
+				return ret;
+			*val = data;
+			return IIO_VAL_INT;
+		case IIO_TEMP:
+			ret = iio_read_channel_raw(state->tmp_adc_channel, &data);
+			if (ret)
+				return ret;
+			*val = data;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = clk_get_rate(state->sync_clk);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * vCONV = vNOMINAL_SPAN * (DAC_CODE / 2**14) - 40V
+		 * vMAX  = vNOMINAL_SPAN * (2**14 / 2**14) - 40V
+		 * vMIN  = vNOMINAL_SPAN * (0 / 2**14) - 40V
+		 * vADJ  = vCONV * (2000 / rSET) * (vREF / 1.2)
+		 * vSPAN = vADJ_MAX - vADJ_MIN
+		 * See datasheet page 49, section FULL-SCALE REDUCTION
+		 */
+		*val = AD8460_NOMINAL_VOLTAGE_SPAN * 2000 * state->refio_1p2v_mv;
+		*val2 = state->ext_resistor_ohms * 1200;
+		return IIO_VAL_FRACTIONAL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad8460_select_fault_type(int chan_type, enum iio_event_direction dir)
+{
+	switch (chan_type) {
+	case IIO_VOLTAGE:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			return AD8460_OVERVOLTAGE_POS;
+		case IIO_EV_DIR_FALLING:
+			return AD8460_OVERVOLTAGE_NEG;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CURRENT:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			return AD8460_OVERCURRENT_SRC;
+		case IIO_EV_DIR_FALLING:
+			return AD8460_OVERCURRENT_SNK;
+		default:
+			return -EINVAL;
+		}
+	case IIO_TEMP:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			return AD8460_OVERTEMPERATURE;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad8460_write_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info, int val, int val2)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	unsigned int fault;
+
+	if (type != IIO_EV_TYPE_THRESH)
+		return -EINVAL;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	fault = ad8460_select_fault_type(chan->type, dir);
+	if (fault < 0)
+		return fault;
+
+	return ad8460_set_fault_threshold(state, fault, val);
+}
+
+static int ad8460_read_event_value(struct iio_dev *indio_dev,
+				   const struct iio_chan_spec *chan,
+				   enum iio_event_type type,
+				   enum iio_event_direction dir,
+				   enum iio_event_info info, int *val, int *val2)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	unsigned int fault;
+
+	if (type != IIO_EV_TYPE_THRESH)
+		return -EINVAL;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	fault = ad8460_select_fault_type(chan->type, dir);
+	if (fault < 0)
+		return fault;
+
+	return ad8460_get_fault_threshold(state, fault, val);
+}
+
+static int ad8460_write_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir, int val)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	unsigned int fault;
+
+	if (type != IIO_EV_TYPE_THRESH)
+		return -EINVAL;
+
+	fault = ad8460_select_fault_type(chan->type, dir);
+	if (fault < 0)
+		return fault;
+
+	return ad8460_set_fault_threshold_en(state, fault, val);
+}
+
+static int ad8460_read_event_config(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+	unsigned int fault;
+	bool en;
+	int ret;
+
+	if (type != IIO_EV_TYPE_THRESH)
+		return -EINVAL;
+
+	fault = ad8460_select_fault_type(chan->type, dir);
+	if (fault < 0)
+		return fault;
+
+	ret = ad8460_get_fault_threshold_en(state, fault, &en);
+	if (ret)
+		return ret;
+
+	return en;
+}
+
+static int ad8460_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			     unsigned int writeval, unsigned int *readval)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(state->regmap, reg, readval);
+
+	return regmap_write(state->regmap, reg, writeval);
+}
+
+static int ad8460_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+
+	return ad8460_enable_apg_mode(state, 0);
+}
+
+static int ad8460_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct ad8460_state *state = iio_priv(indio_dev);
+
+	return ad8460_enable_apg_mode(state, 1);
+}
+
+static const struct iio_buffer_setup_ops ad8460_buffer_setup_ops = {
+	.preenable = &ad8460_buffer_preenable,
+	.postdisable = &ad8460_buffer_postdisable,
+};
+
+static const struct iio_info ad8460_info = {
+	.read_raw = &ad8460_read_raw,
+	.write_raw = &ad8460_write_raw,
+	.write_event_value = &ad8460_write_event_value,
+	.read_event_value = &ad8460_read_event_value,
+	.write_event_config = &ad8460_write_event_config,
+	.read_event_config = &ad8460_read_event_config,
+	.debugfs_reg_access = &ad8460_reg_access,
+};
+
+static const struct iio_enum ad8460_powerdown_mode_enum = {
+	.items = ad8460_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad8460_powerdown_modes),
+	.get = ad8460_get_powerdown_mode,
+	.set = ad8460_set_powerdown_mode,
+};
+
+#define AD8460_CHAN_EXT_INFO(_name, _what, _read, _write) {		\
+	.name = _name,							\
+	.read = (_read),						\
+	.write = (_write),						\
+	.private = (_what),						\
+	.shared = IIO_SEPARATE,						\
+}
+
+static struct iio_chan_spec_ext_info ad8460_ext_info[] = {
+	AD8460_CHAN_EXT_INFO("raw0", 0, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw1", 1, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw2", 2, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw3", 3, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw4", 4, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw5", 5, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw6", 6, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw7", 7, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw8", 8, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw9", 9, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw10", 10, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw11", 11, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw12", 12, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw13", 13, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw14", 14, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("raw15", 15, ad8460_dac_input_read,
+			     ad8460_dac_input_write),
+	AD8460_CHAN_EXT_INFO("toggle_en", 0, ad8460_read_toggle_en,
+			     ad8460_write_toggle_en),
+	AD8460_CHAN_EXT_INFO("symbol", 0, ad8460_read_symbol,
+			     ad8460_write_symbol),
+	AD8460_CHAN_EXT_INFO("powerdown", 0, ad8460_read_powerdown,
+			     ad8460_write_powerdown),
+	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad8460_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
+			   &ad8460_powerdown_mode_enum),
+	{}
+};
+
+static const struct iio_event_spec ad8460_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
+#define AD8460_VOLTAGE_CHAN {					\
+	.type = IIO_VOLTAGE,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_SAMP_FREQ) |	\
+			      BIT(IIO_CHAN_INFO_RAW) |		\
+			      BIT(IIO_CHAN_INFO_SCALE),		\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel = 0,						\
+	.scan_index = 0,					\
+	.scan_type = {						\
+		.sign = 'u',					\
+		.realbits = 14,					\
+		.storagebits = 16,				\
+		.endianness = IIO_CPU,				\
+	},                                                      \
+	.ext_info = ad8460_ext_info,                            \
+	.event_spec = ad8460_events,				\
+	.num_event_specs = ARRAY_SIZE(ad8460_events),		\
+}
+
+#define AD8460_CURRENT_CHAN {					\
+	.type = IIO_CURRENT,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel = 0,						\
+	.scan_index = -1,					\
+	.event_spec = ad8460_events,				\
+	.num_event_specs = ARRAY_SIZE(ad8460_events),		\
+}
+
+#define AD8460_TEMP_CHAN {					\
+	.type = IIO_TEMP,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel = 0,						\
+	.scan_index = -1,					\
+	.event_spec = ad8460_events,				\
+	.num_event_specs = 1,					\
+}
+
+static const struct iio_chan_spec ad8460_channels[] = {
+	AD8460_VOLTAGE_CHAN,
+	AD8460_CURRENT_CHAN,
+};
+
+static const struct iio_chan_spec ad8460_channels_with_tmp_adc[] = {
+	AD8460_VOLTAGE_CHAN,
+	AD8460_CURRENT_CHAN,
+	AD8460_TEMP_CHAN,
+};
+
+static const struct regmap_config ad8460_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x7F,
+};
+
+static const char * const ad8460_supplies[] = {
+	"avdd_3p3v", "dvdd_3p3v", "vcc_5v", "hvcc", "hvee", "vref_5v"
+};
+
+static int ad8460_probe(struct spi_device *spi)
+{
+	struct ad8460_state *state;
+	struct iio_dev *indio_dev;
+	struct device *dev;
+	u32 tmp[2], temp;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	state = iio_priv(indio_dev);
+
+	indio_dev->name = "ad8460";
+	indio_dev->info = &ad8460_info;
+
+	state->spi = spi;
+	dev = &spi->dev;
+
+	state->regmap = devm_regmap_init_spi(spi, &ad8460_regmap_config);
+	if (IS_ERR(state->regmap))
+		return dev_err_probe(dev, PTR_ERR(state->regmap),
+				     "Failed to initialize regmap");
+
+	devm_mutex_init(dev, &state->lock);
+
+	state->sync_clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(state->sync_clk))
+		return dev_err_probe(dev, PTR_ERR(state->sync_clk),
+				     "Failed to get sync clk\n");
+
+	state->tmp_adc_channel = devm_iio_channel_get(dev, "ad8460-tmp");
+	if (IS_ERR(state->tmp_adc_channel)) {
+		if (PTR_ERR(state->tmp_adc_channel) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		indio_dev->channels = ad8460_channels;
+		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels);
+	} else {
+		indio_dev->channels = ad8460_channels_with_tmp_adc;
+		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels_with_tmp_adc);
+	}
+
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad8460_supplies),
+					     ad8460_supplies);
+	if (ret) {
+		dev_err(dev, "Failed to enable power supplies\n");
+		return ret;
+	}
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "refio_1p2v");
+	if (ret < 0 && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "Failed to get reference voltage\n");
+
+	state->refio_1p2v_mv = ret == -ENODEV ? 1200 : ret / 1000;
+
+	if (!in_range(state->refio_1p2v_mv, AD8460_MIN_VREFIO_UV / 1000,
+		      AD8460_MAX_VREFIO_UV / 1000))
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid ref voltage range(%u mV) [%u mV, %u mV]\n",
+				     state->refio_1p2v_mv,
+				     AD8460_MIN_VREFIO_UV / 1000,
+				     AD8460_MAX_VREFIO_UV / 1000);
+
+	ret = device_property_read_u32(dev, "adi,external-resistor-ohms",
+				       &state->ext_resistor_ohms);
+	if (ret)
+		state->ext_resistor_ohms = 2000;
+	else if (!in_range(state->ext_resistor_ohms, AD8460_MIN_EXT_RESISTOR_OHMS,
+			   AD8460_MAX_EXT_RESISTOR_OHMS))
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid resistor set range(%u) [%u, %u]\n",
+				     state->ext_resistor_ohms,
+				     AD8460_MIN_EXT_RESISTOR_OHMS,
+				     AD8460_MAX_EXT_RESISTOR_OHMS);
+
+	ret = device_property_read_u32_array(dev, "adi,range-microamp",
+					     tmp, ARRAY_SIZE(tmp));
+	if (!ret) {
+		if (in_range(tmp[1], 0, AD8460_ABS_MAX_OVERCURRENT_UA))
+			regmap_write(state->regmap, AD8460_CTRL_REG(0x08),
+				     FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) |
+				     AD8460_CURRENT_LIMIT_CONV(tmp[1]));
+
+		if (in_range(tmp[0], -AD8460_ABS_MAX_OVERCURRENT_UA, 0))
+			regmap_write(state->regmap, AD8460_CTRL_REG(0x09),
+				     FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) |
+				     AD8460_CURRENT_LIMIT_CONV(abs(tmp[0])));
+	}
+
+	ret = device_property_read_u32_array(dev, "adi,range-microvolt",
+					     tmp, ARRAY_SIZE(tmp));
+	if (!ret) {
+		if (in_range(tmp[1], 0, AD8460_ABS_MAX_OVERVOLTAGE_UV))
+			regmap_write(state->regmap, AD8460_CTRL_REG(0x0A),
+				     FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) |
+				     AD8460_VOLTAGE_LIMIT_CONV(tmp[1]));
+
+		if (in_range(tmp[0], -AD8460_ABS_MAX_OVERVOLTAGE_UV, 0))
+			regmap_write(state->regmap, AD8460_CTRL_REG(0x0B),
+				     FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) |
+				     AD8460_VOLTAGE_LIMIT_CONV(abs(tmp[0])));
+	}
+
+	ret = device_property_read_u32(dev, "adi,max-millicelsius", &temp);
+	if (!ret) {
+		if (in_range(temp, AD8460_MIN_OVERTEMPERATURE_MC,
+			     AD8460_MAX_OVERTEMPERATURE_MC))
+			regmap_write(state->regmap, AD8460_CTRL_REG(0x0C),
+				     FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) |
+				     AD8460_TEMP_LIMIT_CONV(abs(temp)));
+	}
+
+	ret = ad8460_reset(state);
+	if (ret)
+		return ret;
+
+	/* Enables DAC by default */
+	ret = regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x01),
+				 AD8460_HVDAC_SLEEP_MSK,
+				 FIELD_PREP(AD8460_HVDAC_SLEEP_MSK, 0));
+	if (ret)
+		return ret;
+
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->setup_ops = &ad8460_buffer_setup_ops;
+
+	ret = devm_iio_dmaengine_buffer_setup_ext(dev, indio_dev, "tx",
+						  IIO_BUFFER_DIRECTION_OUT);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to get DMA buffer\n");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id ad8460_of_match[] = {
+	{ .compatible = "adi, ad8460" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad8460_of_match);
+
+static struct spi_driver ad8460_driver = {
+	.driver = {
+		.name = "ad8460",
+		.of_match_table = ad8460_of_match,
+	},
+	.probe = ad8460_probe,
+};
+module_spi_driver(ad8460_driver);
+
+MODULE_AUTHOR("Mariel Tinaco <mariel.tinaco@analog.com");
+MODULE_DESCRIPTION("AD8460 DAC driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(IIO_DMAENGINE_BUFFER);
-- 
2.34.1

Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC

[Jonathan Cameron]

On Thu, 12 Sep 2024 17:54:35 +0800
Mariel Tinaco <Mariel.Tinaco@analog.com> wrote:

> The AD8460 is a “bits in, power out” high voltage, high-power,
> high-speed driver optimized for large output current (up to ±1 A)
> and high slew rate (up to ±1800 V/μs) at high voltage (up to ±40 V)
> into capacitive loads.
> 
> A digital engine implements user-configurable features: modes for
> digital input, programmable supply current, and fault monitoring
> and programmable protection settings for output current,
> output voltage, and junction temperature. The AD8460 operates on
> high voltage dual supplies up to ±55 V and a single low voltage
> supply of 5 V.
> 
> Signed-off-by: Mariel Tinaco <Mariel.Tinaco@analog.com>
Hi Mariel

A few minor comments from me.  I'd like it to sit on the list a while
longer, but if there is nothing else I can make minor tweaks whilst
applying.

Jonathan

> diff --git a/drivers/iio/dac/ad8460.c b/drivers/iio/dac/ad8460.c
> new file mode 100644
> index 000000000000..9ce3a0f288ba
> --- /dev/null
> +++ b/drivers/iio/dac/ad8460.c
> @@ -0,0 +1,947 @@


> +static struct iio_chan_spec_ext_info ad8460_ext_info[] = {
> +	AD8460_CHAN_EXT_INFO("raw0", 0, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw1", 1, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw2", 2, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw3", 3, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw4", 4, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw5", 5, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw6", 6, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw7", 7, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw8", 8, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw9", 9, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw10", 10, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw11", 11, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw12", 12, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw13", 13, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw14", 14, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw15", 15, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("toggle_en", 0, ad8460_read_toggle_en,
> +			     ad8460_write_toggle_en),
> +	AD8460_CHAN_EXT_INFO("symbol", 0, ad8460_read_symbol,
> +			     ad8460_write_symbol),
> +	AD8460_CHAN_EXT_INFO("powerdown", 0, ad8460_read_powerdown,
> +			     ad8460_write_powerdown),
> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad8460_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
> +			   &ad8460_powerdown_mode_enum),
> +	{}
	{ }
is my style preference.  Mostly I want consistency and happened to pick this
for IIO.

> +};

> +#define AD8460_TEMP_CHAN {					\
> +	.type = IIO_TEMP,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.output = 1,						\

An output temperature channel?  That's a heater which seems unlikely
on this device.

> +	.indexed = 1,						\
> +	.channel = 0,						\
> +	.scan_index = -1,					\
> +	.event_spec = ad8460_events,				\
> +	.num_event_specs = 1,					\
> +}

> +static int ad8460_probe(struct spi_device *spi)
> +{
> +	struct ad8460_state *state;
> +	struct iio_dev *indio_dev;
> +	struct device *dev;
> +	u32 tmp[2], temp;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	state = iio_priv(indio_dev);
> +
> +	indio_dev->name = "ad8460";
> +	indio_dev->info = &ad8460_info;
> +
> +	state->spi = spi;
> +	dev = &spi->dev;
Might as well do this one where you declare above.
	struct device *dev = &spi->dev;

> +
> +	state->regmap = devm_regmap_init_spi(spi, &ad8460_regmap_config);
> +	if (IS_ERR(state->regmap))
> +		return dev_err_probe(dev, PTR_ERR(state->regmap),
> +				     "Failed to initialize regmap");
> +
> +	devm_mutex_init(dev, &state->lock);

Check return value.  devm registration can potentially fail.

...

> +
> +	/* Enables DAC by default */
> +	ret = regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x01),
> +				 AD8460_HVDAC_SLEEP_MSK,
> +				 FIELD_PREP(AD8460_HVDAC_SLEEP_MSK, 0));

regmap_clear_bits() perhaps.

> +	if (ret)
> +		return ret;
> +
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->setup_ops = &ad8460_buffer_setup_ops;
> +
> +	ret = devm_iio_dmaengine_buffer_setup_ext(dev, indio_dev, "tx",
> +						  IIO_BUFFER_DIRECTION_OUT);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to get DMA buffer\n");
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC

[Christophe JAILLET]

Le 12/09/2024 à 11:54, Mariel Tinaco a écrit :
> The AD8460 is a “bits in, power out” high voltage, high-power,
> high-speed driver optimized for large output current (up to ±1 A)
> and high slew rate (up to ±1800 V/μs) at high voltage (up to ±40 V)
> into capacitive loads.
> 
> A digital engine implements user-configurable features: modes for
> digital input, programmable supply current, and fault monitoring
> and programmable protection settings for output current,
> output voltage, and junction temperature. The AD8460 operates on
> high voltage dual supplies up to ±55 V and a single low voltage
> supply of 5 V.
> 
> Signed-off-by: Mariel Tinaco <Mariel.Tinaco-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> ---

Hi,

...

> +#define AD8460_CHAN_EXT_INFO(_name, _what, _read, _write) {		\
> +	.name = _name,							\
> +	.read = (_read),						\
> +	.write = (_write),						\
> +	.private = (_what),						\

Why () for _read, _write, _what?
(or why no () for _name?)

> +	.shared = IIO_SEPARATE,						\
> +}
> +
> +static struct iio_chan_spec_ext_info ad8460_ext_info[] = {

I think this can be static const struct.

> +	AD8460_CHAN_EXT_INFO("raw0", 0, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw1", 1, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw2", 2, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw3", 3, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw4", 4, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw5", 5, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw6", 6, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw7", 7, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw8", 8, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw9", 9, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw10", 10, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw11", 11, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw12", 12, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw13", 13, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw14", 14, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("raw15", 15, ad8460_dac_input_read,
> +			     ad8460_dac_input_write),
> +	AD8460_CHAN_EXT_INFO("toggle_en", 0, ad8460_read_toggle_en,
> +			     ad8460_write_toggle_en),
> +	AD8460_CHAN_EXT_INFO("symbol", 0, ad8460_read_symbol,
> +			     ad8460_write_symbol),
> +	AD8460_CHAN_EXT_INFO("powerdown", 0, ad8460_read_powerdown,
> +			     ad8460_write_powerdown),
> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad8460_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
> +			   &ad8460_powerdown_mode_enum),
> +	{}
> +};

...

> +static int ad8460_probe(struct spi_device *spi)
> +{
> +	struct ad8460_state *state;
> +	struct iio_dev *indio_dev;
> +	struct device *dev;
> +	u32 tmp[2], temp;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	state = iio_priv(indio_dev);
> +
> +	indio_dev->name = "ad8460";
> +	indio_dev->info = &ad8460_info;
> +
> +	state->spi = spi;
> +	dev = &spi->dev;
> +
> +	state->regmap = devm_regmap_init_spi(spi, &ad8460_regmap_config);
> +	if (IS_ERR(state->regmap))
> +		return dev_err_probe(dev, PTR_ERR(state->regmap),
> +				     "Failed to initialize regmap");
> +
> +	devm_mutex_init(dev, &state->lock);
> +
> +	state->sync_clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(state->sync_clk))
> +		return dev_err_probe(dev, PTR_ERR(state->sync_clk),
> +				     "Failed to get sync clk\n");
> +
> +	state->tmp_adc_channel = devm_iio_channel_get(dev, "ad8460-tmp");
> +	if (IS_ERR(state->tmp_adc_channel)) {
> +		if (PTR_ERR(state->tmp_adc_channel) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		indio_dev->channels = ad8460_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels);
> +	} else {
> +		indio_dev->channels = ad8460_channels_with_tmp_adc;
> +		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels_with_tmp_adc);
> +	}
> +
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad8460_supplies),
> +					     ad8460_supplies);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable power supplies\n");
> +		return ret;

Nitpick: return dev_err_probe() as done in other places?

> +	}
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "refio_1p2v");
> +	if (ret < 0 && ret != -ENODEV)
> +		return dev_err_probe(dev, ret, "Failed to get reference voltage\n");
> +
> +	state->refio_1p2v_mv = ret == -ENODEV ? 1200 : ret / 1000;

...

CJ

Re: [PATCH v4 1/2] dt-bindings: iio: dac: add docs for ad8460

[Krzysztof Kozlowski]

On Thu, Sep 12, 2024 at 05:54:34PM +0800, Mariel Tinaco wrote:
> This adds the bindings documentation for the 14-bit
> High Voltage, High Current, Waveform Generator
> Digital-to-Analog converter.
> 
> Signed-off-by: Mariel Tinaco <Mariel.Tinaco@analog.com>
> ---
>  .../bindings/iio/dac/adi,ad8460.yaml          | 164 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 171 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC

[kernel test robot]

Hi Mariel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on fec496684388685647652ab4213454fbabdab099]

url:    https://github.com/intel-lab-lkp/linux/commits/Mariel-Tinaco/dt-bindings-iio-dac-add-docs-for-ad8460/20240912-175718
base:   fec496684388685647652ab4213454fbabdab099
patch link:    https://lore.kernel.org/r/20240912095435.18639-3-Mariel.Tinaco%40analog.com
patch subject: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC
config: sparc-randconfig-r071-20240921 (https://download.01.org/0day-ci/archive/20240921/202409210849.cRodncgA-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409210849.cRodncgA-lkp@intel.com/

smatch warnings:
drivers/iio/dac/ad8460.c:545 ad8460_write_event_value() warn: unsigned 'fault' is never less than zero.
drivers/iio/dac/ad8460.c:545 ad8460_write_event_value() warn: error code type promoted to positive: 'fault'
drivers/iio/dac/ad8460.c:567 ad8460_read_event_value() warn: unsigned 'fault' is never less than zero.
drivers/iio/dac/ad8460.c:567 ad8460_read_event_value() warn: error code type promoted to positive: 'fault'
drivers/iio/dac/ad8460.c:585 ad8460_write_event_config() warn: unsigned 'fault' is never less than zero.
drivers/iio/dac/ad8460.c:585 ad8460_write_event_config() warn: error code type promoted to positive: 'fault'
drivers/iio/dac/ad8460.c:605 ad8460_read_event_config() warn: unsigned 'fault' is never less than zero.
drivers/iio/dac/ad8460.c:605 ad8460_read_event_config() warn: error code type promoted to positive: 'fault'

vim +/fault +545 drivers/iio/dac/ad8460.c

   528	
   529	static int ad8460_write_event_value(struct iio_dev *indio_dev,
   530					    const struct iio_chan_spec *chan,
   531					    enum iio_event_type type,
   532					    enum iio_event_direction dir,
   533					    enum iio_event_info info, int val, int val2)
   534	{
   535		struct ad8460_state *state = iio_priv(indio_dev);
   536		unsigned int fault;
   537	
   538		if (type != IIO_EV_TYPE_THRESH)
   539			return -EINVAL;
   540	
   541		if (info != IIO_EV_INFO_VALUE)
   542			return -EINVAL;
   543	
   544		fault = ad8460_select_fault_type(chan->type, dir);
 > 545		if (fault < 0)
   546			return fault;
   547	
   548		return ad8460_set_fault_threshold(state, fault, val);
   549	}
   550	
   551	static int ad8460_read_event_value(struct iio_dev *indio_dev,
   552					   const struct iio_chan_spec *chan,
   553					   enum iio_event_type type,
   554					   enum iio_event_direction dir,
   555					   enum iio_event_info info, int *val, int *val2)
   556	{
   557		struct ad8460_state *state = iio_priv(indio_dev);
   558		unsigned int fault;
   559	
   560		if (type != IIO_EV_TYPE_THRESH)
   561			return -EINVAL;
   562	
   563		if (info != IIO_EV_INFO_VALUE)
   564			return -EINVAL;
   565	
   566		fault = ad8460_select_fault_type(chan->type, dir);
 > 567		if (fault < 0)
   568			return fault;
   569	
   570		return ad8460_get_fault_threshold(state, fault, val);
   571	}
   572	
   573	static int ad8460_write_event_config(struct iio_dev *indio_dev,
   574					     const struct iio_chan_spec *chan,
   575					     enum iio_event_type type,
   576					     enum iio_event_direction dir, int val)
   577	{
   578		struct ad8460_state *state = iio_priv(indio_dev);
   579		unsigned int fault;
   580	
   581		if (type != IIO_EV_TYPE_THRESH)
   582			return -EINVAL;
   583	
   584		fault = ad8460_select_fault_type(chan->type, dir);
 > 585		if (fault < 0)
   586			return fault;
   587	
   588		return ad8460_set_fault_threshold_en(state, fault, val);
   589	}
   590	
   591	static int ad8460_read_event_config(struct iio_dev *indio_dev,
   592					    const struct iio_chan_spec *chan,
   593					    enum iio_event_type type,
   594					    enum iio_event_direction dir)
   595	{
   596		struct ad8460_state *state = iio_priv(indio_dev);
   597		unsigned int fault;
   598		bool en;
   599		int ret;
   600	
   601		if (type != IIO_EV_TYPE_THRESH)
   602			return -EINVAL;
   603	
   604		fault = ad8460_select_fault_type(chan->type, dir);
 > 605		if (fault < 0)
   606			return fault;
   607	
   608		ret = ad8460_get_fault_threshold_en(state, fault, &en);
   609		if (ret)
   610			return ret;
   611	
   612		return en;
   613	}
   614	

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

Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC

[Jonathan Cameron]

On Sat, 21 Sep 2024 09:11:12 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi Mariel,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on fec496684388685647652ab4213454fbabdab099]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Mariel-Tinaco/dt-bindings-iio-dac-add-docs-for-ad8460/20240912-175718
> base:   fec496684388685647652ab4213454fbabdab099
> patch link:    https://lore.kernel.org/r/20240912095435.18639-3-Mariel.Tinaco%40analog.com
> patch subject: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC
> config: sparc-randconfig-r071-20240921 (https://download.01.org/0day-ci/archive/20240921/202409210849.cRodncgA-lkp@intel.com/config)
> compiler: sparc64-linux-gcc (GCC) 14.1.0
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202409210849.cRodncgA-lkp@intel.com/
> 
> smatch warnings:
> drivers/iio/dac/ad8460.c:545 ad8460_write_event_value() warn: unsigned 'fault' is never less than zero.
> drivers/iio/dac/ad8460.c:545 ad8460_write_event_value() warn: error code type promoted to positive: 'fault'
> drivers/iio/dac/ad8460.c:567 ad8460_read_event_value() warn: unsigned 'fault' is never less than zero.
> drivers/iio/dac/ad8460.c:567 ad8460_read_event_value() warn: error code type promoted to positive: 'fault'
> drivers/iio/dac/ad8460.c:585 ad8460_write_event_config() warn: unsigned 'fault' is never less than zero.
> drivers/iio/dac/ad8460.c:585 ad8460_write_event_config() warn: error code type promoted to positive: 'fault'
> drivers/iio/dac/ad8460.c:605 ad8460_read_event_config() warn: unsigned 'fault' is never less than zero.
> drivers/iio/dac/ad8460.c:605 ad8460_read_event_config() warn: error code type promoted to positive: 'fault'
> 
> vim +/fault +545 drivers/iio/dac/ad8460.c
> 
>    528	
>    529	static int ad8460_write_event_value(struct iio_dev *indio_dev,
>    530					    const struct iio_chan_spec *chan,
>    531					    enum iio_event_type type,
>    532					    enum iio_event_direction dir,
>    533					    enum iio_event_info info, int val, int val2)
>    534	{
>    535		struct ad8460_state *state = iio_priv(indio_dev);
>    536		unsigned int fault;
I fixed this up by making all the local fault variables int instead.

Jonathan

>    537	
>    538		if (type != IIO_EV_TYPE_THRESH)
>    539			return -EINVAL;
>    540	
>    541		if (info != IIO_EV_INFO_VALUE)
>    542			return -EINVAL;
>    543	
>    544		fault = ad8460_select_fault_type(chan->type, dir);
>  > 545		if (fault < 0)  
>    546			return fault;
>    547	
>    548		return ad8460_set_fault_threshold(state, fault, val);
>    549	}
>    550	
>    551	static int ad8460_read_event_value(struct iio_dev *indio_dev,
>    552					   const struct iio_chan_spec *chan,
>    553					   enum iio_event_type type,
>    554					   enum iio_event_direction dir,
>    555					   enum iio_event_info info, int *val, int *val2)
>    556	{
>    557		struct ad8460_state *state = iio_priv(indio_dev);
>    558		unsigned int fault;
>    559	
>    560		if (type != IIO_EV_TYPE_THRESH)
>    561			return -EINVAL;
>    562	
>    563		if (info != IIO_EV_INFO_VALUE)
>    564			return -EINVAL;
>    565	
>    566		fault = ad8460_select_fault_type(chan->type, dir);
>  > 567		if (fault < 0)  
>    568			return fault;
>    569	
>    570		return ad8460_get_fault_threshold(state, fault, val);
>    571	}
>    572	
>    573	static int ad8460_write_event_config(struct iio_dev *indio_dev,
>    574					     const struct iio_chan_spec *chan,
>    575					     enum iio_event_type type,
>    576					     enum iio_event_direction dir, int val)
>    577	{
>    578		struct ad8460_state *state = iio_priv(indio_dev);
>    579		unsigned int fault;
>    580	
>    581		if (type != IIO_EV_TYPE_THRESH)
>    582			return -EINVAL;
>    583	
>    584		fault = ad8460_select_fault_type(chan->type, dir);
>  > 585		if (fault < 0)  
>    586			return fault;
>    587	
>    588		return ad8460_set_fault_threshold_en(state, fault, val);
>    589	}
>    590	
>    591	static int ad8460_read_event_config(struct iio_dev *indio_dev,
>    592					    const struct iio_chan_spec *chan,
>    593					    enum iio_event_type type,
>    594					    enum iio_event_direction dir)
>    595	{
>    596		struct ad8460_state *state = iio_priv(indio_dev);
>    597		unsigned int fault;
>    598		bool en;
>    599		int ret;
>    600	
>    601		if (type != IIO_EV_TYPE_THRESH)
>    602			return -EINVAL;
>    603	
>    604		fault = ad8460_select_fault_type(chan->type, dir);
>  > 605		if (fault < 0)  
>    606			return fault;
>    607	
>    608		ret = ad8460_get_fault_threshold_en(state, fault, &en);
>    609		if (ret)
>    610			return ret;
>    611	
>    612		return en;
>    613	}
>    614	
> 

Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC

[Jonathan Cameron]

On Sat, 14 Sep 2024 20:21:56 +0200
Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:

> Le 12/09/2024 à 11:54, Mariel Tinaco a écrit :
> > The AD8460 is a “bits in, power out” high voltage, high-power,
> > high-speed driver optimized for large output current (up to ±1 A)
> > and high slew rate (up to ±1800 V/μs) at high voltage (up to ±40 V)
> > into capacitive loads.
> > 
> > A digital engine implements user-configurable features: modes for
> > digital input, programmable supply current, and fault monitoring
> > and programmable protection settings for output current,
> > output voltage, and junction temperature. The AD8460 operates on
> > high voltage dual supplies up to ±55 V and a single low voltage
> > supply of 5 V.
> > 
> > Signed-off-by: Mariel Tinaco <Mariel.Tinaco-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> > ---  

Rather than go around again, I fixed up all the comments made
and the autobuilder issues then applied this.

Diff follows. The only bit I'm not 100% sure on was your intent
with the temperature channel. I've made it an input but shout if
I'm missing something.

With this diff applied on top, applied to the togreg branch of iio.git
which is only pushed out as testing for now as I'll rebase on rc1
once available.

Thanks,

Jonathan

diff --git a/drivers/iio/dac/ad8460.c b/drivers/iio/dac/ad8460.c
index 9ce3a0f288ba..dc8c76ba573d 100644
--- a/drivers/iio/dac/ad8460.c
+++ b/drivers/iio/dac/ad8460.c
@@ -533,7 +533,7 @@ static int ad8460_write_event_value(struct iio_dev *indio_dev,
                                    enum iio_event_info info, int val, int val2)
 {
        struct ad8460_state *state = iio_priv(indio_dev);
-       unsigned int fault;
+       int fault;
 
        if (type != IIO_EV_TYPE_THRESH)
                return -EINVAL;
@@ -555,7 +555,7 @@ static int ad8460_read_event_value(struct iio_dev *indio_dev,
                                   enum iio_event_info info, int *val, int *val2)
 {
        struct ad8460_state *state = iio_priv(indio_dev);
-       unsigned int fault;
+       int fault;
 
        if (type != IIO_EV_TYPE_THRESH)
                return -EINVAL;
@@ -576,7 +576,7 @@ static int ad8460_write_event_config(struct iio_dev *indio_dev,
                                     enum iio_event_direction dir, int val)
 {
        struct ad8460_state *state = iio_priv(indio_dev);
-       unsigned int fault;
+       int fault;
 
        if (type != IIO_EV_TYPE_THRESH)
                return -EINVAL;
@@ -594,9 +594,8 @@ static int ad8460_read_event_config(struct iio_dev *indio_dev,
                                    enum iio_event_direction dir)
 {
        struct ad8460_state *state = iio_priv(indio_dev);
-       unsigned int fault;
+       int fault, ret;
        bool en;
-       int ret;
 
        if (type != IIO_EV_TYPE_THRESH)
                return -EINVAL;
@@ -660,14 +659,14 @@ static const struct iio_enum ad8460_powerdown_mode_enum = {
 };
 
 #define AD8460_CHAN_EXT_INFO(_name, _what, _read, _write) {            \
-       .name = _name,                                                  \
+       .name = (_name),                                                \
        .read = (_read),                                                \
        .write = (_write),                                              \
        .private = (_what),                                             \
        .shared = IIO_SEPARATE,                                         \
 }
 
-static struct iio_chan_spec_ext_info ad8460_ext_info[] = {
+static const struct iio_chan_spec_ext_info ad8460_ext_info[] = {
        AD8460_CHAN_EXT_INFO("raw0", 0, ad8460_dac_input_read,
                             ad8460_dac_input_write),
        AD8460_CHAN_EXT_INFO("raw1", 1, ad8460_dac_input_read,
@@ -709,7 +708,7 @@ static struct iio_chan_spec_ext_info ad8460_ext_info[] = {
        IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad8460_powerdown_mode_enum),
        IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
                           &ad8460_powerdown_mode_enum),
-       {}
+       { }
 };
 
 static const struct iio_event_spec ad8460_events[] = {
@@ -761,7 +760,6 @@ static const struct iio_event_spec ad8460_events[] = {
 #define AD8460_TEMP_CHAN {                                     \
        .type = IIO_TEMP,                                       \
        .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
-       .output = 1,                                            \
        .indexed = 1,                                           \
        .channel = 0,                                           \
        .scan_index = -1,                                       \
@@ -792,9 +790,9 @@ static const char * const ad8460_supplies[] = {
 
 static int ad8460_probe(struct spi_device *spi)
 {
+       struct device *dev  = &spi->dev;
        struct ad8460_state *state;
        struct iio_dev *indio_dev;
-       struct device *dev;
        u32 tmp[2], temp;
        int ret;
 
@@ -808,14 +806,15 @@ static int ad8460_probe(struct spi_device *spi)
        indio_dev->info = &ad8460_info;
 
        state->spi = spi;
-       dev = &spi->dev;
 
        state->regmap = devm_regmap_init_spi(spi, &ad8460_regmap_config);
        if (IS_ERR(state->regmap))
                return dev_err_probe(dev, PTR_ERR(state->regmap),
                                     "Failed to initialize regmap");
 
-       devm_mutex_init(dev, &state->lock);
+       ret = devm_mutex_init(dev, &state->lock);
+       if (ret)
+               return ret;
 
        state->sync_clk = devm_clk_get_enabled(dev, NULL);
        if (IS_ERR(state->sync_clk))
@@ -835,10 +834,9 @@ static int ad8460_probe(struct spi_device *spi)
 
        ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad8460_supplies),
                                             ad8460_supplies);
-       if (ret) {
-               dev_err(dev, "Failed to enable power supplies\n");
-               return ret;
-       }
+       if (ret)
+               return dev_err_probe(dev, ret,
+                                    "Failed to enable power supplies\n");
 
        ret = devm_regulator_get_enable_read_voltage(dev, "refio_1p2v");
        if (ret < 0 && ret != -ENODEV)
@@ -908,9 +906,8 @@ static int ad8460_probe(struct spi_device *spi)
                return ret;
 
        /* Enables DAC by default */
-       ret = regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x01),
-                                AD8460_HVDAC_SLEEP_MSK,
-                                FIELD_PREP(AD8460_HVDAC_SLEEP_MSK, 0));
+       ret = regmap_clear_bits(state->regmap, AD8460_CTRL_REG(0x01),
+                               AD8460_HVDAC_SLEEP_MSK);
        if (ret)
                return ret;
 

> 
> Hi,
> 
> ...
> 
> > +#define AD8460_CHAN_EXT_INFO(_name, _what, _read, _write) {		\
> > +	.name = _name,							\
> > +	.read = (_read),						\
> > +	.write = (_write),						\
> > +	.private = (_what),						\  
> 
> Why () for _read, _write, _what?
> (or why no () for _name?)
> 
> > +	.shared = IIO_SEPARATE,						\
> > +}
> > +
> > +static struct iio_chan_spec_ext_info ad8460_ext_info[] = {  
> 
> I think this can be static const struct.
> 
> > +	AD8460_CHAN_EXT_INFO("raw0", 0, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw1", 1, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw2", 2, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw3", 3, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw4", 4, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw5", 5, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw6", 6, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw7", 7, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw8", 8, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw9", 9, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw10", 10, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw11", 11, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw12", 12, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw13", 13, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw14", 14, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("raw15", 15, ad8460_dac_input_read,
> > +			     ad8460_dac_input_write),
> > +	AD8460_CHAN_EXT_INFO("toggle_en", 0, ad8460_read_toggle_en,
> > +			     ad8460_write_toggle_en),
> > +	AD8460_CHAN_EXT_INFO("symbol", 0, ad8460_read_symbol,
> > +			     ad8460_write_symbol),
> > +	AD8460_CHAN_EXT_INFO("powerdown", 0, ad8460_read_powerdown,
> > +			     ad8460_write_powerdown),
> > +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad8460_powerdown_mode_enum),
> > +	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
> > +			   &ad8460_powerdown_mode_enum),
> > +	{}
> > +};  
> 
> ...
> 
> > +static int ad8460_probe(struct spi_device *spi)
> > +{
> > +	struct ad8460_state *state;
> > +	struct iio_dev *indio_dev;
> > +	struct device *dev;
> > +	u32 tmp[2], temp;
> > +	int ret;
> > +
> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	state = iio_priv(indio_dev);
> > +
> > +	indio_dev->name = "ad8460";
> > +	indio_dev->info = &ad8460_info;
> > +
> > +	state->spi = spi;
> > +	dev = &spi->dev;
> > +
> > +	state->regmap = devm_regmap_init_spi(spi, &ad8460_regmap_config);
> > +	if (IS_ERR(state->regmap))
> > +		return dev_err_probe(dev, PTR_ERR(state->regmap),
> > +				     "Failed to initialize regmap");
> > +
> > +	devm_mutex_init(dev, &state->lock);
> > +
> > +	state->sync_clk = devm_clk_get_enabled(dev, NULL);
> > +	if (IS_ERR(state->sync_clk))
> > +		return dev_err_probe(dev, PTR_ERR(state->sync_clk),
> > +				     "Failed to get sync clk\n");
> > +
> > +	state->tmp_adc_channel = devm_iio_channel_get(dev, "ad8460-tmp");
> > +	if (IS_ERR(state->tmp_adc_channel)) {
> > +		if (PTR_ERR(state->tmp_adc_channel) == -EPROBE_DEFER)
> > +			return -EPROBE_DEFER;
> > +		indio_dev->channels = ad8460_channels;
> > +		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels);
> > +	} else {
> > +		indio_dev->channels = ad8460_channels_with_tmp_adc;
> > +		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels_with_tmp_adc);
> > +	}
> > +
> > +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad8460_supplies),
> > +					     ad8460_supplies);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to enable power supplies\n");
> > +		return ret;  
> 
> Nitpick: return dev_err_probe() as done in other places?
> 
> > +	}
> > +
> > +	ret = devm_regulator_get_enable_read_voltage(dev, "refio_1p2v");
> > +	if (ret < 0 && ret != -ENODEV)
> > +		return dev_err_probe(dev, ret, "Failed to get reference voltage\n");
> > +
> > +	state->refio_1p2v_mv = ret == -ENODEV ? 1200 : ret / 1000;  
> 
> ...
> 
> CJ

RE: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC

[Tinaco, Mariel]

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, September 28, 2024 10:20 PM
> To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Cc: Tinaco, Mariel <Mariel.Tinaco@analog.com>; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Lars-Peter Clausen
> <lars@metafoo.de>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Conor Dooley <conor+dt@kernel.org>;
> Marcelo Schmitt <marcelo.schmitt1@gmail.com>; Dimitri Fedrau
> <dima.fedrau@gmail.com>; David Lechner <dlechner@baylibre.com>; Nuno Sá
> <noname.nuno@gmail.com>
> Subject: Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC
> 
> [External]
> 
> On Sat, 14 Sep 2024 20:21:56 +0200
> Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
> 
> > Le 12/09/2024 à 11:54, Mariel Tinaco a écrit :
> > > The AD8460 is a “bits in, power out” high voltage, high-power,
> > > high-speed driver optimized for large output current (up to ±1 A)
> > > and high slew rate (up to ±1800 V/μs) at high voltage (up to ±40 V)
> > > into capacitive loads.
> > >
> > > A digital engine implements user-configurable features: modes for
> > > digital input, programmable supply current, and fault monitoring and
> > > programmable protection settings for output current, output voltage,
> > > and junction temperature. The AD8460 operates on high voltage dual
> > > supplies up to ±55 V and a single low voltage supply of 5 V.
> > >
> > > Signed-off-by: Mariel Tinaco
> > > <Mariel.Tinaco-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> > > ---
> 
> Rather than go around again, I fixed up all the comments made and the
> autobuilder issues then applied this.
> 
> Diff follows. The only bit I'm not 100% sure on was your intent with the
> temperature channel. I've made it an input but shout if I'm missing something.
> 
> With this diff applied on top, applied to the togreg branch of iio.git which is
> only pushed out as testing for now as I'll rebase on rc1 once available.
> 
> Thanks,
> 
> Jonathan
> 

Hi Jonathan,

Thank you for finding the time to fix up the inline comments from the
previous rounds! I have created a patch for that but was unable to send it
yet because I'm still clueless about the temp channel. Apologies about that

About the temperature channel, it does make sense to set it as input since the
value is read-only. About the implementation of the channel, I'm wondering
what would happen if the consumer-get-channel would throw -EPROBE_DEFER
half the time? Is it not possible to skip it over since the channel is optional
anyway? Or does this defer error from the consumer mean that the other
configurations for the succeeding attributes will be blocked, which is why we
have to return probe instantly?

	state->tmp_adc_channel = devm_iio_channel_get(dev, "ad8460-tmp");
	if (IS_ERR(state->tmp_adc_channel)) {
		state->tmp_adc_channel = NULL;
		indio_dev->channels = ad8460_channels;
		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels);
	} else {
		ret = iio_get_channel_type(state->tmp_adc_channel, &temp);
		if (ret < 0)
			return ret;

		if (temp != IIO_TEMP)
			return dev_err_probe(dev, -EINVAL,
					     "Incompatible channel type %d\n", temp);

		indio_dev->channels = ad8460_channels_with_tmp_adc;
		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels_with_tmp_adc);
	}

I also found other implementations where the type of channel is checked. Thought
That maybe it's a good addition for security.

Thanks,

Mariel

RE: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC

[Tinaco, Mariel]

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, September 28, 2024 10:20 PM
> To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Cc: Tinaco, Mariel <Mariel.Tinaco@analog.com>; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Lars-Peter Clausen
> <lars@metafoo.de>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Conor Dooley <conor+dt@kernel.org>;
> Marcelo Schmitt <marcelo.schmitt1@gmail.com>; Dimitri Fedrau
> <dima.fedrau@gmail.com>; David Lechner <dlechner@baylibre.com>; Nuno Sá
> <noname.nuno@gmail.com>
> Subject: Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC
> 
> [External]
> 
> On Sat, 14 Sep 2024 20:21:56 +0200
> Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
> 
> > Le 12/09/2024 à 11:54, Mariel Tinaco a écrit :
> > > The AD8460 is a “bits in, power out” high voltage, high-power,
> > > high-speed driver optimized for large output current (up to ±1 A)
> > > and high slew rate (up to ±1800 V/μs) at high voltage (up to ±40 V)
> > > into capacitive loads.
> > >
> > > A digital engine implements user-configurable features: modes for
> > > digital input, programmable supply current, and fault monitoring and
> > > programmable protection settings for output current, output voltage,
> > > and junction temperature. The AD8460 operates on high voltage dual
> > > supplies up to ±55 V and a single low voltage supply of 5 V.
> > >
> > > Signed-off-by: Mariel Tinaco
> > > <Mariel.Tinaco-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> > > ---
> 
> Rather than go around again, I fixed up all the comments made and the
> autobuilder issues then applied this.
> 
> Diff follows. The only bit I'm not 100% sure on was your intent with the
> temperature channel. I've made it an input but shout if I'm missing something.
> 
> With this diff applied on top, applied to the togreg branch of iio.git which is
> only pushed out as testing for now as I'll rebase on rc1 once available.
> 
> Thanks,
> 
> Jonathan
> 

Hi Jonathan,

Thank you for finding the time to fix up the inline comments from the
previous rounds! I have created a patch for that but was unable to send it
yet because I'm still clueless about the temp channel. Apologies about that

About the temperature channel, it does make sense to set it as input since the
value is read-only. About the implementation of the channel, I'm wondering
what would happen if the consumer-get-channel would throw -EPROBE_DEFER
half the time? Is it not possible to skip it over since the channel is optional
anyway? Or does this defer error from the consumer mean that the other
configurations for the succeeding attributes will be blocked, which is why we
have to return probe instantly?

Or perhaps we can throw a warning instead if a defer error is thrown?

	state->tmp_adc_channel = devm_iio_channel_get(dev, "ad8460-tmp");
	if (IS_ERR(state->tmp_adc_channel)) {
		state->tmp_adc_channel = NULL;
		indio_dev->channels = ad8460_channels;
		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels);
	} else {
		ret = iio_get_channel_type(state->tmp_adc_channel, &temp);
		if (ret < 0)
			return ret;

		if (temp != IIO_TEMP)
			return dev_err_probe(dev, -EINVAL,
					     "Incompatible channel type %d\n", temp);

		indio_dev->channels = ad8460_channels_with_tmp_adc;
		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels_with_tmp_adc);
	}

I also found other implementations where the type of channel is checked. Thought
That maybe it's a good addition for security.

Thanks,

Mariel

Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC

[Jonathan Cameron]

On Mon, 30 Sep 2024 04:28:09 +0000
"Tinaco, Mariel" <Mariel.Tinaco@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, September 28, 2024 10:20 PM
> > To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > Cc: Tinaco, Mariel <Mariel.Tinaco@analog.com>; linux-iio@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Lars-Peter Clausen
> > <lars@metafoo.de>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> > <krzk+dt@kernel.org>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Conor Dooley <conor+dt@kernel.org>;
> > Marcelo Schmitt <marcelo.schmitt1@gmail.com>; Dimitri Fedrau
> > <dima.fedrau@gmail.com>; David Lechner <dlechner@baylibre.com>; Nuno Sá
> > <noname.nuno@gmail.com>
> > Subject: Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC
> > 
> > [External]
> > 
> > On Sat, 14 Sep 2024 20:21:56 +0200
> > Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
> >   
> > > Le 12/09/2024 à 11:54, Mariel Tinaco a écrit :  
> > > > The AD8460 is a “bits in, power out” high voltage, high-power,
> > > > high-speed driver optimized for large output current (up to ±1 A)
> > > > and high slew rate (up to ±1800 V/μs) at high voltage (up to ±40 V)
> > > > into capacitive loads.
> > > >
> > > > A digital engine implements user-configurable features: modes for
> > > > digital input, programmable supply current, and fault monitoring and
> > > > programmable protection settings for output current, output voltage,
> > > > and junction temperature. The AD8460 operates on high voltage dual
> > > > supplies up to ±55 V and a single low voltage supply of 5 V.
> > > >
> > > > Signed-off-by: Mariel Tinaco
> > > > <Mariel.Tinaco-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> > > > ---  
> > 
> > Rather than go around again, I fixed up all the comments made and the
> > autobuilder issues then applied this.
> > 
> > Diff follows. The only bit I'm not 100% sure on was your intent with the
> > temperature channel. I've made it an input but shout if I'm missing something.
> > 
> > With this diff applied on top, applied to the togreg branch of iio.git which is
> > only pushed out as testing for now as I'll rebase on rc1 once available.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> 
> Hi Jonathan,
> 
> Thank you for finding the time to fix up the inline comments from the
> previous rounds! I have created a patch for that but was unable to send it
> yet because I'm still clueless about the temp channel. Apologies about that
> 
> About the temperature channel, it does make sense to set it as input since the
> value is read-only. 

Input means that we are measuring a signal from the outside world.  As you can
read the temperature it should be an input (doesn't matter that it comes from
elsewhere - in this case an ADC channel is providing the service of reading
that voltage for us).

> About the implementation of the channel, I'm wondering
> what would happen if the consumer-get-channel would throw -EPROBE_DEFER
> half the time?

It will return -EPROBE_DEFER, it then later the driver providing the ADC channel will probe
and we will go around again with it succeeding.  Roughly speaking every
time a driver is successfully bound (probe finishes) the kernel tries again
for any deferred instances.

The only nasty corner is the DT supplies the channel but we don't have the
driver for the ADC built. I'd argue that is a kernel miss configuration
where the right approach is to fix that and provide the ADC driver.


> Is it not possible to skip it over since the channel is optional
> anyway? Or does this defer error from the consumer mean that the other
> configurations for the succeeding attributes will be blocked, which is why we
> have to return probe instantly?

If we don't defer then we never try again and succeed.
We could skip it but that would effectively be so unreliable we would be
better off not providing that feature at all as it will be the source
of lots of bug reports.

> 
> 	state->tmp_adc_channel = devm_iio_channel_get(dev, "ad8460-tmp");
> 	if (IS_ERR(state->tmp_adc_channel)) {
> 		state->tmp_adc_channel = NULL;
> 		indio_dev->channels = ad8460_channels;
> 		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels);
> 	} else {
> 		ret = iio_get_channel_type(state->tmp_adc_channel, &temp);
> 		if (ret < 0)
> 			return ret;
> 
> 		if (temp != IIO_TEMP)
> 			return dev_err_probe(dev, -EINVAL,
> 					     "Incompatible channel type %d\n", temp);
> 
> 		indio_dev->channels = ad8460_channels_with_tmp_adc;
> 		indio_dev->num_channels = ARRAY_SIZE(ad8460_channels_with_tmp_adc);
> 	}
> 
> I also found other implementations where the type of channel is checked. Thought
> That maybe it's a good addition for security.

in this case it would be a DT bug on a very simple binding so I'm not sure we care.
It is also very unlikely to be a temperature channel given we need to read the voltage
from this chips output pin

Jonathan


> 
> Thanks,
> 
> Mariel