* [PATCH v3 0/2] AD4130
@ 2022-04-19 15:08 Cosmin Tanislav
2022-04-19 15:08 ` [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130 Cosmin Tanislav
2022-04-19 15:08 ` [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver Cosmin Tanislav
0 siblings, 2 replies; 15+ messages in thread
From: Cosmin Tanislav @ 2022-04-19 15:08 UTC (permalink / raw)
Cc: Jonathan Cameron, Rob Herring, Linus Walleij, linux-iio,
linux-gpio, linux-kernel, devicetree, Cosmin Tanislav
AD4130-8 is an ultra-low power, high precision, measurement solution for
low bandwidth battery operated applications.
The fully integrated AFE (Analog Front-End) includes a multiplexer for up
to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
selectable filter options, smart sequencer, sensor biasing and excitation
options, diagnostics, and a FIFO buffer.
V1 -> V2
* add kernel version to ABI file
* merge ABI patch into driver patch
* make copyright header similar to other drivers
* rearrange includes
* use units.h defines where possible and add unit sufix to
SOFT_RESET_SLEEP define
* remove ending comma to last members of enums / lists
* remove unused FILTER_MAX define
* use BIT macro for PIN_FN_*
* rearrange SETUP_SIZE definition
* group bools in ad4130_state and ad4130_chan_info
* put scale_tbls definition on one line
* remove newline before reg size == 0 check
* put mask used as value in a variable
* remove useless ret = 0 assignment
* make buffer attrs oneline
* use for_each_set_bit in update_scan_mode
* use if else for internal reference voltage error checking
* inline reference voltage check
* check number of vbias pins
* remove .has_int_pin = false
* remove avail_len for IIO_AVAIL_RANGE
* remove useless enabled_channels check in unlink_slot
* remove unused AD4130_RESET_CLK_COUNT define
* only call fwnode_handle_put for child in case of error
* default adi,reference-select to REFIN1
* default adi,int-ref-en to false
* of_irq_get_byname -> fwnode_irq_get_byname
* P1 -> P2 as interrupt pin options
* add missing comma in db3_freq_avail init
* cast values to u64 to make math using units.h work
* add datasheet reference to IRQ polarity
* add comment about disabling channels in predisable
* add part number prefix find_table_index
* return voltage from get_ref_voltage
* add datasheet reference for internal reference voltage selection
* add comment explaining AIN and GPIO pin sharing
* parse channel setup before parsing excitation pins
* only validate excitation pin if value is not off
* use FIELD_PREP for bipolar and int_ref_en
* put devm_regmap_init call on one line
* introduce a slot_info struct to contain setup_info for each slot
* enable internal reference automatically if needed
* decide mclk sel based on adi,ext-clk-freq and adi,int-clk-out
* dt-bindings: use internal reference explicitly
* dt-bindings: set type for adi,excitation-pin-0
* dt-bindings: set $ref for adi,vbias-pins
* dt-bindings: remove minItems from interrupts property
* dt-bindings: remove adi,int-ref-en default value
* dt-bindings: remove adi,bipolar default value
* dt-bindings: inline adi,int-ref-en description
* dt-bindings: default adi,reference-select to REFIN1
* dt-bindings: clean up description for diff-channels and
adi,reference-select
* dt-bindings: add more text to interrupt-names description
* dt-bindings: turn interrupt-names into a single string
* dt-bindings: add maxItems to adi,vbias-pins
V2 -> V3
* dt-bindings: add interrupt controller include to example
* dt-bindings: remove $ref in diff-channels
Cosmin Tanislav (2):
dt-bindings: iio: adc: add AD4130
iio: adc: ad4130: add AD4130 driver
.../ABI/testing/sysfs-bus-iio-adc-ad4130 | 36 +
.../bindings/iio/adc/adi,ad4130.yaml | 264 +++
MAINTAINERS | 8 +
drivers/iio/adc/Kconfig | 13 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4130.c | 2078 +++++++++++++++++
6 files changed, 2400 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
create mode 100644 drivers/iio/adc/ad4130.c
--
2.35.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130
2022-04-19 15:08 [PATCH v3 0/2] AD4130 Cosmin Tanislav
@ 2022-04-19 15:08 ` Cosmin Tanislav
2022-04-26 0:37 ` Rob Herring
2022-05-01 16:09 ` Jonathan Cameron
2022-04-19 15:08 ` [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver Cosmin Tanislav
1 sibling, 2 replies; 15+ messages in thread
From: Cosmin Tanislav @ 2022-04-19 15:08 UTC (permalink / raw)
Cc: Jonathan Cameron, Rob Herring, Linus Walleij, linux-iio,
linux-gpio, linux-kernel, devicetree, Cosmin Tanislav
AD4130-8 is an ultra-low power, high precision, measurement solution for
low bandwidth battery operated applications.
The fully integrated AFE (Analog Front-End) includes a multiplexer for up
to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
selectable filter options, smart sequencer, sensor biasing and excitation
options, diagnostics, and a FIFO buffer.
Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
.../bindings/iio/adc/adi,ad4130.yaml | 264 ++++++++++++++++++
1 file changed, 264 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
new file mode 100644
index 000000000000..32996b62cd20
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
@@ -0,0 +1,264 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2022 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad4130.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD4130 ADC device driver
+
+maintainers:
+ - Cosmin Tanislav <cosmin.tanislav@analog.com>
+
+description: |
+ Bindings for the Analog Devices AD4130 ADC. Datasheet can be found here:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD4130-8.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad4130-8-16-lfcsp
+ - adi,ad4130-8-16-wlcsp
+ - adi,ad4130-8-24-lfcsp
+ - adi,ad4130-8-24-wlcsp
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+ description: phandle to the master clock (mclk)
+
+ clock-names:
+ items:
+ - const: mclk
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ description: |
+ Specify which interrupt pin should be configured as Data Ready / FIFO
+ interrupt.
+ Default if not supplied is dout-int.
+ enum:
+ - dout-int
+ - clk
+ - p2
+ - dout
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ refin1-supply:
+ description: refin1 supply. Can be used as reference for conversion.
+
+ refin2-supply:
+ description: refin2 supply. Can be used as reference for conversion.
+
+ avdd-supply:
+ description: AVDD voltage supply. Can be used as reference for conversion.
+
+ iovdd-supply:
+ description: IOVDD voltage supply. Used for the chip interface.
+
+ spi-max-frequency:
+ maximum: 5000000
+
+ adi,int-clk-out:
+ description: Specify if the internal clock should be exposed on the CLK pin.
+ type: boolean
+
+ adi,ext-clk-freq:
+ description: Specify the frequency of the external clock.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [76800, 153600]
+ default: 76800
+
+ adi,bipolar:
+ description: Specify if the device should be used in bipolar mode.
+ type: boolean
+
+ adi,vbias-pins:
+ description: Analog inputs to apply a voltage bias of (AVDD − AVSS) / 2 to.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ maxItems: 16
+ items:
+ minimum: 0
+ maximum: 15
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+patternProperties:
+ "^channel@([0-9]|1[0-5])$":
+ type: object
+ $ref: adc.yaml
+
+ properties:
+ reg:
+ description: |
+ The channel number.
+ items:
+ minimum: 0
+ maximum: 15
+
+ diff-channels:
+ description: |
+ Besides the analog inputs available, internal inputs can be used.
+ 16: Internal temperature sensor.
+ 17: AVSS
+ 18: Internal reference
+ 19: DGND
+ 20: (AVDD − AVSS)/6+
+ 21: (AVDD − AVSS)/6-
+ 22: (IOVDD − DGND)/6+
+ 23: (IOVDD − DGND)/6-
+ 24: (ALDO − AVSS)/6+
+ 25: (ALDO − AVSS)/6-
+ 26: (DLDO − DGND)/6+
+ 27: (DLDO − DGND)/6-
+ 28: V_MV_P
+ 29: V_MV_M
+ items:
+ minimum: 0
+ maximum: 29
+
+ adi,reference-select:
+ description: |
+ Select the reference source to use when converting on the
+ specific channel. Valid values are:
+ 0: REFIN1(+)/REFIN1(−)
+ 1: REFIN2(+)/REFIN2(−)
+ 2: REFOUT/AVSS (Internal reference)
+ 3: AVDD/AVSS
+ If not specified, REFIN1 is used.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2, 3]
+ default: 0
+
+ adi,excitation-pin-0:
+ description: |
+ Analog input to apply excitation current to while the channel
+ is active.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 15
+ default: 0
+
+ adi,excitation-pin-1:
+ description: |
+ Analog input to apply excitation current to while this channel
+ is active.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 15
+ default: 0
+
+ adi,excitation-current-0-nanoamps:
+ description: |
+ Excitation current in nanoamps to be applied to pin specified in
+ adi,excitation-pin-0 while this channel is active.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 100, 10000, 20000, 50000, 100000, 150000, 200000]
+ default: 0
+
+ adi,excitation-current-1-nanoamps:
+ description: |
+ Excitation current in nanoamps to be applied to pin specified in
+ adi,excitation-pin-1 while this channel is active.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 100, 10000, 20000, 50000, 100000, 150000, 200000]
+ default: 0
+
+ adi,burnout-current-nanoamps:
+ description: |
+ Burnout current in nanoamps to be applied for this channel.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 500, 2000, 4000]
+ default: 0
+
+ adi,buffered-positive:
+ description: Enable buffered mode for positive input.
+ type: boolean
+
+ adi,buffered-negative:
+ description: Enable buffered mode for negative input.
+ type: boolean
+
+ required:
+ - reg
+ - diff-channels
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,ad4130-8-24-wlcsp";
+ reg = <0>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ spi-max-frequency = <5000000>;
+ interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio>;
+
+ channel@0 {
+ reg = <0>;
+
+ adi,reference-select = <2>;
+
+ /* AIN8, AIN9 */
+ diff-channels = <8 9>;
+ };
+
+ channel@1 {
+ reg = <1>;
+
+ adi,reference-select = <2>;
+
+ /* AIN10, AIN11 */
+ diff-channels = <10 11>;
+ };
+
+ channel@2 {
+ reg = <2>;
+
+ adi,reference-select = <2>;
+
+ /* Temperature Sensor, DGND */
+ diff-channels = <16 19>;
+ };
+
+ channel@3 {
+ reg = <3>;
+
+ adi,reference-select = <2>;
+
+ /* Internal reference, DGND */
+ diff-channels = <18 19>;
+ };
+
+ channel@4 {
+ reg = <4>;
+
+ adi,reference-select = <2>;
+
+ /* DGND, DGND */
+ diff-channels = <19 19>;
+ };
+ };
+ };
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver
2022-04-19 15:08 [PATCH v3 0/2] AD4130 Cosmin Tanislav
2022-04-19 15:08 ` [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130 Cosmin Tanislav
@ 2022-04-19 15:08 ` Cosmin Tanislav
2022-05-01 16:08 ` Jonathan Cameron
1 sibling, 1 reply; 15+ messages in thread
From: Cosmin Tanislav @ 2022-04-19 15:08 UTC (permalink / raw)
Cc: Jonathan Cameron, Rob Herring, Linus Walleij, linux-iio,
linux-gpio, linux-kernel, devicetree, Cosmin Tanislav
AD4130-8 is an ultra-low power, high precision, measurement solution for
low bandwidth battery operated applications.
The fully integrated AFE (Analog Front-End) includes a multiplexer for up
to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
selectable filter options, smart sequencer, sensor biasing and excitation
options, diagnostics, and a FIFO buffer.
Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
.../ABI/testing/sysfs-bus-iio-adc-ad4130 | 36 +
MAINTAINERS | 8 +
drivers/iio/adc/Kconfig | 13 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4130.c | 2078 +++++++++++++++++
5 files changed, 2136 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
create mode 100644 drivers/iio/adc/ad4130.c
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
new file mode 100644
index 000000000000..09857dbcac32
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
@@ -0,0 +1,36 @@
+What: /sys/bus/iio/devices/iio:deviceX/in_voltage-voltage_filter_mode_available
+KernelVersion: 5.18
+Contact: linux-iio@vger.kernel.org
+Description:
+ Reading returns a list with the possible filter modes.
+ "sinc4" - Sinc 4. Excellent noise performance. Long 1st
+ conversion time. No natural 50/60Hz rejection.
+ "sinc4+sinc1" - Sinc4 + averaging by 8. Low 1st conversion time.
+ "sinc3" - Sinc3. Moderate 1st conversion time. Good noise
+ performance.
+ "sinc3+rej60" - Sinc3 + 60Hz rejection. At a sampling frequency
+ of 50Hz, achieves simultaneous 50Hz and 60Hz
+ rejection.
+ "sinc3+sinc1" - Sinc3 + averaging by 8. Low 1st conversion time.
+ Best used with a sampling frequency of at least
+ 216.19Hz.
+ "sinc3+pf1" - Sinc3 + Post Filter 1.
+ 53dB rejection @ 50Hz, 58dB rejection @ 60Hz.
+ "sinc3+pf2" - Sinc3 + Post Filter 2.
+ 70dB rejection @ 50Hz, 70dB rejection @ 60Hz.
+ "sinc3+pf3" - Sinc3 + Post Filter 3.
+ 99dB rejection @ 50Hz, 103dB rejection @ 60Hz.
+ "sinc3+pf4" - Sinc3 + Post Filter 4.
+ 103dB rejection @ 50Hz, 109dB rejection @ 60Hz.
+
+What: /sys/bus/iio/devices/iio:deviceX/in_voltageY-voltageZ_filter_mode
+KernelVersion: 5.18
+Contact: linux-iio@vger.kernel.org
+Description:
+ Set the filter mode of the differential channel. When the filter
+ mode changes, the in_voltageY-voltageZ_sampling_frequency and
+ in_voltageY-voltageZ_sampling_frequency_available attributes
+ might also change to accommodate the new filter mode.
+ If the current sampling frequency is out of range for the new
+ filter mode, the sampling frequency will be changed to the
+ closest valid one.
diff --git a/MAINTAINERS b/MAINTAINERS
index b2840da5ce1f..a28dcbbeae34 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1074,6 +1074,14 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
T: git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
F: drivers/net/amt.c
+ANALOG DEVICES INC AD4130 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/adc/adi,ad4130.yaml
+F: drivers/iio/adc/ad4130.c
+
ANALOG DEVICES INC AD7192 DRIVER
M: Alexandru Tachici <alexandru.tachici@analog.com>
L: linux-iio@vger.kernel.org
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 48ace7412874..3599608afcf2 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -21,6 +21,19 @@ config AD_SIGMA_DELTA
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
+config AD4130
+ tristate "Analog Device AD4130 ADC Driver"
+ depends on SPI
+ select IIO_BUFFER
+ select IIO_KFIFO_BUF
+ select REGMAP_SPI
+ help
+ Say yes here to build support for Analog Devices AD4130-8 SPI analog
+ to digital converters (ADC).
+
+ To compile this driver as a module, choose M here: the module will be
+ called ad4130.
+
config AD7091R5
tristate "Analog Devices AD7091R5 ADC Driver"
depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 39d806f6d457..c3aa7e4eca2f 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_AD4130) += ad4130.o
obj-$(CONFIG_AD7091R5) += ad7091r5.o ad7091r-base.o
obj-$(CONFIG_AD7124) += ad7124.o
obj-$(CONFIG_AD7192) += ad7192.o
diff --git a/drivers/iio/adc/ad4130.c b/drivers/iio/adc/ad4130.c
new file mode 100644
index 000000000000..dd2f18734cba
--- /dev/null
+++ b/drivers/iio/adc/ad4130.c
@@ -0,0 +1,2078 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2022 Analog Devices, Inc.
+ * Author: Cosmin Tanislav <cosmin.tanislav@analog.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/units.h>
+
+#include <asm/div64.h>
+#include <asm/unaligned.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/iio/sysfs.h>
+
+#define AD4130_8_NAME "ad4130-8"
+
+#define AD4130_COMMS_READ_MASK BIT(6)
+
+#define AD4130_REG_STATUS 0x00
+#define AD4130_STATUS_POR_FLAG_MASK BIT(4)
+
+#define AD4130_REG_ADC_CONTROL 0x01
+#define AD4130_BIPOLAR_MASK BIT(14)
+#define AD4130_INT_REF_VAL_MASK BIT(13)
+#define AD4130_INT_REF_2_5V 2500000
+#define AD4130_INT_REF_1_25V 1250000
+#define AD4130_CSB_EN_MASK BIT(9)
+#define AD4130_INT_REF_EN_MASK BIT(8)
+#define AD4130_MODE_MASK GENMASK(5, 2)
+#define AD4130_MCLK_SEL_MASK GENMASK(1, 0)
+
+#define AD4130_REG_DATA 0x02
+
+#define AD4130_REG_IO_CONTROL 0x03
+#define AD4130_INT_PIN_SEL_MASK GENMASK(9, 8)
+#define AD4130_GPIO_DATA_MASK GENMASK(7, 4)
+#define AD4130_GPIO_CTRL_MASK GENMASK(3, 0)
+
+#define AD4130_REG_VBIAS 0x04
+
+#define AD4130_REG_ID 0x05
+
+#define AD4130_REG_ERROR 0x06
+
+#define AD4130_REG_ERROR_EN 0x07
+
+#define AD4130_REG_CHANNEL_X(x) (0x09 + (x))
+#define AD4130_CHANNEL_EN_MASK BIT(23)
+#define AD4130_SETUP_MASK GENMASK(22, 20)
+#define AD4130_AINP_MASK GENMASK(17, 13)
+#define AD4130_AINM_MASK GENMASK(12, 8)
+#define AD4130_IOUT1_MASK GENMASK(7, 4)
+#define AD4130_IOUT2_MASK GENMASK(3, 0)
+
+#define AD4130_REG_CONFIG_X(x) (0x19 + (x))
+#define AD4130_IOUT1_VAL_MASK GENMASK(15, 13)
+#define AD4130_IOUT2_VAL_MASK GENMASK(12, 10)
+#define AD4130_BURNOUT_MASK GENMASK(9, 8)
+#define AD4130_REF_BUFP_MASK BIT(7)
+#define AD4130_REF_BUFM_MASK BIT(6)
+#define AD4130_REF_SEL_MASK GENMASK(5, 4)
+#define AD4130_PGA_MASK GENMASK(3, 1)
+#define AD4130_PGA_NUM 8
+
+#define AD4130_REG_FILTER_X(x) (0x21 + (x))
+#define AD4130_FILTER_MODE_MASK GENMASK(15, 12)
+#define AD4130_FILTER_SELECT_MASK GENMASK(10, 0)
+#define AD4130_FS_MIN 1
+#define AD4130_MAX_ODR 2400
+
+#define AD4130_REG_FIFO_CONTROL 0x3a
+#define AD4130_ADD_FIFO_HEADER_MASK BIT(18)
+#define AD4130_FIFO_MODE_MASK GENMASK(17, 16)
+#define AD4130_WATERMARK_INT_EN_MASK BIT(9)
+#define AD4130_WATERMARK_MASK GENMASK(7, 0)
+#define AD4130_WATERMARK_256 0
+
+#define AD4130_REG_FIFO_STATUS 0x3b
+
+#define AD4130_REG_FIFO_DATA 0x3d
+#define AD4130_FIFO_SIZE 256
+#define AD4130_FIFO_MAX_SAMPLE_SIZE 3
+
+#define AD4130_MAX_GPIOS 4
+#define AD4130_MAX_SETUPS 8
+#define AD4130_MAX_CHANNELS 16
+#define AD4130_MAX_ANALOG_PINS 16
+#define AD4130_MAX_DIFF_INPUTS 30
+
+#define AD4130_AIN2_P1 0x2
+#define AD4130_AIN3_P2 0x3
+
+#define AD4130_MCLK_FREQ_76_8KHZ 76800
+#define AD4130_MCLK_FREQ_153_6KHZ 153600
+
+#define AD4130_RESET_BUF_SIZE 8
+
+#define AD4130_SOFT_RESET_SLEEP_US (160 * MICRO / AD4130_MCLK_FREQ_76_8KHZ)
+
+#define AD4130_INVALID_SLOT -1
+
+static const unsigned int ad4130_reg_size[] = {
+ [AD4130_REG_STATUS] = 1,
+ [AD4130_REG_ADC_CONTROL] = 2,
+ [AD4130_REG_IO_CONTROL] = 2,
+ [AD4130_REG_VBIAS] = 2,
+ [AD4130_REG_ID] = 1,
+ [AD4130_REG_ERROR] = 2,
+ [AD4130_REG_ERROR_EN] = 2,
+ [AD4130_REG_CHANNEL_X(0) ...
+ AD4130_REG_CHANNEL_X(AD4130_MAX_CHANNELS)] = 3,
+ [AD4130_REG_CONFIG_X(0) ...
+ AD4130_REG_CONFIG_X(AD4130_MAX_SETUPS)] = 2,
+ [AD4130_REG_FILTER_X(0) ...
+ AD4130_REG_FILTER_X(AD4130_MAX_SETUPS)] = 3,
+ [AD4130_REG_FIFO_CONTROL] = 3,
+ [AD4130_REG_FIFO_STATUS] = 1,
+};
+
+enum ad4130_id {
+ ID_AD4130_8_24_WLCSP,
+ ID_AD4130_8_24_LFCSP,
+ ID_AD4130_8_16_WLCSP,
+ ID_AD4130_8_16_LFCSP,
+};
+
+enum ad4130_int_ref_val {
+ AD4130_INT_REF_VAL_2_5V,
+ AD4130_INT_REF_VAL_1_25V,
+};
+
+enum ad4130_mclk_sel {
+ AD4130_MCLK_76_8KHZ,
+ AD4130_MCLK_76_8KHZ_OUT,
+ AD4130_MCLK_76_8KHZ_EXT,
+ AD4130_MCLK_153_6KHZ_EXT,
+ AD4130_MCLK_SEL_MAX
+};
+
+enum ad4130_int_pin_sel {
+ AD4130_INT_PIN_DOUT_OR_INT,
+ AD4130_INT_PIN_CLK,
+ AD4130_INT_PIN_P2,
+ AD4130_INT_PIN_DOUT,
+};
+
+enum ad4130_iout {
+ AD4130_IOUT_OFF,
+ AD4130_IOUT_10000NA,
+ AD4130_IOUT_20000NA,
+ AD4130_IOUT_50000NA,
+ AD4130_IOUT_100000NA,
+ AD4130_IOUT_150000NA,
+ AD4130_IOUT_200000NA,
+ AD4130_IOUT_100NA,
+ AD4130_IOUT_MAX,
+};
+
+enum ad4130_burnout {
+ AD4130_BURNOUT_OFF,
+ AD4130_BURNOUT_500NA,
+ AD4130_BURNOUT_2000NA,
+ AD4130_BURNOUT_4000NA,
+ AD4130_BURNOUT_MAX
+};
+
+enum ad4130_ref_sel {
+ AD4130_REF_REFIN1,
+ AD4130_REF_REFIN2,
+ AD4130_REF_REFOUT_AVSS,
+ AD4130_REF_AVDD_AVSS,
+ AD4130_REF_SEL_MAX
+};
+
+enum ad4130_fifo_mode {
+ AD4130_FIFO_MODE_DISABLED = 0b00,
+ AD4130_FIFO_MODE_WATERMARK = 0b01,
+};
+
+enum ad4130_mode {
+ AD4130_MODE_CONTINUOUS = 0b0000,
+ AD4130_MODE_IDLE = 0b0100,
+};
+
+enum ad4130_filter_mode {
+ AD4130_FILTER_SINC4,
+ AD4130_FILTER_SINC4_SINC1,
+ AD4130_FILTER_SINC3,
+ AD4130_FILTER_SINC3_REJ60,
+ AD4130_FILTER_SINC3_SINC1,
+ AD4130_FILTER_SINC3_PF1,
+ AD4130_FILTER_SINC3_PF2,
+ AD4130_FILTER_SINC3_PF3,
+ AD4130_FILTER_SINC3_PF4,
+};
+
+enum ad4130_pin_function {
+ AD4130_PIN_FN_NONE,
+ AD4130_PIN_FN_SPECIAL = BIT(0),
+ AD4130_PIN_FN_DIFF = BIT(1),
+ AD4130_PIN_FN_EXCITATION = BIT(2),
+ AD4130_PIN_FN_VBIAS = BIT(3),
+};
+
+struct ad4130_chip_info {
+ const char *name;
+ u8 resolution;
+ bool has_int_pin;
+};
+
+struct ad4130_setup_info {
+ unsigned int iout0_val;
+ unsigned int iout1_val;
+ unsigned int burnout;
+ unsigned int pga;
+ unsigned int fs;
+ bool ref_bufp;
+ bool ref_bufm;
+ u32 ref_sel;
+ enum ad4130_filter_mode filter_mode;
+};
+
+struct ad4130_slot_info {
+ struct ad4130_setup_info setup;
+ unsigned int enabled_channels;
+ unsigned int channels;
+};
+
+struct ad4130_chan_info {
+ struct ad4130_setup_info setup;
+ u32 iout0;
+ u32 iout1;
+ int slot;
+ bool enabled;
+ bool initialized;
+};
+
+struct ad4130_filter_config {
+ enum ad4130_filter_mode filter_mode;
+ unsigned int odr_div;
+ unsigned int fs_max;
+ unsigned int db3_div;
+ enum iio_available_type samp_freq_avail_type;
+ int samp_freq_avail_len;
+ int samp_freq_avail[3][2];
+ enum iio_available_type db3_freq_avail_type;
+ int db3_freq_avail_len;
+ int db3_freq_avail[3][2];
+};
+
+struct ad4130_state {
+ const struct ad4130_chip_info *chip_info;
+ struct spi_device *spi;
+ struct regmap *regmap;
+ struct clk *mclk;
+ struct regulator_bulk_data regulators[4];
+ u32 irq_trigger;
+ u32 inv_irq_trigger;
+
+ /*
+ * Synchronize access to members of driver state, and ensure atomicity
+ * of consecutive regmap operations.
+ */
+ struct mutex lock;
+ struct completion completion;
+
+ struct iio_chan_spec chans[AD4130_MAX_CHANNELS];
+ struct ad4130_chan_info chans_info[AD4130_MAX_CHANNELS];
+ struct ad4130_slot_info slots_info[AD4130_MAX_SETUPS];
+ enum ad4130_pin_function pins_fn[AD4130_MAX_ANALOG_PINS];
+ u32 vbias_pins[AD4130_MAX_ANALOG_PINS];
+ u32 num_vbias_pins;
+ int scale_tbls[AD4130_REF_SEL_MAX][AD4130_PGA_NUM][2];
+ struct gpio_chip gc;
+ unsigned int gpio_offsets[AD4130_MAX_GPIOS];
+ unsigned int num_gpios;
+
+ u32 int_pin_sel;
+ u32 int_ref_uv;
+ u32 mclk_sel;
+ bool int_ref_en;
+ bool bipolar;
+
+ unsigned int num_enabled_channels;
+ unsigned int effective_watermark;
+ unsigned int watermark;
+
+ struct spi_message fifo_msg;
+ struct spi_transfer fifo_xfer[2];
+
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ */
+ u8 reset_buf[AD4130_RESET_BUF_SIZE] ____cacheline_aligned;
+ u8 reg_write_tx_buf[4];
+ u8 reg_read_tx_buf[1];
+ u8 reg_read_rx_buf[3];
+ u8 fifo_tx_buf[2];
+ u8 fifo_rx_buf[AD4130_FIFO_SIZE *
+ AD4130_FIFO_MAX_SAMPLE_SIZE];
+};
+
+static const char * const ad4130_int_pin_names[] = {
+ [AD4130_INT_PIN_DOUT_OR_INT] = "dout-int",
+ [AD4130_INT_PIN_CLK] = "clk",
+ [AD4130_INT_PIN_P2] = "p2",
+ [AD4130_INT_PIN_DOUT] = "dout",
+};
+
+static const unsigned int ad4130_iout_current_na_tbl[AD4130_IOUT_MAX] = {
+ [AD4130_IOUT_OFF] = 0,
+ [AD4130_IOUT_100NA] = 100,
+ [AD4130_IOUT_10000NA] = 10000,
+ [AD4130_IOUT_20000NA] = 20000,
+ [AD4130_IOUT_50000NA] = 50000,
+ [AD4130_IOUT_100000NA] = 100000,
+ [AD4130_IOUT_150000NA] = 150000,
+ [AD4130_IOUT_200000NA] = 200000,
+};
+
+static const unsigned int ad4130_burnout_current_na_tbl[AD4130_BURNOUT_MAX] = {
+ [AD4130_BURNOUT_OFF] = 0,
+ [AD4130_BURNOUT_500NA] = 500,
+ [AD4130_BURNOUT_2000NA] = 2000,
+ [AD4130_BURNOUT_4000NA] = 4000,
+};
+
+#define AD4130_VARIABLE_ODR_CONFIG(_filter_mode, _odr_div, _fs_max, _db3_div) \
+{ \
+ .filter_mode = (_filter_mode), \
+ .odr_div = (_odr_div), \
+ .fs_max = (_fs_max), \
+ .db3_div = (_db3_div), \
+ .samp_freq_avail_type = IIO_AVAIL_RANGE, \
+ .samp_freq_avail = { \
+ { AD4130_MAX_ODR, (_odr_div) * (_fs_max) }, \
+ { AD4130_MAX_ODR, (_odr_div) * (_fs_max) }, \
+ { AD4130_MAX_ODR, (_odr_div) }, \
+ }, \
+ .db3_freq_avail_type = IIO_AVAIL_RANGE, \
+ .db3_freq_avail = { \
+ { \
+ AD4130_MAX_ODR * (_db3_div), \
+ (_odr_div) * (_fs_max) * MILLI, \
+ }, \
+ { \
+ AD4130_MAX_ODR * (_db3_div), \
+ (_odr_div) * (_fs_max) * MILLI, \
+ }, \
+ { \
+ AD4130_MAX_ODR * (_db3_div), \
+ (_odr_div) * MILLI, \
+ }, \
+ }, \
+}
+
+#define AD4130_FIXED_ODR_CONFIG(_filter_mode, _odr_div, _db3_div) \
+{ \
+ .filter_mode = (_filter_mode), \
+ .odr_div = (_odr_div), \
+ .fs_max = AD4130_FS_MIN, \
+ .db3_div = (_db3_div), \
+ .samp_freq_avail_type = IIO_AVAIL_LIST, \
+ .samp_freq_avail_len = 1, \
+ .samp_freq_avail = { \
+ { AD4130_MAX_ODR, (_odr_div) }, \
+ }, \
+ .db3_freq_avail_type = IIO_AVAIL_LIST, \
+ .db3_freq_avail_len = 1, \
+ .db3_freq_avail = { \
+ { \
+ AD4130_MAX_ODR * MILLI, \
+ (_odr_div) * (_db3_div), \
+ }, \
+ }, \
+}
+
+static const struct ad4130_filter_config ad4130_filter_configs[] = {
+ AD4130_VARIABLE_ODR_CONFIG(AD4130_FILTER_SINC4, 1, 10, 234),
+ AD4130_VARIABLE_ODR_CONFIG(AD4130_FILTER_SINC4_SINC1, 11, 10, 590),
+ AD4130_VARIABLE_ODR_CONFIG(AD4130_FILTER_SINC3, 1, 2047, 268),
+ AD4130_VARIABLE_ODR_CONFIG(AD4130_FILTER_SINC3_REJ60, 1, 2047, 268),
+ AD4130_VARIABLE_ODR_CONFIG(AD4130_FILTER_SINC3_SINC1, 10, 2047, 545),
+ AD4130_FIXED_ODR_CONFIG(AD4130_FILTER_SINC3_PF1, 92, 675),
+ AD4130_FIXED_ODR_CONFIG(AD4130_FILTER_SINC3_PF2, 100, 675),
+ AD4130_FIXED_ODR_CONFIG(AD4130_FILTER_SINC3_PF3, 124, 675),
+ AD4130_FIXED_ODR_CONFIG(AD4130_FILTER_SINC3_PF4, 148, 675),
+};
+
+static const char * const ad4130_filter_modes_str[] = {
+ [AD4130_FILTER_SINC4] = "sinc4",
+ [AD4130_FILTER_SINC4_SINC1] = "sinc4+sinc1",
+ [AD4130_FILTER_SINC3] = "sinc3",
+ [AD4130_FILTER_SINC3_REJ60] = "sinc3+rej60",
+ [AD4130_FILTER_SINC3_SINC1] = "sinc3+sinc1",
+ [AD4130_FILTER_SINC3_PF1] = "sinc3+pf1",
+ [AD4130_FILTER_SINC3_PF2] = "sinc3+pf2",
+ [AD4130_FILTER_SINC3_PF3] = "sinc3+pf3",
+ [AD4130_FILTER_SINC3_PF4] = "sinc3+pf4",
+};
+
+static unsigned int ad4130_data_reg_size(struct ad4130_state *st)
+{
+ return st->chip_info->resolution / 8;
+}
+
+static int ad4130_get_reg_size(struct ad4130_state *st, unsigned int reg,
+ unsigned int *size)
+{
+ if (reg >= ARRAY_SIZE(ad4130_reg_size))
+ return -EINVAL;
+
+ if (reg == AD4130_REG_DATA) {
+ *size = ad4130_data_reg_size(st);
+ return 0;
+ }
+
+ *size = ad4130_reg_size[reg];
+ if (!*size)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int ad4130_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+ struct ad4130_state *st = context;
+ unsigned int size;
+ int ret;
+
+ ret = ad4130_get_reg_size(st, reg, &size);
+ if (ret)
+ return ret;
+
+ st->reg_write_tx_buf[0] = reg;
+
+ switch (size) {
+ case 3:
+ put_unaligned_be24(val, &st->reg_write_tx_buf[1]);
+ break;
+ case 2:
+ put_unaligned_be16(val, &st->reg_write_tx_buf[1]);
+ break;
+ case 1:
+ st->reg_write_tx_buf[1] = val;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return spi_write(st->spi, st->reg_write_tx_buf, size + 1);
+}
+
+static int ad4130_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+ struct ad4130_state *st = context;
+ struct spi_transfer t[] = {
+ {
+ .tx_buf = st->reg_read_tx_buf,
+ .len = sizeof(st->reg_read_tx_buf),
+ },
+ {
+ .rx_buf = st->reg_read_rx_buf,
+ },
+ };
+ unsigned int size;
+ int ret;
+
+ ret = ad4130_get_reg_size(st, reg, &size);
+ if (ret)
+ return ret;
+
+ st->reg_read_tx_buf[0] = AD4130_COMMS_READ_MASK | reg;
+ t[1].len = size;
+
+ ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+ if (ret)
+ return ret;
+
+ switch (size) {
+ case 3:
+ *val = get_unaligned_be24(st->reg_read_rx_buf);
+ break;
+ case 2:
+ *val = get_unaligned_be16(st->reg_read_rx_buf);
+ break;
+ case 1:
+ *val = st->reg_read_rx_buf[0];
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return 0;
+}
+
+static const struct regmap_config ad4130_regmap_config = {
+ .reg_read = ad4130_reg_read,
+ .reg_write = ad4130_reg_write,
+};
+
+static int ad4130_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+ return GPIO_LINE_DIRECTION_OUT;
+}
+
+static void ad4130_gpio_set(struct gpio_chip *gc, unsigned int offset,
+ int value)
+{
+ struct ad4130_state *st = gpiochip_get_data(gc);
+ unsigned int real_offset = st->gpio_offsets[offset];
+ unsigned int mask = FIELD_PREP(AD4130_GPIO_DATA_MASK, BIT(real_offset));
+
+ regmap_update_bits(st->regmap, AD4130_REG_IO_CONTROL, mask,
+ value ? mask : 0);
+}
+
+static int ad4130_set_mode(struct ad4130_state *st, enum ad4130_mode mode)
+{
+ return regmap_update_bits(st->regmap, AD4130_REG_ADC_CONTROL,
+ AD4130_MODE_MASK,
+ FIELD_PREP(AD4130_MODE_MASK, mode));
+}
+
+static int ad4130_set_watermark_interrupt_en(struct ad4130_state *st, bool en)
+{
+ unsigned int mask = AD4130_WATERMARK_INT_EN_MASK;
+
+ return regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
+ mask, en ? mask : 0);
+}
+
+static unsigned int ad4130_watermark_reg_val(unsigned int val)
+{
+ if (val == AD4130_FIFO_SIZE)
+ val = AD4130_WATERMARK_256;
+
+ return val;
+}
+
+static int ad4130_set_fifo_mode(struct ad4130_state *st,
+ enum ad4130_fifo_mode mode)
+{
+ return regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
+ AD4130_FIFO_MODE_MASK,
+ FIELD_PREP(AD4130_FIFO_MODE_MASK, mode));
+}
+
+static void ad4130_push_fifo_data(struct iio_dev *indio_dev)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int data_reg_size = ad4130_data_reg_size(st);
+ unsigned int transfer_len = st->effective_watermark * data_reg_size;
+ unsigned int set_size = st->num_enabled_channels * data_reg_size;
+ unsigned int i;
+ int ret;
+
+ st->fifo_tx_buf[1] = ad4130_watermark_reg_val(st->effective_watermark);
+ st->fifo_xfer[1].len = transfer_len;
+
+ ret = spi_sync(st->spi, &st->fifo_msg);
+ if (ret)
+ return;
+
+ for (i = 0; i < transfer_len; i += set_size)
+ iio_push_to_buffers(indio_dev, &st->fifo_rx_buf[i]);
+}
+
+static irqreturn_t ad4130_irq_handler(int irq, void *private)
+{
+ struct iio_dev *indio_dev = private;
+ struct ad4130_state *st = iio_priv(indio_dev);
+
+ if (iio_buffer_enabled(indio_dev))
+ ad4130_push_fifo_data(indio_dev);
+ else
+ complete(&st->completion);
+
+ return IRQ_HANDLED;
+}
+
+static int ad4130_find_slot(struct ad4130_state *st,
+ struct ad4130_setup_info *target_setup_info,
+ unsigned int *slot, bool *overwrite)
+{
+ unsigned int i;
+
+ *slot = AD4130_INVALID_SLOT;
+ *overwrite = false;
+
+ for (i = 0; i < AD4130_MAX_SETUPS; i++) {
+ struct ad4130_slot_info *slot_info = &st->slots_info[i];
+
+ /* Immediately accept a matching setup info. */
+ if (!memcmp(target_setup_info, &slot_info->setup,
+ sizeof(*target_setup_info))) {
+ *slot = i;
+ return 0;
+ }
+
+ /* Ignore all setups which are used by enabled channels. */
+ if (slot_info->enabled_channels)
+ continue;
+
+ /* Find the least used slot. */
+ if (*slot == AD4130_INVALID_SLOT ||
+ slot_info->channels < st->slots_info[*slot].channels)
+ *slot = i;
+ }
+
+ if (*slot == AD4130_INVALID_SLOT)
+ return -EINVAL;
+
+ *overwrite = true;
+
+ return 0;
+}
+
+static void ad4130_unlink_channel(struct ad4130_state *st, unsigned int channel)
+{
+ struct ad4130_chan_info *chan_info = &st->chans_info[channel];
+ struct ad4130_slot_info *slot_info = &st->slots_info[chan_info->slot];
+
+ chan_info->slot = AD4130_INVALID_SLOT;
+ slot_info->channels--;
+}
+
+static int ad4130_unlink_slot(struct ad4130_state *st, unsigned int slot)
+{
+ unsigned int i;
+
+ for (i = 0; i < AD4130_MAX_CHANNELS; i++) {
+ struct ad4130_chan_info *chan_info = &st->chans_info[i];
+
+ if (!chan_info->initialized || chan_info->slot != slot)
+ continue;
+
+ ad4130_unlink_channel(st, i);
+ }
+
+ return 0;
+}
+
+static int ad4130_link_channel_slot(struct ad4130_state *st,
+ unsigned int channel, unsigned int slot)
+{
+ struct ad4130_slot_info *slot_info = &st->slots_info[slot];
+ struct ad4130_chan_info *chan_info = &st->chans_info[channel];
+ int ret;
+
+ ret = regmap_update_bits(st->regmap, AD4130_REG_CHANNEL_X(channel),
+ AD4130_SETUP_MASK,
+ FIELD_PREP(AD4130_SETUP_MASK, slot));
+ if (ret)
+ return ret;
+
+ chan_info->slot = slot;
+ slot_info->channels++;
+
+ return 0;
+}
+
+static int ad4130_write_slot_setup(struct ad4130_state *st,
+ unsigned int slot,
+ struct ad4130_setup_info *setup_info)
+{
+ unsigned int val;
+ int ret;
+
+ val = FIELD_PREP(AD4130_IOUT1_VAL_MASK, setup_info->iout0_val) |
+ FIELD_PREP(AD4130_IOUT1_VAL_MASK, setup_info->iout1_val) |
+ FIELD_PREP(AD4130_BURNOUT_MASK, setup_info->burnout) |
+ FIELD_PREP(AD4130_REF_BUFP_MASK, setup_info->ref_bufp) |
+ FIELD_PREP(AD4130_REF_BUFM_MASK, setup_info->ref_bufm) |
+ FIELD_PREP(AD4130_REF_SEL_MASK, setup_info->ref_sel) |
+ FIELD_PREP(AD4130_PGA_MASK, setup_info->pga);
+
+ ret = regmap_write(st->regmap, AD4130_REG_CONFIG_X(slot), val);
+ if (ret)
+ return ret;
+
+ val = FIELD_PREP(AD4130_FILTER_MODE_MASK, setup_info->filter_mode) |
+ FIELD_PREP(AD4130_FILTER_SELECT_MASK, setup_info->fs);
+
+ ret = regmap_write(st->regmap, AD4130_REG_FILTER_X(slot), val);
+ if (ret)
+ return ret;
+
+ memcpy(&st->slots_info[slot].setup, setup_info, sizeof(*setup_info));
+
+ return 0;
+}
+
+static int ad4130_write_channel_setup(struct ad4130_state *st,
+ unsigned int channel, bool on_enable)
+{
+ struct ad4130_chan_info *chan_info = &st->chans_info[channel];
+ struct ad4130_setup_info *setup_info = &chan_info->setup;
+ bool overwrite;
+ int slot;
+ int ret;
+
+ /*
+ * The following cases need to be handled.
+ *
+ * 1. Enabled and linked channel with setup changes:
+ * - Find slot. If not possible, return error.
+ * - Unlink channel from current slot.
+ * - If slot has channels linked to it, unlink all channels, and
+ * write the new setup to it.
+ * - Link channel to new slot.
+ *
+ * 2. Soon to be enabled and unlinked channel:
+ * - Find slot. If not possible, return error.
+ * - If slot has channels linked to it, unlink all channels, and
+ * write the new setup to it.
+ * - Link channel to slot.
+ *
+ * 3. Disabled and linked channel with setup changes:
+ * - Unlink channel from current slot.
+ *
+ * 4. Soon to be enabled and linked channel:
+ * 5. Disabled and unlinked channel with setup changes:
+ * - Do nothing.
+ */
+
+ /* Case 4 */
+ if (on_enable && chan_info->slot != AD4130_INVALID_SLOT)
+ return 0;
+
+ if (!on_enable && !chan_info->enabled) {
+ if (chan_info->slot != AD4130_INVALID_SLOT)
+ /* Case 3 */
+ ad4130_unlink_channel(st, channel);
+
+ /* Case 3 & 5 */
+ return 0;
+ }
+
+ /* Case 1 & 2 */
+ ret = ad4130_find_slot(st, setup_info, &slot, &overwrite);
+ if (ret)
+ return ret;
+
+ if (chan_info->slot != AD4130_INVALID_SLOT)
+ /* Case 1 only */
+ ad4130_unlink_channel(st, channel);
+
+ if (overwrite) {
+ ret = ad4130_unlink_slot(st, slot);
+ if (ret)
+ return ret;
+
+ ret = ad4130_write_slot_setup(st, slot, setup_info);
+ if (ret)
+ return ret;
+ }
+
+ return ad4130_link_channel_slot(st, channel, slot);
+}
+
+static int ad4130_set_channel_enable(struct ad4130_state *st,
+ unsigned int channel, bool status)
+{
+ struct ad4130_chan_info *chan_info = &st->chans_info[channel];
+ struct ad4130_slot_info *slot_info;
+ unsigned int mask = AD4130_CHANNEL_EN_MASK;
+ int ret;
+
+ if (chan_info->enabled == status)
+ return 0;
+
+ if (status) {
+ ret = ad4130_write_channel_setup(st, channel, true);
+ if (ret)
+ return ret;
+ }
+
+ slot_info = &st->slots_info[chan_info->slot];
+
+ ret = regmap_update_bits(st->regmap, AD4130_REG_CHANNEL_X(channel),
+ mask, status ? mask : 0);
+ if (ret)
+ return ret;
+
+ slot_info->enabled_channels += status ? 1 : -1;
+ chan_info->enabled = status;
+
+ return 0;
+}
+
+static void ad4130_freq_to_fs(enum ad4130_filter_mode filter_mode,
+ int val, int val2, unsigned int *fs, bool db3)
+{
+ const struct ad4130_filter_config *filter_config =
+ &ad4130_filter_configs[filter_mode];
+ u64 dividend, divisor;
+ int temp;
+
+ dividend = filter_config->fs_max * filter_config->odr_div *
+ ((u64)val * NANO + val2);
+ divisor = (u64)AD4130_MAX_ODR * NANO;
+
+ if (db3) {
+ dividend *= MILLI;
+ divisor *= filter_config->db3_div;
+ }
+
+ temp = AD4130_FS_MIN + filter_config->fs_max -
+ DIV64_U64_ROUND_CLOSEST(dividend, divisor);
+
+ if (temp < AD4130_FS_MIN)
+ temp = AD4130_FS_MIN;
+ else if (temp > filter_config->fs_max)
+ temp = filter_config->fs_max;
+
+ *fs = temp;
+}
+
+static void ad4130_fs_to_freq(enum ad4130_filter_mode filter_mode,
+ unsigned int fs, int *val, int *val2, bool db3)
+{
+ const struct ad4130_filter_config *filter_config =
+ &ad4130_filter_configs[filter_mode];
+ unsigned int dividend, divisor;
+ u64 temp;
+
+ dividend = (filter_config->fs_max - fs + AD4130_FS_MIN) *
+ AD4130_MAX_ODR;
+ divisor = filter_config->fs_max * filter_config->odr_div;
+
+ if (db3) {
+ dividend *= filter_config->db3_div;
+ divisor *= MILLI;
+ }
+
+ temp = div_u64((u64)dividend * NANO, divisor);
+ *val = div_u64_rem(temp, NANO, val2);
+}
+
+static int ad4130_set_filter_mode(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ unsigned int val)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int channel = chan->scan_index;
+ struct ad4130_chan_info *chan_info = &st->chans_info[channel];
+ struct ad4130_setup_info *setup_info = &chan_info->setup;
+ enum ad4130_filter_mode old_filter_mode;
+ int freq_val, freq_val2;
+ unsigned int old_fs;
+ int ret = 0;
+
+ mutex_lock(&st->lock);
+ if (setup_info->filter_mode == val)
+ goto out;
+
+ old_fs = setup_info->fs;
+ old_filter_mode = setup_info->filter_mode;
+
+ ad4130_fs_to_freq(setup_info->filter_mode, setup_info->fs,
+ &freq_val, &freq_val2, false);
+
+ ad4130_freq_to_fs(val, freq_val, freq_val2, &setup_info->fs, false);
+
+ setup_info->filter_mode = val;
+
+ ret = ad4130_write_channel_setup(st, channel, false);
+ if (ret) {
+ setup_info->fs = old_fs;
+ setup_info->filter_mode = old_filter_mode;
+ }
+
+ out:
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int ad4130_get_filter_mode(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int channel = chan->scan_index;
+ struct ad4130_setup_info *setup_info = &st->chans_info[channel].setup;
+ enum ad4130_filter_mode filter_mode;
+
+ mutex_lock(&st->lock);
+ filter_mode = setup_info->filter_mode;
+ mutex_unlock(&st->lock);
+
+ return filter_mode;
+}
+
+static const struct iio_enum ad4130_filter_mode_enum = {
+ .items = ad4130_filter_modes_str,
+ .num_items = ARRAY_SIZE(ad4130_filter_modes_str),
+ .set = ad4130_set_filter_mode,
+ .get = ad4130_get_filter_mode,
+};
+
+static const struct iio_chan_spec_ext_info ad4130_filter_mode_ext_info[] = {
+ IIO_ENUM("filter_mode", IIO_SEPARATE, &ad4130_filter_mode_enum),
+ IIO_ENUM_AVAILABLE("filter_mode", IIO_SHARED_BY_TYPE,
+ &ad4130_filter_mode_enum),
+ { }
+};
+
+static const struct iio_chan_spec ad4130_channel_template = {
+ .type = IIO_VOLTAGE,
+ .indexed = 1,
+ .differential = 1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
+ .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
+ .ext_info = ad4130_filter_mode_ext_info,
+ .scan_type = {
+ .sign = 'u',
+ .endianness = IIO_BE,
+ },
+};
+
+static int ad4130_set_channel_pga(struct ad4130_state *st, unsigned int channel,
+ int val, int val2)
+{
+ struct ad4130_chan_info *chan_info = &st->chans_info[channel];
+ struct ad4130_setup_info *setup_info = &chan_info->setup;
+ unsigned int pga, old_pga;
+ int ret = 0;
+
+ for (pga = 0; pga < AD4130_PGA_NUM; pga++)
+ if (val == st->scale_tbls[setup_info->ref_sel][pga][0] &&
+ val2 == st->scale_tbls[setup_info->ref_sel][pga][1])
+ break;
+
+ if (pga == AD4130_PGA_NUM)
+ return -EINVAL;
+
+ mutex_lock(&st->lock);
+ if (pga == setup_info->pga)
+ goto out;
+
+ old_pga = setup_info->pga;
+ setup_info->pga = pga;
+
+ ret = ad4130_write_channel_setup(st, channel, false);
+ if (ret)
+ setup_info->pga = old_pga;
+
+out:
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int ad4130_set_channel_freq(struct ad4130_state *st,
+ unsigned int channel,
+ int val, int val2, bool db3)
+{
+ struct ad4130_chan_info *chan_info = &st->chans_info[channel];
+ struct ad4130_setup_info *setup_info = &chan_info->setup;
+ unsigned int fs, old_fs;
+ int ret = 0;
+
+ mutex_lock(&st->lock);
+ old_fs = setup_info->fs;
+
+ ad4130_freq_to_fs(setup_info->filter_mode, val, val2, &fs, db3);
+
+ if (fs == setup_info->fs)
+ goto out;
+
+ setup_info->fs = fs;
+
+ ret = ad4130_write_channel_setup(st, channel, false);
+ if (ret)
+ setup_info->fs = old_fs;
+
+out:
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int _ad4130_read_sample(struct iio_dev *indio_dev, unsigned int channel,
+ int *val)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad4130_set_channel_enable(st, channel, true);
+ if (ret)
+ return ret;
+
+ reinit_completion(&st->completion);
+
+ ret = ad4130_set_mode(st, AD4130_MODE_CONTINUOUS);
+ if (ret)
+ return ret;
+
+ ret = wait_for_completion_timeout(&st->completion,
+ msecs_to_jiffies(1000));
+ if (!ret)
+ return -ETIMEDOUT;
+
+ ret = ad4130_set_mode(st, AD4130_MODE_IDLE);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(st->regmap, AD4130_REG_DATA, val);
+ if (ret)
+ return ret;
+
+ ret = ad4130_set_channel_enable(st, channel, false);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+}
+
+static int ad4130_read_sample(struct iio_dev *indio_dev, unsigned int channel,
+ int *val)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ mutex_lock(&st->lock);
+ ret = _ad4130_read_sample(indio_dev, channel, val);
+ mutex_unlock(&st->lock);
+
+ iio_device_release_direct_mode(indio_dev);
+
+ return ret;
+}
+
+static int ad4130_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long info)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int channel = chan->scan_index;
+ struct ad4130_setup_info *setup_info = &st->chans_info[channel].setup;
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ return ad4130_read_sample(indio_dev, channel, val);
+ case IIO_CHAN_INFO_SCALE:
+ mutex_lock(&st->lock);
+ *val = st->scale_tbls[setup_info->ref_sel][setup_info->pga][0];
+ *val2 = st->scale_tbls[setup_info->ref_sel][setup_info->pga][1];
+ mutex_unlock(&st->lock);
+
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_OFFSET:
+ *val = st->bipolar ? -(1 << (chan->scan_type.realbits - 1)) : 0;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ mutex_lock(&st->lock);
+ ad4130_fs_to_freq(setup_info->filter_mode, setup_info->fs,
+ val, val2, false);
+ mutex_unlock(&st->lock);
+
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ mutex_lock(&st->lock);
+ ad4130_fs_to_freq(setup_info->filter_mode, setup_info->fs,
+ val, val2, true);
+ mutex_unlock(&st->lock);
+
+ return IIO_VAL_INT_PLUS_NANO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad4130_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long info)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int channel = chan->scan_index;
+ struct ad4130_setup_info *setup_info = &st->chans_info[channel].setup;
+ const struct ad4130_filter_config *filter_config;
+
+ switch (info) {
+ case IIO_CHAN_INFO_SCALE:
+ *vals = (int *)st->scale_tbls[setup_info->ref_sel];
+ *length = ARRAY_SIZE(st->scale_tbls[setup_info->ref_sel]) * 2;
+
+ *type = IIO_VAL_INT_PLUS_NANO;
+
+ return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ mutex_lock(&st->lock);
+ filter_config = &ad4130_filter_configs[setup_info->filter_mode];
+ mutex_unlock(&st->lock);
+
+ *vals = (int *)filter_config->samp_freq_avail;
+ *length = filter_config->samp_freq_avail_len * 2;
+ *type = IIO_VAL_FRACTIONAL;
+
+ return filter_config->samp_freq_avail_type;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ mutex_lock(&st->lock);
+ filter_config = &ad4130_filter_configs[setup_info->filter_mode];
+ mutex_unlock(&st->lock);
+
+ *vals = (int *)filter_config->db3_freq_avail;
+ *length = filter_config->db3_freq_avail_len * 2;
+ *type = IIO_VAL_FRACTIONAL;
+
+ return filter_config->db3_freq_avail_type;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad4130_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long info)
+{
+ switch (info) {
+ case IIO_CHAN_INFO_SCALE:
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ return IIO_VAL_INT_PLUS_NANO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad4130_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long info)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int channel = chan->scan_index;
+
+ switch (info) {
+ case IIO_CHAN_INFO_SCALE:
+ return ad4130_set_channel_pga(st, channel, val, val2);
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return ad4130_set_channel_freq(st, channel, val, val2, false);
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ return ad4130_set_channel_freq(st, channel, val, val2, true);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad4130_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+ unsigned int writeval, unsigned int *readval)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+
+ if (readval)
+ return regmap_read(st->regmap, reg, readval);
+
+ return regmap_write(st->regmap, reg, writeval);
+}
+
+static int ad4130_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int channel;
+ unsigned int val = 0;
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ for_each_set_bit(channel, scan_mask, indio_dev->num_channels) {
+ ret = ad4130_set_channel_enable(st, channel, true);
+ if (ret)
+ goto out;
+
+ val++;
+ }
+
+ st->num_enabled_channels = val;
+
+out:
+ mutex_unlock(&st->lock);
+
+ return 0;
+}
+
+static int ad4130_set_fifo_watermark(struct iio_dev *indio_dev, unsigned int val)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int eff;
+ int ret;
+
+ if (val > AD4130_FIFO_SIZE)
+ return -EINVAL;
+
+ /*
+ * Always set watermark to a multiple of the number of enabled channels
+ * to avoid making the FIFO unaligned.
+ */
+ eff = rounddown(val, st->num_enabled_channels);
+
+ mutex_lock(&st->lock);
+
+ ret = regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
+ AD4130_WATERMARK_MASK,
+ FIELD_PREP(AD4130_WATERMARK_MASK,
+ ad4130_watermark_reg_val(eff)));
+ if (ret)
+ goto out;
+
+ st->effective_watermark = eff;
+ st->watermark = val;
+
+out:
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static const struct iio_info ad4130_info = {
+ .read_raw = ad4130_read_raw,
+ .read_avail = ad4130_read_avail,
+ .write_raw_get_fmt = ad4130_write_raw_get_fmt,
+ .write_raw = ad4130_write_raw,
+ .update_scan_mode = ad4130_update_scan_mode,
+ .hwfifo_set_watermark = ad4130_set_fifo_watermark,
+ .debugfs_reg_access = ad4130_reg_access,
+};
+
+static int ad4130_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ ret = ad4130_set_watermark_interrupt_en(st, true);
+ if (ret)
+ goto out;
+
+ /*
+ * When the chip enters FIFO mode, IRQ polarity is inversed.
+ * See datasheet pages: 65, FIFO Watermark Interrupt section,
+ * and 71, Bit Descriptions for STATUS Register, RDYB.
+ */
+ ret = irq_set_irq_type(st->spi->irq, st->inv_irq_trigger);
+ if (ret)
+ goto out;
+
+ ret = ad4130_set_fifo_mode(st, AD4130_FIFO_MODE_WATERMARK);
+ if (ret)
+ goto out;
+
+ ret = ad4130_set_mode(st, AD4130_MODE_CONTINUOUS);
+
+out:
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static int ad4130_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int i;
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ ret = ad4130_set_mode(st, AD4130_MODE_IDLE);
+ if (ret)
+ goto out;
+
+ /* When the chip exits FIFO mode, IRQ polarity returns to normal. */
+ ret = irq_set_irq_type(st->spi->irq, st->irq_trigger);
+ if (ret)
+ goto out;
+
+ ret = ad4130_set_fifo_mode(st, AD4130_FIFO_MODE_DISABLED);
+ if (ret)
+ goto out;
+
+ ret = ad4130_set_watermark_interrupt_en(st, false);
+ if (ret)
+ goto out;
+
+ /*
+ * update_scan_mode is not called in the disable path, disable all
+ * channels here.
+ */
+ for (i = 0; i < indio_dev->num_channels; i++) {
+ ret = ad4130_set_channel_enable(st, i, false);
+ if (ret)
+ goto out;
+ }
+
+out:
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
+static const struct iio_buffer_setup_ops ad4130_buffer_ops = {
+ .postenable = ad4130_buffer_postenable,
+ .predisable = ad4130_buffer_predisable,
+};
+
+static ssize_t ad4130_get_fifo_watermark(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ad4130_state *st = iio_priv(dev_to_iio_dev(dev));
+ unsigned int val;
+
+ mutex_lock(&st->lock);
+ val = st->watermark;
+ mutex_unlock(&st->lock);
+
+ return sysfs_emit(buf, "%d\n", val);
+}
+
+static ssize_t ad4130_get_fifo_enabled(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ad4130_state *st = iio_priv(dev_to_iio_dev(dev));
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(st->regmap, AD4130_REG_FIFO_CONTROL, &val);
+ if (ret)
+ return ret;
+
+ val = FIELD_GET(AD4130_FIFO_MODE_MASK, val);
+
+ return sysfs_emit(buf, "%d\n", val != AD4130_FIFO_MODE_DISABLED);
+}
+
+static IIO_CONST_ATTR(hwfifo_watermark_min, "1");
+static IIO_CONST_ATTR(hwfifo_watermark_max, __stringify(AD4130_FIFO_SIZE));
+static IIO_DEVICE_ATTR(hwfifo_watermark, 0444, ad4130_get_fifo_watermark, NULL, 0);
+static IIO_DEVICE_ATTR(hwfifo_enabled, 0444, ad4130_get_fifo_enabled, NULL, 0);
+
+static const struct attribute *ad4130_fifo_attributes[] = {
+ &iio_const_attr_hwfifo_watermark_min.dev_attr.attr,
+ &iio_const_attr_hwfifo_watermark_max.dev_attr.attr,
+ &iio_dev_attr_hwfifo_watermark.dev_attr.attr,
+ &iio_dev_attr_hwfifo_enabled.dev_attr.attr,
+ NULL
+};
+
+static int ad4130_find_table_index(const unsigned int *tbl, size_t len,
+ unsigned int val)
+{
+ unsigned int i;
+
+ for (i = 0; i < len; i++)
+ if (tbl[i] == val)
+ return i;
+
+ return -EINVAL;
+}
+
+static int ad4130_get_ref_voltage(struct ad4130_state *st,
+ enum ad4130_ref_sel ref_sel)
+{
+ struct device *dev = &st->spi->dev;
+ int ret;
+
+ switch (ref_sel) {
+ case AD4130_REF_REFIN1:
+ ret = regulator_get_voltage(st->regulators[2].consumer);
+ break;
+ case AD4130_REF_REFIN2:
+ ret = regulator_get_voltage(st->regulators[3].consumer);
+ break;
+ case AD4130_REF_AVDD_AVSS:
+ ret = regulator_get_voltage(st->regulators[0].consumer);
+ break;
+ case AD4130_REF_REFOUT_AVSS:
+ ret = st->int_ref_uv;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Cannot use reference %u\n",
+ ref_sel);
+
+ return ret;
+}
+
+static int ad4130_parse_fw_setup(struct ad4130_state *st,
+ struct fwnode_handle *child,
+ struct ad4130_setup_info *setup_info)
+{
+ struct device *dev = &st->spi->dev;
+ u32 current_na;
+ int ret;
+
+ current_na = 0;
+ fwnode_property_read_u32(child, "adi,excitation-current-0-nanoamps",
+ ¤t_na);
+ ret = ad4130_find_table_index(ad4130_iout_current_na_tbl,
+ AD4130_IOUT_MAX, current_na);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "Invalid excitation current %unA\n",
+ current_na);
+ setup_info->iout0_val = ret;
+
+ current_na = 0;
+ fwnode_property_read_u32(child, "adi,excitation-current-1-nanoamps",
+ ¤t_na);
+ ret = ad4130_find_table_index(ad4130_iout_current_na_tbl,
+ AD4130_IOUT_MAX, current_na);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "Invalid excitation current %unA\n",
+ current_na);
+ setup_info->iout1_val = ret;
+
+ current_na = 0;
+ fwnode_property_read_u32(child, "adi,burnout-current-nanoamps",
+ ¤t_na);
+ ret = ad4130_find_table_index(ad4130_burnout_current_na_tbl,
+ AD4130_BURNOUT_MAX, current_na);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "Invalid burnout current %unA\n",
+ current_na);
+ setup_info->burnout = ret;
+
+ setup_info->ref_bufp = fwnode_property_read_bool(child, "adi,buffered-positive");
+ setup_info->ref_bufm = fwnode_property_read_bool(child, "adi,buffered-negative");
+
+ setup_info->ref_sel = AD4130_REF_REFIN1;
+ fwnode_property_read_u32(child, "adi,reference-select",
+ &setup_info->ref_sel);
+ if (setup_info->ref_sel >= AD4130_REF_SEL_MAX)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid reference selected %u\n",
+ setup_info->ref_sel);
+
+ if (setup_info->ref_sel == AD4130_REF_REFOUT_AVSS)
+ st->int_ref_en = true;
+
+ ret = ad4130_get_ref_voltage(st, setup_info->ref_sel);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int ad4130_validate_diff_channel(struct ad4130_state *st, u32 pin)
+{
+ struct device *dev = &st->spi->dev;
+
+ if (pin >= AD4130_MAX_DIFF_INPUTS)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid diffreential channel %u\n", pin);
+
+ if (pin >= AD4130_MAX_ANALOG_PINS)
+ return 0;
+
+ if (st->pins_fn[pin] == AD4130_PIN_FN_SPECIAL)
+ return dev_err_probe(dev, -EINVAL,
+ "Pin %u already used with fn %u\n", pin,
+ st->pins_fn[pin]);
+
+ st->pins_fn[pin] |= AD4130_PIN_FN_DIFF;
+
+ return 0;
+}
+
+static int ad4130_validate_diff_channels(struct ad4130_state *st,
+ u32 *pins, unsigned int len)
+{
+ unsigned int i;
+ int ret = 0;
+
+ for (i = 0; i < len; i++) {
+ ret = ad4130_validate_diff_channel(st, pins[i]);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
+static int ad4130_validate_excitation_pin(struct ad4130_state *st, u32 pin)
+{
+ struct device *dev = &st->spi->dev;
+
+ if (pin >= AD4130_MAX_ANALOG_PINS)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid excitation pin %u\n", pin);
+
+ if (st->pins_fn[pin] == AD4130_PIN_FN_SPECIAL)
+ return dev_err_probe(dev, -EINVAL,
+ "Pin %u already used with fn %u\n", pin,
+ st->pins_fn[pin]);
+
+ st->pins_fn[pin] |= AD4130_PIN_FN_EXCITATION;
+
+ return 0;
+}
+
+static int ad4130_validate_vbias_pin(struct ad4130_state *st, u32 pin)
+{
+ struct device *dev = &st->spi->dev;
+
+ if (pin >= AD4130_MAX_ANALOG_PINS)
+ return dev_err_probe(dev, -EINVAL, "Invalid vbias pin %u\n",
+ pin);
+
+ if (st->pins_fn[pin] == AD4130_PIN_FN_SPECIAL)
+ return dev_err_probe(dev, -EINVAL,
+ "Pin %u already used with fn %u\n", pin,
+ st->pins_fn[pin]);
+
+ st->pins_fn[pin] |= AD4130_PIN_FN_VBIAS;
+
+ return 0;
+}
+
+static int ad4130_validate_vbias_pins(struct ad4130_state *st,
+ u32 *pins, unsigned int len)
+{
+ unsigned int i;
+ int ret = 0;
+
+ for (i = 0; i < st->num_vbias_pins; i++) {
+ ret = ad4130_validate_vbias_pin(st, pins[i]);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
+static int ad4130_parse_fw_channel(struct iio_dev *indio_dev,
+ struct fwnode_handle *child)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ unsigned int index = indio_dev->num_channels++;
+ struct device *dev = &st->spi->dev;
+ struct ad4130_chan_info *chan_info;
+ struct iio_chan_spec *chan;
+ u32 pins[2];
+ int ret;
+
+ if (index >= AD4130_MAX_CHANNELS)
+ return dev_err_probe(dev, -EINVAL, "Too many channels\n");
+
+ chan = &st->chans[index];
+ chan_info = &st->chans_info[index];
+
+ *chan = ad4130_channel_template;
+ chan->scan_type.realbits = st->chip_info->resolution;
+ chan->scan_type.storagebits = st->chip_info->resolution;
+ chan->scan_index = index;
+
+ chan_info->slot = AD4130_INVALID_SLOT;
+ chan_info->setup.fs = AD4130_FS_MIN;
+ chan_info->initialized = true;
+
+ ret = fwnode_property_read_u32_array(child, "diff-channels", pins,
+ ARRAY_SIZE(pins));
+ if (ret)
+ return ret;
+
+ ret = ad4130_validate_diff_channels(st, pins, ARRAY_SIZE(pins));
+ if (ret)
+ return ret;
+
+ chan->channel = pins[0];
+ chan->channel2 = pins[1];
+
+ ret = ad4130_parse_fw_setup(st, child, &chan_info->setup);
+ if (ret)
+ return ret;
+
+ fwnode_property_read_u32(child, "adi,excitation-pin-0",
+ &chan_info->iout0);
+ if (chan_info->setup.iout0_val != AD4130_IOUT_OFF) {
+ ret = ad4130_validate_excitation_pin(st, chan_info->iout0);
+ if (ret)
+ return ret;
+ }
+
+ fwnode_property_read_u32(child, "adi,excitation-pin-1",
+ &chan_info->iout1);
+ if (chan_info->setup.iout1_val != AD4130_IOUT_OFF) {
+ ret = ad4130_validate_excitation_pin(st, chan_info->iout1);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ad4130_parse_fw_children(struct iio_dev *indio_dev)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ struct device *dev = &st->spi->dev;
+ struct fwnode_handle *child;
+ int ret;
+
+ indio_dev->channels = st->chans;
+
+ device_for_each_child_node(dev, child) {
+ ret = ad4130_parse_fw_channel(indio_dev, child);
+ if (ret) {
+ fwnode_handle_put(child);
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static int ad4310_parse_fw(struct iio_dev *indio_dev)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ struct device *dev = &st->spi->dev;
+ u32 ext_clk_freq = AD4130_MCLK_FREQ_76_8KHZ;
+ bool int_clk_out = false;
+ unsigned int i;
+ int avdd_uv;
+ int irq;
+ int ret;
+
+ st->mclk = devm_clk_get_optional(dev, "mclk");
+ if (IS_ERR(st->mclk))
+ return dev_err_probe(dev, PTR_ERR(st->mclk),
+ "Failed to get mclk\n");
+
+ st->int_pin_sel = AD4130_INT_PIN_DOUT_OR_INT;
+
+ for (i = 0; i < ARRAY_SIZE(ad4130_int_pin_names); i++) {
+ irq = fwnode_irq_get_byname(dev_fwnode(dev),
+ ad4130_int_pin_names[i]);
+ if (irq > 0) {
+ st->int_pin_sel = i;
+ break;
+ }
+ }
+
+ if (st->int_pin_sel == AD4130_INT_PIN_DOUT ||
+ (st->int_pin_sel == AD4130_INT_PIN_DOUT_OR_INT &&
+ !st->chip_info->has_int_pin))
+ return dev_err_probe(dev, -EINVAL,
+ "Cannot use DOUT as interrupt pin\n");
+
+ if (st->int_pin_sel == AD4130_INT_PIN_P2)
+ st->pins_fn[AD4130_AIN3_P2] = AD4130_PIN_FN_SPECIAL;
+
+ int_clk_out = device_property_read_bool(dev, "adi,int-clk-out");
+ if (st->mclk && int_clk_out)
+ return dev_err_probe(dev, -EINVAL,
+ "Cannot expose internal clock\n");
+
+ device_property_read_u32(dev, "adi,ext-clk-freq", &ext_clk_freq);
+ if (ext_clk_freq != AD4130_MCLK_FREQ_153_6KHZ &&
+ ext_clk_freq != AD4130_MCLK_FREQ_76_8KHZ)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid external clock frequency %u\n",
+ ext_clk_freq);
+
+ if (st->mclk && ext_clk_freq == AD4130_MCLK_FREQ_153_6KHZ)
+ st->mclk_sel = AD4130_MCLK_153_6KHZ_EXT;
+ else if (st->mclk)
+ st->mclk_sel = AD4130_MCLK_76_8KHZ_EXT;
+ else if (int_clk_out)
+ st->mclk_sel = AD4130_MCLK_76_8KHZ_OUT;
+ else
+ st->mclk_sel = AD4130_MCLK_76_8KHZ;
+
+ if (st->int_pin_sel == AD4130_INT_PIN_CLK &&
+ st->mclk_sel != AD4130_MCLK_76_8KHZ)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid clock %u for interrupt pin %u\n",
+ st->mclk_sel, st->int_pin_sel);
+
+ st->int_ref_uv = AD4130_INT_REF_2_5V;
+
+ /*
+ * When the AVDD supply is set to below 2.5V the internal reference of
+ * 1.25V should be selected.
+ * See datasheet page 37, section ADC REFERENCE.
+ */
+ avdd_uv = regulator_get_voltage(st->regulators[0].consumer);
+ if (avdd_uv > 0 && avdd_uv < AD4130_INT_REF_2_5V)
+ st->int_ref_uv = AD4130_INT_REF_1_25V;
+
+ st->bipolar = device_property_read_bool(dev, "adi,bipolar");
+
+ ret = device_property_count_u32(dev, "adi,vbias-pins");
+ if (ret > 0) {
+ if (ret > AD4130_MAX_ANALOG_PINS)
+ return dev_err_probe(dev, -EINVAL,
+ "Too many vbias pins %u\n", ret);
+
+ st->num_vbias_pins = ret;
+
+ ret = device_property_read_u32_array(dev, "adi,vbias-pins",
+ st->vbias_pins,
+ st->num_vbias_pins);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to read vbias pins\n");
+
+ ret = ad4130_validate_vbias_pins(st, st->vbias_pins,
+ st->num_vbias_pins);
+ if (ret)
+ return ret;
+ }
+
+ ret = ad4130_parse_fw_children(indio_dev);
+ if (ret)
+ return ret;
+
+ /*
+ * Output-only GPIO functionality is available on pins AIN2 through
+ * AIN5. If these pins are used for anything else, do not expose them.
+ */
+ for (i = 0; i < AD4130_MAX_GPIOS; i++) {
+ if (st->pins_fn[i + AD4130_AIN2_P1] != AD4130_PIN_FN_NONE)
+ continue;
+
+ st->gpio_offsets[st->num_gpios++] = i;
+ }
+
+ return 0;
+}
+
+static void ad4130_fill_scale_tbls(struct ad4130_state *st)
+{
+ unsigned int i, j;
+
+ for (i = 0; i < AD4130_REF_SEL_MAX; i++) {
+ int ret;
+
+ ret = ad4130_get_ref_voltage(st, i);
+ if (ret < 0)
+ continue;
+
+ for (j = 0; j < AD4130_PGA_NUM; j++) {
+ unsigned int pow = st->chip_info->resolution + j -
+ st->bipolar;
+ unsigned int nv = div_u64((((u64)ret * NANO) >>
+ pow), MILLI);
+
+ st->scale_tbls[i][j][0] = 0;
+ st->scale_tbls[i][j][1] = nv;
+ }
+ }
+}
+
+static void ad4130_clk_disable_unprepare(void *clk)
+{
+ clk_disable_unprepare(clk);
+}
+
+static int ad4130_setup(struct iio_dev *indio_dev)
+{
+ struct ad4130_state *st = iio_priv(indio_dev);
+ struct device *dev = &st->spi->dev;
+ unsigned int int_ref_val;
+ unsigned long rate = AD4130_MCLK_FREQ_76_8KHZ;
+ unsigned int val;
+ unsigned int i;
+ int ret;
+
+ if (st->mclk_sel == AD4130_MCLK_153_6KHZ_EXT)
+ rate = AD4130_MCLK_FREQ_153_6KHZ;
+
+ ret = clk_set_rate(st->mclk, rate);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(st->mclk);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(dev, ad4130_clk_disable_unprepare,
+ st->mclk);
+ if (ret)
+ return ret;
+
+ if (st->int_ref_uv == AD4130_INT_REF_2_5V)
+ int_ref_val = AD4130_INT_REF_VAL_2_5V;
+ else
+ int_ref_val = AD4130_INT_REF_VAL_1_25V;
+
+ /* Switch to SPI 4-wire mode. */
+ val = AD4130_CSB_EN_MASK;
+ val |= FIELD_PREP(AD4130_BIPOLAR_MASK, st->bipolar);
+ val |= FIELD_PREP(AD4130_INT_REF_EN_MASK, st->int_ref_en);
+ val |= FIELD_PREP(AD4130_MODE_MASK, AD4130_MODE_IDLE);
+ val |= FIELD_PREP(AD4130_MCLK_SEL_MASK, st->mclk_sel);
+ val |= FIELD_PREP(AD4130_INT_REF_VAL_MASK, int_ref_val);
+
+ ret = regmap_write(st->regmap, AD4130_REG_ADC_CONTROL, val);
+ if (ret)
+ return ret;
+
+ val = FIELD_PREP(AD4130_INT_PIN_SEL_MASK, st->int_pin_sel);
+ for (i = 0; i < st->num_gpios; i++)
+ val |= BIT(st->gpio_offsets[i]);
+
+ ret = regmap_write(st->regmap, AD4130_REG_IO_CONTROL, val);
+ if (ret)
+ return ret;
+
+ val = 0;
+ for (i = 0; i < st->num_vbias_pins; i++)
+ val |= BIT(st->vbias_pins[i]);
+
+ ret = regmap_write(st->regmap, AD4130_REG_VBIAS, val);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
+ AD4130_ADD_FIFO_HEADER_MASK, 0);
+ if (ret)
+ return ret;
+
+ /* FIFO watermark interrupt starts out as enabled, disable it. */
+ ret = ad4130_set_watermark_interrupt_en(st, false);
+ if (ret)
+ return ret;
+
+ /* Setup channels. */
+ for (i = 0; i < indio_dev->num_channels; i++) {
+ struct ad4130_chan_info *chan_info = &st->chans_info[i];
+ struct iio_chan_spec *chan = &st->chans[i];
+ unsigned int val;
+
+ val = FIELD_PREP(AD4130_AINP_MASK, chan->channel) |
+ FIELD_PREP(AD4130_AINM_MASK, chan->channel2) |
+ FIELD_PREP(AD4130_IOUT1_MASK, chan_info->iout0) |
+ FIELD_PREP(AD4130_IOUT2_MASK, chan_info->iout1);
+
+ ret = regmap_write(st->regmap, AD4130_REG_CHANNEL_X(i), val);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ad4130_soft_reset(struct ad4130_state *st)
+{
+ int ret;
+
+ ret = spi_write(st->spi, st->reset_buf, sizeof(st->reset_buf));
+ if (ret)
+ return ret;
+
+ fsleep(AD4130_SOFT_RESET_SLEEP_US);
+
+ return 0;
+}
+
+static void ad4130_disable_regulators(void *data)
+{
+ struct ad4130_state *st = data;
+
+ regulator_bulk_disable(ARRAY_SIZE(st->regulators), st->regulators);
+}
+
+static int ad4130_probe(struct spi_device *spi)
+{
+ const struct ad4130_chip_info *info;
+ struct device *dev = &spi->dev;
+ struct iio_dev *indio_dev;
+ struct ad4130_state *st;
+ int ret;
+
+ info = device_get_match_data(dev);
+ if (!info)
+ return -ENODEV;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+
+ memset(st->reset_buf, 0xff, sizeof(st->reset_buf));
+ init_completion(&st->completion);
+ mutex_init(&st->lock);
+ st->chip_info = info;
+ st->spi = spi;
+
+ /*
+ * Xfer: [ XFR1 ] [ XFR2 ]
+ * Master: 0x7D N ......................
+ * Slave: ...... DATA1 DATA2 ... DATAN
+ */
+ st->fifo_tx_buf[0] = AD4130_COMMS_READ_MASK | AD4130_REG_FIFO_DATA;
+ st->fifo_xfer[0].tx_buf = st->fifo_tx_buf;
+ st->fifo_xfer[0].len = sizeof(st->fifo_tx_buf);
+ st->fifo_xfer[1].rx_buf = st->fifo_rx_buf;
+ spi_message_init_with_transfers(&st->fifo_msg, st->fifo_xfer,
+ ARRAY_SIZE(st->fifo_xfer));
+
+ indio_dev->name = st->chip_info->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &ad4130_info;
+
+ st->regmap = devm_regmap_init(dev, NULL, st, &ad4130_regmap_config);
+ if (IS_ERR(st->regmap))
+ return PTR_ERR(st->regmap);
+
+ st->regulators[0].supply = "avdd";
+ st->regulators[1].supply = "iovdd";
+ st->regulators[2].supply = "refin1";
+ st->regulators[3].supply = "refin2";
+
+ ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(st->regulators),
+ st->regulators);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to get regulators\n");
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(st->regulators), st->regulators);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to enable regulators\n");
+
+ ret = devm_add_action_or_reset(dev, ad4130_disable_regulators, st);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to add regulators disable action\n");
+
+ ret = ad4130_soft_reset(st);
+ if (ret)
+ return ret;
+
+ ret = ad4310_parse_fw(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = ad4130_setup(indio_dev);
+ if (ret)
+ return ret;
+
+ ad4130_fill_scale_tbls(st);
+
+ if (st->num_gpios) {
+ st->gc.owner = THIS_MODULE;
+ st->gc.label = st->chip_info->name;
+ st->gc.base = -1;
+ st->gc.ngpio = AD4130_MAX_GPIOS;
+ st->gc.parent = dev;
+ st->gc.can_sleep = true;
+ st->gc.get_direction = ad4130_gpio_get_direction;
+ st->gc.set = ad4130_gpio_set;
+
+ ret = devm_gpiochip_add_data(dev, &st->gc, st);
+ if (ret)
+ return ret;
+ }
+
+ ret = devm_iio_kfifo_buffer_setup_ext(dev, indio_dev,
+ &ad4130_buffer_ops,
+ ad4130_fifo_attributes);
+ if (ret)
+ return ret;
+
+ ret = devm_request_threaded_irq(dev, spi->irq, NULL,
+ ad4130_irq_handler, IRQF_ONESHOT,
+ indio_dev->name, indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to request irq\n");
+
+ st->irq_trigger = irq_get_trigger_type(spi->irq);
+ if (st->irq_trigger & IRQF_TRIGGER_RISING)
+ st->inv_irq_trigger = IRQF_TRIGGER_FALLING;
+ else if (st->irq_trigger & IRQF_TRIGGER_FALLING)
+ st->inv_irq_trigger = IRQF_TRIGGER_RISING;
+ else
+ return dev_err_probe(dev, -EINVAL, "Invalid irq flags: %u\n",
+ st->irq_trigger);
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct ad4130_chip_info ad4130_chip_info_tbl[] = {
+ [ID_AD4130_8_24_WLCSP] = {
+ .name = AD4130_8_NAME,
+ .resolution = 24,
+ .has_int_pin = true,
+ },
+ [ID_AD4130_8_24_LFCSP] = {
+ .name = AD4130_8_NAME,
+ .resolution = 24,
+ },
+ [ID_AD4130_8_16_WLCSP] = {
+ .name = AD4130_8_NAME,
+ .resolution = 16,
+ .has_int_pin = true,
+ },
+ [ID_AD4130_8_16_LFCSP] = {
+ .name = AD4130_8_NAME,
+ .resolution = 16,
+ },
+};
+
+static const struct of_device_id ad4130_of_match[] = {
+ {
+ .compatible = "adi,ad4130-8-16-lfcsp",
+ .data = &ad4130_chip_info_tbl[ID_AD4130_8_16_LFCSP],
+ },
+ {
+ .compatible = "adi,ad4130-8-16-wlcsp",
+ .data = &ad4130_chip_info_tbl[ID_AD4130_8_16_WLCSP],
+ },
+ {
+ .compatible = "adi,ad4130-8-24-lfcsp",
+ .data = &ad4130_chip_info_tbl[ID_AD4130_8_24_LFCSP],
+ },
+ {
+ .compatible = "adi,ad4130-8-24-wlcsp",
+ .data = &ad4130_chip_info_tbl[ID_AD4130_8_24_WLCSP],
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ad4130_of_match);
+
+static struct spi_driver ad4130_driver = {
+ .driver = {
+ .name = "ad4130",
+ .of_match_table = ad4130_of_match,
+ },
+ .probe = ad4130_probe,
+};
+module_spi_driver(ad4130_driver);
+
+MODULE_AUTHOR("Cosmin Tanislav <cosmin.tanislav@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD4130 SPI driver");
+MODULE_LICENSE("GPL");
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130
2022-04-19 15:08 ` [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130 Cosmin Tanislav
@ 2022-04-26 0:37 ` Rob Herring
2022-04-27 12:47 ` Cosmin Tanislav
2022-06-08 8:31 ` Cosmin Tanislav
2022-05-01 16:09 ` Jonathan Cameron
1 sibling, 2 replies; 15+ messages in thread
From: Rob Herring @ 2022-04-26 0:37 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Jonathan Cameron, Linus Walleij, linux-iio, linux-gpio,
linux-kernel, devicetree, Cosmin Tanislav
On Tue, Apr 19, 2022 at 06:08:27PM +0300, Cosmin Tanislav wrote:
> AD4130-8 is an ultra-low power, high precision, measurement solution for
> low bandwidth battery operated applications.
>
> The fully integrated AFE (Analog Front-End) includes a multiplexer for up
> to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
> Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
> selectable filter options, smart sequencer, sensor biasing and excitation
> options, diagnostics, and a FIFO buffer.
>
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> ---
> .../bindings/iio/adc/adi,ad4130.yaml | 264 ++++++++++++++++++
> 1 file changed, 264 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
> new file mode 100644
> index 000000000000..32996b62cd20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
> @@ -0,0 +1,264 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2022 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4130.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD4130 ADC device driver
> +
> +maintainers:
> + - Cosmin Tanislav <cosmin.tanislav@analog.com>
> +
> +description: |
> + Bindings for the Analog Devices AD4130 ADC. Datasheet can be found here:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4130-8.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad4130-8-16-lfcsp
> + - adi,ad4130-8-16-wlcsp
> + - adi,ad4130-8-24-lfcsp
> + - adi,ad4130-8-24-wlcsp
What is lfcsp? wlcsp seems to be the package type which generally
shouldn't be part of the compatible.
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> + description: phandle to the master clock (mclk)
> +
> + clock-names:
> + items:
> + - const: mclk
> +
> + interrupts:
> + maxItems: 1
> +
> + interrupt-names:
> + description: |
Don't need '|' if there is no formatting to preserve.
> + Specify which interrupt pin should be configured as Data Ready / FIFO
> + interrupt.
> + Default if not supplied is dout-int.
default: dout-int
> + enum:
> + - dout-int
> + - clk
> + - p2
> + - dout
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + refin1-supply:
> + description: refin1 supply. Can be used as reference for conversion.
> +
> + refin2-supply:
> + description: refin2 supply. Can be used as reference for conversion.
> +
> + avdd-supply:
> + description: AVDD voltage supply. Can be used as reference for conversion.
> +
> + iovdd-supply:
> + description: IOVDD voltage supply. Used for the chip interface.
> +
> + spi-max-frequency:
> + maximum: 5000000
> +
> + adi,int-clk-out:
> + description: Specify if the internal clock should be exposed on the CLK pin.
> + type: boolean
> +
> + adi,ext-clk-freq:
> + description: Specify the frequency of the external clock.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [76800, 153600]
> + default: 76800
> +
> + adi,bipolar:
> + description: Specify if the device should be used in bipolar mode.
> + type: boolean
> +
> + adi,vbias-pins:
> + description: Analog inputs to apply a voltage bias of (AVDD − AVSS) / 2 to.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + maxItems: 16
> + items:
> + minimum: 0
> + maximum: 15
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +patternProperties:
> + "^channel@([0-9]|1[0-5])$":
> + type: object
> + $ref: adc.yaml
unevaluatedProperties: false
> +
> + properties:
> + reg:
> + description: |
> + The channel number.
> + items:
> + minimum: 0
> + maximum: 15
> +
> + diff-channels:
> + description: |
> + Besides the analog inputs available, internal inputs can be used.
> + 16: Internal temperature sensor.
> + 17: AVSS
> + 18: Internal reference
> + 19: DGND
> + 20: (AVDD − AVSS)/6+
> + 21: (AVDD − AVSS)/6-
> + 22: (IOVDD − DGND)/6+
> + 23: (IOVDD − DGND)/6-
> + 24: (ALDO − AVSS)/6+
> + 25: (ALDO − AVSS)/6-
> + 26: (DLDO − DGND)/6+
> + 27: (DLDO − DGND)/6-
> + 28: V_MV_P
> + 29: V_MV_M
> + items:
> + minimum: 0
> + maximum: 29
> +
> + adi,reference-select:
> + description: |
> + Select the reference source to use when converting on the
> + specific channel. Valid values are:
> + 0: REFIN1(+)/REFIN1(−)
> + 1: REFIN2(+)/REFIN2(−)
> + 2: REFOUT/AVSS (Internal reference)
> + 3: AVDD/AVSS
> + If not specified, REFIN1 is used.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 3]
> + default: 0
> +
> + adi,excitation-pin-0:
> + description: |
> + Analog input to apply excitation current to while the channel
> + is active.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 15
> + default: 0
> +
> + adi,excitation-pin-1:
> + description: |
> + Analog input to apply excitation current to while this channel
> + is active.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 15
> + default: 0
> +
> + adi,excitation-current-0-nanoamps:
> + description: |
> + Excitation current in nanoamps to be applied to pin specified in
> + adi,excitation-pin-0 while this channel is active.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 100, 10000, 20000, 50000, 100000, 150000, 200000]
> + default: 0
> +
> + adi,excitation-current-1-nanoamps:
> + description: |
> + Excitation current in nanoamps to be applied to pin specified in
> + adi,excitation-pin-1 while this channel is active.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 100, 10000, 20000, 50000, 100000, 150000, 200000]
> + default: 0
> +
> + adi,burnout-current-nanoamps:
> + description: |
> + Burnout current in nanoamps to be applied for this channel.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 500, 2000, 4000]
> + default: 0
> +
> + adi,buffered-positive:
> + description: Enable buffered mode for positive input.
> + type: boolean
> +
> + adi,buffered-negative:
> + description: Enable buffered mode for negative input.
> + type: boolean
> +
> + required:
> + - reg
> + - diff-channels
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "adi,ad4130-8-24-wlcsp";
> + reg = <0>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + spi-max-frequency = <5000000>;
> + interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
> + interrupt-parent = <&gpio>;
> +
> + channel@0 {
> + reg = <0>;
> +
> + adi,reference-select = <2>;
> +
> + /* AIN8, AIN9 */
> + diff-channels = <8 9>;
> + };
> +
> + channel@1 {
> + reg = <1>;
> +
> + adi,reference-select = <2>;
> +
> + /* AIN10, AIN11 */
> + diff-channels = <10 11>;
> + };
> +
> + channel@2 {
> + reg = <2>;
> +
> + adi,reference-select = <2>;
> +
> + /* Temperature Sensor, DGND */
> + diff-channels = <16 19>;
> + };
> +
> + channel@3 {
> + reg = <3>;
> +
> + adi,reference-select = <2>;
> +
> + /* Internal reference, DGND */
> + diff-channels = <18 19>;
> + };
> +
> + channel@4 {
> + reg = <4>;
> +
> + adi,reference-select = <2>;
> +
> + /* DGND, DGND */
> + diff-channels = <19 19>;
> + };
> + };
> + };
> --
> 2.35.3
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130
2022-04-26 0:37 ` Rob Herring
@ 2022-04-27 12:47 ` Cosmin Tanislav
2022-04-27 21:41 ` Rob Herring
2022-06-08 8:31 ` Cosmin Tanislav
1 sibling, 1 reply; 15+ messages in thread
From: Cosmin Tanislav @ 2022-04-27 12:47 UTC (permalink / raw)
To: Rob Herring
Cc: Jonathan Cameron, Linus Walleij, linux-iio, linux-gpio,
linux-kernel, devicetree, Cosmin Tanislav
On 4/26/22 03:37, Rob Herring wrote:
> On Tue, Apr 19, 2022 at 06:08:27PM +0300, Cosmin Tanislav wrote:
>> AD4130-8 is an ultra-low power, high precision, measurement solution for
>> low bandwidth battery operated applications.
>>
>> The fully integrated AFE (Analog Front-End) includes a multiplexer for up
>> to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
>> Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
>> selectable filter options, smart sequencer, sensor biasing and excitation
>> options, diagnostics, and a FIFO buffer.
>>
>> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
>> ---
>> .../bindings/iio/adc/adi,ad4130.yaml | 264 ++++++++++++++++++
>> 1 file changed, 264 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
>> new file mode 100644
>> index 000000000000..32996b62cd20
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
>> @@ -0,0 +1,264 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +# Copyright 2022 Analog Devices Inc.
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4130.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Analog Devices AD4130 ADC device driver
>> +
>> +maintainers:
>> + - Cosmin Tanislav <cosmin.tanislav@analog.com>
>> +
>> +description: |
>> + Bindings for the Analog Devices AD4130 ADC. Datasheet can be found here:
>> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4130-8.pdf
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - adi,ad4130-8-16-lfcsp
>> + - adi,ad4130-8-16-wlcsp
>> + - adi,ad4130-8-24-lfcsp
>> + - adi,ad4130-8-24-wlcsp
>
> What is lfcsp? wlcsp seems to be the package type which generally
> shouldn't be part of the compatible.
lfcsp is a different package type. Sadly, lfcsp provides less interrupt
options. On lfcsp, dout-int inside interrupt-names actually only means
DOUT, while on wlcsp, it means INT. This is why we need to distinguish
between the different package types. I can't think of any way around it,
see my reply to Nathan for V1.
dout support is not implemented in the driver right now because when the
interrupt pin is configured as dout, FIFO interrupts are unsupported, so
the entire buffered part of the driver is useless, and extra logic is
needed for IRQ detection then.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130
2022-04-27 12:47 ` Cosmin Tanislav
@ 2022-04-27 21:41 ` Rob Herring
0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2022-04-27 21:41 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Jonathan Cameron, Linus Walleij, linux-iio, linux-gpio,
linux-kernel, devicetree, Cosmin Tanislav
On Wed, Apr 27, 2022 at 03:47:13PM +0300, Cosmin Tanislav wrote:
>
>
> On 4/26/22 03:37, Rob Herring wrote:
> > On Tue, Apr 19, 2022 at 06:08:27PM +0300, Cosmin Tanislav wrote:
> > > AD4130-8 is an ultra-low power, high precision, measurement solution for
> > > low bandwidth battery operated applications.
> > >
> > > The fully integrated AFE (Analog Front-End) includes a multiplexer for up
> > > to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
> > > Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
> > > selectable filter options, smart sequencer, sensor biasing and excitation
> > > options, diagnostics, and a FIFO buffer.
> > >
> > > Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> > > ---
> > > .../bindings/iio/adc/adi,ad4130.yaml | 264 ++++++++++++++++++
> > > 1 file changed, 264 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
> > > new file mode 100644
> > > index 000000000000..32996b62cd20
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
> > > @@ -0,0 +1,264 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +# Copyright 2022 Analog Devices Inc.
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/adc/adi,ad4130.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Analog Devices AD4130 ADC device driver
> > > +
> > > +maintainers:
> > > + - Cosmin Tanislav <cosmin.tanislav@analog.com>
> > > +
> > > +description: |
> > > + Bindings for the Analog Devices AD4130 ADC. Datasheet can be found here:
> > > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4130-8.pdf
> > > +
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - adi,ad4130-8-16-lfcsp
> > > + - adi,ad4130-8-16-wlcsp
> > > + - adi,ad4130-8-24-lfcsp
> > > + - adi,ad4130-8-24-wlcsp
> >
> > What is lfcsp? wlcsp seems to be the package type which generally
> > shouldn't be part of the compatible.
>
> lfcsp is a different package type. Sadly, lfcsp provides less interrupt
> options. On lfcsp, dout-int inside interrupt-names actually only means
> DOUT, while on wlcsp, it means INT. This is why we need to distinguish
> between the different package types. I can't think of any way around it,
> see my reply to Nathan for V1.
>
> dout support is not implemented in the driver right now because when the
> interrupt pin is configured as dout, FIFO interrupts are unsupported, so
> the entire buffered part of the driver is useless, and extra logic is
> needed for IRQ detection then.
Please capture all this in the binding. At least enough I don't ask the
same question again when I've forgotten about this.
Rob
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver
2022-04-19 15:08 ` [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver Cosmin Tanislav
@ 2022-05-01 16:08 ` Jonathan Cameron
2022-05-02 8:53 ` Cosmin Tanislav
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2022-05-01 16:08 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Rob Herring, Linus Walleij, linux-iio, linux-gpio, linux-kernel,
devicetree, Cosmin Tanislav
On Tue, 19 Apr 2022 18:08:28 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:
> AD4130-8 is an ultra-low power, high precision, measurement solution for
> low bandwidth battery operated applications.
>
> The fully integrated AFE (Analog Front-End) includes a multiplexer for up
> to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
> Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
> selectable filter options, smart sequencer, sensor biasing and excitation
> options, diagnostics, and a FIFO buffer.
>
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
Hi Cosmin,
Sorry this one took me a while to get back to. Last week was a little
crazy on the day job front.
Anyhow, whilst reading this I realised we have some unclear ABI documentation
and hence divergence in interpretation of the units of hwfifo_watermark.
See below. I've not done a thorough audit as don't have enough time today,
but we have at least some drivers treating it as being in scans (+ the core
does this for watermark) and some treating it as being in individual channel
readings... My intent IIRC was that it was in scans as otherwise you run into
the problem you are working around with needing to tweak it to match scans
within each driver.
A few other trivial things inline including the alignment issue I emailed
the list about but haven't sent patches out for yet (about 90 driver are
affected... *sigh*).
Thanks,
Jonathan
> ---
...
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 39d806f6d457..c3aa7e4eca2f 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_AD4130) += ad4130.o
> obj-$(CONFIG_AD7091R5) += ad7091r5.o ad7091r-base.o
> obj-$(CONFIG_AD7124) += ad7124.o
> obj-$(CONFIG_AD7192) += ad7192.o
> diff --git a/drivers/iio/adc/ad4130.c b/drivers/iio/adc/ad4130.c
> new file mode 100644
> index 000000000000..dd2f18734cba
> --- /dev/null
> +++ b/drivers/iio/adc/ad4130.c
...
> +struct ad4130_state {
> + const struct ad4130_chip_info *chip_info;
> + struct spi_device *spi;
> + struct regmap *regmap;
> + struct clk *mclk;
> + struct regulator_bulk_data regulators[4];
> + u32 irq_trigger;
> + u32 inv_irq_trigger;
> +
> + /*
> + * Synchronize access to members of driver state, and ensure atomicity
> + * of consecutive regmap operations.
> + */
> + struct mutex lock;
> + struct completion completion;
> +
> + struct iio_chan_spec chans[AD4130_MAX_CHANNELS];
> + struct ad4130_chan_info chans_info[AD4130_MAX_CHANNELS];
> + struct ad4130_slot_info slots_info[AD4130_MAX_SETUPS];
> + enum ad4130_pin_function pins_fn[AD4130_MAX_ANALOG_PINS];
> + u32 vbias_pins[AD4130_MAX_ANALOG_PINS];
> + u32 num_vbias_pins;
> + int scale_tbls[AD4130_REF_SEL_MAX][AD4130_PGA_NUM][2];
> + struct gpio_chip gc;
> + unsigned int gpio_offsets[AD4130_MAX_GPIOS];
> + unsigned int num_gpios;
> +
> + u32 int_pin_sel;
> + u32 int_ref_uv;
> + u32 mclk_sel;
> + bool int_ref_en;
> + bool bipolar;
> +
> + unsigned int num_enabled_channels;
> + unsigned int effective_watermark;
> + unsigned int watermark;
> +
> + struct spi_message fifo_msg;
> + struct spi_transfer fifo_xfer[2];
> +
> + /*
> + * DMA (thus cache coherency maintenance) requires the
> + * transfer buffers to live in their own cache lines.
> + */
> + u8 reset_buf[AD4130_RESET_BUF_SIZE] ____cacheline_aligned;
This needs a fix along with the other set that I'll hopefully get this weekend.
In meantime if you change this to __aligned(IIO_ALIGN); then the fix I have
coming shortly will change that define to be large enough for the odd ARM systems
with 128 byte requirement for DMA and 64 byte L1 cache alignment (which is what
___cacheline_aligned guarantees.
> + u8 reg_write_tx_buf[4];
> + u8 reg_read_tx_buf[1];
> + u8 reg_read_rx_buf[3];
> + u8 fifo_tx_buf[2];
> + u8 fifo_rx_buf[AD4130_FIFO_SIZE *
> + AD4130_FIFO_MAX_SAMPLE_SIZE];
> +};
> +
> +static int ad4130_set_fifo_watermark(struct iio_dev *indio_dev, unsigned int val)
> +{
> + struct ad4130_state *st = iio_priv(indio_dev);
> + unsigned int eff;
> + int ret;
> +
> + if (val > AD4130_FIFO_SIZE)
> + return -EINVAL;
> +
> + /*
> + * Always set watermark to a multiple of the number of enabled channels
> + * to avoid making the FIFO unaligned.
> + */
> + eff = rounddown(val, st->num_enabled_channels);
> +
> + mutex_lock(&st->lock);
> +
> + ret = regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
> + AD4130_WATERMARK_MASK,
> + FIELD_PREP(AD4130_WATERMARK_MASK,
> + ad4130_watermark_reg_val(eff)));
> + if (ret)
> + goto out;
> +
> + st->effective_watermark = eff;
> + st->watermark = val;
Hmm this is a potential inconsistency in the IIO ABI.
ABI docs describes watermark as being number of 'scan elements' which is
not the clearest text we could have gone with...
Now I may well have made a mistake in the following as it's rather a long time
since I last looked at the core handling for this...
The core treats it as number datum (which is same as a scan) when using
it for the main watermark attribute and also when using watermarks with the
kfifo (the IIO fifo is made up of objects each of which is a scan. So kfifo_len()
returns the number of scans.
Looking very quickly at a few other drivers
adxl367 seems to use number of samples.
adxl372 is using number of scans.
bmc150 hardware seems to work on basis of frame count which I 'think' is probably scans.
fxls8962 uses 'samples count' which is not clearly defined in the datasheet but there
is an example showing that it's scans (I think)...
lsm6dsx - some of the fifos used with this are based on tagged data so the connection to
what hits the front end buffers is non obvious.
So, not great for consistency :(
Going forwards i think we should standardize the hardware fifo watermark on what is being
used for the software watermark which I believe is number of scans.
Not necessary much we can do about old drivers though due to risk of breaking ABI...
We should make the documentation clearer though.
> +
> +out:
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
...
> +
> +static int ad4130_parse_fw_channel(struct iio_dev *indio_dev,
> + struct fwnode_handle *child)
> +{
> + struct ad4130_state *st = iio_priv(indio_dev);
> + unsigned int index = indio_dev->num_channels++;
> + struct device *dev = &st->spi->dev;
> + struct ad4130_chan_info *chan_info;
> + struct iio_chan_spec *chan;
> + u32 pins[2];
> + int ret;
> +
> + if (index >= AD4130_MAX_CHANNELS)
> + return dev_err_probe(dev, -EINVAL, "Too many channels\n");
> +
> + chan = &st->chans[index];
> + chan_info = &st->chans_info[index];
> +
> + *chan = ad4130_channel_template;
> + chan->scan_type.realbits = st->chip_info->resolution;
> + chan->scan_type.storagebits = st->chip_info->resolution;
> + chan->scan_index = index;
> +
> + chan_info->slot = AD4130_INVALID_SLOT;
> + chan_info->setup.fs = AD4130_FS_MIN;
> + chan_info->initialized = true;
> +
> + ret = fwnode_property_read_u32_array(child, "diff-channels", pins,
> + ARRAY_SIZE(pins));
> + if (ret)
> + return ret;
> +
> + ret = ad4130_validate_diff_channels(st, pins, ARRAY_SIZE(pins));
> + if (ret)
> + return ret;
> +
> + chan->channel = pins[0];
> + chan->channel2 = pins[1];
> +
> + ret = ad4130_parse_fw_setup(st, child, &chan_info->setup);
> + if (ret)
> + return ret;
> +
> + fwnode_property_read_u32(child, "adi,excitation-pin-0",
> + &chan_info->iout0);
> + if (chan_info->setup.iout0_val != AD4130_IOUT_OFF) {
It would be slightly better to set an explicit default value here as the fact it
is 0 is hidden by the enum. e.g.
chan_info->iout0 = AD4130_IOUT_OFF;
fwnode_property_read_u32(child, "adi,excitation-pin-0",
&chan_info->iout0);
if (chan_info->....
That would save reviewers wondering what the default is and having to go
check the enum (and I'm lazy :)
> + ret = ad4130_validate_excitation_pin(st, chan_info->iout0);
> + if (ret)
> + return ret;
> + }
> +
> + fwnode_property_read_u32(child, "adi,excitation-pin-1",
> + &chan_info->iout1);
> + if (chan_info->setup.iout1_val != AD4130_IOUT_OFF) {
> + ret = ad4130_validate_excitation_pin(st, chan_info->iout1);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ad4130_parse_fw_children(struct iio_dev *indio_dev)
> +{
> + struct ad4130_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> + struct fwnode_handle *child;
> + int ret;
> +
> + indio_dev->channels = st->chans;
> +
> + device_for_each_child_node(dev, child) {
> + ret = ad4130_parse_fw_channel(indio_dev, child);
> + if (ret) {
> + fwnode_handle_put(child);
> + break;
Trivial, but slight preference for return ret here and
then return 0 below.
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int ad4310_parse_fw(struct iio_dev *indio_dev)
> +{
> + struct ad4130_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> + u32 ext_clk_freq = AD4130_MCLK_FREQ_76_8KHZ;
> + bool int_clk_out = false;
> + unsigned int i;
> + int avdd_uv;
> + int irq;
> + int ret;
> +
...
> +
> + device_property_read_u32(dev, "adi,ext-clk-freq", &ext_clk_freq);
I'll note this in reviewing the binding, but the naming should incorporate the
units.
> + if (ext_clk_freq != AD4130_MCLK_FREQ_153_6KHZ &&
> + ext_clk_freq != AD4130_MCLK_FREQ_76_8KHZ)
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid external clock frequency %u\n",
> + ext_clk_freq);
> +
...
> +
> + return 0;
> +}
> +
> +
> +static int ad4130_setup(struct iio_dev *indio_dev)
> +{
> + struct ad4130_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> + unsigned int int_ref_val;
> + unsigned long rate = AD4130_MCLK_FREQ_76_8KHZ;
> + unsigned int val;
> + unsigned int i;
> + int ret;
> +
> + if (st->mclk_sel == AD4130_MCLK_153_6KHZ_EXT)
> + rate = AD4130_MCLK_FREQ_153_6KHZ;
> +
> + ret = clk_set_rate(st->mclk, rate);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(st->mclk);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev, ad4130_clk_disable_unprepare,
> + st->mclk);
> + if (ret)
> + return ret;
> +
> + if (st->int_ref_uv == AD4130_INT_REF_2_5V)
> + int_ref_val = AD4130_INT_REF_VAL_2_5V;
> + else
> + int_ref_val = AD4130_INT_REF_VAL_1_25V;
> +
> + /* Switch to SPI 4-wire mode. */
> + val = AD4130_CSB_EN_MASK;
Trivial but I'd prefer to see this treated as a field as well for consistency.
val = FIELD_PREP(AD4130_CSB_EN_MASK, 1);
> + val |= FIELD_PREP(AD4130_BIPOLAR_MASK, st->bipolar);
> + val |= FIELD_PREP(AD4130_INT_REF_EN_MASK, st->int_ref_en);
> + val |= FIELD_PREP(AD4130_MODE_MASK, AD4130_MODE_IDLE);
> + val |= FIELD_PREP(AD4130_MCLK_SEL_MASK, st->mclk_sel);
> + val |= FIELD_PREP(AD4130_INT_REF_VAL_MASK, int_ref_val);
> +
> + ret = regmap_write(st->regmap, AD4130_REG_ADC_CONTROL, val);
> + if (ret)
> + return ret;
> +
> + val = FIELD_PREP(AD4130_INT_PIN_SEL_MASK, st->int_pin_sel);
> + for (i = 0; i < st->num_gpios; i++)
> + val |= BIT(st->gpio_offsets[i]);
> +
> + ret = regmap_write(st->regmap, AD4130_REG_IO_CONTROL, val);
> + if (ret)
> + return ret;
> +
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130
2022-04-19 15:08 ` [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130 Cosmin Tanislav
2022-04-26 0:37 ` Rob Herring
@ 2022-05-01 16:09 ` Jonathan Cameron
1 sibling, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2022-05-01 16:09 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Rob Herring, Linus Walleij, linux-iio, linux-gpio, linux-kernel,
devicetree, Cosmin Tanislav
On Tue, 19 Apr 2022 18:08:27 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:
> AD4130-8 is an ultra-low power, high precision, measurement solution for
> low bandwidth battery operated applications.
>
> The fully integrated AFE (Analog Front-End) includes a multiplexer for up
> to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
> Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
> selectable filter options, smart sequencer, sensor biasing and excitation
> options, diagnostics, and a FIFO buffer.
>
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
Hi Cosmin,
A few things inline to add to fixing the lack of detail Rob highlighted.
Thanks,
Jonathan
> ---
> .../bindings/iio/adc/adi,ad4130.yaml | 264 ++++++++++++++++++
> 1 file changed, 264 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
> new file mode 100644
> index 000000000000..32996b62cd20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
> @@ -0,0 +1,264 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2022 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4130.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD4130 ADC device driver
> +
> +maintainers:
> + - Cosmin Tanislav <cosmin.tanislav@analog.com>
> +
> +description: |
> + Bindings for the Analog Devices AD4130 ADC. Datasheet can be found here:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4130-8.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad4130-8-16-lfcsp
> + - adi,ad4130-8-16-wlcsp
> + - adi,ad4130-8-24-lfcsp
> + - adi,ad4130-8-24-wlcsp
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> + description: phandle to the master clock (mclk)
> +
> + clock-names:
> + items:
> + - const: mclk
> +
> + interrupts:
> + maxItems: 1
> +
> + interrupt-names:
> + description: |
> + Specify which interrupt pin should be configured as Data Ready / FIFO
> + interrupt.
> + Default if not supplied is dout-int.
> + enum:
> + - dout-int
> + - clk
> + - p2
> + - dout
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + refin1-supply:
> + description: refin1 supply. Can be used as reference for conversion.
> +
> + refin2-supply:
> + description: refin2 supply. Can be used as reference for conversion.
> +
> + avdd-supply:
> + description: AVDD voltage supply. Can be used as reference for conversion.
> +
> + iovdd-supply:
> + description: IOVDD voltage supply. Used for the chip interface.
> +
> + spi-max-frequency:
> + maximum: 5000000
> +
> + adi,int-clk-out:
> + description: Specify if the internal clock should be exposed on the CLK pin.
> + type: boolean
> +
> + adi,ext-clk-freq:
> + description: Specify the frequency of the external clock.
Units? Even better if we can map this to one of the standard unit types and
include the unit in the name.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [76800, 153600]
> + default: 76800
> +
> + adi,bipolar:
> + description: Specify if the device should be used in bipolar mode.
> + type: boolean
> +
> + adi,vbias-pins:
> + description: Analog inputs to apply a voltage bias of (AVDD − AVSS) / 2 to.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + maxItems: 16
> + items:
> + minimum: 0
> + maximum: 15
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +patternProperties:
> + "^channel@([0-9]|1[0-5])$":
> + type: object
> + $ref: adc.yaml
> +
> + properties:
> + reg:
> + description: |
> + The channel number.
This isn't used explicitly in the driver. I'm wondering
if perhaps it should be rather than using the order in which the
child nodes are found...
The driver would then need to cope with potential holes however
(or just reject a binding where they occur?).
> + items:
> + minimum: 0
> + maximum: 15
> +
> + diff-channels:
> + description: |
> + Besides the analog inputs available, internal inputs can be used.
> + 16: Internal temperature sensor.
> + 17: AVSS
> + 18: Internal reference
> + 19: DGND
> + 20: (AVDD − AVSS)/6+
> + 21: (AVDD − AVSS)/6-
> + 22: (IOVDD − DGND)/6+
> + 23: (IOVDD − DGND)/6-
> + 24: (ALDO − AVSS)/6+
> + 25: (ALDO − AVSS)/6-
> + 26: (DLDO − DGND)/6+
> + 27: (DLDO − DGND)/6-
> + 28: V_MV_P
> + 29: V_MV_M
> + items:
> + minimum: 0
> + maximum: 29
> +
> + adi,reference-select:
> + description: |
> + Select the reference source to use when converting on the
> + specific channel. Valid values are:
> + 0: REFIN1(+)/REFIN1(−)
> + 1: REFIN2(+)/REFIN2(−)
> + 2: REFOUT/AVSS (Internal reference)
> + 3: AVDD/AVSS
> + If not specified, REFIN1 is used.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 3]
> + default: 0
> +
> + adi,excitation-pin-0:
> + description: |
> + Analog input to apply excitation current to while the channel
> + is active.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 15
> + default: 0
> +
> + adi,excitation-pin-1:
> + description: |
> + Analog input to apply excitation current to while this channel
> + is active.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 15
> + default: 0
> +
> + adi,excitation-current-0-nanoamps:
> + description: |
> + Excitation current in nanoamps to be applied to pin specified in
> + adi,excitation-pin-0 while this channel is active.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 100, 10000, 20000, 50000, 100000, 150000, 200000]
> + default: 0
> +
> + adi,excitation-current-1-nanoamps:
> + description: |
> + Excitation current in nanoamps to be applied to pin specified in
> + adi,excitation-pin-1 while this channel is active.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 100, 10000, 20000, 50000, 100000, 150000, 200000]
> + default: 0
> +
> + adi,burnout-current-nanoamps:
> + description: |
> + Burnout current in nanoamps to be applied for this channel.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 500, 2000, 4000]
> + default: 0
> +
> + adi,buffered-positive:
> + description: Enable buffered mode for positive input.
> + type: boolean
> +
> + adi,buffered-negative:
> + description: Enable buffered mode for negative input.
> + type: boolean
> +
> + required:
> + - reg
> + - diff-channels
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "adi,ad4130-8-24-wlcsp";
> + reg = <0>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + spi-max-frequency = <5000000>;
> + interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
> + interrupt-parent = <&gpio>;
> +
> + channel@0 {
> + reg = <0>;
> +
> + adi,reference-select = <2>;
> +
> + /* AIN8, AIN9 */
> + diff-channels = <8 9>;
> + };
> +
> + channel@1 {
> + reg = <1>;
> +
> + adi,reference-select = <2>;
> +
> + /* AIN10, AIN11 */
> + diff-channels = <10 11>;
> + };
> +
> + channel@2 {
> + reg = <2>;
> +
> + adi,reference-select = <2>;
> +
> + /* Temperature Sensor, DGND */
> + diff-channels = <16 19>;
> + };
> +
> + channel@3 {
> + reg = <3>;
> +
> + adi,reference-select = <2>;
> +
> + /* Internal reference, DGND */
> + diff-channels = <18 19>;
> + };
> +
> + channel@4 {
> + reg = <4>;
> +
> + adi,reference-select = <2>;
> +
> + /* DGND, DGND */
> + diff-channels = <19 19>;
> + };
> + };
> + };
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver
2022-05-01 16:08 ` Jonathan Cameron
@ 2022-05-02 8:53 ` Cosmin Tanislav
2022-05-07 16:35 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Cosmin Tanislav @ 2022-05-02 8:53 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Rob Herring, Linus Walleij, linux-iio, linux-gpio, linux-kernel,
devicetree, Cosmin Tanislav
On 5/1/22 19:08, Jonathan Cameron wrote:
> On Tue, 19 Apr 2022 18:08:28 +0300
> Cosmin Tanislav <demonsingur@gmail.com> wrote:
>
>> AD4130-8 is an ultra-low power, high precision, measurement solution for
>> low bandwidth battery operated applications.
>>
>> The fully integrated AFE (Analog Front-End) includes a multiplexer for up
>> to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
>> Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
>> selectable filter options, smart sequencer, sensor biasing and excitation
>> options, diagnostics, and a FIFO buffer.
>>
>> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
>
> Hi Cosmin,
>
> Sorry this one took me a while to get back to. Last week was a little
> crazy on the day job front.
>
> Anyhow, whilst reading this I realised we have some unclear ABI documentation
> and hence divergence in interpretation of the units of hwfifo_watermark.
> See below. I've not done a thorough audit as don't have enough time today,
> but we have at least some drivers treating it as being in scans (+ the core
> does this for watermark) and some treating it as being in individual channel
> readings... My intent IIRC was that it was in scans as otherwise you run into
> the problem you are working around with needing to tweak it to match scans
> within each driver.
>
> A few other trivial things inline including the alignment issue I emailed
> the list about but haven't sent patches out for yet (about 90 driver are
> affected... *sigh*).
>
> Thanks,
>
> Jonathan
> >
>> ---
>
> ...
>
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> index 39d806f6d457..c3aa7e4eca2f 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_AD4130) += ad4130.o
>> obj-$(CONFIG_AD7091R5) += ad7091r5.o ad7091r-base.o
>> obj-$(CONFIG_AD7124) += ad7124.o
>> obj-$(CONFIG_AD7192) += ad7192.o
>> diff --git a/drivers/iio/adc/ad4130.c b/drivers/iio/adc/ad4130.c
>> new file mode 100644
>> index 000000000000..dd2f18734cba
>> --- /dev/null
>> +++ b/drivers/iio/adc/ad4130.c
>
> ...
>
>> +struct ad4130_state {
>> + const struct ad4130_chip_info *chip_info;
>> + struct spi_device *spi;
>> + struct regmap *regmap;
>> + struct clk *mclk;
>> + struct regulator_bulk_data regulators[4];
>> + u32 irq_trigger;
>> + u32 inv_irq_trigger;
>> +
>> + /*
>> + * Synchronize access to members of driver state, and ensure atomicity
>> + * of consecutive regmap operations.
>> + */
>> + struct mutex lock;
>> + struct completion completion;
>> +
>> + struct iio_chan_spec chans[AD4130_MAX_CHANNELS];
>> + struct ad4130_chan_info chans_info[AD4130_MAX_CHANNELS];
>> + struct ad4130_slot_info slots_info[AD4130_MAX_SETUPS];
>> + enum ad4130_pin_function pins_fn[AD4130_MAX_ANALOG_PINS];
>> + u32 vbias_pins[AD4130_MAX_ANALOG_PINS];
>> + u32 num_vbias_pins;
>> + int scale_tbls[AD4130_REF_SEL_MAX][AD4130_PGA_NUM][2];
>> + struct gpio_chip gc;
>> + unsigned int gpio_offsets[AD4130_MAX_GPIOS];
>> + unsigned int num_gpios;
>> +
>> + u32 int_pin_sel;
>> + u32 int_ref_uv;
>> + u32 mclk_sel;
>> + bool int_ref_en;
>> + bool bipolar;
>> +
>> + unsigned int num_enabled_channels;
>> + unsigned int effective_watermark;
>> + unsigned int watermark;
>> +
>> + struct spi_message fifo_msg;
>> + struct spi_transfer fifo_xfer[2];
>> +
>> + /*
>> + * DMA (thus cache coherency maintenance) requires the
>> + * transfer buffers to live in their own cache lines.
>> + */
>> + u8 reset_buf[AD4130_RESET_BUF_SIZE] ____cacheline_aligned;
>
> This needs a fix along with the other set that I'll hopefully get this weekend.
> In meantime if you change this to __aligned(IIO_ALIGN); then the fix I have
> coming shortly will change that define to be large enough for the odd ARM systems
> with 128 byte requirement for DMA and 64 byte L1 cache alignment (which is what
> ___cacheline_aligned guarantees.
>
Sure thing.
>
>> + u8 reg_write_tx_buf[4];
>> + u8 reg_read_tx_buf[1];
>> + u8 reg_read_rx_buf[3];
>> + u8 fifo_tx_buf[2];
>> + u8 fifo_rx_buf[AD4130_FIFO_SIZE *
>> + AD4130_FIFO_MAX_SAMPLE_SIZE];
>> +};
>> +
>
>> +static int ad4130_set_fifo_watermark(struct iio_dev *indio_dev, unsigned int val)
>> +{
>> + struct ad4130_state *st = iio_priv(indio_dev);
>> + unsigned int eff;
>> + int ret;
>> +
>> + if (val > AD4130_FIFO_SIZE)
>> + return -EINVAL;
>> +
>> + /*
>> + * Always set watermark to a multiple of the number of enabled channels
>> + * to avoid making the FIFO unaligned.
>> + */
>> + eff = rounddown(val, st->num_enabled_channels);
>> +
>> + mutex_lock(&st->lock);
>> +
>> + ret = regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
>> + AD4130_WATERMARK_MASK,
>> + FIELD_PREP(AD4130_WATERMARK_MASK,
>> + ad4130_watermark_reg_val(eff)));
>> + if (ret)
>> + goto out;
>> +
>> + st->effective_watermark = eff;
>> + st->watermark = val;
>
> Hmm this is a potential inconsistency in the IIO ABI.
> ABI docs describes watermark as being number of 'scan elements' which is
> not the clearest text we could have gone with...
>
> Now I may well have made a mistake in the following as it's rather a long time
> since I last looked at the core handling for this...
>
> The core treats it as number datum (which is same as a scan) when using
> it for the main watermark attribute and also when using watermarks with the
> kfifo (the IIO fifo is made up of objects each of which is a scan. So kfifo_len()
> returns the number of scans.
>
> Looking very quickly at a few other drivers
> adxl367 seems to use number of samples.
> adxl372 is using number of scans.
> bmc150 hardware seems to work on basis of frame count which I 'think' is probably scans.
> fxls8962 uses 'samples count' which is not clearly defined in the datasheet but there
> is an example showing that it's scans (I think)...
> lsm6dsx - some of the fifos used with this are based on tagged data so the connection to
> what hits the front end buffers is non obvious.
>
> So, not great for consistency :(
>
> Going forwards i think we should standardize the hardware fifo watermark on what is being
> used for the software watermark which I believe is number of scans.
> Not necessary much we can do about old drivers though due to risk of breaking ABI...
> We should make the documentation clearer though.
>
I was confused too, but this seemed more logical to me at the time, and
since you didn't say anything regarding it on ADXL367, I did it the same
way here. I guess we can't go back and change it now on ADXL367, I'm
sorry for this. I'll fix it.
>> +
>> +out:
>> + mutex_unlock(&st->lock);
>> +
>> + return ret;
>> +}
>> +
>
>
> ...
>
>> +
>> +static int ad4130_parse_fw_channel(struct iio_dev *indio_dev,
>> + struct fwnode_handle *child)
>> +{
>> + struct ad4130_state *st = iio_priv(indio_dev);
>> + unsigned int index = indio_dev->num_channels++;
>> + struct device *dev = &st->spi->dev;
>> + struct ad4130_chan_info *chan_info;
>> + struct iio_chan_spec *chan;
>> + u32 pins[2];
>> + int ret;
>> +
>> + if (index >= AD4130_MAX_CHANNELS)
>> + return dev_err_probe(dev, -EINVAL, "Too many channels\n");
>> +
>> + chan = &st->chans[index];
>> + chan_info = &st->chans_info[index];
>> +
>> + *chan = ad4130_channel_template;
>> + chan->scan_type.realbits = st->chip_info->resolution;
>> + chan->scan_type.storagebits = st->chip_info->resolution;
>> + chan->scan_index = index;
>> +
>> + chan_info->slot = AD4130_INVALID_SLOT;
>> + chan_info->setup.fs = AD4130_FS_MIN;
>> + chan_info->initialized = true;
>> +
>> + ret = fwnode_property_read_u32_array(child, "diff-channels", pins,
>> + ARRAY_SIZE(pins));
>> + if (ret)
>> + return ret;
>> +
>> + ret = ad4130_validate_diff_channels(st, pins, ARRAY_SIZE(pins));
>> + if (ret)
>> + return ret;
>> +
>> + chan->channel = pins[0];
>> + chan->channel2 = pins[1];
>> +
>> + ret = ad4130_parse_fw_setup(st, child, &chan_info->setup);
>> + if (ret)
>> + return ret;
>> +
>> + fwnode_property_read_u32(child, "adi,excitation-pin-0",
>> + &chan_info->iout0);
>> + if (chan_info->setup.iout0_val != AD4130_IOUT_OFF) {
>
> It would be slightly better to set an explicit default value here as the fact it
> is 0 is hidden by the enum. e.g.
> chan_info->iout0 = AD4130_IOUT_OFF;
> fwnode_property_read_u32(child, "adi,excitation-pin-0",
> &chan_info->iout0);
> if (chan_info->....
> That would save reviewers wondering what the default is and having to go
> check the enum (and I'm lazy :)
I understand the idea, but the default value for iout0 is not
AD4130_IOUT_OFF. iout0 is the pin that iout0_val current is
applied to, and AD4130_IOUT_OFF is a value for iout0_val.
Look at ad4130_parse_fw_setup.
For iout0, I guess I could do
#define AD4130_AIN0 0x0
...
chan_info->iout0 = AD4130_AIN0;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int ad4310_parse_fw(struct iio_dev *indio_dev)
>> +{
>> + struct ad4130_state *st = iio_priv(indio_dev);
>> + struct device *dev = &st->spi->dev;
>> + u32 ext_clk_freq = AD4130_MCLK_FREQ_76_8KHZ;
>> + bool int_clk_out = false;
>> + unsigned int i;
>> + int avdd_uv;
>> + int irq;
>> + int ret;
>> +
>
> ...
>
>> +
>> + device_property_read_u32(dev, "adi,ext-clk-freq", &ext_clk_freq);
>
> I'll note this in reviewing the binding, but the naming should incorporate the
> units.
>
>> + if (ext_clk_freq != AD4130_MCLK_FREQ_153_6KHZ &&
>> + ext_clk_freq != AD4130_MCLK_FREQ_76_8KHZ)
>> + return dev_err_probe(dev, -EINVAL,
>> + "Invalid external clock frequency %u\n",
>> + ext_clk_freq);
>> +
>
> ...
>
>> +
>> + return 0;
>> +}
>> +
>
>> +
>> +static int ad4130_setup(struct iio_dev *indio_dev)
>> +{
>> + struct ad4130_state *st = iio_priv(indio_dev);
>> + struct device *dev = &st->spi->dev;
>> + unsigned int int_ref_val;
>> + unsigned long rate = AD4130_MCLK_FREQ_76_8KHZ;
>> + unsigned int val;
>> + unsigned int i;
>> + int ret;
>> +
>> + if (st->mclk_sel == AD4130_MCLK_153_6KHZ_EXT)
>> + rate = AD4130_MCLK_FREQ_153_6KHZ;
>> +
>> + ret = clk_set_rate(st->mclk, rate);
>> + if (ret)
>> + return ret;
>> +
>> + ret = clk_prepare_enable(st->mclk);
>> + if (ret)
>> + return ret;
>> +
>> + ret = devm_add_action_or_reset(dev, ad4130_clk_disable_unprepare,
>> + st->mclk);
>> + if (ret)
>> + return ret;
>> +
>> + if (st->int_ref_uv == AD4130_INT_REF_2_5V)
>> + int_ref_val = AD4130_INT_REF_VAL_2_5V;
>> + else
>> + int_ref_val = AD4130_INT_REF_VAL_1_25V;
>> +
>> + /* Switch to SPI 4-wire mode. */
>> + val = AD4130_CSB_EN_MASK;
>
> Trivial but I'd prefer to see this treated as a field as well for consistency.
> val = FIELD_PREP(AD4130_CSB_EN_MASK, 1);
Already did, but I waited for your input before pushing the new
patchset.
> >> + val |= FIELD_PREP(AD4130_BIPOLAR_MASK, st->bipolar);
>> + val |= FIELD_PREP(AD4130_INT_REF_EN_MASK, st->int_ref_en);
>> + val |= FIELD_PREP(AD4130_MODE_MASK, AD4130_MODE_IDLE);
>> + val |= FIELD_PREP(AD4130_MCLK_SEL_MASK, st->mclk_sel);
>> + val |= FIELD_PREP(AD4130_INT_REF_VAL_MASK, int_ref_val);
>> +
>> + ret = regmap_write(st->regmap, AD4130_REG_ADC_CONTROL, val);
>> + if (ret)
>> + return ret;
>> +
>> + val = FIELD_PREP(AD4130_INT_PIN_SEL_MASK, st->int_pin_sel);
>> + for (i = 0; i < st->num_gpios; i++)
>> + val |= BIT(st->gpio_offsets[i]);
>> +
>> + ret = regmap_write(st->regmap, AD4130_REG_IO_CONTROL, val);
>> + if (ret)
>> + return ret;
>> +
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver
2022-05-02 8:53 ` Cosmin Tanislav
@ 2022-05-07 16:35 ` Jonathan Cameron
2022-05-07 16:49 ` Cosmin Tanislav
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2022-05-07 16:35 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Rob Herring, Linus Walleij, linux-iio, linux-gpio, linux-kernel,
devicetree, Cosmin Tanislav
> >
> >> +static int ad4130_set_fifo_watermark(struct iio_dev *indio_dev, unsigned int val)
> >> +{
> >> + struct ad4130_state *st = iio_priv(indio_dev);
> >> + unsigned int eff;
> >> + int ret;
> >> +
> >> + if (val > AD4130_FIFO_SIZE)
> >> + return -EINVAL;
> >> +
> >> + /*
> >> + * Always set watermark to a multiple of the number of enabled channels
> >> + * to avoid making the FIFO unaligned.
> >> + */
> >> + eff = rounddown(val, st->num_enabled_channels);
> >> +
> >> + mutex_lock(&st->lock);
> >> +
> >> + ret = regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
> >> + AD4130_WATERMARK_MASK,
> >> + FIELD_PREP(AD4130_WATERMARK_MASK,
> >> + ad4130_watermark_reg_val(eff)));
> >> + if (ret)
> >> + goto out;
> >> +
> >> + st->effective_watermark = eff;
> >> + st->watermark = val;
> >
> > Hmm this is a potential inconsistency in the IIO ABI.
> > ABI docs describes watermark as being number of 'scan elements' which is
> > not the clearest text we could have gone with...
> >
> > Now I may well have made a mistake in the following as it's rather a long time
> > since I last looked at the core handling for this...
> >
> > The core treats it as number datum (which is same as a scan) when using
> > it for the main watermark attribute and also when using watermarks with the
> > kfifo (the IIO fifo is made up of objects each of which is a scan. So kfifo_len()
> > returns the number of scans.
> >
> > Looking very quickly at a few other drivers
> > adxl367 seems to use number of samples.
> > adxl372 is using number of scans.
> > bmc150 hardware seems to work on basis of frame count which I 'think' is probably scans.
> > fxls8962 uses 'samples count' which is not clearly defined in the datasheet but there
> > is an example showing that it's scans (I think)...
> > lsm6dsx - some of the fifos used with this are based on tagged data so the connection to
> > what hits the front end buffers is non obvious.
> >
> > So, not great for consistency :(
> >
> > Going forwards i think we should standardize the hardware fifo watermark on what is being
> > used for the software watermark which I believe is number of scans.
> > Not necessary much we can do about old drivers though due to risk of breaking ABI...
> > We should make the documentation clearer though.
> >
>
> I was confused too, but this seemed more logical to me at the time, and
> since you didn't say anything regarding it on ADXL367, I did it the same
> way here. I guess we can't go back and change it now on ADXL367, I'm
> sorry for this. I'll fix it.
I missed it. Review is never perfect (mine definitely aren't!)
Thinking more on the adxl367. We still have a window to fix that as
the driver isn't yet in a release kernel. Would you mind spinning a
patch to fix that one? Even if we miss the rc cycle (it's a bit tight
timing wise) we can sneak it in as an early fix in stable without
significant risk of breaking anyone's userspace.
There might be other drivers that have that interpretation we can't
fix but if we can reduce the scope of the problem by changing the adxl367
that would be great.
We should also definitely improve the docs and perhaps add a note to say
that due to need to maintain ABI, a few drivers use scans * number of channels
rather than scans.
>
> >> +
> >> +out:
> >> + mutex_unlock(&st->lock);
> >> +
> >> + return ret;
> >> +}
> >> +
> >
> >
> > ...
> >
> >> +
> >> +static int ad4130_parse_fw_channel(struct iio_dev *indio_dev,
> >> + struct fwnode_handle *child)
> >> +{
> >> + struct ad4130_state *st = iio_priv(indio_dev);
> >> + unsigned int index = indio_dev->num_channels++;
> >> + struct device *dev = &st->spi->dev;
> >> + struct ad4130_chan_info *chan_info;
> >> + struct iio_chan_spec *chan;
> >> + u32 pins[2];
> >> + int ret;
> >> +
> >> + if (index >= AD4130_MAX_CHANNELS)
> >> + return dev_err_probe(dev, -EINVAL, "Too many channels\n");
> >> +
> >> + chan = &st->chans[index];
> >> + chan_info = &st->chans_info[index];
> >> +
> >> + *chan = ad4130_channel_template;
> >> + chan->scan_type.realbits = st->chip_info->resolution;
> >> + chan->scan_type.storagebits = st->chip_info->resolution;
> >> + chan->scan_index = index;
> >> +
> >> + chan_info->slot = AD4130_INVALID_SLOT;
> >> + chan_info->setup.fs = AD4130_FS_MIN;
> >> + chan_info->initialized = true;
> >> +
> >> + ret = fwnode_property_read_u32_array(child, "diff-channels", pins,
> >> + ARRAY_SIZE(pins));
> >> + if (ret)
> >> + return ret;
> >> +
> >> + ret = ad4130_validate_diff_channels(st, pins, ARRAY_SIZE(pins));
> >> + if (ret)
> >> + return ret;
> >> +
> >> + chan->channel = pins[0];
> >> + chan->channel2 = pins[1];
> >> +
> >> + ret = ad4130_parse_fw_setup(st, child, &chan_info->setup);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + fwnode_property_read_u32(child, "adi,excitation-pin-0",
> >> + &chan_info->iout0);
> >> + if (chan_info->setup.iout0_val != AD4130_IOUT_OFF) {
> >
> > It would be slightly better to set an explicit default value here as the fact it
> > is 0 is hidden by the enum. e.g.
> > chan_info->iout0 = AD4130_IOUT_OFF;
> > fwnode_property_read_u32(child, "adi,excitation-pin-0",
> > &chan_info->iout0);
> > if (chan_info->....
> > That would save reviewers wondering what the default is and having to go
> > check the enum (and I'm lazy :)
>
> I understand the idea, but the default value for iout0 is not
> AD4130_IOUT_OFF. iout0 is the pin that iout0_val current is
> applied to, and AD4130_IOUT_OFF is a value for iout0_val.
> Look at ad4130_parse_fw_setup.
>
> For iout0, I guess I could do
> #define AD4130_AIN0 0x0
> ...
> chan_info->iout0 = AD4130_AIN0;
Oops. I got confused. Code is fine as it is. Adding the define isn't going to make
it much clearer.
>
> >> + }
> >> + }
> >> +
> >> + return ret;
> >> +}
> >> +
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver
2022-05-07 16:35 ` Jonathan Cameron
@ 2022-05-07 16:49 ` Cosmin Tanislav
2022-05-07 17:04 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Cosmin Tanislav @ 2022-05-07 16:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Rob Herring, Linus Walleij, linux-iio, linux-gpio, linux-kernel,
devicetree, Cosmin Tanislav
On 5/7/22 19:35, Jonathan Cameron wrote:
>
>>>
>>>> +static int ad4130_set_fifo_watermark(struct iio_dev *indio_dev, unsigned int val)
>>>> +{
>>>> + struct ad4130_state *st = iio_priv(indio_dev);
>>>> + unsigned int eff;
>>>> + int ret;
>>>> +
>>>> + if (val > AD4130_FIFO_SIZE)
>>>> + return -EINVAL;
>>>> +
>>>> + /*
>>>> + * Always set watermark to a multiple of the number of enabled channels
>>>> + * to avoid making the FIFO unaligned.
>>>> + */
>>>> + eff = rounddown(val, st->num_enabled_channels);
>>>> +
>>>> + mutex_lock(&st->lock);
>>>> +
>>>> + ret = regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
>>>> + AD4130_WATERMARK_MASK,
>>>> + FIELD_PREP(AD4130_WATERMARK_MASK,
>>>> + ad4130_watermark_reg_val(eff)));
>>>> + if (ret)
>>>> + goto out;
>>>> +
>>>> + st->effective_watermark = eff;
>>>> + st->watermark = val;
>>>
>>> Hmm this is a potential inconsistency in the IIO ABI.
>>> ABI docs describes watermark as being number of 'scan elements' which is
>>> not the clearest text we could have gone with...
>>>
>>> Now I may well have made a mistake in the following as it's rather a long time
>>> since I last looked at the core handling for this...
>>>
>>> The core treats it as number datum (which is same as a scan) when using
>>> it for the main watermark attribute and also when using watermarks with the
>>> kfifo (the IIO fifo is made up of objects each of which is a scan. So kfifo_len()
>>> returns the number of scans.
>>>
>>> Looking very quickly at a few other drivers
>>> adxl367 seems to use number of samples.
>>> adxl372 is using number of scans.
>>> bmc150 hardware seems to work on basis of frame count which I 'think' is probably scans.
>>> fxls8962 uses 'samples count' which is not clearly defined in the datasheet but there
>>> is an example showing that it's scans (I think)...
>>> lsm6dsx - some of the fifos used with this are based on tagged data so the connection to
>>> what hits the front end buffers is non obvious.
>>>
>>> So, not great for consistency :(
>>>
>>> Going forwards i think we should standardize the hardware fifo watermark on what is being
>>> used for the software watermark which I believe is number of scans.
>>> Not necessary much we can do about old drivers though due to risk of breaking ABI...
>>> We should make the documentation clearer though.
>>>
>>
>> I was confused too, but this seemed more logical to me at the time, and
>> since you didn't say anything regarding it on ADXL367, I did it the same
>> way here. I guess we can't go back and change it now on ADXL367, I'm
>> sorry for this. I'll fix it.
>
> I missed it. Review is never perfect (mine definitely aren't!)
>
> Thinking more on the adxl367. We still have a window to fix that as
> the driver isn't yet in a release kernel. Would you mind spinning a
> patch to fix that one? Even if we miss the rc cycle (it's a bit tight
> timing wise) we can sneak it in as an early fix in stable without
> significant risk of breaking anyone's userspace.
>
I hope Monday is not too late to do it?
I can also try to do the changes tomorrow but I don't have the hardware
anymore so I won't be able to test until I get it back, which is only
next week.
> There might be other drivers that have that interpretation we can't
> fix but if we can reduce the scope of the problem by changing the adxl367
> that would be great.
>
> We should also definitely improve the docs and perhaps add a note to say
> that due to need to maintain ABI, a few drivers use scans * number of channels
> rather than scans.
I guess I could also do that at the same time.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver
2022-05-07 16:49 ` Cosmin Tanislav
@ 2022-05-07 17:04 ` Jonathan Cameron
2022-05-14 13:32 ` Cosmin Tanislav
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2022-05-07 17:04 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Rob Herring, Linus Walleij, linux-iio, linux-gpio, linux-kernel,
devicetree, Cosmin Tanislav
On Sat, 7 May 2022 19:49:17 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:
> On 5/7/22 19:35, Jonathan Cameron wrote:
> >
> >>>
> >>>> +static int ad4130_set_fifo_watermark(struct iio_dev *indio_dev, unsigned int val)
> >>>> +{
> >>>> + struct ad4130_state *st = iio_priv(indio_dev);
> >>>> + unsigned int eff;
> >>>> + int ret;
> >>>> +
> >>>> + if (val > AD4130_FIFO_SIZE)
> >>>> + return -EINVAL;
> >>>> +
> >>>> + /*
> >>>> + * Always set watermark to a multiple of the number of enabled channels
> >>>> + * to avoid making the FIFO unaligned.
> >>>> + */
> >>>> + eff = rounddown(val, st->num_enabled_channels);
> >>>> +
> >>>> + mutex_lock(&st->lock);
> >>>> +
> >>>> + ret = regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
> >>>> + AD4130_WATERMARK_MASK,
> >>>> + FIELD_PREP(AD4130_WATERMARK_MASK,
> >>>> + ad4130_watermark_reg_val(eff)));
> >>>> + if (ret)
> >>>> + goto out;
> >>>> +
> >>>> + st->effective_watermark = eff;
> >>>> + st->watermark = val;
> >>>
> >>> Hmm this is a potential inconsistency in the IIO ABI.
> >>> ABI docs describes watermark as being number of 'scan elements' which is
> >>> not the clearest text we could have gone with...
> >>>
> >>> Now I may well have made a mistake in the following as it's rather a long time
> >>> since I last looked at the core handling for this...
> >>>
> >>> The core treats it as number datum (which is same as a scan) when using
> >>> it for the main watermark attribute and also when using watermarks with the
> >>> kfifo (the IIO fifo is made up of objects each of which is a scan. So kfifo_len()
> >>> returns the number of scans.
> >>>
> >>> Looking very quickly at a few other drivers
> >>> adxl367 seems to use number of samples.
> >>> adxl372 is using number of scans.
> >>> bmc150 hardware seems to work on basis of frame count which I 'think' is probably scans.
> >>> fxls8962 uses 'samples count' which is not clearly defined in the datasheet but there
> >>> is an example showing that it's scans (I think)...
> >>> lsm6dsx - some of the fifos used with this are based on tagged data so the connection to
> >>> what hits the front end buffers is non obvious.
> >>>
> >>> So, not great for consistency :(
> >>>
> >>> Going forwards i think we should standardize the hardware fifo watermark on what is being
> >>> used for the software watermark which I believe is number of scans.
> >>> Not necessary much we can do about old drivers though due to risk of breaking ABI...
> >>> We should make the documentation clearer though.
> >>>
> >>
> >> I was confused too, but this seemed more logical to me at the time, and
> >> since you didn't say anything regarding it on ADXL367, I did it the same
> >> way here. I guess we can't go back and change it now on ADXL367, I'm
> >> sorry for this. I'll fix it.
> >
> > I missed it. Review is never perfect (mine definitely aren't!)
> >
> > Thinking more on the adxl367. We still have a window to fix that as
> > the driver isn't yet in a release kernel. Would you mind spinning a
> > patch to fix that one? Even if we miss the rc cycle (it's a bit tight
> > timing wise) we can sneak it in as an early fix in stable without
> > significant risk of breaking anyone's userspace.
> >
>
> I hope Monday is not too late to do it?
Any time next week should be fine. If it ends up later that's fine as well.
We have at least a few weeks until the 5.18 release and then if we were to land
this during the first few weeks of the next cycle that would be fine as well.
No one should be insane enough to not pick up at least the first few stable
releases of a new kernel!
> I can also try to do the changes tomorrow but I don't have the hardware
> anymore so I won't be able to test until I get it back, which is only
> next week.
>
> > There might be other drivers that have that interpretation we can't
> > fix but if we can reduce the scope of the problem by changing the adxl367
> > that would be great.
> >
> > We should also definitely improve the docs and perhaps add a note to say
> > that due to need to maintain ABI, a few drivers use scans * number of channels
> > rather than scans.
>
> I guess I could also do that at the same time.
Perfect :)
Thanks for sorting this out.
Jonathan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver
2022-05-07 17:04 ` Jonathan Cameron
@ 2022-05-14 13:32 ` Cosmin Tanislav
2022-05-14 16:24 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Cosmin Tanislav @ 2022-05-14 13:32 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Rob Herring, Linus Walleij, linux-iio, linux-gpio, linux-kernel,
devicetree, Cosmin Tanislav
On 5/7/22 20:04, Jonathan Cameron wrote:
> On Sat, 7 May 2022 19:49:17 +0300
> Cosmin Tanislav <demonsingur@gmail.com> wrote:
>
>> On 5/7/22 19:35, Jonathan Cameron wrote:
>>>
>>>>>
>>>>>> +static int ad4130_set_fifo_watermark(struct iio_dev *indio_dev, unsigned int val)
>>>>>> +{
>>>>>> + struct ad4130_state *st = iio_priv(indio_dev);
>>>>>> + unsigned int eff;
>>>>>> + int ret;
>>>>>> +
>>>>>> + if (val > AD4130_FIFO_SIZE)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + /*
>>>>>> + * Always set watermark to a multiple of the number of enabled channels
>>>>>> + * to avoid making the FIFO unaligned.
>>>>>> + */
>>>>>> + eff = rounddown(val, st->num_enabled_channels);
>>>>>> +
>>>>>> + mutex_lock(&st->lock);
>>>>>> +
>>>>>> + ret = regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
>>>>>> + AD4130_WATERMARK_MASK,
>>>>>> + FIELD_PREP(AD4130_WATERMARK_MASK,
>>>>>> + ad4130_watermark_reg_val(eff)));
>>>>>> + if (ret)
>>>>>> + goto out;
>>>>>> +
>>>>>> + st->effective_watermark = eff;
>>>>>> + st->watermark = val;
>>>>>
>>>>> Hmm this is a potential inconsistency in the IIO ABI.
>>>>> ABI docs describes watermark as being number of 'scan elements' which is
>>>>> not the clearest text we could have gone with...
>>>>>
>>>>> Now I may well have made a mistake in the following as it's rather a long time
>>>>> since I last looked at the core handling for this...
>>>>>
>>>>> The core treats it as number datum (which is same as a scan) when using
>>>>> it for the main watermark attribute and also when using watermarks with the
>>>>> kfifo (the IIO fifo is made up of objects each of which is a scan. So kfifo_len()
>>>>> returns the number of scans.
>>>>>
>>>>> Looking very quickly at a few other drivers
>>>>> adxl367 seems to use number of samples.
>>>>> adxl372 is using number of scans.
>>>>> bmc150 hardware seems to work on basis of frame count which I 'think' is probably scans.
>>>>> fxls8962 uses 'samples count' which is not clearly defined in the datasheet but there
>>>>> is an example showing that it's scans (I think)...
>>>>> lsm6dsx - some of the fifos used with this are based on tagged data so the connection to
>>>>> what hits the front end buffers is non obvious.
>>>>>
>>>>> So, not great for consistency :(
>>>>>
>>>>> Going forwards i think we should standardize the hardware fifo watermark on what is being
>>>>> used for the software watermark which I believe is number of scans.
>>>>> Not necessary much we can do about old drivers though due to risk of breaking ABI...
>>>>> We should make the documentation clearer though.
>>>>>
>>>>
>>>> I was confused too, but this seemed more logical to me at the time, and
>>>> since you didn't say anything regarding it on ADXL367, I did it the same
>>>> way here. I guess we can't go back and change it now on ADXL367, I'm
>>>> sorry for this. I'll fix it.
>>>
>>> I missed it. Review is never perfect (mine definitely aren't!)
>>>
>>> Thinking more on the adxl367. We still have a window to fix that as
>>> the driver isn't yet in a release kernel. Would you mind spinning a
>>> patch to fix that one? Even if we miss the rc cycle (it's a bit tight
>>> timing wise) we can sneak it in as an early fix in stable without
>>> significant risk of breaking anyone's userspace.
>>>
>>
>> I hope Monday is not too late to do it?
>
> Any time next week should be fine. If it ends up later that's fine as well.
> We have at least a few weeks until the 5.18 release and then if we were to land
> this during the first few weeks of the next cycle that would be fine as well.
> No one should be insane enough to not pick up at least the first few stable
> releases of a new kernel!
>
>> I can also try to do the changes tomorrow but I don't have the hardware
>> anymore so I won't be able to test until I get it back, which is only
>> next week.
>>
>>> There might be other drivers that have that interpretation we can't
>>> fix but if we can reduce the scope of the problem by changing the adxl367
>>> that would be great.
>>>
>>> We should also definitely improve the docs and perhaps add a note to say
>>> that due to need to maintain ABI, a few drivers use scans * number of channels
>>> rather than scans.
>>
>> I guess I could also do that at the same time.
>
> Perfect :)
>
> Thanks for sorting this out.
>
> Jonathan
>
>
I've just had another good look at ADXL367. It seems that I'm quick at
forgetting stuff about the code I write.
In adxl367_set_fifo_samples(), fifo_watermark is multiplied by
fifo_set_size, then, if it is larger than the maximum watermark,
it is capped at the maximum watermark, and then, it is divided
by fifo_set_size again.
In the end, fifo_watermark actually seems to mean number of scans
in that driver too.
So this was a huge confusion. The different thing about AD4130 is that
the register actually means number of samples, and not number of scans,
so that's where that confussion stemmed from.
Sorry for wasting your time on this.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver
2022-05-14 13:32 ` Cosmin Tanislav
@ 2022-05-14 16:24 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2022-05-14 16:24 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Rob Herring, Linus Walleij, linux-iio, linux-gpio, linux-kernel,
devicetree, Cosmin Tanislav
On Sat, 14 May 2022 16:32:25 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:
> On 5/7/22 20:04, Jonathan Cameron wrote:
> > On Sat, 7 May 2022 19:49:17 +0300
> > Cosmin Tanislav <demonsingur@gmail.com> wrote:
> >
> >> On 5/7/22 19:35, Jonathan Cameron wrote:
> >>>
> >>>>>
> >>>>>> +static int ad4130_set_fifo_watermark(struct iio_dev *indio_dev, unsigned int val)
> >>>>>> +{
> >>>>>> + struct ad4130_state *st = iio_priv(indio_dev);
> >>>>>> + unsigned int eff;
> >>>>>> + int ret;
> >>>>>> +
> >>>>>> + if (val > AD4130_FIFO_SIZE)
> >>>>>> + return -EINVAL;
> >>>>>> +
> >>>>>> + /*
> >>>>>> + * Always set watermark to a multiple of the number of enabled channels
> >>>>>> + * to avoid making the FIFO unaligned.
> >>>>>> + */
> >>>>>> + eff = rounddown(val, st->num_enabled_channels);
> >>>>>> +
> >>>>>> + mutex_lock(&st->lock);
> >>>>>> +
> >>>>>> + ret = regmap_update_bits(st->regmap, AD4130_REG_FIFO_CONTROL,
> >>>>>> + AD4130_WATERMARK_MASK,
> >>>>>> + FIELD_PREP(AD4130_WATERMARK_MASK,
> >>>>>> + ad4130_watermark_reg_val(eff)));
> >>>>>> + if (ret)
> >>>>>> + goto out;
> >>>>>> +
> >>>>>> + st->effective_watermark = eff;
> >>>>>> + st->watermark = val;
> >>>>>
> >>>>> Hmm this is a potential inconsistency in the IIO ABI.
> >>>>> ABI docs describes watermark as being number of 'scan elements' which is
> >>>>> not the clearest text we could have gone with...
> >>>>>
> >>>>> Now I may well have made a mistake in the following as it's rather a long time
> >>>>> since I last looked at the core handling for this...
> >>>>>
> >>>>> The core treats it as number datum (which is same as a scan) when using
> >>>>> it for the main watermark attribute and also when using watermarks with the
> >>>>> kfifo (the IIO fifo is made up of objects each of which is a scan. So kfifo_len()
> >>>>> returns the number of scans.
> >>>>>
> >>>>> Looking very quickly at a few other drivers
> >>>>> adxl367 seems to use number of samples.
> >>>>> adxl372 is using number of scans.
> >>>>> bmc150 hardware seems to work on basis of frame count which I 'think' is probably scans.
> >>>>> fxls8962 uses 'samples count' which is not clearly defined in the datasheet but there
> >>>>> is an example showing that it's scans (I think)...
> >>>>> lsm6dsx - some of the fifos used with this are based on tagged data so the connection to
> >>>>> what hits the front end buffers is non obvious.
> >>>>>
> >>>>> So, not great for consistency :(
> >>>>>
> >>>>> Going forwards i think we should standardize the hardware fifo watermark on what is being
> >>>>> used for the software watermark which I believe is number of scans.
> >>>>> Not necessary much we can do about old drivers though due to risk of breaking ABI...
> >>>>> We should make the documentation clearer though.
> >>>>>
> >>>>
> >>>> I was confused too, but this seemed more logical to me at the time, and
> >>>> since you didn't say anything regarding it on ADXL367, I did it the same
> >>>> way here. I guess we can't go back and change it now on ADXL367, I'm
> >>>> sorry for this. I'll fix it.
> >>>
> >>> I missed it. Review is never perfect (mine definitely aren't!)
> >>>
> >>> Thinking more on the adxl367. We still have a window to fix that as
> >>> the driver isn't yet in a release kernel. Would you mind spinning a
> >>> patch to fix that one? Even if we miss the rc cycle (it's a bit tight
> >>> timing wise) we can sneak it in as an early fix in stable without
> >>> significant risk of breaking anyone's userspace.
> >>>
> >>
> >> I hope Monday is not too late to do it?
> >
> > Any time next week should be fine. If it ends up later that's fine as well.
> > We have at least a few weeks until the 5.18 release and then if we were to land
> > this during the first few weeks of the next cycle that would be fine as well.
> > No one should be insane enough to not pick up at least the first few stable
> > releases of a new kernel!
> >
> >> I can also try to do the changes tomorrow but I don't have the hardware
> >> anymore so I won't be able to test until I get it back, which is only
> >> next week.
> >>
> >>> There might be other drivers that have that interpretation we can't
> >>> fix but if we can reduce the scope of the problem by changing the adxl367
> >>> that would be great.
> >>>
> >>> We should also definitely improve the docs and perhaps add a note to say
> >>> that due to need to maintain ABI, a few drivers use scans * number of channels
> >>> rather than scans.
> >>
> >> I guess I could also do that at the same time.
> >
> > Perfect :)
> >
> > Thanks for sorting this out.
> >
> > Jonathan
> >
> >
>
> I've just had another good look at ADXL367. It seems that I'm quick at
> forgetting stuff about the code I write.
>
> In adxl367_set_fifo_samples(), fifo_watermark is multiplied by
> fifo_set_size, then, if it is larger than the maximum watermark,
> it is capped at the maximum watermark, and then, it is divided
> by fifo_set_size again.
>
> In the end, fifo_watermark actually seems to mean number of scans
> in that driver too.
>
> So this was a huge confusion. The different thing about AD4130 is that
> the register actually means number of samples, and not number of scans,
> so that's where that confussion stemmed from.
>
> Sorry for wasting your time on this.
No problem and thanks for looking into this in more depth!
Jonathan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130
2022-04-26 0:37 ` Rob Herring
2022-04-27 12:47 ` Cosmin Tanislav
@ 2022-06-08 8:31 ` Cosmin Tanislav
1 sibling, 0 replies; 15+ messages in thread
From: Cosmin Tanislav @ 2022-06-08 8:31 UTC (permalink / raw)
To: Rob Herring
Cc: Jonathan Cameron, Linus Walleij, linux-iio, linux-gpio,
linux-kernel, devicetree, Cosmin Tanislav
On 4/26/22 03:37, Rob Herring wrote:
> On Tue, Apr 19, 2022 at 06:08:27PM +0300, Cosmin Tanislav wrote:
>> AD4130-8 is an ultra-low power, high precision, measurement solution for
>> low bandwidth battery operated applications.
>>
>> The fully integrated AFE (Analog Front-End) includes a multiplexer for up
>> to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
>> Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
>> selectable filter options, smart sequencer, sensor biasing and excitation
>> options, diagnostics, and a FIFO buffer.
>>
>> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
>> ---
>> .../bindings/iio/adc/adi,ad4130.yaml | 264 ++++++++++++++++++
>> 1 file changed, 264 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
>> new file mode 100644
>> index 000000000000..32996b62cd20
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
>> @@ -0,0 +1,264 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +# Copyright 2022 Analog Devices Inc.
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4130.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Analog Devices AD4130 ADC device driver
>> +
>> +maintainers:
>> + - Cosmin Tanislav <cosmin.tanislav@analog.com>
>> +
>> +description: |
>> + Bindings for the Analog Devices AD4130 ADC. Datasheet can be found here:
>> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4130-8.pdf
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - adi,ad4130-8-16-lfcsp
>> + - adi,ad4130-8-16-wlcsp
>> + - adi,ad4130-8-24-lfcsp
>> + - adi,ad4130-8-24-wlcsp
>
> What is lfcsp? wlcsp seems to be the package type which generally
> shouldn't be part of the compatible.
>
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + clocks:
>> + maxItems: 1
>> + description: phandle to the master clock (mclk)
>> +
>> + clock-names:
>> + items:
>> + - const: mclk
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + interrupt-names:
>> + description: |
>
> Don't need '|' if there is no formatting to preserve.
>
>> + Specify which interrupt pin should be configured as Data Ready / FIFO
>> + interrupt.
>> + Default if not supplied is dout-int.
>
> default: dout-int
>
By the way. interrupt-names cannot have a default value.
Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml:
properties:interrupt-names: Additional properties are not allowed
('default' was unexpected)
from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
>> + enum:
>> + - dout-int
>> + - clk
>> + - p2
>> + - dout
>> +
>> + '#address-cells':
>> + const: 1
>> +
>> + '#size-cells':
>> + const: 0
>> +
>> + refin1-supply:
>> + description: refin1 supply. Can be used as reference for conversion.
>> +
>> + refin2-supply:
>> + description: refin2 supply. Can be used as reference for conversion.
>> +
>> + avdd-supply:
>> + description: AVDD voltage supply. Can be used as reference for conversion.
>> +
>> + iovdd-supply:
>> + description: IOVDD voltage supply. Used for the chip interface.
>> +
>> + spi-max-frequency:
>> + maximum: 5000000
>> +
>> + adi,int-clk-out:
>> + description: Specify if the internal clock should be exposed on the CLK pin.
>> + type: boolean
>> +
>> + adi,ext-clk-freq:
>> + description: Specify the frequency of the external clock.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [76800, 153600]
>> + default: 76800
>> +
>> + adi,bipolar:
>> + description: Specify if the device should be used in bipolar mode.
>> + type: boolean
>> +
>> + adi,vbias-pins:
>> + description: Analog inputs to apply a voltage bias of (AVDD − AVSS) / 2 to.
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + maxItems: 16
>> + items:
>> + minimum: 0
>> + maximum: 15
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> +
>> +patternProperties:
>> + "^channel@([0-9]|1[0-5])$":
>> + type: object
>> + $ref: adc.yaml
>
> unevaluatedProperties: false
>
>> +
>> + properties:
>> + reg:
>> + description: |
>> + The channel number.
>> + items:
>> + minimum: 0
>> + maximum: 15
>> +
>> + diff-channels:
>> + description: |
>> + Besides the analog inputs available, internal inputs can be used.
>> + 16: Internal temperature sensor.
>> + 17: AVSS
>> + 18: Internal reference
>> + 19: DGND
>> + 20: (AVDD − AVSS)/6+
>> + 21: (AVDD − AVSS)/6-
>> + 22: (IOVDD − DGND)/6+
>> + 23: (IOVDD − DGND)/6-
>> + 24: (ALDO − AVSS)/6+
>> + 25: (ALDO − AVSS)/6-
>> + 26: (DLDO − DGND)/6+
>> + 27: (DLDO − DGND)/6-
>> + 28: V_MV_P
>> + 29: V_MV_M
>> + items:
>> + minimum: 0
>> + maximum: 29
>> +
>> + adi,reference-select:
>> + description: |
>> + Select the reference source to use when converting on the
>> + specific channel. Valid values are:
>> + 0: REFIN1(+)/REFIN1(−)
>> + 1: REFIN2(+)/REFIN2(−)
>> + 2: REFOUT/AVSS (Internal reference)
>> + 3: AVDD/AVSS
>> + If not specified, REFIN1 is used.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [0, 1, 2, 3]
>> + default: 0
>> +
>> + adi,excitation-pin-0:
>> + description: |
>> + Analog input to apply excitation current to while the channel
>> + is active.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 0
>> + maximum: 15
>> + default: 0
>> +
>> + adi,excitation-pin-1:
>> + description: |
>> + Analog input to apply excitation current to while this channel
>> + is active.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 0
>> + maximum: 15
>> + default: 0
>> +
>> + adi,excitation-current-0-nanoamps:
>> + description: |
>> + Excitation current in nanoamps to be applied to pin specified in
>> + adi,excitation-pin-0 while this channel is active.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [0, 100, 10000, 20000, 50000, 100000, 150000, 200000]
>> + default: 0
>> +
>> + adi,excitation-current-1-nanoamps:
>> + description: |
>> + Excitation current in nanoamps to be applied to pin specified in
>> + adi,excitation-pin-1 while this channel is active.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [0, 100, 10000, 20000, 50000, 100000, 150000, 200000]
>> + default: 0
>> +
>> + adi,burnout-current-nanoamps:
>> + description: |
>> + Burnout current in nanoamps to be applied for this channel.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [0, 500, 2000, 4000]
>> + default: 0
>> +
>> + adi,buffered-positive:
>> + description: Enable buffered mode for positive input.
>> + type: boolean
>> +
>> + adi,buffered-negative:
>> + description: Enable buffered mode for negative input.
>> + type: boolean
>> +
>> + required:
>> + - reg
>> + - diff-channels
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> + spi {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + adc@0 {
>> + compatible = "adi,ad4130-8-24-wlcsp";
>> + reg = <0>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + spi-max-frequency = <5000000>;
>> + interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
>> + interrupt-parent = <&gpio>;
>> +
>> + channel@0 {
>> + reg = <0>;
>> +
>> + adi,reference-select = <2>;
>> +
>> + /* AIN8, AIN9 */
>> + diff-channels = <8 9>;
>> + };
>> +
>> + channel@1 {
>> + reg = <1>;
>> +
>> + adi,reference-select = <2>;
>> +
>> + /* AIN10, AIN11 */
>> + diff-channels = <10 11>;
>> + };
>> +
>> + channel@2 {
>> + reg = <2>;
>> +
>> + adi,reference-select = <2>;
>> +
>> + /* Temperature Sensor, DGND */
>> + diff-channels = <16 19>;
>> + };
>> +
>> + channel@3 {
>> + reg = <3>;
>> +
>> + adi,reference-select = <2>;
>> +
>> + /* Internal reference, DGND */
>> + diff-channels = <18 19>;
>> + };
>> +
>> + channel@4 {
>> + reg = <4>;
>> +
>> + adi,reference-select = <2>;
>> +
>> + /* DGND, DGND */
>> + diff-channels = <19 19>;
>> + };
>> + };
>> + };
>> --
>> 2.35.3
>>
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-06-08 9:11 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-19 15:08 [PATCH v3 0/2] AD4130 Cosmin Tanislav
2022-04-19 15:08 ` [PATCH v3 1/2] dt-bindings: iio: adc: add AD4130 Cosmin Tanislav
2022-04-26 0:37 ` Rob Herring
2022-04-27 12:47 ` Cosmin Tanislav
2022-04-27 21:41 ` Rob Herring
2022-06-08 8:31 ` Cosmin Tanislav
2022-05-01 16:09 ` Jonathan Cameron
2022-04-19 15:08 ` [PATCH v3 2/2] iio: adc: ad4130: add AD4130 driver Cosmin Tanislav
2022-05-01 16:08 ` Jonathan Cameron
2022-05-02 8:53 ` Cosmin Tanislav
2022-05-07 16:35 ` Jonathan Cameron
2022-05-07 16:49 ` Cosmin Tanislav
2022-05-07 17:04 ` Jonathan Cameron
2022-05-14 13:32 ` Cosmin Tanislav
2022-05-14 16:24 ` 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).