This series adds support for high-speed, high-precision AD4000 series of SAR ADCs.
Most uncommon things about this set are:
1) These devices have the same SPI (Strange Peripheral Interface) as AD7944
devices, which has been documented in ad7944.rst [1].
The device tree description for SPI connections and mode can be the same as of
ad7944 adi,spi-mode [2].
Because ad4000 driver does not currently support daisy-chain mode, I simplified
things a little bit. If having a more complete doc is preferred, I'm fine
changing to that.
[1]: https://lore.kernel.org/linux-iio/[email protected]/
[2]: https://lore.kernel.org/linux-iio/[email protected]/
2) Differently from AD7944, AD4000 devices have a configuration register to
toggle some features. For instance, turbo mode is set through configuration
register rather than an external pin. This simplifies hardware connections,
but then requires software interface. So, additional ABI being proposed
in sysfs-bus-iio-adc-ad4000. The one I'm most in doubt about is
span_compression_en which affects the in_voltageY_scale attribute.
That might be instead supported by providing _scale_available and allowing write
to _scale. Anyway, let me know how bad those look like :)
Thanks,
Marcelo
Marcelo Schmitt (2):
dt-bindings: iio: adc: Add AD4000
iio: adc: Add support for AD4000
.../ABI/testing/sysfs-bus-iio-adc-ad4000 | 36 +
.../bindings/iio/adc/adi,ad4000.yaml | 151 ++++
MAINTAINERS | 9 +
drivers/iio/adc/Kconfig | 12 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4000.c | 666 ++++++++++++++++++
6 files changed, 875 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
create mode 100644 drivers/iio/adc/ad4000.c
--
2.43.0
Add device tree documentation for AD4000 series of ADC devices.
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
Signed-off-by: Marcelo Schmitt <[email protected]>
---
Pasting relevant comment from cover letter here to aid reviewers.
These devices have the same SPI (Strange Peripheral Interface) as AD7944
devices, which has been documented in ad7944.rst [1].
The device tree description for SPI connections and mode can be the same as of
ad7944 adi,spi-mode [2].
Because ad4000 driver does not currently support daisy-chain mode, I simplified
things a little bit. If having a more complete doc is preferred, I'm fine
changing to that.
[1]: https://lore.kernel.org/linux-iio/[email protected]/
[2]: https://lore.kernel.org/linux-iio/[email protected]/
.../bindings/iio/adc/adi,ad4000.yaml | 151 ++++++++++++++++++
MAINTAINERS | 7 +
2 files changed, 158 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
new file mode 100644
index 000000000000..9e3d6a3920ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
@@ -0,0 +1,151 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD4000 ADC device driver
+
+maintainers:
+ - Marcelo Schmitt <[email protected]>
+
+description: |
+ Analog Devices AD4000 family of Analog to Digital Converters with SPI support.
+ Specifications can be found at:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad4000
+ - adi,ad4001
+ - adi,ad4002
+ - adi,ad4003
+ - adi,ad4004
+ - adi,ad4005
+ - adi,ad4006
+ - adi,ad4007
+ - adi,ad4008
+ - adi,ad4010
+ - adi,ad4011
+ - adi,ad4020
+ - adi,ad4021
+ - adi,ad4022
+ - adi,adaq4001
+ - adi,adaq4003
+
+ reg: true
+ spi-max-frequency: true
+
+ vref-supply:
+ description: Phandle to the regulator for ADC reference voltage.
+
+ adi,gain-milli:
+ description: |
+ The hardware gain applied to the ADC input (in milli units).
+ The gain provided by the ADC input scaler is defined by the hardware
+ connections between chip pins OUT+, R1K-, R1K1-, R1K+, R1K1+, and OUT-.
+ If not present, default to 1000 (no actual gain applied).
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [454, 909, 1000, 1900]
+ default: 1000
+
+ adi,spi-cs-mode:
+ type: boolean
+ description: |
+ This property indicates the SPI wiring configuration.
+
+ When this property is omitted, it indicates that the device SDI pin is
+ connected to SPI controller CS line and device CNV pin has been connected
+ to a GPIO. Datasheets call this "4-wire mode".
+
+ When this property is present, the driver must assume standard SPI
+ connections which, for these devices, consists of connecting the
+ controller CS line to device CNV pin. This configuration is
+ (misleadingly) called "3-wire mode" in datasheets.
+
+ cnv-gpios:
+ description: The GPIO connected to the CNV pin.
+ maxItems: 1
+
+patternProperties:
+ "^channel@([0-1])$":
+ $ref: adc.yaml
+ type: object
+ description: Represents the external channel connected to the ADC.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ diff-channels: true
+
+ required:
+ - reg
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - vref-supply
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+ - if:
+ properties:
+ adi,spi-cs-mode: false
+ then:
+ required:
+ - cnv-gpios
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ /* Example for a AD4000 devices */
+ adc@0 {
+ compatible = "adi,ad4020";
+ reg = <0>;
+ spi-max-frequency = <71000000>;
+ vref-supply = <&vref>;
+ cnv-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ channel@0 {
+ reg = <0>;
+ diff-channels = <0 1>;
+ };
+ };
+ };
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ /* Example for a ADAQ4000 devices */
+ adc@0 {
+ compatible = "adi,adaq4003";
+ reg = <0>;
+ spi-max-frequency = <80000000>;
+ vref-supply = <&vref>;
+ adi,spi-cs-mode;
+ adi,gain-milli = <454>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ channel@0 {
+ reg = <0>;
+ diff-channels = <0 1>;
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 2662ec49b297..3ca90f842298 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1135,6 +1135,13 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
F: drivers/iio/dac/ad3552r.c
+ANALOG DEVICES INC AD4000 DRIVER
+M: Marcelo Schmitt <[email protected]>
+L: [email protected]
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
+
ANALOG DEVICES INC AD4130 DRIVER
M: Cosmin Tanislav <[email protected]>
L: [email protected]
--
2.43.0
Add support for AD4000 series of high accuracy, high speed, low power,
successive aproximation register (SAR) ADCs.
Signed-off-by: Marcelo Schmitt <[email protected]>
---
Pasting relevant comment from cover letter here to aid reviewers.
Differently from AD7944, AD4000 devices have a configuration register to
toggle some features. For instance, turbo mode is set through configuration
register rather than an external pin. This simplifies hardware connections,
but then requires software interface. So, additional ABI being proposed
in sysfs-bus-iio-adc-ad4000. The one I'm most in doubt about is
span_compression_en which affects the in_voltageY_scale attribute.
That might be instead supported by providing _scale_available and allowing write
to _scale.
.../ABI/testing/sysfs-bus-iio-adc-ad4000 | 36 +
MAINTAINERS | 2 +
drivers/iio/adc/Kconfig | 12 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4000.c | 666 ++++++++++++++++++
5 files changed, 717 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
create mode 100644 drivers/iio/adc/ad4000.c
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
new file mode 100644
index 000000000000..98fb7539ad6d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
@@ -0,0 +1,36 @@
+What: /sys/bus/iio/devices/iio:deviceX/turbo_en
+KernelVersion: 6.9
+Contact: [email protected]
+Description:
+ This attribute is used to enable/disable turbo mode allowing
+ slower SPI clock rates (at a minimum SCK rate of 75 MHz) to
+ achieve the maximum throughput of 2 MSPS.
+
+What: /sys/bus/iio/devices/iio:deviceX/span_compression_en
+KernelVersion: 6.9
+Contact: [email protected]
+Description:
+ This attribute is used to enable/disable the input span
+ compression feature that reduces the ADC input range by 10% from
+ the top and bottom of the range while still accessing all
+ available ADC codes. Enabling span compression causes a
+ decrease in ADC scale which is reflected in the channel
+ in_voltageY_scale attribute.
+
+What: /sys/bus/iio/devices/iio:deviceX/status_bits_en
+KernelVersion: 6.9
+Contact: [email protected]
+Description:
+ This attribute is used to make the chip append a 6-bit status
+ word at the end of conversion results. The 6 status bits contain
+ the configuration register fields for OV clamp flag, span
+ compression, high-z mode, and turbo mode.
+
+What: /sys/bus/iio/devices/iio:deviceX/high_impedance_en
+KernelVersion: 6.9
+Contact: [email protected]
+Description:
+ This attribute is used to enable/disable high impedance mode
+ (high-z) which reduces nonlinear charge kickback to the ADC
+ input.
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 3ca90f842298..2ae98c700ff0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1140,7 +1140,9 @@ M: Marcelo Schmitt <[email protected]>
L: [email protected]
S: Supported
W: https://ez.analog.com/linux-software-drivers
+F: Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
+F: drivers/iio/adc/ad4000.c
ANALOG DEVICES INC AD4130 DRIVER
M: Cosmin Tanislav <[email protected]>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 0d9282fa67f5..15db35f00ef0 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -21,6 +21,18 @@ config AD_SIGMA_DELTA
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
+config AD4000
+ tristate "Analog Devices AD4000 ADC Driver"
+ depends on SPI
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ Say yes here to build support for Analog Devices AD4000 high speed
+ SPI analog to digital converters (ADC).
+
+ To compile this driver as a module, choose M here: the module will be
+ called ad4000.
+
config AD4130
tristate "Analog Device AD4130 ADC Driver"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index b3c434722364..f535d617ae89 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -6,6 +6,7 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
+obj-$(CONFIG_AD4000) += ad4000.o
obj-$(CONFIG_AD4130) += ad4130.o
obj-$(CONFIG_AD7091R) += ad7091r-base.o
obj-$(CONFIG_AD7091R5) += ad7091r5.o
diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
new file mode 100644
index 000000000000..f77104755979
--- /dev/null
+++ b/drivers/iio/adc/ad4000.c
@@ -0,0 +1,666 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD4000 SPI ADC driver
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+#include <asm/unaligned.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/units.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define AD400X_READ_COMMAND 0x54
+#define AD400X_WRITE_COMMAND 0x14
+
+#define AD4000_CONFIG_REG_MSK 0xFF
+
+/* AD4000 Configuration Register programmable bits */
+#define AD4000_STATUS BIT(4) /* Status bits output */
+#define AD4000_SPAN_COMP BIT(3) /* Input span compression */
+#define AD4000_HIGHZ BIT(2) /* High impedance mode */
+#define AD4000_TURBO BIT(1) /* Turbo mode */
+
+#define AD4000_16BIT_MSK GENMASK(31, 16)
+#define AD4000_18BIT_MSK GENMASK(31, 14)
+#define AD4000_20BIT_MSK GENMASK(31, 12)
+
+#define AD4000_CHANNEL(_sign, _real_bits) \
+ { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_type = { \
+ .sign = _sign, \
+ .realbits = _real_bits, \
+ .storagebits = 32, \
+ .endianness = IIO_BE, \
+ }, \
+ } \
+
+enum ad4000_ids {
+ ID_AD4000,
+ ID_AD4001,
+ ID_AD4002,
+ ID_AD4003,
+ ID_AD4004,
+ ID_AD4005,
+ ID_AD4006,
+ ID_AD4007,
+ ID_AD4008,
+ ID_AD4010,
+ ID_AD4011,
+ ID_AD4020,
+ ID_AD4021,
+ ID_AD4022,
+ ID_ADAQ4001,
+ ID_ADAQ4003,
+};
+
+struct ad4000_chip_info {
+ const char *dev_name;
+ struct iio_chan_spec chan_spec;
+};
+
+static const struct ad4000_chip_info ad4000_chips[] = {
+ [ID_AD4000] = {
+ .dev_name = "ad4000",
+ .chan_spec = AD4000_CHANNEL('u', 16),
+ },
+ [ID_AD4001] = {
+ .dev_name = "ad4001",
+ .chan_spec = AD4000_CHANNEL('s', 16),
+ },
+ [ID_AD4002] = {
+ .dev_name = "ad4002",
+ .chan_spec = AD4000_CHANNEL('u', 18),
+ },
+ [ID_AD4003] = {
+ .dev_name = "ad4003",
+ .chan_spec = AD4000_CHANNEL('s', 18),
+ },
+ [ID_AD4004] = {
+ .dev_name = "ad4004",
+ .chan_spec = AD4000_CHANNEL('u', 16),
+ },
+ [ID_AD4005] = {
+ .dev_name = "ad4005",
+ .chan_spec = AD4000_CHANNEL('s', 16),
+ },
+ [ID_AD4006] = {
+ .dev_name = "ad4006",
+ .chan_spec = AD4000_CHANNEL('u', 18),
+ },
+ [ID_AD4007] = {
+ .dev_name = "ad4007",
+ .chan_spec = AD4000_CHANNEL('s', 18),
+ },
+ [ID_AD4008] = {
+ .dev_name = "ad4008",
+ .chan_spec = AD4000_CHANNEL('u', 16),
+ },
+ [ID_AD4010] = {
+ .dev_name = "ad4010",
+ .chan_spec = AD4000_CHANNEL('u', 18),
+ },
+ [ID_AD4011] = {
+ .dev_name = "ad4011",
+ .chan_spec = AD4000_CHANNEL('s', 18),
+ },
+ [ID_AD4020] = {
+ .dev_name = "ad4020",
+ .chan_spec = AD4000_CHANNEL('s', 20),
+ },
+ [ID_AD4021] = {
+ .dev_name = "ad4021",
+ .chan_spec = AD4000_CHANNEL('s', 20),
+ },
+ [ID_AD4022] = {
+ .dev_name = "ad4022",
+ .chan_spec = AD4000_CHANNEL('s', 20),
+ },
+ [ID_ADAQ4001] = {
+ .dev_name = "adaq4001",
+ .chan_spec = AD4000_CHANNEL('s', 16),
+ },
+ [ID_ADAQ4003] = {
+ .dev_name = "adaq4003",
+ .chan_spec = AD4000_CHANNEL('s', 18),
+ },
+};
+
+enum ad4000_gains {
+ AD4000_0454_GAIN = 0,
+ AD4000_0909_GAIN = 1,
+ AD4000_1_GAIN = 2,
+ AD4000_1900_GAIN = 3,
+ AD4000_GAIN_LEN
+};
+
+/*
+ * Gains stored and computed as fractions to avoid introducing rounding erros.
+ */
+static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = {
+ [AD4000_0454_GAIN] = { 227, 500 },
+ [AD4000_0909_GAIN] = { 909, 1000 },
+ [AD4000_1_GAIN] = { 1, 1 },
+ [AD4000_1900_GAIN] = { 19, 10 },
+};
+
+struct ad4000_state {
+ struct spi_device *spi;
+ struct gpio_desc *cnv_gpio;
+ int vref;
+ bool status_bits;
+ bool span_comp;
+ bool turbo_mode;
+ bool high_z_mode;
+
+ enum ad4000_gains pin_gain;
+ int scale_tbl[AD4000_GAIN_LEN][2];
+
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ */
+ union {
+ struct {
+ u8 sample_buf[4];
+ s64 timestamp;
+ } scan;
+ u8 d8[2];
+ } data __aligned(IIO_DMA_MINALIGN);
+};
+
+static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits)
+{
+ int val, val2, tmp0, tmp1, i;
+ u64 tmp2;
+
+ val2 = scale_bits;
+ for (i = 0; i < AD4000_GAIN_LEN; i++) {
+ val = st->vref / 1000;
+ /* Multiply by MILLI here to avoid losing precision */
+ val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI,
+ ad4000_gains_frac[i][0]);
+ /* Would multiply by NANO here but we already multiplied by MILLI */
+ tmp2 = shift_right((u64)val * MICRO, val2);
+ tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1);
+ st->scale_tbl[i][0] = tmp0; /* Integer part */
+ st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */
+ }
+}
+
+static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
+{
+ struct spi_transfer t = {
+ .tx_buf = st->data.d8,
+ .len = 2,
+ };
+ struct spi_message m;
+
+ put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val, st->data.d8);
+
+ spi_message_init(&m);
+ spi_message_add_tail(&t, &m);
+
+ return spi_sync(st->spi, &m);
+}
+
+static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
+{
+ struct spi_transfer t = {0};
+ struct spi_message m;
+ int ret;
+
+ st->data.d8[0] = AD400X_READ_COMMAND;
+
+ t.rx_buf = st->data.d8;
+ t.tx_buf = st->data.d8;
+ t.len = 2;
+
+ spi_message_init_with_transfers(&m, &t, 1);
+
+ ret = spi_sync(st->spi, &m);
+ if (ret < 0)
+ return ret;
+
+ *val = FIELD_GET(AD4000_CONFIG_REG_MSK, get_unaligned_be16(st->data.d8));
+
+ return ret;
+}
+
+static int ad4000_read_sample(struct ad4000_state *st, uint32_t *val)
+{
+ struct spi_transfer t = {0};
+ struct spi_message m;
+ int ret;
+
+ t.rx_buf = &st->data.scan.sample_buf;
+ t.len = 4;
+ t.delay.value = 60;
+ t.delay.unit = SPI_DELAY_UNIT_NSECS;
+
+ spi_message_init_with_transfers(&m, &t, 1);
+
+ if (st->cnv_gpio)
+ gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH);
+
+ ret = spi_sync(st->spi, &m);
+ if (ret < 0)
+ return ret;
+
+ if (st->cnv_gpio)
+ gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
+
+ *val = get_unaligned_be32(&st->data.scan.sample_buf);
+
+ return 0;
+}
+
+static int ad4000_single_conversion(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, int *val)
+{
+ struct ad4000_state *st = iio_priv(indio_dev);
+ u32 sample;
+ int ret;
+
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = ad4000_read_sample(st, &sample);
+
+ iio_device_release_direct_mode(indio_dev);
+
+ if (ret)
+ return ret;
+
+ switch (chan->scan_type.realbits) {
+ case 16:
+ sample = FIELD_GET(AD4000_16BIT_MSK, sample);
+ break;
+ case 18:
+ sample = FIELD_GET(AD4000_18BIT_MSK, sample);
+ break;
+ case 20:
+ sample = FIELD_GET(AD4000_20BIT_MSK, sample);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (chan->scan_type.sign == 's')
+ *val = sign_extend32(sample, chan->scan_type.realbits - 1);
+
+ return IIO_VAL_INT;
+}
+
+static int ad4000_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long info)
+{
+ struct ad4000_state *st = iio_priv(indio_dev);
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ return ad4000_single_conversion(indio_dev, chan, val);
+ case IIO_CHAN_INFO_SCALE:
+ *val = st->scale_tbl[st->pin_gain][0];
+ *val2 = st->scale_tbl[st->pin_gain][1];
+ if (st->span_comp)
+ *val2 = DIV_ROUND_CLOSEST(*val2 * 4, 5);
+ return IIO_VAL_INT_PLUS_NANO;
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static ssize_t ad4000_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct ad4000_state *st = iio_priv(indio_dev);
+
+ switch ((u32)this_attr->address) {
+ case AD4000_STATUS:
+ return sysfs_emit(buf, "%d\n", st->status_bits);
+ case AD4000_SPAN_COMP:
+ return sysfs_emit(buf, "%d\n", st->span_comp);
+ case AD4000_HIGHZ:
+ return sysfs_emit(buf, "%d\n", st->high_z_mode);
+ case AD4000_TURBO:
+ return sysfs_emit(buf, "%d\n", st->turbo_mode);
+ default:
+ return -EINVAL;
+ }
+}
+
+static ssize_t ad4000_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct ad4000_state *st = iio_priv(indio_dev);
+ struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+ unsigned int reg_val;
+ bool val;
+ int ret;
+
+ ret = kstrtobool(buf, &val);
+ if (ret < 0)
+ return ret;
+
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = ad4000_read_reg(st, ®_val);
+ if (ret < 0)
+ goto err_release;
+
+ switch ((u32)this_attr->address) {
+ case AD4000_STATUS:
+ reg_val &= ~AD4000_STATUS;
+ reg_val |= FIELD_PREP(AD4000_STATUS, val);
+ ret = ad4000_write_reg(st, reg_val);
+ if (ret < 0)
+ goto err_release;
+
+ st->status_bits = val;
+ break;
+ case AD4000_SPAN_COMP:
+ reg_val &= ~AD4000_SPAN_COMP;
+ reg_val |= FIELD_PREP(AD4000_SPAN_COMP, val);
+ ret = ad4000_write_reg(st, reg_val);
+ if (ret < 0)
+ goto err_release;
+
+ st->span_comp = val;
+ break;
+ case AD4000_HIGHZ:
+ reg_val &= ~AD4000_HIGHZ;
+ reg_val |= FIELD_PREP(AD4000_HIGHZ, val);
+ ret = ad4000_write_reg(st, reg_val);
+ if (ret < 0)
+ goto err_release;
+
+ st->high_z_mode = val;
+ break;
+ case AD4000_TURBO:
+ reg_val &= ~AD4000_TURBO;
+ reg_val |= FIELD_PREP(AD4000_TURBO, val);
+ ret = ad4000_write_reg(st, reg_val);
+ if (ret < 0)
+ goto err_release;
+
+ st->turbo_mode = val;
+ break;
+ default:
+ ret = -EINVAL;
+ goto err_release;
+ }
+
+err_release:
+ iio_device_release_direct_mode(indio_dev);
+ return ret ? ret : len;
+}
+
+static IIO_DEVICE_ATTR(status_bits_en, 0644, ad4000_show, ad4000_store,
+ AD4000_STATUS);
+
+static IIO_DEVICE_ATTR(span_compression_en, 0644, ad4000_show, ad4000_store,
+ AD4000_SPAN_COMP);
+
+static IIO_DEVICE_ATTR(high_impedance_en, 0644, ad4000_show, ad4000_store,
+ AD4000_HIGHZ);
+
+static IIO_DEVICE_ATTR(turbo_en, 0644, ad4000_show, ad4000_store,
+ AD4000_TURBO);
+
+static struct attribute *ad4000_attributes[] = {
+ &iio_dev_attr_status_bits_en.dev_attr.attr,
+ &iio_dev_attr_span_compression_en.dev_attr.attr,
+ &iio_dev_attr_high_impedance_en.dev_attr.attr,
+ &iio_dev_attr_turbo_en.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group ad4000_attribute_group = {
+ .attrs = ad4000_attributes,
+};
+
+static irqreturn_t ad4000_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ad4000_state *st = iio_priv(indio_dev);
+ struct spi_transfer t = {0};
+ struct spi_message m;
+ int ret;
+
+ t.rx_buf = &st->data.scan.sample_buf;
+ t.len = 4;
+ t.delay.value = 60;
+ t.delay.unit = SPI_DELAY_UNIT_NSECS;
+
+ spi_message_init_with_transfers(&m, &t, 1);
+
+ if (st->cnv_gpio)
+ gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
+
+ ret = spi_sync(st->spi, &m);
+ if (ret < 0)
+ goto err_out;
+
+ if (st->cnv_gpio)
+ gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan,
+ iio_get_time_ns(indio_dev));
+
+err_out:
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+}
+
+static const struct iio_info ad4000_info = {
+ .read_raw = &ad4000_read_raw,
+ .attrs = &ad4000_attribute_group,
+};
+
+static void ad4000_regulator_disable(void *reg)
+{
+ regulator_disable(reg);
+}
+
+static int ad4000_probe(struct spi_device *spi)
+{
+ const struct ad4000_chip_info *chip;
+ struct regulator *vref_reg;
+ struct iio_dev *indio_dev;
+ struct ad4000_state *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ chip = (const struct ad4000_chip_info *)device_get_match_data(&spi->dev);
+ if (!chip)
+ return -EINVAL;
+
+ st = iio_priv(indio_dev);
+ st->spi = spi;
+
+ vref_reg = devm_regulator_get(&spi->dev, "vref");
+ if (IS_ERR(vref_reg))
+ return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
+ "Failed to get vref regulator\n");
+
+ ret = regulator_enable(vref_reg);
+ if (ret < 0)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to enable voltage regulator\n");
+
+ ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
+ if (ret)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to add regulator disable action\n");
+
+ st->vref = regulator_get_voltage(vref_reg);
+ if (st->vref < 0)
+ return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
+
+ if (!device_property_present(&spi->dev, "adi,spi-cs-mode")) {
+ st->cnv_gpio = devm_gpiod_get(&spi->dev, "cnv", GPIOD_OUT_HIGH);
+ if (IS_ERR(st->cnv_gpio)) {
+ if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
+ "Failed to get CNV GPIO");
+ }
+ }
+
+ indio_dev->name = chip->dev_name;
+ indio_dev->info = &ad4000_info;
+ indio_dev->channels = &chip->chan_spec;
+ indio_dev->num_channels = 1;
+
+ if (device_property_present(&spi->dev, "adi,gain-milli")) {
+ u32 val;
+
+ ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
+ if (ret)
+ return ret;
+
+ switch (val) {
+ case 454:
+ st->pin_gain = AD4000_0454_GAIN;
+ break;
+ case 909:
+ st->pin_gain = AD4000_0909_GAIN;
+ break;
+ case 1000:
+ st->pin_gain = AD4000_1_GAIN;
+ break;
+ case 1900:
+ st->pin_gain = AD4000_1900_GAIN;
+ break;
+ default:
+ return dev_err_probe(&spi->dev, -EINVAL,
+ "Invalid firmware provided gain\n");
+ }
+ } else {
+ st->pin_gain = AD4000_1_GAIN;
+ }
+
+ /*
+ * ADCs that output twos complement code have one less bit to express
+ * voltage magnitude.
+ */
+ if (chip->chan_spec.scan_type.sign == 's')
+ ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1);
+ else
+ ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits);
+
+ ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+ &iio_pollfunc_store_time,
+ &ad4000_trigger_handler, NULL);
+ if (ret)
+ return ret;
+
+ return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad4000_id[] = {
+ { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] },
+ { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] },
+ { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] },
+ { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] },
+ { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] },
+ { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] },
+ { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] },
+ { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] },
+ { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] },
+ { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] },
+ { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] },
+ { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] },
+ { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] },
+ { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] },
+ { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] },
+ { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, ad4000_id);
+
+static const struct of_device_id ad4000_of_match[] = {
+ { .compatible = "adi,ad4000",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4000] },
+ { .compatible = "adi,ad4001",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4001] },
+ { .compatible = "adi,ad4002",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4002] },
+ { .compatible = "adi,ad4003",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4003] },
+ { .compatible = "adi,ad4004",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4004] },
+ { .compatible = "adi,ad4005",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4005] },
+ { .compatible = "adi,ad4006",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4006] },
+ { .compatible = "adi,ad4007",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4007] },
+ { .compatible = "adi,ad4008",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4008] },
+ { .compatible = "adi,ad4010",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4010] },
+ { .compatible = "adi,ad4011",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4011] },
+ { .compatible = "adi,ad4020",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4020] },
+ { .compatible = "adi,ad4021",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4021] },
+ { .compatible = "adi,ad4022",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4022] },
+ { .compatible = "adi,adaq4001",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_ADAQ4001] },
+ { .compatible = "adi,adaq4003",
+ .data = (struct ad4000_chip_info *)&ad4000_chips[ID_ADAQ4003] },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad4000_of_match);
+
+static struct spi_driver ad4000_driver = {
+ .driver = {
+ .name = "ad4000",
+ .of_match_table = ad4000_of_match,
+ },
+ .probe = ad4000_probe,
+ .id_table = ad4000_id,
+};
+module_spi_driver(ad4000_driver);
+
+MODULE_AUTHOR("Mircea Caprioru <[email protected]>");
+MODULE_AUTHOR("Marcelo Schmitt <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD4000 ADC driver");
+MODULE_LICENSE("GPL");
--
2.43.0
On Fri, 22 Mar 2024 19:05:08 -0300, Marcelo Schmitt wrote:
> Add device tree documentation for AD4000 series of ADC devices.
>
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
>
> Signed-off-by: Marcelo Schmitt <[email protected]>
> ---
> Pasting relevant comment from cover letter here to aid reviewers.
>
> These devices have the same SPI (Strange Peripheral Interface) as AD7944
> devices, which has been documented in ad7944.rst [1].
> The device tree description for SPI connections and mode can be the same as of
> ad7944 adi,spi-mode [2].
> Because ad4000 driver does not currently support daisy-chain mode, I simplified
> things a little bit. If having a more complete doc is preferred, I'm fine
> changing to that.
>
> [1]: https://lore.kernel.org/linux-iio/[email protected]/
> [2]: https://lore.kernel.org/linux-iio/[email protected]/
>
> .../bindings/iio/adc/adi,ad4000.yaml | 151 ++++++++++++++++++
> MAINTAINERS | 7 +
> 2 files changed, 158 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected)
from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected)
from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/81665b5f0d37d593e6d299528de8d68da8574077.1711131830.git.marcelo.schmitt@analog.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected)
> from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected)
> from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
>
ok, adding proper #address-cells and #size-cells fixes the warning.
'#address-cells':
const: 1
'#size-cells':
const: 0
I'm assuming missing those in v1 doesn't hurt review so will wait for some
feedback before sending a v2.
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/81665b5f0d37d593e6d299528de8d68da8574077.1711131830.git.marcelo.schmitt@analog.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>
On 23/03/2024 04:29, Marcelo Schmitt wrote:
>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected)
>> from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected)
>> from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
>>
>
> ok, adding proper #address-cells and #size-cells fixes the warning.
>
> '#address-cells':
> const: 1
>
> '#size-cells':
> const: 0
>
> I'm assuming missing those in v1 doesn't hurt review so will wait for some
> feedback before sending a v2.
Hurts in a way it is a proof you did not test your binding before
sending. Performing review on untested code might be a waste of
reviewers time. Please test your code before sending it. I am not going
to perform review of untested code.
It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.
Best regards,
Krzysztof
On Fri, 22 Mar 2024 19:05:08 -0300
Marcelo Schmitt <[email protected]> wrote:
> Add device tree documentation for AD4000 series of ADC devices.
>
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
>
> Signed-off-by: Marcelo Schmitt <[email protected]>
> ---
> Pasting relevant comment from cover letter here to aid reviewers.
>
> These devices have the same SPI (Strange Peripheral Interface) as AD7944
> devices, which has been documented in ad7944.rst [1].
> The device tree description for SPI connections and mode can be the same as of
> ad7944 adi,spi-mode [2].
> Because ad4000 driver does not currently support daisy-chain mode, I simplified
> things a little bit. If having a more complete doc is preferred, I'm fine
> changing to that.
>
> [1]: https://lore.kernel.org/linux-iio/[email protected]/
> [2]: https://lore.kernel.org/linux-iio/[email protected]/
>
> .../bindings/iio/adc/adi,ad4000.yaml | 151 ++++++++++++++++++
> MAINTAINERS | 7 +
> 2 files changed, 158 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> new file mode 100644
> index 000000000000..9e3d6a3920ea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> @@ -0,0 +1,151 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD4000 ADC device driver
> +
> +maintainers:
> + - Marcelo Schmitt <[email protected]>
> +
> +description: |
> + Analog Devices AD4000 family of Analog to Digital Converters with SPI support.
> + Specifications can be found at:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad4000
> + - adi,ad4001
> + - adi,ad4002
> + - adi,ad4003
> + - adi,ad4004
> + - adi,ad4005
> + - adi,ad4006
> + - adi,ad4007
> + - adi,ad4008
> + - adi,ad4010
> + - adi,ad4011
> + - adi,ad4020
> + - adi,ad4021
> + - adi,ad4022
> + - adi,adaq4001
> + - adi,adaq4003
> +
> + reg: true
> + spi-max-frequency: true
> +
> + vref-supply:
> + description: Phandle to the regulator for ADC reference voltage.
> +
> + adi,gain-milli:
> + description: |
> + The hardware gain applied to the ADC input (in milli units).
> + The gain provided by the ADC input scaler is defined by the hardware
> + connections between chip pins OUT+, R1K-, R1K1-, R1K+, R1K1+, and OUT-.
> + If not present, default to 1000 (no actual gain applied).
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [454, 909, 1000, 1900]
> + default: 1000
> +
> + adi,spi-cs-mode:
We've just merged a driver for the ad7944 and bindings which has a
similar 3-wire-mode. Please share the approach used in that binding.
Whilst it seems we don't have the other mode here, I think we still want
to use a similar enum.
+CC David to take a look at this one given he went through long
discussions on how to deal with it for the driver he was working on
so probably remembers the reasoning etc better than I do :)
Jonathan
> + type: boolean
> + description: |
> + This property indicates the SPI wiring configuration.
> +
> + When this property is omitted, it indicates that the device SDI pin is
> + connected to SPI controller CS line and device CNV pin has been connected
> + to a GPIO. Datasheets call this "4-wire mode".
> +
> + When this property is present, the driver must assume standard SPI
> + connections which, for these devices, consists of connecting the
> + controller CS line to device CNV pin. This configuration is
> + (misleadingly) called "3-wire mode" in datasheets.
> +
> + cnv-gpios:
> + description: The GPIO connected to the CNV pin.
> + maxItems: 1
> +
> +patternProperties:
> + "^channel@([0-1])$":
> + $ref: adc.yaml
> + type: object
> + description: Represents the external channel connected to the ADC.
> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + diff-channels: true
> +
> + required:
> + - reg
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - vref-supply
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> + - if:
> + properties:
> + adi,spi-cs-mode: false
> + then:
> + required:
> + - cnv-gpios
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + /* Example for a AD4000 devices */
> + adc@0 {
> + compatible = "adi,ad4020";
> + reg = <0>;
> + spi-max-frequency = <71000000>;
> + vref-supply = <&vref>;
> + cnv-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + channel@0 {
> + reg = <0>;
> + diff-channels = <0 1>;
> + };
> + };
> + };
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + /* Example for a ADAQ4000 devices */
> + adc@0 {
> + compatible = "adi,adaq4003";
> + reg = <0>;
> + spi-max-frequency = <80000000>;
> + vref-supply = <&vref>;
> + adi,spi-cs-mode;
> + adi,gain-milli = <454>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + channel@0 {
> + reg = <0>;
> + diff-channels = <0 1>;
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2662ec49b297..3ca90f842298 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1135,6 +1135,13 @@ W: https://ez.analog.com/linux-software-drivers
> F: Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
> F: drivers/iio/dac/ad3552r.c
>
> +ANALOG DEVICES INC AD4000 DRIVER
> +M: Marcelo Schmitt <[email protected]>
> +L: [email protected]
> +S: Supported
> +W: https://ez.analog.com/linux-software-drivers
> +F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> +
> ANALOG DEVICES INC AD4130 DRIVER
> M: Cosmin Tanislav <[email protected]>
> L: [email protected]
On Fri, 22 Mar 2024 19:05:34 -0300
Marcelo Schmitt <[email protected]> wrote:
> Add support for AD4000 series of high accuracy, high speed, low power,
> successive aproximation register (SAR) ADCs.
>
> Signed-off-by: Marcelo Schmitt <[email protected]>
> ---
> Pasting relevant comment from cover letter here to aid reviewers.
>
> Differently from AD7944, AD4000 devices have a configuration register to
> toggle some features. For instance, turbo mode is set through configuration
> register rather than an external pin. This simplifies hardware connections,
> but then requires software interface. So, additional ABI being proposed
> in sysfs-bus-iio-adc-ad4000. The one I'm most in doubt about is
> span_compression_en which affects the in_voltageY_scale attribute.
> That might be instead supported by providing _scale_available and allowing write
> to _scale.
Yes. That's what I suggested inline before reading this properly. Much
prefer that as standard tooling will know how to use it.
Various comments inline. In particularly look at the spi helpers.
It's unusual to see a driver do so much manual handling of transfers.
Jonathan
>
> .../ABI/testing/sysfs-bus-iio-adc-ad4000 | 36 +
> MAINTAINERS | 2 +
> drivers/iio/adc/Kconfig | 12 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad4000.c | 666 ++++++++++++++++++
> 5 files changed, 717 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
> create mode 100644 drivers/iio/adc/ad4000.c
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
> new file mode 100644
> index 000000000000..98fb7539ad6d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
> @@ -0,0 +1,36 @@
> +What: /sys/bus/iio/devices/iio:deviceX/turbo_en
> +KernelVersion: 6.9
> +Contact: [email protected]
> +Description:
> + This attribute is used to enable/disable turbo mode allowing
> + slower SPI clock rates (at a minimum SCK rate of 75 MHz) to
> + achieve the maximum throughput of 2 MSPS.
When would we turn this on or off from userspace? From this brief description
it sounds like either we are trying to run very fast and the SPI bus can't clock
quick enough for the non turbo mode. In which case detect that we need it
and then turn it on.
Right now I have no idea why I'd ever turn this off. Who doesn't want to
press the turbo button?
> +
> +What: /sys/bus/iio/devices/iio:deviceX/span_compression_en
> +KernelVersion: 6.9
> +Contact: [email protected]
> +Description:
> + This attribute is used to enable/disable the input span
> + compression feature that reduces the ADC input range by 10% from
> + the top and bottom of the range while still accessing all
> + available ADC codes. Enabling span compression causes a
> + decrease in ADC scale which is reflected in the channel
> + in_voltageY_scale attribute.
Can you not control it via in_voltageY_scale then?
> +
> +What: /sys/bus/iio/devices/iio:deviceX/status_bits_en
> +KernelVersion: 6.9
> +Contact: [email protected]
> +Description:
> + This attribute is used to make the chip append a 6-bit status
> + word at the end of conversion results. The 6 status bits contain
> + the configuration register fields for OV clamp flag, span
> + compression, high-z mode, and turbo mode.
Why would I want this? Which of these isn't something I control?
This sounds like a debug feature we shouldn't enable.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/high_impedance_en
> +KernelVersion: 6.9
> +Contact: [email protected]
> +Description:
> + This attribute is used to enable/disable high impedance mode
> + (high-z) which reduces nonlinear charge kickback to the ADC
> + input.
For this one, do we have precendence in other drivers? I'm find with a control
just not sure if this the write ABI to use.
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index b3c434722364..f535d617ae89 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -6,6 +6,7 @@
> # When adding new entries keep the list in alphabetical order
> obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
> obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> +obj-$(CONFIG_AD4000) += ad4000.o
> obj-$(CONFIG_AD4130) += ad4130.o
> obj-$(CONFIG_AD7091R) += ad7091r-base.o
> obj-$(CONFIG_AD7091R5) += ad7091r5.o
> diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> new file mode 100644
> index 000000000000..f77104755979
> --- /dev/null
> +++ b/drivers/iio/adc/ad4000.c
> @@ -0,0 +1,666 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD4000 SPI ADC driver
> + *
> + * Copyright 2024 Analog Devices Inc.
> + */
> +#include <asm/unaligned.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/math.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
Why? Probably want mod_devicetable.h
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/units.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
You aren't providing a trigger so shouldn't need this one I think.
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#define AD400X_READ_COMMAND 0x54
> +#define AD400X_WRITE_COMMAND 0x14
> +
> +#define AD4000_CONFIG_REG_MSK 0xFF
> +
> +/* AD4000 Configuration Register programmable bits */
> +#define AD4000_STATUS BIT(4) /* Status bits output */
> +#define AD4000_SPAN_COMP BIT(3) /* Input span compression */
> +#define AD4000_HIGHZ BIT(2) /* High impedance mode */
> +#define AD4000_TURBO BIT(1) /* Turbo mode */
> +
> +#define AD4000_16BIT_MSK GENMASK(31, 16)
> +#define AD4000_18BIT_MSK GENMASK(31, 14)
> +#define AD4000_20BIT_MSK GENMASK(31, 12)
> +
> +#define AD4000_CHANNEL(_sign, _real_bits) \
> + { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_type = { \
> + .sign = _sign, \
> + .realbits = _real_bits, \
> + .storagebits = 32, \
Why store a 16 bit value in 32 bits?
Obviously will make code more complex to handling it differently but
the memory saving could be significant if the buffer is large.
> + .endianness = IIO_BE, \
> + }, \
> + } \
> +
> +
> +static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
> +{
> + struct spi_transfer t = {
> + .tx_buf = st->data.d8,
> + .len = 2,
> + };
> + struct spi_message m;
> +
> + put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val, st->data.d8);
> +
> + spi_message_init(&m);
> + spi_message_add_tail(&t, &m);
Why can't use init_with_transfers? Though maybe spi_write() is enough here and in
other cases. + look at spi_write_then_read() and see if that's appropriate for
others.
> +
> + return spi_sync(st->spi, &m);
> +}
> +
> +static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
> +{
> + struct spi_transfer t = {0};
> + struct spi_message m;
> + int ret;
> +
> + st->data.d8[0] = AD400X_READ_COMMAND;
> +
> + t.rx_buf = st->data.d8;
> + t.tx_buf = st->data.d8;
> + t.len = 2;
As below on structure initialization.
> +
> + spi_message_init_with_transfers(&m, &t, 1);
> +
> + ret = spi_sync(st->spi, &m);
> + if (ret < 0)
> + return ret;
> +
> + *val = FIELD_GET(AD4000_CONFIG_REG_MSK, get_unaligned_be16(st->data.d8));
> +
> + return ret;
> +}
> +
> +static int ad4000_read_sample(struct ad4000_state *st, uint32_t *val)
> +{
> + struct spi_transfer t = {0};
> + struct spi_message m;
> + int ret;
> +
> + t.rx_buf = &st->data.scan.sample_buf;
> + t.len = 4;
> + t.delay.value = 60;
> + t.delay.unit = SPI_DELAY_UNIT_NSECS;
Similar to below, use c99 style structure initialization.
> +
> + spi_message_init_with_transfers(&m, &t, 1);
> +
> + if (st->cnv_gpio)
> + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH);
> +
> + ret = spi_sync(st->spi, &m);
> + if (ret < 0)
> + return ret;
> +
> + if (st->cnv_gpio)
> + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
> +
> + *val = get_unaligned_be32(&st->data.scan.sample_buf);
> +
> + return 0;
> +}
> +
> +static int ad4000_single_conversion(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int *val)
> +{
> + struct ad4000_state *st = iio_priv(indio_dev);
> + u32 sample;
> + int ret;
> +
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = ad4000_read_sample(st, &sample);
> +
> + iio_device_release_direct_mode(indio_dev);
> +
> + if (ret)
> + return ret;
Scoped version works here too to avoid the need for delaying the error check.
(see below)
> +
> + switch (chan->scan_type.realbits) {
> + case 16:
> + sample = FIELD_GET(AD4000_16BIT_MSK, sample);
> + break;
> + case 18:
> + sample = FIELD_GET(AD4000_18BIT_MSK, sample);
> + break;
> + case 20:
> + sample = FIELD_GET(AD4000_20BIT_MSK, sample);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (chan->scan_type.sign == 's')
> + *val = sign_extend32(sample, chan->scan_type.realbits - 1);
> +
> + return IIO_VAL_INT;
> +}
> +static ssize_t ad4000_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct ad4000_state *st = iio_priv(indio_dev);
> + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> + unsigned int reg_val;
> + bool val;
> + int ret;
> +
> + ret = kstrtobool(buf, &val);
> + if (ret < 0)
> + return ret;
> +
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = ad4000_read_reg(st, ®_val);
> + if (ret < 0)
> + goto err_release;
> +
> + switch ((u32)this_attr->address) {
> + case AD4000_STATUS:
> + reg_val &= ~AD4000_STATUS;
> + reg_val |= FIELD_PREP(AD4000_STATUS, val);
> + ret = ad4000_write_reg(st, reg_val);
> + if (ret < 0)
> + goto err_release;
> +
> + st->status_bits = val;
> + break;
> + case AD4000_SPAN_COMP:
> + reg_val &= ~AD4000_SPAN_COMP;
> + reg_val |= FIELD_PREP(AD4000_SPAN_COMP, val);
> + ret = ad4000_write_reg(st, reg_val);
> + if (ret < 0)
> + goto err_release;
> +
> + st->span_comp = val;
> + break;
> + case AD4000_HIGHZ:
> + reg_val &= ~AD4000_HIGHZ;
> + reg_val |= FIELD_PREP(AD4000_HIGHZ, val);
> + ret = ad4000_write_reg(st, reg_val);
> + if (ret < 0)
> + goto err_release;
> +
> + st->high_z_mode = val;
> + break;
> + case AD4000_TURBO:
> + reg_val &= ~AD4000_TURBO;
> + reg_val |= FIELD_PREP(AD4000_TURBO, val);
> + ret = ad4000_write_reg(st, reg_val);
> + if (ret < 0)
> + goto err_release;
> +
> + st->turbo_mode = val;
> + break;
> + default:
> + ret = -EINVAL;
> + goto err_release;
> + }
> +
> +err_release:
> + iio_device_release_direct_mode(indio_dev);
The scoped cleanup.h based version of this is no upstream and would clean#
this code up a lot by allowing early returns
See iio_device_claim_direct_scoped()
> + return ret ? ret : len;
> +}
> +static irqreturn_t ad4000_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct ad4000_state *st = iio_priv(indio_dev);
> + struct spi_transfer t = {0};
Spaces around the 0, though technically I think you just need {};
> + struct spi_message m;
> + int ret;
> +
> + t.rx_buf = &st->data.scan.sample_buf;
> + t.len = 4;
> + t.delay.value = 60;
> + t.delay.unit = SPI_DELAY_UNIT_NSECS;
Better still.
struct spi_transfer t = {
.rx_buf = &...
etc
};
> +
> + spi_message_init_with_transfers(&m, &t, 1);
> +
> + if (st->cnv_gpio)
> + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
> +
> + ret = spi_sync(st->spi, &m);
> + if (ret < 0)
> + goto err_out;
> +
> + if (st->cnv_gpio)
> + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan,
> + iio_get_time_ns(indio_dev));
> +
> +err_out:
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info ad4000_info = {
> + .read_raw = &ad4000_read_raw,
> + .attrs = &ad4000_attribute_group,
> +};
> +
> +static void ad4000_regulator_disable(void *reg)
> +{
> + regulator_disable(reg);
> +}
> +
> +static int ad4000_probe(struct spi_device *spi)
> +{
> + const struct ad4000_chip_info *chip;
> + struct regulator *vref_reg;
> + struct iio_dev *indio_dev;
> + struct ad4000_state *st;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + chip = (const struct ad4000_chip_info *)device_get_match_data(&spi->dev);
Shouldn't need the cast. It's casting const void * to another const pointer
which the c spec allows to be done implicitly
> + if (!chip)
> + return -EINVAL;
> +
> + st = iio_priv(indio_dev);
> + st->spi = spi;
> +
> + vref_reg = devm_regulator_get(&spi->dev, "vref");
I'm guessing there are other power supplies? You should enable
them as well and given you don't use the voltage of the others you
can just use the calls to get them enabled.
> + if (IS_ERR(vref_reg))
> + return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
> + "Failed to get vref regulator\n");
> +
> + ret = regulator_enable(vref_reg);
> + if (ret < 0)
> + return dev_err_probe(&spi->dev, ret,
> + "Failed to enable voltage regulator\n");
> +
> + ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
> + if (ret)
> + return dev_err_probe(&spi->dev, ret,
> + "Failed to add regulator disable action\n");
> +
> + st->vref = regulator_get_voltage(vref_reg);
> + if (st->vref < 0)
> + return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
> +
> + if (!device_property_present(&spi->dev, "adi,spi-cs-mode")) {
> + st->cnv_gpio = devm_gpiod_get(&spi->dev, "cnv", GPIOD_OUT_HIGH);
> + if (IS_ERR(st->cnv_gpio)) {
> + if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
> + "Failed to get CNV GPIO");
> + }
> + }
> +
> + indio_dev->name = chip->dev_name;
> + indio_dev->info = &ad4000_info;
> + indio_dev->channels = &chip->chan_spec;
> + indio_dev->num_channels = 1;
> +
> + if (device_property_present(&spi->dev, "adi,gain-milli")) {
> + u32 val;
> +
> + ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
> + if (ret)
> + return ret;
> +
> + switch (val) {
> + case 454:
> + st->pin_gain = AD4000_0454_GAIN;
> + break;
> + case 909:
> + st->pin_gain = AD4000_0909_GAIN;
> + break;
> + case 1000:
> + st->pin_gain = AD4000_1_GAIN;
> + break;
> + case 1900:
> + st->pin_gain = AD4000_1900_GAIN;
> + break;
> + default:
> + return dev_err_probe(&spi->dev, -EINVAL,
> + "Invalid firmware provided gain\n");
> + }
> + } else {
> + st->pin_gain = AD4000_1_GAIN;
For defaults, a nice pattern is to just set them before the
attempt to read the property and don't update them if the property
isn't available.
st->pin_gain = AD4000_1_GAIN;
if (device_property_present(&spi->dev, "adi,gain-milli")) {
...
}
> + }
> +static const struct spi_device_id ad4000_id[] = {
> + { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] },
> + { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] },
> + { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] },
> + { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] },
> + { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] },
> + { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] },
> + { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] },
> + { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] },
> + { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] },
> + { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] },
> + { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] },
> + { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] },
> + { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] },
> + { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] },
> + { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] },
> + { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] },
> + { }
> +};
> +MODULE_DEVICE_TABLE(spi, ad4000_id);
> +
> +static const struct of_device_id ad4000_of_match[] = {
> + { .compatible = "adi,ad4000",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4000] },
Why the casts? Shouldn't need them as data is a const void * and you
are casting from another const pointer.
> + { .compatible = "adi,ad4001",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4001] },
> + { .compatible = "adi,ad4002",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4002] },
> + { .compatible = "adi,ad4003",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4003] },
> + { .compatible = "adi,ad4004",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4004] },
> + { .compatible = "adi,ad4005",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4005] },
> + { .compatible = "adi,ad4006",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4006] },
> + { .compatible = "adi,ad4007",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4007] },
> + { .compatible = "adi,ad4008",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4008] },
> + { .compatible = "adi,ad4010",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4010] },
> + { .compatible = "adi,ad4011",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4011] },
> + { .compatible = "adi,ad4020",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4020] },
> + { .compatible = "adi,ad4021",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4021] },
> + { .compatible = "adi,ad4022",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4022] },
> + { .compatible = "adi,adaq4001",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_ADAQ4001] },
> + { .compatible = "adi,adaq4003",
> + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_ADAQ4003] },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad4000_of_match);
On Sat, Mar 23, 2024 at 1:45 PM Jonathan Cameron <[email protected]> wrote:
>
> On Fri, 22 Mar 2024 19:05:08 -0300
> Marcelo Schmitt <[email protected]> wrote:
>
> > Add device tree documentation for AD4000 series of ADC devices.
> >
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> >
> > Signed-off-by: Marcelo Schmitt <[email protected]>
> > ---
> > Pasting relevant comment from cover letter here to aid reviewers.
> >
> > These devices have the same SPI (Strange Peripheral Interface) as AD7944
> > devices, which has been documented in ad7944.rst [1].
> > The device tree description for SPI connections and mode can be the same as of
> > ad7944 adi,spi-mode [2].
> > Because ad4000 driver does not currently support daisy-chain mode, I simplified
> > things a little bit. If having a more complete doc is preferred, I'm fine
> > changing to that.
Yes, having a complete binding is always preferred [1]. Bindings
should never omit anything just because it isn't implemented in the
driver.
[1]: https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bindings.html
..
> > +
> > + adi,spi-cs-mode:
>
> We've just merged a driver for the ad7944 and bindings which has a
> similar 3-wire-mode. Please share the approach used in that binding.
> Whilst it seems we don't have the other mode here, I think we still want
> to use a similar enum.
The ad40xx chips actually do have the same daisy chain mode. So the
exact same property and all enum values apply.
> +CC David to take a look at this one given he went through long
> discussions on how to deal with it for the driver he was working on
> so probably remembers the reasoning etc better than I do :)
>
In addition to the SPI wiring modes, the proposed bindings are also
missing power supplies and the busy interrupt.
Also, since the ADAQ chips are quite different from the AD chips, it
would be very helpful for reviewers (and git history) to split out
adding those chips to the DT bindings and driver into separate
patches. This way we can clearly see which features only apply to the
ADAQ chips.
Here is what I would consider a reasonably complete binding for the
AD40XX chips (excluding ADAQ for now as I suggested).
---
# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
%YAML 1.2
---
$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
title: Analog Devices AD4000 and similar Analog to Digital Converters
maintainers:
- Marcelo Schmitt <[email protected]>
description: |
Analog Devices AD4000 family of Analog to Digital Converters with SPI support.
Specifications can be found at:
https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
$ref: /schemas/spi/spi-peripheral-props.yaml#
properties:
compatible:
enum:
- adi,ad4000
- adi,ad4001
- adi,ad4002
- adi,ad4003
- adi,ad4004
- adi,ad4005
- adi,ad4006
- adi,ad4007
- adi,ad4008
- adi,ad4010
- adi,ad4011
- adi,ad4020
- adi,ad4021
- adi,ad4022
reg:
maxItems: 1
spi-max-frequency:
maximum: 102040816 # for VIO > 2.7 V, 81300813 for VIO > 1.7 V
spi-cpha: true
adi,spi-mode:
$ref: /schemas/types.yaml#/definitions/string
enum: [ single, chain ]
description: |
This property indicates the SPI wiring configuration.
When this property is omitted, it is assumed that the device is using what
the datasheet calls "4-wire mode". This is the conventional SPI mode used
when there are multiple devices on the same bus. In this mode, the CNV
line is used to initiate the conversion and the SDI line is connected to
CS on the SPI controller.
When this property is present, it indicates that the device is using one
of the following alternative wiring configurations:
* single: The datasheet calls this "3-wire mode". (NOTE: The datasheet's
definition of 3-wire mode is NOT at all related to the standard
spi-3wire property!) This mode is often used when the ADC is the only
device on the bus. In this mode, SDI is tied to VIO, and the CNV line
can be connected to the CS line of the SPI controller or to a GPIO, in
which case the CS line of the controller is unused.
* chain: The datasheet calls this "chain mode". This mode is used to save
on wiring when multiple ADCs are used. In this mode, the SDI line of
one chip is tied to the SDO of the next chip in the chain and the SDI of
the last chip in the chain is tied to GND. Only the first chip in the
chain is connected to the SPI bus. The CNV line of all chips are tied
together. The CS line of the SPI controller can be used as the CNV line
only if it is active high.
'#daisy-chained-devices': true
vdd-supply:
description: A 1.8V supply that powers the chip (VDD).
vio-supply:
description:
A 1.8V to 5V supply for the digital inputs and outputs (VIO).
ref-supply:
description:
A 2.5 to 5V supply for the external reference voltage (REF).
cnv-gpios:
description:
The Convert Input (CNV). This input has multiple functions. It initiates
the conversions and selects the SPI mode of the device (chain or CS). In
'single' mode, this property is omitted if the CNV pin is connected to the
CS line of the SPI controller.
maxItems: 1
interrupts:
description:
The SDO pin can also function as a busy indicator. This node should be
connected to an interrupt that is triggered when the SDO line goes low
while the SDI line is high and the CNV line is low ('single' mode) or the
SDI line is low and the CNV line is high ('multi' mode); or when the SDO
line goes high while the SDI and CNV lines are high (chain mode),
maxItems: 1
required:
- compatible
- reg
- vdd-supply
- vio-supply
- ref-supply
allOf:
# in '4-wire' mode, cnv-gpios is required, for other modes it is optional
- if:
not:
required:
- adi,spi-mode
then:
required:
- cnv-gpios
# chain mode has lower SCLK max rate
- if:
required:
- adi,spi-mode
properties:
adi,spi-mode:
const: chain
then:
properties:
spi-max-frequency:
maximum: 50000000 # for VIO > 2.7 V, 40000000 for VIO > 1.7 V
required:
- '#daisy-chained-devices'
else:
properties:
'#daisy-chained-devices': false
unevaluatedProperties: false
examples:
- |
#include <dt-bindings/gpio/gpio.h>
spi {
#address-cells = <1>;
#size-cells = <0>;
adc@0 {
compatible = "adi,ad4020";
reg = <0>;
spi-max-frequency = <71000000>;
vdd-supply = <&supply_1_8V>;
vio-supply = <&supply_1_8V>;
ref-supply = <&supply_5V>;
cnv-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH>;
};
};
On Sat, Mar 23, 2024 at 3:18 PM David Lechner <[email protected]> wrote:
..
> Here is what I would consider a reasonably complete binding for the
> AD40XX chips (excluding ADAQ for now as I suggested).
I missed one...
I also think it makes sense for the High-Z mode selection to be a DT
property since needing to enable it or disable it depends entirely on
what is connected to the analog input pins.
---
adi,high-z-input:
type: boolean
description:
High-Z mode allows the amplifier and RC filter in front of the ADC to be
chosen based on the signal bandwidth of interest, rather than the settling
requirements of the switched capacitor SAR ADC inputs.
On 03/23, David Lechner wrote:
> On Sat, Mar 23, 2024 at 3:18 PM David Lechner <[email protected]> wrote:
>
> ...
>
> > Here is what I would consider a reasonably complete binding for the
> > AD40XX chips (excluding ADAQ for now as I suggested).
>
> I missed one...
>
> I also think it makes sense for the High-Z mode selection to be a DT
> property since needing to enable it or disable it depends entirely on
> what is connected to the analog input pins.
>
> ---
>
> adi,high-z-input:
> type: boolean
> description:
> High-Z mode allows the amplifier and RC filter in front of the ADC to be
> chosen based on the signal bandwidth of interest, rather than the settling
> requirements of the switched capacitor SAR ADC inputs.
ok, will do the suggested changes, including provide AD and ADAQ in separate patches.
Thanks,
Marcelo
On Fri, Mar 22, 2024 at 5:06 PM Marcelo Schmitt
<[email protected]> wrote:
>
> Add support for AD4000 series of high accuracy, high speed, low power,
> successive aproximation register (SAR) ADCs.
>
> Signed-off-by: Marcelo Schmitt <[email protected]>
> ---
> Pasting relevant comment from cover letter here to aid reviewers.
>
> Differently from AD7944, AD4000 devices have a configuration register to
> toggle some features. For instance, turbo mode is set through configuration
> register rather than an external pin. This simplifies hardware connections,
> but then requires software interface. So, additional ABI being proposed
> in sysfs-bus-iio-adc-ad4000. The one I'm most in doubt about is
> span_compression_en which affects the in_voltageY_scale attribute.
> That might be instead supported by providing _scale_available and allowing write
> to _scale.
>
> .../ABI/testing/sysfs-bus-iio-adc-ad4000 | 36 +
> MAINTAINERS | 2 +
> drivers/iio/adc/Kconfig | 12 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad4000.c | 666 ++++++++++++++++++
> 5 files changed, 717 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
> create mode 100644 drivers/iio/adc/ad4000.c
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
> new file mode 100644
> index 000000000000..98fb7539ad6d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
> @@ -0,0 +1,36 @@
> +What: /sys/bus/iio/devices/iio:deviceX/turbo_en
> +KernelVersion: 6.9
> +Contact: [email protected]
> +Description:
> + This attribute is used to enable/disable turbo mode allowing
> + slower SPI clock rates (at a minimum SCK rate of 75 MHz) to
> + achieve the maximum throughput of 2 MSPS.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/span_compression_en
> +KernelVersion: 6.9
> +Contact: [email protected]
> +Description:
> + This attribute is used to enable/disable the input span
> + compression feature that reduces the ADC input range by 10% from
> + the top and bottom of the range while still accessing all
> + available ADC codes. Enabling span compression causes a
> + decrease in ADC scale which is reflected in the channel
> + in_voltageY_scale attribute.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/status_bits_en
> +KernelVersion: 6.9
> +Contact: [email protected]
> +Description:
> + This attribute is used to make the chip append a 6-bit status
> + word at the end of conversion results. The 6 status bits contain
> + the configuration register fields for OV clamp flag, span
> + compression, high-z mode, and turbo mode.
> +
I agree with Jonathan that the three attributes above are not needed
(for the reasons he mentioned).
> +What: /sys/bus/iio/devices/iio:deviceX/high_impedance_en
> +KernelVersion: 6.9
> +Contact: [email protected]
> +Description:
> + This attribute is used to enable/disable high impedance mode
> + (high-z) which reduces nonlinear charge kickback to the ADC
> + input.
> +
As I mentioned in the DT patch, this one seems like it should be a DT
property, not a runtime setting.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3ca90f842298..2ae98c700ff0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1140,7 +1140,9 @@ M: Marcelo Schmitt <[email protected]>
> L: [email protected]
> S: Supported
> W: https://ez.analog.com/linux-software-drivers
> +F: Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000
> F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> +F: drivers/iio/adc/ad4000.c
>
> ANALOG DEVICES INC AD4130 DRIVER
> M: Cosmin Tanislav <[email protected]>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 0d9282fa67f5..15db35f00ef0 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -21,6 +21,18 @@ config AD_SIGMA_DELTA
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
>
> +config AD4000
> + tristate "Analog Devices AD4000 ADC Driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes here to build support for Analog Devices AD4000 high speed
> + SPI analog to digital converters (ADC).
> +
> + To compile this driver as a module, choose M here: the module will be
> + called ad4000.
> +
> config AD4130
> tristate "Analog Device AD4130 ADC Driver"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index b3c434722364..f535d617ae89 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -6,6 +6,7 @@
> # When adding new entries keep the list in alphabetical order
> obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
> obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
> +obj-$(CONFIG_AD4000) += ad4000.o
> obj-$(CONFIG_AD4130) += ad4130.o
> obj-$(CONFIG_AD7091R) += ad7091r-base.o
> obj-$(CONFIG_AD7091R5) += ad7091r5.o
> diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
> new file mode 100644
> index 000000000000..f77104755979
> --- /dev/null
> +++ b/drivers/iio/adc/ad4000.c
> @@ -0,0 +1,666 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AD4000 SPI ADC driver
> + *
> + * Copyright 2024 Analog Devices Inc.
> + */
> +#include <asm/unaligned.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/math.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/units.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#define AD400X_READ_COMMAND 0x54
> +#define AD400X_WRITE_COMMAND 0x14
> +
> +#define AD4000_CONFIG_REG_MSK 0xFF
> +
> +/* AD4000 Configuration Register programmable bits */
> +#define AD4000_STATUS BIT(4) /* Status bits output */
> +#define AD4000_SPAN_COMP BIT(3) /* Input span compression */
> +#define AD4000_HIGHZ BIT(2) /* High impedance mode */
> +#define AD4000_TURBO BIT(1) /* Turbo mode */
> +
> +#define AD4000_16BIT_MSK GENMASK(31, 16)
> +#define AD4000_18BIT_MSK GENMASK(31, 14)
> +#define AD4000_20BIT_MSK GENMASK(31, 12)
> +
> +#define AD4000_CHANNEL(_sign, _real_bits) \
> + { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
Some chips are fully differential and some are pseudo-differential.
For the fully differential chips, I would expect
.differential = 1,
As discussed on other recent drivers, psudo-differential are
differential = 0 though.
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_type = { \
> + .sign = _sign, \
> + .realbits = _real_bits, \
> + .storagebits = 32, \
> + .endianness = IIO_BE, \
I think this should be IIO_CPU and we should make use of
bits_per_word in the SPI xfers like we do in the ad7944 driver.
Otherwise, I think we might have difficutly to achieve max sample rate
for the 18 and 20-bit chips once we get SPI offload support added.
> + }, \
> + } \
> +
> +enum ad4000_ids {
> + ID_AD4000,
> + ID_AD4001,
> + ID_AD4002,
> + ID_AD4003,
> + ID_AD4004,
> + ID_AD4005,
> + ID_AD4006,
> + ID_AD4007,
> + ID_AD4008,
> + ID_AD4010,
> + ID_AD4011,
> + ID_AD4020,
> + ID_AD4021,
> + ID_AD4022,
> + ID_ADAQ4001,
> + ID_ADAQ4003,
> +};
IMHO, these types of enums just make more work (noise) for people
reading the code and don't add anything useful.
> +
> +struct ad4000_chip_info {
> + const char *dev_name;
> + struct iio_chan_spec chan_spec;
> +};
> +
> +static const struct ad4000_chip_info ad4000_chips[] = {
> + [ID_AD4000] = {
> + .dev_name = "ad4000",
> + .chan_spec = AD4000_CHANNEL('u', 16),
> + },
> + [ID_AD4001] = {
> + .dev_name = "ad4001",
> + .chan_spec = AD4000_CHANNEL('s', 16),
> + },
> + [ID_AD4002] = {
> + .dev_name = "ad4002",
> + .chan_spec = AD4000_CHANNEL('u', 18),
> + },
> + [ID_AD4003] = {
> + .dev_name = "ad4003",
> + .chan_spec = AD4000_CHANNEL('s', 18),
> + },
> + [ID_AD4004] = {
> + .dev_name = "ad4004",
> + .chan_spec = AD4000_CHANNEL('u', 16),
> + },
> + [ID_AD4005] = {
> + .dev_name = "ad4005",
> + .chan_spec = AD4000_CHANNEL('s', 16),
> + },
> + [ID_AD4006] = {
> + .dev_name = "ad4006",
> + .chan_spec = AD4000_CHANNEL('u', 18),
> + },
> + [ID_AD4007] = {
> + .dev_name = "ad4007",
> + .chan_spec = AD4000_CHANNEL('s', 18),
> + },
> + [ID_AD4008] = {
> + .dev_name = "ad4008",
> + .chan_spec = AD4000_CHANNEL('u', 16),
> + },
> + [ID_AD4010] = {
> + .dev_name = "ad4010",
> + .chan_spec = AD4000_CHANNEL('u', 18),
> + },
> + [ID_AD4011] = {
> + .dev_name = "ad4011",
> + .chan_spec = AD4000_CHANNEL('s', 18),
> + },
> + [ID_AD4020] = {
> + .dev_name = "ad4020",
> + .chan_spec = AD4000_CHANNEL('s', 20),
> + },
> + [ID_AD4021] = {
> + .dev_name = "ad4021",
> + .chan_spec = AD4000_CHANNEL('s', 20),
> + },
> + [ID_AD4022] = {
> + .dev_name = "ad4022",
> + .chan_spec = AD4000_CHANNEL('s', 20),
> + },
> + [ID_ADAQ4001] = {
> + .dev_name = "adaq4001",
> + .chan_spec = AD4000_CHANNEL('s', 16),
> + },
> + [ID_ADAQ4003] = {
> + .dev_name = "adaq4003",
> + .chan_spec = AD4000_CHANNEL('s', 18),
> + },
> +};
As mentioned in my DT bindings reply, I think it would be helpful for
reviewers (and git history) to move adding ADAQ support to a separate
patch since they have some significant differences from the AD parts.
> +
> +enum ad4000_gains {
> + AD4000_0454_GAIN = 0,
> + AD4000_0909_GAIN = 1,
> + AD4000_1_GAIN = 2,
> + AD4000_1900_GAIN = 3,
> + AD4000_GAIN_LEN
> +};
> +
> +/*
> + * Gains stored and computed as fractions to avoid introducing rounding erros.
spelling: errors
> + */
> +static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = {
> + [AD4000_0454_GAIN] = { 227, 500 },
> + [AD4000_0909_GAIN] = { 909, 1000 },
> + [AD4000_1_GAIN] = { 1, 1 },
> + [AD4000_1900_GAIN] = { 19, 10 },
> +};
> +
> +struct ad4000_state {
> + struct spi_device *spi;
> + struct gpio_desc *cnv_gpio;
> + int vref;
> + bool status_bits;
> + bool span_comp;
> + bool turbo_mode;
> + bool high_z_mode;
> +
> + enum ad4000_gains pin_gain;
> + int scale_tbl[AD4000_GAIN_LEN][2];
> +
> + /*
> + * DMA (thus cache coherency maintenance) requires the
> + * transfer buffers to live in their own cache lines.
> + */
> + union {
> + struct {
> + u8 sample_buf[4];
> + s64 timestamp;
Usually we see __aligned(8) applied to the timestamp (I'm guessing
some archs need it?)
> + } scan;
> + u8 d8[2];
> + } data __aligned(IIO_DMA_MINALIGN);
> +};
> +
> +static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits)
> +{
> + int val, val2, tmp0, tmp1, i;
> + u64 tmp2;
> +
> + val2 = scale_bits;
> + for (i = 0; i < AD4000_GAIN_LEN; i++) {
> + val = st->vref / 1000;
> + /* Multiply by MILLI here to avoid losing precision */
> + val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI,
> + ad4000_gains_frac[i][0]);
> + /* Would multiply by NANO here but we already multiplied by MILLI */
> + tmp2 = shift_right((u64)val * MICRO, val2);
> + tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1);
> + st->scale_tbl[i][0] = tmp0; /* Integer part */
> + st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */
> + }
> +}
> +
> +static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
> +{
> + struct spi_transfer t = {
> + .tx_buf = st->data.d8,
> + .len = 2,
> + };
> + struct spi_message m;
> +
> + put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val, st->data.d8);
> +
> + spi_message_init(&m);
> + spi_message_add_tail(&t, &m);
> +
> + return spi_sync(st->spi, &m);
> +}
> +
> +static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
> +{
> + struct spi_transfer t = {0};
> + struct spi_message m;
> + int ret;
> +
> + st->data.d8[0] = AD400X_READ_COMMAND;
> +
> + t.rx_buf = st->data.d8;
> + t.tx_buf = st->data.d8;
> + t.len = 2;
> +
> + spi_message_init_with_transfers(&m, &t, 1);
> +
> + ret = spi_sync(st->spi, &m);
> + if (ret < 0)
> + return ret;
> +
> + *val = FIELD_GET(AD4000_CONFIG_REG_MSK, get_unaligned_be16(st->data.d8));
> +
> + return ret;
> +}
> +
> +static int ad4000_read_sample(struct ad4000_state *st, uint32_t *val)
As with the ad7944 driver, I would expect different handling for the
different SPI wiring modes. This looks like it only works with 4-wire
mode.
> +{
> + struct spi_transfer t = {0};
> + struct spi_message m;
> + int ret;
> +
> + t.rx_buf = &st->data.scan.sample_buf;
> + t.len = 4;
> + t.delay.value = 60;
> + t.delay.unit = SPI_DELAY_UNIT_NSECS;
> +
> + spi_message_init_with_transfers(&m, &t, 1);
> +
> + if (st->cnv_gpio)
> + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH);
> +
> + ret = spi_sync(st->spi, &m);
> + if (ret < 0)
> + return ret;
> +
> + if (st->cnv_gpio)
> + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
> +
> + *val = get_unaligned_be32(&st->data.scan.sample_buf);
> +
> + return 0;
> +}
> +
> +static int ad4000_single_conversion(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int *val)
> +{
> + struct ad4000_state *st = iio_priv(indio_dev);
> + u32 sample;
> + int ret;
> +
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = ad4000_read_sample(st, &sample);
> +
> + iio_device_release_direct_mode(indio_dev);
> +
> + if (ret)
> + return ret;
> +
> + switch (chan->scan_type.realbits) {
> + case 16:
> + sample = FIELD_GET(AD4000_16BIT_MSK, sample);
> + break;
> + case 18:
> + sample = FIELD_GET(AD4000_18BIT_MSK, sample);
> + break;
> + case 20:
> + sample = FIELD_GET(AD4000_20BIT_MSK, sample);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (chan->scan_type.sign == 's')
> + *val = sign_extend32(sample, chan->scan_type.realbits - 1);
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int ad4000_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long info)
> +{
> + struct ad4000_state *st = iio_priv(indio_dev);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_RAW:
> + return ad4000_single_conversion(indio_dev, chan, val);
> + case IIO_CHAN_INFO_SCALE:
> + *val = st->scale_tbl[st->pin_gain][0];
> + *val2 = st->scale_tbl[st->pin_gain][1];
> + if (st->span_comp)
> + *val2 = DIV_ROUND_CLOSEST(*val2 * 4, 5);
> + return IIO_VAL_INT_PLUS_NANO;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
..
> +static irqreturn_t ad4000_trigger_handler(int irq, void *p)
I would expect different handling for the different SPI wiring modes here too.
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct ad4000_state *st = iio_priv(indio_dev);
> + struct spi_transfer t = {0};
> + struct spi_message m;
> + int ret;
> +
> + t.rx_buf = &st->data.scan.sample_buf;
> + t.len = 4;
> + t.delay.value = 60;
> + t.delay.unit = SPI_DELAY_UNIT_NSECS;
> +
> + spi_message_init_with_transfers(&m, &t, 1);
> +
> + if (st->cnv_gpio)
> + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
> +
> + ret = spi_sync(st->spi, &m);
> + if (ret < 0)
> + goto err_out;
> +
> + if (st->cnv_gpio)
> + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan,
> + iio_get_time_ns(indio_dev));
> +
> +err_out:
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info ad4000_info = {
> + .read_raw = &ad4000_read_raw,
> + .attrs = &ad4000_attribute_group,
> +};
> +
> +static void ad4000_regulator_disable(void *reg)
> +{
> + regulator_disable(reg);
> +}
> +
> +static int ad4000_probe(struct spi_device *spi)
> +{
> + const struct ad4000_chip_info *chip;
> + struct regulator *vref_reg;
> + struct iio_dev *indio_dev;
> + struct ad4000_state *st;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + chip = (const struct ad4000_chip_info *)device_get_match_data(&spi->dev);
Should this be spi_get_device_match_data()?
Also don't need the cast here.
> + if (!chip)
> + return -EINVAL;
> +
> + st = iio_priv(indio_dev);
> + st->spi = spi;
> +
> + vref_reg = devm_regulator_get(&spi->dev, "vref");
This should to be devm_regulator_get_optional(), otherwise it can
return a "dummy" regulator if one is missing in the devicetree which
will fail when getting the voltage.
> + if (IS_ERR(vref_reg))
> + return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
> + "Failed to get vref regulator\n");
> +
> + ret = regulator_enable(vref_reg);
> + if (ret < 0)
> + return dev_err_probe(&spi->dev, ret,
> + "Failed to enable voltage regulator\n");
> +
> + ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
> + if (ret)
> + return dev_err_probe(&spi->dev, ret,
> + "Failed to add regulator disable action\n");
> +
> + st->vref = regulator_get_voltage(vref_reg);
> + if (st->vref < 0)
> + return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
> +
> + if (!device_property_present(&spi->dev, "adi,spi-cs-mode")) {
> + st->cnv_gpio = devm_gpiod_get(&spi->dev, "cnv", GPIOD_OUT_HIGH);
> + if (IS_ERR(st->cnv_gpio)) {
> + if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
> + "Failed to get CNV GPIO");
> + }
> + }
Since the DT bindings should be the same for the SPI wirting modes, we
should have the same property and logic as the ad7944 driver here.
> +
> + indio_dev->name = chip->dev_name;
> + indio_dev->info = &ad4000_info;
> + indio_dev->channels = &chip->chan_spec;
> + indio_dev->num_channels = 1;
> +
> + if (device_property_present(&spi->dev, "adi,gain-milli")) {
> + u32 val;
> +
> + ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
> + if (ret)
> + return ret;
> +
> + switch (val) {
> + case 454:
> + st->pin_gain = AD4000_0454_GAIN;
> + break;
> + case 909:
> + st->pin_gain = AD4000_0909_GAIN;
> + break;
> + case 1000:
> + st->pin_gain = AD4000_1_GAIN;
> + break;
> + case 1900:
> + st->pin_gain = AD4000_1900_GAIN;
> + break;
> + default:
> + return dev_err_probe(&spi->dev, -EINVAL,
> + "Invalid firmware provided gain\n");
> + }
> + } else {
> + st->pin_gain = AD4000_1_GAIN;
> + }
> +
> + /*
> + * ADCs that output twos complement code have one less bit to express
> + * voltage magnitude.
> + */
> + if (chip->chan_spec.scan_type.sign == 's')
> + ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1);
> + else
> + ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits);
> +
AFAICT, the gain stuff only applies to ADAQ chips, so it seems odd to
call this for all chips (and have the scale attribute report this for
chips that don't support it).
> + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> + &iio_pollfunc_store_time,
> + &ad4000_trigger_handler, NULL);
> + if (ret)
> + return ret;
> +
> + return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
..
On Sat, 23 Mar 2024 16:53:17 -0500
> > +
> > + /*
> > + * DMA (thus cache coherency maintenance) requires the
> > + * transfer buffers to live in their own cache lines.
> > + */
> > + union {
> > + struct {
> > + u8 sample_buf[4];
> > + s64 timestamp;
>
> Usually we see __aligned(8) applied to the timestamp (I'm guessing
> some archs need it?)
>
Good spot. Yes, x86_32 is the one that we most commonly refer to for this.
It aligns s64 to only 32 bits whereas IIO ABI is always naturally aligned.
> > + } scan;
> > + u8 d8[2];
> > + } data __aligned(IIO_DMA_MINALIGN);
> > +};
On Sat, Mar 23, 2024 at 4:53 PM David Lechner <[email protected]> wrote:
>
> On Fri, Mar 22, 2024 at 5:06 PM Marcelo Schmitt
> <[email protected]> wrote:
> >
..
> > +
> > + vref_reg = devm_regulator_get(&spi->dev, "vref");
>
> This should to be devm_regulator_get_optional(), otherwise it can
> return a "dummy" regulator if one is missing in the devicetree which
> will fail when getting the voltage.
>
> > + if (IS_ERR(vref_reg))
> > + return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
> > + "Failed to get vref regulator\n");
> > +
> > + ret = regulator_enable(vref_reg);
> > + if (ret < 0)
> > + return dev_err_probe(&spi->dev, ret,
> > + "Failed to enable voltage regulator\n");
> > +
> > + ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
> > + if (ret)
> > + return dev_err_probe(&spi->dev, ret,
> > + "Failed to add regulator disable action\n");
> > +
> > + st->vref = regulator_get_voltage(vref_reg);
> > + if (st->vref < 0)
> > + return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
> > +
> > + if (!device_property_present(&spi->dev, "adi,spi-cs-mode")) {
> > + st->cnv_gpio = devm_gpiod_get(&spi->dev, "cnv", GPIOD_OUT_HIGH);
> > + if (IS_ERR(st->cnv_gpio)) {
> > + if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
> > +
> > + return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
> > + "Failed to get CNV GPIO");
> > + }
> > + }
>
In a review for a different patch, Jonathan said he would prefer
devm_regulator_get() and failing in regulator_get_voltage() rather
than using devm_regulator_get_optional() so I think the same would
apply here and my suggestion should be overruled.