* [PATCH 1/3] iio: add adddac subdirectory
@ 2021-10-28 13:48 Cosmin Tanislav
2021-10-28 13:48 ` [PATCH 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Cosmin Tanislav @ 2021-10-28 13:48 UTC (permalink / raw)
Cc: demonsingur, cosmin.tanislav, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring, linux-iio,
devicetree, linux-kernel
For IIO devices that expose both ADC and DAC functionality.
Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
drivers/iio/Kconfig | 1 +
drivers/iio/Makefile | 1 +
drivers/iio/addac/Kconfig | 8 ++++++++
drivers/iio/addac/Makefile | 6 ++++++
4 files changed, 16 insertions(+)
create mode 100644 drivers/iio/addac/Kconfig
create mode 100644 drivers/iio/addac/Makefile
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 2334ad249b46..4fb4321a72cb 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -70,6 +70,7 @@ config IIO_TRIGGERED_EVENT
source "drivers/iio/accel/Kconfig"
source "drivers/iio/adc/Kconfig"
+source "drivers/iio/addac/Kconfig"
source "drivers/iio/afe/Kconfig"
source "drivers/iio/amplifiers/Kconfig"
source "drivers/iio/cdc/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 65e39bd4f934..8d48c70fee4d 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_IIO_TRIGGERED_EVENT) += industrialio-triggered-event.o
obj-y += accel/
obj-y += adc/
+obj-y += addac/
obj-y += afe/
obj-y += amplifiers/
obj-y += buffer/
diff --git a/drivers/iio/addac/Kconfig b/drivers/iio/addac/Kconfig
new file mode 100644
index 000000000000..2e64d7755d5e
--- /dev/null
+++ b/drivers/iio/addac/Kconfig
@@ -0,0 +1,8 @@
+#
+# ADC DAC drivers
+#
+# When adding new entries keep the list in alphabetical order
+
+menu "Analog to digital and digital to analog converters"
+
+endmenu
diff --git a/drivers/iio/addac/Makefile b/drivers/iio/addac/Makefile
new file mode 100644
index 000000000000..b888b9ee12da
--- /dev/null
+++ b/drivers/iio/addac/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for industrial I/O ADDAC drivers
+#
+
+# When adding new entries keep the list in alphabetical order
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] dt-bindings: iio: add AD74413R
2021-10-28 13:48 [PATCH 1/3] iio: add adddac subdirectory Cosmin Tanislav
@ 2021-10-28 13:48 ` Cosmin Tanislav
2021-10-28 14:48 ` Jonathan Cameron
2021-10-28 13:48 ` [PATCH 3/3] iio: addac: add AD74413R driver Cosmin Tanislav
2021-10-28 13:56 ` [PATCH v2 1/3] iio: add adddac subdirectory Cosmin Tanislav
2 siblings, 1 reply; 15+ messages in thread
From: Cosmin Tanislav @ 2021-10-28 13:48 UTC (permalink / raw)
Cc: demonsingur, cosmin.tanislav, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring, linux-iio,
devicetree, linux-kernel
Add device tree bindings for AD74413R.
Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
.../bindings/iio/addac/adi,ad74413r.yaml | 163 ++++++++++++++++++
include/dt-bindings/iio/addac/adi,ad74413r.h | 30 ++++
2 files changed, 193 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
create mode 100644 include/dt-bindings/iio/addac/adi,ad74413r.h
diff --git a/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml b/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
new file mode 100644
index 000000000000..ed4ee3047fbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
@@ -0,0 +1,163 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/addac/adi,ad74413r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD74413R/AD74412R device driver
+
+maintainers:
+ - Cosmin Tanislav <cosmin.tanislav@analog.com>
+
+description: |
+ The AD74413R and AD74412R are quad-channel software configurable input/output
+ solutions for building and process control applications. They contain
+ functionality for analog output, analog input, digital input, resistance
+ temperature detector, and thermocouple measurements integrated
+ into a single chip solution with an SPI interface.
+ The devices feature a 16-bit ADC and four configurable 13-bit DACs to provide
+ four configurable input/output channels and a suite of diagnostic functions.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad74413r
+ - adi,ad74412r
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ spi-max-frequency:
+ maximum: 1000000
+
+ spi-cpol: true
+
+ interrupts:
+ maxItems: 1
+
+ refin-supply:
+ description:
+ Reference voltage regulator.
+
+ adi,rsense-resistance-ohms:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ RSense resistance values in Ohms.
+
+required:
+ - compatible
+ - reg
+ - spi-max-frequency
+ - spi-cpol
+ - refin-supply
+ - adi,rsense-resistance-ohm
+
+additionalProperties: false
+
+patternProperties:
+ "^channel@[0-3]$":
+ type: object
+ description: Represents the external channels which are connected to the device.
+
+ properties:
+ reg:
+ description: |
+ The channel number. It can have up to 4 channels numbered from 0 to 3.
+ maxItems: 1
+ minimum: 0
+ maximum: 3
+
+ adi,ch-func:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Channel function.
+ HART functions are not supported on AD74412R.
+ 0 - CH_FUNC_HIGH_IMPEDANCE
+ 1 - CH_FUNC_VOLTAGE_OUTPUT
+ 2 - CH_FUNC_CURRENT_OUTPUT
+ 3 - CH_FUNC_VOLTAGE_INPUT
+ 4 - CH_FUNC_CURRENT_INPUT_EXT_POWER
+ 5 - CH_FUNC_CURRENT_INPUT_LOOP_POWER
+ 6 - CH_FUNC_RESISTANCE_INPUT
+ 7 - CH_FUNC_DIGITAL_INPUT_LOGIC
+ 8 - CH_FUNC_DIGITAL_INPUT_LOOP_POWER
+ 9 - CH_FUNC_CURRENT_INPUT_EXT_POWER_HART
+ 10 - CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART
+ maxItems: 1
+ minimum: 0
+ maximum: 10
+
+ adi,gpo-config:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ GPO config.
+ 0 - GPO_CONFIG_100K_PULL_DOWN
+ 1 - GPO_CONFIG_LOGIC
+ 3 - GPO_CONFIG_DEBOUNCED_COMPARATOR
+ 4 - GPO_CONFIG_HIGH_IMPEDANCE
+ maxItems: 1
+ enum: [0, 1, 3, 4]
+
+ required:
+ - reg
+
+examples:
+ - |
+ spi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cs-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
+ status = "okay";
+
+ ad74413r@0 {
+ compatible = "adi,ad74413r";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ spi-cpol;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ interrupt-parent = <&gpio>;
+ interrupts = <26 0>;
+
+ refin-supply = <&ad74413r_refin>;
+ adi,rsense-resistance-ohm = <100>;
+
+ channel@0 {
+ reg = <0>;
+
+ adi,ch-func = <CH_FUNC_VOLTAGE_OUTPUT>;
+ adi,gpo-config = <GPO_CONFIG_LOGIC>;
+ };
+
+ channel@1 {
+ reg = <1>;
+
+ adi,ch-func = <CH_FUNC_CURRENT_OUTPUT>;
+ adi,gpo-config = <GPO_CONFIG_LOGIC>;
+ };
+
+ channel@2 {
+ reg = <2>;
+
+ adi,ch-func = <CH_FUNC_VOLTAGE_INPUT>;
+ adi,gpo-config = <GPO_CONFIG_DEBOUNCED_COMPARATOR>;
+ };
+
+ channel@3 {
+ reg = <3>;
+
+ adi,ch-func = <CH_FUNC_CURRENT_INPUT_EXT_POWER>;
+ adi,gpo-config = <GPO_CONFIG_DEBOUNCED_COMPARATOR>;
+ };
+ };
+ };
+...
diff --git a/include/dt-bindings/iio/addac/adi,ad74413r.h b/include/dt-bindings/iio/addac/adi,ad74413r.h
new file mode 100644
index 000000000000..bde558d9731c
--- /dev/null
+++ b/include/dt-bindings/iio/addac/adi,ad74413r.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DT_BINDINGS_ADI_AD74413R_H
+#define _DT_BINDINGS_ADI_AD74413R_H
+
+#define GPO_CONFIG_100K_PULL_DOWN 0x0
+#define GPO_CONFIG_LOGIC 0x1
+#define GPO_CONFIG_LOGIC_PARALLEL 0x2
+#define GPO_CONFIG_DEBOUNCED_COMPARATOR 0x3
+#define GPO_CONFIG_HIGH_IMPEDANCE 0x4
+
+#define GPO_CONFIG_MIN GPO_CONFIG_100K_PULL_DOWN
+#define GPO_CONFIG_MAX GPO_CONFIG_HIGH_IMPEDANCE
+
+#define CH_FUNC_HIGH_IMPEDANCE 0x0
+#define CH_FUNC_VOLTAGE_OUTPUT 0x1
+#define CH_FUNC_CURRENT_OUTPUT 0x2
+#define CH_FUNC_VOLTAGE_INPUT 0x3
+#define CH_FUNC_CURRENT_INPUT_EXT_POWER 0x4
+#define CH_FUNC_CURRENT_INPUT_LOOP_POWER 0x5
+#define CH_FUNC_RESISTANCE_INPUT 0x6
+#define CH_FUNC_DIGITAL_INPUT_LOGIC 0x7
+#define CH_FUNC_DIGITAL_INPUT_LOOP_POWER 0x8
+#define CH_FUNC_CURRENT_INPUT_EXT_POWER_HART 0x9
+#define CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART 0xA
+
+#define CH_FUNC_MIN CH_FUNC_HIGH_IMPEDANCE
+#define CH_FUNC_MAX CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART
+
+#endif /* _DT_BINDINGS_ADI_AD74413R_H */
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] iio: addac: add AD74413R driver
2021-10-28 13:48 [PATCH 1/3] iio: add adddac subdirectory Cosmin Tanislav
2021-10-28 13:48 ` [PATCH 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
@ 2021-10-28 13:48 ` Cosmin Tanislav
2021-10-28 15:58 ` Jonathan Cameron
2021-10-31 22:47 ` kernel test robot
2021-10-28 13:56 ` [PATCH v2 1/3] iio: add adddac subdirectory Cosmin Tanislav
2 siblings, 2 replies; 15+ messages in thread
From: Cosmin Tanislav @ 2021-10-28 13:48 UTC (permalink / raw)
Cc: demonsingur, cosmin.tanislav, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring, linux-iio,
devicetree, linux-kernel
Add IIO ADDAC driver for AD74413R.
Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
MAINTAINERS | 9 +
drivers/iio/addac/Kconfig | 12 +
drivers/iio/addac/Makefile | 1 +
drivers/iio/addac/ad74413r.c | 1389 ++++++++++++++++++++++++++++++++++
4 files changed, 1411 insertions(+)
create mode 100644 drivers/iio/addac/ad74413r.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 37982beaf91f..174f0fd3d71c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1069,6 +1069,15 @@ W: http://ez.analog.com/community/linux-device-drivers
F: Documentation/devicetree/bindings/iio/adc/adi,ad7780.yaml
F: drivers/iio/adc/ad7780.c
+ANALOG DEVICES INC AD74413R DRIVER
+M: Cosmin Tanislav <cosmin.tanislav@analog.com>
+L: linux-iio@vger.kernel.org
+S: Supported
+W: http://ez.analog.com/community/linux-device-drivers
+F: Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
+F: drivers/iio/addac/ad74413r.c
+F: include/dt-bindings/iio/addac/adi,ad74413r.h
+
ANALOG DEVICES INC AD9389B DRIVER
M: Hans Verkuil <hverkuil-cisco@xs4all.nl>
L: linux-media@vger.kernel.org
diff --git a/drivers/iio/addac/Kconfig b/drivers/iio/addac/Kconfig
index 2e64d7755d5e..06b26f8d0144 100644
--- a/drivers/iio/addac/Kconfig
+++ b/drivers/iio/addac/Kconfig
@@ -5,4 +5,16 @@
menu "Analog to digital and digital to analog converters"
+config AD74413R
+ tristate "Analog Devices AD74413R/AD74412R driver"
+ depends on GPIOLIB && SPI
+ select REGMAP_SPI
+ select CRC8
+ help
+ Say yes here to build support for Analog Devices AD74413R/AD74412R
+ quad-channel software configurable input/output solution.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad74413r.
+
endmenu
diff --git a/drivers/iio/addac/Makefile b/drivers/iio/addac/Makefile
index b888b9ee12da..cfd4bbe64ad3 100644
--- a/drivers/iio/addac/Makefile
+++ b/drivers/iio/addac/Makefile
@@ -4,3 +4,4 @@
#
# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AD74413R) += ad74413r.o
diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
new file mode 100644
index 000000000000..76d2e09a17ee
--- /dev/null
+++ b/drivers/iio/addac/ad74413r.c
@@ -0,0 +1,1389 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Analog Devices, Inc.
+ * Author: Cosmin Tanislav <cosmin.tanislav@analog.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/crc8.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <dt-bindings/iio/addac/adi,ad74413r.h>
+
+#define AD74413R_CRC_POLYNOMIAL 0x7
+DECLARE_CRC8_TABLE(ad74413r_crc8_table);
+
+#define AD74413R_CHANNEL_MAX 4
+
+struct ad74413r_config {
+ bool hart_support;
+};
+
+struct ad74413r_channel_config {
+ u32 gpo_config;
+ u32 func;
+ bool initialized;
+};
+
+struct ad74413r_state {
+ struct ad74413r_channel_config channel_configs[AD74413R_CHANNEL_MAX];
+ struct gpio_chip gpiochip;
+ struct mutex lock;
+ struct completion adc_data_completion;
+
+ struct spi_device *spi;
+ struct iio_dev *indio_dev;
+ struct regulator *refin_reg;
+ struct regmap *regmap;
+ struct device *dev;
+ struct iio_trigger *trig;
+ const struct ad74413r_config *config;
+
+ u32 rsense_resistance_ohms;
+
+ size_t adc_active_channels;
+ struct spi_message adc_samples_msg;
+ struct spi_transfer adc_samples_xfer[AD74413R_CHANNEL_MAX + 1];
+ struct {
+ __be32 rx_buf[AD74413R_CHANNEL_MAX];
+ s64 timestamp;
+ } adc_samples ____cacheline_aligned;
+ __be32 adc_samples_tx_buf[AD74413R_CHANNEL_MAX];
+ __be32 regmap_tx_buf;
+ __be32 regmap_rx_buf;
+};
+
+#define AD74413R_REG_NOP 0x00
+
+#define AD74413R_REG_CH_FUNC_SETUP_X(x) (0x01 + (x))
+#define AD74413R_CH_FUNC_SETUP_MASK GENMASK(3, 0)
+
+#define AD74413R_REG_ADC_CONFIG_X(x) (0x05 + (x))
+#define AD74413R_ADC_CONFIG_RANGE_MASK GENMASK(7, 5)
+#define AD74413R_ADC_CONFIG_REJECTION_MASK GENMASK(4, 3)
+#define AD74413R_ADC_RANGE_10V 0b000
+#define AD74413R_ADC_RANGE_2P5V_EXT_POW 0b001
+#define AD74413R_ADC_RANGE_2P5V_INT_POW 0b010
+#define AD74413R_ADC_RANGE_5V_BI_DIR 0b011
+#define AD74413R_ADC_REJECTION_50_60 0b00
+#define AD74413R_ADC_REJECTION_NONE 0b01
+#define AD74413R_ADC_REJECTION_50_60_HART 0b10
+#define AD74413R_ADC_REJECTION_HART 0b11
+
+#define AD74413R_REG_DAC_CODE_X(x) (0x16 + (x))
+#define AD74413R_DAC_CODE_MAX ((1 << 13) - 1)
+
+#define AD74413R_REG_GPO_PAR_DATA 0x0d
+#define AD74413R_REG_GPO_CONFIG_X(x) (0x0e + (x))
+#define AD74413R_GPO_CONFIG_GPO_DATA_MASK BIT(3)
+#define AD74413R_GPO_CONFIG_DATA_LOW 0
+#define AD74413R_GPO_CONFIG_DATA_HIGH AD74413R_GPO_CONFIG_GPO_DATA_MASK
+#define AD74413R_GPO_CONFIG_GPO_SELECT_MASK GENMASK(2, 0)
+
+#define AD74413R_REG_ADC_CONV_CTRL 0x23
+#define AD74413R_CONV_SEQ_MASK GENMASK(9, 8)
+#define AD74413R_CONV_SEQ_ON 0b00
+#define AD74413R_CONV_SEQ_SINGLE 0b01
+#define AD74413R_CONV_SEQ_CONTINUOUS 0b10
+#define AD74413R_CONV_SEQ_OFF 0b11
+#define AD74413R_CH_EN_MASK(x) BIT(x)
+#define AD74413R_CH_EN_SHIFT(x) x
+
+#define AD74413R_REG_DIN_COMP_OUT 0x25
+#define AD74413R_DIN_COMP_OUT_MASK_X(x) BIT(x)
+#define AD74413R_DIN_COMP_OUT_SHIFT_X(x) x
+
+#define AD74413R_REG_ADC_RESULT_X(x) (0x26 + (x))
+#define AD74413R_ADC_RESULT_MAX ((1 << 16) - 1)
+
+#define AD74413R_REG_READ_SELECT 0x41
+
+#define AD74413R_REG_CMD_KEY 0x44
+#define AD74413R_CMD_KEY_LDAC 0x953a
+
+#define AD74413R_REG_SILLICON_REV 0x46
+#define AD74413R_SILLICON_REV_ID_MASK GENMASK(7, 0)
+#define AD74413R_SILLICON_REV_ID_EXPECTED 0x8
+
+#define AD74413R_ADC_DATA_TIMEOUT 1000
+
+static const int ad74413r_adc_sampling_rates[] = {
+ 20,
+ 4800,
+};
+
+static const int ad74413r_adc_sampling_rates_hart[] = {
+ 10,
+ 20,
+ 1200,
+ 4800,
+};
+
+static int ad74413r_crc(u8 *buf)
+{
+ return crc8(ad74413r_crc8_table, buf, 3, 0);
+}
+
+static void ad74413r_format_reg_write(unsigned int reg, unsigned int val, u8 *buf)
+{
+ buf[0] = reg & 0xff;
+ buf[1] = (val >> 8) & 0xff;
+ buf[2] = val & 0xff;
+ buf[3] = ad74413r_crc(buf);
+}
+
+static int ad74413r_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+ struct device *dev = context;
+ struct spi_device *spi = to_spi_device(dev);
+ struct ad74413r_state *st = spi_get_drvdata(spi);
+ u8 *buf = (u8 *)&st->regmap_tx_buf;
+
+ ad74413r_format_reg_write(reg, val, buf);
+
+ return spi_write(spi, buf, 4);
+}
+
+static int ad74413r_crc_check(struct ad74413r_state *st, u8 *buf)
+{
+ u8 expected_crc = ad74413r_crc(buf);
+
+ if (buf[3] != expected_crc) {
+ dev_err(st->dev, "Bad CRC, data: %02x%02x%02x, expected: %02x, received: %02x\n",
+ buf[0], buf[1], buf[2], expected_crc, buf[3]);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad74413r_reg_read_raw(void *context, unsigned int reg, u8 *buf,
+ unsigned int count)
+{
+ struct device *dev = context;
+ struct spi_device *spi = to_spi_device(dev);
+ struct ad74413r_state *st = spi_get_drvdata(spi);
+ unsigned int i;
+ int ret;
+
+ ret = ad74413r_reg_write(context, AD74413R_REG_READ_SELECT, reg);
+ if (ret) {
+ dev_err(dev, "Failed to write read select register: %d\n", ret);
+ return ret;
+ }
+
+ for (i = 0; i < count; i++, buf += 4) {
+ ret = spi_read(spi, buf, 4);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_crc_check(st, buf);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ad74413r_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+ struct device *dev = context;
+ struct spi_device *spi = to_spi_device(dev);
+ struct ad74413r_state *st = spi_get_drvdata(spi);
+ u8 *buf = (u8 *)&st->regmap_rx_buf;
+ int ret;
+
+ ret = ad74413r_reg_read_raw(context, reg, buf, 1);
+ if (ret)
+ return ret;
+
+ *val = (buf[1] << 8) | buf[2];
+
+ return 0;
+}
+
+const struct regmap_config ad74413r_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 16,
+ .reg_read = ad74413r_reg_read,
+ .reg_write = ad74413r_reg_write,
+};
+
+static int ad74413r_set_gpo_mode(struct ad74413r_state *st, unsigned int offset, u8 mode)
+{
+ if (mode < GPO_CONFIG_MIN || mode > GPO_CONFIG_MAX) {
+ dev_err(st->dev, "Invalid gpo config select mode\n");
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(st->regmap, AD74413R_REG_GPO_CONFIG_X(offset),
+ AD74413R_GPO_CONFIG_GPO_SELECT_MASK, mode);
+}
+
+static void ad74413r_gpio_set(struct gpio_chip *chip, unsigned int offset, int val)
+{
+ struct ad74413r_state *st = gpiochip_get_data(chip);
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[offset];
+ int ret;
+
+ if (channel_config->gpo_config != GPO_CONFIG_LOGIC) {
+ dev_err(st->dev, "Cannot set gpo %u, not in logic mode", offset);
+ return;
+ }
+
+ ret = ad74413r_set_gpo_mode(st, offset, GPO_CONFIG_LOGIC);
+ if (ret)
+ return;
+
+ regmap_update_bits(st->regmap, AD74413R_REG_GPO_CONFIG_X(offset),
+ AD74413R_GPO_CONFIG_GPO_DATA_MASK,
+ val ? AD74413R_GPO_CONFIG_DATA_HIGH
+ : AD74413R_GPO_CONFIG_DATA_LOW);
+}
+
+static void ad74413r_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask,
+ unsigned long *bits)
+{
+ struct ad74413r_state *st = gpiochip_get_data(chip);
+ unsigned int offset = 0;
+ int ret;
+
+ for_each_set_bit_from(offset, mask, AD74413R_CHANNEL_MAX) {
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[offset];
+
+ if (channel_config->gpo_config != GPO_CONFIG_LOGIC) {
+ dev_err(st->dev, "Cannot set gpo %u, not in logic mode\n", offset);
+ continue;
+ }
+
+ ret = ad74413r_set_gpo_mode(st, offset, GPO_CONFIG_LOGIC_PARALLEL);
+ if (ret)
+ return;
+ }
+
+ regmap_update_bits(st->regmap, AD74413R_REG_GPO_PAR_DATA, *mask, *bits);
+}
+
+static int ad74413r_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct ad74413r_state *st = gpiochip_get_data(chip);
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[offset];
+ unsigned int status;
+ int ret;
+
+ if (channel_config->gpo_config != GPO_CONFIG_DEBOUNCED_COMPARATOR) {
+ dev_err(st->dev, "Cannot get gpo %u, not in comparator mode\n", offset);
+ return -EINVAL;
+ }
+
+ ret = regmap_read(st->regmap, AD74413R_REG_DIN_COMP_OUT, &status);
+ if (ret)
+ return ret;
+
+ status &= AD74413R_DIN_COMP_OUT_MASK_X(offset);
+ status >>= AD74413R_DIN_COMP_OUT_SHIFT_X(offset);
+
+ return status;
+}
+
+static int ad74413r_set_channel_dac_code(struct ad74413r_state *st, unsigned int channel,
+ uint16_t dac_code)
+{
+ int ret;
+
+ if (dac_code > AD74413R_DAC_CODE_MAX) {
+ dev_err(st->dev, "Invalid dac code %d\n", dac_code);
+ return -EINVAL;
+ }
+
+ ret = regmap_write(st->regmap, AD74413R_REG_DAC_CODE_X(channel), dac_code);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(st->regmap, AD74413R_REG_CMD_KEY, AD74413R_CMD_KEY_LDAC);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int ad74413r_channel_set_function(struct ad74413r_state *st, unsigned int channel, u8 func)
+{
+ if (func < CH_FUNC_MIN || func > CH_FUNC_MAX) {
+ dev_err(st->dev, "Invalid channel function %d\n", func);
+ return -EINVAL;
+ }
+
+ if (!st->config->hart_support && (func == CH_FUNC_CURRENT_INPUT_EXT_POWER_HART
+ || func == CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART)) {
+ dev_err(st->dev, "HART function not supported %d\n", func);
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(st->regmap, AD74413R_REG_CH_FUNC_SETUP_X(channel),
+ AD74413R_CH_FUNC_SETUP_MASK, func);
+}
+
+static int ad74413r_set_adc_conv_seq(struct ad74413r_state *st, unsigned int status)
+{
+ int ret;
+
+ /*
+ * These bits do not clear when a conversion completes.
+ * To enable a subsequent conversion, repeat the write to enable the conversion.
+ */
+ ret = regmap_write_bits(st->regmap, AD74413R_REG_ADC_CONV_CTRL,
+ AD74413R_CONV_SEQ_MASK,
+ FIELD_PREP(AD74413R_CONV_SEQ_MASK, status));
+ if (ret)
+ return ret;
+
+ // Wait 100us before starting conversions
+ usleep_range(100, 120);
+
+ return 0;
+}
+
+static int ad74413r_set_adc_channel_enable(struct ad74413r_state *st, unsigned int channel,
+ bool status)
+{
+ unsigned int val = status << AD74413R_CH_EN_SHIFT(channel);
+
+ return regmap_update_bits(st->regmap, AD74413R_REG_ADC_CONV_CTRL,
+ AD74413R_CH_EN_MASK(channel), val);
+}
+
+static int ad74413r_get_adc_range(struct ad74413r_state *st, unsigned int channel,
+ unsigned int *val)
+{
+ int ret;
+
+ ret = regmap_read(st->regmap, AD74413R_REG_ADC_CONFIG_X(channel), val);
+ if (ret)
+ return ret;
+
+ *val = FIELD_GET(AD74413R_ADC_CONFIG_RANGE_MASK, *val);
+
+ return 0;
+}
+
+static int ad74413r_get_adc_rejection(struct ad74413r_state *st, unsigned int channel,
+ unsigned int *val)
+{
+ int ret;
+
+ ret = regmap_read(st->regmap, AD74413R_REG_ADC_CONFIG_X(channel), val);
+ if (ret)
+ return ret;
+
+ *val = FIELD_GET(AD74413R_ADC_CONFIG_REJECTION_MASK, *val);
+
+ return 0;
+}
+
+static int ad74413r_set_adc_rejection(struct ad74413r_state *st, unsigned int channel,
+ unsigned int val)
+{
+ if (!st->config->hart_support && (val == AD74413R_ADC_REJECTION_50_60_HART
+ || val == AD74413R_ADC_REJECTION_HART)) {
+ dev_err(st->dev, "HART rate not supported %d\n", val);
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(st->regmap, AD74413R_REG_ADC_CONFIG_X(channel),
+ AD74413R_ADC_CONFIG_REJECTION_MASK,
+ FIELD_PREP(AD74413R_ADC_CONFIG_REJECTION_MASK, val));
+}
+
+static int ad74413r_get_adc_rate(struct ad74413r_state *st, unsigned int channel, int *val)
+{
+ unsigned int rej;
+ int ret;
+
+ ret = ad74413r_get_adc_rejection(st, channel, &rej);
+ if (ret)
+ return ret;
+
+ switch (rej) {
+ case AD74413R_ADC_REJECTION_50_60:
+ *val = 20;
+ break;
+ case AD74413R_ADC_REJECTION_NONE:
+ *val = 4800;
+ break;
+ case AD74413R_ADC_REJECTION_50_60_HART:
+ *val = 10;
+ break;
+ case AD74413R_ADC_REJECTION_HART:
+ *val = 1200;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC rejection invalid\n", channel);
+ return -EINVAL;
+ }
+
+ return IIO_VAL_INT;
+}
+
+static int ad74413r_set_adc_rate(struct ad74413r_state *st, unsigned int channel, int val)
+{
+ unsigned int rej;
+
+ switch (val) {
+ case 20:
+ rej = AD74413R_ADC_REJECTION_50_60;
+ break;
+ case 4800:
+ rej = AD74413R_ADC_REJECTION_NONE;
+ break;
+ case 10:
+ rej = AD74413R_ADC_REJECTION_50_60_HART;
+ break;
+ case 1200:
+ rej = AD74413R_ADC_REJECTION_HART;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC rate invalid\n", channel);
+ return -EINVAL;
+ }
+
+ return ad74413r_set_adc_rejection(st, channel, rej);
+}
+
+static int ad74413r_get_adc_voltage_range(struct ad74413r_state *st, unsigned int channel,
+ int *val)
+{
+ unsigned int range;
+ int ret;
+
+ ret = ad74413r_get_adc_range(st, channel, &range);
+ if (ret)
+ return ret;
+
+ switch (range) {
+ case AD74413R_ADC_RANGE_10V:
+ *val = 10000;
+ break;
+ case AD74413R_ADC_RANGE_2P5V_EXT_POW:
+ case AD74413R_ADC_RANGE_2P5V_INT_POW:
+ *val = 2500;
+ break;
+ case AD74413R_ADC_RANGE_5V_BI_DIR:
+ *val = 5000;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC range invalid\n", channel);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad74413r_get_adc_voltage_offset(struct ad74413r_state *st, unsigned int channel,
+ int *val)
+{
+ unsigned int range;
+ int ret;
+
+ ret = ad74413r_get_adc_range(st, channel, &range);
+ if (ret)
+ return ret;
+
+ switch (range) {
+ case AD74413R_ADC_RANGE_10V:
+ case AD74413R_ADC_RANGE_2P5V_EXT_POW:
+ *val = 0;
+ break;
+ case AD74413R_ADC_RANGE_2P5V_INT_POW:
+ case AD74413R_ADC_RANGE_5V_BI_DIR:
+ *val = -2500;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC range invalid\n", channel);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad74413r_get_input_voltage_scale(struct ad74413r_state *st, unsigned int channel,
+ int *val, int *val2)
+{
+ int ret;
+
+ ret = ad74413r_get_adc_voltage_range(st, channel, val);
+ if (ret)
+ return ret;
+
+ *val2 = AD74413R_ADC_RESULT_MAX;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static int ad74413r_get_input_voltage_offset(struct ad74413r_state *st, unsigned int channel,
+ int *val, int *val2)
+{
+ unsigned int range;
+ int ret;
+
+ ret = ad74413r_get_adc_range(st, channel, &range);
+ if (ret)
+ return ret;
+
+ switch (range) {
+ case AD74413R_ADC_RANGE_10V:
+ case AD74413R_ADC_RANGE_2P5V_EXT_POW:
+ *val = 0;
+ break;
+ case AD74413R_ADC_RANGE_2P5V_INT_POW:
+ *val = -AD74413R_ADC_RESULT_MAX;
+ break;
+ case AD74413R_ADC_RANGE_5V_BI_DIR:
+ *val = -AD74413R_ADC_RESULT_MAX / 2;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC range invalid\n", channel);
+ return -EINVAL;
+ }
+
+ *val2 = 0;
+
+ return IIO_VAL_INT;
+}
+
+static int ad74413r_get_input_current_scale(struct ad74413r_state *st, unsigned int channel,
+ int *val, int *val2)
+{
+ int ret;
+
+ ret = ad74413r_get_adc_voltage_range(st, channel, val);
+ if (ret)
+ return ret;
+
+ *val2 = AD74413R_ADC_RESULT_MAX * st->rsense_resistance_ohms;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static int ad74413_get_input_current_offset(struct ad74413r_state *st, unsigned int channel,
+ int *val, int *val2)
+{
+ int range;
+ int offset;
+ int ret;
+
+ ret = ad74413r_get_adc_voltage_range(st, channel, &range);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_get_adc_voltage_offset(st, channel, &offset);
+ if (ret)
+ return ret;
+
+ *val = offset * AD74413R_ADC_RESULT_MAX / range;
+ *val2 = 0;
+
+ return IIO_VAL_INT;
+}
+
+static int ad74413r_get_output_voltage_scale(struct ad74413r_state *st,
+ int *val, int *val2)
+{
+ *val = 11000;
+ *val2 = AD74413R_DAC_CODE_MAX;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static int ad74413r_get_output_current_scale(struct ad74413r_state *st, int *val, int *val2)
+{
+ *val = regulator_get_voltage(st->refin_reg);
+ *val2 = st->rsense_resistance_ohms * AD74413R_DAC_CODE_MAX * 1000;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static irqreturn_t ad74413r_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ unsigned int i;
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ ret = spi_sync(st->spi, &st->adc_samples_msg);
+ if (ret)
+ goto out;
+
+ for (i = 0; i < st->adc_active_channels; i++)
+ ad74413r_crc_check(st, (u8 *)&st->adc_samples.rx_buf[i]);
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &st->adc_samples,
+ iio_get_time_ns(indio_dev));
+
+out:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ mutex_unlock(&st->lock);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t ad74413r_adc_data_interrupt(int irq, void *data)
+{
+ struct ad74413r_state *st = data;
+
+ if (iio_buffer_enabled(st->indio_dev))
+ iio_trigger_poll(st->trig);
+ else
+ complete(&st->adc_data_completion);
+
+ return IRQ_HANDLED;
+}
+
+static int ad74413r_get_single_adc_result(struct ad74413r_state *st, unsigned int channel,
+ int *val)
+{
+ unsigned int uval;
+ int ret;
+
+ ret = iio_device_claim_direct_mode(st->indio_dev);
+ if (ret)
+ return ret;
+
+ reinit_completion(&st->adc_data_completion);
+
+ ret = ad74413r_set_adc_channel_enable(st, channel, true);
+ if (ret)
+ goto out;
+
+ ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_SINGLE);
+ if (ret)
+ goto out;
+
+ ret = wait_for_completion_timeout(&st->adc_data_completion,
+ msecs_to_jiffies(AD74413R_ADC_DATA_TIMEOUT));
+ if (!ret) {
+ ret = -ETIMEDOUT;
+ goto out;
+ }
+
+ ret = regmap_read(st->regmap, AD74413R_REG_ADC_RESULT_X(channel), &uval);
+ if (ret)
+ goto out;
+
+ ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
+ if (ret)
+ goto out;
+
+ ret = ad74413r_set_adc_channel_enable(st, channel, false);
+ if (ret)
+ goto out;
+
+ *val = uval;
+ ret = IIO_VAL_INT;
+
+out:
+ iio_device_release_direct_mode(st->indio_dev);
+
+ return ret;
+}
+
+static int ad74413r_get_single_resistance_result(struct ad74413r_state *st, unsigned int channel,
+ int *val)
+{
+ unsigned int uval;
+ int ret;
+
+ ret = ad74413r_get_single_adc_result(st, channel, &uval);
+ if (ret < 0)
+ return ret;
+
+ if (uval == AD74413R_ADC_RESULT_MAX)
+ *val = 0;
+ else
+ *val = DIV_ROUND_CLOSEST(uval * 2100, AD74413R_ADC_RESULT_MAX - uval);
+
+ return ret;
+}
+
+static int ad74413r_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *active_scan_mask)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ struct spi_transfer *xfer;
+ unsigned int channel;
+ int transfer_index = 0;
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ spi_message_init(&st->adc_samples_msg);
+ st->adc_active_channels = 0;
+
+ for (channel = 0; channel < AD74413R_CHANNEL_MAX; channel++) {
+ bool status = test_bit(channel, active_scan_mask);
+ u8 *tx_buf;
+
+ ret = ad74413r_set_adc_channel_enable(st, channel, status);
+ if (ret)
+ goto out;
+
+ if (!status)
+ continue;
+
+ st->adc_active_channels++;
+
+ xfer = &st->adc_samples_xfer[transfer_index];
+
+ if (transfer_index == 0)
+ xfer->rx_buf = NULL;
+ else
+ xfer->rx_buf = &st->adc_samples.rx_buf[transfer_index - 1];
+
+ tx_buf = (u8 *)&st->adc_samples_tx_buf[transfer_index];
+
+ xfer->tx_buf = tx_buf;
+ xfer->len = 4;
+ xfer->cs_change = 1;
+
+ ad74413r_format_reg_write(AD74413R_REG_READ_SELECT,
+ AD74413R_REG_ADC_RESULT_X(channel),
+ tx_buf);
+
+ spi_message_add_tail(xfer, &st->adc_samples_msg);
+
+ transfer_index++;
+ }
+
+ if (transfer_index == 0)
+ goto out;
+
+ xfer = &st->adc_samples_xfer[transfer_index];
+
+ xfer->tx_buf = NULL;
+ xfer->rx_buf = &st->adc_samples.rx_buf[transfer_index - 1];
+ xfer->len = 4;
+ xfer->cs_change = 0;
+
+ spi_message_add_tail(xfer, &st->adc_samples_msg);
+
+out:
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int ad74413r_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+
+ return ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_CONTINUOUS);
+}
+
+static int ad74413r_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+
+ return ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
+}
+
+static int ad74413r_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ int *val, int *val2, long info)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ int ret = -EINVAL;
+
+ mutex_lock(&st->lock);
+
+ switch (info) {
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_ALTVOLTAGE:
+ if (chan->output)
+ ret = ad74413r_get_output_voltage_scale(st, val, val2);
+ else
+ ret = ad74413r_get_input_voltage_scale(st, chan->channel,
+ val, val2);
+ break;
+
+ case IIO_CURRENT:
+ if (chan->output)
+ ret = ad74413r_get_output_current_scale(st, val, val2);
+ else
+ ret = ad74413r_get_input_current_scale(st, chan->channel,
+ val, val2);
+ break;
+ default:
+ break;
+ }
+ break;
+ case IIO_CHAN_INFO_OFFSET:
+ switch (chan->type) {
+ case IIO_ALTVOLTAGE:
+ if (!chan->output)
+ ret = ad74413r_get_input_voltage_offset(st, chan->channel,
+ val, val2);
+ break;
+ case IIO_CURRENT:
+ if (!chan->output)
+ ret = ad74413_get_input_current_offset(st, chan->channel,
+ val, val2);
+ break;
+ default:
+ break;
+ }
+ break;
+ case IIO_CHAN_INFO_RAW:
+ if (!chan->output)
+ ret = ad74413r_get_single_adc_result(st, chan->channel, val);
+ break;
+ case IIO_CHAN_INFO_PROCESSED:
+ switch (chan->type) {
+ case IIO_RESISTANCE:
+ if (!chan->output)
+ ret = ad74413r_get_single_resistance_result(st, chan->channel,
+ val);
+ break;
+ default:
+ break;
+ }
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (!chan->output)
+ ret = ad74413r_get_adc_rate(st, chan->channel, val);
+ break;
+ default:
+ break;
+ }
+
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int ad74413r_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ int val, int val2, long info)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ int ret = -EINVAL;
+
+ mutex_lock(&st->lock);
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ switch (chan->type) {
+ case IIO_ALTVOLTAGE:
+ case IIO_CURRENT:
+ ret = ad74413r_set_channel_dac_code(st, chan->channel, val);
+ break;
+ default:
+ break;
+ }
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (!chan->output)
+ ret = ad74413r_set_adc_rate(st, chan->channel, val);
+ break;
+ default:
+ break;
+ }
+
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int ad74413r_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length, long info)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ int ret = -EINVAL;
+
+ switch (info) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (st->config->hart_support) {
+ *vals = ad74413r_adc_sampling_rates_hart;
+ *length = ARRAY_SIZE(ad74413r_adc_sampling_rates_hart);
+ } else {
+ *vals = ad74413r_adc_sampling_rates;
+ *length = ARRAY_SIZE(ad74413r_adc_sampling_rates);
+ }
+ *type = IIO_VAL_INT;
+ ret = IIO_AVAIL_LIST;
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+static const struct iio_buffer_setup_ops ad74413r_buffer_ops = {
+ .postenable = &ad74413r_buffer_postenable,
+ .predisable = &ad74413r_buffer_predisable,
+};
+
+static const struct iio_trigger_ops ad74413r_trigger_ops = {
+ .validate_device = iio_trigger_validate_own_device,
+};
+
+static const struct iio_info ad74413r_info = {
+ .read_raw = &ad74413r_read_raw,
+ .write_raw = &ad74413r_write_raw,
+ .read_avail = &ad74413r_read_avail,
+ .update_scan_mode = &ad74413r_update_scan_mode,
+};
+
+#define AD74413R_CHANNEL(_type, _output, extra_mask_separate) \
+ .type = _type, \
+ .indexed = 1, \
+ .output = _output, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | extra_mask_separate
+
+#define AD74413R_DAC_CHANNEL(_type, extra_mask_separate) \
+ { \
+ AD74413R_CHANNEL(_type, 1, extra_mask_separate), \
+ }
+
+#define AD74413R_ADC_CHANNEL(_type, extra_mask_separate) \
+ { \
+ AD74413R_CHANNEL(_type, 0, extra_mask_separate | BIT(IIO_CHAN_INFO_SAMP_FREQ)), \
+ .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 16, \
+ .storagebits = 32, \
+ .shift = 8, \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+#define AD74413R_ADC_VOLTAGE_CHANNEL \
+ AD74413R_ADC_CHANNEL(IIO_ALTVOLTAGE, BIT(IIO_CHAN_INFO_SCALE) \
+ | BIT(IIO_CHAN_INFO_OFFSET))
+
+#define AD74413R_ADC_CURRENT_CHANNEL \
+ AD74413R_ADC_CHANNEL(IIO_CURRENT, BIT(IIO_CHAN_INFO_SCALE) \
+ | BIT(IIO_CHAN_INFO_OFFSET))
+
+static struct iio_chan_spec ad74413r_high_impedance_channels[] = {
+ AD74413R_ADC_VOLTAGE_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_voltage_output_channels[] = {
+ AD74413R_DAC_CHANNEL(IIO_ALTVOLTAGE, BIT(IIO_CHAN_INFO_SCALE)),
+ AD74413R_ADC_CURRENT_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_current_output_channels[] = {
+ AD74413R_DAC_CHANNEL(IIO_CURRENT, BIT(IIO_CHAN_INFO_SCALE)),
+ AD74413R_ADC_VOLTAGE_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_voltage_input_channels[] = {
+ AD74413R_ADC_VOLTAGE_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_current_input_channels[] = {
+ AD74413R_ADC_CURRENT_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_resistance_input_channels[] = {
+ AD74413R_ADC_CHANNEL(IIO_RESISTANCE, BIT(IIO_CHAN_INFO_PROCESSED)),
+};
+
+static struct iio_chan_spec ad74413r_digital_input_channels[] = {
+ AD74413R_ADC_VOLTAGE_CHANNEL,
+};
+
+static int ad74413r_of_parse_channel_config(struct ad74413r_state *st,
+ struct device_node *channel_node)
+{
+ struct ad74413r_channel_config *channel_config;
+ u32 index;
+ int ret;
+
+ ret = of_property_read_u32(channel_node, "reg", &index);
+ if (ret) {
+ dev_err(st->dev, "Failed to read channel reg: %d\n", ret);
+ return ret;
+ }
+
+ if (index > AD74413R_CHANNEL_MAX) {
+ dev_err(st->dev, "Channel index %u is too large\n", index);
+ return -EINVAL;
+ }
+
+ channel_config = &st->channel_configs[index];
+ if (channel_config->initialized) {
+ dev_err(st->dev, "Channel %u has already been initialized\n", index);
+ return -EINVAL;
+ }
+
+ channel_config->func = CH_FUNC_HIGH_IMPEDANCE;
+ of_property_read_u32(channel_node, "adi,ch-func", &channel_config->func);
+
+ channel_config->gpo_config = GPO_CONFIG_100K_PULL_DOWN;
+ of_property_read_u32(channel_node, "adi,gpo-config", &channel_config->gpo_config);
+
+ if (channel_config->gpo_config == GPO_CONFIG_LOGIC_PARALLEL) {
+ dev_err(st->dev, "GPO config logic parallel is unsupported\n");
+ return -EINVAL;
+ }
+
+ ret = ad74413r_channel_set_function(st, index, channel_config->func);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_set_gpo_mode(st, index, channel_config->gpo_config);
+ if (ret)
+ return ret;
+
+ channel_config->initialized = true;
+
+ return 0;
+}
+
+static int ad74413r_parse_channel_configs(struct ad74413r_state *st)
+{
+ struct device_node *channel_node = NULL;
+ int ret;
+
+ for_each_available_child_of_node(st->dev->of_node, channel_node) {
+ if (!of_node_name_eq(channel_node, "channel"))
+ continue;
+
+ ret = ad74413r_of_parse_channel_config(st, channel_node);
+ if (ret) {
+ dev_err(st->dev, "Failed to parse channel %s config: %d\n",
+ channel_node->name, ret);
+ goto put_channel_node;
+ }
+ }
+
+put_channel_node:
+ of_node_put(channel_node);
+
+ return ret;
+}
+
+static int ad74413r_get_iio_channels(struct ad74413r_state *st, u8 func,
+ struct iio_chan_spec **iio_channels, unsigned int *count)
+{
+ switch (func) {
+ case CH_FUNC_HIGH_IMPEDANCE:
+ *iio_channels = ad74413r_high_impedance_channels;
+ *count = ARRAY_SIZE(ad74413r_high_impedance_channels);
+ break;
+ case CH_FUNC_VOLTAGE_OUTPUT:
+ *iio_channels = ad74413r_voltage_output_channels;
+ *count = ARRAY_SIZE(ad74413r_voltage_output_channels);
+ break;
+ case CH_FUNC_CURRENT_OUTPUT:
+ *iio_channels = ad74413r_current_output_channels;
+ *count = ARRAY_SIZE(ad74413r_current_output_channels);
+ break;
+ case CH_FUNC_VOLTAGE_INPUT:
+ *iio_channels = ad74413r_voltage_input_channels;
+ *count = ARRAY_SIZE(ad74413r_voltage_input_channels);
+ break;
+ case CH_FUNC_CURRENT_INPUT_EXT_POWER:
+ case CH_FUNC_CURRENT_INPUT_LOOP_POWER:
+ case CH_FUNC_CURRENT_INPUT_EXT_POWER_HART:
+ case CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART:
+ *iio_channels = ad74413r_current_input_channels;
+ *count = ARRAY_SIZE(ad74413r_current_input_channels);
+ break;
+ case CH_FUNC_RESISTANCE_INPUT:
+ *iio_channels = ad74413r_resistance_input_channels;
+ *count = ARRAY_SIZE(ad74413r_resistance_input_channels);
+ break;
+ case CH_FUNC_DIGITAL_INPUT_LOGIC:
+ case CH_FUNC_DIGITAL_INPUT_LOOP_POWER:
+ *iio_channels = ad74413r_digital_input_channels;
+ *count = ARRAY_SIZE(ad74413r_digital_input_channels);
+ break;
+ default:
+ dev_err(st->dev, "Unhandled channel config function %d\n", func);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad74413r_count_iio_channels(struct ad74413r_state *st)
+{
+ unsigned int index;
+ int ret;
+
+ st->indio_dev->num_channels = 0;
+
+ for (index = 0; index < AD74413R_CHANNEL_MAX; index++) {
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[index];
+ struct iio_chan_spec *local_channels;
+ unsigned int num_local_channels;
+
+ ret = ad74413r_get_iio_channels(st, channel_config->func, &local_channels,
+ &num_local_channels);
+ if (ret)
+ return ret;
+
+ st->indio_dev->num_channels += num_local_channels;
+ }
+
+ return 0;
+}
+
+static int ad74413r_setup_iio_channels(struct ad74413r_state *st)
+{
+ struct iio_chan_spec *channels;
+ unsigned int index;
+ int ret;
+
+ channels = devm_kcalloc(st->dev, sizeof(*channels),
+ st->indio_dev->num_channels, GFP_KERNEL);
+ if (!channels)
+ return -ENOMEM;
+
+ st->indio_dev->channels = channels;
+
+ for (index = 0; index < AD74413R_CHANNEL_MAX; index++) {
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[index];
+ struct iio_chan_spec *local_channels;
+ unsigned int num_local_channels;
+ unsigned int chan_index;
+
+ ret = ad74413r_get_iio_channels(st, channel_config->func, &local_channels,
+ &num_local_channels);
+ if (ret)
+ return ret;
+
+ memcpy(channels, local_channels, num_local_channels * sizeof(*local_channels));
+
+ for (chan_index = 0; chan_index < num_local_channels; chan_index++) {
+ struct iio_chan_spec *local_channel = &channels[chan_index];
+
+ local_channel->channel = index;
+ if (local_channel->output)
+ local_channel->scan_index = -1;
+ else
+ local_channel->scan_index = index;
+
+ }
+
+ channels += num_local_channels;
+ }
+
+ return 0;
+}
+
+static void ad74413r_regulator_disable(void *regulator)
+{
+ regulator_disable(regulator);
+}
+
+static int ad74413r_probe(struct spi_device *spi)
+{
+ struct ad74413r_state *st;
+ struct iio_dev *indio_dev;
+ unsigned int sillicon_rev_id;
+ const char *name;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (indio_dev == NULL)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ spi_set_drvdata(spi, st);
+
+ st->spi = spi;
+ st->dev = &spi->dev;
+ st->config = of_device_get_match_data(&spi->dev);
+ st->indio_dev = indio_dev;
+ mutex_init(&st->lock);
+ init_completion(&st->adc_data_completion);
+
+ name = dev_name(st->dev);
+
+ st->regmap = devm_regmap_init(st->dev, NULL, &spi->dev, &ad74413r_regmap_config);
+ if (IS_ERR(st->regmap)) {
+ ret = PTR_ERR(st->regmap);
+ dev_err(st->dev, "Failed to create regmap: %d\n", ret);
+ return ret;
+ }
+
+ ret = regmap_read(st->regmap, AD74413R_REG_SILLICON_REV, &sillicon_rev_id);
+ if (ret) {
+ dev_err(st->dev, "Failed to read sillicon rev: %d\n", ret);
+ return ret;
+ }
+
+ sillicon_rev_id = FIELD_GET(AD74413R_SILLICON_REV_ID_MASK, sillicon_rev_id);
+ if (sillicon_rev_id != AD74413R_SILLICON_REV_ID_EXPECTED) {
+ dev_err(st->dev, "Read sillicon rev id %d does not match expected rev id %d\n",
+ sillicon_rev_id, AD74413R_SILLICON_REV_ID_EXPECTED);
+ return -EINVAL;
+ }
+
+ st->refin_reg = devm_regulator_get(st->dev, "refin");
+ if (IS_ERR(st->refin_reg)) {
+ dev_err(st->dev, "Failed to get refin regulator: %d\n", ret);
+ return PTR_ERR(st->refin_reg);
+ }
+
+ ret = regulator_enable(st->refin_reg);
+ if (ret) {
+ dev_err(st->dev, "Failed to enable refin regulator: %d\n", ret);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(st->dev, ad74413r_regulator_disable,
+ st->refin_reg);
+ if (ret) {
+ dev_err(st->dev, "Failed to add refin regulator disable action: %d\n", ret);
+ return ret;
+ }
+
+ ret = of_property_read_u32(st->dev->of_node, "adi,rsense-resistance-ohms",
+ &st->rsense_resistance_ohms);
+ if (ret) {
+ dev_err(st->dev, "Failed to get rsense resistance: %d\n", ret);
+ return ret;
+ }
+
+ st->gpiochip.label = name;
+ st->gpiochip.base = -1;
+ st->gpiochip.ngpio = AD74413R_CHANNEL_MAX;
+ st->gpiochip.parent = st->dev;
+ st->gpiochip.can_sleep = true;
+ st->gpiochip.set = ad74413r_gpio_set;
+ st->gpiochip.set_multiple = ad74413r_gpio_set_multiple;
+ st->gpiochip.owner = THIS_MODULE;
+ st->gpiochip.get = ad74413r_gpio_get;
+
+ ret = devm_gpiochip_add_data(st->dev, &st->gpiochip, st);
+ if (ret) {
+ dev_err(st->dev, "Failed to add gpio chip: %d\n", ret);
+ return ret;
+ }
+
+ st->trig = devm_iio_trigger_alloc(st->dev, "%s-dev%d", name, indio_dev->id);
+ if (!st->trig)
+ return -ENOMEM;
+
+ st->trig->ops = &ad74413r_trigger_ops;
+ st->trig->dev.parent = st->dev;
+ iio_trigger_set_drvdata(st->trig, st);
+
+ ret = devm_iio_trigger_register(st->dev, st->trig);
+ if (ret)
+ return ret;
+
+ indio_dev->dev.parent = st->dev;
+ indio_dev->name = name;
+ indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
+ indio_dev->info = &ad74413r_info;
+ indio_dev->trig = iio_trigger_get(st->trig);
+
+ ret = ad74413r_parse_channel_configs(st);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_count_iio_channels(st);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_setup_iio_channels(st);
+ if (ret) {
+ dev_err(st->dev, "Failed to setup iio channels: %d\n", ret);
+ return ret;
+ }
+
+ ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
+ if (ret)
+ return ret;
+
+ ret = devm_request_irq(st->dev, spi->irq, ad74413r_adc_data_interrupt,
+ IRQF_TRIGGER_FALLING, name, st);
+ if (ret) {
+ dev_err(st->dev, "Failed to request threaded irq: %d\n", ret);
+ return ret;
+ }
+
+ ret = devm_iio_triggered_buffer_setup(st->dev, indio_dev, &iio_pollfunc_store_time,
+ &ad74413r_trigger_handler, &ad74413r_buffer_ops);
+ if (ret) {
+ dev_err(st->dev, "Failed to setup triggered buffer: %d\n", ret);
+ return ret;
+ }
+
+ return devm_iio_device_register(st->dev, indio_dev);
+}
+
+static int ad74413r_unregister_driver(struct spi_driver *spi)
+{
+ spi_unregister_driver(spi);
+
+ return 0;
+}
+
+static int __init ad74413r_register_driver(struct spi_driver *spi)
+{
+ crc8_populate_msb(ad74413r_crc8_table, AD74413R_CRC_POLYNOMIAL);
+
+ return spi_register_driver(spi);
+}
+
+static const struct ad74413r_config ad74412r_config_data = {
+ .hart_support = false,
+};
+
+static const struct ad74413r_config ad74413r_config_data = {
+ .hart_support = true,
+};
+
+static const struct of_device_id ad74413r_dt_id[] = {
+ {
+ .compatible = "adi,ad74412r",
+ .data = &ad74412r_config_data,
+ },
+ {
+ .compatible = "adi,ad74413r",
+ .data = &ad74413r_config_data,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ad74413r_dt_id);
+
+static struct spi_driver ad74413r_driver = {
+ .driver = {
+ .name = "ad74413r",
+ .of_match_table = ad74413r_dt_id,
+ },
+ .probe = ad74413r_probe,
+};
+
+module_driver(ad74413r_driver, ad74413r_register_driver, ad74413r_unregister_driver);
+
+MODULE_AUTHOR("Cosmin Tanislav <cosmin.tanislav@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD74413R ADDAC");
+MODULE_LICENSE("GPL v2");
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] iio: add adddac subdirectory
2021-10-28 13:48 [PATCH 1/3] iio: add adddac subdirectory Cosmin Tanislav
2021-10-28 13:48 ` [PATCH 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
2021-10-28 13:48 ` [PATCH 3/3] iio: addac: add AD74413R driver Cosmin Tanislav
@ 2021-10-28 13:56 ` Cosmin Tanislav
2021-10-28 13:56 ` [PATCH v2 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
` (3 more replies)
2 siblings, 4 replies; 15+ messages in thread
From: Cosmin Tanislav @ 2021-10-28 13:56 UTC (permalink / raw)
Cc: demonsingur, cosmin.tanislav, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring, linux-iio,
devicetree, linux-kernel
From: Cosmin Tanislav <cosmin.tanislav@analog.com>
For IIO devices that expose both ADC and DAC functionality.
Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
drivers/iio/Kconfig | 1 +
drivers/iio/Makefile | 1 +
drivers/iio/addac/Kconfig | 8 ++++++++
drivers/iio/addac/Makefile | 6 ++++++
4 files changed, 16 insertions(+)
create mode 100644 drivers/iio/addac/Kconfig
create mode 100644 drivers/iio/addac/Makefile
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 2334ad249b46..4fb4321a72cb 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -70,6 +70,7 @@ config IIO_TRIGGERED_EVENT
source "drivers/iio/accel/Kconfig"
source "drivers/iio/adc/Kconfig"
+source "drivers/iio/addac/Kconfig"
source "drivers/iio/afe/Kconfig"
source "drivers/iio/amplifiers/Kconfig"
source "drivers/iio/cdc/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 65e39bd4f934..8d48c70fee4d 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_IIO_TRIGGERED_EVENT) += industrialio-triggered-event.o
obj-y += accel/
obj-y += adc/
+obj-y += addac/
obj-y += afe/
obj-y += amplifiers/
obj-y += buffer/
diff --git a/drivers/iio/addac/Kconfig b/drivers/iio/addac/Kconfig
new file mode 100644
index 000000000000..2e64d7755d5e
--- /dev/null
+++ b/drivers/iio/addac/Kconfig
@@ -0,0 +1,8 @@
+#
+# ADC DAC drivers
+#
+# When adding new entries keep the list in alphabetical order
+
+menu "Analog to digital and digital to analog converters"
+
+endmenu
diff --git a/drivers/iio/addac/Makefile b/drivers/iio/addac/Makefile
new file mode 100644
index 000000000000..b888b9ee12da
--- /dev/null
+++ b/drivers/iio/addac/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for industrial I/O ADDAC drivers
+#
+
+# When adding new entries keep the list in alphabetical order
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] dt-bindings: iio: add AD74413R
2021-10-28 13:56 ` [PATCH v2 1/3] iio: add adddac subdirectory Cosmin Tanislav
@ 2021-10-28 13:56 ` Cosmin Tanislav
2021-10-28 19:08 ` Rob Herring
2021-10-28 13:56 ` [PATCH v2 3/3] iio: addac: add AD74413R driver Cosmin Tanislav
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Cosmin Tanislav @ 2021-10-28 13:56 UTC (permalink / raw)
Cc: demonsingur, cosmin.tanislav, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring, linux-iio,
devicetree, linux-kernel
From: Cosmin Tanislav <cosmin.tanislav@analog.com>
Add device tree bindings for AD74413R.
Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
.../bindings/iio/addac/adi,ad74413r.yaml | 163 ++++++++++++++++++
include/dt-bindings/iio/addac/adi,ad74413r.h | 30 ++++
2 files changed, 193 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
create mode 100644 include/dt-bindings/iio/addac/adi,ad74413r.h
diff --git a/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml b/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
new file mode 100644
index 000000000000..ed4ee3047fbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
@@ -0,0 +1,163 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/addac/adi,ad74413r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD74413R/AD74412R device driver
+
+maintainers:
+ - Cosmin Tanislav <cosmin.tanislav@analog.com>
+
+description: |
+ The AD74413R and AD74412R are quad-channel software configurable input/output
+ solutions for building and process control applications. They contain
+ functionality for analog output, analog input, digital input, resistance
+ temperature detector, and thermocouple measurements integrated
+ into a single chip solution with an SPI interface.
+ The devices feature a 16-bit ADC and four configurable 13-bit DACs to provide
+ four configurable input/output channels and a suite of diagnostic functions.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad74413r
+ - adi,ad74412r
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ spi-max-frequency:
+ maximum: 1000000
+
+ spi-cpol: true
+
+ interrupts:
+ maxItems: 1
+
+ refin-supply:
+ description:
+ Reference voltage regulator.
+
+ adi,rsense-resistance-ohms:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ RSense resistance values in Ohms.
+
+required:
+ - compatible
+ - reg
+ - spi-max-frequency
+ - spi-cpol
+ - refin-supply
+ - adi,rsense-resistance-ohm
+
+additionalProperties: false
+
+patternProperties:
+ "^channel@[0-3]$":
+ type: object
+ description: Represents the external channels which are connected to the device.
+
+ properties:
+ reg:
+ description: |
+ The channel number. It can have up to 4 channels numbered from 0 to 3.
+ maxItems: 1
+ minimum: 0
+ maximum: 3
+
+ adi,ch-func:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Channel function.
+ HART functions are not supported on AD74412R.
+ 0 - CH_FUNC_HIGH_IMPEDANCE
+ 1 - CH_FUNC_VOLTAGE_OUTPUT
+ 2 - CH_FUNC_CURRENT_OUTPUT
+ 3 - CH_FUNC_VOLTAGE_INPUT
+ 4 - CH_FUNC_CURRENT_INPUT_EXT_POWER
+ 5 - CH_FUNC_CURRENT_INPUT_LOOP_POWER
+ 6 - CH_FUNC_RESISTANCE_INPUT
+ 7 - CH_FUNC_DIGITAL_INPUT_LOGIC
+ 8 - CH_FUNC_DIGITAL_INPUT_LOOP_POWER
+ 9 - CH_FUNC_CURRENT_INPUT_EXT_POWER_HART
+ 10 - CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART
+ maxItems: 1
+ minimum: 0
+ maximum: 10
+
+ adi,gpo-config:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ GPO config.
+ 0 - GPO_CONFIG_100K_PULL_DOWN
+ 1 - GPO_CONFIG_LOGIC
+ 3 - GPO_CONFIG_DEBOUNCED_COMPARATOR
+ 4 - GPO_CONFIG_HIGH_IMPEDANCE
+ maxItems: 1
+ enum: [0, 1, 3, 4]
+
+ required:
+ - reg
+
+examples:
+ - |
+ spi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cs-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
+ status = "okay";
+
+ ad74413r@0 {
+ compatible = "adi,ad74413r";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ spi-cpol;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ interrupt-parent = <&gpio>;
+ interrupts = <26 0>;
+
+ refin-supply = <&ad74413r_refin>;
+ adi,rsense-resistance-ohm = <100>;
+
+ channel@0 {
+ reg = <0>;
+
+ adi,ch-func = <CH_FUNC_VOLTAGE_OUTPUT>;
+ adi,gpo-config = <GPO_CONFIG_LOGIC>;
+ };
+
+ channel@1 {
+ reg = <1>;
+
+ adi,ch-func = <CH_FUNC_CURRENT_OUTPUT>;
+ adi,gpo-config = <GPO_CONFIG_LOGIC>;
+ };
+
+ channel@2 {
+ reg = <2>;
+
+ adi,ch-func = <CH_FUNC_VOLTAGE_INPUT>;
+ adi,gpo-config = <GPO_CONFIG_DEBOUNCED_COMPARATOR>;
+ };
+
+ channel@3 {
+ reg = <3>;
+
+ adi,ch-func = <CH_FUNC_CURRENT_INPUT_EXT_POWER>;
+ adi,gpo-config = <GPO_CONFIG_DEBOUNCED_COMPARATOR>;
+ };
+ };
+ };
+...
diff --git a/include/dt-bindings/iio/addac/adi,ad74413r.h b/include/dt-bindings/iio/addac/adi,ad74413r.h
new file mode 100644
index 000000000000..bde558d9731c
--- /dev/null
+++ b/include/dt-bindings/iio/addac/adi,ad74413r.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DT_BINDINGS_ADI_AD74413R_H
+#define _DT_BINDINGS_ADI_AD74413R_H
+
+#define GPO_CONFIG_100K_PULL_DOWN 0x0
+#define GPO_CONFIG_LOGIC 0x1
+#define GPO_CONFIG_LOGIC_PARALLEL 0x2
+#define GPO_CONFIG_DEBOUNCED_COMPARATOR 0x3
+#define GPO_CONFIG_HIGH_IMPEDANCE 0x4
+
+#define GPO_CONFIG_MIN GPO_CONFIG_100K_PULL_DOWN
+#define GPO_CONFIG_MAX GPO_CONFIG_HIGH_IMPEDANCE
+
+#define CH_FUNC_HIGH_IMPEDANCE 0x0
+#define CH_FUNC_VOLTAGE_OUTPUT 0x1
+#define CH_FUNC_CURRENT_OUTPUT 0x2
+#define CH_FUNC_VOLTAGE_INPUT 0x3
+#define CH_FUNC_CURRENT_INPUT_EXT_POWER 0x4
+#define CH_FUNC_CURRENT_INPUT_LOOP_POWER 0x5
+#define CH_FUNC_RESISTANCE_INPUT 0x6
+#define CH_FUNC_DIGITAL_INPUT_LOGIC 0x7
+#define CH_FUNC_DIGITAL_INPUT_LOOP_POWER 0x8
+#define CH_FUNC_CURRENT_INPUT_EXT_POWER_HART 0x9
+#define CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART 0xA
+
+#define CH_FUNC_MIN CH_FUNC_HIGH_IMPEDANCE
+#define CH_FUNC_MAX CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART
+
+#endif /* _DT_BINDINGS_ADI_AD74413R_H */
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] iio: addac: add AD74413R driver
2021-10-28 13:56 ` [PATCH v2 1/3] iio: add adddac subdirectory Cosmin Tanislav
2021-10-28 13:56 ` [PATCH v2 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
@ 2021-10-28 13:56 ` Cosmin Tanislav
2021-10-28 16:02 ` [PATCH v2 1/3] iio: add adddac subdirectory Jonathan Cameron
2021-10-29 8:00 ` Sa, Nuno
3 siblings, 0 replies; 15+ messages in thread
From: Cosmin Tanislav @ 2021-10-28 13:56 UTC (permalink / raw)
Cc: demonsingur, cosmin.tanislav, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring, linux-iio,
devicetree, linux-kernel
From: Cosmin Tanislav <cosmin.tanislav@analog.com>
Add IIO ADDAC driver for AD74413R.
Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
MAINTAINERS | 9 +
drivers/iio/addac/Kconfig | 12 +
drivers/iio/addac/Makefile | 1 +
drivers/iio/addac/ad74413r.c | 1389 ++++++++++++++++++++++++++++++++++
4 files changed, 1411 insertions(+)
create mode 100644 drivers/iio/addac/ad74413r.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 37982beaf91f..174f0fd3d71c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1069,6 +1069,15 @@ W: http://ez.analog.com/community/linux-device-drivers
F: Documentation/devicetree/bindings/iio/adc/adi,ad7780.yaml
F: drivers/iio/adc/ad7780.c
+ANALOG DEVICES INC AD74413R DRIVER
+M: Cosmin Tanislav <cosmin.tanislav@analog.com>
+L: linux-iio@vger.kernel.org
+S: Supported
+W: http://ez.analog.com/community/linux-device-drivers
+F: Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
+F: drivers/iio/addac/ad74413r.c
+F: include/dt-bindings/iio/addac/adi,ad74413r.h
+
ANALOG DEVICES INC AD9389B DRIVER
M: Hans Verkuil <hverkuil-cisco@xs4all.nl>
L: linux-media@vger.kernel.org
diff --git a/drivers/iio/addac/Kconfig b/drivers/iio/addac/Kconfig
index 2e64d7755d5e..06b26f8d0144 100644
--- a/drivers/iio/addac/Kconfig
+++ b/drivers/iio/addac/Kconfig
@@ -5,4 +5,16 @@
menu "Analog to digital and digital to analog converters"
+config AD74413R
+ tristate "Analog Devices AD74413R/AD74412R driver"
+ depends on GPIOLIB && SPI
+ select REGMAP_SPI
+ select CRC8
+ help
+ Say yes here to build support for Analog Devices AD74413R/AD74412R
+ quad-channel software configurable input/output solution.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad74413r.
+
endmenu
diff --git a/drivers/iio/addac/Makefile b/drivers/iio/addac/Makefile
index b888b9ee12da..cfd4bbe64ad3 100644
--- a/drivers/iio/addac/Makefile
+++ b/drivers/iio/addac/Makefile
@@ -4,3 +4,4 @@
#
# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AD74413R) += ad74413r.o
diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
new file mode 100644
index 000000000000..76d2e09a17ee
--- /dev/null
+++ b/drivers/iio/addac/ad74413r.c
@@ -0,0 +1,1389 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Analog Devices, Inc.
+ * Author: Cosmin Tanislav <cosmin.tanislav@analog.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/crc8.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <dt-bindings/iio/addac/adi,ad74413r.h>
+
+#define AD74413R_CRC_POLYNOMIAL 0x7
+DECLARE_CRC8_TABLE(ad74413r_crc8_table);
+
+#define AD74413R_CHANNEL_MAX 4
+
+struct ad74413r_config {
+ bool hart_support;
+};
+
+struct ad74413r_channel_config {
+ u32 gpo_config;
+ u32 func;
+ bool initialized;
+};
+
+struct ad74413r_state {
+ struct ad74413r_channel_config channel_configs[AD74413R_CHANNEL_MAX];
+ struct gpio_chip gpiochip;
+ struct mutex lock;
+ struct completion adc_data_completion;
+
+ struct spi_device *spi;
+ struct iio_dev *indio_dev;
+ struct regulator *refin_reg;
+ struct regmap *regmap;
+ struct device *dev;
+ struct iio_trigger *trig;
+ const struct ad74413r_config *config;
+
+ u32 rsense_resistance_ohms;
+
+ size_t adc_active_channels;
+ struct spi_message adc_samples_msg;
+ struct spi_transfer adc_samples_xfer[AD74413R_CHANNEL_MAX + 1];
+ struct {
+ __be32 rx_buf[AD74413R_CHANNEL_MAX];
+ s64 timestamp;
+ } adc_samples ____cacheline_aligned;
+ __be32 adc_samples_tx_buf[AD74413R_CHANNEL_MAX];
+ __be32 regmap_tx_buf;
+ __be32 regmap_rx_buf;
+};
+
+#define AD74413R_REG_NOP 0x00
+
+#define AD74413R_REG_CH_FUNC_SETUP_X(x) (0x01 + (x))
+#define AD74413R_CH_FUNC_SETUP_MASK GENMASK(3, 0)
+
+#define AD74413R_REG_ADC_CONFIG_X(x) (0x05 + (x))
+#define AD74413R_ADC_CONFIG_RANGE_MASK GENMASK(7, 5)
+#define AD74413R_ADC_CONFIG_REJECTION_MASK GENMASK(4, 3)
+#define AD74413R_ADC_RANGE_10V 0b000
+#define AD74413R_ADC_RANGE_2P5V_EXT_POW 0b001
+#define AD74413R_ADC_RANGE_2P5V_INT_POW 0b010
+#define AD74413R_ADC_RANGE_5V_BI_DIR 0b011
+#define AD74413R_ADC_REJECTION_50_60 0b00
+#define AD74413R_ADC_REJECTION_NONE 0b01
+#define AD74413R_ADC_REJECTION_50_60_HART 0b10
+#define AD74413R_ADC_REJECTION_HART 0b11
+
+#define AD74413R_REG_DAC_CODE_X(x) (0x16 + (x))
+#define AD74413R_DAC_CODE_MAX ((1 << 13) - 1)
+
+#define AD74413R_REG_GPO_PAR_DATA 0x0d
+#define AD74413R_REG_GPO_CONFIG_X(x) (0x0e + (x))
+#define AD74413R_GPO_CONFIG_GPO_DATA_MASK BIT(3)
+#define AD74413R_GPO_CONFIG_DATA_LOW 0
+#define AD74413R_GPO_CONFIG_DATA_HIGH AD74413R_GPO_CONFIG_GPO_DATA_MASK
+#define AD74413R_GPO_CONFIG_GPO_SELECT_MASK GENMASK(2, 0)
+
+#define AD74413R_REG_ADC_CONV_CTRL 0x23
+#define AD74413R_CONV_SEQ_MASK GENMASK(9, 8)
+#define AD74413R_CONV_SEQ_ON 0b00
+#define AD74413R_CONV_SEQ_SINGLE 0b01
+#define AD74413R_CONV_SEQ_CONTINUOUS 0b10
+#define AD74413R_CONV_SEQ_OFF 0b11
+#define AD74413R_CH_EN_MASK(x) BIT(x)
+#define AD74413R_CH_EN_SHIFT(x) x
+
+#define AD74413R_REG_DIN_COMP_OUT 0x25
+#define AD74413R_DIN_COMP_OUT_MASK_X(x) BIT(x)
+#define AD74413R_DIN_COMP_OUT_SHIFT_X(x) x
+
+#define AD74413R_REG_ADC_RESULT_X(x) (0x26 + (x))
+#define AD74413R_ADC_RESULT_MAX ((1 << 16) - 1)
+
+#define AD74413R_REG_READ_SELECT 0x41
+
+#define AD74413R_REG_CMD_KEY 0x44
+#define AD74413R_CMD_KEY_LDAC 0x953a
+
+#define AD74413R_REG_SILLICON_REV 0x46
+#define AD74413R_SILLICON_REV_ID_MASK GENMASK(7, 0)
+#define AD74413R_SILLICON_REV_ID_EXPECTED 0x8
+
+#define AD74413R_ADC_DATA_TIMEOUT 1000
+
+static const int ad74413r_adc_sampling_rates[] = {
+ 20,
+ 4800,
+};
+
+static const int ad74413r_adc_sampling_rates_hart[] = {
+ 10,
+ 20,
+ 1200,
+ 4800,
+};
+
+static int ad74413r_crc(u8 *buf)
+{
+ return crc8(ad74413r_crc8_table, buf, 3, 0);
+}
+
+static void ad74413r_format_reg_write(unsigned int reg, unsigned int val, u8 *buf)
+{
+ buf[0] = reg & 0xff;
+ buf[1] = (val >> 8) & 0xff;
+ buf[2] = val & 0xff;
+ buf[3] = ad74413r_crc(buf);
+}
+
+static int ad74413r_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+ struct device *dev = context;
+ struct spi_device *spi = to_spi_device(dev);
+ struct ad74413r_state *st = spi_get_drvdata(spi);
+ u8 *buf = (u8 *)&st->regmap_tx_buf;
+
+ ad74413r_format_reg_write(reg, val, buf);
+
+ return spi_write(spi, buf, 4);
+}
+
+static int ad74413r_crc_check(struct ad74413r_state *st, u8 *buf)
+{
+ u8 expected_crc = ad74413r_crc(buf);
+
+ if (buf[3] != expected_crc) {
+ dev_err(st->dev, "Bad CRC, data: %02x%02x%02x, expected: %02x, received: %02x\n",
+ buf[0], buf[1], buf[2], expected_crc, buf[3]);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad74413r_reg_read_raw(void *context, unsigned int reg, u8 *buf,
+ unsigned int count)
+{
+ struct device *dev = context;
+ struct spi_device *spi = to_spi_device(dev);
+ struct ad74413r_state *st = spi_get_drvdata(spi);
+ unsigned int i;
+ int ret;
+
+ ret = ad74413r_reg_write(context, AD74413R_REG_READ_SELECT, reg);
+ if (ret) {
+ dev_err(dev, "Failed to write read select register: %d\n", ret);
+ return ret;
+ }
+
+ for (i = 0; i < count; i++, buf += 4) {
+ ret = spi_read(spi, buf, 4);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_crc_check(st, buf);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ad74413r_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+ struct device *dev = context;
+ struct spi_device *spi = to_spi_device(dev);
+ struct ad74413r_state *st = spi_get_drvdata(spi);
+ u8 *buf = (u8 *)&st->regmap_rx_buf;
+ int ret;
+
+ ret = ad74413r_reg_read_raw(context, reg, buf, 1);
+ if (ret)
+ return ret;
+
+ *val = (buf[1] << 8) | buf[2];
+
+ return 0;
+}
+
+const struct regmap_config ad74413r_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 16,
+ .reg_read = ad74413r_reg_read,
+ .reg_write = ad74413r_reg_write,
+};
+
+static int ad74413r_set_gpo_mode(struct ad74413r_state *st, unsigned int offset, u8 mode)
+{
+ if (mode < GPO_CONFIG_MIN || mode > GPO_CONFIG_MAX) {
+ dev_err(st->dev, "Invalid gpo config select mode\n");
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(st->regmap, AD74413R_REG_GPO_CONFIG_X(offset),
+ AD74413R_GPO_CONFIG_GPO_SELECT_MASK, mode);
+}
+
+static void ad74413r_gpio_set(struct gpio_chip *chip, unsigned int offset, int val)
+{
+ struct ad74413r_state *st = gpiochip_get_data(chip);
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[offset];
+ int ret;
+
+ if (channel_config->gpo_config != GPO_CONFIG_LOGIC) {
+ dev_err(st->dev, "Cannot set gpo %u, not in logic mode", offset);
+ return;
+ }
+
+ ret = ad74413r_set_gpo_mode(st, offset, GPO_CONFIG_LOGIC);
+ if (ret)
+ return;
+
+ regmap_update_bits(st->regmap, AD74413R_REG_GPO_CONFIG_X(offset),
+ AD74413R_GPO_CONFIG_GPO_DATA_MASK,
+ val ? AD74413R_GPO_CONFIG_DATA_HIGH
+ : AD74413R_GPO_CONFIG_DATA_LOW);
+}
+
+static void ad74413r_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask,
+ unsigned long *bits)
+{
+ struct ad74413r_state *st = gpiochip_get_data(chip);
+ unsigned int offset = 0;
+ int ret;
+
+ for_each_set_bit_from(offset, mask, AD74413R_CHANNEL_MAX) {
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[offset];
+
+ if (channel_config->gpo_config != GPO_CONFIG_LOGIC) {
+ dev_err(st->dev, "Cannot set gpo %u, not in logic mode\n", offset);
+ continue;
+ }
+
+ ret = ad74413r_set_gpo_mode(st, offset, GPO_CONFIG_LOGIC_PARALLEL);
+ if (ret)
+ return;
+ }
+
+ regmap_update_bits(st->regmap, AD74413R_REG_GPO_PAR_DATA, *mask, *bits);
+}
+
+static int ad74413r_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct ad74413r_state *st = gpiochip_get_data(chip);
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[offset];
+ unsigned int status;
+ int ret;
+
+ if (channel_config->gpo_config != GPO_CONFIG_DEBOUNCED_COMPARATOR) {
+ dev_err(st->dev, "Cannot get gpo %u, not in comparator mode\n", offset);
+ return -EINVAL;
+ }
+
+ ret = regmap_read(st->regmap, AD74413R_REG_DIN_COMP_OUT, &status);
+ if (ret)
+ return ret;
+
+ status &= AD74413R_DIN_COMP_OUT_MASK_X(offset);
+ status >>= AD74413R_DIN_COMP_OUT_SHIFT_X(offset);
+
+ return status;
+}
+
+static int ad74413r_set_channel_dac_code(struct ad74413r_state *st, unsigned int channel,
+ uint16_t dac_code)
+{
+ int ret;
+
+ if (dac_code > AD74413R_DAC_CODE_MAX) {
+ dev_err(st->dev, "Invalid dac code %d\n", dac_code);
+ return -EINVAL;
+ }
+
+ ret = regmap_write(st->regmap, AD74413R_REG_DAC_CODE_X(channel), dac_code);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(st->regmap, AD74413R_REG_CMD_KEY, AD74413R_CMD_KEY_LDAC);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int ad74413r_channel_set_function(struct ad74413r_state *st, unsigned int channel, u8 func)
+{
+ if (func < CH_FUNC_MIN || func > CH_FUNC_MAX) {
+ dev_err(st->dev, "Invalid channel function %d\n", func);
+ return -EINVAL;
+ }
+
+ if (!st->config->hart_support && (func == CH_FUNC_CURRENT_INPUT_EXT_POWER_HART
+ || func == CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART)) {
+ dev_err(st->dev, "HART function not supported %d\n", func);
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(st->regmap, AD74413R_REG_CH_FUNC_SETUP_X(channel),
+ AD74413R_CH_FUNC_SETUP_MASK, func);
+}
+
+static int ad74413r_set_adc_conv_seq(struct ad74413r_state *st, unsigned int status)
+{
+ int ret;
+
+ /*
+ * These bits do not clear when a conversion completes.
+ * To enable a subsequent conversion, repeat the write to enable the conversion.
+ */
+ ret = regmap_write_bits(st->regmap, AD74413R_REG_ADC_CONV_CTRL,
+ AD74413R_CONV_SEQ_MASK,
+ FIELD_PREP(AD74413R_CONV_SEQ_MASK, status));
+ if (ret)
+ return ret;
+
+ // Wait 100us before starting conversions
+ usleep_range(100, 120);
+
+ return 0;
+}
+
+static int ad74413r_set_adc_channel_enable(struct ad74413r_state *st, unsigned int channel,
+ bool status)
+{
+ unsigned int val = status << AD74413R_CH_EN_SHIFT(channel);
+
+ return regmap_update_bits(st->regmap, AD74413R_REG_ADC_CONV_CTRL,
+ AD74413R_CH_EN_MASK(channel), val);
+}
+
+static int ad74413r_get_adc_range(struct ad74413r_state *st, unsigned int channel,
+ unsigned int *val)
+{
+ int ret;
+
+ ret = regmap_read(st->regmap, AD74413R_REG_ADC_CONFIG_X(channel), val);
+ if (ret)
+ return ret;
+
+ *val = FIELD_GET(AD74413R_ADC_CONFIG_RANGE_MASK, *val);
+
+ return 0;
+}
+
+static int ad74413r_get_adc_rejection(struct ad74413r_state *st, unsigned int channel,
+ unsigned int *val)
+{
+ int ret;
+
+ ret = regmap_read(st->regmap, AD74413R_REG_ADC_CONFIG_X(channel), val);
+ if (ret)
+ return ret;
+
+ *val = FIELD_GET(AD74413R_ADC_CONFIG_REJECTION_MASK, *val);
+
+ return 0;
+}
+
+static int ad74413r_set_adc_rejection(struct ad74413r_state *st, unsigned int channel,
+ unsigned int val)
+{
+ if (!st->config->hart_support && (val == AD74413R_ADC_REJECTION_50_60_HART
+ || val == AD74413R_ADC_REJECTION_HART)) {
+ dev_err(st->dev, "HART rate not supported %d\n", val);
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(st->regmap, AD74413R_REG_ADC_CONFIG_X(channel),
+ AD74413R_ADC_CONFIG_REJECTION_MASK,
+ FIELD_PREP(AD74413R_ADC_CONFIG_REJECTION_MASK, val));
+}
+
+static int ad74413r_get_adc_rate(struct ad74413r_state *st, unsigned int channel, int *val)
+{
+ unsigned int rej;
+ int ret;
+
+ ret = ad74413r_get_adc_rejection(st, channel, &rej);
+ if (ret)
+ return ret;
+
+ switch (rej) {
+ case AD74413R_ADC_REJECTION_50_60:
+ *val = 20;
+ break;
+ case AD74413R_ADC_REJECTION_NONE:
+ *val = 4800;
+ break;
+ case AD74413R_ADC_REJECTION_50_60_HART:
+ *val = 10;
+ break;
+ case AD74413R_ADC_REJECTION_HART:
+ *val = 1200;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC rejection invalid\n", channel);
+ return -EINVAL;
+ }
+
+ return IIO_VAL_INT;
+}
+
+static int ad74413r_set_adc_rate(struct ad74413r_state *st, unsigned int channel, int val)
+{
+ unsigned int rej;
+
+ switch (val) {
+ case 20:
+ rej = AD74413R_ADC_REJECTION_50_60;
+ break;
+ case 4800:
+ rej = AD74413R_ADC_REJECTION_NONE;
+ break;
+ case 10:
+ rej = AD74413R_ADC_REJECTION_50_60_HART;
+ break;
+ case 1200:
+ rej = AD74413R_ADC_REJECTION_HART;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC rate invalid\n", channel);
+ return -EINVAL;
+ }
+
+ return ad74413r_set_adc_rejection(st, channel, rej);
+}
+
+static int ad74413r_get_adc_voltage_range(struct ad74413r_state *st, unsigned int channel,
+ int *val)
+{
+ unsigned int range;
+ int ret;
+
+ ret = ad74413r_get_adc_range(st, channel, &range);
+ if (ret)
+ return ret;
+
+ switch (range) {
+ case AD74413R_ADC_RANGE_10V:
+ *val = 10000;
+ break;
+ case AD74413R_ADC_RANGE_2P5V_EXT_POW:
+ case AD74413R_ADC_RANGE_2P5V_INT_POW:
+ *val = 2500;
+ break;
+ case AD74413R_ADC_RANGE_5V_BI_DIR:
+ *val = 5000;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC range invalid\n", channel);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad74413r_get_adc_voltage_offset(struct ad74413r_state *st, unsigned int channel,
+ int *val)
+{
+ unsigned int range;
+ int ret;
+
+ ret = ad74413r_get_adc_range(st, channel, &range);
+ if (ret)
+ return ret;
+
+ switch (range) {
+ case AD74413R_ADC_RANGE_10V:
+ case AD74413R_ADC_RANGE_2P5V_EXT_POW:
+ *val = 0;
+ break;
+ case AD74413R_ADC_RANGE_2P5V_INT_POW:
+ case AD74413R_ADC_RANGE_5V_BI_DIR:
+ *val = -2500;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC range invalid\n", channel);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad74413r_get_input_voltage_scale(struct ad74413r_state *st, unsigned int channel,
+ int *val, int *val2)
+{
+ int ret;
+
+ ret = ad74413r_get_adc_voltage_range(st, channel, val);
+ if (ret)
+ return ret;
+
+ *val2 = AD74413R_ADC_RESULT_MAX;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static int ad74413r_get_input_voltage_offset(struct ad74413r_state *st, unsigned int channel,
+ int *val, int *val2)
+{
+ unsigned int range;
+ int ret;
+
+ ret = ad74413r_get_adc_range(st, channel, &range);
+ if (ret)
+ return ret;
+
+ switch (range) {
+ case AD74413R_ADC_RANGE_10V:
+ case AD74413R_ADC_RANGE_2P5V_EXT_POW:
+ *val = 0;
+ break;
+ case AD74413R_ADC_RANGE_2P5V_INT_POW:
+ *val = -AD74413R_ADC_RESULT_MAX;
+ break;
+ case AD74413R_ADC_RANGE_5V_BI_DIR:
+ *val = -AD74413R_ADC_RESULT_MAX / 2;
+ break;
+ default:
+ dev_err(st->dev, "Channel %u ADC range invalid\n", channel);
+ return -EINVAL;
+ }
+
+ *val2 = 0;
+
+ return IIO_VAL_INT;
+}
+
+static int ad74413r_get_input_current_scale(struct ad74413r_state *st, unsigned int channel,
+ int *val, int *val2)
+{
+ int ret;
+
+ ret = ad74413r_get_adc_voltage_range(st, channel, val);
+ if (ret)
+ return ret;
+
+ *val2 = AD74413R_ADC_RESULT_MAX * st->rsense_resistance_ohms;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static int ad74413_get_input_current_offset(struct ad74413r_state *st, unsigned int channel,
+ int *val, int *val2)
+{
+ int range;
+ int offset;
+ int ret;
+
+ ret = ad74413r_get_adc_voltage_range(st, channel, &range);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_get_adc_voltage_offset(st, channel, &offset);
+ if (ret)
+ return ret;
+
+ *val = offset * AD74413R_ADC_RESULT_MAX / range;
+ *val2 = 0;
+
+ return IIO_VAL_INT;
+}
+
+static int ad74413r_get_output_voltage_scale(struct ad74413r_state *st,
+ int *val, int *val2)
+{
+ *val = 11000;
+ *val2 = AD74413R_DAC_CODE_MAX;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static int ad74413r_get_output_current_scale(struct ad74413r_state *st, int *val, int *val2)
+{
+ *val = regulator_get_voltage(st->refin_reg);
+ *val2 = st->rsense_resistance_ohms * AD74413R_DAC_CODE_MAX * 1000;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static irqreturn_t ad74413r_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ unsigned int i;
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ ret = spi_sync(st->spi, &st->adc_samples_msg);
+ if (ret)
+ goto out;
+
+ for (i = 0; i < st->adc_active_channels; i++)
+ ad74413r_crc_check(st, (u8 *)&st->adc_samples.rx_buf[i]);
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &st->adc_samples,
+ iio_get_time_ns(indio_dev));
+
+out:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ mutex_unlock(&st->lock);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t ad74413r_adc_data_interrupt(int irq, void *data)
+{
+ struct ad74413r_state *st = data;
+
+ if (iio_buffer_enabled(st->indio_dev))
+ iio_trigger_poll(st->trig);
+ else
+ complete(&st->adc_data_completion);
+
+ return IRQ_HANDLED;
+}
+
+static int ad74413r_get_single_adc_result(struct ad74413r_state *st, unsigned int channel,
+ int *val)
+{
+ unsigned int uval;
+ int ret;
+
+ ret = iio_device_claim_direct_mode(st->indio_dev);
+ if (ret)
+ return ret;
+
+ reinit_completion(&st->adc_data_completion);
+
+ ret = ad74413r_set_adc_channel_enable(st, channel, true);
+ if (ret)
+ goto out;
+
+ ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_SINGLE);
+ if (ret)
+ goto out;
+
+ ret = wait_for_completion_timeout(&st->adc_data_completion,
+ msecs_to_jiffies(AD74413R_ADC_DATA_TIMEOUT));
+ if (!ret) {
+ ret = -ETIMEDOUT;
+ goto out;
+ }
+
+ ret = regmap_read(st->regmap, AD74413R_REG_ADC_RESULT_X(channel), &uval);
+ if (ret)
+ goto out;
+
+ ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
+ if (ret)
+ goto out;
+
+ ret = ad74413r_set_adc_channel_enable(st, channel, false);
+ if (ret)
+ goto out;
+
+ *val = uval;
+ ret = IIO_VAL_INT;
+
+out:
+ iio_device_release_direct_mode(st->indio_dev);
+
+ return ret;
+}
+
+static int ad74413r_get_single_resistance_result(struct ad74413r_state *st, unsigned int channel,
+ int *val)
+{
+ unsigned int uval;
+ int ret;
+
+ ret = ad74413r_get_single_adc_result(st, channel, &uval);
+ if (ret < 0)
+ return ret;
+
+ if (uval == AD74413R_ADC_RESULT_MAX)
+ *val = 0;
+ else
+ *val = DIV_ROUND_CLOSEST(uval * 2100, AD74413R_ADC_RESULT_MAX - uval);
+
+ return ret;
+}
+
+static int ad74413r_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *active_scan_mask)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ struct spi_transfer *xfer;
+ unsigned int channel;
+ int transfer_index = 0;
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ spi_message_init(&st->adc_samples_msg);
+ st->adc_active_channels = 0;
+
+ for (channel = 0; channel < AD74413R_CHANNEL_MAX; channel++) {
+ bool status = test_bit(channel, active_scan_mask);
+ u8 *tx_buf;
+
+ ret = ad74413r_set_adc_channel_enable(st, channel, status);
+ if (ret)
+ goto out;
+
+ if (!status)
+ continue;
+
+ st->adc_active_channels++;
+
+ xfer = &st->adc_samples_xfer[transfer_index];
+
+ if (transfer_index == 0)
+ xfer->rx_buf = NULL;
+ else
+ xfer->rx_buf = &st->adc_samples.rx_buf[transfer_index - 1];
+
+ tx_buf = (u8 *)&st->adc_samples_tx_buf[transfer_index];
+
+ xfer->tx_buf = tx_buf;
+ xfer->len = 4;
+ xfer->cs_change = 1;
+
+ ad74413r_format_reg_write(AD74413R_REG_READ_SELECT,
+ AD74413R_REG_ADC_RESULT_X(channel),
+ tx_buf);
+
+ spi_message_add_tail(xfer, &st->adc_samples_msg);
+
+ transfer_index++;
+ }
+
+ if (transfer_index == 0)
+ goto out;
+
+ xfer = &st->adc_samples_xfer[transfer_index];
+
+ xfer->tx_buf = NULL;
+ xfer->rx_buf = &st->adc_samples.rx_buf[transfer_index - 1];
+ xfer->len = 4;
+ xfer->cs_change = 0;
+
+ spi_message_add_tail(xfer, &st->adc_samples_msg);
+
+out:
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int ad74413r_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+
+ return ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_CONTINUOUS);
+}
+
+static int ad74413r_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+
+ return ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
+}
+
+static int ad74413r_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ int *val, int *val2, long info)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ int ret = -EINVAL;
+
+ mutex_lock(&st->lock);
+
+ switch (info) {
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_ALTVOLTAGE:
+ if (chan->output)
+ ret = ad74413r_get_output_voltage_scale(st, val, val2);
+ else
+ ret = ad74413r_get_input_voltage_scale(st, chan->channel,
+ val, val2);
+ break;
+
+ case IIO_CURRENT:
+ if (chan->output)
+ ret = ad74413r_get_output_current_scale(st, val, val2);
+ else
+ ret = ad74413r_get_input_current_scale(st, chan->channel,
+ val, val2);
+ break;
+ default:
+ break;
+ }
+ break;
+ case IIO_CHAN_INFO_OFFSET:
+ switch (chan->type) {
+ case IIO_ALTVOLTAGE:
+ if (!chan->output)
+ ret = ad74413r_get_input_voltage_offset(st, chan->channel,
+ val, val2);
+ break;
+ case IIO_CURRENT:
+ if (!chan->output)
+ ret = ad74413_get_input_current_offset(st, chan->channel,
+ val, val2);
+ break;
+ default:
+ break;
+ }
+ break;
+ case IIO_CHAN_INFO_RAW:
+ if (!chan->output)
+ ret = ad74413r_get_single_adc_result(st, chan->channel, val);
+ break;
+ case IIO_CHAN_INFO_PROCESSED:
+ switch (chan->type) {
+ case IIO_RESISTANCE:
+ if (!chan->output)
+ ret = ad74413r_get_single_resistance_result(st, chan->channel,
+ val);
+ break;
+ default:
+ break;
+ }
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (!chan->output)
+ ret = ad74413r_get_adc_rate(st, chan->channel, val);
+ break;
+ default:
+ break;
+ }
+
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int ad74413r_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ int val, int val2, long info)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ int ret = -EINVAL;
+
+ mutex_lock(&st->lock);
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ switch (chan->type) {
+ case IIO_ALTVOLTAGE:
+ case IIO_CURRENT:
+ ret = ad74413r_set_channel_dac_code(st, chan->channel, val);
+ break;
+ default:
+ break;
+ }
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (!chan->output)
+ ret = ad74413r_set_adc_rate(st, chan->channel, val);
+ break;
+ default:
+ break;
+ }
+
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int ad74413r_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length, long info)
+{
+ struct ad74413r_state *st = iio_priv(indio_dev);
+ int ret = -EINVAL;
+
+ switch (info) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (st->config->hart_support) {
+ *vals = ad74413r_adc_sampling_rates_hart;
+ *length = ARRAY_SIZE(ad74413r_adc_sampling_rates_hart);
+ } else {
+ *vals = ad74413r_adc_sampling_rates;
+ *length = ARRAY_SIZE(ad74413r_adc_sampling_rates);
+ }
+ *type = IIO_VAL_INT;
+ ret = IIO_AVAIL_LIST;
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+static const struct iio_buffer_setup_ops ad74413r_buffer_ops = {
+ .postenable = &ad74413r_buffer_postenable,
+ .predisable = &ad74413r_buffer_predisable,
+};
+
+static const struct iio_trigger_ops ad74413r_trigger_ops = {
+ .validate_device = iio_trigger_validate_own_device,
+};
+
+static const struct iio_info ad74413r_info = {
+ .read_raw = &ad74413r_read_raw,
+ .write_raw = &ad74413r_write_raw,
+ .read_avail = &ad74413r_read_avail,
+ .update_scan_mode = &ad74413r_update_scan_mode,
+};
+
+#define AD74413R_CHANNEL(_type, _output, extra_mask_separate) \
+ .type = _type, \
+ .indexed = 1, \
+ .output = _output, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | extra_mask_separate
+
+#define AD74413R_DAC_CHANNEL(_type, extra_mask_separate) \
+ { \
+ AD74413R_CHANNEL(_type, 1, extra_mask_separate), \
+ }
+
+#define AD74413R_ADC_CHANNEL(_type, extra_mask_separate) \
+ { \
+ AD74413R_CHANNEL(_type, 0, extra_mask_separate | BIT(IIO_CHAN_INFO_SAMP_FREQ)), \
+ .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 16, \
+ .storagebits = 32, \
+ .shift = 8, \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+#define AD74413R_ADC_VOLTAGE_CHANNEL \
+ AD74413R_ADC_CHANNEL(IIO_ALTVOLTAGE, BIT(IIO_CHAN_INFO_SCALE) \
+ | BIT(IIO_CHAN_INFO_OFFSET))
+
+#define AD74413R_ADC_CURRENT_CHANNEL \
+ AD74413R_ADC_CHANNEL(IIO_CURRENT, BIT(IIO_CHAN_INFO_SCALE) \
+ | BIT(IIO_CHAN_INFO_OFFSET))
+
+static struct iio_chan_spec ad74413r_high_impedance_channels[] = {
+ AD74413R_ADC_VOLTAGE_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_voltage_output_channels[] = {
+ AD74413R_DAC_CHANNEL(IIO_ALTVOLTAGE, BIT(IIO_CHAN_INFO_SCALE)),
+ AD74413R_ADC_CURRENT_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_current_output_channels[] = {
+ AD74413R_DAC_CHANNEL(IIO_CURRENT, BIT(IIO_CHAN_INFO_SCALE)),
+ AD74413R_ADC_VOLTAGE_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_voltage_input_channels[] = {
+ AD74413R_ADC_VOLTAGE_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_current_input_channels[] = {
+ AD74413R_ADC_CURRENT_CHANNEL,
+};
+
+static struct iio_chan_spec ad74413r_resistance_input_channels[] = {
+ AD74413R_ADC_CHANNEL(IIO_RESISTANCE, BIT(IIO_CHAN_INFO_PROCESSED)),
+};
+
+static struct iio_chan_spec ad74413r_digital_input_channels[] = {
+ AD74413R_ADC_VOLTAGE_CHANNEL,
+};
+
+static int ad74413r_of_parse_channel_config(struct ad74413r_state *st,
+ struct device_node *channel_node)
+{
+ struct ad74413r_channel_config *channel_config;
+ u32 index;
+ int ret;
+
+ ret = of_property_read_u32(channel_node, "reg", &index);
+ if (ret) {
+ dev_err(st->dev, "Failed to read channel reg: %d\n", ret);
+ return ret;
+ }
+
+ if (index > AD74413R_CHANNEL_MAX) {
+ dev_err(st->dev, "Channel index %u is too large\n", index);
+ return -EINVAL;
+ }
+
+ channel_config = &st->channel_configs[index];
+ if (channel_config->initialized) {
+ dev_err(st->dev, "Channel %u has already been initialized\n", index);
+ return -EINVAL;
+ }
+
+ channel_config->func = CH_FUNC_HIGH_IMPEDANCE;
+ of_property_read_u32(channel_node, "adi,ch-func", &channel_config->func);
+
+ channel_config->gpo_config = GPO_CONFIG_100K_PULL_DOWN;
+ of_property_read_u32(channel_node, "adi,gpo-config", &channel_config->gpo_config);
+
+ if (channel_config->gpo_config == GPO_CONFIG_LOGIC_PARALLEL) {
+ dev_err(st->dev, "GPO config logic parallel is unsupported\n");
+ return -EINVAL;
+ }
+
+ ret = ad74413r_channel_set_function(st, index, channel_config->func);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_set_gpo_mode(st, index, channel_config->gpo_config);
+ if (ret)
+ return ret;
+
+ channel_config->initialized = true;
+
+ return 0;
+}
+
+static int ad74413r_parse_channel_configs(struct ad74413r_state *st)
+{
+ struct device_node *channel_node = NULL;
+ int ret;
+
+ for_each_available_child_of_node(st->dev->of_node, channel_node) {
+ if (!of_node_name_eq(channel_node, "channel"))
+ continue;
+
+ ret = ad74413r_of_parse_channel_config(st, channel_node);
+ if (ret) {
+ dev_err(st->dev, "Failed to parse channel %s config: %d\n",
+ channel_node->name, ret);
+ goto put_channel_node;
+ }
+ }
+
+put_channel_node:
+ of_node_put(channel_node);
+
+ return ret;
+}
+
+static int ad74413r_get_iio_channels(struct ad74413r_state *st, u8 func,
+ struct iio_chan_spec **iio_channels, unsigned int *count)
+{
+ switch (func) {
+ case CH_FUNC_HIGH_IMPEDANCE:
+ *iio_channels = ad74413r_high_impedance_channels;
+ *count = ARRAY_SIZE(ad74413r_high_impedance_channels);
+ break;
+ case CH_FUNC_VOLTAGE_OUTPUT:
+ *iio_channels = ad74413r_voltage_output_channels;
+ *count = ARRAY_SIZE(ad74413r_voltage_output_channels);
+ break;
+ case CH_FUNC_CURRENT_OUTPUT:
+ *iio_channels = ad74413r_current_output_channels;
+ *count = ARRAY_SIZE(ad74413r_current_output_channels);
+ break;
+ case CH_FUNC_VOLTAGE_INPUT:
+ *iio_channels = ad74413r_voltage_input_channels;
+ *count = ARRAY_SIZE(ad74413r_voltage_input_channels);
+ break;
+ case CH_FUNC_CURRENT_INPUT_EXT_POWER:
+ case CH_FUNC_CURRENT_INPUT_LOOP_POWER:
+ case CH_FUNC_CURRENT_INPUT_EXT_POWER_HART:
+ case CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART:
+ *iio_channels = ad74413r_current_input_channels;
+ *count = ARRAY_SIZE(ad74413r_current_input_channels);
+ break;
+ case CH_FUNC_RESISTANCE_INPUT:
+ *iio_channels = ad74413r_resistance_input_channels;
+ *count = ARRAY_SIZE(ad74413r_resistance_input_channels);
+ break;
+ case CH_FUNC_DIGITAL_INPUT_LOGIC:
+ case CH_FUNC_DIGITAL_INPUT_LOOP_POWER:
+ *iio_channels = ad74413r_digital_input_channels;
+ *count = ARRAY_SIZE(ad74413r_digital_input_channels);
+ break;
+ default:
+ dev_err(st->dev, "Unhandled channel config function %d\n", func);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad74413r_count_iio_channels(struct ad74413r_state *st)
+{
+ unsigned int index;
+ int ret;
+
+ st->indio_dev->num_channels = 0;
+
+ for (index = 0; index < AD74413R_CHANNEL_MAX; index++) {
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[index];
+ struct iio_chan_spec *local_channels;
+ unsigned int num_local_channels;
+
+ ret = ad74413r_get_iio_channels(st, channel_config->func, &local_channels,
+ &num_local_channels);
+ if (ret)
+ return ret;
+
+ st->indio_dev->num_channels += num_local_channels;
+ }
+
+ return 0;
+}
+
+static int ad74413r_setup_iio_channels(struct ad74413r_state *st)
+{
+ struct iio_chan_spec *channels;
+ unsigned int index;
+ int ret;
+
+ channels = devm_kcalloc(st->dev, sizeof(*channels),
+ st->indio_dev->num_channels, GFP_KERNEL);
+ if (!channels)
+ return -ENOMEM;
+
+ st->indio_dev->channels = channels;
+
+ for (index = 0; index < AD74413R_CHANNEL_MAX; index++) {
+ struct ad74413r_channel_config *channel_config = &st->channel_configs[index];
+ struct iio_chan_spec *local_channels;
+ unsigned int num_local_channels;
+ unsigned int chan_index;
+
+ ret = ad74413r_get_iio_channels(st, channel_config->func, &local_channels,
+ &num_local_channels);
+ if (ret)
+ return ret;
+
+ memcpy(channels, local_channels, num_local_channels * sizeof(*local_channels));
+
+ for (chan_index = 0; chan_index < num_local_channels; chan_index++) {
+ struct iio_chan_spec *local_channel = &channels[chan_index];
+
+ local_channel->channel = index;
+ if (local_channel->output)
+ local_channel->scan_index = -1;
+ else
+ local_channel->scan_index = index;
+
+ }
+
+ channels += num_local_channels;
+ }
+
+ return 0;
+}
+
+static void ad74413r_regulator_disable(void *regulator)
+{
+ regulator_disable(regulator);
+}
+
+static int ad74413r_probe(struct spi_device *spi)
+{
+ struct ad74413r_state *st;
+ struct iio_dev *indio_dev;
+ unsigned int sillicon_rev_id;
+ const char *name;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (indio_dev == NULL)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ spi_set_drvdata(spi, st);
+
+ st->spi = spi;
+ st->dev = &spi->dev;
+ st->config = of_device_get_match_data(&spi->dev);
+ st->indio_dev = indio_dev;
+ mutex_init(&st->lock);
+ init_completion(&st->adc_data_completion);
+
+ name = dev_name(st->dev);
+
+ st->regmap = devm_regmap_init(st->dev, NULL, &spi->dev, &ad74413r_regmap_config);
+ if (IS_ERR(st->regmap)) {
+ ret = PTR_ERR(st->regmap);
+ dev_err(st->dev, "Failed to create regmap: %d\n", ret);
+ return ret;
+ }
+
+ ret = regmap_read(st->regmap, AD74413R_REG_SILLICON_REV, &sillicon_rev_id);
+ if (ret) {
+ dev_err(st->dev, "Failed to read sillicon rev: %d\n", ret);
+ return ret;
+ }
+
+ sillicon_rev_id = FIELD_GET(AD74413R_SILLICON_REV_ID_MASK, sillicon_rev_id);
+ if (sillicon_rev_id != AD74413R_SILLICON_REV_ID_EXPECTED) {
+ dev_err(st->dev, "Read sillicon rev id %d does not match expected rev id %d\n",
+ sillicon_rev_id, AD74413R_SILLICON_REV_ID_EXPECTED);
+ return -EINVAL;
+ }
+
+ st->refin_reg = devm_regulator_get(st->dev, "refin");
+ if (IS_ERR(st->refin_reg)) {
+ dev_err(st->dev, "Failed to get refin regulator: %d\n", ret);
+ return PTR_ERR(st->refin_reg);
+ }
+
+ ret = regulator_enable(st->refin_reg);
+ if (ret) {
+ dev_err(st->dev, "Failed to enable refin regulator: %d\n", ret);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(st->dev, ad74413r_regulator_disable,
+ st->refin_reg);
+ if (ret) {
+ dev_err(st->dev, "Failed to add refin regulator disable action: %d\n", ret);
+ return ret;
+ }
+
+ ret = of_property_read_u32(st->dev->of_node, "adi,rsense-resistance-ohms",
+ &st->rsense_resistance_ohms);
+ if (ret) {
+ dev_err(st->dev, "Failed to get rsense resistance: %d\n", ret);
+ return ret;
+ }
+
+ st->gpiochip.label = name;
+ st->gpiochip.base = -1;
+ st->gpiochip.ngpio = AD74413R_CHANNEL_MAX;
+ st->gpiochip.parent = st->dev;
+ st->gpiochip.can_sleep = true;
+ st->gpiochip.set = ad74413r_gpio_set;
+ st->gpiochip.set_multiple = ad74413r_gpio_set_multiple;
+ st->gpiochip.owner = THIS_MODULE;
+ st->gpiochip.get = ad74413r_gpio_get;
+
+ ret = devm_gpiochip_add_data(st->dev, &st->gpiochip, st);
+ if (ret) {
+ dev_err(st->dev, "Failed to add gpio chip: %d\n", ret);
+ return ret;
+ }
+
+ st->trig = devm_iio_trigger_alloc(st->dev, "%s-dev%d", name, indio_dev->id);
+ if (!st->trig)
+ return -ENOMEM;
+
+ st->trig->ops = &ad74413r_trigger_ops;
+ st->trig->dev.parent = st->dev;
+ iio_trigger_set_drvdata(st->trig, st);
+
+ ret = devm_iio_trigger_register(st->dev, st->trig);
+ if (ret)
+ return ret;
+
+ indio_dev->dev.parent = st->dev;
+ indio_dev->name = name;
+ indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
+ indio_dev->info = &ad74413r_info;
+ indio_dev->trig = iio_trigger_get(st->trig);
+
+ ret = ad74413r_parse_channel_configs(st);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_count_iio_channels(st);
+ if (ret)
+ return ret;
+
+ ret = ad74413r_setup_iio_channels(st);
+ if (ret) {
+ dev_err(st->dev, "Failed to setup iio channels: %d\n", ret);
+ return ret;
+ }
+
+ ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
+ if (ret)
+ return ret;
+
+ ret = devm_request_irq(st->dev, spi->irq, ad74413r_adc_data_interrupt,
+ IRQF_TRIGGER_FALLING, name, st);
+ if (ret) {
+ dev_err(st->dev, "Failed to request threaded irq: %d\n", ret);
+ return ret;
+ }
+
+ ret = devm_iio_triggered_buffer_setup(st->dev, indio_dev, &iio_pollfunc_store_time,
+ &ad74413r_trigger_handler, &ad74413r_buffer_ops);
+ if (ret) {
+ dev_err(st->dev, "Failed to setup triggered buffer: %d\n", ret);
+ return ret;
+ }
+
+ return devm_iio_device_register(st->dev, indio_dev);
+}
+
+static int ad74413r_unregister_driver(struct spi_driver *spi)
+{
+ spi_unregister_driver(spi);
+
+ return 0;
+}
+
+static int __init ad74413r_register_driver(struct spi_driver *spi)
+{
+ crc8_populate_msb(ad74413r_crc8_table, AD74413R_CRC_POLYNOMIAL);
+
+ return spi_register_driver(spi);
+}
+
+static const struct ad74413r_config ad74412r_config_data = {
+ .hart_support = false,
+};
+
+static const struct ad74413r_config ad74413r_config_data = {
+ .hart_support = true,
+};
+
+static const struct of_device_id ad74413r_dt_id[] = {
+ {
+ .compatible = "adi,ad74412r",
+ .data = &ad74412r_config_data,
+ },
+ {
+ .compatible = "adi,ad74413r",
+ .data = &ad74413r_config_data,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ad74413r_dt_id);
+
+static struct spi_driver ad74413r_driver = {
+ .driver = {
+ .name = "ad74413r",
+ .of_match_table = ad74413r_dt_id,
+ },
+ .probe = ad74413r_probe,
+};
+
+module_driver(ad74413r_driver, ad74413r_register_driver, ad74413r_unregister_driver);
+
+MODULE_AUTHOR("Cosmin Tanislav <cosmin.tanislav@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD74413R ADDAC");
+MODULE_LICENSE("GPL v2");
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] dt-bindings: iio: add AD74413R
2021-10-28 13:48 ` [PATCH 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
@ 2021-10-28 14:48 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2021-10-28 14:48 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: cosmin.tanislav, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, linux-iio, devicetree, linux-kernel
On Thu, 28 Oct 2021 16:48:44 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:
> Add device tree bindings for AD74413R.
Hi Cosmin,
Welcome to IIO.
Nice to give a tiny bit of description of the device in here
to expand on what the patch title told us.
Some comments inline.
Thanks,
Jonathan
>
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> ---
> .../bindings/iio/addac/adi,ad74413r.yaml | 163 ++++++++++++++++++
> include/dt-bindings/iio/addac/adi,ad74413r.h | 30 ++++
> 2 files changed, 193 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
> create mode 100644 include/dt-bindings/iio/addac/adi,ad74413r.h
>
> diff --git a/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml b/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
> new file mode 100644
> index 000000000000..ed4ee3047fbe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
> @@ -0,0 +1,163 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/addac/adi,ad74413r.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD74413R/AD74412R device driver
Bindings always describe hardware, not drivers (so we don't expect to
see the word driver in them).
> +
> +maintainers:
> + - Cosmin Tanislav <cosmin.tanislav@analog.com>
> +
> +description: |
> + The AD74413R and AD74412R are quad-channel software configurable input/output
> + solutions for building and process control applications. They contain
> + functionality for analog output, analog input, digital input, resistance
> + temperature detector, and thermocouple measurements integrated
> + into a single chip solution with an SPI interface.
> + The devices feature a 16-bit ADC and four configurable 13-bit DACs to provide
> + four configurable input/output channels and a suite of diagnostic functions.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad74413r
> + - adi,ad74412r
Alphabetical / numeric order preferred.
> +
> + reg:
> + maxItems: 1
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + spi-max-frequency:
> + maximum: 1000000
> +
> + spi-cpol: true
> +
> + interrupts:
> + maxItems: 1
> +
> + refin-supply:
> + description:
> + Reference voltage regulator.
Name pretty much tells us everything in the description.
refin-supply: true
is enough here I think.
> +
> + adi,rsense-resistance-ohms:
> + $ref: /schemas/types.yaml#/definitions/uint32
I think the automatic units stuff in dt-schema should remove the need for
a ref here.
> + description:
> + RSense resistance values in Ohms.
> +
> +required:
> + - compatible
> + - reg
> + - spi-max-frequency
> + - spi-cpol
> + - refin-supply
> + - adi,rsense-resistance-ohm
> +
> +additionalProperties: false
> +
> +patternProperties:
> + "^channel@[0-3]$":
> + type: object
> + description: Represents the external channels which are connected to the device.
> +
> + properties:
> + reg:
> + description: |
> + The channel number. It can have up to 4 channels numbered from 0 to 3.
> + maxItems: 1
> + minimum: 0
> + maximum: 3
> +
> + adi,ch-func:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Channel function.
> + HART functions are not supported on AD74412R.
Ideally enforce that with suitable conditional statements in this binding schema.
> + 0 - CH_FUNC_HIGH_IMPEDANCE
> + 1 - CH_FUNC_VOLTAGE_OUTPUT
> + 2 - CH_FUNC_CURRENT_OUTPUT
> + 3 - CH_FUNC_VOLTAGE_INPUT
> + 4 - CH_FUNC_CURRENT_INPUT_EXT_POWER
> + 5 - CH_FUNC_CURRENT_INPUT_LOOP_POWER
> + 6 - CH_FUNC_RESISTANCE_INPUT
> + 7 - CH_FUNC_DIGITAL_INPUT_LOGIC
> + 8 - CH_FUNC_DIGITAL_INPUT_LOOP_POWER
> + 9 - CH_FUNC_CURRENT_INPUT_EXT_POWER_HART
> + 10 - CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART
I'm not a great fan of large enums, but it is a bit hard to
work out a scheme that would be better for this so I guess we are
stuck with this.
> + maxItems: 1
> + minimum: 0
> + maximum: 10
> +
> + adi,gpo-config:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + GPO config.
GPO is what exactly?
> + 0 - GPO_CONFIG_100K_PULL_DOWN
> + 1 - GPO_CONFIG_LOGIC
Interesting that you have defined 2 in the header...
Why not here?
> + 3 - GPO_CONFIG_DEBOUNCED_COMPARATOR
> + 4 - GPO_CONFIG_HIGH_IMPEDANCE
> + maxItems: 1
> + enum: [0, 1, 3, 4]
> +
> + required:
> + - reg
This is fine if we have a default specified for the other elements
such as what type of channel this is.
> +
> +examples:
> + - |
> + spi0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cs-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
> + status = "okay";
> +
> + ad74413r@0 {
> + compatible = "adi,ad74413r";
> + reg = <0>;
> + spi-max-frequency = <1000000>;
> + spi-cpol;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + interrupt-parent = <&gpio>;
> + interrupts = <26 0>;
> +
> + refin-supply = <&ad74413r_refin>;
> + adi,rsense-resistance-ohm = <100>;
> +
> + channel@0 {
> + reg = <0>;
> +
> + adi,ch-func = <CH_FUNC_VOLTAGE_OUTPUT>;
> + adi,gpo-config = <GPO_CONFIG_LOGIC>;
> + };
> +
> + channel@1 {
> + reg = <1>;
> +
> + adi,ch-func = <CH_FUNC_CURRENT_OUTPUT>;
> + adi,gpo-config = <GPO_CONFIG_LOGIC>;
> + };
> +
> + channel@2 {
> + reg = <2>;
> +
> + adi,ch-func = <CH_FUNC_VOLTAGE_INPUT>;
> + adi,gpo-config = <GPO_CONFIG_DEBOUNCED_COMPARATOR>;
> + };
> +
> + channel@3 {
> + reg = <3>;
> +
> + adi,ch-func = <CH_FUNC_CURRENT_INPUT_EXT_POWER>;
> + adi,gpo-config = <GPO_CONFIG_DEBOUNCED_COMPARATOR>;
> + };
> + };
> + };
> +...
> diff --git a/include/dt-bindings/iio/addac/adi,ad74413r.h b/include/dt-bindings/iio/addac/adi,ad74413r.h
> new file mode 100644
> index 000000000000..bde558d9731c
> --- /dev/null
> +++ b/include/dt-bindings/iio/addac/adi,ad74413r.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _DT_BINDINGS_ADI_AD74413R_H
> +#define _DT_BINDINGS_ADI_AD74413R_H
> +
> +#define GPO_CONFIG_100K_PULL_DOWN 0x0
> +#define GPO_CONFIG_LOGIC 0x1
> +#define GPO_CONFIG_LOGIC_PARALLEL 0x2
> +#define GPO_CONFIG_DEBOUNCED_COMPARATOR 0x3
> +#define GPO_CONFIG_HIGH_IMPEDANCE 0x4
> +
> +#define GPO_CONFIG_MIN GPO_CONFIG_100K_PULL_DOWN
> +#define GPO_CONFIG_MAX GPO_CONFIG_HIGH_IMPEDANCE
> +
> +#define CH_FUNC_HIGH_IMPEDANCE 0x0
> +#define CH_FUNC_VOLTAGE_OUTPUT 0x1
> +#define CH_FUNC_CURRENT_OUTPUT 0x2
> +#define CH_FUNC_VOLTAGE_INPUT 0x3
> +#define CH_FUNC_CURRENT_INPUT_EXT_POWER 0x4
> +#define CH_FUNC_CURRENT_INPUT_LOOP_POWER 0x5
> +#define CH_FUNC_RESISTANCE_INPUT 0x6
> +#define CH_FUNC_DIGITAL_INPUT_LOGIC 0x7
> +#define CH_FUNC_DIGITAL_INPUT_LOOP_POWER 0x8
> +#define CH_FUNC_CURRENT_INPUT_EXT_POWER_HART 0x9
> +#define CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART 0xA
> +
> +#define CH_FUNC_MIN CH_FUNC_HIGH_IMPEDANCE
> +#define CH_FUNC_MAX CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART
> +
> +#endif /* _DT_BINDINGS_ADI_AD74413R_H */
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: addac: add AD74413R driver
2021-10-28 13:48 ` [PATCH 3/3] iio: addac: add AD74413R driver Cosmin Tanislav
@ 2021-10-28 15:58 ` Jonathan Cameron
[not found] ` <CAA+onPPObFeXQwJ450M=5Ue3uE6Cui0M0X49qgn_U1D6su=BcA@mail.gmail.com>
2021-10-31 22:47 ` kernel test robot
1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2021-10-28 15:58 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: cosmin.tanislav, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, linux-iio, devicetree, linux-kernel, Linus Walleij,
Bartosz Golaszewski, linux-gpio
On Thu, 28 Oct 2021 16:48:45 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:
> Add IIO ADDAC driver for AD74413R.
Hi Cosmin,
+CC gpio people given this is also a gpio chip driver.
As for the binding, it's nice to have a little more in the patch description.
Here it would be good to see something about what features are supported
by the driver for example.
For a series like this I'd also suggest using a cover letter. That both
provides a good place to put general comments and also a good place for
people to give tags for the whole series.
Comments inline. I've left the whole code here this time so that
it is visible to those I added to the cc list.
Thanks,
Jonathan
>
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> ---
> MAINTAINERS | 9 +
> drivers/iio/addac/Kconfig | 12 +
> drivers/iio/addac/Makefile | 1 +
> drivers/iio/addac/ad74413r.c | 1389 ++++++++++++++++++++++++++++++++++
> 4 files changed, 1411 insertions(+)
> create mode 100644 drivers/iio/addac/ad74413r.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 37982beaf91f..174f0fd3d71c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1069,6 +1069,15 @@ W: http://ez.analog.com/community/linux-device-drivers
> F: Documentation/devicetree/bindings/iio/adc/adi,ad7780.yaml
> F: drivers/iio/adc/ad7780.c
>
> +ANALOG DEVICES INC AD74413R DRIVER
> +M: Cosmin Tanislav <cosmin.tanislav@analog.com>
> +L: linux-iio@vger.kernel.org
> +S: Supported
> +W: http://ez.analog.com/community/linux-device-drivers
> +F: Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
> +F: drivers/iio/addac/ad74413r.c
> +F: include/dt-bindings/iio/addac/adi,ad74413r.h
> +
> ANALOG DEVICES INC AD9389B DRIVER
> M: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> L: linux-media@vger.kernel.org
> diff --git a/drivers/iio/addac/Kconfig b/drivers/iio/addac/Kconfig
> index 2e64d7755d5e..06b26f8d0144 100644
> --- a/drivers/iio/addac/Kconfig
> +++ b/drivers/iio/addac/Kconfig
> @@ -5,4 +5,16 @@
>
> menu "Analog to digital and digital to analog converters"
>
> +config AD74413R
> + tristate "Analog Devices AD74413R/AD74412R driver"
> + depends on GPIOLIB && SPI
> + select REGMAP_SPI
> + select CRC8
> + help
> + Say yes here to build support for Analog Devices AD74413R/AD74412R
> + quad-channel software configurable input/output solution.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called ad74413r.
> +
> endmenu
> diff --git a/drivers/iio/addac/Makefile b/drivers/iio/addac/Makefile
> index b888b9ee12da..cfd4bbe64ad3 100644
> --- a/drivers/iio/addac/Makefile
> +++ b/drivers/iio/addac/Makefile
> @@ -4,3 +4,4 @@
> #
>
> # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_AD74413R) += ad74413r.o
> diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> new file mode 100644
> index 000000000000..76d2e09a17ee
> --- /dev/null
> +++ b/drivers/iio/addac/ad74413r.c
> @@ -0,0 +1,1389 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 Analog Devices, Inc.
> + * Author: Cosmin Tanislav <cosmin.tanislav@analog.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/crc8.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
mod_devicetable.h
> +#include <linux/of.h>
> +#include <linux/of_device.h>
We are attempting to move all new drivers (and many old ones)
over to the generic firmware interfaces in linux/property.h
as that gives us support for other firmware types more or less for
free.
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <dt-bindings/iio/addac/adi,ad74413r.h>
> +
> +#define AD74413R_CRC_POLYNOMIAL 0x7
> +DECLARE_CRC8_TABLE(ad74413r_crc8_table);
> +
> +#define AD74413R_CHANNEL_MAX 4
> +
> +struct ad74413r_config {
> + bool hart_support;
> +};
> +
> +struct ad74413r_channel_config {
> + u32 gpo_config;
> + u32 func;
> + bool initialized;
> +};
> +
> +struct ad74413r_state {
> + struct ad74413r_channel_config channel_configs[AD74413R_CHANNEL_MAX];
> + struct gpio_chip gpiochip;
> + struct mutex lock;
> + struct completion adc_data_completion;
> +
> + struct spi_device *spi;
> + struct iio_dev *indio_dev;
> + struct regulator *refin_reg;
> + struct regmap *regmap;
> + struct device *dev;
> + struct iio_trigger *trig;
> + const struct ad74413r_config *config;
> +
> + u32 rsense_resistance_ohms;
> +
> + size_t adc_active_channels;
> + struct spi_message adc_samples_msg;
> + struct spi_transfer adc_samples_xfer[AD74413R_CHANNEL_MAX + 1];
> + struct {
> + __be32 rx_buf[AD74413R_CHANNEL_MAX];
> + s64 timestamp;
> + } adc_samples ____cacheline_aligned;
I would add a comment somewhere here to say you are using this single ____cacheline_aligned
element to ensure that all these buffers don't share a cacheline with anything esle.
It doesn't matter that they share space with each other.
> + __be32 adc_samples_tx_buf[AD74413R_CHANNEL_MAX];
> + __be32 regmap_tx_buf;
> + __be32 regmap_rx_buf;
> +};
> +
> +#define AD74413R_REG_NOP 0x00
> +
> +#define AD74413R_REG_CH_FUNC_SETUP_X(x) (0x01 + (x))
> +#define AD74413R_CH_FUNC_SETUP_MASK GENMASK(3, 0)
> +
> +#define AD74413R_REG_ADC_CONFIG_X(x) (0x05 + (x))
> +#define AD74413R_ADC_CONFIG_RANGE_MASK GENMASK(7, 5)
> +#define AD74413R_ADC_CONFIG_REJECTION_MASK GENMASK(4, 3)
> +#define AD74413R_ADC_RANGE_10V 0b000
> +#define AD74413R_ADC_RANGE_2P5V_EXT_POW 0b001
> +#define AD74413R_ADC_RANGE_2P5V_INT_POW 0b010
> +#define AD74413R_ADC_RANGE_5V_BI_DIR 0b011
> +#define AD74413R_ADC_REJECTION_50_60 0b00
> +#define AD74413R_ADC_REJECTION_NONE 0b01
> +#define AD74413R_ADC_REJECTION_50_60_HART 0b10
> +#define AD74413R_ADC_REJECTION_HART 0b11
> +
> +#define AD74413R_REG_DAC_CODE_X(x) (0x16 + (x))
> +#define AD74413R_DAC_CODE_MAX ((1 << 13) - 1)
> +
> +#define AD74413R_REG_GPO_PAR_DATA 0x0d
> +#define AD74413R_REG_GPO_CONFIG_X(x) (0x0e + (x))
> +#define AD74413R_GPO_CONFIG_GPO_DATA_MASK BIT(3)
> +#define AD74413R_GPO_CONFIG_DATA_LOW 0
> +#define AD74413R_GPO_CONFIG_DATA_HIGH AD74413R_GPO_CONFIG_GPO_DATA_MASK
> +#define AD74413R_GPO_CONFIG_GPO_SELECT_MASK GENMASK(2, 0)
> +
> +#define AD74413R_REG_ADC_CONV_CTRL 0x23
> +#define AD74413R_CONV_SEQ_MASK GENMASK(9, 8)
> +#define AD74413R_CONV_SEQ_ON 0b00
> +#define AD74413R_CONV_SEQ_SINGLE 0b01
> +#define AD74413R_CONV_SEQ_CONTINUOUS 0b10
> +#define AD74413R_CONV_SEQ_OFF 0b11
> +#define AD74413R_CH_EN_MASK(x) BIT(x)
> +#define AD74413R_CH_EN_SHIFT(x) x
> +
> +#define AD74413R_REG_DIN_COMP_OUT 0x25
> +#define AD74413R_DIN_COMP_OUT_MASK_X(x) BIT(x)
> +#define AD74413R_DIN_COMP_OUT_SHIFT_X(x) x
> +
> +#define AD74413R_REG_ADC_RESULT_X(x) (0x26 + (x))
> +#define AD74413R_ADC_RESULT_MAX ((1 << 16) - 1)
> +
> +#define AD74413R_REG_READ_SELECT 0x41
> +
> +#define AD74413R_REG_CMD_KEY 0x44
> +#define AD74413R_CMD_KEY_LDAC 0x953a
> +
> +#define AD74413R_REG_SILLICON_REV 0x46
> +#define AD74413R_SILLICON_REV_ID_MASK GENMASK(7, 0)
> +#define AD74413R_SILLICON_REV_ID_EXPECTED 0x8
> +
> +#define AD74413R_ADC_DATA_TIMEOUT 1000
> +
> +static const int ad74413r_adc_sampling_rates[] = {
> + 20,
> + 4800,
> +};
> +
> +static const int ad74413r_adc_sampling_rates_hart[] = {
> + 10,
> + 20,
> + 1200,
> + 4800,
> +};
> +
> +static int ad74413r_crc(u8 *buf)
> +{
> + return crc8(ad74413r_crc8_table, buf, 3, 0);
> +}
> +
> +static void ad74413r_format_reg_write(unsigned int reg, unsigned int val, u8 *buf)
> +{
> + buf[0] = reg & 0xff;
As it's a u8, why not make the parameter the right type?
> + buf[1] = (val >> 8) & 0xff;
put_unaligned_be16()
> + buf[2] = val & 0xff;
> + buf[3] = ad74413r_crc(buf);
> +}
> +
> +static int ad74413r_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> + struct device *dev = context;
> + struct spi_device *spi = to_spi_device(dev);
> + struct ad74413r_state *st = spi_get_drvdata(spi);
> + u8 *buf = (u8 *)&st->regmap_tx_buf;
> +
> + ad74413r_format_reg_write(reg, val, buf);
> +
> + return spi_write(spi, buf, 4);
> +}
> +
> +static int ad74413r_crc_check(struct ad74413r_state *st, u8 *buf)
> +{
> + u8 expected_crc = ad74413r_crc(buf);
> +
> + if (buf[3] != expected_crc) {
> + dev_err(st->dev, "Bad CRC, data: %02x%02x%02x, expected: %02x, received: %02x\n",
> + buf[0], buf[1], buf[2], expected_crc, buf[3]);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int ad74413r_reg_read_raw(void *context, unsigned int reg, u8 *buf,
> + unsigned int count)
> +{
> + struct device *dev = context;
Taking the raw device doesn't make much sense here. Make it take a type
parameter and a more useful one at that.
> + struct spi_device *spi = to_spi_device(dev);
> + struct ad74413r_state *st = spi_get_drvdata(spi);
> + unsigned int i;
> + int ret;
> +
> + ret = ad74413r_reg_write(context, AD74413R_REG_READ_SELECT, reg);
> + if (ret) {
> + dev_err(dev, "Failed to write read select register: %d\n", ret);
> + return ret;
> + }
> +
> + for (i = 0; i < count; i++, buf += 4) {
> + ret = spi_read(spi, buf, 4);
> + if (ret)
> + return ret;
> +
> + ret = ad74413r_crc_check(st, buf);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ad74413r_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> + struct device *dev = context;
> + struct spi_device *spi = to_spi_device(dev);
> + struct ad74413r_state *st = spi_get_drvdata(spi);
> + u8 *buf = (u8 *)&st->regmap_rx_buf;
> + int ret;
> +
> + ret = ad74413r_reg_read_raw(context, reg, buf, 1);
> + if (ret)
> + return ret;
> +
> + *val = (buf[1] << 8) | buf[2];
get_unaligned_be16()
> +
> + return 0;
> +}
> +
> +const struct regmap_config ad74413r_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 16,
> + .reg_read = ad74413r_reg_read,
> + .reg_write = ad74413r_reg_write,
> +};
> +
> +static int ad74413r_set_gpo_mode(struct ad74413r_state *st, unsigned int offset, u8 mode)
> +{
> + if (mode < GPO_CONFIG_MIN || mode > GPO_CONFIG_MAX) {
> + dev_err(st->dev, "Invalid gpo config select mode\n");
Can we avoid getting here with values that will trigger this?
> + return -EINVAL;
> + }
> +
> + return regmap_update_bits(st->regmap, AD74413R_REG_GPO_CONFIG_X(offset),
> + AD74413R_GPO_CONFIG_GPO_SELECT_MASK, mode);
> +}
> +
> +static void ad74413r_gpio_set(struct gpio_chip *chip, unsigned int offset, int val)
> +{
> + struct ad74413r_state *st = gpiochip_get_data(chip);
> + struct ad74413r_channel_config *channel_config = &st->channel_configs[offset];
> + int ret;
> +
> + if (channel_config->gpo_config != GPO_CONFIG_LOGIC) {
> + dev_err(st->dev, "Cannot set gpo %u, not in logic mode", offset);
> + return;
> + }
> +
> + ret = ad74413r_set_gpo_mode(st, offset, GPO_CONFIG_LOGIC);
> + if (ret)
> + return;
> +
> + regmap_update_bits(st->regmap, AD74413R_REG_GPO_CONFIG_X(offset),
> + AD74413R_GPO_CONFIG_GPO_DATA_MASK,
> + val ? AD74413R_GPO_CONFIG_DATA_HIGH
> + : AD74413R_GPO_CONFIG_DATA_LOW);
> +}
> +
> +static void ad74413r_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask,
> + unsigned long *bits)
> +{
> + struct ad74413r_state *st = gpiochip_get_data(chip);
> + unsigned int offset = 0;
> + int ret;
> +
> + for_each_set_bit_from(offset, mask, AD74413R_CHANNEL_MAX) {
> + struct ad74413r_channel_config *channel_config = &st->channel_configs[offset];
> +
> + if (channel_config->gpo_config != GPO_CONFIG_LOGIC) {
> + dev_err(st->dev, "Cannot set gpo %u, not in logic mode\n", offset);
As below, this should be prevented at probe time not runtime.
> + continue;
> + }
> +
> + ret = ad74413r_set_gpo_mode(st, offset, GPO_CONFIG_LOGIC_PARALLEL);
> + if (ret)
> + return;
> + }
> +
> + regmap_update_bits(st->regmap, AD74413R_REG_GPO_PAR_DATA, *mask, *bits);
> +}
> +
> +static int ad74413r_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct ad74413r_state *st = gpiochip_get_data(chip);
> + struct ad74413r_channel_config *channel_config = &st->channel_configs[offset];
> + unsigned int status;
> + int ret;
> +
> + if (channel_config->gpo_config != GPO_CONFIG_DEBOUNCED_COMPARATOR) {
> + dev_err(st->dev, "Cannot get gpo %u, not in comparator mode\n", offset);
Rule it out in the dt binding and enforce it at probe time not here which is far too late.
> + return -EINVAL;
> + }
> +
> + ret = regmap_read(st->regmap, AD74413R_REG_DIN_COMP_OUT, &status);
> + if (ret)
> + return ret;
> +
> + status &= AD74413R_DIN_COMP_OUT_MASK_X(offset);
> + status >>= AD74413R_DIN_COMP_OUT_SHIFT_X(offset);
As you are only checking one bit.
return status & BIT(offset);
If gpio needs a 0/1, then ? 1 : 0;
> +
> + return status;
> +}
> +
> +static int ad74413r_set_channel_dac_code(struct ad74413r_state *st, unsigned int channel,
> + uint16_t dac_code)
> +{
> + int ret;
> +
> + if (dac_code > AD74413R_DAC_CODE_MAX) {
> + dev_err(st->dev, "Invalid dac code %d\n", dac_code);
> + return -EINVAL;
> + }
> +
> + ret = regmap_write(st->regmap, AD74413R_REG_DAC_CODE_X(channel), dac_code);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD74413R_REG_CMD_KEY, AD74413R_CMD_KEY_LDAC);
> + if (ret)
> + return ret;
> +
> + return 0;
return regmap_write()
> +}
> +
> +static int ad74413r_channel_set_function(struct ad74413r_state *st, unsigned int channel, u8 func)
> +{
> + if (func < CH_FUNC_MIN || func > CH_FUNC_MAX) {
> + dev_err(st->dev, "Invalid channel function %d\n", func);
> + return -EINVAL;
> + }
> +
> + if (!st->config->hart_support && (func == CH_FUNC_CURRENT_INPUT_EXT_POWER_HART
> + || func == CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART)) {
> + dev_err(st->dev, "HART function not supported %d\n", func);
Move to caller perhaps? I'd not expect a function at this level to have to reject inputs.
> + return -EINVAL;
> + }
> +
> + return regmap_update_bits(st->regmap, AD74413R_REG_CH_FUNC_SETUP_X(channel),
> + AD74413R_CH_FUNC_SETUP_MASK, func);
> +}
> +
> +static int ad74413r_set_adc_conv_seq(struct ad74413r_state *st, unsigned int status)
> +{
> + int ret;
> +
> + /*
> + * These bits do not clear when a conversion completes.
> + * To enable a subsequent conversion, repeat the write to enable the conversion.
> + */
> + ret = regmap_write_bits(st->regmap, AD74413R_REG_ADC_CONV_CTRL,
> + AD74413R_CONV_SEQ_MASK,
> + FIELD_PREP(AD74413R_CONV_SEQ_MASK, status));
> + if (ret)
> + return ret;
> +
> + // Wait 100us before starting conversions
/* */ format for all comments in IIO (other than SPDX where there are cases that mean it has to be different...)
> + usleep_range(100, 120);
> +
> + return 0;
> +}
> +
> +static int ad74413r_set_adc_channel_enable(struct ad74413r_state *st, unsigned int channel,
> + bool status)
> +{
> + unsigned int val = status << AD74413R_CH_EN_SHIFT(channel);
> +
> + return regmap_update_bits(st->regmap, AD74413R_REG_ADC_CONV_CTRL,
> + AD74413R_CH_EN_MASK(channel), val);
> +}
> +
> +static int ad74413r_get_adc_range(struct ad74413r_state *st, unsigned int channel,
> + unsigned int *val)
> +{
> + int ret;
> +
> + ret = regmap_read(st->regmap, AD74413R_REG_ADC_CONFIG_X(channel), val);
> + if (ret)
> + return ret;
> +
> + *val = FIELD_GET(AD74413R_ADC_CONFIG_RANGE_MASK, *val);
> +
> + return 0;
> +}
> +
> +static int ad74413r_get_adc_rejection(struct ad74413r_state *st, unsigned int channel,
> + unsigned int *val)
> +{
> + int ret;
> +
> + ret = regmap_read(st->regmap, AD74413R_REG_ADC_CONFIG_X(channel), val);
> + if (ret)
> + return ret;
> +
> + *val = FIELD_GET(AD74413R_ADC_CONFIG_REJECTION_MASK, *val);
> +
> + return 0;
> +}
> +
> +static int ad74413r_set_adc_rejection(struct ad74413r_state *st, unsigned int channel,
> + unsigned int val)
> +{
> + if (!st->config->hart_support && (val == AD74413R_ADC_REJECTION_50_60_HART
> + || val == AD74413R_ADC_REJECTION_HART)) {
Perhaps makes more sense to move this check to the caller?
> + dev_err(st->dev, "HART rate not supported %d\n", val);
> + return -EINVAL;
> + }
> +
> + return regmap_update_bits(st->regmap, AD74413R_REG_ADC_CONFIG_X(channel),
> + AD74413R_ADC_CONFIG_REJECTION_MASK,
> + FIELD_PREP(AD74413R_ADC_CONFIG_REJECTION_MASK, val));
> +}
> +
> +static int ad74413r_get_adc_rate(struct ad74413r_state *st, unsigned int channel, int *val)
> +{
> + unsigned int rej;
> + int ret;
> +
> + ret = ad74413r_get_adc_rejection(st, channel, &rej);
> + if (ret)
> + return ret;
> +
> + switch (rej) {
> + case AD74413R_ADC_REJECTION_50_60:
> + *val = 20;
> + break;
> + case AD74413R_ADC_REJECTION_NONE:
> + *val = 4800;
> + break;
> + case AD74413R_ADC_REJECTION_50_60_HART:
> + *val = 10;
> + break;
> + case AD74413R_ADC_REJECTION_HART:
> + *val = 1200;
> + break;
> + default:
> + dev_err(st->dev, "Channel %u ADC rejection invalid\n", channel);
> + return -EINVAL;
> + }
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int ad74413r_set_adc_rate(struct ad74413r_state *st, unsigned int channel, int val)
> +{
> + unsigned int rej;
> +
> + switch (val) {
> + case 20:
> + rej = AD74413R_ADC_REJECTION_50_60;
> + break;
> + case 4800:
> + rej = AD74413R_ADC_REJECTION_NONE;
> + break;
> + case 10:
> + rej = AD74413R_ADC_REJECTION_50_60_HART;
> + break;
> + case 1200:
> + rej = AD74413R_ADC_REJECTION_HART;
> + break;
> + default:
> + dev_err(st->dev, "Channel %u ADC rate invalid\n", channel);
> + return -EINVAL;
> + }
> +
> + return ad74413r_set_adc_rejection(st, channel, rej);
> +}
> +
> +static int ad74413r_get_adc_voltage_range(struct ad74413r_state *st, unsigned int channel,
> + int *val)
> +{
> + unsigned int range;
> + int ret;
> +
> + ret = ad74413r_get_adc_range(st, channel, &range);
> + if (ret)
> + return ret;
> +
> + switch (range) {
> + case AD74413R_ADC_RANGE_10V:
> + *val = 10000;
> + break;
> + case AD74413R_ADC_RANGE_2P5V_EXT_POW:
> + case AD74413R_ADC_RANGE_2P5V_INT_POW:
> + *val = 2500;
> + break;
> + case AD74413R_ADC_RANGE_5V_BI_DIR:
> + *val = 5000;
> + break;
> + default:
> + dev_err(st->dev, "Channel %u ADC range invalid\n", channel);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int ad74413r_get_adc_voltage_offset(struct ad74413r_state *st, unsigned int channel,
> + int *val)
> +{
> + unsigned int range;
> + int ret;
> +
> + ret = ad74413r_get_adc_range(st, channel, &range);
> + if (ret)
> + return ret;
> +
> + switch (range) {
> + case AD74413R_ADC_RANGE_10V:
> + case AD74413R_ADC_RANGE_2P5V_EXT_POW:
> + *val = 0;
> + break;
> + case AD74413R_ADC_RANGE_2P5V_INT_POW:
> + case AD74413R_ADC_RANGE_5V_BI_DIR:
> + *val = -2500;
> + break;
> + default:
> + dev_err(st->dev, "Channel %u ADC range invalid\n", channel);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int ad74413r_get_input_voltage_scale(struct ad74413r_state *st, unsigned int channel,
> + int *val, int *val2)
> +{
> + int ret;
> +
> + ret = ad74413r_get_adc_voltage_range(st, channel, val);
> + if (ret)
> + return ret;
> +
> + *val2 = AD74413R_ADC_RESULT_MAX;
interesting. Often these are 1/(N**2) rather than 1/(N**2 - 1) as here.
Noise level is probably high enough this doesn't really matter though.
Data sheets are never entirely clear if ranges are inclusive or not...
> +
> + return IIO_VAL_FRACTIONAL;
> +}
> +
> +static int ad74413r_get_input_voltage_offset(struct ad74413r_state *st, unsigned int channel,
> + int *val, int *val2)
> +{
> + unsigned int range;
> + int ret;
> +
> + ret = ad74413r_get_adc_range(st, channel, &range);
> + if (ret)
> + return ret;
> +
> + switch (range) {
> + case AD74413R_ADC_RANGE_10V:
> + case AD74413R_ADC_RANGE_2P5V_EXT_POW:
> + *val = 0;
> + break;
> + case AD74413R_ADC_RANGE_2P5V_INT_POW:
> + *val = -AD74413R_ADC_RESULT_MAX;
> + break;
> + case AD74413R_ADC_RANGE_5V_BI_DIR:
> + *val = -AD74413R_ADC_RESULT_MAX / 2;
> + break;
> + default:
> + dev_err(st->dev, "Channel %u ADC range invalid\n", channel);
> + return -EINVAL;
> + }
> +
> + *val2 = 0;
As below.
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int ad74413r_get_input_current_scale(struct ad74413r_state *st, unsigned int channel,
> + int *val, int *val2)
> +{
> + int ret;
> +
> + ret = ad74413r_get_adc_voltage_range(st, channel, val);
> + if (ret)
> + return ret;
> +
> + *val2 = AD74413R_ADC_RESULT_MAX * st->rsense_resistance_ohms;
> +
> + return IIO_VAL_FRACTIONAL;
> +}
> +
> +static int ad74413_get_input_current_offset(struct ad74413r_state *st, unsigned int channel,
> + int *val, int *val2)
> +{
> + int range;
> + int offset;
> + int ret;
> +
> + ret = ad74413r_get_adc_voltage_range(st, channel, &range);
> + if (ret)
> + return ret;
> +
> + ret = ad74413r_get_adc_voltage_offset(st, channel, &offset);
> + if (ret)
> + return ret;
> +
> + *val = offset * AD74413R_ADC_RESULT_MAX / range;
> + *val2 = 0;
No need to set *val2 when IIO_VAL_INT being returned. The IIO core won't read it.
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int ad74413r_get_output_voltage_scale(struct ad74413r_state *st,
> + int *val, int *val2)
> +{
> + *val = 11000;
> + *val2 = AD74413R_DAC_CODE_MAX;
> +
> + return IIO_VAL_FRACTIONAL;
> +}
> +
> +static int ad74413r_get_output_current_scale(struct ad74413r_state *st, int *val, int *val2)
> +{
> + *val = regulator_get_voltage(st->refin_reg);
> + *val2 = st->rsense_resistance_ohms * AD74413R_DAC_CODE_MAX * 1000;
> +
> + return IIO_VAL_FRACTIONAL;
> +}
> +
> +static irqreturn_t ad74413r_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct ad74413r_state *st = iio_priv(indio_dev);
> + unsigned int i;
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + ret = spi_sync(st->spi, &st->adc_samples_msg);
> + if (ret)
> + goto out;
> +
> + for (i = 0; i < st->adc_active_channels; i++)
> + ad74413r_crc_check(st, (u8 *)&st->adc_samples.rx_buf[i]);
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, &st->adc_samples,
> + iio_get_time_ns(indio_dev));
> +
> +out:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + mutex_unlock(&st->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t ad74413r_adc_data_interrupt(int irq, void *data)
> +{
> + struct ad74413r_state *st = data;
> +
> + if (iio_buffer_enabled(st->indio_dev))
> + iio_trigger_poll(st->trig);
> + else
> + complete(&st->adc_data_completion);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ad74413r_get_single_adc_result(struct ad74413r_state *st, unsigned int channel,
> + int *val)
> +{
> + unsigned int uval;
> + int ret;
> +
> + ret = iio_device_claim_direct_mode(st->indio_dev);
Pass indio_dev into these functions instead of st then navigate the other way
(which can be worked out at compile time rather than runtime).
> + if (ret)
> + return ret;
> +
> + reinit_completion(&st->adc_data_completion);
> +
> + ret = ad74413r_set_adc_channel_enable(st, channel, true);
> + if (ret)
> + goto out;
> +
> + ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_SINGLE);
> + if (ret)
> + goto out;
> +
> + ret = wait_for_completion_timeout(&st->adc_data_completion,
> + msecs_to_jiffies(AD74413R_ADC_DATA_TIMEOUT));
> + if (!ret) {
> + ret = -ETIMEDOUT;
> + goto out;
> + }
> +
> + ret = regmap_read(st->regmap, AD74413R_REG_ADC_RESULT_X(channel), &uval);
> + if (ret)
> + goto out;
> +
> + ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
> + if (ret)
> + goto out;
> +
> + ret = ad74413r_set_adc_channel_enable(st, channel, false);
> + if (ret)
> + goto out;
> +
> + *val = uval;
> + ret = IIO_VAL_INT;
> +
> +out:
> + iio_device_release_direct_mode(st->indio_dev);
hmm. You may have a potential deadlock here as mutex is taken outside of this
claim of direct_mode(). In update_scan_mode() you also take the mutex and that
can occur with the internal lock used for avoiding races in mode change also being
held. Hence you have lock inversion. IIRC the solution is to claim direct mode
before taking your local mutex. recent report for this for the dln2 adc driver.
https://lore.kernel.org/linux-iio/20211028153711.581fd9c8@jic23-huawei/T/#t
> +
> + return ret;
> +}
> +
> +static int ad74413r_get_single_resistance_result(struct ad74413r_state *st, unsigned int channel,
> + int *val)
> +{
> + unsigned int uval;
> + int ret;
> +
> + ret = ad74413r_get_single_adc_result(st, channel, &uval);
> + if (ret < 0)
> + return ret;
> +
> + if (uval == AD74413R_ADC_RESULT_MAX)
> + *val = 0;
> + else
> + *val = DIV_ROUND_CLOSEST(uval * 2100, AD74413R_ADC_RESULT_MAX - uval);
> +
> + return ret;
> +}
> +
> +static int ad74413r_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *active_scan_mask)
> +{
> + struct ad74413r_state *st = iio_priv(indio_dev);
> + struct spi_transfer *xfer;
> + unsigned int channel;
> + int transfer_index = 0;
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + spi_message_init(&st->adc_samples_msg);
> + st->adc_active_channels = 0;
> +
> + for (channel = 0; channel < AD74413R_CHANNEL_MAX; channel++) {
> + bool status = test_bit(channel, active_scan_mask);
Perhaps cleaner as a pair of loops for_each_set_bit() and for_each_clear_bit()
Something like.
for_each_clear_bit(channel, active_scan_mask, AD74413R_CHANNEL_MAX) {
ret = ad74413r_set_adc_channel_enable, st, channel, false);
if (ret)
return ret;
}
for_each_set_bit(channel, active_scan_mask, AD74413R_CHANNEL_MAX) {
u8 *tx_buf;
ret = ad74413r_set_adc_cannel_enable, st, channel, true);
if (ret)
return ret;
st->adc_active_channels++;
xfer = &st->adc_samples_xfer[transfer_index];
if (transfer_index == 0)
xfer->rx_buf = NULL;
...
}
> + u8 *tx_buf;
> +
> + ret = ad74413r_set_adc_channel_enable(st, channel, status);
> + if (ret)
> + goto out;
> +
> + if (!status)
> + continue;
> +
> + st->adc_active_channels++;
> +
> + xfer = &st->adc_samples_xfer[transfer_index];
> +
> + if (transfer_index == 0)
> + xfer->rx_buf = NULL;
> + else
> + xfer->rx_buf = &st->adc_samples.rx_buf[transfer_index - 1];
> +
> + tx_buf = (u8 *)&st->adc_samples_tx_buf[transfer_index];
> +
> + xfer->tx_buf = tx_buf;
> + xfer->len = 4;
> + xfer->cs_change = 1;
> +
> + ad74413r_format_reg_write(AD74413R_REG_READ_SELECT,
> + AD74413R_REG_ADC_RESULT_X(channel),
> + tx_buf);
> +
> + spi_message_add_tail(xfer, &st->adc_samples_msg);
> +
> + transfer_index++;
> + }
> +
> + if (transfer_index == 0)
> + goto out;
> +
> + xfer = &st->adc_samples_xfer[transfer_index];
> +
> + xfer->tx_buf = NULL;
> + xfer->rx_buf = &st->adc_samples.rx_buf[transfer_index - 1];
> + xfer->len = 4;
> + xfer->cs_change = 0;
I guess this is a terminating message - add a comment on why it is needed.
> +
> + spi_message_add_tail(xfer, &st->adc_samples_msg);
> +
> +out:
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> +static int ad74413r_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct ad74413r_state *st = iio_priv(indio_dev);
> +
> + return ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_CONTINUOUS);
> +}
> +
> +static int ad74413r_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct ad74413r_state *st = iio_priv(indio_dev);
> +
> + return ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
> +}
> +
> +static int ad74413r_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> + int *val, int *val2, long info)
> +{
> + struct ad74413r_state *st = iio_priv(indio_dev);
> + int ret = -EINVAL;
Whilst it is more code I'd prefer to see this set is explicitly in the error paths.
> +
> + mutex_lock(&st->lock);
It may be better to move this further down the tree so as to avoid locking in paths
were we aren't doing anything + to allow more direct returns avoiding the need
to read all the way to the end of the function to see what happens in a given path.
> +
> + switch (info) {
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_ALTVOLTAGE:
> + if (chan->output)
> + ret = ad74413r_get_output_voltage_scale(st, val, val2);
> + else
> + ret = ad74413r_get_input_voltage_scale(st, chan->channel,
> + val, val2);
> + break;
> +
> + case IIO_CURRENT:
> + if (chan->output)
> + ret = ad74413r_get_output_current_scale(st, val, val2);
> + else
> + ret = ad74413r_get_input_current_scale(st, chan->channel,
> + val, val2);
> + break;
> + default:
> + break;
> + }
> + break;
> + case IIO_CHAN_INFO_OFFSET:
> + switch (chan->type) {
> + case IIO_ALTVOLTAGE:
> + if (!chan->output)
> + ret = ad74413r_get_input_voltage_offset(st, chan->channel,
> + val, val2);
> + break;
> + case IIO_CURRENT:
> + if (!chan->output)
> + ret = ad74413_get_input_current_offset(st, chan->channel,
> + val, val2);
> + break;
> + default:
> + break;
> + }
> + break;
> + case IIO_CHAN_INFO_RAW:
> + if (!chan->output)
> + ret = ad74413r_get_single_adc_result(st, chan->channel, val);
> + break;
> + case IIO_CHAN_INFO_PROCESSED:
> + switch (chan->type) {
> + case IIO_RESISTANCE:
> + if (!chan->output)
flip this to
if (chan->output) {
ret = -EINVAL;
break;
}
to reduce the indentation. Handling errors rather than good paths out of line of the main
flow is also easier to read.
> + ret = ad74413r_get_single_resistance_result(st, chan->channel,
> + val);
> + break;
> + default:
> + break;
> + }
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (!chan->output)
> + ret = ad74413r_get_adc_rate(st, chan->channel, val);
> + break;
> + default:
> + break;
> + }
> +
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> +static int ad74413r_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> + int val, int val2, long info)
> +{
> + struct ad74413r_state *st = iio_priv(indio_dev);
> + int ret = -EINVAL;
> +
> + mutex_lock(&st->lock);
I would move the mutex into the individual branches. That will let you return
errors directly from all other paths which is easier to read.
> +
> + switch (info) {
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->type) {
> + case IIO_ALTVOLTAGE:
> + case IIO_CURRENT:
> + ret = ad74413r_set_channel_dac_code(st, chan->channel, val);
> + break;
> + default:
> + break;
> + }
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (!chan->output)
> + ret = ad74413r_set_adc_rate(st, chan->channel, val);
> + break;
> + default:
> + break;
> + }
> +
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> +static int ad74413r_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length, long info)
> +{
> + struct ad74413r_state *st = iio_priv(indio_dev);
> + int ret = -EINVAL;
> +
> + switch (info) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (st->config->hart_support) {
> + *vals = ad74413r_adc_sampling_rates_hart;
> + *length = ARRAY_SIZE(ad74413r_adc_sampling_rates_hart);
> + } else {
> + *vals = ad74413r_adc_sampling_rates;
> + *length = ARRAY_SIZE(ad74413r_adc_sampling_rates);
> + }
> + *type = IIO_VAL_INT;
> + ret = IIO_AVAIL_LIST;
> + break;
> + default:
> + break;
> + }
> +
> + return ret;
put direct returns in the switch case statements rather than down here.
> +}
> +
> +static const struct iio_buffer_setup_ops ad74413r_buffer_ops = {
> + .postenable = &ad74413r_buffer_postenable,
> + .predisable = &ad74413r_buffer_predisable,
> +};
> +
> +static const struct iio_trigger_ops ad74413r_trigger_ops = {
> + .validate_device = iio_trigger_validate_own_device,
> +};
> +
> +static const struct iio_info ad74413r_info = {
> + .read_raw = &ad74413r_read_raw,
> + .write_raw = &ad74413r_write_raw,
> + .read_avail = &ad74413r_read_avail,
> + .update_scan_mode = &ad74413r_update_scan_mode,
> +};
> +
> +#define AD74413R_CHANNEL(_type, _output, extra_mask_separate) \
> + .type = _type, \
> + .indexed = 1, \
> + .output = _output, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | extra_mask_separate
> +
> +#define AD74413R_DAC_CHANNEL(_type, extra_mask_separate) \
> + { \
> + AD74413R_CHANNEL(_type, 1, extra_mask_separate), \
> + }
> +
> +#define AD74413R_ADC_CHANNEL(_type, extra_mask_separate) \
> + { \
> + AD74413R_CHANNEL(_type, 0, extra_mask_separate | BIT(IIO_CHAN_INFO_SAMP_FREQ)), \
> + .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 16, \
> + .storagebits = 32, \
> + .shift = 8, \
> + .endianness = IIO_BE, \
> + }, \
> + }
> +
> +#define AD74413R_ADC_VOLTAGE_CHANNEL \
> + AD74413R_ADC_CHANNEL(IIO_ALTVOLTAGE, BIT(IIO_CHAN_INFO_SCALE) \
> + | BIT(IIO_CHAN_INFO_OFFSET))
> +
> +#define AD74413R_ADC_CURRENT_CHANNEL \
> + AD74413R_ADC_CHANNEL(IIO_CURRENT, BIT(IIO_CHAN_INFO_SCALE) \
> + | BIT(IIO_CHAN_INFO_OFFSET))
> +
> +static struct iio_chan_spec ad74413r_high_impedance_channels[] = {
> + AD74413R_ADC_VOLTAGE_CHANNEL,
> +};
> +
> +static struct iio_chan_spec ad74413r_voltage_output_channels[] = {
> + AD74413R_DAC_CHANNEL(IIO_ALTVOLTAGE, BIT(IIO_CHAN_INFO_SCALE)),
> + AD74413R_ADC_CURRENT_CHANNEL,
These would benefit from explanatory comments as not obvious why a DAC
would have a paired ADC. I'm also curious why this is altvoltage
rather than voltage (DC).
> +};
> +
> +static struct iio_chan_spec ad74413r_current_output_channels[] = {
> + AD74413R_DAC_CHANNEL(IIO_CURRENT, BIT(IIO_CHAN_INFO_SCALE)),
> + AD74413R_ADC_VOLTAGE_CHANNEL,
> +};
> +
> +static struct iio_chan_spec ad74413r_voltage_input_channels[] = {
> + AD74413R_ADC_VOLTAGE_CHANNEL,
> +};
> +
> +static struct iio_chan_spec ad74413r_current_input_channels[] = {
> + AD74413R_ADC_CURRENT_CHANNEL,
> +};
> +
> +static struct iio_chan_spec ad74413r_resistance_input_channels[] = {
> + AD74413R_ADC_CHANNEL(IIO_RESISTANCE, BIT(IIO_CHAN_INFO_PROCESSED)),
> +};
> +
> +static struct iio_chan_spec ad74413r_digital_input_channels[] = {
> + AD74413R_ADC_VOLTAGE_CHANNEL,
> +};
> +
> +static int ad74413r_of_parse_channel_config(struct ad74413r_state *st,
> + struct device_node *channel_node)
> +{
> + struct ad74413r_channel_config *channel_config;
> + u32 index;
> + int ret;
> +
> + ret = of_property_read_u32(channel_node, "reg", &index);
Nothing in here looks to be device tree specific. Please change to using
the generic firmware property functions in include/linux/property.h
> + if (ret) {
> + dev_err(st->dev, "Failed to read channel reg: %d\n", ret);
> + return ret;
> + }
> +
> + if (index > AD74413R_CHANNEL_MAX) {
> + dev_err(st->dev, "Channel index %u is too large\n", index);
> + return -EINVAL;
> + }
> +
> + channel_config = &st->channel_configs[index];
> + if (channel_config->initialized) {
> + dev_err(st->dev, "Channel %u has already been initialized\n", index);
> + return -EINVAL;
> + }
> +
> + channel_config->func = CH_FUNC_HIGH_IMPEDANCE;
> + of_property_read_u32(channel_node, "adi,ch-func", &channel_config->func);
> +
> + channel_config->gpo_config = GPO_CONFIG_100K_PULL_DOWN;
> + of_property_read_u32(channel_node, "adi,gpo-config", &channel_config->gpo_config);
> +
> + if (channel_config->gpo_config == GPO_CONFIG_LOGIC_PARALLEL) {
> + dev_err(st->dev, "GPO config logic parallel is unsupported\n");
> + return -EINVAL;
Error out of this function is fine, but why not just leave this channel disabled
rather than failing to probe which I think is what will currently happen?
> + }
> +
> + ret = ad74413r_channel_set_function(st, index, channel_config->func);
> + if (ret)
> + return ret;
> +
> + ret = ad74413r_set_gpo_mode(st, index, channel_config->gpo_config);
Not all of these seem likely to be valid for all types of channel.
> + if (ret)
> + return ret;
> +
> + channel_config->initialized = true;
> +
> + return 0;
> +}
> +
> +static int ad74413r_parse_channel_configs(struct ad74413r_state *st)
> +{
> + struct device_node *channel_node = NULL;
> + int ret;
> +
> + for_each_available_child_of_node(st->dev->of_node, channel_node) {
> + if (!of_node_name_eq(channel_node, "channel"))
> + continue;
I'm not sure we should be doing this matching on node name. If you need
to do that, then add some sort of compatible rather than this.
> +
> + ret = ad74413r_of_parse_channel_config(st, channel_node);
> + if (ret) {
> + dev_err(st->dev, "Failed to parse channel %s config: %d\n",
> + channel_node->name, ret);
> + goto put_channel_node;
> + }
> + }
> +
> +put_channel_node:
> + of_node_put(channel_node);
I think you should only do this if you break out of the loop without finishing it -
so error path only.
> +
> + return ret;
> +}
> +
> +static int ad74413r_get_iio_channels(struct ad74413r_state *st, u8 func,
> + struct iio_chan_spec **iio_channels, unsigned int *count)
> +{
> + switch (func) {
> + case CH_FUNC_HIGH_IMPEDANCE:
Perhaps turn this function into an array lookup?
> + *iio_channels = ad74413r_high_impedance_channels;
> + *count = ARRAY_SIZE(ad74413r_high_impedance_channels);
> + break;
> + case CH_FUNC_VOLTAGE_OUTPUT:
> + *iio_channels = ad74413r_voltage_output_channels;
> + *count = ARRAY_SIZE(ad74413r_voltage_output_channels);
> + break;
> + case CH_FUNC_CURRENT_OUTPUT:
> + *iio_channels = ad74413r_current_output_channels;
> + *count = ARRAY_SIZE(ad74413r_current_output_channels);
> + break;
> + case CH_FUNC_VOLTAGE_INPUT:
> + *iio_channels = ad74413r_voltage_input_channels;
> + *count = ARRAY_SIZE(ad74413r_voltage_input_channels);
> + break;
> + case CH_FUNC_CURRENT_INPUT_EXT_POWER:
> + case CH_FUNC_CURRENT_INPUT_LOOP_POWER:
> + case CH_FUNC_CURRENT_INPUT_EXT_POWER_HART:
> + case CH_FUNC_CURRENT_INPUT_LOOP_POWER_HART:
> + *iio_channels = ad74413r_current_input_channels;
> + *count = ARRAY_SIZE(ad74413r_current_input_channels);
> + break;
> + case CH_FUNC_RESISTANCE_INPUT:
> + *iio_channels = ad74413r_resistance_input_channels;
> + *count = ARRAY_SIZE(ad74413r_resistance_input_channels);
> + break;
> + case CH_FUNC_DIGITAL_INPUT_LOGIC:
> + case CH_FUNC_DIGITAL_INPUT_LOOP_POWER:
> + *iio_channels = ad74413r_digital_input_channels;
> + *count = ARRAY_SIZE(ad74413r_digital_input_channels);
> + break;
> + default:
> + dev_err(st->dev, "Unhandled channel config function %d\n", func);
> + return -EINVAL;
> + }
> +
> + return 0;
return 0 where you break above so we don't have to look down here to find out what
happens.
> +}
> +
> +static int ad74413r_count_iio_channels(struct ad74413r_state *st)
> +{
> + unsigned int index;
> + int ret;
> +
> + st->indio_dev->num_channels = 0;
> +
> + for (index = 0; index < AD74413R_CHANNEL_MAX; index++) {
i works for a local index variable.
> + struct ad74413r_channel_config *channel_config = &st->channel_configs[index];
> + struct iio_chan_spec *local_channels;
> + unsigned int num_local_channels;
chans and num_chans are verbose enough names for these to not be confusing.
> +
> + ret = ad74413r_get_iio_channels(st, channel_config->func, &local_channels,
> + &num_local_channels);
> + if (ret)
> + return ret;
> +
> + st->indio_dev->num_channels += num_local_channels;
> + }
> +
> + return 0;
> +}
> +
> +static int ad74413r_setup_iio_channels(struct ad74413r_state *st)
Pass in the struct iio_dev, not st to avoid need for st->indio_dev
loop that you currently have.
> +{
> + struct iio_chan_spec *channels;
> + unsigned int index;
> + int ret;
> +
> + channels = devm_kcalloc(st->dev, sizeof(*channels),
> + st->indio_dev->num_channels, GFP_KERNEL);
> + if (!channels)
> + return -ENOMEM;
> +
> + st->indio_dev->channels = channels;
> +
> + for (index = 0; index < AD74413R_CHANNEL_MAX; index++) {
> + struct ad74413r_channel_config *channel_config = &st->channel_configs[index];
> + struct iio_chan_spec *local_channels;
> + unsigned int num_local_channels;
> + unsigned int chan_index;
> +
> + ret = ad74413r_get_iio_channels(st, channel_config->func, &local_channels,
> + &num_local_channels);
get usually implies some sort of reference count. Perhaps find is more appropriate?
> + if (ret)
> + return ret;
> +
> + memcpy(channels, local_channels, num_local_channels * sizeof(*local_channels));
Try to keep line lengths under 80 chars except where that would significantly hurt readability.
> +
> + for (chan_index = 0; chan_index < num_local_channels; chan_index++) {
> + struct iio_chan_spec *local_channel = &channels[chan_index];
local_chan is a long name for this - perhaps just call it chan
> +
> + local_channel->channel = index;
> + if (local_channel->output)
> + local_channel->scan_index = -1;
> + else
> + local_channel->scan_index = index;
> +
> + }
> +
> + channels += num_local_channels;
> + }
> +
> + return 0;
> +}
> +
> +static void ad74413r_regulator_disable(void *regulator)
> +{
> + regulator_disable(regulator);
> +}
> +
> +static int ad74413r_probe(struct spi_device *spi)
> +{
> + struct ad74413r_state *st;
> + struct iio_dev *indio_dev;
> + unsigned int sillicon_rev_id;
> + const char *name;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (indio_dev == NULL)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + spi_set_drvdata(spi, st);
> +
> + st->spi = spi;
> + st->dev = &spi->dev;
> + st->config = of_device_get_match_data(&spi->dev);
> + st->indio_dev = indio_dev;
That is almost always a sign of a design issue. You should always
be navigating in one direction, not both. Normally seeing this implies
we have callbacks taking the wrong data structure for the model we are
using for IIO drivers.
> + mutex_init(&st->lock);
> + init_completion(&st->adc_data_completion);
> +
> + name = dev_name(st->dev);
> +
> + st->regmap = devm_regmap_init(st->dev, NULL, &spi->dev, &ad74413r_regmap_config);
> + if (IS_ERR(st->regmap)) {
> + ret = PTR_ERR(st->regmap);
> + dev_err(st->dev, "Failed to create regmap: %d\n", ret);
> + return ret;
> + }
> +
> + ret = regmap_read(st->regmap, AD74413R_REG_SILLICON_REV, &sillicon_rev_id);
> + if (ret) {
> + dev_err(st->dev, "Failed to read sillicon rev: %d\n", ret);
> + return ret;
> + }
> +
> + sillicon_rev_id = FIELD_GET(AD74413R_SILLICON_REV_ID_MASK, sillicon_rev_id);
> + if (sillicon_rev_id != AD74413R_SILLICON_REV_ID_EXPECTED) {
> + dev_err(st->dev, "Read sillicon rev id %d does not match expected rev id %d\n",
> + sillicon_rev_id, AD74413R_SILLICON_REV_ID_EXPECTED);
Not supporting future silicon revs is unusual...
> + return -EINVAL;
> + }
> +
> + st->refin_reg = devm_regulator_get(st->dev, "refin");
> + if (IS_ERR(st->refin_reg)) {
> + dev_err(st->dev, "Failed to get refin regulator: %d\n", ret);
return dev_err_probe() as it might be deferred and we both want to only print in real error
cases + stash the information needed to debug deferred probe issues.
> + return PTR_ERR(st->refin_reg);
> + }
> +
> + ret = regulator_enable(st->refin_reg);
> + if (ret) {
> + dev_err(st->dev, "Failed to enable refin regulator: %d\n", ret);
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(st->dev, ad74413r_regulator_disable,
> + st->refin_reg);
> + if (ret) {
> + dev_err(st->dev, "Failed to add refin regulator disable action: %d\n", ret);
Some error paths are incredibly unlikely to happen, so I would look at whether you can
reduce the debug prints in path such as this one.
> + return ret;
> + }
> +
> + ret = of_property_read_u32(st->dev->of_node, "adi,rsense-resistance-ohms",
> + &st->rsense_resistance_ohms);
As mentioned above, device_property_read_u32()..
> + if (ret) {
> + dev_err(st->dev, "Failed to get rsense resistance: %d\n", ret);
> + return ret;
> + }
> +
> + st->gpiochip.label = name;
> + st->gpiochip.base = -1;
> + st->gpiochip.ngpio = AD74413R_CHANNEL_MAX;
> + st->gpiochip.parent = st->dev;
> + st->gpiochip.can_sleep = true;
> + st->gpiochip.set = ad74413r_gpio_set;
> + st->gpiochip.set_multiple = ad74413r_gpio_set_multiple;
> + st->gpiochip.owner = THIS_MODULE;
> + st->gpiochip.get = ad74413r_gpio_get;
> +
> + ret = devm_gpiochip_add_data(st->dev, &st->gpiochip, st);
> + if (ret) {
> + dev_err(st->dev, "Failed to add gpio chip: %d\n", ret);
> + return ret;
> + }
> +
> + st->trig = devm_iio_trigger_alloc(st->dev, "%s-dev%d", name, indio_dev->id);
> + if (!st->trig)
> + return -ENOMEM;
> +
> + st->trig->ops = &ad74413r_trigger_ops;
> + st->trig->dev.parent = st->dev;
> + iio_trigger_set_drvdata(st->trig, st);
> +
> + ret = devm_iio_trigger_register(st->dev, st->trig);
> + if (ret)
> + return ret;
> +
> + indio_dev->dev.parent = st->dev;
Set by the IIO core for you by default.
> + indio_dev->name = name;
For IIO this need to be the part number. I'm not sure that's the case currently.
> + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> + indio_dev->info = &ad74413r_info;
> + indio_dev->trig = iio_trigger_get(st->trig);
> +
> + ret = ad74413r_parse_channel_configs(st);
> + if (ret)
> + return ret;
> +
> + ret = ad74413r_count_iio_channels(st);
> + if (ret)
> + return ret;
> +
> + ret = ad74413r_setup_iio_channels(st);
> + if (ret) {
> + dev_err(st->dev, "Failed to setup iio channels: %d\n", ret);
> + return ret;
> + }
> +
> + ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
> + if (ret)
> + return ret;
> +
> + ret = devm_request_irq(st->dev, spi->irq, ad74413r_adc_data_interrupt,
> + IRQF_TRIGGER_FALLING, name, st);
> + if (ret) {
> + dev_err(st->dev, "Failed to request threaded irq: %d\n", ret);
> + return ret;
> + }
> +
> + ret = devm_iio_triggered_buffer_setup(st->dev, indio_dev, &iio_pollfunc_store_time,
> + &ad74413r_trigger_handler, &ad74413r_buffer_ops);
> + if (ret) {
> + dev_err(st->dev, "Failed to setup triggered buffer: %d\n", ret);
> + return ret;
> + }
> +
> + return devm_iio_device_register(st->dev, indio_dev);
> +}
> +
> +static int ad74413r_unregister_driver(struct spi_driver *spi)
> +{
> + spi_unregister_driver(spi);
> +
> + return 0;
> +}
> +
> +static int __init ad74413r_register_driver(struct spi_driver *spi)
> +{
> + crc8_populate_msb(ad74413r_crc8_table, AD74413R_CRC_POLYNOMIAL);
> +
> + return spi_register_driver(spi);
> +}
> +
> +static const struct ad74413r_config ad74412r_config_data = {
> + .hart_support = false,
> +};
> +
> +static const struct ad74413r_config ad74413r_config_data = {
> + .hart_support = true,
> +};
> +
> +static const struct of_device_id ad74413r_dt_id[] = {
> + {
> + .compatible = "adi,ad74412r",
> + .data = &ad74412r_config_data,
> + },
> + {
> + .compatible = "adi,ad74413r",
> + .data = &ad74413r_config_data,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ad74413r_dt_id);
> +
> +static struct spi_driver ad74413r_driver = {
> + .driver = {
> + .name = "ad74413r",
> + .of_match_table = ad74413r_dt_id,
> + },
> + .probe = ad74413r_probe,
> +};
> +
> +module_driver(ad74413r_driver, ad74413r_register_driver, ad74413r_unregister_driver);
> +
> +MODULE_AUTHOR("Cosmin Tanislav <cosmin.tanislav@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD74413R ADDAC");
> +MODULE_LICENSE("GPL v2");
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] iio: add adddac subdirectory
2021-10-28 13:56 ` [PATCH v2 1/3] iio: add adddac subdirectory Cosmin Tanislav
2021-10-28 13:56 ` [PATCH v2 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
2021-10-28 13:56 ` [PATCH v2 3/3] iio: addac: add AD74413R driver Cosmin Tanislav
@ 2021-10-28 16:02 ` Jonathan Cameron
2021-10-29 8:00 ` Sa, Nuno
3 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2021-10-28 16:02 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: cosmin.tanislav, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, linux-iio, devicetree, linux-kernel
On Thu, 28 Oct 2021 16:56:03 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:
> From: Cosmin Tanislav <cosmin.tanislav@analog.com>
>
> For IIO devices that expose both ADC and DAC functionality.
>
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
Why v2?
Should be a change log (+ I would suggest a cover letter).
Also, reply to previous version if there haven't been comments that
you are replying to..
I'll assume most v1 comments apply to v2 and hence wait for v3.
Please leave some time for others to review these versions before
sending a v3.
Jonathan
> ---
> drivers/iio/Kconfig | 1 +
> drivers/iio/Makefile | 1 +
> drivers/iio/addac/Kconfig | 8 ++++++++
> drivers/iio/addac/Makefile | 6 ++++++
> 4 files changed, 16 insertions(+)
> create mode 100644 drivers/iio/addac/Kconfig
> create mode 100644 drivers/iio/addac/Makefile
>
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 2334ad249b46..4fb4321a72cb 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -70,6 +70,7 @@ config IIO_TRIGGERED_EVENT
>
> source "drivers/iio/accel/Kconfig"
> source "drivers/iio/adc/Kconfig"
> +source "drivers/iio/addac/Kconfig"
> source "drivers/iio/afe/Kconfig"
> source "drivers/iio/amplifiers/Kconfig"
> source "drivers/iio/cdc/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 65e39bd4f934..8d48c70fee4d 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_IIO_TRIGGERED_EVENT) += industrialio-triggered-event.o
>
> obj-y += accel/
> obj-y += adc/
> +obj-y += addac/
> obj-y += afe/
> obj-y += amplifiers/
> obj-y += buffer/
> diff --git a/drivers/iio/addac/Kconfig b/drivers/iio/addac/Kconfig
> new file mode 100644
> index 000000000000..2e64d7755d5e
> --- /dev/null
> +++ b/drivers/iio/addac/Kconfig
> @@ -0,0 +1,8 @@
> +#
> +# ADC DAC drivers
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +menu "Analog to digital and digital to analog converters"
> +
> +endmenu
> diff --git a/drivers/iio/addac/Makefile b/drivers/iio/addac/Makefile
> new file mode 100644
> index 000000000000..b888b9ee12da
> --- /dev/null
> +++ b/drivers/iio/addac/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for industrial I/O ADDAC drivers
> +#
> +
> +# When adding new entries keep the list in alphabetical order
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: iio: add AD74413R
2021-10-28 13:56 ` [PATCH v2 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
@ 2021-10-28 19:08 ` Rob Herring
0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2021-10-28 19:08 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Rob Herring, linux-iio, cosmin.tanislav, Jonathan Cameron,
linux-kernel, devicetree, Lars-Peter Clausen, Michael Hennerich
On Thu, 28 Oct 2021 16:56:04 +0300, Cosmin Tanislav wrote:
> From: Cosmin Tanislav <cosmin.tanislav@analog.com>
>
> Add device tree bindings for AD74413R.
>
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> ---
> .../bindings/iio/addac/adi,ad74413r.yaml | 163 ++++++++++++++++++
> include/dt-bindings/iio/addac/adi,ad74413r.h | 30 ++++
> 2 files changed, 193 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
> create mode 100644 include/dt-bindings/iio/addac/adi,ad74413r.h
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml: patternProperties:^channel@[0-3]$:properties:reg: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml: patternProperties:^channel@[0-3]$:properties:reg: 'maximum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml: patternProperties:^channel@[0-3]$:properties:adi,ch-func: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml: patternProperties:^channel@[0-3]$:properties:adi,ch-func: 'maximum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml: patternProperties:^channel@[0-3]$:properties:adi,gpo-config: 'enum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml: patternProperties:^channel@[0-3]$:properties:reg: 'anyOf' conditional failed, one must be fixed:
'minimum' is not one of ['maxItems', 'description', 'deprecated']
hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
'maximum' is not one of ['maxItems', 'description', 'deprecated']
hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
Additional properties are not allowed ('minimum', 'maximum' were unexpected)
hint: Arrays must be described with a combination of minItems/maxItems/items
'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref']
1 is less than the minimum of 2
hint: Arrays must be described with a combination of minItems/maxItems/items
hint: cell array properties must define how many entries and what the entries are when there is more than one entry.
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml: properties:adi,rsense-resistance-ohms: '$ref' should not be valid under {'const': '$ref'}
hint: Standard unit suffix properties don't need a type $ref
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml: ignoring, error in schema: patternProperties: ^channel@[0-3]$: properties: adi,gpo-config
warning: no schema found in file: ./Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
Error: Documentation/devicetree/bindings/iio/addac/adi,ad74413r.example.dts:23.32-33 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/iio/addac/adi,ad74413r.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1547508
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH v2 1/3] iio: add adddac subdirectory
2021-10-28 13:56 ` [PATCH v2 1/3] iio: add adddac subdirectory Cosmin Tanislav
` (2 preceding siblings ...)
2021-10-28 16:02 ` [PATCH v2 1/3] iio: add adddac subdirectory Jonathan Cameron
@ 2021-10-29 8:00 ` Sa, Nuno
2021-10-30 15:59 ` Jonathan Cameron
3 siblings, 1 reply; 15+ messages in thread
From: Sa, Nuno @ 2021-10-29 8:00 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Tanislav, Cosmin, Lars-Peter Clausen, Hennerich, Michael,
Jonathan Cameron, Rob Herring, linux-iio, devicetree,
linux-kernel
Hi,
> -----Original Message-----
> From: Cosmin Tanislav <demonsingur@gmail.com>
> Sent: Thursday, October 28, 2021 3:56 PM
> Cc: demonsingur@gmail.com; Tanislav, Cosmin
> <Cosmin.Tanislav@analog.com>; Lars-Peter Clausen
> <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Jonathan Cameron
> <jic23@kernel.org>; Rob Herring <robh+dt@kernel.org>; linux-
> iio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH v2 1/3] iio: add adddac subdirectory
>
> [External]
>
> From: Cosmin Tanislav <cosmin.tanislav@analog.com>
>
> For IIO devices that expose both ADC and DAC functionality.
>
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> ---
One thing here that I'm not too sure is the naming of the directory.
I'm starting to see in ADI more and more of this highly integrated devices... For
example this one [1], is something we have someone already working one
and it has ADCs, DAC, amplifiers. So, I'm just wondering if now
it's not the time where we just have a generic enough place for these kind
of "combo" devices? Being that said, I have no idea about what name we could
give :)
[1]: https://www.analog.com/media/en/technical-documentation/data-sheets/AD7293.pdf
- Nuno Sá
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: addac: add AD74413R driver
[not found] ` <CAA+onPPObFeXQwJ450M=5Ue3uE6Cui0M0X49qgn_U1D6su=BcA@mail.gmail.com>
@ 2021-10-30 15:54 ` Jonathan Cameron
[not found] ` <CAA+onPN89FJwUFRc3P7ROxPW1qcbSFSq9XxyA-UZw6Mx3XEO-A@mail.gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2021-10-30 15:54 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: cosmin.tanislav, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, linux-iio, devicetree, linux-kernel, Linus Walleij,
Bartosz Golaszewski, linux-gpio
@Linus, some gpio related discussion below...
On Fri, 29 Oct 2021 00:38:31 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:
Too much context removed. I had to go back and look earlier in the thread
to work out what was being discussed. Particularly as I think these aren't
even in order!
> > Often these are 1/(N**2) rather than 1/(N**2 - 1) as here.
> > Noise level is probably high enough this doesn't really matter though.
> > Data sheets are never entirely clear if ranges are inclusive or not...
>
> My max values are defined as being inclusive.
> On a 13bit DAC, raw values are in the range of 0 to 2**13 - 1, inclusive.
>
OK
> > Rule it out in the dt binding and enforce it at probe time not here which
> is far too late.
> > As below, this should be prevented at probe time not runtime.
>
> This is done so that the GPIO indices are kept the same as the hardware
> channels, 0, 1, 2, 3.
> Depending on their mode, some GPIOs can only be read and some of them can
> only be written to.
> I'm not sure how you would want to do this at probe time?
I'm not totally following, but took more a look at the datasheet.
Device has 4 GPO pins whch is fine. Those are simple output only pins. For
those, if they are in a mode where you are controlling them then you can
cache the value - if they are in comparator mode then they aren't really
acting as GPIO pins at all the value you are reading is reflecting the
input on the other side of the device on a different pin. So in that
case don't register these with the GPIO subsystem at all.
Instead you are registering channels selected from A,B,C,D
>
> Logic parallel mode is reserved for set_multiple, when the GPIO is in logic
> mode.
So, it took me a while to understand what we would loose by 'only' providing
the logic parallel mode. If we only had logic high / logic low as the options
then a sensible driver option would be to map any GPO configuration to the
logic parallel mode. It enables more functionality. However, that got me thinking
for why we had high impedance and 100kOhms as options.
These allow you to implement shared buses over these pins. Which incidentally
should probably be mapped through to the various gpio subsystem controls
to reflect these options.
So the state combinations you might well have would be...
Logic low / logic high
100kOhm pull down / logic high (something like an i2c bus would use this)
tristate so logic low / logic high / high impedance.(don't care or off)
Other than the first one, these require you to not be in the GPO mode.
However, this is all stuff that depends on what these are wired to - so the
dts should reflect that rather than simply setting the mode to one of
+ 0 - GPO_CONFIG_100K_PULL_DOWN
+ 1 - GPO_CONFIG_LOGIC
+ 3 - GPO_CONFIG_DEBOUNCED_COMPARATOR
+ 4 - GPO_CONFIG_HIGH_IMPEDANCE
The only exception being the debounce comparator. So the question is where
would that be wired to?
> GPIOs are referred to as GPO in the datasheet, so I used this name in the
> driver too.
Sort of... As above, the output pins are GPO, the input pins are the ones
the comparator is running on - whilst there value is relected on the outputs
when in the right mode (and that's the only one you can read them in) they
are not the same channel. I'm not sure they should map to the same index...
>
> > Error out of this function is fine, but why not just leave this channel
> disabled
> rather than failing to probe which I think is what will currently happen?
>
> Sure, I could make invalid channel and gpo functions fallback to default
> function.
> But why tolerate errors in the dts configuration?
I don't think it is an error but rather a function you haven't enabled yet
in a specific driver. dts is for the hardware, not your particular driver.
Mind you, I think the binding around this needs to change entirely anyway
so this will probably not end up relevant.
Jonathan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] iio: add adddac subdirectory
2021-10-29 8:00 ` Sa, Nuno
@ 2021-10-30 15:59 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2021-10-30 15:59 UTC (permalink / raw)
To: Sa, Nuno
Cc: Cosmin Tanislav, Tanislav, Cosmin, Lars-Peter Clausen, Hennerich,
Michael, Rob Herring, linux-iio, devicetree, linux-kernel
On Fri, 29 Oct 2021 08:00:05 +0000
"Sa, Nuno" <Nuno.Sa@analog.com> wrote:
> Hi,
>
> > -----Original Message-----
> > From: Cosmin Tanislav <demonsingur@gmail.com>
> > Sent: Thursday, October 28, 2021 3:56 PM
> > Cc: demonsingur@gmail.com; Tanislav, Cosmin
> > <Cosmin.Tanislav@analog.com>; Lars-Peter Clausen
> > <lars@metafoo.de>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Jonathan Cameron
> > <jic23@kernel.org>; Rob Herring <robh+dt@kernel.org>; linux-
> > iio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: [PATCH v2 1/3] iio: add adddac subdirectory
> >
> > [External]
> >
> > From: Cosmin Tanislav <cosmin.tanislav@analog.com>
> >
> > For IIO devices that expose both ADC and DAC functionality.
> >
> > Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> > ---
>
> One thing here that I'm not too sure is the naming of the directory.
> I'm starting to see in ADI more and more of this highly integrated devices... For
> example this one [1], is something we have someone already working one
> and it has ADCs, DAC, amplifiers. So, I'm just wondering if now
> it's not the time where we just have a generic enough place for these kind
> of "combo" devices? Being that said, I have no idea about what name we could
> give :)
>
> [1]: https://www.analog.com/media/en/technical-documentation/data-sheets/AD7293.pdf
> - Nuno Sá
Naming is always fun. I don't want to have combo start picking up IMUs so
we need to be a bit careful.
We could take the approach we have done with proximity and light sensors of effectively
declaring one type to the dominant one. There it's a bit clearer though - you don't
buy a proximity sensor if you want to just measure light levels.
Here there isn't always a dominant type. The example here is titled input / output
device so no preference of one over the other. The GPIO stuff is kind of a feature
bolted on, so ADDAC is generic enough.
For the ad7293 it does call it a Power Amplifiers 'with' the other stuff so maybe
just sticking to amplifier as the type is the way to go.
Meh, to a certain extent it doesn't matter - we can safely move these around
once we have more of them in the tree. The adt7316 is still in staging and
is an ADDAC as well so I think we should put that category in for now.
Jonathan
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: addac: add AD74413R driver
2021-10-28 13:48 ` [PATCH 3/3] iio: addac: add AD74413R driver Cosmin Tanislav
2021-10-28 15:58 ` Jonathan Cameron
@ 2021-10-31 22:47 ` kernel test robot
1 sibling, 0 replies; 15+ messages in thread
From: kernel test robot @ 2021-10-31 22:47 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: kbuild-all, demonsingur, cosmin.tanislav, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring, linux-iio,
devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6557 bytes --]
Hi Cosmin,
I love your patch! Yet something to improve:
[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on robh/for-next linus/master v5.15-rc7 next-20211029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Cosmin-Tanislav/iio-add-adddac-subdirectory/20211028-225639
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/94ef24d0c0f35d99ddebc9dda85e262b70ea87db
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Cosmin-Tanislav/iio-add-adddac-subdirectory/20211028-225639
git checkout 94ef24d0c0f35d99ddebc9dda85e262b70ea87db
# save the attached .config to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/iio/addac/ad74413r.c: In function 'ad74413r_probe':
>> drivers/iio/addac/ad74413r.c:1289:72: error: 'struct iio_dev' has no member named 'id'
1289 | st->trig = devm_iio_trigger_alloc(st->dev, "%s-dev%d", name, indio_dev->id);
| ^~
vim +1289 drivers/iio/addac/ad74413r.c
1202
1203 static int ad74413r_probe(struct spi_device *spi)
1204 {
1205 struct ad74413r_state *st;
1206 struct iio_dev *indio_dev;
1207 unsigned int sillicon_rev_id;
1208 const char *name;
1209 int ret;
1210
1211 indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
1212 if (indio_dev == NULL)
1213 return -ENOMEM;
1214
1215 st = iio_priv(indio_dev);
1216 spi_set_drvdata(spi, st);
1217
1218 st->spi = spi;
1219 st->dev = &spi->dev;
1220 st->config = of_device_get_match_data(&spi->dev);
1221 st->indio_dev = indio_dev;
1222 mutex_init(&st->lock);
1223 init_completion(&st->adc_data_completion);
1224
1225 name = dev_name(st->dev);
1226
1227 st->regmap = devm_regmap_init(st->dev, NULL, &spi->dev, &ad74413r_regmap_config);
1228 if (IS_ERR(st->regmap)) {
1229 ret = PTR_ERR(st->regmap);
1230 dev_err(st->dev, "Failed to create regmap: %d\n", ret);
1231 return ret;
1232 }
1233
1234 ret = regmap_read(st->regmap, AD74413R_REG_SILLICON_REV, &sillicon_rev_id);
1235 if (ret) {
1236 dev_err(st->dev, "Failed to read sillicon rev: %d\n", ret);
1237 return ret;
1238 }
1239
1240 sillicon_rev_id = FIELD_GET(AD74413R_SILLICON_REV_ID_MASK, sillicon_rev_id);
1241 if (sillicon_rev_id != AD74413R_SILLICON_REV_ID_EXPECTED) {
1242 dev_err(st->dev, "Read sillicon rev id %d does not match expected rev id %d\n",
1243 sillicon_rev_id, AD74413R_SILLICON_REV_ID_EXPECTED);
1244 return -EINVAL;
1245 }
1246
1247 st->refin_reg = devm_regulator_get(st->dev, "refin");
1248 if (IS_ERR(st->refin_reg)) {
1249 dev_err(st->dev, "Failed to get refin regulator: %d\n", ret);
1250 return PTR_ERR(st->refin_reg);
1251 }
1252
1253 ret = regulator_enable(st->refin_reg);
1254 if (ret) {
1255 dev_err(st->dev, "Failed to enable refin regulator: %d\n", ret);
1256 return ret;
1257 }
1258
1259 ret = devm_add_action_or_reset(st->dev, ad74413r_regulator_disable,
1260 st->refin_reg);
1261 if (ret) {
1262 dev_err(st->dev, "Failed to add refin regulator disable action: %d\n", ret);
1263 return ret;
1264 }
1265
1266 ret = of_property_read_u32(st->dev->of_node, "adi,rsense-resistance-ohms",
1267 &st->rsense_resistance_ohms);
1268 if (ret) {
1269 dev_err(st->dev, "Failed to get rsense resistance: %d\n", ret);
1270 return ret;
1271 }
1272
1273 st->gpiochip.label = name;
1274 st->gpiochip.base = -1;
1275 st->gpiochip.ngpio = AD74413R_CHANNEL_MAX;
1276 st->gpiochip.parent = st->dev;
1277 st->gpiochip.can_sleep = true;
1278 st->gpiochip.set = ad74413r_gpio_set;
1279 st->gpiochip.set_multiple = ad74413r_gpio_set_multiple;
1280 st->gpiochip.owner = THIS_MODULE;
1281 st->gpiochip.get = ad74413r_gpio_get;
1282
1283 ret = devm_gpiochip_add_data(st->dev, &st->gpiochip, st);
1284 if (ret) {
1285 dev_err(st->dev, "Failed to add gpio chip: %d\n", ret);
1286 return ret;
1287 }
1288
> 1289 st->trig = devm_iio_trigger_alloc(st->dev, "%s-dev%d", name, indio_dev->id);
1290 if (!st->trig)
1291 return -ENOMEM;
1292
1293 st->trig->ops = &ad74413r_trigger_ops;
1294 st->trig->dev.parent = st->dev;
1295 iio_trigger_set_drvdata(st->trig, st);
1296
1297 ret = devm_iio_trigger_register(st->dev, st->trig);
1298 if (ret)
1299 return ret;
1300
1301 indio_dev->dev.parent = st->dev;
1302 indio_dev->name = name;
1303 indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
1304 indio_dev->info = &ad74413r_info;
1305 indio_dev->trig = iio_trigger_get(st->trig);
1306
1307 ret = ad74413r_parse_channel_configs(st);
1308 if (ret)
1309 return ret;
1310
1311 ret = ad74413r_count_iio_channels(st);
1312 if (ret)
1313 return ret;
1314
1315 ret = ad74413r_setup_iio_channels(st);
1316 if (ret) {
1317 dev_err(st->dev, "Failed to setup iio channels: %d\n", ret);
1318 return ret;
1319 }
1320
1321 ret = ad74413r_set_adc_conv_seq(st, AD74413R_CONV_SEQ_OFF);
1322 if (ret)
1323 return ret;
1324
1325 ret = devm_request_irq(st->dev, spi->irq, ad74413r_adc_data_interrupt,
1326 IRQF_TRIGGER_FALLING, name, st);
1327 if (ret) {
1328 dev_err(st->dev, "Failed to request threaded irq: %d\n", ret);
1329 return ret;
1330 }
1331
1332 ret = devm_iio_triggered_buffer_setup(st->dev, indio_dev, &iio_pollfunc_store_time,
1333 &ad74413r_trigger_handler, &ad74413r_buffer_ops);
1334 if (ret) {
1335 dev_err(st->dev, "Failed to setup triggered buffer: %d\n", ret);
1336 return ret;
1337 }
1338
1339 return devm_iio_device_register(st->dev, indio_dev);
1340 }
1341
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66130 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: addac: add AD74413R driver
[not found] ` <CAA+onPN89FJwUFRc3P7ROxPW1qcbSFSq9XxyA-UZw6Mx3XEO-A@mail.gmail.com>
@ 2021-11-03 17:33 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2021-11-03 17:33 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: cosmin.tanislav, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, linux-iio, devicetree, linux-kernel, Linus Walleij,
Bartosz Golaszewski, linux-gpio
On Wed, 3 Nov 2021 16:17:24 +0200
Cosmin Tanislav <demonsingur@gmail.com> wrote:
> > Too much context removed. I had to go back and look earlier in the thread
> > to work out what was being discussed. Particularly as I think these
> aren't
> > even in order!
>
> I'm sorry, I'm new to the LKML.
>
> > > > Rule it out in the dt binding and enforce it at probe time not here
> which
> > > is far too late.
> > > > As below, this should be prevented at probe time not runtime.
> > >
> > > This is done so that the GPIO indices are kept the same as the hardware
> > > channels, 0, 1, 2, 3.
> > > Depending on their mode, some GPIOs can only be read and some of them
> can
> > > only be written to.
> > > I'm not sure how you would want to do this at probe time?
> >
> > I'm not totally following, but took more a look at the datasheet.
> >
> > Device has 4 GPO pins whch is fine. Those are simple output only pins.
> For
> > those, if they are in a mode where you are controlling them then you can
> > cache the value - if they are in comparator mode then they aren't really
> > acting as GPIO pins at all the value you are reading is reflecting the
> > input on the other side of the device on a different pin. So in that
> > case don't register these with the GPIO subsystem at all.
> > Instead you are registering channels selected from A,B,C,D
> >
> > >
> > > Logic parallel mode is reserved for set_multiple, when the GPIO is in
> logic
> > > mode.
> > So, it took me a while to understand what we would loose by 'only'
> providing
> > the logic parallel mode. If we only had logic high / logic low as the
> options
> > then a sensible driver option would be to map any GPO configuration to the
> > logic parallel mode. It enables more functionality. However, that got
> me thinking
> > for why we had high impedance and 100kOhms as options.
> >
> > These allow you to implement shared buses over these pins. Which
> incidentally
> > should probably be mapped through to the various gpio subsystem controls
> > to reflect these options.
> >
> > So the state combinations you might well have would be...
> >
> > Logic low / logic high
> > 100kOhm pull down / logic high (something like an i2c bus would use this)
> > tristate so logic low / logic high / high impedance.(don't care or off)
> >
> > Other than the first one, these require you to not be in the GPO mode.
> >
> > However, this is all stuff that depends on what these are wired to - so
> the
> > dts should reflect that rather than simply setting the mode to one of
> >
> > + 0 - GPO_CONFIG_100K_PULL_DOWN
> > + 1 - GPO_CONFIG_LOGIC
> > + 3 - GPO_CONFIG_DEBOUNCED_COMPARATOR
> > + 4 - GPO_CONFIG_HIGH_IMPEDANCE
> >
> > The only exception being the debounce comparator. So the question is where
> > would that be wired to?
> >
> > > GPIOs are referred to as GPO in the datasheet, so I used this name in
> the
> > > driver too.
> >
> > Sort of... As above, the output pins are GPO, the input pins are the ones
> > the comparator is running on - whilst there value is relected on the
> outputs
> > when in the right mode (and that's the only one you can read them in) they
> > are not the same channel. I'm not sure they should map to the same
> index...
> >
> > >
> > > > Error out of this function is fine, but why not just leave this
> channel
> > > disabled
> > > rather than failing to probe which I think is what will currently
> happen?
> > >
> > > Sure, I could make invalid channel and gpo functions fallback to default
> > > function.
> > > But why tolerate errors in the dts configuration?
> >
> > I don't think it is an error but rather a function you haven't enabled yet
> > in a specific driver. dts is for the hardware, not your particular
> driver.
> >
> > Mind you, I think the binding around this needs to change entirely anyway
> > so this will probably not end up relevant.
>
> Okay, so after some more thought I realized where I went wrong with
> this implementation. I'm writing down my ideas before I go implement
> it so we can get on the same page.
>
> Device-tree configuration should only specify if a gpo is in comparator
> mode or not. When not in comparator mode, the gpo is in output mode.
> When in output mode, it is exposed as an output-only gpio pin.
>
> When a channel is in digital input mode, an input-only gpio is exposed
> for the value of the comparator result, which can be read via the
> DIN_COMP_OUT register.
Yes, though I think you also need to somehow reflect that it's an entirely
different input rather than being the same pin as the output gpios
would be on if in that mode. I'm not sure how you would do that in the
gpio subsystem, possibly the offset?
>
> Regarding high impedance and pull down resistor, should I be implementing
> these features by implementing gpio_chip->set_config function and
> handling the PIN_CONFIG_BIAS_HIGH_IMPEDANCE,
> PIN_CONFIG_BIAS_PULL_DOWN flags?
Yes, I think so.
>
> The comparator also has a setting for debounce time, so I could also handle
> PIN_CONFIG_INPUT_DEBOUNCE.
Sound right to me, but gpio people may disagree :)
Jonathan
>
> Thank you for your patience.
> Cosmin.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-11-03 17:29 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-28 13:48 [PATCH 1/3] iio: add adddac subdirectory Cosmin Tanislav
2021-10-28 13:48 ` [PATCH 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
2021-10-28 14:48 ` Jonathan Cameron
2021-10-28 13:48 ` [PATCH 3/3] iio: addac: add AD74413R driver Cosmin Tanislav
2021-10-28 15:58 ` Jonathan Cameron
[not found] ` <CAA+onPPObFeXQwJ450M=5Ue3uE6Cui0M0X49qgn_U1D6su=BcA@mail.gmail.com>
2021-10-30 15:54 ` Jonathan Cameron
[not found] ` <CAA+onPN89FJwUFRc3P7ROxPW1qcbSFSq9XxyA-UZw6Mx3XEO-A@mail.gmail.com>
2021-11-03 17:33 ` Jonathan Cameron
2021-10-31 22:47 ` kernel test robot
2021-10-28 13:56 ` [PATCH v2 1/3] iio: add adddac subdirectory Cosmin Tanislav
2021-10-28 13:56 ` [PATCH v2 2/3] dt-bindings: iio: add AD74413R Cosmin Tanislav
2021-10-28 19:08 ` Rob Herring
2021-10-28 13:56 ` [PATCH v2 3/3] iio: addac: add AD74413R driver Cosmin Tanislav
2021-10-28 16:02 ` [PATCH v2 1/3] iio: add adddac subdirectory Jonathan Cameron
2021-10-29 8:00 ` Sa, Nuno
2021-10-30 15:59 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).