Add support for ADAQ776x-1 ADC Family
Subsystems (4)
cover Add support for ADAQ776x-1 ADC Family
This adds support for the ADAQ7767-1, ADAQ7768-1 and ADAQ7769-1 devices.
The ADAQ7768-1 and ADAQ7769-1 integrate a programmable gain amplifier (PGA) with 7 and 8 gain options, respectively. The ADAQ7767-1 and ADAQ7769-1 also feature a 3-pin selectable Anti-aliasing filter (AAF) gain.
Jonathan Santos (4):
dt-bindings: iio: adc: ad7768-1: add new supported parts
iio: adc: ad7768-1: introduce chip info for future multidevice support
iio: adc: ad7768-1: use devm_regulator_get_enable_read_voltage
iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
.../bindings/iio/adc/adi,ad7768-1.yaml | 48 ++- drivers/iio/adc/ad7768-1.c | 389 +++++++++++++++--- 2 files changed, 382 insertions(+), 55 deletions(-)
base-commit: 0a686b9c4f847dc21346df8e56d5b119918fefef
Signature · 1 line
2.34.1
PATCH 1/4 dt-bindings: iio: adc: ad7768-1: add new supported parts
Add compatibles for supported parts in the ad7768-1 family:
ADAQ7767-1, ADAQ7768-1 and ADAQ7769-1
Add property and checks for AFF gain, supported by ADAQ7767-1 and ADAQ7769-1 parts:
adi,aaf-gain
--- .../bindings/iio/adc/adi,ad7768-1.yaml | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +44 -4
@@ -4,18 +4,26 @@ $id: http://devicetree.org/schemas/iio/adc/adi,ad7768-1.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Analog Devices AD7768-1 ADC device driver +title: Analog Devices AD7768-1 ADC family device driver maintainers: - Michael Hennerich <[email protected]> description: | - Datasheet at: - https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf + Analog Devices AD7768-1 24-Bit Single Channel Low Power sigma-delta ADC family + + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7767-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7768-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7769-1.pdf properties: compatible: - const: adi,ad7768-1 + enum: + - adi,ad7768-1 + - adi,adaq7767-1 + - adi,adaq7768-1 + - adi,adaq7769-1 reg: maxItems: 1
@@ -58,6 +66,23 @@ properties: description: ADC reference voltage supply + adi,aaf-gain: + description: | + Specifies the gain of the Analog Anti-Aliasing Filter (AAF) applied to the + ADC input, measured in milli-units. The AAF provides additional signal + rejection within the frequency range of fs ± f3dB, where fs is the sampling + frequency, and f3dB is the -3dB cutoff frequency. The specific values of + fs and f3dB, as well as the rejection intensity, depend on the digital + filter configuration. + + This parameter is required for the ADAQ7767-1 and ADAQ7769-1 devices. + The gain is determined by the selected input pin: + * For the ADAQ7767-1: The input selection of IN1±, IN2± or IN3±. + * For the ADAQ7769-1: The connections of OUT_PGA to IN1_AAF+, IN2_AAF+, + or IN3_AAF+. + $ref: /schemas/types.yaml#/definitions/uint16 + enum: [143, 364, 1000] + adi,sync-in-gpios: maxItems: 1 description:
@@ -147,6 +172,21 @@ patternProperties: allOf: - $ref: /schemas/spi/spi-peripheral-props.yaml# + # AAF Gain property only applies to ADAQ7767-1 and ADAQ7769-1 devices + - if: + properties: + compatible: + contains: + enum: + - adi,adaq7767-1 + - adi,adaq7769-1 + then: + required: + - adi,aaf-gain + else: + properties: + adi,aaf-gain: false + unevaluatedProperties: false examples:
Signature · 1 line
2.34.1
Re: [PATCH 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts
Quoted 17 lines · Jonathan Santos
Add compatibles for supported parts in the ad7768-1 family:
ADAQ7767-1, ADAQ7768-1 and ADAQ7769-1Add property and checks for AFF gain, supported by ADAQ7767-1 and ADAQ7769-1 parts:
adi,aaf-gainSigned-off-by Jonathan Santos <[email protected]>--- .../bindings/iio/adc/adi,ad7768-1.yaml | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-)Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +0
-0
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml line 7
Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +1
-1@@ -4,18 +4,26 @@ $id: http://devicetree.org/schemas/iio/adc/adi,ad7768-1.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Analog Devices AD7768-1 ADC device driver +title: Analog Devices AD7768-1 ADC family device driver
If doing this, drop device driver. It should not be here in the first place.
Quoted 26 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +11
-3maintainers: - Michael Hennerich <[email protected]> description: | - Datasheet at: - https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf + Analog Devices AD7768-1 24-Bit Single Channel Low Power sigma-delta ADC family + + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7767-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7768-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7769-1.pdf properties: compatible: - const: adi,ad7768-1 + enum: + - adi,ad7768-1 + - adi,adaq7767-1 + - adi,adaq7768-1 + - adi,adaq7769-1 reg: maxItems: 1
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 72
Quoted 7 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +4
-0@@ -58,6 +66,23 @@ properties: description: ADC reference voltage supply + adi,aaf-gain: + description: | + Specifies the gain of the Analog Anti-Aliasing Filter (AAF) applied to the + ADC input, measured in milli-units. The AAF provides additional signal
What is milli unit? Isn't gain in dB, so maybe you want mB? Quite unpopular to see mB, but we cannot use 1/100 of dB, so I could understand it.
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 76
Quoted 4 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +4
-0+ rejection within the frequency range of fs ± f3dB, where fs is the sampling + frequency, and f3dB is the -3dB cutoff frequency. The specific values of + fs and f3dB, as well as the rejection intensity, depend on the digital + filter configuration.
Best regards,
Krzysztof
Re: [PATCH 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts
Quoted 21 lines · Krzysztof Kozlowski
Quoted 20 lines · Jonathan Santos
Add compatibles for supported parts in the ad7768-1 family:
ADAQ7767-1, ADAQ7768-1 and ADAQ7769-1Add property and checks for AFF gain, supported by ADAQ7767-1 and ADAQ7769-1 parts:
adi,aaf-gainSigned-off-by Jonathan Santos <[email protected]>--- .../bindings/iio/adc/adi,ad7768-1.yaml | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-)Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +0
-0@@ -4,18 +4,26 @@ $id: http://devicetree.org/schemas/iio/adc/adi,ad7768-1.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml#
Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml line 7
Quoted 2 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +1
-1-title: Analog Devices AD7768-1 ADC device driver +title: Analog Devices AD7768-1 ADC family device driverIf doing this, drop device driver. It should not be here in the first place.
Noted.
Quoted 29 lines
Quoted 29 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +11
-3maintainers: - Michael Hennerich <[email protected]> description: | - Datasheet at: - https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf + Analog Devices AD7768-1 24-Bit Single Channel Low Power sigma-delta ADC family + + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7767-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7768-1.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq7769-1.pdf properties: compatible: - const: adi,ad7768-1 + enum: + - adi,ad7768-1 + - adi,adaq7767-1 + - adi,adaq7768-1 + - adi,adaq7769-1 reg: maxItems: 1@@ -58,6 +66,23 @@ properties: description: ADC reference voltage supply
Quoted 9 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 72
Quoted 4 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +4
-0+ adi,aaf-gain: + description: | + Specifies the gain of the Analog Anti-Aliasing Filter (AAF) applied to the + ADC input, measured in milli-units. The AAF provides additional signalWhat is milli unit? Isn't gain in dB, so maybe you want mB? Quite unpopular to see mB, but we cannot use 1/100 of dB, so I could understand it.
Actually, the gain is expressed in V/V, not in dB. I may have phrased it poorly, but since there are fractional values like 0.364 and 0.143, I chose to represent it in milli-units.
Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml properties: · line 76
Quoted 4 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +4
-0+ rejection within the frequency range of fs ± f3dB, where fs is the sampling + frequency, and f3dB is the -3dB cutoff frequency. The specific values of + fs and f3dB, as well as the rejection intensity, depend on the digital + filter configuration.Best regards,
Krzysztof
Re: [PATCH 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts
Quoted 6 lines · Jonathan Santos
Quoted 6 lines
Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +4
-0+ adi,aaf-gain: + description: | + Specifies the gain of the Analog Anti-Aliasing Filter (AAF) applied to the + ADC input, measured in milli-units. The AAF provides additional signal
Quoted 8 lines
Quoted 4 lines
What is milli unit? Isn't gain in dB, so maybe you want mB? Quite unpopular to see mB, but we cannot use 1/100 of dB, so I could understand it.
Actually, the gain is expressed in V/V, not in dB. I may have phrased it poorly, but since there are fractional values like 0.364 and 0.143, I chose to represent it in milli-units.
Why your reply to is corrupted:
"[email protected]"?
What sort of unit is milli-unit? Isn't this 1/1000 of some BASE unit, but you do not have here a base?
I think you want just basis point if this is V/V (already in common property suffixes)
Best regards,
Krzysztof
Jonathan Cameron
maintainerRe: [PATCH 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts
Quoted 20 lines · On Thu, 14 Aug 2025 08:03:23 +0200 Krzysztof Kozlowski <[email protected]> wrote:
Quoted 6 lines · Jonathan Santos
Quoted 6 lines
Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +4
-0+ adi,aaf-gain: + description: | + Specifies the gain of the Analog Anti-Aliasing Filter (AAF) applied to the + ADC input, measured in milli-units. The AAF provides additional signalQuoted 8 lines
Quoted 4 lines
What is milli unit? Isn't gain in dB, so maybe you want mB? Quite unpopular to see mB, but we cannot use 1/100 of dB, so I could understand it.
Actually, the gain is expressed in V/V, not in dB. I may have phrased it poorly, but since there are fractional values like 0.364 and 0.143, I chose to represent it in milli-units.
Why your reply to is corrupted:
"[email protected]"?
Quoted 5 lines
What sort of unit is milli-unit? Isn't this 1/1000 of some BASE unit, but you do not have here a base?
I think you want just basis point if this is V/V (already in common property suffixes)
Nice. I didn't know about -bp. That does sound like a good choice for ratio
stuff and here would be 100x larger actual values which is fine.
Quoted 4 lines
Best regards,
Krzysztof
Re: [PATCH 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts
Quoted 23 lines · Jonathan Cameron
Quoted 20 lines · On Thu, 14 Aug 2025 08:03:23 +0200 Krzysztof Kozlowski <[email protected]> wrote:
Quoted 6 lines · Jonathan Santos
Quoted 6 lines
Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +4
-0+ adi,aaf-gain: + description: | + Specifies the gain of the Analog Anti-Aliasing Filter (AAF) applied to the + ADC input, measured in milli-units. The AAF provides additional signalQuoted 8 lines
Quoted 4 lines
What is milli unit? Isn't gain in dB, so maybe you want mB? Quite unpopular to see mB, but we cannot use 1/100 of dB, so I could understand it.
Actually, the gain is expressed in V/V, not in dB. I may have phrased it poorly, but since there are fractional values like 0.364 and 0.143, I chose to represent it in milli-units.
Why your reply to is corrupted:
"[email protected]"?
Quoted 8 lines
Quoted 5 lines
What sort of unit is milli-unit? Isn't this 1/1000 of some BASE unit, but you do not have here a base?
I think you want just basis point if this is V/V (already in common property suffixes)
Nice. I didn't know about -bp. That does sound like a good choice for ratio
stuff and here would be 100x larger actual values which is fine.
Yes, it would be, but the here it is 1000x larger than the actual value (1/1000 V/V). I don't see another unit in property-units.yaml for this specifc case. Maybe using -milli suffix like in 'adi,ad4000.yaml' and 'adi,ad7380.yaml'?
Quoted 4 lines
Quoted 4 lines
Best regards,
Krzysztof
Re: [PATCH 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts
Quoted 27 lines · Jonathan Santos
Quoted 26 lines · Jonathan Cameron
Quoted 23 lines · On Thu, 14 Aug 2025 08:03:23 +0200 Krzysztof Kozlowski <[email protected]> wrote:
Quoted 6 lines · Jonathan Santos
Quoted 6 lines
Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml +4
-0+ adi,aaf-gain: + description: | + Specifies the gain of the Analog Anti-Aliasing Filter (AAF) applied to the + ADC input, measured in milli-units. The AAF provides additional signalQuoted 8 lines
Quoted 4 lines
What is milli unit? Isn't gain in dB, so maybe you want mB? Quite unpopular to see mB, but we cannot use 1/100 of dB, so I could understand it.
Actually, the gain is expressed in V/V, not in dB. I may have phrased it poorly, but since there are fractional values like 0.364 and 0.143, I chose to represent it in milli-units.
Why your reply to is corrupted:
"[email protected]"?What sort of unit is milli-unit? Isn't this 1/1000 of some BASE unit, but you do not have here a base?
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
I think you want just basis point if this is V/V (already in common property suffixes)
Nice. I didn't know about -bp. That does sound like a good choice for ratio
stuff and here would be 100x larger actual values which is fine.Yes, it would be, but the here it is 1000x larger than the actual value (1/1000 V/V). I don't see another unit in
Huh? How? 1000x larger would be = 1... This makes no sense...
Quoted 2 lines
property-units.yamlfor this specifc case. Maybe using -milli suffix like in 'adi,ad4000.yaml' and 'adi,ad7380.yaml'?
Best regards,
Krzysztof
Re: [PATCH 1/4] dt-bindings: iio: adc: ad7768-1: add new supported parts
Hi,
Quoted 30 lines · Krzysztof Kozlowski
Quoted 29 lines · Jonathan Santos
Quoted 28 lines · Jonathan Cameron
Quoted 25 lines · On Thu, 14 Aug 2025 08:03:23 +0200 Krzysztof Kozlowski <[email protected]> wrote:
Quoted 6 lines · Jonathan Santos
Quoted 6 lines
Quoted 5 lines
(quoted diff) +4
-0+ adi,aaf-gain: + description: | + Specifies the gain of the Analog Anti-Aliasing Filter (AAF) applied to the + ADC input, measured in milli-units. The AAF provides additional signalQuoted 8 lines
Quoted 4 lines
What is milli unit? Isn't gain in dB, so maybe you want mB? Quite unpopular to see mB, but we cannot use 1/100 of dB, so I could understand it.
Actually, the gain is expressed in V/V, not in dB. I may have phrased it poorly, but since there are fractional values like 0.364 and 0.143, I chose to represent it in milli-units.
Why your reply to is corrupted:
"[email protected]"?What sort of unit is milli-unit? Isn't this 1/1000 of some BASE unit, but you do not have here a base?
I think you want just basis point if this is V/V (already in common property suffixes)
Quoted 13 lines
Quoted 6 lines
Quoted 3 lines
Nice. I didn't know about -bp. That does sound like a good choice for ratio
stuff and here would be 100x larger actual values which is fine.Yes, it would be, but the here it is 1000x larger than the actual value (1/1000 V/V). I don't see another unit in
Huh? How? 1000x larger would be = 1... This makes no sense...
Quoted 2 lines
property-units.yamlfor this specifc case. Maybe using -milli suffix like in 'adi,ad4000.yaml' and 'adi,ad7380.yaml'?
After having a look at the data sheets for these parts (AD7768-1, ADAQ7767-1, ADAQ7768-1, ADAQ7769-1), I think this indeed has similar semantics to adi,gain-milli. The data sheet says the AAF selection has an impact on the -3dB cutoff frequency but, for a given ODR, there is no difference between the reported −3 dB Bandwidth (kHz) in Table 12, Table 32, and Table 35 from AD7768-1, ADAQ7767-1, and ADAQ7769-1, respectively. So, looks like this is just about signal amplification/attenuation and so the use of adi,gain-milli sounds appropriate to me.
Considering the AAF doesn't really impact the -3dB cutoff frequency, maybe the property could be documented like:
adi,gain-milli:
description: |
Specifies the gain applied by the Analog Anti-Aliasing Filter (AAF) to the ADC input (in milli units). The hardware gain is determined by which input
pin(s) the signal goes through into the AAF. The possible connections are:
* For the ADAQ7767-1: input signal connected to IN1±, IN2± or IN3±.
* For the ADAQ7769-1: OUT_PGA pin connected to IN1_AAF+, IN2_AAF+, or IN3_AAF+.
If not present, default to 1000 (no actual gain applied).
$ref: /schemas/types.yaml#/definitions/uint16
enum: [143, 364, 1000]
default: 1000
Would something along those lines make sense for these devices?
Best regards,
Marcelo
PATCH 2/4 iio: adc: ad7768-1: introduce chip info for future multidevice support
In reply to Jonathan SantosAdd Chip info struct in SPI device to store channel information for each supported part.
--- drivers/iio/adc/ad7768-1.c | 76 ++++++++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 23 deletions(-)
drivers/iio/adc/ad7768-1.c +53 -23
@@ -106,6 +106,7 @@#define AD7768_GPIO_READ_MSK GENMASK(3, 0)#define AD7768_VCM_OFF 0x07 +#define AD7768_CHAN_INFO_NONE 0#define AD7768_TRIGGER_SOURCE_SYNC_IDX 0
@@ -213,6 +214,13 @@ static const struct iio_scan_type ad7768_scan_type[] = { }, }; +struct ad7768_chip_info { +const char *name; +const struct iio_chan_spec *channel_spec; +const unsigned long *available_masks; +int num_channels; +}; +struct ad7768_state {struct spi_device *spi;struct regmap *regmap;
@@ -234,6 +242,7 @@ struct ad7768_state {struct gpio_desc *gpio_reset;const char *labels[AD7768_MAX_CHANNELS ];struct gpio_chip gpiochip; +const struct ad7768_chip_info *chip;bool en_spi_sync;/* * DMA (thus cache coherency maintenance) may require the
@@ -750,24 +759,27 @@ static const struct iio_chan_spec_ext_info ad7768_ext_info[] = { { } }; +#define AD7768_CHAN (_idx, _msk_avail) { \ + .type = IIO_VOLTAGE,\ + .info_mask_separate_available = _msk_avail,\ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),\ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \ + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),\ + .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),\ + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),\ + .ext_info = ad7768_ext_info,\ + .indexed = 1,\ + .channel = _idx,\ + .scan_index = _idx,\ + .has_ext_scan_type = 1,\ + .ext_scan_type = ad7768_scan_type,\ + .num_ext_scan_type = ARRAY_SIZE(ad7768_scan_type),\ +} +static const struct iio_chan_spec ad7768_channels[] = { - { - .type =IIO_VOLTAGE , - .info_mask_separate =BIT (IIO_CHAN_INFO_RAW ), - .info_mask_shared_by_type =BIT (IIO_CHAN_INFO_SCALE ) | -BIT (IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY ) | -BIT (IIO_CHAN_INFO_OVERSAMPLING_RATIO ), - .info_mask_shared_by_type_available =BIT (IIO_CHAN_INFO_OVERSAMPLING_RATIO ), - .info_mask_shared_by_all =BIT (IIO_CHAN_INFO_SAMP_FREQ ), - .info_mask_shared_by_all_available =BIT (IIO_CHAN_INFO_SAMP_FREQ ), - .ext_info = ad7768_ext_info, - .indexed =1 , - .channel =0 , - .scan_index =0 , - .has_ext_scan_type =1 , - .ext_scan_type = ad7768_scan_type, - .num_ext_scan_type =ARRAY_SIZE (ad7768_scan_type), - }, +AD7768_CHAN (0 ,AD7768_CHAN_INFO_NONE ), }; static int ad7768_read_raw(struct iio_dev *indio_dev,
@@ -1334,6 +1346,18 @@ static int ad7768_register_regulators(struct device *dev, struct ad7768_state *sreturn 0 ; } +static const unsigned long ad7768_channel_masks[] = { +BIT (0 ), +0 , +}; + +static const struct ad7768_chip_info ad7768_chip_info = { + .name ="ad7768-1" , + .channel_spec = ad7768_channels, + .num_channels =ARRAY_SIZE (ad7768_channels), + .available_masks = ad7768_channel_masks, +}; +static int ad7768_probe (struct spi_device *spi) {struct ad7768_state *st;
@@ -1392,9 +1416,15 @@ static int ad7768_probe(struct spi_device *spi) st->mclk_freq =clk_get_rate (st->mclk); - indio_dev->channels = ad7768_channels; - indio_dev->num_channels =ARRAY_SIZE (ad7768_channels); - indio_dev->name =spi_get_device_id (spi)->name; + st->chip =spi_get_device_match_data (spi); +if (!st->chip) +return dev_err_probe (&spi->dev, -ENODEV , +"Could not find chip info data\n" ); + + indio_dev->channels = st->chip->channel_spec; + indio_dev->num_channels = st->chip->num_channels; + indio_dev->available_scan_masks = st->chip->available_masks; + indio_dev->name = st->chip->name; indio_dev->info = &ad7768_info; indio_dev->modes =INDIO_DIRECT_MODE ;
@@ -1411,7 +1441,7 @@ static int ad7768_probe(struct spi_device *spi)init_completion (&st->completion); - ret =ad7768_set_channel_label (indio_dev,ARRAY_SIZE (ad7768_channels)); + ret =ad7768_set_channel_label (indio_dev, st->chip->num_channels);if (ret)return ret;
@@ -1430,13 +1460,13 @@ static int ad7768_probe(struct spi_device *spi) }static const struct spi_device_id ad7768_id_table[] = { - {"ad7768-1" ,0 }, + {"ad7768-1" , (kernel_ulong_t)&ad7768_chip_info }, { } };MODULE_DEVICE_TABLE (spi, ad7768_id_table);static const struct of_device_id ad7768_of_match[] = { - { .compatible ="adi,ad7768-1" }, + { .compatible ="adi,ad7768-1" , .data = &ad7768_chip_info }, { } };MODULE_DEVICE_TABLE (of, ad7768_of_match);
Signature · 1 line
2.34.1
Re: [PATCH 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support
Quoted 3 lines · Jonathan Santos
Add Chip info struct in SPI device to store channel information for each supported part.
...
drivers/iio/adc/ad7768-1.c static int ad7768_register_regulators(struct device *dev, struct ad7768_state *s · line 1351
Quoted 4 lines
drivers/iio/adc/ad7768-1.c +3
-0+static const unsigned long ad7768_channel_masks[] = { +BIT (0 ), +0 ,
Please, drop the trailing comma. This '0' is a terminator, nothing should go after it.
Quoted 1 line
+};
Signature · 2 lines
With Best Regards, Andy Shevchenko
Jonathan Cameron
maintainerRe: [PATCH 2/4] iio: adc: ad7768-1: introduce chip info for future multidevice support
In reply to Jonathan SantosQuoted 4 lines · On Tue, 12 Aug 2025 23:48:57 -0300 Jonathan Santos…
Add Chip info struct in SPI device to store channel information for each supported part.
Signed-off-by Jonathan Santos <[email protected]>
Some minor comments
drivers/iio/adc/ad7768-1.c line 109
Quoted 13 lines
--- drivers/iio/adc/ad7768-1.c | 76 ++++++++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 23 deletions(-)drivers/iio/adc/ad7768-1.c +1
-0@@ -106,6 +106,7 @@#define AD7768_GPIO_READ_MSK GENMASK(3, 0)#define AD7768_VCM_OFF 0x07 +#define AD7768_CHAN_INFO_NONE 0
I'm not convinced this is worthwhile vs 0 which is fairly obviously a 'nothing to see here' value.
drivers/iio/adc/ad7768-1.c static const struct iio_scan_type ad7768_scan_type[] = { · line 221
Quoted 12 lines
drivers/iio/adc/ad7768-1.c +5
-0#define AD7768_TRIGGER_SOURCE_SYNC_IDX 0@@ -213,6 +214,13 @@ static const struct iio_scan_type ad7768_scan_type[] = { }, }; +struct ad7768_chip_info { +const char *name; +const struct iio_chan_spec *channel_spec; +const unsigned long *available_masks; +int num_channels;
I guess hole concerns pushed num_channels after the pointers. That's fine but if you move available_masks up one line, then we can still have channel_spec and the thing that says how bit it is next to each other.
Quoted 1 line
drivers/iio/adc/ad7768-1.c +1
-0+};
PATCH 3/4 iio: adc: ad7768-1: use devm_regulator_get_enable_read_voltage
In reply to Jonathan SantosUse devm_regulator_get_enable_read_voltage function as a standard and concise way of reading the voltage from the regulator and keep the regulator enabled. Replace the regulator descriptor with the direct voltage value in the device struct.
--- drivers/iio/adc/ad7768-1.c | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-)
drivers/iio/adc/ad7768-1.c +7 -22
@@ -225,7 +225,7 @@ struct ad7768_state {struct spi_device *spi;struct regmap *regmap;struct regmap *regmap24; -struct regulator *vref; +int vref_uv;struct regulator_dev *vcm_rdev;unsigned int vcm_output_sel;struct clk *mclk;
@@ -809,7 +809,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,return IIO_VAL_INT ;case IIO_CHAN_INFO_SCALE : - scale_uv =regulator_get_voltage (st->vref); + scale_uv = st->vref_uv;if (scale_uv <0 )return scale_uv;
@@ -1146,13 +1146,6 @@ static const struct iio_trigger_ops ad7768_trigger_ops = { .validate_device = iio_trigger_validate_own_device, }; -static void ad7768_regulator_disable (void *data) -{ -struct ad7768_state *st = data; - -regulator_disable (st->vref); -} -static int ad7768_set_channel_label (struct iio_dev *indio_dev,int num_channels) {
@@ -1396,19 +1389,11 @@ static int ad7768_probe(struct spi_device *spi)return dev_err_probe (&spi->dev,PTR_ERR (st->regmap24),"Failed to initialize regmap24" ); - st->vref =devm_regulator_get (&spi->dev,"vref" ); -if (IS_ERR (st->vref)) -return PTR_ERR (st->vref); - - ret =regulator_enable (st->vref); -if (ret) { -dev_err (&spi->dev,"Failed to enable specified vref supply\n" ); -return ret; - } - - ret =devm_add_action_or_reset (&spi->dev, ad7768_regulator_disable, st); -if (ret) -return ret; + ret =devm_regulator_get_enable_read_voltage (&spi->dev,"vref" ); +if (ret <0 ) +return dev_err_probe (&spi->dev, ret, +"Failed to get VREF voltage\n" ); + st->vref_uv = ret; st->mclk =devm_clk_get_enabled (&spi->dev,"mclk" );if (IS_ERR (st->mclk))
Signature · 1 line
2.34.1
Re: [PATCH 3/4] iio: adc: ad7768-1: use devm_regulator_get_enable_read_voltage
Quoted 2 lines · Jonathan Santos
Use
devm_regulator_get_enable_read_voltagefunction as a standard and
..._voltage()
(Note the parentheses)
Quoted 3 lines
concise way of reading the voltage from the regulator and keep the regulator enabled. Replace the regulator descriptor with the direct voltage value in the device struct.
...
drivers/iio/adc/ad7768-1.c struct ad7768_state {
Quoted 12 lines
struct
ad7768_state{struct spi_device *spi; struct regmap *regmap; struct regmap *regmap24;drivers/iio/adc/ad7768-1.c +1
-1-struct regulator *vref; +int vref_uv;struct regulator_dev *vcm_rdev;unsigned int vcm_output_sel;struct clk *mclk; };
Does pahole agree with your arrangement?
Signature · 2 lines
With Best Regards, Andy Shevchenko
David Lechner
reviewerRe: [PATCH 3/4] iio: adc: ad7768-1: use devm_regulator_get_enable_read_voltage
In reply to Jonathan SantosQuoted 24 lines · Jonathan Santos
Use
devm_regulator_get_enable_read_voltagefunction as a standard and concise way of reading the voltage from the regulator and keep the regulator enabled. Replace the regulator descriptor with the direct voltage value in the device struct.Signed-off-by Jonathan Santos <[email protected]>--- drivers/iio/adc/ad7768-1.c | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-)drivers/iio/adc/ad7768-1.c +1
-1@@ -225,7 +225,7 @@ struct ad7768_state {struct spi_device *spi;struct regmap *regmap;struct regmap *regmap24; -struct regulator *vref; +int vref_uv;struct regulator_dev *vcm_rdev;unsigned int vcm_output_sel;struct clk *mclk;
drivers/iio/adc/ad7768-1.c static int ad7768_read_raw(struct iio_dev *indio_dev, · line 814
Quoted 7 lines
drivers/iio/adc/ad7768-1.c +1
-1@@ -809,7 +809,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,return IIO_VAL_INT ;case IIO_CHAN_INFO_SCALE : - scale_uv =regulator_get_voltage (st->vref); + scale_uv = st->vref_uv;if (scale_uv <0 )return scale_uv;
Can also drop the error check since we've already done that in probe.
Re: [PATCH 3/4] iio: adc: ad7768-1: use devm_regulator_get_enable_read_voltage
In reply to Jonathan SantosQuoted 7 lines · Jonathan Santos
Use
devm_regulator_get_enable_read_voltagefunction as a standard and concise way of reading the voltage from the regulator and keep the regulator enabled. Replace the regulator descriptor with the direct voltage value in the device struct.Signed-off-by Jonathan Santos <[email protected]>---
With David's comment fixed:
Quoted 64 lines
drivers/iio/adc/ad7768-1.c | 29 +++++++----------------------
1 file changed, 7 insertions(+), 22 deletions(-)drivers/iio/adc/ad7768-1.c +1
-1@@ -225,7 +225,7 @@ struct ad7768_state { struct spi_device *spi; struct regmap *regmap; struct regmap *regmap24; -struct regulator *vref; +int vref_uv; struct regulator_dev *vcm_rdev; unsigned int vcm_output_sel; struct clk *mclk;@@ -809,7 +809,7 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, return IIO_VAL_INT;
caseIIO_CHAN_INFO_SCALE:drivers/iio/adc/ad7768-1.c +1
-1- scale_uv =regulator_get_voltage (st->vref); + scale_uv = st->vref_uv;if (
scale_uv< 0)
returnscale_uv;drivers/iio/adc/ad7768-1.c +0
-0@@ -1146,13 +1146,6 @@ static const struct iio_trigger_ops ad7768_trigger_ops = { .validate_device = iio_trigger_validate_own_device, };drivers/iio/adc/ad7768-1.c +0
-7-static void ad7768_regulator_disable (void *data) -{ -struct ad7768_state *st = data; - -regulator_disable (st->vref); -} -static int
ad7768_set_channel_label(struct iio_dev *indio_dev, int num_channels){drivers/iio/adc/ad7768-1.c +0
-0@@ -1396,19 +1389,11 @@ static int ad7768_probe(struct spi_device *spi) return dev_err_probe(&spi->dev, PTR_ERR(st->regmap24), "Failed to initialize regmap24");drivers/iio/adc/ad7768-1.c +5
-13- st->vref =devm_regulator_get (&spi->dev,"vref" ); -if (IS_ERR (st->vref)) -return PTR_ERR (st->vref); - - ret =regulator_enable (st->vref); -if (ret) { -dev_err (&spi->dev,"Failed to enable specified vref supply\n" ); -return ret; - } - - ret =devm_add_action_or_reset (&spi->dev, ad7768_regulator_disable, st); -if (ret) -return ret; + ret =devm_regulator_get_enable_read_voltage (&spi->dev,"vref" ); +if (ret <0 ) +return dev_err_probe (&spi->dev, ret, + "Failed to get VREF voltage\n"); + st->vref_uv = ret;
st->mclk=devm_clk_get_enabled(&spi->dev, "mclk"); if (IS_ERR(st->mclk))
Re: [PATCH 3/4] iio: adc: ad7768-1: use devm_regulator_get_enable_read_voltage
In reply to Jonathan SantosQuoted 7 lines · Jonathan Santos
Use
devm_regulator_get_enable_read_voltagefunction as a standard and concise way of reading the voltage from the regulator and keep the regulator enabled. Replace the regulator descriptor with the direct voltage value in the device struct.Signed-off-by Jonathan Santos <[email protected]>---
With David's and Andy's comments addressed:
PATCH 4/4 iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
In reply to Jonathan SantosAdd support for ADAQ7767/68/69-1 series, which includes PGIA and Anti-aliasing filter (AAF) gains.
The PGA gain is configured in run-time through the scale attribute, if supported by the device. The scale options are updated according to the output data width.
The AAF gain is configured in the devicetree and it should correspond to the AAF channel selected in hardware.
--- drivers/iio/adc/ad7768-1.c | 286 ++++++++++++++++++++++++++++++++++++- 1 file changed, 279 insertions(+), 7 deletions(-)
drivers/iio/adc/ad7768-1.c +279 -7
@@ -16,6 +16,7 @@#include <linux/interrupt.h> #include <linux/minmax.h> #include <linux/module.h> +#include <linux/rational.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/regulator/driver.h>
@@ -98,16 +99,22 @@/* AD7768_REG_GPIO_CONTROL */ #define AD7768_GPIO_UNIVERSAL_EN BIT(7)#define AD7768_GPIO_CONTROL_MSK GENMASK(3, 0) +#define AD7768_GPIO_PGIA_EN (AD7768_GPIO_UNIVERSAL_EN | GENMASK(2, 0))/* AD7768_REG_GPIO_WRITE */ #define AD7768_GPIO_WRITE_MSK GENMASK(3, 0) +#define AD7768_GPIO_WRITE (x) FIELD_PREP(AD7768_GPIO_WRITE_MSK, x)/* AD7768_REG_GPIO_READ */ #define AD7768_GPIO_READ_MSK GENMASK(3, 0) +#define AD7768_GPIO_READ (x) FIELD_PREP(AD7768_GPIO_READ_MSK, x)#define AD7768_VCM_OFF 0x07#define AD7768_CHAN_INFO_NONE 0 +#define ADAQ776X_GAIN_MAX_NANO (128 * NANO) +#define ADAQ776X_MAX_GAIN_MODES 8 +#define AD7768_TRIGGER_SOURCE_SYNC_IDX 0#define AD7768_MAX_CHANNELS 1
@@ -154,6 +161,52 @@ enum ad7768_scan_type { AD7768_SCAN_TYPE_HIGH_SPEED, }; +enum { +AD7768_PGA_GAIN_0 , +AD7768_PGA_GAIN_1 , +AD7768_PGA_GAIN_2 , +AD7768_PGA_GAIN_3 , +AD7768_PGA_GAIN_4 , +AD7768_PGA_GAIN_5 , +AD7768_PGA_GAIN_6 , +AD7768_PGA_GAIN_7 , +AD7768_MAX_PGA_GAIN , +}; + +enum { +AD7768_AAF_IN1 , +AD7768_AAF_IN2 , +AD7768_AAF_IN3 , +}; + +/* PGA and AAF gains in V/V */ +static const int adaq7768_gains[7 ] = { + [AD7768_PGA_GAIN_0 ] =325 ,/* 0.325 */ + [AD7768_PGA_GAIN_1 ] =650 ,/* 0.650 */ + [AD7768_PGA_GAIN_2 ] =1300 ,/* 1.300 */ + [AD7768_PGA_GAIN_3 ] =2600 ,/* 2.600 */ + [AD7768_PGA_GAIN_4 ] =5200 ,/* 5.200 */ + [AD7768_PGA_GAIN_5 ] =10400 ,/* 10.400 */ + [AD7768_PGA_GAIN_6 ] =20800 /* 20.800 */ +}; + +static const int adaq7769_gains[8 ] = { + [AD7768_PGA_GAIN_0 ] =1000 ,/* 1.000 */ + [AD7768_PGA_GAIN_1 ] =2000 ,/* 2.000 */ + [AD7768_PGA_GAIN_2 ] =4000 ,/* 4.000 */ + [AD7768_PGA_GAIN_3 ] =8000 ,/* 8.000 */ + [AD7768_PGA_GAIN_4 ] =16000 ,/* 16.000 */ + [AD7768_PGA_GAIN_5 ] =32000 ,/* 32.000 */ + [AD7768_PGA_GAIN_6 ] =64000 ,/* 64.000 */ + [AD7768_PGA_GAIN_7 ] =128000 /* 128.000 */ +}; + +static const int ad7768_aaf_gains[3 ] = { + [AD7768_AAF_IN1 ] =1000 ,/* 1.000 */ + [AD7768_AAF_IN2 ] =364 ,/* 0.364 */ + [AD7768_AAF_IN3 ] =143 /* 0.143 */ +}; +/* -3dB cutoff frequency multipliers (relative to ODR) for each filter type. */ static const int ad7768_filter_3db_odr_multiplier[] = { [AD7768_FILTER_SINC5 ] =204 ,/* 0.204 */
@@ -216,6 +269,12 @@ static const struct iio_scan_type ad7768_scan_type[] = {struct ad7768_chip_info {const char *name; +bool has_variable_aaf; +bool has_pga; +int num_pga_modes; +int default_pga_mode; +int pgia_mode2pin_offset; +const int *pga_gains;const struct iio_chan_spec *channel_spec;const unsigned long *available_masks;int num_channels;
@@ -236,6 +295,9 @@ struct ad7768_state {unsigned int samp_freq;unsigned int samp_freq_avail[ARRAY_SIZE (ad7768_mclk_div_rates)];unsigned int samp_freq_avail_len; +int pga_gain_mode; +int aaf_gain; +int scale_tbl[ADAQ776X_MAX_GAIN_MODES ][2 ];struct completion completion;struct iio_trigger *trig;struct gpio_desc *gpio_sync_in;
@@ -466,6 +528,43 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,return ret; } +static void ad7768_fill_scale_tbl (struct iio_dev *dev) +{ +struct ad7768_state *st =iio_priv (dev); +const struct iio_scan_type *scan_type; +int val, val2, tmp0, tmp1, i; +unsigned long denominator, numerator; +u64 tmp2; + + scan_type =iio_get_current_scan_type (dev, &dev->channels[0 ]); +if (scan_type->sign =='s' ) + val2 = scan_type->realbits -1 ; +else + val2 = scan_type->realbits; + +for (i =0 ; i < st->chip->num_pga_modes; i++) { +/* Convert gain to a fraction format */ + numerator = st->chip->pga_gains[i]; + denominator =MILLI ; +if (st->chip->has_variable_aaf) { + numerator *= ad7768_aaf_gains[st->aaf_gain]; + denominator *=MILLI ; + } + +rational_best_approximation (numerator, denominator, __INT_MAX__, __INT_MAX__, + &numerator, &denominator); + + val = st->vref_uv /1000 ; +/* Multiply by MILLI here to avoid losing precision */ + val =mult_frac (val, denominator *MILLI , numerator); +/* Would multiply by NANO here but we already multiplied by MILLI */ + tmp2 =shift_right ((u64 )val *MICRO , val2); + tmp0 = (int )div_s64_rem (tmp2,NANO , &tmp1); + st->scale_tbl[i][0 ] = tmp0;/* Integer part */ + st->scale_tbl[i][1 ] =abs (tmp1);/* Fractional part */ + } +} +static int ad7768_set_sinc3_dec_rate (struct ad7768_state *st,unsigned int dec_rate) {
@@ -567,12 +666,68 @@ static int ad7768_configure_dig_fil(struct iio_dev *dev, st->oversampling_ratio = ad7768_dec_rate_values[dec_rate_idx]; } +/* Update scale table: scale values vary according to the precision */ +ad7768_fill_scale_tbl (dev); +ad7768_fill_samp_freq_tbl (st);/* A sync-in pulse is required after every configuration change */ return ad7768_send_sync_pulse (st); } +static int ad7768_calc_pga_gain (struct ad7768_state *st,int gain_int, +int gain_fract,int precision) +{ +u64 gain_nano, tmp; +int gain_idx; + + precision--; + gain_nano = gain_int *NANO + gain_fract; +if (gain_nano <0 || gain_nano >ADAQ776X_GAIN_MAX_NANO ) +return -EINVAL ; + + tmp =DIV_ROUND_CLOSEST_ULL (gain_nano << precision,NANO ); + gain_nano =DIV_ROUND_CLOSEST_ULL (st->vref_uv, tmp); +if (st->chip->has_variable_aaf) +/* remove the AAF gain from the overall gain */ + gain_nano =DIV_ROUND_CLOSEST_ULL (gain_nano *MILLI , + ad7768_aaf_gains[st->aaf_gain]); + tmp = st->chip->num_pga_modes; + gain_idx =find_closest (gain_nano, st->chip->pga_gains, tmp); + +return gain_idx; +} + +static int ad7768_set_pga_gain (struct ad7768_state *st, +int gain_mode) +{ +int pgia_pins_value =abs (gain_mode - st->chip->pgia_mode2pin_offset); +int check_val; +int ret; + +/* Check GPIO control register */ + ret =regmap_read (st->regmap,AD7768_REG_GPIO_CONTROL , &check_val); +if (ret <0 ) +return ret; + +if ((check_val &AD7768_GPIO_PGIA_EN ) !=AD7768_GPIO_PGIA_EN ) { +/* Enable PGIA GPIOs and set them as output */ + ret =regmap_write (st->regmap,AD7768_REG_GPIO_CONTROL ,AD7768_GPIO_PGIA_EN ); +if (ret <0 ) +return ret; + } + +/* Write the respective gain values to GPIOs 0, 1, 2 */ + ret =regmap_write (st->regmap,AD7768_REG_GPIO_WRITE , +AD7768_GPIO_WRITE (pgia_pins_value)); +if (ret <0 ) +return ret; + + st->pga_gain_mode = gain_mode; + +return 0 ; +} +static int ad7768_gpio_direction_input (struct gpio_chip *chip,unsigned int offset) {struct iio_dev *indio_dev =gpiochip_get_data (chip);
@@ -782,13 +937,17 @@ static const struct iio_chan_spec ad7768_channels[] = { AD7768_CHAN(0, AD7768_CHAN_INFO_NONE), }; +static const struct iio_chan_spec adaq776x_channels[] = { +AD7768_CHAN (0 ,BIT (IIO_CHAN_INFO_SCALE )), +}; +static int ad7768_read_raw (struct iio_dev *indio_dev,struct iio_chan_spec const *chan,int *val,int *val2,long info) {struct ad7768_state *st =iio_priv (indio_dev);const struct iio_scan_type *scan_type; -int scale_uv, ret, temp; +int ret, temp; scan_type =iio_get_current_scan_type (indio_dev, chan);if (IS_ERR (scan_type))
@@ -809,12 +968,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,return IIO_VAL_INT ;case IIO_CHAN_INFO_SCALE : - scale_uv = st->vref_uv; -if (scale_uv <0 ) -return scale_uv; - - *val = (scale_uv *2 ) /1000 ; - *val2 = scan_type->realbits; +if (st->chip->has_pga) { + *val = st->scale_tbl[st->pga_gain_mode][0 ]; + *val2 = st->scale_tbl[st->pga_gain_mode][1 ]; +return IIO_VAL_INT_PLUS_NANO ; + } + *val = st->vref_uv /1000 ; +if (st->chip->has_variable_aaf) + *val = (*val *MILLI ) / ad7768_aaf_gains[st->aaf_gain]; +/* +* ADC output code is two's complement so only (realbits - 1) +* bits express voltage magnitude. +*/ + *val2 = scan_type->realbits -1 ;return IIO_VAL_FRACTIONAL_LOG2 ;
@@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, *length = st->samp_freq_avail_len; *type =IIO_VAL_INT ;return IIO_AVAIL_LIST ; +case IIO_CHAN_INFO_SCALE : + *vals = (int *)st->scale_tbl; + *length = st->chip->num_pga_modes *2 ; + *type =IIO_VAL_INT_PLUS_NANO ; +return IIO_AVAIL_LIST ;default :return -EINVAL ; } } +static int ad7768_write_raw_get_fmt (struct iio_dev *indio_dev, +struct iio_chan_spec const *chan,long mask) +{ +switch (mask) { +case IIO_CHAN_INFO_SCALE : +return IIO_VAL_INT_PLUS_NANO ; +default : +return IIO_VAL_INT_PLUS_MICRO ; + } + +return -EINVAL ; +} +static int __ad7768_write_raw (struct iio_dev *indio_dev,struct iio_chan_spec const *chan,int val,int val2,long info) {struct ad7768_state *st =iio_priv (indio_dev); +const struct iio_scan_type *scan_type; +int gain_mode;int ret; + scan_type =iio_get_current_scan_type (indio_dev, chan); +if (IS_ERR (scan_type)) +return PTR_ERR (scan_type); +switch (info) {case IIO_CHAN_INFO_SAMP_FREQ :return ad7768_set_freq (st, val);
@@ -892,6 +1082,13 @@ static int __ad7768_write_raw(struct iio_dev *indio_dev,/* Update sampling frequency */ return ad7768_set_freq (st, st->samp_freq); +case IIO_CHAN_INFO_SCALE : +if (!st->chip->has_pga) +return -EOPNOTSUPP ; + + gain_mode =ad7768_calc_pga_gain (st, val, val2, + scan_type->realbits); +return ad7768_set_pga_gain (st, gain_mode);default :return -EINVAL ; }
@@ -933,6 +1130,7 @@ static const struct iio_info ad7768_info = { .read_raw = &ad7768_read_raw, .read_avail = &ad7768_read_avail, .write_raw = &ad7768_write_raw, + .write_raw_get_fmt = &ad7768_write_raw_get_fmt, .read_label = ad7768_read_label, .get_current_scan_type = &ad7768_get_current_scan_type, .debugfs_reg_access = &ad7768_reg_access,
@@ -1351,10 +1549,46 @@ static const struct ad7768_chip_info ad7768_chip_info = { .available_masks = ad7768_channel_masks, }; +static const struct ad7768_chip_info adaq7767_chip_info = { + .name ="adaq7767-1" , + .channel_spec = ad7768_channels, + .num_channels =ARRAY_SIZE (ad7768_channels), + .available_masks = ad7768_channel_masks, + .has_pga = false, + .has_variable_aaf = true +}; + +static const struct ad7768_chip_info adaq7768_chip_info = { + .name ="adaq7768-1" , + .channel_spec = adaq776x_channels, + .num_channels =ARRAY_SIZE (adaq776x_channels), + .available_masks = ad7768_channel_masks, + .pga_gains = adaq7768_gains, + .default_pga_mode =AD7768_PGA_GAIN_2 , + .num_pga_modes =ARRAY_SIZE (adaq7768_gains), + .pgia_mode2pin_offset =6 , + .has_pga = true, + .has_variable_aaf = false +}; + +static const struct ad7768_chip_info adaq7769_chip_info = { + .name ="adaq7769-1" , + .channel_spec = adaq776x_channels, + .num_channels =ARRAY_SIZE (adaq776x_channels), + .available_masks = ad7768_channel_masks, + .pga_gains = adaq7769_gains, + .default_pga_mode =AD7768_PGA_GAIN_0 , + .num_pga_modes =ARRAY_SIZE (adaq7769_gains), + .pgia_mode2pin_offset =0 , + .has_pga = true, + .has_variable_aaf = true +}; +static int ad7768_probe (struct spi_device *spi) {struct ad7768_state *st;struct iio_dev *indio_dev; +u32 val;int ret; indio_dev =devm_iio_device_alloc (&spi->dev,sizeof (*st));
@@ -1418,6 +1652,35 @@ static int ad7768_probe(struct spi_device *spi)if (ret)return ret; + st->aaf_gain =AD7768_AAF_IN1 ; + ret =device_property_read_u32 (&spi->dev,"adi,aaf-gain" , &val); +if (ret) { +/* AAF gain required, but not specified */ +if (st->chip->has_variable_aaf) +return dev_err_probe (&spi->dev, -EINVAL ,"AAF gain not specified\n" ); + + }else if (!st->chip->has_variable_aaf) { +/* AAF gain provided, but not supported */ +return dev_err_probe (&spi->dev, -EINVAL ,"AAF gain not supported for %s\n" , + st->chip->name); + }else { +/* Device supports variable AAF gain, validate and set the gain */ +switch (val) { +case 1000 : + st->aaf_gain =AD7768_AAF_IN1 ; +break ; +case 364 : + st->aaf_gain =AD7768_AAF_IN2 ; +break ; +case 143 : + st->aaf_gain =AD7768_AAF_IN3 ; +break ; +default : +return dev_err_probe (&spi->dev, -EINVAL , +"Invalid firmware provided gain\n" ); + } + } + ret =ad7768_setup (indio_dev);if (ret <0 ) {dev_err (&spi->dev,"AD7768 setup failed\n" );
@@ -1426,6 +1689,9 @@ static int ad7768_probe(struct spi_device *spi)init_completion (&st->completion); +if (st->chip->has_pga) +ad7768_set_pga_gain (st, st->chip->default_pga_mode); + ret =ad7768_set_channel_label (indio_dev, st->chip->num_channels);if (ret)return ret;
@@ -1446,12 +1712,18 @@ static int ad7768_probe(struct spi_device *spi)static const struct spi_device_id ad7768_id_table[] = { {"ad7768-1" , (kernel_ulong_t)&ad7768_chip_info }, + {"adaq7767-1" , (kernel_ulong_t)&adaq7767_chip_info }, + {"adaq7768-1" , (kernel_ulong_t)&adaq7768_chip_info }, + {"adaq7769-1" , (kernel_ulong_t)&adaq7769_chip_info }, { } };MODULE_DEVICE_TABLE (spi, ad7768_id_table);static const struct of_device_id ad7768_of_match[] = { { .compatible ="adi,ad7768-1" , .data = &ad7768_chip_info }, + { .compatible ="adi,adaq7767-1" , .data = &adaq7767_chip_info }, + { .compatible ="adi,adaq7768-1" , .data = &adaq7768_chip_info }, + { .compatible ="adi,adaq7769-1" , .data = &adaq7769_chip_info }, { } };MODULE_DEVICE_TABLE (of, ad7768_of_match);
Signature · 1 line
2.34.1
Re: [PATCH 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
Quoted 366 lines · Jonathan Santos
Add support for ADAQ7767/68/69-1 series, which includes PGIA and Anti-aliasing filter (AAF) gains.
The PGA gain is configured in run-time through the scale attribute, if supported by the device. The scale options are updated according to the output data width.
The AAF gain is configured in the devicetree and it should correspond to the AAF channel selected in hardware.
drivers/iio/adc/ad7768-1.c +238
-7+#define AD7768_GPIO_READ (x) FIELD_PREP(AD7768_GPIO_READ_MSK, x)#define AD7768_VCM_OFF 0x07#define AD7768_CHAN_INFO_NONE 0 +#define ADAQ776X_GAIN_MAX_NANO (128 * NANO) +#define ADAQ776X_MAX_GAIN_MODES 8 +#define AD7768_TRIGGER_SOURCE_SYNC_IDX 0#define AD7768_MAX_CHANNELS 1@@ -154,6 +161,52 @@ enum ad7768_scan_type { AD7768_SCAN_TYPE_HIGH_SPEED, }; +enum { +AD7768_PGA_GAIN_0 , +AD7768_PGA_GAIN_1 , +AD7768_PGA_GAIN_2 , +AD7768_PGA_GAIN_3 , +AD7768_PGA_GAIN_4 , +AD7768_PGA_GAIN_5 , +AD7768_PGA_GAIN_6 , +AD7768_PGA_GAIN_7 , +AD7768_MAX_PGA_GAIN , +}; + +enum { +AD7768_AAF_IN1 , +AD7768_AAF_IN2 , +AD7768_AAF_IN3 , +}; + +/* PGA and AAF gains in V/V */ +static const int adaq7768_gains[7 ] = { + [AD7768_PGA_GAIN_0 ] =325 ,/* 0.325 */ + [AD7768_PGA_GAIN_1 ] =650 ,/* 0.650 */ + [AD7768_PGA_GAIN_2 ] =1300 ,/* 1.300 */ + [AD7768_PGA_GAIN_3 ] =2600 ,/* 2.600 */ + [AD7768_PGA_GAIN_4 ] =5200 ,/* 5.200 */ + [AD7768_PGA_GAIN_5 ] =10400 ,/* 10.400 */ + [AD7768_PGA_GAIN_6 ] =20800 /* 20.800 */ +}; + +static const int adaq7769_gains[8 ] = { + [AD7768_PGA_GAIN_0 ] =1000 ,/* 1.000 */ + [AD7768_PGA_GAIN_1 ] =2000 ,/* 2.000 */ + [AD7768_PGA_GAIN_2 ] =4000 ,/* 4.000 */ + [AD7768_PGA_GAIN_3 ] =8000 ,/* 8.000 */ + [AD7768_PGA_GAIN_4 ] =16000 ,/* 16.000 */ + [AD7768_PGA_GAIN_5 ] =32000 ,/* 32.000 */ + [AD7768_PGA_GAIN_6 ] =64000 ,/* 64.000 */ + [AD7768_PGA_GAIN_7 ] =128000 /* 128.000 */ +}; + +static const int ad7768_aaf_gains[3 ] = { + [AD7768_AAF_IN1 ] =1000 ,/* 1.000 */ + [AD7768_AAF_IN2 ] =364 ,/* 0.364 */ + [AD7768_AAF_IN3 ] =143 /* 0.143 */ +}; +/* -3dB cutoff frequency multipliers (relative to ODR) for each filter type. */ static const int ad7768_filter_3db_odr_multiplier[] = { [AD7768_FILTER_SINC5 ] =204 ,/* 0.204 */ @@ -216,6 +269,12 @@ static const struct iio_scan_type ad7768_scan_type[] = {struct ad7768_chip_info {const char *name; +bool has_variable_aaf; +bool has_pga; +int num_pga_modes; +int default_pga_mode; +int pgia_mode2pin_offset; +const int *pga_gains;const struct iio_chan_spec *channel_spec;const unsigned long *available_masks;int num_channels;@@ -236,6 +295,9 @@ struct ad7768_state {unsigned int samp_freq;unsigned int samp_freq_avail[ARRAY_SIZE (ad7768_mclk_div_rates)];unsigned int samp_freq_avail_len; +int pga_gain_mode; +int aaf_gain; +int scale_tbl[ADAQ776X_MAX_GAIN_MODES ][2 ];struct completion completion;struct iio_trigger *trig;struct gpio_desc *gpio_sync_in;@@ -466,6 +528,43 @@ static int ad7768_reg_access(struct iio_dev *indio_dev,return ret; } +static void ad7768_fill_scale_tbl (struct iio_dev *dev) +{ +struct ad7768_state *st =iio_priv (dev); +const struct iio_scan_type *scan_type; +int val, val2, tmp0, tmp1, i; +unsigned long denominator, numerator; +u64 tmp2; + + scan_type =iio_get_current_scan_type (dev, &dev->channels[0 ]); +if (scan_type->sign =='s' ) + val2 = scan_type->realbits -1 ; +else + val2 = scan_type->realbits; + +for (i =0 ; i < st->chip->num_pga_modes; i++) { +/* Convert gain to a fraction format */ + numerator = st->chip->pga_gains[i]; + denominator =MILLI ; +if (st->chip->has_variable_aaf) { + numerator *= ad7768_aaf_gains[st->aaf_gain]; + denominator *=MILLI ; + } + +rational_best_approximation (numerator, denominator, __INT_MAX__, __INT_MAX__, + &numerator, &denominator); + + val = st->vref_uv /1000 ; +/* Multiply by MILLI here to avoid losing precision */ + val =mult_frac (val, denominator *MILLI , numerator); +/* Would multiply by NANO here but we already multiplied by MILLI */ + tmp2 =shift_right ((u64 )val *MICRO , val2); + tmp0 = (int )div_s64_rem (tmp2,NANO , &tmp1); + st->scale_tbl[i][0 ] = tmp0;/* Integer part */ + st->scale_tbl[i][1 ] =abs (tmp1);/* Fractional part */ + } +} +static int ad7768_set_sinc3_dec_rate (struct ad7768_state *st,unsigned int dec_rate) {@@ -567,12 +666,68 @@ static int ad7768_configure_dig_fil(struct iio_dev *dev, st->oversampling_ratio = ad7768_dec_rate_values[dec_rate_idx]; } +/* Update scale table: scale values vary according to the precision */ +ad7768_fill_scale_tbl (dev); +ad7768_fill_samp_freq_tbl (st);/* A sync-in pulse is required after every configuration change */ return ad7768_send_sync_pulse (st); } +static int ad7768_calc_pga_gain (struct ad7768_state *st,int gain_int, +int gain_fract,int precision) +{ +u64 gain_nano, tmp; +int gain_idx; + + precision--; + gain_nano = gain_int *NANO + gain_fract; +if (gain_nano <0 || gain_nano >ADAQ776X_GAIN_MAX_NANO ) +return -EINVAL ; + + tmp =DIV_ROUND_CLOSEST_ULL (gain_nano << precision,NANO ); + gain_nano =DIV_ROUND_CLOSEST_ULL (st->vref_uv, tmp); +if (st->chip->has_variable_aaf) +/* remove the AAF gain from the overall gain */ + gain_nano =DIV_ROUND_CLOSEST_ULL (gain_nano *MILLI , + ad7768_aaf_gains[st->aaf_gain]); + tmp = st->chip->num_pga_modes; + gain_idx =find_closest (gain_nano, st->chip->pga_gains, tmp); + +return gain_idx; +} + +static int ad7768_set_pga_gain (struct ad7768_state *st, +int gain_mode) +{ +int pgia_pins_value =abs (gain_mode - st->chip->pgia_mode2pin_offset); +int check_val; +int ret; + +/* Check GPIO control register */ + ret =regmap_read (st->regmap,AD7768_REG_GPIO_CONTROL , &check_val); +if (ret <0 ) +return ret; + +if ((check_val &AD7768_GPIO_PGIA_EN ) !=AD7768_GPIO_PGIA_EN ) { +/* Enable PGIA GPIOs and set them as output */ + ret =regmap_write (st->regmap,AD7768_REG_GPIO_CONTROL ,AD7768_GPIO_PGIA_EN ); +if (ret <0 ) +return ret; + } + +/* Write the respective gain values to GPIOs 0, 1, 2 */ + ret =regmap_write (st->regmap,AD7768_REG_GPIO_WRITE , +AD7768_GPIO_WRITE (pgia_pins_value)); +if (ret <0 ) +return ret; + + st->pga_gain_mode = gain_mode; + +return 0 ; +} +static int ad7768_gpio_direction_input (struct gpio_chip *chip,unsigned int offset) {struct iio_dev *indio_dev =gpiochip_get_data (chip);@@ -782,13 +937,17 @@ static const struct iio_chan_spec ad7768_channels[] = { AD7768_CHAN(0, AD7768_CHAN_INFO_NONE), }; +static const struct iio_chan_spec adaq776x_channels[] = { +AD7768_CHAN (0 ,BIT (IIO_CHAN_INFO_SCALE )), +}; +static int ad7768_read_raw (struct iio_dev *indio_dev,struct iio_chan_spec const *chan,int *val,int *val2,long info) {struct ad7768_state *st =iio_priv (indio_dev);const struct iio_scan_type *scan_type; -int scale_uv, ret, temp; +int ret, temp; scan_type =iio_get_current_scan_type (indio_dev, chan);if (IS_ERR (scan_type))@@ -809,12 +968,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev,return IIO_VAL_INT ;case IIO_CHAN_INFO_SCALE : - scale_uv = st->vref_uv; -if (scale_uv <0 ) -return scale_uv; - - *val = (scale_uv *2 ) /1000 ; - *val2 = scan_type->realbits; +if (st->chip->has_pga) { + *val = st->scale_tbl[st->pga_gain_mode][0 ]; + *val2 = st->scale_tbl[st->pga_gain_mode][1 ]; +return IIO_VAL_INT_PLUS_NANO ; + } + *val = st->vref_uv /1000 ; +if (st->chip->has_variable_aaf) + *val = (*val *MILLI ) / ad7768_aaf_gains[st->aaf_gain]; +/* +* ADC output code is two's complement so only (realbits - 1) +* bits express voltage magnitude. +*/ + *val2 = scan_type->realbits -1 ;return IIO_VAL_FRACTIONAL_LOG2 ;@@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, *length = st->samp_freq_avail_len; *type =IIO_VAL_INT ;return IIO_AVAIL_LIST ; +case IIO_CHAN_INFO_SCALE : + *vals = (int *)st->scale_tbl; + *length = st->chip->num_pga_modes *2 ; + *type =IIO_VAL_INT_PLUS_NANO ; +return IIO_AVAIL_LIST ;default :return -EINVAL ; } } +static int ad7768_write_raw_get_fmt (struct iio_dev *indio_dev, +struct iio_chan_spec const *chan,long mask) +{ +switch (mask) { +case IIO_CHAN_INFO_SCALE : +return IIO_VAL_INT_PLUS_NANO ; +default : +return IIO_VAL_INT_PLUS_MICRO ; + } + +return -EINVAL ; +} +static int __ad7768_write_raw (struct iio_dev *indio_dev,struct iio_chan_spec const *chan,int val,int val2,long info) {struct ad7768_state *st =iio_priv (indio_dev); +const struct iio_scan_type *scan_type; +int gain_mode;int ret; + scan_type =iio_get_current_scan_type (indio_dev, chan); +if (IS_ERR (scan_type)) +return PTR_ERR (scan_type); +switch (info) {case IIO_CHAN_INFO_SAMP_FREQ :return ad7768_set_freq (st, val);@@ -892,6 +1082,13 @@ static int __ad7768_write_raw(struct iio_dev *indio_dev,/* Update sampling frequency */ return ad7768_set_freq (st, st->samp_freq); +case IIO_CHAN_INFO_SCALE : +if (!st->chip->has_pga) +return -EOPNOTSUPP ; + + gain_mode =ad7768_calc_pga_gain (st, val, val2, + scan_type->realbits); +return ad7768_set_pga_gain (st, gain_mode);default :return -EINVAL ; }@@ -933,6 +1130,7 @@ static const struct iio_info ad7768_info = { .read_raw = &ad7768_read_raw, .read_avail = &ad7768_read_avail, .write_raw = &ad7768_write_raw, + .write_raw_get_fmt = &ad7768_write_raw_get_fmt, .read_label = ad7768_read_label, .get_current_scan_type = &ad7768_get_current_scan_type, .debugfs_reg_access = &ad7768_reg_access,@@ -1351,10 +1549,46 @@ static const struct ad7768_chip_info ad7768_chip_info = { .available_masks = ad7768_channel_masks, }; +static const struct ad7768_chip_info adaq7767_chip_info = { + .name ="adaq7767-1" , + .channel_spec = ad7768_channels, + .num_channels =ARRAY_SIZE (ad7768_channels), + .available_masks = ad7768_channel_masks, + .has_pga = false, + .has_variable_aaf = true +}; + +static const struct ad7768_chip_info adaq7768_chip_info = { + .name ="adaq7768-1" , + .channel_spec = adaq776x_channels, + .num_channels =ARRAY_SIZE (adaq776x_channels), + .available_masks = ad7768_channel_masks, + .pga_gains = adaq7768_gains, + .default_pga_mode =AD7768_PGA_GAIN_2 , + .num_pga_modes =ARRAY_SIZE (adaq7768_gains), + .pgia_mode2pin_offset =6 , + .has_pga = true, + .has_variable_aaf = false +}; + +static const struct ad7768_chip_info adaq7769_chip_info = { + .name ="adaq7769-1" , + .channel_spec = adaq776x_channels, + .num_channels =ARRAY_SIZE (adaq776x_channels), + .available_masks = ad7768_channel_masks, + .pga_gains = adaq7769_gains, + .default_pga_mode =AD7768_PGA_GAIN_0 , + .num_pga_modes =ARRAY_SIZE (adaq7769_gains), + .pgia_mode2pin_offset =0 , + .has_pga = true, + .has_variable_aaf = true +}; +static int ad7768_probe (struct spi_device *spi) {struct ad7768_state *st;struct iio_dev *indio_dev; +u32 val;int ret;
...
drivers/iio/adc/ad7768-1.c static int ad7768_probe(struct spi_device *spi) · line 1661
Quoted 8 lines
drivers/iio/adc/ad7768-1.c +7
-0+ st->aaf_gain =AD7768_AAF_IN1 ; + ret =device_property_read_u32 (&spi->dev,"adi,aaf-gain" , &val); +if (ret) { +/* AAF gain required, but not specified */ +if (st->chip->has_variable_aaf) +return dev_err_probe (&spi->dev, -EINVAL ,"AAF gain not specified\n" ); +
Stray blank line.
drivers/iio/adc/ad7768-1.c static int ad7768_probe(struct spi_device *spi) · line 1665
Quoted 6 lines
drivers/iio/adc/ad7768-1.c +4
-0+ }else if (!st->chip->has_variable_aaf) { +/* AAF gain provided, but not supported */ +return dev_err_probe (&spi->dev, -EINVAL ,"AAF gain not supported for %s\n" , + st->chip->name);
I would rewrite these conditionals as
if (ret && ->has_variable_aaf)
return dev_err_probe(...);
if (!ret && ! ->has_variable_aaf)
return dev_err_probe(...);Quoted 1 line
+ } else {
It will be redundant 'else' after the above being applied.
Quoted 16 lines
drivers/iio/adc/ad7768-1.c +16
-0+/* Device supports variable AAF gain, validate and set the gain */ +switch (val) { +case 1000 : + st->aaf_gain =AD7768_AAF_IN1 ; +break ; +case 364 : + st->aaf_gain =AD7768_AAF_IN2 ; +break ; +case 143 : + st->aaf_gain =AD7768_AAF_IN3 ; +break ; +default : +return dev_err_probe (&spi->dev, -EINVAL , +"Invalid firmware provided gain\n" ); + } + }
Signature · 2 lines
With Best Regards, Andy Shevchenko
kernel test robot
build botRe: [PATCH 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
In reply to Jonathan Santoskernel test robot noticed the following build errors:
Hi Jonathan,
kernel test robot noticed the following build errors:
[auto build test ERROR on 0a686b9c4f847dc21346df8e56d5b119918fefef]
url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Santos/dt-bindings-iio-adc-ad7768-1-add-new-supported-parts/20250813-145315 base: 0a686b9c4f847dc21346df8e56d5b119918fefef patch link: https://lore.kernel.org/r/f0c1cbc9c2994a90113788cad57df1f32f9db45e.1754617360.git.Jonathan.Santos%40analog.com patch subject: [PATCH 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family config: hexagon-randconfig-002-20250814 (https://download.01.org/0day-ci/archive/20250814/[email protected]/config) compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 3769ce013be2879bf0b329c14a16f5cb766f26ce) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250814/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: rational_best_approximation >>> referenced by ad7768-1.c:554 (drivers/iio/adc/ad7768-1.c:554) >>> drivers/iio/adc/ad7768-1.o:(ad7768_configure_dig_fil) in archive vmlinux.a >>> referenced by ad7768-1.c:554 (drivers/iio/adc/ad7768-1.c:554) >>> drivers/iio/adc/ad7768-1.o:(ad7768_configure_dig_fil) in archive vmlinux.a
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
In reply to Jonathan SantosQuoted 7 lines · Jonathan Santos
Add support for ADAQ7767/68/69-1 series, which includes PGIA and Anti-aliasing filter (AAF) gains.
The PGA gain is configured in run-time through the scale attribute, if supported by the device. The scale options are updated according to the output data width.
Quoted 5 lines
The AAF gain is configured in the devicetree and it should correspond to the AAF channel selected in hardware.
Signed-off-by Jonathan Santos <[email protected]>---
Hi Jonathan,
Some comments from me...
Quoted 143 lines
drivers/iio/adc/ad7768-1.c | 286 ++++++++++++++++++++++++++++++++++++-
1 file changed, 279 insertions(+), 7 deletions(-)drivers/iio/adc/ad7768-1.c +2
-0@@ -16,6 +16,7 @@ #include <linux/interrupt.h> #include <linux/minmax.h> #include <linux/module.h> +#include <linux/rational.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/regulator/driver.h>@@ -98,16 +99,22 @@ /* AD7768_REG_GPIO_CONTROL */ #define AD7768_GPIO_UNIVERSAL_EN BIT(7) #define AD7768_GPIO_CONTROL_MSK GENMASK(3, 0) +#define AD7768_GPIO_PGIA_EN (AD7768_GPIO_UNIVERSAL_EN | GENMASK(2, 0))
/*AD7768_REG_GPIO_WRITE*/
#defineAD7768_GPIO_WRITE_MSKGENMASK(3, 0)
+#defineAD7768_GPIO_WRITE(x)FIELD_PREP(AD7768_GPIO_WRITE_MSK, x)
/*AD7768_REG_GPIO_READ*/
#defineAD7768_GPIO_READ_MSKGENMASK(3, 0)
+#defineAD7768_GPIO_READ(x)FIELD_PREP(AD7768_GPIO_READ_MSK, x)
#defineAD7768_VCM_OFF0x07
#defineAD7768_CHAN_INFO_NONE0drivers/iio/adc/ad7768-1.c +3
-0+#define ADAQ776X_GAIN_MAX_NANO (128 * NANO) +#define ADAQ776X_MAX_GAIN_MODES 8 +#define
AD7768_TRIGGER_SOURCE_SYNC_IDX0
#defineAD7768_MAX_CHANNELS1drivers/iio/adc/ad7768-1.c +0
-0@@ -154,6 +161,52 @@ enum ad7768_scan_type { AD7768_SCAN_TYPE_HIGH_SPEED, };drivers/iio/adc/ad7768-1.c +46
-0+enum { +AD7768_PGA_GAIN_0 , +AD7768_PGA_GAIN_1 , +AD7768_PGA_GAIN_2 , +AD7768_PGA_GAIN_3 , +AD7768_PGA_GAIN_4 , +AD7768_PGA_GAIN_5 , +AD7768_PGA_GAIN_6 , +AD7768_PGA_GAIN_7 , +AD7768_MAX_PGA_GAIN , +}; + +enum { +AD7768_AAF_IN1 , +AD7768_AAF_IN2 , +AD7768_AAF_IN3 , +}; + +/* PGA and AAF gains in V/V */ +static const int adaq7768_gains[7 ] = { + [AD7768_PGA_GAIN_0 ] =325 ,/* 0.325 */ + [AD7768_PGA_GAIN_1 ] =650 ,/* 0.650 */ + [AD7768_PGA_GAIN_2 ] =1300 ,/* 1.300 */ + [AD7768_PGA_GAIN_3 ] =2600 ,/* 2.600 */ + [AD7768_PGA_GAIN_4 ] =5200 ,/* 5.200 */ + [AD7768_PGA_GAIN_5 ] =10400 ,/* 10.400 */ + [AD7768_PGA_GAIN_6 ] =20800 /* 20.800 */ +}; + +static const int adaq7769_gains[8 ] = { + [AD7768_PGA_GAIN_0 ] =1000 ,/* 1.000 */ + [AD7768_PGA_GAIN_1 ] =2000 ,/* 2.000 */ + [AD7768_PGA_GAIN_2 ] =4000 ,/* 4.000 */ + [AD7768_PGA_GAIN_3 ] =8000 ,/* 8.000 */ + [AD7768_PGA_GAIN_4 ] =16000 ,/* 16.000 */ + [AD7768_PGA_GAIN_5 ] =32000 ,/* 32.000 */ + [AD7768_PGA_GAIN_6 ] =64000 ,/* 64.000 */ + [AD7768_PGA_GAIN_7 ] =128000 /* 128.000 */ +}; + +static const int ad7768_aaf_gains[3 ] = { + [AD7768_AAF_IN1 ] =1000 ,/* 1.000 */ + [AD7768_AAF_IN2 ] =364 ,/* 0.364 */ + [AD7768_AAF_IN3 ] =143 /* 0.143 */ +}; +/* -3dB cutoff frequency multipliers (relative to ODR) for each filter type. */ static const int ad7768_filter_3db_odr_multiplier[] = {
[AD7768_FILTER_SINC5] = 204, /* 0.204 */drivers/iio/adc/ad7768-1.c +0
-0@@ -216,6 +269,12 @@ static const struct iio_scan_type ad7768_scan_type[] = {
structad7768_chip_info{
const char *name;drivers/iio/adc/ad7768-1.c +6
-0+bool has_variable_aaf; +bool has_pga; +int num_pga_modes; +int default_pga_mode; +int pgia_mode2pin_offset; +const int *pga_gains;const struct
iio_chan_spec*channel_spec;
const unsigned long*available_masks;
intnum_channels;drivers/iio/adc/ad7768-1.c +3
-0@@ -236,6 +295,9 @@ struct ad7768_state { unsigned int samp_freq; unsigned int samp_freq_avail[ARRAY_SIZE(ad7768_mclk_div_rates)]; unsigned int samp_freq_avail_len; +int pga_gain_mode; +int aaf_gain; +int scale_tbl[ADAQ776X_MAX_GAIN_MODES ][2 ]; struct completion completion; struct iio_trigger *trig; struct gpio_desc *gpio_sync_in;@@ -466,6 +528,43 @@ static int ad7768_reg_access(struct iio_dev *indio_dev, return ret; }drivers/iio/adc/ad7768-1.c +24
-0+static void ad7768_fill_scale_tbl (struct iio_dev *dev) +{ +struct ad7768_state *st =iio_priv (dev); +const struct iio_scan_type *scan_type; +int val, val2, tmp0, tmp1, i; +unsigned long denominator, numerator; +u64 tmp2; + + scan_type =iio_get_current_scan_type (dev, &dev->channels[0 ]); +if (scan_type->sign =='s' ) + val2 = scan_type->realbits -1 ; +else + val2 = scan_type->realbits; + +for (i =0 ; i < st->chip->num_pga_modes; i++) { +/* Convert gain to a fraction format */ + numerator = st->chip->pga_gains[i]; + denominator =MILLI ; +if (st->chip->has_variable_aaf) { + numerator *= ad7768_aaf_gains[st->aaf_gain]; + denominator *=MILLI ; + } + + rational_best_approximation(numerator, denominator, __INT_MAX__,
__INT_MAX__,
drivers/iio/adc/ad7768-1.c
Quoted 2 lines
drivers/iio/adc/ad7768-1.c +2
-0+ &numerator, &denominator); +
Hmm, I would user kernel limits.h (INT_MAX)
Quoted 31 lines
drivers/iio/adc/ad7768-1.c +4
-0+ val = st->vref_uv /1000 ; +/* Multiply by MILLI here to avoid losing precision */ + val =mult_frac (val, denominator *MILLI , numerator); +/* Would multiply by NANO here but we already multiplied by MILLI */
drivers/iio/adc/ad7768-1.c +7
-0+ tmp2 =shift_right ((u64 )val *MICRO , val2); + tmp0 = (int )div_s64_rem (tmp2,NANO , &tmp1); + st->scale_tbl[i][0 ] = tmp0;/* Integer part */ + st->scale_tbl[i][1 ] =abs (tmp1);/* Fractional part */ + } +} +static int
ad7768_set_sinc3_dec_rate(struct ad7768_state *st, unsigned int dec_rate){drivers/iio/adc/ad7768-1.c +0
-0@@ -567,12 +666,68 @@ static int ad7768_configure_dig_fil(struct iio_dev *dev, st->oversampling_ratio = ad7768_dec_rate_values[dec_rate_idx]; }drivers/iio/adc/ad7768-1.c +3
-0+/* Update scale table: scale values vary according to the precision */ +ad7768_fill_scale_tbl (dev); +
ad7768_fill_samp_freq_tbl(st);
/* A sync-in pulse is required after every configuration change */ returnad7768_send_sync_pulse(st);
}drivers/iio/adc/ad7768-1.c +3
-0+static int ad7768_calc_pga_gain (struct ad7768_state *st,int gain_int, +int gain_fract,int precision) +{
drivers/iio/adc/ad7768-1.c static int ad7768_configure_dig_fil(struct iio_dev *dev, · line 690
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+u64 gain_nano, tmp; +int gain_idx; + + precision--; + gain_nano = gain_int *NANO + gain_fract; +if (gain_nano <0 || gain_nano >ADAQ776X_GAIN_MAX_NANO ) +return -EINVAL ; + + tmp =DIV_ROUND_CLOSEST_ULL (gain_nano << precision,NANO ); + gain_nano =DIV_ROUND_CLOSEST_ULL (st->vref_uv, tmp);
Does not look like we need u64 divison above...
Quoted 28 lines
drivers/iio/adc/ad7768-1.c +25
-0+if (st->chip->has_variable_aaf) +/* remove the AAF gain from the overall gain */ + gain_nano = DIV_ROUND_CLOSEST_ULL(gain_nano * MILLI, + ad7768_aaf_gains[st->aaf_gain]); + tmp = st->chip->num_pga_modes; + gain_idx =find_closest (gain_nano, st->chip->pga_gains, tmp); + +return gain_idx; +} + +static int ad7768_set_pga_gain (struct ad7768_state *st, + int gain_mode) +{ +int pgia_pins_value =abs (gain_mode - st->chip->pgia_mode2pin_offset); +int check_val; +int ret; + +/* Check GPIO control register */ + ret =regmap_read (st->regmap,AD7768_REG_GPIO_CONTROL , &check_val); +if (ret <0 ) +return ret; + +if ((check_val &AD7768_GPIO_PGIA_EN ) !=AD7768_GPIO_PGIA_EN ) { +/* Enable PGIA GPIOs and set them as output */ + ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL,
AD7768_GPIO_PGIA_EN);drivers/iio/adc/ad7768-1.c +2
-0+if (ret <0 ) +return ret;
drivers/iio/adc/ad7768-1.c static int ad7768_configure_dig_fil(struct iio_dev *dev, · line 727
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+ } + +/* Write the respective gain values to GPIOs 0, 1, 2 */ + ret =regmap_write (st->regmap,AD7768_REG_GPIO_WRITE , + AD7768_GPIO_WRITE(pgia_pins_value)); +if (ret <0 ) +return ret; + + st->pga_gain_mode = gain_mode; +
It looks the above function could use some locking.
Quoted 49 lines
drivers/iio/adc/ad7768-1.c +3
-0+return 0 ; +} +static int
ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
{
structiio_dev*indio_dev=gpiochip_get_data(chip);drivers/iio/adc/ad7768-1.c +0
-0@@ -782,13 +937,17 @@ static const struct iio_chan_spec ad7768_channels[] = { AD7768_CHAN(0, AD7768_CHAN_INFO_NONE), };drivers/iio/adc/ad7768-1.c +4
-0+static const struct iio_chan_spec adaq776x_channels[] = { +AD7768_CHAN (0 ,BIT (IIO_CHAN_INFO_SCALE )), +}; +static int
ad7768_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long info){
structad7768_state*st =iio_priv(indio_dev); const structiio_scan_type*scan_type;drivers/iio/adc/ad7768-1.c +1
-1-int scale_uv, ret, temp; +int ret, temp;
scan_type=iio_get_current_scan_type(indio_dev, chan); if (IS_ERR(scan_type))drivers/iio/adc/ad7768-1.c +0
-0@@ -809,12 +968,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, return IIO_VAL_INT;
caseIIO_CHAN_INFO_SCALE:drivers/iio/adc/ad7768-1.c +13
-6- scale_uv = st->vref_uv; -if (scale_uv <0 ) -return scale_uv; - - *val = (scale_uv *2 ) /1000 ; - *val2 = scan_type->realbits; +if (st->chip->has_pga) { + *val = st->scale_tbl[st->pga_gain_mode][0 ]; + *val2 = st->scale_tbl[st->pga_gain_mode][1 ]; +return IIO_VAL_INT_PLUS_NANO ; + } + *val = st->vref_uv /1000 ; +if (st->chip->has_variable_aaf) + *val = (*val *MILLI ) / ad7768_aaf_gains[st->aaf_gain]; +/* +* ADC output code is two's complement so only (realbits - 1) +* bits express voltage magnitude. +*/ + *val2 = scan_type->realbits -1 ;
Quoted 1 line
I'm a bit confused. Is there something wrong with the original code that needs to be fixed? The above change seems to effectively change behavior of the code with had before.
- Nuno Sá
Quoted 181 lines
return
IIO_VAL_FRACTIONAL_LOG2;drivers/iio/adc/ad7768-1.c +5
-0@@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, *length = st->samp_freq_avail_len; *type = IIO_VAL_INT; return IIO_AVAIL_LIST; +case IIO_CHAN_INFO_SCALE : + *vals = (int *)st->scale_tbl; + *length = st->chip->num_pga_modes *2 ; + *type =IIO_VAL_INT_PLUS_NANO ; +return IIO_AVAIL_LIST ;default:
return-EINVAL;
}
}drivers/iio/adc/ad7768-1.c +13
-0+static int ad7768_write_raw_get_fmt (struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, long mask) +{ +switch (mask) { +case IIO_CHAN_INFO_SCALE : +return IIO_VAL_INT_PLUS_NANO ; +default : +return IIO_VAL_INT_PLUS_MICRO ; + } + +return -EINVAL ; +} +static int
__ad7768_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int val, int val2, long info){
structad7768_state*st =iio_priv(indio_dev);drivers/iio/adc/ad7768-1.c +2
-0+const struct iio_scan_type *scan_type; +int gain_mode;int ret;
drivers/iio/adc/ad7768-1.c +4
-0+ scan_type =iio_get_current_scan_type (indio_dev, chan); +if (IS_ERR (scan_type)) +return PTR_ERR (scan_type); +switch (info) {
caseIIO_CHAN_INFO_SAMP_FREQ:
returnad7768_set_freq(st, val);drivers/iio/adc/ad7768-1.c +0
-0@@ -892,6 +1082,13 @@ static int __ad7768_write_raw(struct iio_dev *indio_dev,
/* Update sampling frequency */ returnad7768_set_freq(st, st->samp_freq);drivers/iio/adc/ad7768-1.c +7
-0+case IIO_CHAN_INFO_SCALE : +if (!st->chip->has_pga) +return -EOPNOTSUPP ; + + gain_mode =ad7768_calc_pga_gain (st, val, val2, + scan_type->realbits); +return ad7768_set_pga_gain (st, gain_mode);default:
return-EINVAL;
}drivers/iio/adc/ad7768-1.c +1
-0@@ -933,6 +1130,7 @@ static const struct iio_info ad7768_info = { .read_raw = &ad7768_read_raw, .read_avail = &ad7768_read_avail, .write_raw = &ad7768_write_raw, + .write_raw_get_fmt = &ad7768_write_raw_get_fmt, .read_label = ad7768_read_label, .get_current_scan_type = &ad7768_get_current_scan_type, .debugfs_reg_access = &ad7768_reg_access,@@ -1351,10 +1549,46 @@ static const struct ad7768_chip_info ad7768_chip_info = { .available_masks = ad7768_channel_masks, };drivers/iio/adc/ad7768-1.c +35
-0+static const struct ad7768_chip_info adaq7767_chip_info = { + .name ="adaq7767-1" , + .channel_spec = ad7768_channels, + .num_channels =ARRAY_SIZE (ad7768_channels), + .available_masks = ad7768_channel_masks, + .has_pga = false, + .has_variable_aaf = true +}; + +static const struct ad7768_chip_info adaq7768_chip_info = { + .name ="adaq7768-1" , + .channel_spec = adaq776x_channels, + .num_channels =ARRAY_SIZE (adaq776x_channels), + .available_masks = ad7768_channel_masks, + .pga_gains = adaq7768_gains, + .default_pga_mode =AD7768_PGA_GAIN_2 , + .num_pga_modes =ARRAY_SIZE (adaq7768_gains), + .pgia_mode2pin_offset =6 , + .has_pga = true, + .has_variable_aaf = false +}; + +static const struct ad7768_chip_info adaq7769_chip_info = { + .name ="adaq7769-1" , + .channel_spec = adaq776x_channels, + .num_channels =ARRAY_SIZE (adaq776x_channels), + .available_masks = ad7768_channel_masks, + .pga_gains = adaq7769_gains, + .default_pga_mode =AD7768_PGA_GAIN_0 , + .num_pga_modes =ARRAY_SIZE (adaq7769_gains), + .pgia_mode2pin_offset =0 , + .has_pga = true, + .has_variable_aaf = true +}; +static int
ad7768_probe(struct spi_device *spi)
{
structad7768_state*st;
structiio_dev*indio_dev;drivers/iio/adc/ad7768-1.c +1
-0+u32 val;int ret;
indio_dev=devm_iio_device_alloc(&spi->dev, sizeof(*st));drivers/iio/adc/ad7768-1.c +0
-0@@ -1418,6 +1652,35 @@ static int ad7768_probe(struct spi_device *spi) if (ret) return ret;drivers/iio/adc/ad7768-1.c +6
-0+ st->aaf_gain =AD7768_AAF_IN1 ; + ret =device_property_read_u32 (&spi->dev,"adi,aaf-gain" , &val); +if (ret) { +/* AAF gain required, but not specified */ +if (st->chip->has_variable_aaf) + return dev_err_probe(&spi->dev, -EINVAL, "AAF gain notspecified\n");
drivers/iio/adc/ad7768-1.c +4
-0+ + }else if (!st->chip->has_variable_aaf) { +/* AAF gain provided, but not supported */ + return dev_err_probe(&spi->dev, -EINVAL, "AAF gain not supportedfor %s\n",
drivers/iio/adc/ad7768-1.c +19
-0+ st->chip->name); + }else { +/* Device supports variable AAF gain, validate and set the gain */ +switch (val) { +case 1000 : + st->aaf_gain =AD7768_AAF_IN1 ; +break ; +case 364 : + st->aaf_gain =AD7768_AAF_IN2 ; +break ; +case 143 : + st->aaf_gain =AD7768_AAF_IN3 ; +break ; +default : +return dev_err_probe (&spi->dev, -EINVAL , + "Invalid firmware provided gain\n"); + } + } +ret =
ad7768_setup(indio_dev);
if (ret < 0) {
dev_err(&spi->dev, "AD7768 setup failed\n");drivers/iio/adc/ad7768-1.c +0
-0@@ -1426,6 +1689,9 @@ static int ad7768_probe(struct spi_device *spi)
init_completion(&st->completion);drivers/iio/adc/ad7768-1.c +3
-0+if (st->chip->has_pga) +ad7768_set_pga_gain (st, st->chip->default_pga_mode); +ret =
ad7768_set_channel_label(indio_dev, st->chip->num_channels); if (ret)
return ret;drivers/iio/adc/ad7768-1.c +0
-0@@ -1446,12 +1712,18 @@ static int ad7768_probe(struct spi_device *spi)
static const structspi_device_idad7768_id_table[] = { { "ad7768-1", (kernel_ulong_t)&ad7768_chip_info },drivers/iio/adc/ad7768-1.c +3
-0+ { "adaq7767-1", (kernel_ulong_t)&adaq7767_chip_info }, + { "adaq7768-1", (kernel_ulong_t)&adaq7768_chip_info }, + { "adaq7769-1", (kernel_ulong_t)&adaq7769_chip_info },{ }
};
MODULE_DEVICE_TABLE(spi,ad7768_id_table);
static const structof_device_idad7768_of_match[] = {
{ .compatible = "adi,ad7768-1", .data =&ad7768_chip_info},drivers/iio/adc/ad7768-1.c +3
-0+ { .compatible = "adi,adaq7767-1", .data = &adaq7767_chip_info }, + { .compatible = "adi,adaq7768-1", .data = &adaq7768_chip_info }, + { .compatible = "adi,adaq7769-1", .data = &adaq7769_chip_info },{ }
};
MODULE_DEVICE_TABLE(of,ad7768_of_match);
Re: [PATCH 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
Quoted 18 lines · Nuno Sá
Quoted 7 lines · Jonathan Santos
Add support for ADAQ7767/68/69-1 series, which includes PGIA and Anti-aliasing filter (AAF) gains.
The PGA gain is configured in run-time through the scale attribute, if supported by the device. The scale options are updated according to the output data width.
Quoted 5 lines
The AAF gain is configured in the devicetree and it should correspond to the AAF channel selected in hardware.
Signed-off-by Jonathan Santos <[email protected]>---
Hi Jonathan,
Some comments from me...
...
Quoted 86 lines
Quoted 18 lines
drivers/iio/adc/ad7768-1.c +15
-0+static int ad7768_set_pga_gain (struct ad7768_state *st, + int gain_mode) +{ +int pgia_pins_value =abs (gain_mode - st->chip->pgia_mode2pin_offset); +int check_val; +int ret; + +/* Check GPIO control register */ + ret =regmap_read (st->regmap,AD7768_REG_GPIO_CONTROL , &check_val); +if (ret <0 ) +return ret; + +if ((check_val &AD7768_GPIO_PGIA_EN ) !=AD7768_GPIO_PGIA_EN ) { +/* Enable PGIA GPIOs and set them as output */ + ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL,
AD7768_GPIO_PGIA_EN);drivers/iio/adc/ad7768-1.c +2
-0+if (ret <0 ) +return ret;drivers/iio/adc/ad7768-1.c static int ad7768_configure_dig_fil(struct iio_dev *dev, · line 727
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+ } + +/* Write the respective gain values to GPIOs 0, 1, 2 */ + ret =regmap_write (st->regmap,AD7768_REG_GPIO_WRITE , + AD7768_GPIO_WRITE(pgia_pins_value)); +if (ret <0 ) +return ret; + + st->pga_gain_mode = gain_mode; +It looks the above function could use some locking.
Quoted 49 lines
drivers/iio/adc/ad7768-1.c +3
-0+return 0 ; +} +static int
ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
{
structiio_dev*indio_dev=gpiochip_get_data(chip);drivers/iio/adc/ad7768-1.c +0
-0@@ -782,13 +937,17 @@ static const struct iio_chan_spec ad7768_channels[] = { AD7768_CHAN(0, AD7768_CHAN_INFO_NONE), };drivers/iio/adc/ad7768-1.c +4
-0+static const struct iio_chan_spec adaq776x_channels[] = { +AD7768_CHAN (0 ,BIT (IIO_CHAN_INFO_SCALE )), +}; +static int
ad7768_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long info){
structad7768_state*st =iio_priv(indio_dev); const structiio_scan_type*scan_type;drivers/iio/adc/ad7768-1.c +1
-1-int scale_uv, ret, temp; +int ret, temp;
scan_type=iio_get_current_scan_type(indio_dev, chan); if (IS_ERR(scan_type))drivers/iio/adc/ad7768-1.c +0
-0@@ -809,12 +968,19 @@ static int ad7768_read_raw(struct iio_dev *indio_dev, return IIO_VAL_INT;
caseIIO_CHAN_INFO_SCALE:drivers/iio/adc/ad7768-1.c +13
-6- scale_uv = st->vref_uv; -if (scale_uv <0 ) -return scale_uv; - - *val = (scale_uv *2 ) /1000 ; - *val2 = scan_type->realbits; +if (st->chip->has_pga) { + *val = st->scale_tbl[st->pga_gain_mode][0 ]; + *val2 = st->scale_tbl[st->pga_gain_mode][1 ]; +return IIO_VAL_INT_PLUS_NANO ; + } + *val = st->vref_uv /1000 ; +if (st->chip->has_variable_aaf) + *val = (*val *MILLI ) / ad7768_aaf_gains[st->aaf_gain]; +/* +* ADC output code is two's complement so only (realbits - 1) +* bits express voltage magnitude. +*/ + *val2 = scan_type->realbits -1 ;Quoted 1 line
I'm a bit confused. Is there something wrong with the original code that needs to be fixed? The above change seems to effectively change behavior of the code with had before.
Quoted 1 line
- Nuno Sá
I think it is more of a semantic clarification than a behavioral change per se. Previously, the calculation gave the impression of a bipolar unsigned input, since it was using the full scale. With the update, we’re explicitly considering the sign bit, which makes it clear that the input is a two’s complement signal.
Altough pehaps this should not be mixed into the patch.
Quoted 7 lines
Quoted 7 lines
return
IIO_VAL_FRACTIONAL_LOG2;drivers/iio/adc/ad7768-1.c +1
-0@@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, *length = st->samp_freq_avail_len; *type = IIO_VAL_INT; return IIO_AVAIL_LIST; +case IIO_CHAN_INFO_SCALE :
Quoted 8 lines
Quoted 8 lines
drivers/iio/adc/ad7768-1.c +4
-0+ *vals = (int *)st->scale_tbl; + *length = st->chip->num_pga_modes *2 ; + *type =IIO_VAL_INT_PLUS_NANO ; +return IIO_AVAIL_LIST ;default:
return-EINVAL;
}
}
...
- Jonathan Santos
Re: [PATCH 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
In reply to Jonathan SantosHi Jonathan,
A few thoughts from me.
Quoted 6 lines · Jonathan Santos
Add support for ADAQ7767/68/69-1 series, which includes PGIA and Anti-aliasing filter (AAF) gains.
The PGA gain is configured in run-time through the scale attribute, if supported by the device. The scale options are updated according to the output data width.
This could provide more information/context. How the PGA is controlled/configured?
Quoted 3 lines
The AAF gain is configured in the devicetree and it should correspond to the AAF channel selected in hardware.
Would this be more clear if stated the other way around?
The AAF gain is defined by hardware connections and thus is specified in device tree. ?
Quoted 6 lines
Signed-off-by Jonathan Santos <[email protected]>--- drivers/iio/adc/ad7768-1.c | 286 ++++++++++++++++++++++++++++++++++++- 1 file changed, 279 insertions(+), 7 deletions(-)
...
Quoted 18 lines
drivers/iio/adc/ad7768-1.c +17
-0+static void ad7768_fill_scale_tbl (struct iio_dev *dev) +{ +struct ad7768_state *st =iio_priv (dev); +const struct iio_scan_type *scan_type; +int val, val2, tmp0, tmp1, i; +unsigned long denominator, numerator; +u64 tmp2; + + scan_type =iio_get_current_scan_type (dev, &dev->channels[0 ]); +if (scan_type->sign =='s' ) + val2 = scan_type->realbits -1 ; +else + val2 = scan_type->realbits; + +for (i =0 ; i < st->chip->num_pga_modes; i++) { +/* Convert gain to a fraction format */ + numerator = st->chip->pga_gains[i];
drivers/iio/adc/ad7768-1.c static int ad7768_reg_access(struct iio_dev *indio_dev, · line 557
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+ denominator =MILLI ; +if (st->chip->has_variable_aaf) { + numerator *= ad7768_aaf_gains[st->aaf_gain]; + denominator *=MILLI ; + } + +rational_best_approximation (numerator, denominator, __INT_MAX__, __INT_MAX__, + &numerator, &denominator); + + val = st->vref_uv /1000 ;
What about keeping the reference in µV (not dividing by MILLI)?
drivers/iio/adc/ad7768-1.c static int ad7768_reg_access(struct iio_dev *indio_dev, · line 559
Quoted 2 lines
drivers/iio/adc/ad7768-1.c +2
-0+/* Multiply by MILLI here to avoid losing precision */ + val =mult_frac (val, denominator *MILLI , numerator);
then this can be just
val = mult_frac(val, denominator, numerator);
drivers/iio/adc/ad7768-1.c static int ad7768_reg_access(struct iio_dev *indio_dev, · line 562
Quoted 3 lines
drivers/iio/adc/ad7768-1.c +3
-0+/* Would multiply by NANO here but we already multiplied by MILLI */ + tmp2 =shift_right ((u64 )val *MICRO , val2); + tmp0 = (int )div_s64_rem (tmp2,NANO , &tmp1);
val is never negative here so can use div_u64_rem() or maybe do_div().
Quoted 5 lines
drivers/iio/adc/ad7768-1.c +5
-0+ st->scale_tbl[i][0 ] = tmp0;/* Integer part */ + st->scale_tbl[i][1 ] =abs (tmp1);/* Fractional part */ + } +} +
...
drivers/iio/adc/ad7768-1.c static int ad7768_configure_dig_fil(struct iio_dev *dev, · line 684
Quoted 8 lines
drivers/iio/adc/ad7768-1.c +7
-0+static int ad7768_calc_pga_gain (struct ad7768_state *st,int gain_int, +int gain_fract,int precision) +{ +u64 gain_nano, tmp; +int gain_idx; + + precision--;
This is odd out of context.
Also, it only applies to ADCs that provide output codes in two's complement format. See comment below.
drivers/iio/adc/ad7768-1.c static int ad7768_configure_dig_fil(struct iio_dev *dev, · line 686
Quoted 2 lines
drivers/iio/adc/ad7768-1.c +2
-0+ gain_nano = gain_int *NANO + gain_fract; +if (gain_nano <0 || gain_nano >ADAQ776X_GAIN_MAX_NANO )
I've seen some build tools complain about comparisons like gain_nano < 0 with gain_nano being u64. Since that's unsigned, it can never be < 0. And in the context of gain/attenuation, we know gain_nano shall never be negative. Would just drop the gain_nano < 0 comparison. Or maybe clamp() the value?
Quoted 14 lines
drivers/iio/adc/ad7768-1.c +14
-0+return -EINVAL ; + + tmp =DIV_ROUND_CLOSEST_ULL (gain_nano << precision,NANO ); + gain_nano =DIV_ROUND_CLOSEST_ULL (st->vref_uv, tmp); +if (st->chip->has_variable_aaf) +/* remove the AAF gain from the overall gain */ + gain_nano =DIV_ROUND_CLOSEST_ULL (gain_nano *MILLI , + ad7768_aaf_gains[st->aaf_gain]); + tmp = st->chip->num_pga_modes; + gain_idx =find_closest (gain_nano, st->chip->pga_gains, tmp); + +return gain_idx; +} +
...
drivers/iio/adc/ad7768-1.c static int ad7768_configure_dig_fil(struct iio_dev *dev, · line 723
Quoted 5 lines
drivers/iio/adc/ad7768-1.c +5
-0+ +/* Write the respective gain values to GPIOs 0, 1, 2 */ + ret =regmap_write (st->regmap,AD7768_REG_GPIO_WRITE , +AD7768_GPIO_WRITE (pgia_pins_value)); +if (ret <0 )
I think regmap_write() doesn't return positive values so we can have 'if (ret)' here.
Quoted 7 lines
drivers/iio/adc/ad7768-1.c +7
-0+return ret; + + st->pga_gain_mode = gain_mode; + +return 0 ; +} +
...
Quoted 5 lines
drivers/iio/adc/ad7768-1.c +0
-4case IIO_CHAN_INFO_SCALE : - scale_uv = st->vref_uv; -if (scale_uv <0 ) -return scale_uv; -
drivers/iio/adc/ad7768-1.c static int ad7768_read_raw(struct iio_dev *indio_dev, · line 978
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +8
-2- *val = (scale_uv *2 ) /1000 ; - *val2 = scan_type->realbits; +if (st->chip->has_pga) { + *val = st->scale_tbl[st->pga_gain_mode][0 ]; + *val2 = st->scale_tbl[st->pga_gain_mode][1 ]; +return IIO_VAL_INT_PLUS_NANO ; + } + *val = st->vref_uv /1000 ; +if (st->chip->has_variable_aaf) + *val = (*val *MILLI ) / ad7768_aaf_gains[st->aaf_gain];
Similar thing here. Would it make sense to use st->vref_uv without dividing it by MILLI?
drivers/iio/adc/ad7768-1.c static int ad7768_read_raw(struct iio_dev *indio_dev, · line 983
Quoted 5 lines
drivers/iio/adc/ad7768-1.c +5
-0+/* +* ADC output code is two's complement so only (realbits - 1) +* bits express voltage magnitude. +*/ + *val2 = scan_type->realbits -1 ;
I see the rationally for this. Instead of doing 'scale_uv * 2' to account for the range of negative values, this is now using one less precision bit which shall lead to the same result after going through IIO_VAL_FRACTIONAL_LOG2 handling. I personally prefer the realbits - 1 logic, but others may prefer to avoid this change since it was already working with 'scale_uv * 2'.
Quoted 17 lines
drivers/iio/adc/ad7768-1.c +5
-0return IIO_VAL_FRACTIONAL_LOG2 ;@@ -869,18 +1035,42 @@ static int ad7768_read_avail(struct iio_dev *indio_dev, *length = st->samp_freq_avail_len; *type =IIO_VAL_INT ;return IIO_AVAIL_LIST ; +case IIO_CHAN_INFO_SCALE : + *vals = (int *)st->scale_tbl; + *length = st->chip->num_pga_modes *2 ; + *type =IIO_VAL_INT_PLUS_NANO ; +return IIO_AVAIL_LIST ;default :return -EINVAL ; } }
...
drivers/iio/adc/ad7768-1.c static int __ad7768_write_raw(struct iio_dev *indio_dev, · line 1090
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +6
-0@@ -892,6 +1082,13 @@ static int __ad7768_write_raw(struct iio_dev *indio_dev,/* Update sampling frequency */ return ad7768_set_freq (st, st->samp_freq); +case IIO_CHAN_INFO_SCALE : +if (!st->chip->has_pga) +return -EOPNOTSUPP ; + + gain_mode =ad7768_calc_pga_gain (st, val, val2, + scan_type->realbits);
Check scan_type.sign and pass scan_type->realbits - 1 to ad7768_calc_pga_gain() if the ADC output codes are in two's complement format.
Quoted 4 lines
drivers/iio/adc/ad7768-1.c +1
-0+return ad7768_set_pga_gain (st, gain_mode);default :return -EINVAL ; }
...
drivers/iio/adc/ad7768-1.c static const struct ad7768_chip_info ad7768_chip_info = { · line 1557
Quoted 6 lines
drivers/iio/adc/ad7768-1.c +6
-0+static const struct ad7768_chip_info adaq7767_chip_info = { + .name ="adaq7767-1" , + .channel_spec = ad7768_channels, + .num_channels =ARRAY_SIZE (ad7768_channels), + .available_masks = ad7768_channel_masks, + .has_pga = false,
I think these flag initialization can be omitted when they are false.
Quoted 4 lines
drivers/iio/adc/ad7768-1.c +4
-0+ .has_variable_aaf = true +}; + +static const struct ad7768_chip_info adaq7768_chip_info = {
drivers/iio/adc/ad7768-1.c static const struct ad7768_chip_info ad7768_chip_info = { · line 1571
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+ .name = "adaq7768-1", + .channel_spec = adaq776x_channels, + .num_channels = ARRAY_SIZE(adaq776x_channels), + .available_masks = ad7768_channel_masks, + .pga_gains = adaq7768_gains, + .default_pga_mode = AD7768_PGA_GAIN_2, + .num_pga_modes = ARRAY_SIZE(adaq7768_gains), + .pgia_mode2pin_offset = 6, + .has_pga = true, + .has_variable_aaf = false
Same here.
Quoted 2 lines
(quoted diff) +2
-0+}; +
...
Quoted 5 lines
drivers/iio/adc/ad7768-1.c +1
-0@@ -1418,6 +1652,35 @@ static int ad7768_probe(struct spi_device *spi)if (ret)return ret; + st->aaf_gain =AD7768_AAF_IN1 ;
drivers/iio/adc/ad7768-1.c static int ad7768_probe(struct spi_device *spi) · line 1665
Quoted 10 lines
drivers/iio/adc/ad7768-1.c +10
-0+ ret =device_property_read_u32 (&spi->dev,"adi,aaf-gain" , &val); +if (ret) { +/* AAF gain required, but not specified */ +if (st->chip->has_variable_aaf) +return dev_err_probe (&spi->dev, -EINVAL ,"AAF gain not specified\n" ); + + }else if (!st->chip->has_variable_aaf) { +/* AAF gain provided, but not supported */ +return dev_err_probe (&spi->dev, -EINVAL ,"AAF gain not supported for %s\n" , + st->chip->name);
Not really sure what to do in these cases. Can't we just ignore or warn on properties for unsupported features?
Best regards,
Marcelo
Re: [PATCH 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
Quoted 2 lines · Marcelo Schmitt
Quoted 1 line · Jonathan Santos
Add support for ADAQ7767/68/69-1 series, which includes PGIA and
...
Quoted 12 lines
drivers/iio/adc/ad7768-1.c static int ad7768_configure_dig_fil(struct iio_dev *dev, · line 684
Quoted 7 lines
drivers/iio/adc/ad7768-1.c +7
-0+static int ad7768_calc_pga_gain (struct ad7768_state *st,int gain_int, +int gain_fract,int precision) +{ +u64 gain_nano, tmp; +int gain_idx; + + precision--;This is odd out of context.
Also, it only applies to ADCs that provide output codes in two's complement format. See comment below.
Quoted 6 lines
drivers/iio/adc/ad7768-1.c static int ad7768_configure_dig_fil(struct iio_dev *dev, · line 686
Quoted 2 lines
drivers/iio/adc/ad7768-1.c +2
-0+ gain_nano = gain_int *NANO + gain_fract; +if (gain_nano <0 || gain_nano >ADAQ776X_GAIN_MAX_NANO )I've seen some build tools complain about comparisons like
gain_nano< 0 withgain_nanobeing u64. Since that's unsigned, it can never be < 0. And in the context of gain/attenuation, we knowgain_nanoshall never be negative. Would just drop thegain_nano< 0 comparison. Or maybeclamp()the value?
in_range() can be used as well.
Quoted 13 lines
Quoted 13 lines
drivers/iio/adc/ad7768-1.c +13
-0+return -EINVAL ; + + tmp =DIV_ROUND_CLOSEST_ULL (gain_nano << precision,NANO ); + gain_nano =DIV_ROUND_CLOSEST_ULL (st->vref_uv, tmp); +if (st->chip->has_variable_aaf) +/* remove the AAF gain from the overall gain */ + gain_nano =DIV_ROUND_CLOSEST_ULL (gain_nano *MILLI , + ad7768_aaf_gains[st->aaf_gain]); + tmp = st->chip->num_pga_modes; + gain_idx =find_closest (gain_nano, st->chip->pga_gains, tmp); + +return gain_idx; +}
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 12 lines · Jonathan Santos
This adds support for the ADAQ7767-1, ADAQ7768-1 and ADAQ7769-1 devices.
The ADAQ7768-1 and ADAQ7769-1 integrate a programmable gain amplifier (PGA) with 7 and 8 gain options, respectively. The ADAQ7767-1 and ADAQ7769-1 also feature a 3-pin selectable Anti-aliasing filter (AAF) gain.
Jonathan Santos (4):
dt-bindings: iio: adc: ad7768-1: add new supported parts
iio: adc: ad7768-1: introduce chip info for future multidevice support
iio: adc: ad7768-1: usedevm_regulator_get_enable_read_voltage
iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family
Quoted 6 lines
.../bindings/iio/adc/adi,ad7768-1.yaml | 48 ++- drivers/iio/adc/ad7768-1.c | 389 +++++++++++++++---
2 files changed, 382 insertions(+), 55 deletions(-)base-commit:
0a686b9c4f847dc21346df8e56d5b119918fefef
With Andy's feedback fixed: