* [PATCH v2 0/9] Add support for AD411x
@ 2024-05-14 7:22 Dumitru Ceclan via B4 Relay
2024-05-14 7:22 ` [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x Dumitru Ceclan via B4 Relay
` (9 more replies)
0 siblings, 10 replies; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan,
Jonathan Cameron
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 responsability 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 patch:
(iio: adc: ad7173: Use device_for_each_child_node_scoped() to simplify error paths.)
https://lore.kernel.org/all/20240330190849.1321065-6-jic23@kernel.org
Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com>
---
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/20240401-ad4111-v1-0-34618a9cc502@analog.com
---
Dumitru Ceclan (9):
dt-bindings: adc: ad7173: add support for ad411x
iio: adc: ad7173: fix buffers enablement for ad7176-2
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: Add ad7173_device_info names
iio: adc: ad7173: Remove index from temp channel
iio: adc: ad7173: Add support for AD411x devices
iio: adc: ad7173: Reduce device info struct size
.../devicetree/bindings/iio/adc/adi,ad7173.yaml | 118 +++++-
drivers/iio/adc/ad7173.c | 438 ++++++++++++++++++---
2 files changed, 497 insertions(+), 59 deletions(-)
---
base-commit: 5ab61121a34759eb2418977f0b3589b7edc57776
change-id: 20240312-ad4111-7eeb34eb4a5f
Best regards,
--
Dumitru Ceclan <dumitru.ceclan@analog.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
@ 2024-05-14 7:22 ` Dumitru Ceclan via B4 Relay
2024-05-15 22:37 ` David Lechner
2024-05-14 7:22 ` [PATCH v2 2/9] iio: adc: ad7173: fix buffers enablement for ad7176-2 Dumitru Ceclan via B4 Relay
` (8 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
From: Dumitru Ceclan <dumitru.ceclan@analog.com>
Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
AD411x family ADCs support a VCOM pin, dedicated for single-ended usage.
AD4111/AD4112 support current channels, usage is implemented by
specifying channel reg values bigger than 15.
Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
.../devicetree/bindings/iio/adc/adi,ad7173.yaml | 118 ++++++++++++++++++++-
1 file changed, 117 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
index ea6cfcd0aff4..6cc3514f5ed8 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,6 +145,31 @@ patternProperties:
maximum: 15
diff-channels:
+ description: |
+ For using current channels specify select the current inputs
+ and enable the adi,current-channel property.
+
+ Family AD411x supports a dedicated VCOM voltage input.
+ To select it set the second channel to 16.
+ (VIN2, VCOM) -> 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)−
+ Supported only by AD4111, AD4112:
+ 12: IIN3+
+ 11: IIN3−
+ 13: IIN2+
+ 10: IIN2−
+ 14: IIN1+
+ 9: IIN1−
+ 15: IIN0+
+ 8: IIN0−
+
items:
minimum: 0
maximum: 31
@@ -154,6 +195,23 @@ 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
+
+ adi,channel-type:
+ description:
+ Used to differentiate between different channel types as the device
+ register configurations are the same for all usage types.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum:
+ - single-ended
+ - pseudo-differential
+ - differential
+ default: differential
+
required:
- reg
- diff-channels
@@ -166,7 +224,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 +244,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 +298,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]
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 2/9] iio: adc: ad7173: fix buffers enablement for ad7176-2
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
2024-05-14 7:22 ` [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x Dumitru Ceclan via B4 Relay
@ 2024-05-14 7:22 ` Dumitru Ceclan via B4 Relay
2024-05-14 7:22 ` [PATCH v2 3/9] iio: adc: ad7173: refactor channel configuration parsing Dumitru Ceclan via B4 Relay
` (7 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
From: Dumitru Ceclan <dumitru.ceclan@analog.com>
AD7176-2 does not feature input buffers and marks corespondent register
bits as read only. Enable buffers only on supported models.
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
drivers/iio/adc/ad7173.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index f6d29abe1d04..850574437bda 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -145,6 +145,7 @@ struct ad7173_device_info {
unsigned int id;
char *name;
bool has_temp;
+ bool has_input_buf;
bool has_int_ref;
bool has_ref2;
u8 num_gpios;
@@ -212,6 +213,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 4,
.num_gpios = 2,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.clock = 2 * HZ_PER_MHZ,
.sinc5_data_rates = ad7173_sinc5_data_rates,
@@ -224,6 +226,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 8,
.num_gpios = 4,
.has_temp = false,
+ .has_input_buf = true,
.has_ref2 = true,
.clock = 2 * HZ_PER_MHZ,
.sinc5_data_rates = ad7173_sinc5_data_rates,
@@ -237,6 +240,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 8,
.num_gpios = 4,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.has_ref2 = true,
.clock = 2 * HZ_PER_MHZ,
@@ -251,6 +255,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 4,
.num_gpios = 2,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.clock = 16 * HZ_PER_MHZ,
.sinc5_data_rates = ad7175_sinc5_data_rates,
@@ -263,6 +268,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 8,
.num_gpios = 4,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.has_ref2 = true,
.clock = 16 * HZ_PER_MHZ,
@@ -277,6 +283,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 4,
.num_gpios = 2,
.has_temp = false,
+ .has_input_buf = false,
.has_int_ref = true,
.clock = 16 * HZ_PER_MHZ,
.sinc5_data_rates = ad7175_sinc5_data_rates,
@@ -289,6 +296,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_configs = 4,
.num_gpios = 2,
.has_temp = true,
+ .has_input_buf = true,
.has_int_ref = true,
.clock = 16 * HZ_PER_MHZ,
.odr_start_value = AD7177_ODR_START_VALUE,
@@ -932,7 +940,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
AD7173_CH_ADDRESS(chan_arr[chan_index].channel,
chan_arr[chan_index].channel2);
chan_st_priv->cfg.bipolar = false;
- chan_st_priv->cfg.input_buf = true;
+ chan_st_priv->cfg.input_buf = st->info->has_input_buf;
chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
st->adc_mode |= AD7173_ADC_MODE_REF_EN;
@@ -989,7 +997,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
chan_st_priv->chan_reg = chan_index;
- chan_st_priv->cfg.input_buf = true;
+ 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");
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 3/9] iio: adc: ad7173: refactor channel configuration parsing
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
2024-05-14 7:22 ` [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x Dumitru Ceclan via B4 Relay
2024-05-14 7:22 ` [PATCH v2 2/9] iio: adc: ad7173: fix buffers enablement for ad7176-2 Dumitru Ceclan via B4 Relay
@ 2024-05-14 7:22 ` Dumitru Ceclan via B4 Relay
2024-05-19 16:58 ` Jonathan Cameron
2024-05-14 7:22 ` [PATCH v2 4/9] iio: adc: ad7173: refactor ain and vref selection Dumitru Ceclan via B4 Relay
` (6 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan,
Jonathan Cameron
From: Dumitru Ceclan <dumitru.ceclan@analog.com>
Move configurations regarding number of channels from
*_fw_parse_device_config to *_fw_parse_channel_config.
Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://lore.kernel.org/all/20240303162148.3ad91aa2@jic23-huawei/
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
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 850574437bda..d5ad3f01dec7 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -918,7 +918,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);
@@ -1013,7 +1029,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];
@@ -1072,16 +1087,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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 4/9] iio: adc: ad7173: refactor ain and vref selection
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
` (2 preceding siblings ...)
2024-05-14 7:22 ` [PATCH v2 3/9] iio: adc: ad7173: refactor channel configuration parsing Dumitru Ceclan via B4 Relay
@ 2024-05-14 7:22 ` Dumitru Ceclan via B4 Relay
2024-05-15 22:54 ` David Lechner
2024-05-19 19:17 ` Jonathan Cameron
2024-05-14 7:22 ` [PATCH v2 5/9] iio: adc: ad7173: add support for special inputs Dumitru Ceclan via B4 Relay
` (5 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
From: Dumitru Ceclan <dumitru.ceclan@analog.com>
Move validation of analog inputs and reference voltage selection to
separate functions to reduce the size of the channel config parsing
function and improve readability.
Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
drivers/iio/adc/ad7173.c | 62 ++++++++++++++++++++++++++++++++++--------------
1 file changed, 44 insertions(+), 18 deletions(-)
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index d5ad3f01dec7..fb33534d63a9 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -911,6 +911,44 @@ static int ad7173_register_clk_provider(struct iio_dev *indio_dev)
&st->int_clk_hw);
}
+static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
+ unsigned int ain[2])
+{
+ struct device *dev = &st->sd.spi->dev;
+
+ for (int i = 0; i < 2; i++) {
+ if (ain[i] < st->info->num_inputs)
+ continue;
+
+ return dev_err_probe(dev, -EINVAL,
+ "Input pin number out of range for pair (%d %d).\n",
+ ain[0], ain[1]);
+ }
+
+ return 0;
+}
+
+static int ad7173_validate_reference(struct ad7173_state *st, int ref_sel)
+{
+ struct device *dev = &st->sd.spi->dev;
+ int ret;
+
+ if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF && !st->info->has_int_ref)
+ return dev_err_probe(dev, -EINVAL,
+ "Internal reference is not available on current model.\n");
+
+ if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 && !st->info->has_ref2)
+ return dev_err_probe(dev, -EINVAL,
+ "External reference 2 is not available on current model.\n");
+
+ ret = ad7173_get_ref_voltage_milli(st, ref_sel);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Cannot use reference %u\n",
+ ref_sel);
+
+ return 0;
+}
+
static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
{
struct ad7173_channel *chans_st_arr, *chan_st_priv;
@@ -971,11 +1009,9 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
if (ret)
return ret;
- if (ain[0] >= st->info->num_inputs ||
- ain[1] >= st->info->num_inputs)
- return dev_err_probe(dev, -EINVAL,
- "Input pin number out of range for pair (%d %d).\n",
- ain[0], ain[1]);
+ ret = ad7173_validate_voltage_ain_inputs(st, ain);
+ if (ret)
+ return ret;
ret = fwnode_property_match_property_string(child,
"adi,reference-select",
@@ -986,19 +1022,9 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
else
ref_sel = ret;
- if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF &&
- !st->info->has_int_ref)
- return dev_err_probe(dev, -EINVAL,
- "Internal reference is not available on current model.\n");
-
- if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 && !st->info->has_ref2)
- return dev_err_probe(dev, -EINVAL,
- "External reference 2 is not available on current model.\n");
-
- ret = ad7173_get_ref_voltage_milli(st, ref_sel);
- if (ret < 0)
- return dev_err_probe(dev, ret,
- "Cannot use reference %u\n", ref_sel);
+ ret = ad7173_validate_reference(st, ref_sel);
+ if (ret)
+ return ret;
if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF)
st->adc_mode |= AD7173_ADC_MODE_REF_EN;
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 5/9] iio: adc: ad7173: add support for special inputs
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
` (3 preceding siblings ...)
2024-05-14 7:22 ` [PATCH v2 4/9] iio: adc: ad7173: refactor ain and vref selection Dumitru Ceclan via B4 Relay
@ 2024-05-14 7:22 ` Dumitru Ceclan via B4 Relay
2024-05-15 23:27 ` David Lechner
2024-05-14 7:22 ` [PATCH v2 6/9] iio: adc: ad7173: Add ad7173_device_info names Dumitru Ceclan via B4 Relay
` (4 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
From: Dumitru Ceclan <dumitru.ceclan@analog.com>
Add support for selecting REF+ and REF- inputs on all models.
Add support for selecting ((AVDD1 − AVSS)/5) inputs
on supported models.
Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
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 fb33534d63a9..1e9ba3070770 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -65,6 +65,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
@@ -145,6 +149,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;
@@ -215,6 +221,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),
@@ -228,6 +235,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),
@@ -243,6 +251,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),
@@ -257,6 +266,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),
@@ -271,6 +281,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),
@@ -285,6 +296,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),
@@ -298,6 +310,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,
@@ -920,6 +933,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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 6/9] iio: adc: ad7173: Add ad7173_device_info names
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
` (4 preceding siblings ...)
2024-05-14 7:22 ` [PATCH v2 5/9] iio: adc: ad7173: add support for special inputs Dumitru Ceclan via B4 Relay
@ 2024-05-14 7:22 ` Dumitru Ceclan via B4 Relay
2024-05-15 23:32 ` David Lechner
2024-05-14 7:22 ` [PATCH v2 7/9] iio: adc: ad7173: Remove index from temp channel Dumitru Ceclan via B4 Relay
` (3 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
From: Dumitru Ceclan <dumitru.ceclan@analog.com>
Add missing names from the device info struct for 3 models to ensure
consistency with the rest of the models.
Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
drivers/iio/adc/ad7173.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 1e9ba3070770..d965b66d4d5a 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -227,6 +227,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
},
[ID_AD7172_4] = {
+ .name = "ad7172-4",
.id = AD7172_4_ID,
.num_inputs = 9,
.num_channels = 8,
@@ -272,6 +273,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
},
[ID_AD7175_8] = {
+ .name = "ad7175-8",
.id = AD7175_8_ID,
.num_inputs = 17,
.num_channels = 16,
@@ -302,6 +304,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
.num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
},
[ID_AD7177_2] = {
+ .name = "ad7177-2",
.id = AD7177_ID,
.num_inputs = 5,
.num_channels = 4,
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 7/9] iio: adc: ad7173: Remove index from temp channel
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
` (5 preceding siblings ...)
2024-05-14 7:22 ` [PATCH v2 6/9] iio: adc: ad7173: Add ad7173_device_info names Dumitru Ceclan via B4 Relay
@ 2024-05-14 7:22 ` Dumitru Ceclan via B4 Relay
2024-05-15 23:34 ` David Lechner
2024-05-14 7:22 ` [PATCH v2 8/9] iio: adc: ad7173: Add support for AD411x devices Dumitru Ceclan via B4 Relay
` (2 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
From: Dumitru Ceclan <dumitru.ceclan@analog.com>
Temperature channel is unique per device, index is not needed.
This is breaking userspace: as main driver has not reached mainline yet
it won't affect users as there are none.
Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
drivers/iio/adc/ad7173.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index d965b66d4d5a..d66b47e1a186 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -828,7 +828,6 @@ static const struct iio_chan_spec ad7173_channel_template = {
static const struct iio_chan_spec ad7173_temp_iio_channel_template = {
.type = IIO_TEMP,
- .indexed = 1,
.channel = AD7173_AIN_TEMP_POS,
.channel2 = AD7173_AIN_TEMP_NEG,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 8/9] iio: adc: ad7173: Add support for AD411x devices
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
` (6 preceding siblings ...)
2024-05-14 7:22 ` [PATCH v2 7/9] iio: adc: ad7173: Remove index from temp channel Dumitru Ceclan via B4 Relay
@ 2024-05-14 7:22 ` Dumitru Ceclan via B4 Relay
2024-05-19 17:13 ` Jonathan Cameron
2024-05-14 7:22 ` [PATCH v2 9/9] iio: adc: ad7173: Reduce device info struct size Dumitru Ceclan via B4 Relay
2024-05-15 22:35 ` [PATCH v2 0/9] Add support for AD411x David Lechner
9 siblings, 1 reply; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
From: Dumitru Ceclan <dumitru.ceclan@analog.com>
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 <dumitru.ceclan@analog.com>
---
drivers/iio/adc/ad7173.c | 316 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 288 insertions(+), 28 deletions(-)
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index d66b47e1a186..f049d79380ac 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
*
@@ -75,7 +76,9 @@
#define AD7176_ID 0x0c90
#define AD7175_2_ID 0x0cd0
#define AD7172_4_ID 0x2050
-#define AD7173_ID 0x30d0
+#define AD7173_AD4111_AD4112_AD4114_ID 0x30d0
+#define AD4116_ID 0x34d0
+#define AD4115_ID 0x38d0
#define AD7175_8_ID 0x3cd0
#define AD7177_ID 0x4fd0
#define AD7173_ID_MASK GENMASK(15, 4)
@@ -106,6 +109,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)
@@ -124,11 +128,20 @@
#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 ad7173_ids {
+ ID_AD4111,
+ ID_AD4112,
+ ID_AD4114,
+ ID_AD4115,
+ ID_AD4116,
ID_AD7172_2,
ID_AD7172_4,
ID_AD7173_8,
@@ -138,22 +151,44 @@ enum ad7173_ids {
ID_AD7177_2,
};
+enum ad4111_current_channels {
+ AD4111_CURRENT_IN0P_IN0N,
+ AD4111_CURRENT_IN1P_IN1N,
+ AD4111_CURRENT_IN2P_IN2N,
+ AD4111_CURRENT_IN3P_IN3N,
+};
+
+enum ad7173_channel_types {
+ AD7173_CHAN_SINGLE_ENDED,
+ AD7173_CHAN_PSEUDO_DIFFERENTIAL,
+ AD7173_CHAN_DIFFERENTIAL,
+};
+
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 volage divider and without.
+ * These inputs cannot be mixed in the channel configuration.
+ * Does not include the VCOM input.
+ */
+ unsigned int num_voltage_inputs_with_divider;
unsigned int num_channels;
unsigned int num_configs;
- unsigned int num_inputs;
+ unsigned int num_voltage_inputs;
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;
};
@@ -195,6 +230,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 */
@@ -210,14 +263,109 @@ 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 ad7173_device_info[] = {
+ [ID_AD4111] = {
+ .name = "ad4111",
+ .id = AD7173_AD4111_AD4112_AD4114_ID,
+ .num_voltage_inputs_with_divider = 8,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_inputs = 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),
+ },
+ [ID_AD4112] = {
+ .name = "ad4112",
+ .id = AD7173_AD4111_AD4112_AD4114_ID,
+ .num_voltage_inputs_with_divider = 8,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_inputs = 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),
+ },
+ [ID_AD4114] = {
+ .name = "ad4114",
+ .id = AD7173_AD4111_AD4112_AD4114_ID,
+ .num_voltage_inputs_with_divider = 16,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_inputs = 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),
+ },
+ [ID_AD4115] = {
+ .name = "ad4115",
+ .id = AD4115_ID,
+ .num_voltage_inputs_with_divider = 16,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_inputs = 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),
+ },
+ [ID_AD4116] = {
+ .name = "ad4116",
+ .id = AD4116_ID,
+ .num_voltage_inputs_with_divider = 11,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_voltage_inputs = 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),
+ },
[ID_AD7172_2] = {
.name = "ad7172-2",
.id = AD7172_2_ID,
- .num_inputs = 5,
+ .num_voltage_inputs = 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,
@@ -229,10 +377,11 @@ static const struct ad7173_device_info ad7173_device_info[] = {
[ID_AD7172_4] = {
.name = "ad7172-4",
.id = AD7172_4_ID,
- .num_inputs = 9,
+ .num_voltage_inputs = 9,
.num_channels = 8,
.num_configs = 8,
.num_gpios = 4,
+ .higher_gpio_bits = false,
.has_temp = false,
.has_input_buf = true,
.has_ref2 = true,
@@ -243,11 +392,12 @@ static const struct ad7173_device_info ad7173_device_info[] = {
},
[ID_AD7173_8] = {
.name = "ad7173-8",
- .id = AD7173_ID,
- .num_inputs = 17,
+ .id = AD7173_AD4111_AD4112_AD4114_ID,
+ .num_voltage_inputs = 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,
@@ -260,10 +410,11 @@ static const struct ad7173_device_info ad7173_device_info[] = {
[ID_AD7175_2] = {
.name = "ad7175-2",
.id = AD7175_2_ID,
- .num_inputs = 5,
+ .num_voltage_inputs = 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,
@@ -275,10 +426,11 @@ static const struct ad7173_device_info ad7173_device_info[] = {
[ID_AD7175_8] = {
.name = "ad7175-8",
.id = AD7175_8_ID,
- .num_inputs = 17,
+ .num_voltage_inputs = 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,
@@ -291,10 +443,11 @@ static const struct ad7173_device_info ad7173_device_info[] = {
[ID_AD7176_2] = {
.name = "ad7176-2",
.id = AD7176_ID,
- .num_inputs = 5,
+ .num_voltage_inputs = 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,
@@ -306,10 +459,11 @@ static const struct ad7173_device_info ad7173_device_info[] = {
[ID_AD7177_2] = {
.name = "ad7177-2",
.id = AD7177_ID,
- .num_inputs = 5,
+ .num_voltage_inputs = 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,
@@ -328,6 +482,12 @@ static const char *const ad7173_ref_sel_str[] = {
[AD7173_SETUP_REF_SEL_AVDD1_AVSS] = "avdd",
};
+static const char *const ad7173_channel_types[] = {
+ [AD7173_CHAN_SINGLE_ENDED] = "single-ended",
+ [AD7173_CHAN_PSEUDO_DIFFERENTIAL] = "pseudo-differential",
+ [AD7173_CHAN_DIFFERENTIAL] = "differential",
+};
+
static const char *const ad7173_clk_sel[] = {
"ext-clk", "xtal"
};
@@ -360,6 +520,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;
@@ -392,7 +561,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);
@@ -687,18 +859,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_inputs_with_divider)
+ *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 ? 1 : 0);
+ 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;
@@ -707,10 +894,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;
@@ -926,13 +1117,37 @@ 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,
+ unsigned int ain[2])
+{
+ struct device *dev = &st->sd.spi->dev;
+ int i;
+
+ if (!st->info->has_current_inputs)
+ return dev_err_probe(dev, -EINVAL,
+ "Model %s does not support current channels\n",
+ st->info->name);
+
+ for (i = 0; i < ARRAY_SIZE(ad4111_current_channel_config); i++)
+ if (ad4111_current_channel_config[i] == AD7173_CH_ADDRESS(ain[0], ain[1]))
+ return 0;
+
+ return dev_err_probe(dev, -EINVAL,
+ "Current channel configuration invalid (%d, %d).\n",
+ ain[0], ain[1]);
+}
+
static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
unsigned int ain[2])
{
struct device *dev = &st->sd.spi->dev;
+ bool ain_selects_normal_input[] = {
+ ain[0] < st->info->num_voltage_inputs,
+ ain[1] < st->info->num_voltage_inputs
+ };
for (int i = 0; i < 2; 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)
@@ -943,11 +1158,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_inputs_with_divider)
+ 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_inputs_with_divider) !=
+ (ain[1] >= st->info->num_voltage_inputs_with_divider)))
+ 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;
}
@@ -979,7 +1210,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[2], 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);
@@ -1032,9 +1263,16 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
if (ret)
return ret;
- ret = ad7173_validate_voltage_ain_inputs(st, ain);
- if (ret)
- return ret;
+ is_current_chan = fwnode_property_read_bool(child, "adi,current-channel");
+ if (is_current_chan) {
+ ret = ad4111_validate_current_ain(st, ain);
+ if (ret)
+ return ret;
+ } else {
+ ret = ad7173_validate_voltage_ain_inputs(st, ain);
+ if (ret)
+ return ret;
+ }
ret = fwnode_property_match_property_string(child,
"adi,reference-select",
@@ -1058,17 +1296,26 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
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;
+ else
+ chan_st_priv->cfg.input_buf = st->info->has_input_buf;
+
+ ret = fwnode_property_match_property_string(child,
+ "adi,channel-type",
+ ad7173_channel_types,
+ ARRAY_SIZE(ad7173_channel_types));
+ chan->differential = (ret < 0 || ret == AD7173_CHAN_DIFFERENTIAL)
+ ? true : false;
+
chan_index++;
}
return 0;
@@ -1195,6 +1442,14 @@ static int ad7173_probe(struct spi_device *spi)
}
static const struct of_device_id ad7173_of_match[] = {
+ { .compatible = "ad4111",
+ .data = &ad7173_device_info[ID_AD4111]},
+ { .compatible = "ad4112",
+ .data = &ad7173_device_info[ID_AD4112]},
+ { .compatible = "ad4114",
+ .data = &ad7173_device_info[ID_AD4114]},
+ { .compatible = "ad4115",
+ .data = &ad7173_device_info[ID_AD4115]},
{ .compatible = "adi,ad7172-2",
.data = &ad7173_device_info[ID_AD7172_2]},
{ .compatible = "adi,ad7172-4",
@@ -1214,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)&ad7173_device_info[ID_AD4111]},
+ { "ad4112", (kernel_ulong_t)&ad7173_device_info[ID_AD4112]},
+ { "ad4114", (kernel_ulong_t)&ad7173_device_info[ID_AD4114]},
+ { "ad4115", (kernel_ulong_t)&ad7173_device_info[ID_AD4115]},
+ { "ad4116", (kernel_ulong_t)&ad7173_device_info[ID_AD4116]},
{ "ad7172-2", (kernel_ulong_t)&ad7173_device_info[ID_AD7172_2]},
{ "ad7172-4", (kernel_ulong_t)&ad7173_device_info[ID_AD7172_4]},
{ "ad7173-8", (kernel_ulong_t)&ad7173_device_info[ID_AD7173_8]},
@@ -1238,5 +1498,5 @@ module_spi_driver(ad7173_driver);
MODULE_IMPORT_NS(IIO_AD_SIGMA_DELTA);
MODULE_AUTHOR("Lars-Peter Clausen <lars@metafo.de>");
MODULE_AUTHOR("Dumitru Ceclan <dumitru.ceclan@analog.com>");
-MODULE_DESCRIPTION("Analog Devices AD7172/AD7173/AD7175/AD7176 ADC driver");
+MODULE_DESCRIPTION("Analog Devices AD717x and AD411x ADC driver");
MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 9/9] iio: adc: ad7173: Reduce device info struct size
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
` (7 preceding siblings ...)
2024-05-14 7:22 ` [PATCH v2 8/9] iio: adc: ad7173: Add support for AD411x devices Dumitru Ceclan via B4 Relay
@ 2024-05-14 7:22 ` Dumitru Ceclan via B4 Relay
2024-05-15 22:35 ` [PATCH v2 0/9] Add support for AD411x David Lechner
9 siblings, 0 replies; 28+ messages in thread
From: Dumitru Ceclan via B4 Relay @ 2024-05-14 7:22 UTC (permalink / raw)
To: Ceclan Dumitru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
From: Dumitru Ceclan <dumitru.ceclan@analog.com>
Reduce the size used by the device info struct by packing the bool
fields within the same byte. This reduces the struct size from 52 bytes
to 44 bytes.
Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
---
drivers/iio/adc/ad7173.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index f049d79380ac..f963c731cae3 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -180,15 +180,15 @@ struct ad7173_device_info {
unsigned int clock;
unsigned int id;
char *name;
- bool has_current_inputs;
- bool has_vcom_input;
- bool has_temp;
+ bool has_current_inputs :1;
+ bool has_vcom_input :1;
+ bool has_temp :1;
/* ((AVDD1 − AVSS)/5) */
- bool has_common_input;
- bool has_input_buf;
- bool has_int_ref;
- bool has_ref2;
- bool higher_gpio_bits;
+ bool has_common_input :1;
+ bool has_input_buf :1;
+ bool has_int_ref :1;
+ bool has_ref2 :1;
+ bool higher_gpio_bits :1;
u8 num_gpios;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/9] Add support for AD411x
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
` (8 preceding siblings ...)
2024-05-14 7:22 ` [PATCH v2 9/9] iio: adc: ad7173: Reduce device info struct size Dumitru Ceclan via B4 Relay
@ 2024-05-15 22:35 ` David Lechner
2024-05-16 16:15 ` Ceclan, Dumitru
9 siblings, 1 reply; 28+ messages in thread
From: David Lechner @ 2024-05-15 22:35 UTC (permalink / raw)
To: dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel, Dumitru Ceclan, Jonathan Cameron
On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
<devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>
> This patch series adds support for the Analog Devices AD4111, AD4112,
> AD4114, AD4115, AD4116 within the existing AD7173 driver.
>
It looks like most of the patches in this series are cleanups and
fixes of the existing driver unrelated to adding AD411x. Perhaps it
would be better to split those out into a separate series so we can
focus on that first? Especially since several of them need to be sent
as fixes for the v6.10 kernel to avoid breaking usespace or bindings
in the next release.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x
2024-05-14 7:22 ` [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x Dumitru Ceclan via B4 Relay
@ 2024-05-15 22:37 ` David Lechner
2024-05-16 15:49 ` Ceclan, Dumitru
0 siblings, 1 reply; 28+ messages in thread
From: David Lechner @ 2024-05-15 22:37 UTC (permalink / raw)
To: dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel, Dumitru Ceclan
On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
<devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
>
> AD411x family ADCs support a VCOM pin, dedicated for single-ended usage.
> AD4111/AD4112 support current channels, usage is implemented by
> specifying channel reg values bigger than 15.
>
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> ---
> .../devicetree/bindings/iio/adc/adi,ad7173.yaml | 118 ++++++++++++++++++++-
> 1 file changed, 117 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> index ea6cfcd0aff4..6cc3514f5ed8 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,6 +145,31 @@ patternProperties:
> maximum: 15
>
> diff-channels:
> + description: |
> + For using current channels specify select the current inputs
> + and enable the adi,current-channel property.
> +
> + Family AD411x supports a dedicated VCOM voltage input.
Did you mean VINCOM? Searching the datasheets for "VCOM" comes up empty.
> + To select it set the second channel to 16.
> + (VIN2, VCOM) -> diff-channels = <2 16>
Jonathan mentioned this in v1 with regard to the current inputs, but
it applies here too. There is a new proposed single-channel property
[1] that would be preferred when an input is used as a single-ended or
pseudo-differential input (i.e. with VINCOM or ADCIN15).
[1]: https://lore.kernel.org/linux-iio/20240514120222.56488-5-alisa.roman@analog.com/
> +
> + 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)−
> + Supported only by AD4111, AD4112:
> + 12: IIN3+
> + 11: IIN3−
> + 13: IIN2+
> + 10: IIN2−
> + 14: IIN1+
> + 9: IIN1−
> + 15: IIN0+
> + 8: IIN0−
I just made a late reply on v1 where Jonathan suggested that the
current inputs are differential with a similar comment to this:
It doesn't seem to me like current inputs are differential if they are
only measuring one current. They take 2 pins because you need a way
for current to come in and go back out, but the datasheet calls them
single-ended inputs.
> +
> items:
> minimum: 0
> maximum: 31
> @@ -154,6 +195,23 @@ 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
> +
> + adi,channel-type:
> + description:
> + Used to differentiate between different channel types as the device
> + register configurations are the same for all usage types.
> + $ref: /schemas/types.yaml#/definitions/string
> + enum:
> + - single-ended
> + - pseudo-differential
> + - differential
> + default: differential
> +
As suggested above, we should soon have diff-channels and
single-channel to differentiate between (fully) differential and
single-ended. Do we actually need to differentiate between
single-ended and pseudo-differential though?
I think the AD4116 datasheet is the only one that uses both of those
terms. It gives the examples that for "single-ended" ADCIN15 would be
connected to AVSS and for "pseudo-differential" ADCIN15 would be
connected to REFOUT (AVSS + 2.5 V). So the only difference seems to be
if the voltage on ADCIN15 is == 0V or != 0V.
To make this like other pseudo-differential chips we have done
recently, it seems to me like we should have an adcin15-supply
property to describe the ADCIN15 input. Then we could use that
property to determine single-ended vs. pseudo-differential (if there
actually is a need for that) and we wouldn't need the adi,channel-type
property.
> required:
> - reg
> - diff-channels
> @@ -166,7 +224,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 +244,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
As discussed recently in the the very similar ad719x bindings [2], we
may have been misunderstanding this limit so far. 15 is a bit
artificially low since input pins can be used more than once in
different "channels". But that is really an issues with the existing
bindings, not just this patch.
[2]: https://lore.kernel.org/linux-iio/20240511122955.2372f56e@jic23-huawei/
> +
> + - 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 +298,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]
>
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/9] iio: adc: ad7173: refactor ain and vref selection
2024-05-14 7:22 ` [PATCH v2 4/9] iio: adc: ad7173: refactor ain and vref selection Dumitru Ceclan via B4 Relay
@ 2024-05-15 22:54 ` David Lechner
2024-05-19 19:17 ` Jonathan Cameron
1 sibling, 0 replies; 28+ messages in thread
From: David Lechner @ 2024-05-15 22:54 UTC (permalink / raw)
To: dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel, Dumitru Ceclan
On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
<devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> Move validation of analog inputs and reference voltage selection to
> separate functions to reduce the size of the channel config parsing
> function and improve readability.
>
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] iio: adc: ad7173: add support for special inputs
2024-05-14 7:22 ` [PATCH v2 5/9] iio: adc: ad7173: add support for special inputs Dumitru Ceclan via B4 Relay
@ 2024-05-15 23:27 ` David Lechner
2024-05-16 16:03 ` Ceclan, Dumitru
0 siblings, 1 reply; 28+ messages in thread
From: David Lechner @ 2024-05-15 23:27 UTC (permalink / raw)
To: dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel, Dumitru Ceclan
On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
<devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> Add support for selecting REF+ and REF- inputs on all models.
> Add support for selecting ((AVDD1 − AVSS)/5) inputs
> on supported models.
>
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> ---
> 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 fb33534d63a9..1e9ba3070770 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -65,6 +65,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
> @@ -145,6 +149,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;
> @@ -215,6 +221,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),
> @@ -228,6 +235,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),
> @@ -243,6 +251,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),
> @@ -257,6 +266,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),
> @@ -271,6 +281,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),
> @@ -285,6 +296,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),
> @@ -298,6 +310,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,
> @@ -920,6 +933,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;
> +
If there is only one valid combination, it seems like these should be
fixed channels like the temperature input rather than something coming
from the device tree.
It looks like on AD411x, it is the case that there is only one valid
option for the reference input in the channel configuration. But in
the case of AD717x since both REF+ and REF- are listed as possible
inputs for both AINPOS0 and AINNEG0, it seems like they could be mixed
and matched with other channels. The datasheet doesn't seem very clear
on this though.
If it is valid to combine, say AIN0 with REF+ though, then the
validation would need to be relaxed. But I'm guessing that is not
actually the case?
> return dev_err_probe(dev, -EINVAL,
> "Input pin number out of range for pair (%d %d).\n",
> ain[0], ain[1]);
>
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] iio: adc: ad7173: Add ad7173_device_info names
2024-05-14 7:22 ` [PATCH v2 6/9] iio: adc: ad7173: Add ad7173_device_info names Dumitru Ceclan via B4 Relay
@ 2024-05-15 23:32 ` David Lechner
2024-05-16 16:09 ` Ceclan, Dumitru
0 siblings, 1 reply; 28+ messages in thread
From: David Lechner @ 2024-05-15 23:32 UTC (permalink / raw)
To: dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel, Dumitru Ceclan
On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
<devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> Add missing names from the device info struct for 3 models to ensure
> consistency with the rest of the models.
>
This affects userspace, right? So probably needs a Fixes: to make sure
this gets into the 6.10 release?
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> ---
> drivers/iio/adc/ad7173.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index 1e9ba3070770..d965b66d4d5a 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -227,6 +227,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> },
> [ID_AD7172_4] = {
> + .name = "ad7172-4",
> .id = AD7172_4_ID,
> .num_inputs = 9,
> .num_channels = 8,
> @@ -272,6 +273,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> },
> [ID_AD7175_8] = {
> + .name = "ad7175-8",
> .id = AD7175_8_ID,
> .num_inputs = 17,
> .num_channels = 16,
> @@ -302,6 +304,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> },
> [ID_AD7177_2] = {
> + .name = "ad7177-2",
> .id = AD7177_ID,
> .num_inputs = 5,
> .num_channels = 4,
>
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 7/9] iio: adc: ad7173: Remove index from temp channel
2024-05-14 7:22 ` [PATCH v2 7/9] iio: adc: ad7173: Remove index from temp channel Dumitru Ceclan via B4 Relay
@ 2024-05-15 23:34 ` David Lechner
2024-05-19 17:08 ` Jonathan Cameron
0 siblings, 1 reply; 28+ messages in thread
From: David Lechner @ 2024-05-15 23:34 UTC (permalink / raw)
To: dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel, Dumitru Ceclan
On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
<devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> Temperature channel is unique per device, index is not needed.
>
> This is breaking userspace: as main driver has not reached mainline yet
> it won't affect users as there are none.
But it is queued up, so probably need a Fixes: here to make sure it
makes it into the release.
>
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> ---
> drivers/iio/adc/ad7173.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index d965b66d4d5a..d66b47e1a186 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -828,7 +828,6 @@ static const struct iio_chan_spec ad7173_channel_template = {
>
> static const struct iio_chan_spec ad7173_temp_iio_channel_template = {
> .type = IIO_TEMP,
> - .indexed = 1,
> .channel = AD7173_AIN_TEMP_POS,
> .channel2 = AD7173_AIN_TEMP_NEG,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x
2024-05-15 22:37 ` David Lechner
@ 2024-05-16 15:49 ` Ceclan, Dumitru
2024-05-16 16:43 ` David Lechner
0 siblings, 1 reply; 28+ messages in thread
From: Ceclan, Dumitru @ 2024-05-16 15:49 UTC (permalink / raw)
To: David Lechner, dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel
On 16/05/2024 01:37, David Lechner wrote:
> On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>>
>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>>
>> Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
>>
>> AD411x family ADCs support a VCOM pin, dedicated for single-ended usage.
>> AD4111/AD4112 support current channels, usage is implemented by
>> specifying channel reg values bigger than 15.
>>
>> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
>> ---
>> .../devicetree/bindings/iio/adc/adi,ad7173.yaml | 118 ++++++++++++++++++++-
>> 1 file changed, 117 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>> index ea6cfcd0aff4..6cc3514f5ed8 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,6 +145,31 @@ patternProperties:
>> maximum: 15
>>
>> diff-channels:
>> + description: |
>> + For using current channels specify select the current inputs
>> + and enable the adi,current-channel property.
>> +
>> + Family AD411x supports a dedicated VCOM voltage input.
>
> Did you mean VINCOM? Searching the datasheets for "VCOM" comes up empty.
>
Yep
>> + To select it set the second channel to 16.
>> + (VIN2, VCOM) -> diff-channels = <2 16>
>
> Jonathan mentioned this in v1 with regard to the current inputs, but
> it applies here too. There is a new proposed single-channel property
> [1] that would be preferred when an input is used as a single-ended or
> pseudo-differential input (i.e. with VINCOM or ADCIN15).
>
> [1]: https://lore.kernel.org/linux-iio/20240514120222.56488-5-alisa.roman@analog.com/
>
Yet here I thought that it was clear from previous conversations that
we are not really dealing with a single-ended/pseudo-differential input,
just a differential ADC that can be used in that manner.
We do not have here such a clear cut case as with AD7194, where an input
is dedicated for single-ended/pseudo usage. Here, the inputs are mix and
match and single-ended/pseudo is obtainable with other pins than VINCOM/
ADCIN15.
When configuring channels we are *always* specifying two voltage inputs.
I strongly disagree using single-channel for voltage channels in these
families of ADC's.
>> +
>> + 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)−
>> + Supported only by AD4111, AD4112:
>> + 12: IIN3+
>> + 11: IIN3−
>> + 13: IIN2+
>> + 10: IIN2−
>> + 14: IIN1+
>> + 9: IIN1−
>> + 15: IIN0+
>> + 8: IIN0−
>
> I just made a late reply on v1 where Jonathan suggested that the
> current inputs are differential with a similar comment to this:
>
> It doesn't seem to me like current inputs are differential if they are
> only measuring one current. They take 2 pins because you need a way
> for current to come in and go back out, but the datasheet calls them
> single-ended inputs.
>
It seemed to me that the conclusion that we arrived to was to expose the
precise pins that are used in the conversion and document the selection.
Yes, it is a single-ended channel. So revert to the way it was in V1 using
single-channel?
>> +
>> items:
>> minimum: 0
>> maximum: 31
>> @@ -154,6 +195,23 @@ 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
>> +
>> + adi,channel-type:
>> + description:
>> + Used to differentiate between different channel types as the device
>> + register configurations are the same for all usage types.
>> + $ref: /schemas/types.yaml#/definitions/string
>> + enum:
>> + - single-ended
>> + - pseudo-differential
>> + - differential
>> + default: differential
>> +
>
> As suggested above, we should soon have diff-channels and
> single-channel to differentiate between (fully) differential and
> single-ended. Do we actually need to differentiate between
> single-ended and pseudo-differential though?
>
Not really, so just a bool differential flag? (this seems weird as we have diff-channels)
> I think the AD4116 datasheet is the only one that uses both of those
> terms. It gives the examples that for "single-ended" ADCIN15 would be
> connected to AVSS and for "pseudo-differential" ADCIN15 would be
> connected to REFOUT (AVSS + 2.5 V). So the only difference seems to be
> if the voltage on ADCIN15 is == 0V or != 0V.
>
In the ad411x yes, over to ad717x it's mixed:
https://lore.kernel.org/all/0fef36f8-a7db-40cc-86bd-9449cb4ab46e@gmail.com/
> To make this like other pseudo-differential chips we have done
> recently, it seems to me like we should have an adcin15-supply
> property to describe the ADCIN15 input. Then we could use that
> property to determine single-ended vs. pseudo-differential (if there
> actually is a need for that) and we wouldn't need the adi,channel-type
> property.
>
I agree that we do not need to differentiate between those two.
But the approach with the supply is too specific, the adi,channel-type
property is not only for AD4116-ADCIN15, but for all models compatible.
>> required:
>> - reg
>> - diff-channels
>> @@ -166,7 +224,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 +244,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
>
> As discussed recently in the the very similar ad719x bindings [2], we
> may have been misunderstanding this limit so far. 15 is a bit
> artificially low since input pins can be used more than once in
> different "channels". But that is really an issues with the existing
> bindings, not just this patch.
>
> [2]: https://lore.kernel.org/linux-iio/20240511122955.2372f56e@jic23-huawei/
>
>
In this case there is a 1-1 correspondence between this reg limit and the number
of channel configuration registers available to the device. Maybe another property
then reg? Sure...but this limitation fits the current situation.
In addition, the device does not work the same as ad719x. If I understood correctly
that documentation, the configuration register needs to be rewritten for every different
input combination. This means that the driver is implemented to overwrite the reg for
every read. This device, it seems to me, is more in the liking's of write all the channel
configs at once, then keep using those.
For AD719x yes, it is artificial. Over here we have a clear reason.
>> +
>> + - 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 +298,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]
>>
>> --
>> 2.43.0
>>
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] iio: adc: ad7173: add support for special inputs
2024-05-15 23:27 ` David Lechner
@ 2024-05-16 16:03 ` Ceclan, Dumitru
0 siblings, 0 replies; 28+ messages in thread
From: Ceclan, Dumitru @ 2024-05-16 16:03 UTC (permalink / raw)
To: David Lechner, dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel
On 16/05/2024 02:27, David Lechner wrote:
> On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>>
>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>>
>> Add support for selecting REF+ and REF- inputs on all models.
>> Add support for selecting ((AVDD1 − AVSS)/5) inputs
>> on supported models.
>>
>> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
>> ---
>> 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 fb33534d63a9..1e9ba3070770 100644
>> --- a/drivers/iio/adc/ad7173.c
>> +++ b/drivers/iio/adc/ad7173.c
>> @@ -65,6 +65,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
>> @@ -145,6 +149,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;
>> @@ -215,6 +221,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),
>> @@ -228,6 +235,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),
>> @@ -243,6 +251,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),
>> @@ -257,6 +266,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),
>> @@ -271,6 +281,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),
>> @@ -285,6 +296,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),
>> @@ -298,6 +310,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,
>> @@ -920,6 +933,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;
>> +
>
> If there is only one valid combination, it seems like these should be
> fixed channels like the temperature input rather than something coming
> from the device tree.
>
As I've said, I do not agree with forcing one channel slot to be used.
I could add a property that spawns this channel. Although as I see under,
I think I'll permit these inputs to be mixed and matched.
> It looks like on AD411x, it is the case that there is only one valid
> option for the reference input in the channel configuration. But in
> the case of AD717x since both REF+ and REF- are listed as possible
> inputs for both AINPOS0 and AINNEG0, it seems like they could be mixed
> and matched with other channels. The datasheet doesn't seem very clear
> on this though.
>
This is imposed artificially, AD411x has the same cross-point mux that
can mix and match all the inputs.
> If it is valid to combine, say AIN0 with REF+ though, then the
> validation would need to be relaxed. But I'm guessing that is not
> actually the case?
>
I think it is the case.
>> return dev_err_probe(dev, -EINVAL,
>> "Input pin number out of range for pair (%d %d).\n",
>> ain[0], ain[1]);
>>
>> --
>> 2.43.0
>>
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] iio: adc: ad7173: Add ad7173_device_info names
2024-05-15 23:32 ` David Lechner
@ 2024-05-16 16:09 ` Ceclan, Dumitru
2024-05-16 16:43 ` David Lechner
0 siblings, 1 reply; 28+ messages in thread
From: Ceclan, Dumitru @ 2024-05-16 16:09 UTC (permalink / raw)
To: David Lechner, dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel
On 16/05/2024 02:32, David Lechner wrote:
> On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>>
>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>>
>> Add missing names from the device info struct for 3 models to ensure
>> consistency with the rest of the models.
>>
>
> This affects userspace, right? So probably needs a Fixes: to make sure
> this gets into the 6.10 release?
>
I don't think that it breaks userspace, just creates an additional file.
This creates the file "name" in the iio:deviceX directory that reads the
string. I do not consider the Fixes: tag a necessity. I consider that it
resolves inconsistencies in the _device_info struct.
>> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
>> ---
>> drivers/iio/adc/ad7173.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
>> index 1e9ba3070770..d965b66d4d5a 100644
>> --- a/drivers/iio/adc/ad7173.c
>> +++ b/drivers/iio/adc/ad7173.c
>> @@ -227,6 +227,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
>> .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
>> },
>> [ID_AD7172_4] = {
>> + .name = "ad7172-4",
>> .id = AD7172_4_ID,
>> .num_inputs = 9,
>> .num_channels = 8,
>> @@ -272,6 +273,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
>> .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
>> },
>> [ID_AD7175_8] = {
>> + .name = "ad7175-8",
>> .id = AD7175_8_ID,
>> .num_inputs = 17,
>> .num_channels = 16,
>> @@ -302,6 +304,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
>> .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
>> },
>> [ID_AD7177_2] = {
>> + .name = "ad7177-2",
>> .id = AD7177_ID,
>> .num_inputs = 5,
>> .num_channels = 4,
>>
>> --
>> 2.43.0
>>
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/9] Add support for AD411x
2024-05-15 22:35 ` [PATCH v2 0/9] Add support for AD411x David Lechner
@ 2024-05-16 16:15 ` Ceclan, Dumitru
0 siblings, 0 replies; 28+ messages in thread
From: Ceclan, Dumitru @ 2024-05-16 16:15 UTC (permalink / raw)
To: David Lechner, dumitru.ceclan
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel, Jonathan Cameron
On 16/05/2024 01:35, David Lechner wrote:
> On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>>
>> This patch series adds support for the Analog Devices AD4111, AD4112,
>> AD4114, AD4115, AD4116 within the existing AD7173 driver.
>>
>
> It looks like most of the patches in this series are cleanups and
> fixes of the existing driver unrelated to adding AD411x. Perhaps it
> would be better to split those out into a separate series so we can
> focus on that first? Especially since several of them need to be sent
> as fixes for the v6.10 kernel to avoid breaking usespace or bindings
> in the next release.
Sure
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x
2024-05-16 15:49 ` Ceclan, Dumitru
@ 2024-05-16 16:43 ` David Lechner
2024-05-19 16:54 ` Jonathan Cameron
0 siblings, 1 reply; 28+ messages in thread
From: David Lechner @ 2024-05-16 16:43 UTC (permalink / raw)
To: Ceclan, Dumitru
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
On Thu, May 16, 2024 at 10:49 AM Ceclan, Dumitru
<mitrutzceclan@gmail.com> wrote:
>
> On 16/05/2024 01:37, David Lechner wrote:
> > On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
> > <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> >>
> >> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> >>
> >> Add support for: AD4111, AD4112, AD4114, AD4115, AD4116.
> >>
> >> AD411x family ADCs support a VCOM pin, dedicated for single-ended usage.
> >> AD4111/AD4112 support current channels, usage is implemented by
> >> specifying channel reg values bigger than 15.
> >>
> >> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> >> ---
> >> .../devicetree/bindings/iio/adc/adi,ad7173.yaml | 118 ++++++++++++++++++++-
> >> 1 file changed, 117 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> >> index ea6cfcd0aff4..6cc3514f5ed8 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,6 +145,31 @@ patternProperties:
> >> maximum: 15
> >>
> >> diff-channels:
> >> + description: |
> >> + For using current channels specify select the current inputs
> >> + and enable the adi,current-channel property.
> >> +
> >> + Family AD411x supports a dedicated VCOM voltage input.
> >
> > Did you mean VINCOM? Searching the datasheets for "VCOM" comes up empty.
> >
> Yep
> >> + To select it set the second channel to 16.
> >> + (VIN2, VCOM) -> diff-channels = <2 16>
> >
> > Jonathan mentioned this in v1 with regard to the current inputs, but
> > it applies here too. There is a new proposed single-channel property
> > [1] that would be preferred when an input is used as a single-ended or
> > pseudo-differential input (i.e. with VINCOM or ADCIN15).
> >
> > [1]: https://lore.kernel.org/linux-iio/20240514120222.56488-5-alisa.roman@analog.com/
> >
> Yet here I thought that it was clear from previous conversations that
> we are not really dealing with a single-ended/pseudo-differential input,
> just a differential ADC that can be used in that manner.
>
> We do not have here such a clear cut case as with AD7194, where an input
> is dedicated for single-ended/pseudo usage. Here, the inputs are mix and
> match and single-ended/pseudo is obtainable with other pins than VINCOM/
> ADCIN15.
>
> When configuring channels we are *always* specifying two voltage inputs.
> I strongly disagree using single-channel for voltage channels in these
> families of ADC's.
Yes, sorry, you are right. I forgot that VINCOM isn't actually
electrically different from the other pins (even if the name makes it
seem like it would be).
>
> >> +
> >> + 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)−
> >> + Supported only by AD4111, AD4112:
> >> + 12: IIN3+
> >> + 11: IIN3−
> >> + 13: IIN2+
> >> + 10: IIN2−
> >> + 14: IIN1+
> >> + 9: IIN1−
> >> + 15: IIN0+
> >> + 8: IIN0−
> >
> > I just made a late reply on v1 where Jonathan suggested that the
> > current inputs are differential with a similar comment to this:
> >
> > It doesn't seem to me like current inputs are differential if they are
> > only measuring one current. They take 2 pins because you need a way
> > for current to come in and go back out, but the datasheet calls them
> > single-ended inputs.
> >
> It seemed to me that the conclusion that we arrived to was to expose the
> precise pins that are used in the conversion and document the selection.
>
> Yes, it is a single-ended channel. So revert to the way it was in V1 using
> single-channel?
I'd like to hear Jonathan's opinion on this one.
>
> >> +
> >> items:
> >> minimum: 0
> >> maximum: 31
> >> @@ -154,6 +195,23 @@ 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
> >> +
> >> + adi,channel-type:
> >> + description:
> >> + Used to differentiate between different channel types as the device
> >> + register configurations are the same for all usage types.
> >> + $ref: /schemas/types.yaml#/definitions/string
> >> + enum:
> >> + - single-ended
> >> + - pseudo-differential
> >> + - differential
> >> + default: differential
> >> +
> >
> > As suggested above, we should soon have diff-channels and
> > single-channel to differentiate between (fully) differential and
> > single-ended. Do we actually need to differentiate between
> > single-ended and pseudo-differential though?
> >
> Not really, so just a bool differential flag? (this seems weird as we have diff-channels)
Or we need to change the proposed single-channel property to allow two
inputs. I guess we'll see what Johnathan has to say.
>
> > I think the AD4116 datasheet is the only one that uses both of those
> > terms. It gives the examples that for "single-ended" ADCIN15 would be
> > connected to AVSS and for "pseudo-differential" ADCIN15 would be
> > connected to REFOUT (AVSS + 2.5 V). So the only difference seems to be
> > if the voltage on ADCIN15 is == 0V or != 0V.
> >
> In the ad411x yes, over to ad717x it's mixed:
> https://lore.kernel.org/all/0fef36f8-a7db-40cc-86bd-9449cb4ab46e@gmail.com/
>
> > To make this like other pseudo-differential chips we have done
> > recently, it seems to me like we should have an adcin15-supply
> > property to describe the ADCIN15 input. Then we could use that
> > property to determine single-ended vs. pseudo-differential (if there
> > actually is a need for that) and we wouldn't need the adi,channel-type
> > property.
> >
>
> I agree that we do not need to differentiate between those two.
> But the approach with the supply is too specific, the adi,channel-type
> property is not only for AD4116-ADCIN15, but for all models compatible.
>
Makes sense, especially given the point above that ADCIN15 isn't
really electrically different from other inputs.
> >> required:
> >> - reg
> >> - diff-channels
> >> @@ -166,7 +224,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 +244,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
> >
> > As discussed recently in the the very similar ad719x bindings [2], we
> > may have been misunderstanding this limit so far. 15 is a bit
> > artificially low since input pins can be used more than once in
> > different "channels". But that is really an issues with the existing
> > bindings, not just this patch.
> >
> > [2]: https://lore.kernel.org/linux-iio/20240511122955.2372f56e@jic23-huawei/
> >
> >
> In this case there is a 1-1 correspondence between this reg limit and the number
> of channel configuration registers available to the device. Maybe another property
> then reg? Sure...but this limitation fits the current situation.
>
> In addition, the device does not work the same as ad719x. If I understood correctly
> that documentation, the configuration register needs to be rewritten for every different
> input combination. This means that the driver is implemented to overwrite the reg for
> every read. This device, it seems to me, is more in the liking's of write all the channel
> configs at once, then keep using those.
>
> For AD719x yes, it is artificial. Over here we have a clear reason.
I thought they worked nearly the same in this regard since they are
sharing a lot of code via adc/ad_sigma_delta.h, It looks to me like
the channel registers are only set up for a raw read (single channel)
or buffered read (only enabled channels), but maybe I didn't look deep
enough. Anyway, not a big deal to me.
>
> >> +
> >> + - 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 +298,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]
> >>
> >> --
> >> 2.43.0
> >>
> >>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] iio: adc: ad7173: Add ad7173_device_info names
2024-05-16 16:09 ` Ceclan, Dumitru
@ 2024-05-16 16:43 ` David Lechner
2024-05-19 17:09 ` Jonathan Cameron
0 siblings, 1 reply; 28+ messages in thread
From: David Lechner @ 2024-05-16 16:43 UTC (permalink / raw)
To: Ceclan, Dumitru
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
On Thu, May 16, 2024 at 11:09 AM Ceclan, Dumitru
<mitrutzceclan@gmail.com> wrote:
>
> On 16/05/2024 02:32, David Lechner wrote:
> > On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
> > <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> >>
> >> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> >>
> >> Add missing names from the device info struct for 3 models to ensure
> >> consistency with the rest of the models.
> >>
> >
> > This affects userspace, right? So probably needs a Fixes: to make sure
> > this gets into the 6.10 release?
> >
> I don't think that it breaks userspace, just creates an additional file.
>
> This creates the file "name" in the iio:deviceX directory that reads the
> string. I do not consider the Fixes: tag a necessity. I consider that it
> resolves inconsistencies in the _device_info struct.
Ah, OK. For some reason, I was thinking that it would default to the
driver name if this was left out.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x
2024-05-16 16:43 ` David Lechner
@ 2024-05-19 16:54 ` Jonathan Cameron
0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2024-05-19 16:54 UTC (permalink / raw)
To: David Lechner
Cc: Ceclan, Dumitru, dumitru.ceclan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-iio, devicetree, linux-kernel
> >
> > >> +
> > >> + 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)−
> > >> + Supported only by AD4111, AD4112:
> > >> + 12: IIN3+
> > >> + 11: IIN3−
> > >> + 13: IIN2+
> > >> + 10: IIN2−
> > >> + 14: IIN1+
> > >> + 9: IIN1−
> > >> + 15: IIN0+
> > >> + 8: IIN0−
> > >
> > > I just made a late reply on v1 where Jonathan suggested that the
> > > current inputs are differential with a similar comment to this:
> > >
> > > It doesn't seem to me like current inputs are differential if they are
> > > only measuring one current. They take 2 pins because you need a way
> > > for current to come in and go back out, but the datasheet calls them
> > > single-ended inputs.
> > >
> > It seemed to me that the conclusion that we arrived to was to expose the
> > precise pins that are used in the conversion and document the selection.
> >
> > Yes, it is a single-ended channel. So revert to the way it was in V1 using
> > single-channel?
>
> I'd like to hear Jonathan's opinion on this one.
Yes. I think we rather went off on a false tangent on this. Any current input
using a shunt is of this form and so far we've treated them as single ended :(
e.g. pac1934 does this for it's sense inputs where there is an external
'sense resitor'. Similar for the stand alone afe driver for a current sense shunt
which is used with a differential voltage input, but presents a single ended
current measurement.
Sorry for misguiding things :(
At the end of this, I don't suppose anyone fancies writing up some notes
on how to describe different types of channel?
>
> >
> > >> +
> > >> items:
> > >> minimum: 0
> > >> maximum: 31
> > >> @@ -154,6 +195,23 @@ 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
I'm lost. Why do we need this one? Is the channel selection not sufficient
to tell us this?
> > >> +
> > >> + adi,channel-type:
> > >> + description:
> > >> + Used to differentiate between different channel types as the device
> > >> + register configurations are the same for all usage types.
> > >> + $ref: /schemas/types.yaml#/definitions/string
> > >> + enum:
> > >> + - single-ended
> > >> + - pseudo-differential
> > >> + - differential
> > >> + default: differential
> > >> +
> > >
> > > As suggested above, we should soon have diff-channels and
> > > single-channel to differentiate between (fully) differential and
> > > single-ended. Do we actually need to differentiate between
> > > single-ended and pseudo-differential though?
> > >
> > Not really, so just a bool differential flag? (this seems weird as we have diff-channels)
>
> Or we need to change the proposed single-channel property to allow two
> inputs. I guess we'll see what Johnathan has to say.
I think single ended fits better for the current channels with just one
parameter.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/9] iio: adc: ad7173: refactor channel configuration parsing
2024-05-14 7:22 ` [PATCH v2 3/9] iio: adc: ad7173: refactor channel configuration parsing Dumitru Ceclan via B4 Relay
@ 2024-05-19 16:58 ` Jonathan Cameron
0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2024-05-19 16:58 UTC (permalink / raw)
To: Dumitru Ceclan via B4 Relay
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan,
Jonathan Cameron
On Tue, 14 May 2024 10:22:48 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> Move configurations regarding number of channels from
> *_fw_parse_device_config to *_fw_parse_channel_config.
>
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Link: https://lore.kernel.org/all/20240303162148.3ad91aa2@jic23-huawei/
>
These are all part of the tag block so no blank line here.
Having one will break some scripts that parse these blocks and
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 7/9] iio: adc: ad7173: Remove index from temp channel
2024-05-15 23:34 ` David Lechner
@ 2024-05-19 17:08 ` Jonathan Cameron
0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2024-05-19 17:08 UTC (permalink / raw)
To: David Lechner
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel, Dumitru Ceclan
On Wed, 15 May 2024 18:34:17 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> >
> > From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> >
> > Temperature channel is unique per device, index is not needed.
> >
> > This is breaking userspace: as main driver has not reached mainline yet
> > it won't affect users as there are none.
>
> But it is queued up, so probably need a Fixes: here to make sure it
> makes it into the release.
Yes. We can fix this but it will need to go as a fix during the rc cycles
rather than wait for the next merge window like the rest of this series.
Jonathan
>
> >
> > Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> > ---
> > drivers/iio/adc/ad7173.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> > index d965b66d4d5a..d66b47e1a186 100644
> > --- a/drivers/iio/adc/ad7173.c
> > +++ b/drivers/iio/adc/ad7173.c
> > @@ -828,7 +828,6 @@ static const struct iio_chan_spec ad7173_channel_template = {
> >
> > static const struct iio_chan_spec ad7173_temp_iio_channel_template = {
> > .type = IIO_TEMP,
> > - .indexed = 1,
> > .channel = AD7173_AIN_TEMP_POS,
> > .channel2 = AD7173_AIN_TEMP_NEG,
> > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> >
> > --
> > 2.43.0
> >
> >
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] iio: adc: ad7173: Add ad7173_device_info names
2024-05-16 16:43 ` David Lechner
@ 2024-05-19 17:09 ` Jonathan Cameron
0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2024-05-19 17:09 UTC (permalink / raw)
To: David Lechner
Cc: Ceclan, Dumitru, dumitru.ceclan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-iio, devicetree, linux-kernel
On Thu, 16 May 2024 11:43:17 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On Thu, May 16, 2024 at 11:09 AM Ceclan, Dumitru
> <mitrutzceclan@gmail.com> wrote:
> >
> > On 16/05/2024 02:32, David Lechner wrote:
> > > On Tue, May 14, 2024 at 2:23 AM Dumitru Ceclan via B4 Relay
> > > <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> > >>
> > >> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> > >>
> > >> Add missing names from the device info struct for 3 models to ensure
> > >> consistency with the rest of the models.
> > >>
> > >
> > > This affects userspace, right? So probably needs a Fixes: to make sure
> > > this gets into the 6.10 release?
> > >
> > I don't think that it breaks userspace, just creates an additional file.
> >
> > This creates the file "name" in the iio:deviceX directory that reads the
> > string. I do not consider the Fixes: tag a necessity. I consider that it
> > resolves inconsistencies in the _device_info struct.
>
> Ah, OK. For some reason, I was thinking that it would default to the
> driver name if this was left out.
Hmm. I'd be tempted to take this a fix as some userspace code relies
on that name being present and it is unusual to not see it.
Jonathan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 8/9] iio: adc: ad7173: Add support for AD411x devices
2024-05-14 7:22 ` [PATCH v2 8/9] iio: adc: ad7173: Add support for AD411x devices Dumitru Ceclan via B4 Relay
@ 2024-05-19 17:13 ` Jonathan Cameron
0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2024-05-19 17:13 UTC (permalink / raw)
To: Dumitru Ceclan via B4 Relay
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
On Tue, 14 May 2024 10:22:53 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> 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 <dumitru.ceclan@analog.com>
A couple of trivial things inline. Otherwise lgtm
> @@ -1058,17 +1296,26 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> 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;
> + else
> + chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> +
> + ret = fwnode_property_match_property_string(child,
> + "adi,channel-type",
> + ad7173_channel_types,
> + ARRAY_SIZE(ad7173_channel_types));
> + chan->differential = (ret < 0 || ret == AD7173_CHAN_DIFFERENTIAL)
> + ? true : false;
That's not a boolean. So better to set 1 or 0.
> +MODULE_DESCRIPTION("Analog Devices AD717x and AD411x ADC driver");
I'm scared of wildcards even in descriptive fields as they often end up covering
something unintended. I'd just go with 'and similar'.
> MODULE_LICENSE("GPL");
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/9] iio: adc: ad7173: refactor ain and vref selection
2024-05-14 7:22 ` [PATCH v2 4/9] iio: adc: ad7173: refactor ain and vref selection Dumitru Ceclan via B4 Relay
2024-05-15 22:54 ` David Lechner
@ 2024-05-19 19:17 ` Jonathan Cameron
1 sibling, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2024-05-19 19:17 UTC (permalink / raw)
To: Dumitru Ceclan via B4 Relay
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
On Tue, 14 May 2024 10:22:49 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> Move validation of analog inputs and reference voltage selection to
Odd space before M
> separate functions to reduce the size of the channel config parsing
> function and improve readability.
>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2024-05-19 19:17 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-14 7:22 [PATCH v2 0/9] Add support for AD411x Dumitru Ceclan via B4 Relay
2024-05-14 7:22 ` [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x Dumitru Ceclan via B4 Relay
2024-05-15 22:37 ` David Lechner
2024-05-16 15:49 ` Ceclan, Dumitru
2024-05-16 16:43 ` David Lechner
2024-05-19 16:54 ` Jonathan Cameron
2024-05-14 7:22 ` [PATCH v2 2/9] iio: adc: ad7173: fix buffers enablement for ad7176-2 Dumitru Ceclan via B4 Relay
2024-05-14 7:22 ` [PATCH v2 3/9] iio: adc: ad7173: refactor channel configuration parsing Dumitru Ceclan via B4 Relay
2024-05-19 16:58 ` Jonathan Cameron
2024-05-14 7:22 ` [PATCH v2 4/9] iio: adc: ad7173: refactor ain and vref selection Dumitru Ceclan via B4 Relay
2024-05-15 22:54 ` David Lechner
2024-05-19 19:17 ` Jonathan Cameron
2024-05-14 7:22 ` [PATCH v2 5/9] iio: adc: ad7173: add support for special inputs Dumitru Ceclan via B4 Relay
2024-05-15 23:27 ` David Lechner
2024-05-16 16:03 ` Ceclan, Dumitru
2024-05-14 7:22 ` [PATCH v2 6/9] iio: adc: ad7173: Add ad7173_device_info names Dumitru Ceclan via B4 Relay
2024-05-15 23:32 ` David Lechner
2024-05-16 16:09 ` Ceclan, Dumitru
2024-05-16 16:43 ` David Lechner
2024-05-19 17:09 ` Jonathan Cameron
2024-05-14 7:22 ` [PATCH v2 7/9] iio: adc: ad7173: Remove index from temp channel Dumitru Ceclan via B4 Relay
2024-05-15 23:34 ` David Lechner
2024-05-19 17:08 ` Jonathan Cameron
2024-05-14 7:22 ` [PATCH v2 8/9] iio: adc: ad7173: Add support for AD411x devices Dumitru Ceclan via B4 Relay
2024-05-19 17:13 ` Jonathan Cameron
2024-05-14 7:22 ` [PATCH v2 9/9] iio: adc: ad7173: Reduce device info struct size Dumitru Ceclan via B4 Relay
2024-05-15 22:35 ` [PATCH v2 0/9] Add support for AD411x David Lechner
2024-05-16 16:15 ` Ceclan, Dumitru
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).