This patch series adds support for the Analog Devices AD4111, AD4112,
AD4114, AD4115, AD4116 within the existing AD7173 driver.
The AD411X family encompasses a series of low power, low noise, 24-bit,
sigma-delta analog-to-digital converters that offer a versatile range of
specifications. They integrate an analog front end suitable for processing
fully differential/single-ended and bipolar voltage inputs.
Particularities of the models:
- All ADCs have inputs with a precision voltage divider with a division
ratio of 10.
- AD4116 has 5 low level inputs without a voltage divider.
- AD4111 and AD4112 support current inputs (0 mA to 20 mA) using a 50ohm
shunt resistor.
Discussions from this patch series have concluded with:
-Datasheets mention single-ended and pseudo differential capabilities by
the means of connecting the negative input of a differential pair (IN-)
to a constant voltage supply and letting the positive input fluctuate.
This is not a special operating mode, it is a capability of the
differential channels to also measure such signals.
-Single-ended and pseudo differential do not need any specific
configuration and cannot be differentiated from differential usage by
the driver side =>
offer adi,channel-type attribute to flag the usage of the channel
-VINCOM is described as a dedicated pin for single-ended channels but as
seen in AD4116, it is a normal input connected to the cross-point
multiplexer (VIN10, VINCOM (single-ended or differential pair)).
This does not mean full functionality in any configuration:
AD4111:"If any two voltage inputs are paired in a configuration other
than what is described in this data sheet, the accuracy of the device
cannot be guaranteed".
-ADCIN15 input pin from AD4116 is specified as the dedicated pin for
pseudo-differential but from the datasheet it results that this pin is
also able to measure single-ended and fully differential channels
("ADCIN11, ADCIN15. (pseudo differential or differential pair)";
"An example is to connect the ADCIN15 pin externally to the AVSS
pin in a single-ended configuration")
As such, detecting the type of usage of a channel is not possible and
will be the responsibility of the user to specify.
If the user has connected a non 0V (in regards to AVSS) supply to
the negative input pin of a channel in a pseudo differential
configuration, the offset of the measurement from AVSS will not be known
from the driver and will need to be measured by other means.
Datasheets:
https://www.analog.com/media/en/technical-documentation/data-sheets/AD4111.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/AD4112.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/AD4114.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/AD4115.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/AD4116.pdf
This series depends on patches:
(iio: adc: ad7173: Use device_for_each_child_node_scoped() to simplify error paths.)
https://lore.kernel.org/all/[email protected]
(dt-bindings: iio: adc: Add single-channel property)
https://lore.kernel.org/linux-iio/[email protected]/
And patch series:
(AD7173 fixes)
https://lore.kernel.org/all/[email protected]/
Signed-off-by: Dumitru Ceclan <[email protected]>
---
Changes in v4:
dt-bindings: adc: ad7173: add support for ad411x
- remove "adi,channel-type"
- add "adi,common-mode-input" and "adi,current-channel" to use
single-channel with both single-ended/pseudo-differential and current
channels
- add restrictions to patternProperties channel to restrict presence of
both diff-channels and single-channel
and
both "adi,current-channel" and "adi,common-mode-input"
- update diff-channels description
- update commit message to current state of the binding
- add a second example to exemplify single-ended and current channels
iio: adc: ad7173: refactor channel configuration parsing
- picked up reviewed-by tag
iio: adc: ad7173: refactor ain and vref selection
- Moved reference error message from validate_reference() to
ad7173_get_ref_voltage_milli()
- Changed from dev_err_probe to dev_err as function can be reached from
non probe paths
- added AD7173_NO_AINS_PER_CHANNEL to remove ambiguity when using the
size of the ain array
iio: adc: ad7173: add support for special inputs
- picked up reviewed-by tag
iio: adc: ad7173: refactor device info structs
- create patch
iio: adc: ad7173: Add support for AD411x devices
- separate chip id defines for ad411<1,2,4>
- fix device_info typos: voltage, VINCOM
- rename num_voltage_inputs and num_voltage_inputs_with_divider to *_voltage_in*
- subtract ch->cfg.bipolar directly
- change to const ain argument in *_validate_current_ain()
- change parsing to new dt structure for current and single-ended channels
- drop adi,channel-type
- refactor device info structs as the previous patch
iio: adc: ad7173: Reduce device info struct size
- remove patch as suggested by David Lechner as it would increase binary
size more than the savings done in RAM
- Link to v3: https://lore.kernel.org/r/[email protected]
Changes in v3:
iio: adc: ad7173: fix buffers enablement for ad7176-2
iio: adc: ad7173: Add ad7173_device_info names
iio: adc: ad7173: Remove index from temp channel
- Remove patches, send in separate series
dt-bindings: adc: ad7173: add support for ad411x
- fix VINCOM typo
- switch current channel definition to use single-channel
- remove pseudo-differential from adi,channel-type, specify that
single-ended will be used for that case as well
- remove diff-channels from required, already defined in adc.yaml
- update constraints to not permit single-channel for models without
current inputs
iio: adc: ad7173: refactor channel configuration parsing
- remove blank line from commit message
iio: adc: ad7173: refactor ain and vref selection
- remove blank space from commit message
iio: adc: ad7173: add support for special inputs
<no changes>
iio: adc: ad7173: Add support for AD411x devices
- remove pseudo diff channel type
- change current channels dt parsing to single-channel
- change module description from wild-card to "and similar"
- add comment to document HW behavior when multiple channels are enabled
in buffered reading mode
iio: adc: ad7173: Reduce device info struct size
<no changes>
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
dt-bindings: adc: ad7173: add support for ad411x
- Add constraint for missing avdd2-supply on ad411x
- Change support for current channels to selecting the actual
diff-channels input values and activating the
adi,current-channel property
- Add constraint for adi,current-channel
- Add adi,channel-type to be able to differentiante in the driver
between single-ended, pseudo-differential and differential channels.
- Update diff-channels description to decribe inputs beside the AINs
iio: adc: ad7173: fix buffers enablement for ad7176-2
- Specify ".has_input_buf = false" for AD7176-2
- Drop fixes tag, specify that configuration bits are read only
iio: adc: ad7173: refactor channel configuration parsing
- Add Link and Suggested-by in commit message
iio: adc: ad7173: refactor ain and vref selection
- Improve commit message to express commit purpose
- Refactor line wrappings due to reduced indent
- Change AINs check to a loop
iio: adc: ad7173: add support for special inputs
- Create patch
iio: adc: ad7173: Add ad7173_device_info names
- Create patch
iio: adc: ad7173: Remove index from temp channel
- Justify in commit message userspace breakage
- Remove index from the correct channel template
iio: adc: ad7173: Add support for AD411x devices
- Add missing validation for VCOM and inputs with voltage divider
- Add missing validation for AD4116 low level inputs
- Add missing ad7173_device_info names
- Add support for setting differential flag depending on the channel type
- Change current channel validation to use actual pin values
- Combine multiple chipID reg values in a single define
(AD7173_AD4111_AD4112_AD4114_ID)
- Rename num_inputs and num_inputs_with_divider to include voltage
- Add comment to specify that num_voltage_inputs_with_divider does not
include the VCOM pin.
- Change break to direct returns where possible in switch cases
- Add fix for ad411x gpio's
iio: adc: ad7173: Reduce device info struct size
- Create patch
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Dumitru Ceclan (6):
dt-bindings: adc: ad7173: add support for ad411x
iio: adc: ad7173: refactor channel configuration parsing
iio: adc: ad7173: refactor ain and vref selection
iio: adc: ad7173: add support for special inputs
iio: adc: ad7173: refactor device info structs
iio: adc: ad7173: Add support for AD411x devices
.../devicetree/bindings/iio/adc/adi,ad7173.yaml | 192 +++++-
drivers/iio/adc/ad7173.c | 682 +++++++++++++++------
2 files changed, 691 insertions(+), 183 deletions(-)
---
base-commit: 02b664413a44903ef349bb70a3f1842cbcee9616
change-id: 20240312-ad4111-7eeb34eb4a5f
Best regards,
--
Dumitru Ceclan <[email protected]>
From: Dumitru Ceclan <[email protected]>
Move configurations regarding number of channels from
*_fw_parse_device_config to *_fw_parse_channel_config.
Suggested-by: Jonathan Cameron <[email protected]>
Link: https://lore.kernel.org/all/20240303162148.3ad91aa2@jic23-huawei/
Reviewed-by: David Lechner <[email protected]>
Reviewed-by: Nuno Sa <[email protected]>
Signed-off-by: Dumitru Ceclan <[email protected]>
---
drivers/iio/adc/ad7173.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 638e2468efbf..6e249628bc64 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -912,7 +912,23 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
struct device *dev = indio_dev->dev.parent;
struct iio_chan_spec *chan_arr, *chan;
unsigned int ain[2], chan_index = 0;
- int ref_sel, ret;
+ int ref_sel, ret, num_channels;
+
+ num_channels = device_get_child_node_count(dev);
+
+ if (st->info->has_temp)
+ num_channels++;
+
+ if (num_channels == 0)
+ return dev_err_probe(dev, -ENODATA, "No channels specified\n");
+
+ if (num_channels > st->info->num_channels)
+ return dev_err_probe(dev, -EINVAL,
+ "Too many channels specified. Maximum is %d, not including temperature channel if supported.\n",
+ st->info->num_channels);
+
+ indio_dev->num_channels = num_channels;
+ st->num_channels = num_channels;
chan_arr = devm_kcalloc(dev, sizeof(*indio_dev->channels),
st->num_channels, GFP_KERNEL);
@@ -1007,7 +1023,6 @@ static int ad7173_fw_parse_device_config(struct iio_dev *indio_dev)
{
struct ad7173_state *st = iio_priv(indio_dev);
struct device *dev = indio_dev->dev.parent;
- unsigned int num_channels;
int ret;
st->regulators[0].supply = ad7173_ref_sel_str[AD7173_SETUP_REF_SEL_EXT_REF];
@@ -1066,16 +1081,6 @@ static int ad7173_fw_parse_device_config(struct iio_dev *indio_dev)
ad7173_sigma_delta_info.irq_line = ret;
- num_channels = device_get_child_node_count(dev);
-
- if (st->info->has_temp)
- num_channels++;
-
- if (num_channels == 0)
- return dev_err_probe(dev, -ENODATA, "No channels specified\n");
- indio_dev->num_channels = num_channels;
- st->num_channels = num_channels;
-
return ad7173_fw_parse_channel_config(indio_dev);
}
--
2.43.0
From: Dumitru Ceclan <[email protected]>
Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
AD411x family ADCs support a VCOM pin. The purpose of this pin is to
offer a dedicated common-mode voltage input for single-ended channels.
This pin is specified as supporting a differential channel with VIN10 on
model AD4116.
AD4111/AD4112 support current channels. Support is implemented using
single-channel and "adi,current-channel".
Signed-off-by: Dumitru Ceclan <[email protected]>
---
.../devicetree/bindings/iio/adc/adi,ad7173.yaml | 192 ++++++++++++++++++++-
1 file changed, 190 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
index ea6cfcd0aff4..d8474eee553e 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
@@ -19,7 +19,18 @@ description: |
primarily for measurement of signals close to DC but also delivers
outstanding performance with input bandwidths out to ~10kHz.
+ Analog Devices AD411x ADC's:
+ The AD411X family encompasses a series of low power, low noise, 24-bit,
+ sigma-delta analog-to-digital converters that offer a versatile range of
+ specifications. They integrate an analog front end suitable for processing
+ fully differential/single-ended and bipolar voltage inputs.
+
Datasheets for supported chips:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD4111.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD4112.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD4114.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD4115.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD4116.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-4.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
@@ -31,6 +42,11 @@ description: |
properties:
compatible:
enum:
+ - adi,ad4111
+ - adi,ad4112
+ - adi,ad4114
+ - adi,ad4115
+ - adi,ad4116
- adi,ad7172-2
- adi,ad7172-4
- adi,ad7173-8
@@ -129,10 +145,54 @@ patternProperties:
maximum: 15
diff-channels:
+ description: |
+ This property is used for defining the inputs of a differential
+ voltage channel. The first value is the positive input and the second
+ value is the negative input of the channel.
+
+ Family AD411x supports a dedicated VINCOM voltage input.
+ To select it set the second channel to 16.
+ (VIN2, VINCOM) -> diff-channels = <2 16>
+
+ There are special values that can be selected besides the voltage
+ analog inputs:
+ 21: REF+
+ 22: REF−
+ Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
+ 19: ((AVDD1 − AVSS)/5)+
+ 20: ((AVDD1 − AVSS)/5)−
+
items:
minimum: 0
maximum: 31
+ single-channel:
+ description: |
+ This property is used for defining a current channel or the positive
+ input of a voltage channel (single-ended or pseudo-differential).
+
+ Models AD4111 and AD4112 support current channels.
+ Example: (IIN2+, IIN2−) -> single-channel = <2>
+ To correctly configure a current channel set the "adi,current-channel"
+ property to true.
+
+ To configure a single-ended/pseudo-differential channel set the
+ "adi,common-mode-channel" property to the desired negative voltage input.
+
+ When used as a voltage channel, special inputs are valid as well.
+ minimum: 0
+ maximum: 31
+
+ adi,common-mode-channel:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ This property is used for defining the negative input of a
+ single-ended or pseudo-differential voltage channel.
+
+ Special inputs are valid as well.
+ minimum: 0
+ maximum: 31
+
adi,reference-select:
description: |
Select the reference source to use when converting on
@@ -154,9 +214,31 @@ patternProperties:
- avdd
default: refout-avss
+ adi,current-channel:
+ description: |
+ Signal that the selected inputs are current channels.
+ Only available on AD4111 and AD4112.
+ type: boolean
+
required:
- reg
- - diff-channels
+
+ allOf:
+ - oneOf:
+ - required: [single-channel]
+ properties:
+ diff-channels: false
+ - required: [diff-channels]
+ properties:
+ single-channel: false
+ adi,current-channel: false
+ adi,common-mode-channel: false
+
+ - if:
+ required: ["adi,common-mode-channel"]
+ then:
+ properties:
+ adi,current-channel: false
required:
- compatible
@@ -166,7 +248,6 @@ allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#
# Only ad7172-4, ad7173-8 and ad7175-8 support vref2
- # Other models have [0-3] channel registers
- if:
properties:
compatible:
@@ -187,6 +268,37 @@ allOf:
- vref
- refout-avss
- avdd
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - adi,ad4114
+ - adi,ad4115
+ - adi,ad4116
+ - adi,ad7173-8
+ - adi,ad7175-8
+ then:
+ patternProperties:
+ "^channel@[0-9a-f]$":
+ properties:
+ reg:
+ maximum: 15
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - adi,ad7172-2
+ - adi,ad7175-2
+ - adi,ad7176-2
+ - adi,ad7177-2
+ then:
+ patternProperties:
+ "^channel@[0-9a-f]$":
+ properties:
reg:
maximum: 3
@@ -210,6 +322,34 @@ allOf:
required:
- adi,reference-select
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - adi,ad4111
+ - adi,ad4112
+ - adi,ad4114
+ - adi,ad4115
+ - adi,ad4116
+ then:
+ properties:
+ avdd2-supply: false
+
+ - if:
+ properties:
+ compatible:
+ not:
+ contains:
+ enum:
+ - adi,ad4111
+ - adi,ad4112
+ then:
+ patternProperties:
+ "^channel@[0-9a-f]$":
+ properties:
+ adi,current-channel: false
+
- if:
anyOf:
- required: [clock-names]
@@ -221,6 +361,7 @@ allOf:
unevaluatedProperties: false
examples:
+ # Example AD7173-8 with external reference connected to REF+/REF-:
- |
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/interrupt-controller/irq.h>
@@ -277,3 +418,50 @@ examples:
};
};
};
+
+ # Example AD4111 with current channel and single-ended channel:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,ad4111";
+ reg = <0>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-names = "rdy";
+ interrupt-parent = <&gpio>;
+ spi-max-frequency = <5000000>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #clock-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ bipolar;
+ diff-channels = <4 5>;
+ };
+
+ // Single ended channel VIN2/VINCOM
+ channel@1 {
+ reg = <1>;
+ bipolar;
+ single-channel = <2>;
+ adi,common-mode-channel = <16>;
+ };
+
+ // Current channel IN2+/IN2-
+ channel@2 {
+ reg = <2>;
+ single-channel = <2>;
+ adi,current-channel;
+ };
+ };
+ };
--
2.43.0
From: Dumitru Ceclan <[email protected]>
Add support for selecting REF+ and REF- inputs on all models.
Add support for selecting ((AVDD1 − AVSS)/5) inputs
on supported models.
Reviewed-by: Nuno Sa <[email protected]>
Signed-off-by: Dumitru Ceclan <[email protected]>
---
drivers/iio/adc/ad7173.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index a20831d99aa5..ebfd2d5f9632 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -66,6 +66,10 @@
FIELD_PREP(AD7173_CH_SETUP_AINNEG_MASK, neg))
#define AD7173_AIN_TEMP_POS 17
#define AD7173_AIN_TEMP_NEG 18
+#define AD7173_AIN_COM_IN_POS 19
+#define AD7173_AIN_COM_IN_NEG 20
+#define AD7173_AIN_REF_POS 21
+#define AD7173_AIN_REF_NEG 22
#define AD7172_2_ID 0x00d0
#define AD7175_ID 0x0cd0
@@ -146,6 +150,8 @@ struct ad7173_device_info {
unsigned int id;
char *name;
bool has_temp;
+ /* ((AVDD1 − AVSS)/5) */
+ bool has_common_input;
bool has_input_buf;
bool has_int_ref;
bool has_ref2;
@@ -216,6 +222,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.has_temp = true,
.has_input_buf = true,
.has_int_ref = true,
+ .has_common_input = true,
.clock = 2 * HZ_PER_MHZ,
.sinc5_data_rates = ad7173_sinc5_data_rates,
.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
@@ -230,6 +237,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.has_temp = false,
.has_input_buf = true,
.has_ref2 = true,
+ .has_common_input = true,
.clock = 2 * HZ_PER_MHZ,
.sinc5_data_rates = ad7173_sinc5_data_rates,
.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
@@ -245,6 +253,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.has_input_buf = true,
.has_int_ref = true,
.has_ref2 = true,
+ .has_common_input = false,
.clock = 2 * HZ_PER_MHZ,
.sinc5_data_rates = ad7173_sinc5_data_rates,
.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
@@ -259,6 +268,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.has_temp = true,
.has_input_buf = true,
.has_int_ref = true,
+ .has_common_input = true,
.clock = 16 * HZ_PER_MHZ,
.sinc5_data_rates = ad7175_sinc5_data_rates,
.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
@@ -274,6 +284,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.has_input_buf = true,
.has_int_ref = true,
.has_ref2 = true,
+ .has_common_input = true,
.clock = 16 * HZ_PER_MHZ,
.sinc5_data_rates = ad7175_sinc5_data_rates,
.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
@@ -288,6 +299,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.has_temp = false,
.has_input_buf = false,
.has_int_ref = true,
+ .has_common_input = false,
.clock = 16 * HZ_PER_MHZ,
.sinc5_data_rates = ad7175_sinc5_data_rates,
.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
@@ -302,6 +314,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.has_temp = true,
.has_input_buf = true,
.has_int_ref = true,
+ .has_common_input = true,
.clock = 16 * HZ_PER_MHZ,
.odr_start_value = AD7177_ODR_START_VALUE,
.sinc5_data_rates = ad7175_sinc5_data_rates,
@@ -918,6 +931,14 @@ static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
if (ain[i] < st->info->num_inputs)
continue;
+ if (ain[i] == AD7173_AIN_REF_POS || ain[i] == AD7173_AIN_REF_NEG)
+ continue;
+
+ if ((ain[i] == AD7173_AIN_COM_IN_POS ||
+ ain[i] == AD7173_AIN_COM_IN_NEG) &&
+ st->info->has_common_input)
+ continue;
+
return dev_err_probe(dev, -EINVAL,
"Input pin number out of range for pair (%d %d).\n",
ain[0], ain[1]);
--
2.43.0
From: Dumitru Ceclan <[email protected]>
Add support for AD4111/AD4112/AD4114/AD4115/AD4116.
The AD411X family encompasses a series of low power, low noise, 24-bit,
sigma-delta analog-to-digital converters that offer a versatile range of
specifications.
This family of ADCs integrates an analog front end suitable for processing
both fully differential and single-ended, bipolar voltage inputs
addressing a wide array of industrial and instrumentation requirements.
- All ADCs have inputs with a precision voltage divider with a division
ratio of 10.
- AD4116 has 5 low level inputs without a voltage divider.
- AD4111 and AD4112 support current inputs (0 mA to 20 mA) using a 50ohm
shunt resistor.
Signed-off-by: Dumitru Ceclan <[email protected]>
---
drivers/iio/adc/ad7173.c | 336 +++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 307 insertions(+), 29 deletions(-)
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index ed8ff8c5f343..91ff984eedf4 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -1,8 +1,9 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * AD717x family SPI ADC driver
+ * AD717x and AD411x family SPI ADC driver
*
* Supported devices:
+ * AD4111/AD4112/AD4114/AD4115/AD4116
* AD7172-2/AD7172-4/AD7173-8/AD7175-2
* AD7175-8/AD7176-2/AD7177-2
*
@@ -77,6 +78,11 @@
#define AD7175_2_ID 0x0cd0
#define AD7172_4_ID 0x2050
#define AD7173_ID 0x30d0
+#define AD4111_ID AD7173_ID
+#define AD4112_ID AD7173_ID
+#define AD4114_ID AD7173_ID
+#define AD4116_ID 0x34d0
+#define AD4115_ID 0x38d0
#define AD7175_8_ID 0x3cd0
#define AD7177_ID 0x4fd0
#define AD7173_ID_MASK GENMASK(15, 4)
@@ -107,6 +113,7 @@
#define AD7173_GPO12_DATA(x) BIT((x) + 0)
#define AD7173_GPO23_DATA(x) BIT((x) + 4)
+#define AD4111_GPO01_DATA(x) BIT((x) + 6)
#define AD7173_GPO_DATA(x) ((x) < 2 ? AD7173_GPO12_DATA(x) : AD7173_GPO23_DATA(x))
#define AD7173_INTERFACE_DATA_STAT BIT(6)
@@ -125,26 +132,46 @@
#define AD7173_VOLTAGE_INT_REF_uV 2500000
#define AD7173_TEMP_SENSIIVITY_uV_per_C 477
#define AD7177_ODR_START_VALUE 0x07
+#define AD4111_SHUNT_RESISTOR_OHM 50
+#define AD4111_DIVIDER_RATIO 10
+#define AD411X_VCOM_INPUT 0X10
+#define AD4111_CURRENT_CHAN_CUTOFF 16
#define AD7173_FILTER_ODR0_MASK GENMASK(5, 0)
#define AD7173_MAX_CONFIGS 8
+enum ad4111_current_channels {
+ AD4111_CURRENT_IN0P_IN0N,
+ AD4111_CURRENT_IN1P_IN1N,
+ AD4111_CURRENT_IN2P_IN2N,
+ AD4111_CURRENT_IN3P_IN3N,
+};
+
struct ad7173_device_info {
const unsigned int *sinc5_data_rates;
unsigned int num_sinc5_data_rates;
unsigned int odr_start_value;
+ /*
+ * AD4116 has both inputs with a voltage divider and without.
+ * These inputs cannot be mixed in the channel configuration.
+ * Does not include the VINCOM input.
+ */
+ unsigned int num_voltage_in_div;
unsigned int num_channels;
unsigned int num_configs;
- unsigned int num_inputs;
+ unsigned int num_voltage_in;
unsigned int clock;
unsigned int id;
char *name;
+ bool has_current_inputs;
+ bool has_vcom_input;
bool has_temp;
/* ((AVDD1 − AVSS)/5) */
bool has_common_input;
bool has_input_buf;
bool has_int_ref;
bool has_ref2;
+ bool higher_gpio_bits;
u8 num_gpios;
};
@@ -186,6 +213,24 @@ struct ad7173_state {
#endif
};
+static unsigned int ad4115_sinc5_data_rates[] = {
+ 24845000, 24845000, 20725000, 20725000, /* 0-3 */
+ 15564000, 13841000, 10390000, 10390000, /* 4-7 */
+ 4994000, 2499000, 1000000, 500000, /* 8-11 */
+ 395500, 200000, 100000, 59890, /* 12-15 */
+ 49920, 20000, 16660, 10000, /* 16-19 */
+ 5000, 2500, 2500, /* 20-22 */
+};
+
+static unsigned int ad4116_sinc5_data_rates[] = {
+ 12422360, 12422360, 12422360, 12422360, /* 0-3 */
+ 10362690, 10362690, 7782100, 6290530, /* 4-7 */
+ 5194800, 2496900, 1007600, 499900, /* 8-11 */
+ 390600, 200300, 100000, 59750, /* 12-15 */
+ 49840, 20000, 16650, 10000, /* 16-19 */
+ 5000, 2500, 1250, /* 20-22 */
+};
+
static const unsigned int ad7173_sinc5_data_rates[] = {
6211000, 6211000, 6211000, 6211000, 6211000, 6211000, 5181000, 4444000, /* 0-7 */
3115000, 2597000, 1007000, 503800, 381000, 200300, 100500, 59520, /* 8-15 */
@@ -201,13 +246,113 @@ static const unsigned int ad7175_sinc5_data_rates[] = {
5000, /* 20 */
};
+static unsigned int ad4111_current_channel_config[] = {
+ [AD4111_CURRENT_IN0P_IN0N] = 0x1E8,
+ [AD4111_CURRENT_IN1P_IN1N] = 0x1C9,
+ [AD4111_CURRENT_IN2P_IN2N] = 0x1AA,
+ [AD4111_CURRENT_IN3P_IN3N] = 0x18B,
+};
+
+static const struct ad7173_device_info ad4111_device_info = {
+ .name = "ad4111",
+ .id = AD4111_ID,
+ .num_voltage_in_div = 8,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_in = 8,
+ .num_gpios = 2,
+ .higher_gpio_bits = true,
+ .has_temp = true,
+ .has_vcom_input = true,
+ .has_input_buf = true,
+ .has_current_inputs = true,
+ .has_int_ref = true,
+ .clock = 2 * HZ_PER_MHZ,
+ .sinc5_data_rates = ad7173_sinc5_data_rates,
+ .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad4112_device_info = {
+ .name = "ad4112",
+ .id = AD4112_ID,
+ .num_voltage_in_div = 8,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_in = 8,
+ .num_gpios = 2,
+ .higher_gpio_bits = true,
+ .has_vcom_input = true,
+ .has_temp = true,
+ .has_input_buf = true,
+ .has_current_inputs = true,
+ .has_int_ref = true,
+ .clock = 2 * HZ_PER_MHZ,
+ .sinc5_data_rates = ad7173_sinc5_data_rates,
+ .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad4114_device_info = {
+ .name = "ad4114",
+ .id = AD4114_ID,
+ .num_voltage_in_div = 16,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_in = 16,
+ .num_gpios = 4,
+ .higher_gpio_bits = true,
+ .has_vcom_input = true,
+ .has_temp = true,
+ .has_input_buf = true,
+ .has_int_ref = true,
+ .clock = 2 * HZ_PER_MHZ,
+ .sinc5_data_rates = ad7173_sinc5_data_rates,
+ .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad4115_device_info = {
+ .name = "ad4115",
+ .id = AD4115_ID,
+ .num_voltage_in_div = 16,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_in = 16,
+ .num_gpios = 4,
+ .higher_gpio_bits = true,
+ .has_vcom_input = true,
+ .has_temp = true,
+ .has_input_buf = true,
+ .has_int_ref = true,
+ .clock = 8 * HZ_PER_MHZ,
+ .sinc5_data_rates = ad4115_sinc5_data_rates,
+ .num_sinc5_data_rates = ARRAY_SIZE(ad4115_sinc5_data_rates),
+};
+
+static const struct ad7173_device_info ad4116_device_info = {
+ .name = "ad4116",
+ .id = AD4116_ID,
+ .num_voltage_in_div = 11,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_in = 16,
+ .num_gpios = 4,
+ .higher_gpio_bits = true,
+ .has_vcom_input = true,
+ .has_temp = true,
+ .has_input_buf = true,
+ .has_int_ref = true,
+ .clock = 4 * HZ_PER_MHZ,
+ .sinc5_data_rates = ad4116_sinc5_data_rates,
+ .num_sinc5_data_rates = ARRAY_SIZE(ad4116_sinc5_data_rates),
+};
+
static const struct ad7173_device_info ad7172_2_device_info = {
.name = "ad7172-2",
.id = AD7172_2_ID,
- .num_inputs = 5,
+ .num_voltage_in = 5,
.num_channels = 4,
.num_configs = 4,
.num_gpios = 2,
+ .higher_gpio_bits = false,
.has_temp = true,
.has_input_buf = true,
.has_int_ref = true,
@@ -220,10 +365,11 @@ static const struct ad7173_device_info ad7172_2_device_info = {
static const struct ad7173_device_info ad7172_4_device_info = {
.name = "ad7172-4",
.id = AD7172_4_ID,
- .num_inputs = 9,
+ .num_voltage_in = 9,
.num_channels = 8,
.num_configs = 8,
.num_gpios = 4,
+ .higher_gpio_bits = false,
.has_temp = false,
.has_input_buf = true,
.has_ref2 = true,
@@ -236,10 +382,11 @@ static const struct ad7173_device_info ad7172_4_device_info = {
static const struct ad7173_device_info ad7173_8_device_info = {
.name = "ad7173-8",
.id = AD7173_ID,
- .num_inputs = 17,
+ .num_voltage_in = 17,
.num_channels = 16,
.num_configs = 8,
.num_gpios = 4,
+ .higher_gpio_bits = false,
.has_temp = true,
.has_input_buf = true,
.has_int_ref = true,
@@ -253,10 +400,11 @@ static const struct ad7173_device_info ad7173_8_device_info = {
static const struct ad7173_device_info ad7175_2_device_info = {
.name = "ad7175-2",
.id = AD7175_2_ID,
- .num_inputs = 5,
+ .num_voltage_in = 5,
.num_channels = 4,
.num_configs = 4,
.num_gpios = 2,
+ .higher_gpio_bits = false,
.has_temp = true,
.has_input_buf = true,
.has_int_ref = true,
@@ -269,10 +417,11 @@ static const struct ad7173_device_info ad7175_2_device_info = {
static const struct ad7173_device_info ad7175_8_device_info = {
.name = "ad7175-8",
.id = AD7175_8_ID,
- .num_inputs = 17,
+ .num_voltage_in = 17,
.num_channels = 16,
.num_configs = 8,
.num_gpios = 4,
+ .higher_gpio_bits = false,
.has_temp = true,
.has_input_buf = true,
.has_int_ref = true,
@@ -286,10 +435,11 @@ static const struct ad7173_device_info ad7175_8_device_info = {
static const struct ad7173_device_info ad7176_2_device_info = {
.name = "ad7176-2",
.id = AD7176_ID,
- .num_inputs = 5,
+ .num_voltage_in = 5,
.num_channels = 4,
.num_configs = 4,
.num_gpios = 2,
+ .higher_gpio_bits = false,
.has_temp = false,
.has_input_buf = false,
.has_int_ref = true,
@@ -302,10 +452,11 @@ static const struct ad7173_device_info ad7176_2_device_info = {
static const struct ad7173_device_info ad7177_2_device_info = {
.name = "ad7177-2",
.id = AD7177_ID,
- .num_inputs = 5,
+ .num_voltage_in = 5,
.num_channels = 4,
.num_configs = 4,
.num_gpios = 2,
+ .higher_gpio_bits = false,
.has_temp = true,
.has_input_buf = true,
.has_int_ref = true,
@@ -355,6 +506,15 @@ static int ad7173_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
return 0;
}
+static int ad4111_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
+ unsigned int offset, unsigned int *reg,
+ unsigned int *mask)
+{
+ *mask = AD4111_GPO01_DATA(offset);
+ *reg = base;
+ return 0;
+}
+
static void ad7173_gpio_disable(void *data)
{
struct ad7173_state *st = data;
@@ -387,7 +547,10 @@ static int ad7173_gpio_init(struct ad7173_state *st)
gpio_regmap.regmap = st->reg_gpiocon_regmap;
gpio_regmap.ngpio = st->info->num_gpios;
gpio_regmap.reg_set_base = AD7173_REG_GPIO;
- gpio_regmap.reg_mask_xlate = ad7173_mask_xlate;
+ if (st->info->higher_gpio_bits)
+ gpio_regmap.reg_mask_xlate = ad4111_mask_xlate;
+ else
+ gpio_regmap.reg_mask_xlate = ad7173_mask_xlate;
st->gpio_regmap = devm_gpio_regmap_register(dev, &gpio_regmap);
ret = PTR_ERR_OR_ZERO(st->gpio_regmap);
@@ -686,18 +849,33 @@ static int ad7173_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
- if (chan->type == IIO_TEMP) {
+
+ switch (chan->type) {
+ case IIO_TEMP:
temp = AD7173_VOLTAGE_INT_REF_uV * MILLI;
temp /= AD7173_TEMP_SENSIIVITY_uV_per_C;
*val = temp;
*val2 = chan->scan_type.realbits;
- } else {
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_VOLTAGE:
*val = ad7173_get_ref_voltage_milli(st, ch->cfg.ref_sel);
*val2 = chan->scan_type.realbits - !!(ch->cfg.bipolar);
+
+ if (chan->channel < st->info->num_voltage_in_div)
+ *val *= AD4111_DIVIDER_RATIO;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CURRENT:
+ *val = ad7173_get_ref_voltage_milli(st, ch->cfg.ref_sel);
+ *val /= AD4111_SHUNT_RESISTOR_OHM;
+ *val2 = chan->scan_type.realbits - ch->cfg.bipolar;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
}
- return IIO_VAL_FRACTIONAL_LOG2;
case IIO_CHAN_INFO_OFFSET:
- if (chan->type == IIO_TEMP) {
+
+ switch (chan->type) {
+ case IIO_TEMP:
/* 0 Kelvin -> raw sample */
temp = -ABSOLUTE_ZERO_MILLICELSIUS;
temp *= AD7173_TEMP_SENSIIVITY_uV_per_C;
@@ -706,10 +884,14 @@ static int ad7173_read_raw(struct iio_dev *indio_dev,
AD7173_VOLTAGE_INT_REF_uV *
MILLI);
*val = -temp;
- } else {
+ return IIO_VAL_INT;
+ case IIO_VOLTAGE:
+ case IIO_CURRENT:
*val = -BIT(chan->scan_type.realbits - 1);
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
}
- return IIO_VAL_INT;
case IIO_CHAN_INFO_SAMP_FREQ:
reg = st->channels[chan->address].cfg.odr;
@@ -736,6 +918,21 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
return ret;
switch (info) {
+ /*
+ * This attribute sets the sampling frequency to each channel individually.
+ * There are no issues for raw or buffered reads of an individual channel.
+ *
+ * When multiple channels are enabled in buffered mode, the effective
+ * sampling rate of a channel is lowered in correlation to the number
+ * of channels enabled and the sampling rate of the other channels.
+ *
+ * Example: 3 channels enabled with rates CH1:6211sps CH2,CH3:10sps
+ * While the reading of CH1 takes only 0.16ms, the reading of CH2 and CH3
+ * will take 100ms each.
+ *
+ * This will cause the reading of CH1 to be actually done once every
+ * 200.16ms, an effective rate of 4.99sps.
+ */
case IIO_CHAN_INFO_SAMP_FREQ:
freq = val * MILLI + val2 / MILLI;
for (i = st->info->odr_start_value; i < st->info->num_sinc5_data_rates - 1; i++)
@@ -916,13 +1113,34 @@ static int ad7173_register_clk_provider(struct iio_dev *indio_dev)
&st->int_clk_hw);
}
+static int ad4111_validate_current_ain(struct ad7173_state *st,
+ const unsigned int ain[AD7173_NO_AINS_PER_CHANNEL])
+{
+ struct device *dev = &st->sd.spi->dev;
+
+ if (!st->info->has_current_inputs)
+ return dev_err_probe(dev, -EINVAL,
+ "Model %s does not support current channels\n",
+ st->info->name);
+
+ if (ain[0] >= ARRAY_SIZE(ad4111_current_channel_config))
+ return dev_err_probe(dev, -EINVAL,
+ "For current channels single-channel must be <[0-3]>\n");
+
+ return 0;
+}
+
static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
const unsigned int ain[AD7173_NO_AINS_PER_CHANNEL])
{
struct device *dev = &st->sd.spi->dev;
+ bool ain_selects_normal_input[] = {
+ ain[0] < st->info->num_voltage_in,
+ ain[1] < st->info->num_voltage_in
+ };
for (int i = 0; i < AD7173_NO_AINS_PER_CHANNEL; i++) {
- if (ain[i] < st->info->num_inputs)
+ if (ain_selects_normal_input[i])
continue;
if (ain[i] == AD7173_AIN_REF_POS || ain[i] == AD7173_AIN_REF_NEG)
@@ -933,11 +1151,27 @@ static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
st->info->has_common_input)
continue;
+ if (st->info->has_vcom_input && ain[i] == AD411X_VCOM_INPUT) {
+ if (ain_selects_normal_input[(i + 1) % 2] &&
+ ain[(i + 1) % 2] >= st->info->num_voltage_in_div)
+ return dev_err_probe(dev, -EINVAL,
+ "VCOM must be paired with inputs having divider.\n");
+
+ continue;
+ }
+
return dev_err_probe(dev, -EINVAL,
"Input pin number out of range for pair (%d %d).\n",
ain[0], ain[1]);
}
+ if ((ain_selects_normal_input[0] && ain_selects_normal_input[1]) &&
+ ((ain[0] >= st->info->num_voltage_in_div) !=
+ (ain[1] >= st->info->num_voltage_in_div)))
+ return dev_err_probe(dev, -EINVAL,
+ "Both inputs must either have a voltage divider or not have: (%d %d).\n",
+ ain[0], ain[1]);
+
return 0;
}
@@ -968,7 +1202,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
struct device *dev = indio_dev->dev.parent;
struct iio_chan_spec *chan_arr, *chan;
unsigned int ain[AD7173_NO_AINS_PER_CHANNEL], chan_index = 0;
- int ref_sel, ret, num_channels;
+ int ref_sel, ret, is_current_chan, num_channels;
num_channels = device_get_child_node_count(dev);
@@ -1015,15 +1249,41 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
device_for_each_child_node_scoped(dev, child) {
chan = &chan_arr[chan_index];
+ *chan = ad7173_channel_template;
chan_st_priv = &chans_st_arr[chan_index];
ret = fwnode_property_read_u32_array(child, "diff-channels",
ain, ARRAY_SIZE(ain));
- if (ret)
- return ret;
+ if (ret) {
+ ret = fwnode_property_read_u32_array(child, "single-channel",
+ ain, 1);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Channel must define one of diff-channels or single-channel.\n");
- ret = ad7173_validate_voltage_ain_inputs(st, ain);
- if (ret)
- return ret;
+ is_current_chan = fwnode_property_read_bool(child, "adi,current-channel");
+ } else {
+ chan->differential = true;
+ }
+
+ if (is_current_chan) {
+ ret = ad4111_validate_current_ain(st, ain);
+ if (ret)
+ return ret;
+ is_current_chan = true;
+ ain[1] = 0;
+ } else {
+ if (!chan->differential) {
+ ret = fwnode_property_read_u32_array(child,
+ "adi,common-mode-channel", ain + 1, 1);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "adi,common-mode-channel must be defined for single-ended channels.\n");
+ }
+ ret = ad7173_validate_voltage_ain_inputs(st, ain);
+ if (ret)
+ return ret;
+ is_current_chan = false;
+ }
ret = fwnode_property_match_property_string(child,
"adi,reference-select",
@@ -1042,22 +1302,30 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
st->adc_mode |= AD7173_ADC_MODE_REF_EN;
chan_st_priv->cfg.ref_sel = ref_sel;
- *chan = ad7173_channel_template;
chan->address = chan_index;
chan->scan_index = chan_index;
chan->channel = ain[0];
chan->channel2 = ain[1];
- chan->differential = true;
-
- chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
chan_st_priv->chan_reg = chan_index;
chan_st_priv->cfg.input_buf = st->info->has_input_buf;
chan_st_priv->cfg.odr = 0;
-
chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
+
if (chan_st_priv->cfg.bipolar)
chan->info_mask_separate |= BIT(IIO_CHAN_INFO_OFFSET);
+ if (is_current_chan) {
+ chan->type = IIO_CURRENT;
+ chan->differential = false;
+ ain[1] = FIELD_GET(AD7173_CH_SETUP_AINNEG_MASK,
+ ad4111_current_channel_config[ain[0]]);
+ ain[0] = FIELD_GET(AD7173_CH_SETUP_AINPOS_MASK,
+ ad4111_current_channel_config[ain[0]]);
+ } else {
+ chan_st_priv->cfg.input_buf = st->info->has_input_buf;
+ }
+ chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
+
chan_index++;
}
return 0;
@@ -1184,6 +1452,11 @@ static int ad7173_probe(struct spi_device *spi)
}
static const struct of_device_id ad7173_of_match[] = {
+ { .compatible = "ad4111", .data = &ad4111_device_info},
+ { .compatible = "ad4112", .data = &ad4112_device_info},
+ { .compatible = "ad4114", .data = &ad4114_device_info},
+ { .compatible = "ad4115", .data = &ad4115_device_info},
+ { .compatible = "ad4116", .data = &ad4116_device_info},
{ .compatible = "adi,ad7172-2", .data = &ad7172_2_device_info},
{ .compatible = "adi,ad7172-4", .data = &ad7172_4_device_info},
{ .compatible = "adi,ad7173-8", .data = &ad7173_8_device_info},
@@ -1196,6 +1469,11 @@ static const struct of_device_id ad7173_of_match[] = {
MODULE_DEVICE_TABLE(of, ad7173_of_match);
static const struct spi_device_id ad7173_id_table[] = {
+ { "ad4111", (kernel_ulong_t)&ad4111_device_info},
+ { "ad4112", (kernel_ulong_t)&ad4112_device_info},
+ { "ad4114", (kernel_ulong_t)&ad4114_device_info},
+ { "ad4115", (kernel_ulong_t)&ad4115_device_info},
+ { "ad4116", (kernel_ulong_t)&ad4116_device_info},
{ "ad7172-2", (kernel_ulong_t)&ad7172_2_device_info},
{ "ad7172-4", (kernel_ulong_t)&ad7172_4_device_info},
{ "ad7173-8", (kernel_ulong_t)&ad7173_8_device_info},
@@ -1220,5 +1498,5 @@ module_spi_driver(ad7173_driver);
MODULE_IMPORT_NS(IIO_AD_SIGMA_DELTA);
MODULE_AUTHOR("Lars-Peter Clausen <[email protected]>");
MODULE_AUTHOR("Dumitru Ceclan <[email protected]>");
-MODULE_DESCRIPTION("Analog Devices AD7172/AD7173/AD7175/AD7176 ADC driver");
+MODULE_DESCRIPTION("Analog Devices AD7173 and similar ADC driver");
MODULE_LICENSE("GPL");
--
2.43.0
On Fri, 31 May 2024 22:42:27 +0300
Dumitru Ceclan via B4 Relay <[email protected]> wrote:
> From: Dumitru Ceclan <[email protected]>
>
> Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
>
> AD411x family ADCs support a VCOM pin. The purpose of this pin is to
> offer a dedicated common-mode voltage input for single-ended channels.
> This pin is specified as supporting a differential channel with VIN10 on
> model AD4116.
>
> AD4111/AD4112 support current channels. Support is implemented using
> single-channel and "adi,current-channel".
>
> Signed-off-by: Dumitru Ceclan <[email protected]>
I like the common-mode-channel solution to the pseudo differential
description. It makes things explicit whilst avoiding an ugly differential
but not differential mess.
However, it feels like a general thing to me not a vendor specific one.
Perhaps makes sense to put in adc.yaml?
One other question that is more me being curious and failing to understand
the datasheet than a request to change anything.
> ---
> .../devicetree/bindings/iio/adc/adi,ad7173.yaml | 192 ++++++++++++++++++++-
> 1 file changed, 190 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> index ea6cfcd0aff4..d8474eee553e 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> @@ -19,7 +19,18 @@ description: |
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
>
> + Analog Devices AD411x ADC's:
> + The AD411X family encompasses a series of low power, low noise, 24-bit,
> + sigma-delta analog-to-digital converters that offer a versatile range of
> + specifications. They integrate an analog front end suitable for processing
> + fully differential/single-ended and bipolar voltage inputs.
> +
> Datasheets for supported chips:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4111.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4112.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4114.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4115.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4116.pdf
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-4.pdf
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
> @@ -31,6 +42,11 @@ description: |
> properties:
> compatible:
> enum:
> + - adi,ad4111
> + - adi,ad4112
> + - adi,ad4114
> + - adi,ad4115
> + - adi,ad4116
> - adi,ad7172-2
> - adi,ad7172-4
> - adi,ad7173-8
> @@ -129,10 +145,54 @@ patternProperties:
> maximum: 15
>
> diff-channels:
> + description: |
> + This property is used for defining the inputs of a differential
> + voltage channel. The first value is the positive input and the second
> + value is the negative input of the channel.
> +
> + Family AD411x supports a dedicated VINCOM voltage input.
> + To select it set the second channel to 16.
> + (VIN2, VINCOM) -> diff-channels = <2 16>
> +
> + There are special values that can be selected besides the voltage
> + analog inputs:
> + 21: REF+
> + 22: REF−
> + Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
> + 19: ((AVDD1 − AVSS)/5)+
> + 20: ((AVDD1 − AVSS)/5)−
That's what it says on the datasheet (so fine to copy that here) but I'm curious, what does
that mean in practice? How can we have negative and postive signals of the difference
between two power supply voltages where I'm fairly sure AVDD1 always greater than AVSS.
Anyhow, that's a problem for the person reading the datasheet to figure out :)
> +
> items:
> minimum: 0
> maximum: 31
>
> + single-channel:
> + description: |
> + This property is used for defining a current channel or the positive
> + input of a voltage channel (single-ended or pseudo-differential).
> +
> + Models AD4111 and AD4112 support current channels.
> + Example: (IIN2+, IIN2−) -> single-channel = <2>
> + To correctly configure a current channel set the "adi,current-channel"
> + property to true.
> +
> + To configure a single-ended/pseudo-differential channel set the
> + "adi,common-mode-channel" property to the desired negative voltage input.
> +
> + When used as a voltage channel, special inputs are valid as well.
> + minimum: 0
> + maximum: 31
> +
> + adi,common-mode-channel:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + This property is used for defining the negative input of a
> + single-ended or pseudo-differential voltage channel.
> +
> + Special inputs are valid as well.
> + minimum: 0
> + maximum: 31
> +
On Fri, 31 May 2024 22:42:30 +0300
Dumitru Ceclan via B4 Relay <[email protected]> wrote:
> From: Dumitru Ceclan <[email protected]>
>
> Add support for selecting REF+ and REF- inputs on all models.
> Add support for selecting ((AVDD1 − AVSS)/5) inputs
> on supported models.
>
> Reviewed-by: Nuno Sa <[email protected]>
> Signed-off-by: Dumitru Ceclan <[email protected]>
Random passing comment inline. This looks fine to me.
> ---
> drivers/iio/adc/ad7173.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index a20831d99aa5..ebfd2d5f9632 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -66,6 +66,10 @@
> FIELD_PREP(AD7173_CH_SETUP_AINNEG_MASK, neg))
> #define AD7173_AIN_TEMP_POS 17
> #define AD7173_AIN_TEMP_NEG 18
> +#define AD7173_AIN_COM_IN_POS 19
> +#define AD7173_AIN_COM_IN_NEG 20
> +#define AD7173_AIN_REF_POS 21
> +#define AD7173_AIN_REF_NEG 22
>
> #define AD7172_2_ID 0x00d0
> #define AD7175_ID 0x0cd0
> @@ -146,6 +150,8 @@ struct ad7173_device_info {
> unsigned int id;
> char *name;
> bool has_temp;
> + /* ((AVDD1 − AVSS)/5) */
> + bool has_common_input;
> bool has_input_buf;
> bool has_int_ref;
> bool has_ref2;
> @@ -216,6 +222,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .has_temp = true,
> .has_input_buf = true,
> .has_int_ref = true,
> + .has_common_input = true,
This is another good reason for breaking up this array (as suggested in v3 review
but perhaps a job for another patch set!)
If you break it up we can see which entry each of these actually is in the diff
rather than having to use line count to figure it out!
> .clock = 2 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7173_sinc5_data_rates,
> .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> @@ -230,6 +237,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
On Fri, 31 May 2024 22:42:32 +0300
Dumitru Ceclan via B4 Relay <[email protected]> wrote:
> From: Dumitru Ceclan <[email protected]>
>
> Add support for AD4111/AD4112/AD4114/AD4115/AD4116.
>
> The AD411X family encompasses a series of low power, low noise, 24-bit,
> sigma-delta analog-to-digital converters that offer a versatile range of
> specifications.
>
> This family of ADCs integrates an analog front end suitable for processing
> both fully differential and single-ended, bipolar voltage inputs
> addressing a wide array of industrial and instrumentation requirements.
>
> - All ADCs have inputs with a precision voltage divider with a division
> ratio of 10.
> - AD4116 has 5 low level inputs without a voltage divider.
> - AD4111 and AD4112 support current inputs (0 mA to 20 mA) using a 50ohm
> shunt resistor.
>
> Signed-off-by: Dumitru Ceclan <[email protected]>
Hi Dumitru,
A follow on comment on the validation code.
Also there is some good docs for the sampling frequency but are they
actually related to the rest of this change? They also raise
questions about ABI compliance that we may want to deal with as
a follow up patch.
A few other trivial things inline.
This is looking pretty good, so hopefully we'll get the last few corners
sorted in v5.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7173.c | 336 +++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 307 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index ed8ff8c5f343..91ff984eedf4 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -1,8 +1,9 @@
> #define AD7173_INTERFACE_DATA_STAT BIT(6)
> @@ -125,26 +132,46 @@
> #define AD7173_VOLTAGE_INT_REF_uV 2500000
> #define AD7173_TEMP_SENSIIVITY_uV_per_C 477
> #define AD7177_ODR_START_VALUE 0x07
> +#define AD4111_SHUNT_RESISTOR_OHM 50
> +#define AD4111_DIVIDER_RATIO 10
> +#define AD411X_VCOM_INPUT 0X10
AD4111_VCOM_INPUT . Looks like one wildcard escaped an earlier edit?
> +#define AD4111_CURRENT_CHAN_CUTOFF 16
>
> @@ -736,6 +918,21 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
> return ret;
>
> switch (info) {
> + /*
> + * This attribute sets the sampling frequency to each channel individually.
frequency for each channel?
> + * There are no issues for raw or buffered reads of an individual channel.
> + *
> + * When multiple channels are enabled in buffered mode, the effective
> + * sampling rate of a channel is lowered in correlation to the number
> + * of channels enabled and the sampling rate of the other channels.
> + *
> + * Example: 3 channels enabled with rates CH1:6211sps CH2,CH3:10sps
> + * While the reading of CH1 takes only 0.16ms, the reading of CH2 and CH3
> + * will take 100ms each.
> + *
> + * This will cause the reading of CH1 to be actually done once every
> + * 200.16ms, an effective rate of 4.99sps.
Hmm. This is a bit unfortunate as if I understand correctly that's not really what
people will expect when they configure the sampling frequency. However I can't immediately
think of a better solution. You could let userspace write a value that is cached
then attempt to get as near as possible as channels are enabled.
Still this looks like a documentation enhancement of existing behavior
in which case any functional change can be in a future patch.
However I don't think the docs update belongs in this patch unless
I'm missing some reason for it?
> + */
> case IIO_CHAN_INFO_SAMP_FREQ:
> freq = val * MILLI + val2 / MILLI;
> for (i = st->info->odr_start_value; i < st->info->num_sinc5_data_rates - 1; i++)
> @@ -916,13 +1113,34 @@ static int ad7173_register_clk_provider(struct iio_dev *indio_dev)
> &st->int_clk_hw);
> }
>
> +static int ad4111_validate_current_ain(struct ad7173_state *st,
> + const unsigned int ain[AD7173_NO_AINS_PER_CHANNEL])
> +{
> + struct device *dev = &st->sd.spi->dev;
> +
> + if (!st->info->has_current_inputs)
> + return dev_err_probe(dev, -EINVAL,
> + "Model %s does not support current channels\n",
> + st->info->name);
> +
> + if (ain[0] >= ARRAY_SIZE(ad4111_current_channel_config))
> + return dev_err_probe(dev, -EINVAL,
> + "For current channels single-channel must be <[0-3]>\n");
> +
> + return 0;
> +}
> +
> static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
> const unsigned int ain[AD7173_NO_AINS_PER_CHANNEL])
> {
Ah. So this got more complex. I'm not convinced it makes the loop worthwhile though.
> struct device *dev = &st->sd.spi->dev;
> + bool ain_selects_normal_input[] = {
> + ain[0] < st->info->num_voltage_in,
> + ain[1] < st->info->num_voltage_in
> + };
>
> for (int i = 0; i < AD7173_NO_AINS_PER_CHANNEL; i++) {
> - if (ain[i] < st->info->num_inputs)
> + if (ain_selects_normal_input[i])
> continue;
>
> if (ain[i] == AD7173_AIN_REF_POS || ain[i] == AD7173_AIN_REF_NEG)
> @@ -933,11 +1151,27 @@ static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
> st->info->has_common_input)
> continue;
>
> + if (st->info->has_vcom_input && ain[i] == AD411X_VCOM_INPUT) {
I'd missed this earlier, but wild cares in defines often bite us. Better
just to pick a part as who knows what other 411X devices will turn up
in future.
> + if (ain_selects_normal_input[(i + 1) % 2] &&
> + ain[(i + 1) % 2] >= st->info->num_voltage_in_div)
This logic looks too complex to me. Add a comment on what is actually being checked.
Given i is 0 or 1, looks like you are just saying to check the other value.
Maybe with it is cleaner in the unrolled version I suggested earlier
Something like.
if (st->info->has_vcom_input) {
if (ain0 == AD411X_VCOM_INPUT &&
ain1 < st->info->num_voltage_in && /* Normal input */
ain1 >= st->info->num_voltage_in_div) /* Not a divider */
return dev_err_probe(...)
if (ain1 == AD11X_VCOM_INPUT &&
ain0 < st->....
}
> + return dev_err_probe(dev, -EINVAL,
> + "VCOM must be paired with inputs having divider.\n");
> +
> + continue;
> + }
> +
> return dev_err_probe(dev, -EINVAL,
> "Input pin number out of range for pair (%d %d).\n",
> ain[0], ain[1]);
> }
>
> + if ((ain_selects_normal_input[0] && ain_selects_normal_input[1]) &&
Set of brackets that don't add anything to logic in the line above.
> + ((ain[0] >= st->info->num_voltage_in_div) !=
> + (ain[1] >= st->info->num_voltage_in_div)))
> + return dev_err_probe(dev, -EINVAL,
> + "Both inputs must either have a voltage divider or not have: (%d %d).\n",
> + ain[0], ain[1]);
> +
> return 0;
> }
>
> @@ -968,7 +1202,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> struct device *dev = indio_dev->dev.parent;
> struct iio_chan_spec *chan_arr, *chan;
> unsigned int ain[AD7173_NO_AINS_PER_CHANNEL], chan_index = 0;
> - int ref_sel, ret, num_channels;
> + int ref_sel, ret, is_current_chan, num_channels;
>
> num_channels = device_get_child_node_count(dev);
>
> @@ -1015,15 +1249,41 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>
> device_for_each_child_node_scoped(dev, child) {
> chan = &chan_arr[chan_index];
> + *chan = ad7173_channel_template;
> chan_st_priv = &chans_st_arr[chan_index];
> ret = fwnode_property_read_u32_array(child, "diff-channels",
> ain, ARRAY_SIZE(ain));
> - if (ret)
> - return ret;
> + if (ret) {
> + ret = fwnode_property_read_u32_array(child, "single-channel",
> + ain, 1);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Channel must define one of diff-channels or single-channel.\n");
>
> - ret = ad7173_validate_voltage_ain_inputs(st, ain);
> - if (ret)
> - return ret;
> + is_current_chan = fwnode_property_read_bool(child, "adi,current-channel");
> + } else {
> + chan->differential = true;
> + }
> +
> + if (is_current_chan) {
> + ret = ad4111_validate_current_ain(st, ain);
> + if (ret)
> + return ret;
> + is_current_chan = true;
> + ain[1] = 0;
Set here and overwritten below? That is correct code, but not nice to read
as we'd expect this to maintain as single meaning throughout whereas it
changes as we go through this function.
> + } else {
> + if (!chan->differential) {
> + ret = fwnode_property_read_u32_array(child,
> + "adi,common-mode-channel", ain + 1, 1);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "adi,common-mode-channel must be defined for single-ended channels.\n");
> + }
> + ret = ad7173_validate_voltage_ain_inputs(st, ain);
> + if (ret)
> + return ret;
> + is_current_chan = false;
> + }
>
> ret = fwnode_property_match_property_string(child,
> "adi,reference-select",
> @@ -1042,22 +1302,30 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> st->adc_mode |= AD7173_ADC_MODE_REF_EN;
> chan_st_priv->cfg.ref_sel = ref_sel;
>
> - *chan = ad7173_channel_template;
> chan->address = chan_index;
> chan->scan_index = chan_index;
> chan->channel = ain[0];
> chan->channel2 = ain[1];
> - chan->differential = true;
> -
> - chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
> chan_st_priv->chan_reg = chan_index;
> chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> chan_st_priv->cfg.odr = 0;
> -
> chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
> +
Trivial: Why the white space change?
Previously this nicely grouped the bipolar check with a use of it, now they
are separated.
> if (chan_st_priv->cfg.bipolar)
> chan->info_mask_separate |= BIT(IIO_CHAN_INFO_OFFSET);
>
> + if (is_current_chan) {
> + chan->type = IIO_CURRENT;
> + chan->differential = false;
> + ain[1] = FIELD_GET(AD7173_CH_SETUP_AINNEG_MASK,
> + ad4111_current_channel_config[ain[0]]);
Above, ain[1] is set to 0 for current channels. Why do that if it is going to be
overwritten here? I think the issue is that you needed ain[1] = 0 so
chan->channel2 = 0. That rather implies these should not be using the same
local variable. I think I'd either set chan->channel, and chan->channel2 up
where the ain[0]/ain[1] are set above, or use a different local variable
for what goes in them.
> + ain[0] = FIELD_GET(AD7173_CH_SETUP_AINPOS_MASK,
> + ad4111_current_channel_config[ain[0]]);
> + } else {
> + chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> + }
> + chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
> +
> chan_index++;
> }
> return 0;
On 01/06/2024 21:35, Jonathan Cameron wrote:
> On Fri, 31 May 2024 22:42:27 +0300
> Dumitru Ceclan via B4 Relay <[email protected]> wrote:
>
>> From: Dumitru Ceclan <[email protected]>
>>
>> Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
>>
>> AD411x family ADCs support a VCOM pin. The purpose of this pin is to
>> offer a dedicated common-mode voltage input for single-ended channels.
>> This pin is specified as supporting a differential channel with VIN10 on
>> model AD4116.
>>
>> AD4111/AD4112 support current channels. Support is implemented using
>> single-channel and "adi,current-channel".
>>
>> Signed-off-by: Dumitru Ceclan <[email protected]>
> I like the common-mode-channel solution to the pseudo differential
> description. It makes things explicit whilst avoiding an ugly differential
> but not differential mess.
>
> However, it feels like a general thing to me not a vendor specific one.
> Perhaps makes sense to put in adc.yaml?
>
Sure
> One other question that is more me being curious and failing to understand
> the datasheet than a request to change anything.
>> ---
>> .../devicetree/bindings/iio/adc/adi,ad7173.yaml | 192 ++++++++++++++++++++-
>> 1 file changed, 190 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>> index ea6cfcd0aff4..d8474eee553e 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>> @@ -19,7 +19,18 @@ description: |
>> primarily for measurement of signals close to DC but also delivers
>> outstanding performance with input bandwidths out to ~10kHz.
>>
>> + Analog Devices AD411x ADC's:
>> + The AD411X family encompasses a series of low power, low noise, 24-bit,
>> + sigma-delta analog-to-digital converters that offer a versatile range of
>> + specifications. They integrate an analog front end suitable for processing
>> + fully differential/single-ended and bipolar voltage inputs.
>> +
>> Datasheets for supported chips:
>> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4111.pdf
>> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4112.pdf
>> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4114.pdf
>> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4115.pdf
>> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4116.pdf
>> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
>> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-4.pdf
>> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
>> @@ -31,6 +42,11 @@ description: |
>> properties:
>> compatible:
>> enum:
>> + - adi,ad4111
>> + - adi,ad4112
>> + - adi,ad4114
>> + - adi,ad4115
>> + - adi,ad4116
>> - adi,ad7172-2
>> - adi,ad7172-4
>> - adi,ad7173-8
>> @@ -129,10 +145,54 @@ patternProperties:
>> maximum: 15
>>
>> diff-channels:
>> + description: |
>> + This property is used for defining the inputs of a differential
>> + voltage channel. The first value is the positive input and the second
>> + value is the negative input of the channel.
>> +
>> + Family AD411x supports a dedicated VINCOM voltage input.
>> + To select it set the second channel to 16.
>> + (VIN2, VINCOM) -> diff-channels = <2 16>
>> +
>> + There are special values that can be selected besides the voltage
>> + analog inputs:
>> + 21: REF+
>> + 22: REF−
>> + Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
>> + 19: ((AVDD1 − AVSS)/5)+
>> + 20: ((AVDD1 − AVSS)/5)−
>
> That's what it says on the datasheet (so fine to copy that here) but I'm curious, what does
> that mean in practice? How can we have negative and postive signals of the difference
> between two power supply voltages where I'm fairly sure AVDD1 always greater than AVSS.
>
I have not tested that as I do not have a model that supports this wired up.
If I had to guess they are the same signal but one should be connected to the
positive input, one to the negative input...but I could be wrong.
> Anyhow, that's a problem for the person reading the datasheet to figure out :)
>
>> +
>> items:
>> minimum: 0
>> maximum: 31
>>
>> + single-channel:
>> + description: |
>> + This property is used for defining a current channel or the positive
>> + input of a voltage channel (single-ended or pseudo-differential).
>> +
>> + Models AD4111 and AD4112 support current channels.
>> + Example: (IIN2+, IIN2−) -> single-channel = <2>
>> + To correctly configure a current channel set the "adi,current-channel"
>> + property to true.
>> +
>> + To configure a single-ended/pseudo-differential channel set the
>> + "adi,common-mode-channel" property to the desired negative voltage input.
>> +
>> + When used as a voltage channel, special inputs are valid as well.
>> + minimum: 0
>> + maximum: 31
>> +
>> + adi,common-mode-channel:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + This property is used for defining the negative input of a
>> + single-ended or pseudo-differential voltage channel.
>> +
>> + Special inputs are valid as well.
>> + minimum: 0
>> + maximum: 31
>> +
>
On 01/06/2024 22:19, Jonathan Cameron wrote:
> On Fri, 31 May 2024 22:42:32 +0300
> Dumitru Ceclan via B4 Relay <[email protected]> wrote:
>
>> From: Dumitru Ceclan <[email protected]>
>>
>> Add support for AD4111/AD4112/AD4114/AD4115/AD4116.
>>
>> The AD411X family encompasses a series of low power, low noise, 24-bit,
>> sigma-delta analog-to-digital converters that offer a versatile range of
>> specifications.
>>
>> This family of ADCs integrates an analog front end suitable for processing
>> both fully differential and single-ended, bipolar voltage inputs
>> addressing a wide array of industrial and instrumentation requirements.
>>
>> - All ADCs have inputs with a precision voltage divider with a division
>> ratio of 10.
>> - AD4116 has 5 low level inputs without a voltage divider.
>> - AD4111 and AD4112 support current inputs (0 mA to 20 mA) using a 50ohm
>> shunt resistor.
>>
>> Signed-off-by: Dumitru Ceclan <[email protected]>
> Hi Dumitru,
>
> A follow on comment on the validation code.
> Also there is some good docs for the sampling frequency but are they
> actually related to the rest of this change? They also raise
> questions about ABI compliance that we may want to deal with as
> a follow up patch.
>
> A few other trivial things inline.
>
> This is looking pretty good, so hopefully we'll get the last few corners
> sorted in v5.
>
> Thanks,
>
> Jonathan
>
>
>> ---
>> drivers/iio/adc/ad7173.c | 336 +++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 307 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
>> index ed8ff8c5f343..91ff984eedf4 100644
>> --- a/drivers/iio/adc/ad7173.c
>> +++ b/drivers/iio/adc/ad7173.c
>> @@ -1,8 +1,9 @@
>
>> #define AD7173_INTERFACE_DATA_STAT BIT(6)
>> @@ -125,26 +132,46 @@
>> #define AD7173_VOLTAGE_INT_REF_uV 2500000
>> #define AD7173_TEMP_SENSIIVITY_uV_per_C 477
>> #define AD7177_ODR_START_VALUE 0x07
>> +#define AD4111_SHUNT_RESISTOR_OHM 50
>> +#define AD4111_DIVIDER_RATIO 10
>> +#define AD411X_VCOM_INPUT 0X10
>
> AD4111_VCOM_INPUT . Looks like one wildcard escaped an earlier edit?
>
>> +#define AD4111_CURRENT_CHAN_CUTOFF 16
>>
>> @@ -736,6 +918,21 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
>> return ret;
>>
>> switch (info) {
>> + /*
>> + * This attribute sets the sampling frequency to each channel individually.
>
> frequency for each channel?
>
>> + * There are no issues for raw or buffered reads of an individual channel.
>> + *
>> + * When multiple channels are enabled in buffered mode, the effective
>> + * sampling rate of a channel is lowered in correlation to the number
>> + * of channels enabled and the sampling rate of the other channels.
>> + *
>> + * Example: 3 channels enabled with rates CH1:6211sps CH2,CH3:10sps
>> + * While the reading of CH1 takes only 0.16ms, the reading of CH2 and CH3
>> + * will take 100ms each.
>> + *
>> + * This will cause the reading of CH1 to be actually done once every
>> + * 200.16ms, an effective rate of 4.99sps.
>
> Hmm. This is a bit unfortunate as if I understand correctly that's not really what
> people will expect when they configure the sampling frequency. However I can't immediately
> think of a better solution. You could let userspace write a value that is cached
> then attempt to get as near as possible as channels are enabled.
>
> Still this looks like a documentation enhancement of existing behavior
> in which case any functional change can be in a future patch.
> However I don't think the docs update belongs in this patch unless
> I'm missing some reason for it?
>
Well, it would seem like this exact behaviour is already documented:
"
What: /sys/bus/iio/devices/iio:deviceX/in_voltageX_sampling_frequency
What: /sys/bus/iio/devices/iio:deviceX/in_powerY_sampling_frequency
What: /sys/bus/iio/devices/iio:deviceX/in_currentZ_sampling_frequency
KernelVersion: 5.20
Contact: [email protected]
Description:
Some devices have separate controls of sampling frequency for
individual channels. If multiple channels are enabled in a scan,
then the sampling_frequency of the scan may be computed from the
per channel sampling frequencies.
"
Does it still make sense to keep this comment here? But if kept, yeah, a different patch
...
On Mon, 3 Jun 2024 13:11:33 +0300
"Ceclan, Dumitru" <[email protected]> wrote:
> On 01/06/2024 22:19, Jonathan Cameron wrote:
> > On Fri, 31 May 2024 22:42:32 +0300
> > Dumitru Ceclan via B4 Relay <[email protected]> wrote:
> >
> >> From: Dumitru Ceclan <[email protected]>
> >>
> >> Add support for AD4111/AD4112/AD4114/AD4115/AD4116.
> >>
> >> The AD411X family encompasses a series of low power, low noise, 24-bit,
> >> sigma-delta analog-to-digital converters that offer a versatile range of
> >> specifications.
> >>
> >> This family of ADCs integrates an analog front end suitable for processing
> >> both fully differential and single-ended, bipolar voltage inputs
> >> addressing a wide array of industrial and instrumentation requirements.
> >>
> >> - All ADCs have inputs with a precision voltage divider with a division
> >> ratio of 10.
> >> - AD4116 has 5 low level inputs without a voltage divider.
> >> - AD4111 and AD4112 support current inputs (0 mA to 20 mA) using a 50ohm
> >> shunt resistor.
> >>
> >> Signed-off-by: Dumitru Ceclan <[email protected]>
> > Hi Dumitru,
> >
> > A follow on comment on the validation code.
> > Also there is some good docs for the sampling frequency but are they
> > actually related to the rest of this change? They also raise
> > questions about ABI compliance that we may want to deal with as
> > a follow up patch.
> >
> > A few other trivial things inline.
> >
> > This is looking pretty good, so hopefully we'll get the last few corners
> > sorted in v5.
> >
> > Thanks,
> >
> > Jonathan
> >
> >
> >> ---
> >> drivers/iio/adc/ad7173.c | 336 +++++++++++++++++++++++++++++++++++++++++++----
> >> 1 file changed, 307 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> >> index ed8ff8c5f343..91ff984eedf4 100644
> >> --- a/drivers/iio/adc/ad7173.c
> >> +++ b/drivers/iio/adc/ad7173.c
> >> @@ -1,8 +1,9 @@
> >
> >> #define AD7173_INTERFACE_DATA_STAT BIT(6)
> >> @@ -125,26 +132,46 @@
> >> #define AD7173_VOLTAGE_INT_REF_uV 2500000
> >> #define AD7173_TEMP_SENSIIVITY_uV_per_C 477
> >> #define AD7177_ODR_START_VALUE 0x07
> >> +#define AD4111_SHUNT_RESISTOR_OHM 50
> >> +#define AD4111_DIVIDER_RATIO 10
> >> +#define AD411X_VCOM_INPUT 0X10
> >
> > AD4111_VCOM_INPUT . Looks like one wildcard escaped an earlier edit?
> >
> >> +#define AD4111_CURRENT_CHAN_CUTOFF 16
> >>
> >> @@ -736,6 +918,21 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
> >> return ret;
> >>
> >> switch (info) {
> >> + /*
> >> + * This attribute sets the sampling frequency to each channel individually.
> >
> > frequency for each channel?
> >
> >> + * There are no issues for raw or buffered reads of an individual channel.
> >> + *
> >> + * When multiple channels are enabled in buffered mode, the effective
> >> + * sampling rate of a channel is lowered in correlation to the number
> >> + * of channels enabled and the sampling rate of the other channels.
> >> + *
> >> + * Example: 3 channels enabled with rates CH1:6211sps CH2,CH3:10sps
> >> + * While the reading of CH1 takes only 0.16ms, the reading of CH2 and CH3
> >> + * will take 100ms each.
> >> + *
> >> + * This will cause the reading of CH1 to be actually done once every
> >> + * 200.16ms, an effective rate of 4.99sps.
> >
> > Hmm. This is a bit unfortunate as if I understand correctly that's not really what
> > people will expect when they configure the sampling frequency. However I can't immediately
> > think of a better solution. You could let userspace write a value that is cached
> > then attempt to get as near as possible as channels are enabled.
> >
> > Still this looks like a documentation enhancement of existing behavior
> > in which case any functional change can be in a future patch.
> > However I don't think the docs update belongs in this patch unless
> > I'm missing some reason for it?
> >
>
> Well, it would seem like this exact behaviour is already documented:
>
> "
> What: /sys/bus/iio/devices/iio:deviceX/in_voltageX_sampling_frequency
> What: /sys/bus/iio/devices/iio:deviceX/in_powerY_sampling_frequency
> What: /sys/bus/iio/devices/iio:deviceX/in_currentZ_sampling_frequency
> KernelVersion: 5.20
> Contact: [email protected]
> Description:
> Some devices have separate controls of sampling frequency for
> individual channels. If multiple channels are enabled in a scan,
> then the sampling_frequency of the scan may be computed from the
> per channel sampling frequencies.
> "
I guess we've hit this before and added very specific documentation around this
that I'd forgotten about. Thanks for pointing it out!
> Does it still make sense to keep this comment here? But if kept, yeah, a different patch
I think there is no harm in having the additional info. Not as if everyone will
check the ABI docs whilst trying to understand the driver code. I didn't for one :(
Jonathan
>
> ...
On Mon, 3 Jun 2024 12:46:10 +0300
"Ceclan, Dumitru" <[email protected]> wrote:
> On 01/06/2024 21:35, Jonathan Cameron wrote:
> > On Fri, 31 May 2024 22:42:27 +0300
> > Dumitru Ceclan via B4 Relay <[email protected]> wrote:
> >
> >> From: Dumitru Ceclan <[email protected]>
> >>
> >> Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
> >>
> >> AD411x family ADCs support a VCOM pin. The purpose of this pin is to
> >> offer a dedicated common-mode voltage input for single-ended channels.
> >> This pin is specified as supporting a differential channel with VIN10 on
> >> model AD4116.
> >>
> >> AD4111/AD4112 support current channels. Support is implemented using
> >> single-channel and "adi,current-channel".
> >>
> >> Signed-off-by: Dumitru Ceclan <[email protected]>
> > I like the common-mode-channel solution to the pseudo differential
> > description. It makes things explicit whilst avoiding an ugly differential
> > but not differential mess.
> >
> > However, it feels like a general thing to me not a vendor specific one.
> > Perhaps makes sense to put in adc.yaml?
> >
>
> Sure
>
> > One other question that is more me being curious and failing to understand
> > the datasheet than a request to change anything.
> >> ---
> >> .../devicetree/bindings/iio/adc/adi,ad7173.yaml | 192 ++++++++++++++++++++-
> >> 1 file changed, 190 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> >> index ea6cfcd0aff4..d8474eee553e 100644
> >> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> >> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> >> @@ -19,7 +19,18 @@ description: |
> >> primarily for measurement of signals close to DC but also delivers
> >> outstanding performance with input bandwidths out to ~10kHz.
> >>
> >> + Analog Devices AD411x ADC's:
> >> + The AD411X family encompasses a series of low power, low noise, 24-bit,
> >> + sigma-delta analog-to-digital converters that offer a versatile range of
> >> + specifications. They integrate an analog front end suitable for processing
> >> + fully differential/single-ended and bipolar voltage inputs.
> >> +
> >> Datasheets for supported chips:
> >> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4111.pdf
> >> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4112.pdf
> >> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4114.pdf
> >> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4115.pdf
> >> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD4116.pdf
> >> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
> >> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-4.pdf
> >> https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
> >> @@ -31,6 +42,11 @@ description: |
> >> properties:
> >> compatible:
> >> enum:
> >> + - adi,ad4111
> >> + - adi,ad4112
> >> + - adi,ad4114
> >> + - adi,ad4115
> >> + - adi,ad4116
> >> - adi,ad7172-2
> >> - adi,ad7172-4
> >> - adi,ad7173-8
> >> @@ -129,10 +145,54 @@ patternProperties:
> >> maximum: 15
> >>
> >> diff-channels:
> >> + description: |
> >> + This property is used for defining the inputs of a differential
> >> + voltage channel. The first value is the positive input and the second
> >> + value is the negative input of the channel.
> >> +
> >> + Family AD411x supports a dedicated VINCOM voltage input.
> >> + To select it set the second channel to 16.
> >> + (VIN2, VINCOM) -> diff-channels = <2 16>
> >> +
> >> + There are special values that can be selected besides the voltage
> >> + analog inputs:
> >> + 21: REF+
> >> + 22: REF−
> >> + Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
> >> + 19: ((AVDD1 − AVSS)/5)+
> >> + 20: ((AVDD1 − AVSS)/5)−
> >
> > That's what it says on the datasheet (so fine to copy that here) but I'm curious, what does
> > that mean in practice? How can we have negative and postive signals of the difference
> > between two power supply voltages where I'm fairly sure AVDD1 always greater than AVSS.
> >
>
> I have not tested that as I do not have a model that supports this wired up.
> If I had to guess they are the same signal but one should be connected to the
> positive input, one to the negative input...but I could be wrong.
If they are, then as far as I we are concerned is this one channel with two
representations depending on whether it is 1st or 2nd in the list?
Can we use one number and hide that detail in the driver?
Seems odd though if that is the case.
I guess if we find out later this is the case we can tighten the binding to
enforce the right one instead of squashing them to one value, but that
is a bit ugly. Any chance of digging out the info? If not we can go ahead
but ideally answering things like this make a our life easier in the long run.
Jonathan
>
> > Anyhow, that's a problem for the person reading the datasheet to figure out :)
> >
> >> +
> >> items:
> >> minimum: 0
> >> maximum: 31
> >>
> >> + single-channel:
> >> + description: |
> >> + This property is used for defining a current channel or the positive
> >> + input of a voltage channel (single-ended or pseudo-differential).
> >> +
> >> + Models AD4111 and AD4112 support current channels.
> >> + Example: (IIN2+, IIN2−) -> single-channel = <2>
> >> + To correctly configure a current channel set the "adi,current-channel"
> >> + property to true.
> >> +
> >> + To configure a single-ended/pseudo-differential channel set the
> >> + "adi,common-mode-channel" property to the desired negative voltage input.
> >> +
> >> + When used as a voltage channel, special inputs are valid as well.
> >> + minimum: 0
> >> + maximum: 31
> >> +
> >> + adi,common-mode-channel:
> >> + $ref: /schemas/types.yaml#/definitions/uint32
> >> + description:
> >> + This property is used for defining the negative input of a
> >> + single-ended or pseudo-differential voltage channel.
> >> +
> >> + Special inputs are valid as well.
> >> + minimum: 0
> >> + maximum: 31
> >> +
> >
>
On 03/06/2024 23:00, Jonathan Cameron wrote:
> On Mon, 3 Jun 2024 12:46:10 +0300
> "Ceclan, Dumitru" <[email protected]> wrote:
>
>> On 01/06/2024 21:35, Jonathan Cameron wrote:
>>> On Fri, 31 May 2024 22:42:27 +0300
>>> Dumitru Ceclan via B4 Relay <[email protected]> wrote:
>>>
>>>> From: Dumitru Ceclan <[email protected]>
...
>>>> + Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
>>>> + 19: ((AVDD1 − AVSS)/5)+
>>>> + 20: ((AVDD1 − AVSS)/5)−
>>>
>>> That's what it says on the datasheet (so fine to copy that here) but I'm curious, what does
>>> that mean in practice? How can we have negative and postive signals of the difference
>>> between two power supply voltages where I'm fairly sure AVDD1 always greater than AVSS.
>>>
>>
>> I have not tested that as I do not have a model that supports this wired up.
>> If I had to guess they are the same signal but one should be connected to the
>> positive input, one to the negative input...but I could be wrong.
>
> If they are, then as far as I we are concerned is this one channel with two
> representations depending on whether it is 1st or 2nd in the list?
> Can we use one number and hide that detail in the driver?
>
> Seems odd though if that is the case.
>
> I guess if we find out later this is the case we can tighten the binding to
> enforce the right one instead of squashing them to one value, but that
> is a bit ugly. Any chance of digging out the info? If not we can go ahead
> but ideally answering things like this make a our life easier in the long run.
>
> Jonathan
>
"(Avdd1/Avss)/5+ as positive input and (Avdd/Avss)/5- as negative
this is used for monitoring power supplies, the inputs must be selected in pair"
Perhaps it's an internal voltage divider...? I dunno
So it seems like this cannot be used as a common mode voltage input.
I'll restrict the driver to only allow these inputs paired together
and rename the define for these selections.
On Wed, 5 Jun 2024 09:54:31 +0300
"Ceclan, Dumitru" <[email protected]> wrote:
> On 03/06/2024 23:00, Jonathan Cameron wrote:
> > On Mon, 3 Jun 2024 12:46:10 +0300
> > "Ceclan, Dumitru" <[email protected]> wrote:
> >
> >> On 01/06/2024 21:35, Jonathan Cameron wrote:
> >>> On Fri, 31 May 2024 22:42:27 +0300
> >>> Dumitru Ceclan via B4 Relay <[email protected]> wrote:
> >>>
> >>>> From: Dumitru Ceclan <[email protected]>
>
> ...
>
> >>>> + Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
> >>>> + 19: ((AVDD1 − AVSS)/5)+
> >>>> + 20: ((AVDD1 − AVSS)/5)−
> >>>
> >>> That's what it says on the datasheet (so fine to copy that here) but I'm curious, what does
> >>> that mean in practice? How can we have negative and postive signals of the difference
> >>> between two power supply voltages where I'm fairly sure AVDD1 always greater than AVSS.
> >>>
> >>
> >> I have not tested that as I do not have a model that supports this wired up.
> >> If I had to guess they are the same signal but one should be connected to the
> >> positive input, one to the negative input...but I could be wrong.
> >
> > If they are, then as far as I we are concerned is this one channel with two
> > representations depending on whether it is 1st or 2nd in the list?
> > Can we use one number and hide that detail in the driver?
> >
> > Seems odd though if that is the case.
> >
> > I guess if we find out later this is the case we can tighten the binding to
> > enforce the right one instead of squashing them to one value, but that
> > is a bit ugly. Any chance of digging out the info? If not we can go ahead
> > but ideally answering things like this make a our life easier in the long run.
> >
> > Jonathan
> >
>
> "(Avdd1/Avss)/5+ as positive input and (Avdd/Avss)/5- as negative
> this is used for monitoring power supplies, the inputs must be selected in pair"
> Perhaps it's an internal voltage divider...? I dunno
>
> So it seems like this cannot be used as a common mode voltage input.
> I'll restrict the driver to only allow these inputs paired together
> and rename the define for these selections.
Most mysterious :) I'd be interested to know what value it reads
back if you ever get the part.
Ah well, great to have gotten that extra detail even if it leaves
more questions!
Jonathan
>
>
>
On 06/06/2024 22:58, Jonathan Cameron wrote:
> On Wed, 5 Jun 2024 09:54:31 +0300
> "Ceclan, Dumitru" <[email protected]> wrote:
>
>> On 03/06/2024 23:00, Jonathan Cameron wrote:
>>> On Mon, 3 Jun 2024 12:46:10 +0300
>>> "Ceclan, Dumitru" <[email protected]> wrote:
>>>
>>>> On 01/06/2024 21:35, Jonathan Cameron wrote:
>>>>> On Fri, 31 May 2024 22:42:27 +0300
>>>>> Dumitru Ceclan via B4 Relay <[email protected]> wrote:
>>>>>
>>>>>> From: Dumitru Ceclan <[email protected]>
>>
>> ...
>>
>>>>>> + Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
>>>>>> + 19: ((AVDD1 − AVSS)/5)+
>>>>>> + 20: ((AVDD1 − AVSS)/5)−
>>>>>
>>>>> That's what it says on the datasheet (so fine to copy that here) but I'm curious, what does
>>>>> that mean in practice? How can we have negative and postive signals of the difference
>>>>> between two power supply voltages where I'm fairly sure AVDD1 always greater than AVSS.
>>>>>
>>>>
>>>> I have not tested that as I do not have a model that supports this wired up.
>>>> If I had to guess they are the same signal but one should be connected to the
>>>> positive input, one to the negative input...but I could be wrong.
>>>
>>> If they are, then as far as I we are concerned is this one channel with two
>>> representations depending on whether it is 1st or 2nd in the list?
>>> Can we use one number and hide that detail in the driver?
>>>
>>> Seems odd though if that is the case.
>>>
>>> I guess if we find out later this is the case we can tighten the binding to
>>> enforce the right one instead of squashing them to one value, but that
>>> is a bit ugly. Any chance of digging out the info? If not we can go ahead
>>> but ideally answering things like this make a our life easier in the long run.
>>>
>>> Jonathan
>>>
>>
>> "(Avdd1/Avss)/5+ as positive input and (Avdd/Avss)/5- as negative
>> this is used for monitoring power supplies, the inputs must be selected in pair"
>> Perhaps it's an internal voltage divider...? I dunno
>>
>> So it seems like this cannot be used as a common mode voltage input.
>> I'll restrict the driver to only allow these inputs paired together
>> and rename the define for these selections.
> Most mysterious :) I'd be interested to know what value it reads
> back if you ever get the part.
>
My best guess now is that the reason for /5 is so that you can measure
the AVDD AVSS difference using the internal 2.5V reference.
So for AVDD 5V, AVSS 0V using the internal ref it would read 1V
I'll let you know if I test this
> Ah well, great to have gotten that extra detail even if it leaves
> more questions!
>
> Jonathan
>