* [PATCH 0/3] iio: adc: ad4695: new driver for AD4695 and similar ADCs
@ 2024-06-12 19:20 David Lechner
2024-06-12 19:20 ` [PATCH 1/3] dt-bindings: iio: adc: add " David Lechner
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: David Lechner @ 2024-06-12 19:20 UTC (permalink / raw)
To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: David Lechner, Michael Hennerich, Nuno Sá, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, linux-doc, Ramona Gradinariu
This is adding DT bindings and a new driver for the AD4695 and similar
devices. The plan is to implement quite a few more features, but this
is a complex chip so we're spreading out the work. To start with, we
have a reasonably complete DT binding and a very basic driver.
This work is being done in collaboration with Analog Devices Inc.,
hence they listed as maintainers rather than me. The code has been
tested on a Zedboard with an EVAL-AD4696FMCZ using the internal LDO,
an external reference and a variety of input channel configurations.
---
David Lechner (3):
dt-bindings: iio: adc: add AD4695 and similar ADCs
iio: adc: ad4695: Add driver for AD4695 and similar ADCs
Documentation: iio: Document ad4695 driver
.../devicetree/bindings/iio/adc/adi,ad4695.yaml | 297 ++++++++
Documentation/iio/ad4695.rst | 145 ++++
Documentation/iio/index.rst | 1 +
MAINTAINERS | 11 +
drivers/iio/adc/Kconfig | 11 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4695.c | 804 +++++++++++++++++++++
7 files changed, 1270 insertions(+)
---
base-commit: cc1ce839526a65620778617da0b022bd88e8a139
change-id: 20240517-iio-adc-ad4695-ef72b2a2cf88
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-12 19:20 [PATCH 0/3] iio: adc: ad4695: new driver for AD4695 and similar ADCs David Lechner @ 2024-06-12 19:20 ` David Lechner 2024-06-13 7:11 ` Krzysztof Kozlowski 2024-06-15 12:41 ` Jonathan Cameron 2024-06-12 19:20 ` [PATCH 2/3] iio: adc: ad4695: Add driver for " David Lechner 2024-06-12 19:20 ` [PATCH 3/3] Documentation: iio: Document ad4695 driver David Lechner 2 siblings, 2 replies; 18+ messages in thread From: David Lechner @ 2024-06-12 19:20 UTC (permalink / raw) To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: David Lechner, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc Add device tree bindings for AD4695 and similar ADCs. Signed-off-by: David Lechner <dlechner@baylibre.com> --- .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 297 +++++++++++++++++++++ MAINTAINERS | 9 + 2 files changed, 306 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml new file mode 100644 index 000000000000..8ff5bbbbef9f --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml @@ -0,0 +1,297 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ad4695.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices Easy Drive Multiplexed SAR Analog to Digital Converters + +maintainers: + - Michael Hennerich <Michael.Hennerich@analog.com> + - Nuno Sá <nuno.sa@analog.com> + +description: | + A family of similar multi-channel analog to digital converters with SPI bus. + + * https://www.analog.com/en/products/ad4695.html + * https://www.analog.com/en/products/ad4696.html + * https://www.analog.com/en/products/ad4697.html + * https://www.analog.com/en/products/ad4698.html + +$ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + oneOf: + - enum: + - adi,ad4695 + - adi,ad4697 + # same chips in WLCSP package with more pins + - items: + - const: adi,ad4695-wlcsp + - const: adi,ad4695 + - items: + - const: adi,ad4697-wlcsp + - const: adi,ad4697 + # same chips with higher max sample rate + - items: + - const: adi,ad4696 + - const: adi,ad4695 + - items: + - const: adi,ad4698 + - const: adi,ad4697 + # same chips with higher max sample rate in WLCSP package + - items: + - const: adi,ad4696-wlcsp + - const: adi,ad4696 + - const: adi,ad4695-wlcsp + - const: adi,ad4695 + - items: + - const: adi,ad4698-wlcsp + - const: adi,ad4698 + - const: adi,ad4697-wlcsp + - const: adi,ad4697 + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 80000000 + + spi-cpol: true + spi-cpha: true + + spi-rx-bus-width: + minimum: 1 + maximum: 4 + + avdd-supply: + description: A 2.7 V to 5.5 V supply that powers the analog circuitry. + + ldo-in-supply: + description: A 2.4 V to 5.5 V supply connected to the internal LDO input. + + vdd-supply: + description: A 1.8V supply that powers the core circuitry. + + vio-supply: + description: A 1.2V to 1.8V supply for the digital inputs and outputs. + + ref-supply: + description: A 2.4 V to 5.1 V supply for the external reference voltage. + + refin-supply: + description: A 2.4 V to 5.1 V supply for the internal reference buffer input. + + com-supply: + description: Common voltage supply for pseudo-differential analog inputs. + + adi,no-ref-current-limit: + $ref: /schemas/types.yaml#/definitions/flag + description: + When this flag is present, the REF Overvoltage Reduced Current protection + is disabled. + + adi,no-ref-high-z: + $ref: /schemas/types.yaml#/definitions/flag + description: + Enable this flag if the ref-supply requires Reference Input High-Z Mode + to be disabled for proper operation. + + cnv-gpios: + description: The Convert Input (CNV). If omitted, CNV is tied to SPI CS. + maxItems: 1 + + reset-gpios: + description: The Reset Input (RESET). Should be configured GPIO_ACTIVE_LOW. + maxItems: 1 + + interrupts: + minItems: 1 + items: + - description: + Signal coming from the BSY_ALT_GP0 or GP3 pin that indicates a busy + condition. + - description: + Signal coming from the BSY_ALT_GP0 or GP2 pin that indicates an alert + condition. + + interrupt-names: + minItems: 1 + items: + - const: busy + - const: alert + + gpio-controller: true + + "#gpio-cells": + const: 2 + description: | + The first cell is the GPn number: 0 to 3. + The second cell takes standard GPIO flags. + + "#address-cells": + const: 1 + "#size-cells": + const: 0 + +patternProperties: + "^channel@[0-9a-f]$": + type: object + $ref: adc.yaml + unevaluatedProperties: false + description: + Describes each individual channel. In addition the properties defined + below, bipolar from adc.yaml is also supported. + + properties: + reg: + maximum: 15 + description: Input pin number (INx). + + adi,pin-pairing: + description: | + The input pin pairing for the negative input. This can be: + - REFGND, normally 0V (single-ended) + - COM, normally V_REF/2, see com-supply (pseudo-differential) + - For even numbered pins, the next odd numbered pin (differential) + $ref: /schemas/types.yaml#/definitions/string + enum: [refgnd, com, next] + default: refgnd + + adi,no-high-z: + $ref: /schemas/types.yaml#/definitions/flag + description: | + Enable this flag if the input pin requires the Analog Input High-Z + Mode to be disabled for proper operation. + + required: + - reg + + allOf: + # only even number pins can be paired with the next pin + - if: + properties: + reg: + not: + multipleOf: 2 + then: + properties: + adi,pin-pairing: + enum: [refgnd, com] + # bipolar mode is not supported with REFGND pairing + - if: + not: + required: + - adi,pin-pairing + then: + properties: + bipolar: false + else: + if: + properties: + adi,pin-pairing: + const: refgnd + then: + properties: + bipolar: false + +required: + - compatible + - reg + - avdd-supply + - vio-supply + +allOf: + - oneOf: + - required: + - ref-supply + - required: + - refin-supply + + - oneOf: + - required: + - ldo-in-supply + - required: + - vdd-supply + + # LFSCP package has fewer pins, so a few things are not valid in that case + - if: + properties: + compatible: + not: + contains: + pattern: -wlcsp$ + then: + properties: + refin-supply: false + spi-rx-bus-width: + maximum: 2 + + # the internal reference buffer always requires high-z mode + - if: + required: + - refin-supply + then: + properties: + adi,no-ref-high-z: false + + # limit channels for 8-channel chips + - if: + properties: + compatible: + contains: + const: adi,ad4697 + then: + patternProperties: + "^channel@[0-7]$": + properties: + reg: + maximum: 7 + "^channel@[8-9a-f]$": false + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + adc@0 { + compatible = "adi,ad4695"; + reg = <0>; + spi-cpol; + spi-cpha; + spi-max-frequency = <80000000>; + avdd-supply = <&supply_2_5V>; + vdd-supply = <&supply_1_8V>; + vio-supply = <&supply_1_2V>; + ref-supply = <&supply_5V>; + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; + + #address-cells = <1>; + #size-cells = <0>; + + /* Differential channel between IN0 and IN1. */ + channel@0 { + reg = <0>; + adi,pin-pairing = "next"; + bipolar; + }; + + /* Single-ended channel between IN2 and REFGND. */ + channel@2 { + reg = <2>; + }; + + /* Pseudo-differential channel between IN3 and COM. */ + channel@f { + reg = <3>; + adi,pin-pairing = "com"; + bipolar; + }; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index c870bc6b8d63..8d15e4089d7c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1209,6 +1209,15 @@ F: Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130 F: Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml F: drivers/iio/adc/ad4130.c +ANALOG DEVICES INC AD4695 DRIVER +M: Michael Hennerich <michael.hennerich@analog.com> +M: Nuno Sá <nuno.sa@analog.com> +R: David Lechner <dlechner@baylibre.com> +L: linux-iio@vger.kernel.org +S: Supported +W: https://ez.analog.com/linux-software-drivers +F: Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml + ANALOG DEVICES INC AD7091R DRIVER M: Marcelo Schmitt <marcelo.schmitt@analog.com> L: linux-iio@vger.kernel.org -- 2.45.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-12 19:20 ` [PATCH 1/3] dt-bindings: iio: adc: add " David Lechner @ 2024-06-13 7:11 ` Krzysztof Kozlowski 2024-06-13 13:57 ` David Lechner 2024-06-15 12:41 ` Jonathan Cameron 1 sibling, 1 reply; 18+ messages in thread From: Krzysztof Kozlowski @ 2024-06-13 7:11 UTC (permalink / raw) To: David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On 12/06/2024 21:20, David Lechner wrote: > Add device tree bindings for AD4695 and similar ADCs. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 297 +++++++++++++++++++++ > MAINTAINERS | 9 + > 2 files changed, 306 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml > new file mode 100644 > index 000000000000..8ff5bbbbef9f > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml > @@ -0,0 +1,297 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/adi,ad4695.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices Easy Drive Multiplexed SAR Analog to Digital Converters > + > +maintainers: > + - Michael Hennerich <Michael.Hennerich@analog.com> > + - Nuno Sá <nuno.sa@analog.com> > + > +description: | > + A family of similar multi-channel analog to digital converters with SPI bus. > + > + * https://www.analog.com/en/products/ad4695.html > + * https://www.analog.com/en/products/ad4696.html > + * https://www.analog.com/en/products/ad4697.html > + * https://www.analog.com/en/products/ad4698.html > + > +$ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + oneOf: > + - enum: > + - adi,ad4695 > + - adi,ad4697 > + # same chips in WLCSP package with more pins > + - items: > + - const: adi,ad4695-wlcsp Usually we do not add compatibles for such differences. Programming model is the same. Same for all other wlcsp. Unless something differs? > + - const: adi,ad4695 > + - items: > + - const: adi,ad4697-wlcsp > + - const: adi,ad4697 > + # same chips with higher max sample rate > + - items: > + - const: adi,ad4696 Anyway, keep all fallbacked variants in one entry, so enum with const fallback. > + - const: adi,ad4695 > + - items: > + - const: adi,ad4698 > + - const: adi,ad4697 > + # same chips with higher max sample rate in WLCSP package > + - items: > + - const: adi,ad4696-wlcsp > + - const: adi,ad4696 That's wrong. ad4696 is compatible with 4695 as stated before. It is not compatible with ad4695-wlcsp. > + - const: adi,ad4695-wlcsp > + - const: adi,ad4695 > + - items: > + - const: adi,ad4698-wlcsp > + - const: adi,ad4698 > + - const: adi,ad4697-wlcsp > + - const: adi,ad4697 > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 80000000 > + > + spi-cpol: true > + spi-cpha: true > + > + spi-rx-bus-width: > + minimum: 1 > + maximum: 4 > + > + avdd-supply: > + description: A 2.7 V to 5.5 V supply that powers the analog circuitry. > + > + ldo-in-supply: > + description: A 2.4 V to 5.5 V supply connected to the internal LDO input. > + > + vdd-supply: > + description: A 1.8V supply that powers the core circuitry. > + > + vio-supply: > + description: A 1.2V to 1.8V supply for the digital inputs and outputs. > + > + ref-supply: > + description: A 2.4 V to 5.1 V supply for the external reference voltage. > + > + refin-supply: > + description: A 2.4 V to 5.1 V supply for the internal reference buffer input. > + > + com-supply: > + description: Common voltage supply for pseudo-differential analog inputs. > + > + adi,no-ref-current-limit: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + When this flag is present, the REF Overvoltage Reduced Current protection > + is disabled. > + > + adi,no-ref-high-z: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + Enable this flag if the ref-supply requires Reference Input High-Z Mode > + to be disabled for proper operation. > + > + cnv-gpios: > + description: The Convert Input (CNV). If omitted, CNV is tied to SPI CS. > + maxItems: 1 > + > + reset-gpios: > + description: The Reset Input (RESET). Should be configured GPIO_ACTIVE_LOW. > + maxItems: 1 > + > + interrupts: > + minItems: 1 > + items: > + - description: > + Signal coming from the BSY_ALT_GP0 or GP3 pin that indicates a busy > + condition. > + - description: > + Signal coming from the BSY_ALT_GP0 or GP2 pin that indicates an alert > + condition. > + > + interrupt-names: > + minItems: 1 > + items: > + - const: busy > + - const: alert > + > + gpio-controller: true > + > + "#gpio-cells": > + const: 2 > + description: | > + The first cell is the GPn number: 0 to 3. > + The second cell takes standard GPIO flags. > + > + "#address-cells": > + const: 1 Blank line > + "#size-cells": > + const: 0 > + Best regards, Krzysztof ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-13 7:11 ` Krzysztof Kozlowski @ 2024-06-13 13:57 ` David Lechner 2024-06-13 14:18 ` Krzysztof Kozlowski 0 siblings, 1 reply; 18+ messages in thread From: David Lechner @ 2024-06-13 13:57 UTC (permalink / raw) To: Krzysztof Kozlowski, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On 6/13/24 2:11 AM, Krzysztof Kozlowski wrote: > On 12/06/2024 21:20, David Lechner wrote: >> Add device tree bindings for AD4695 and similar ADCs. >> >> Signed-off-by: David Lechner <dlechner@baylibre.com> >> --- >> .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 297 +++++++++++++++++++++ >> MAINTAINERS | 9 + >> 2 files changed, 306 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml >> new file mode 100644 >> index 000000000000..8ff5bbbbef9f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml >> @@ -0,0 +1,297 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4695.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Analog Devices Easy Drive Multiplexed SAR Analog to Digital Converters >> + >> +maintainers: >> + - Michael Hennerich <Michael.Hennerich@analog.com> >> + - Nuno Sá <nuno.sa@analog.com> >> + >> +description: | >> + A family of similar multi-channel analog to digital converters with SPI bus. >> + >> + * https://www.analog.com/en/products/ad4695.html >> + * https://www.analog.com/en/products/ad4696.html >> + * https://www.analog.com/en/products/ad4697.html >> + * https://www.analog.com/en/products/ad4698.html >> + >> +$ref: /schemas/spi/spi-peripheral-props.yaml# >> + >> +properties: >> + compatible: >> + oneOf: >> + - enum: >> + - adi,ad4695 >> + - adi,ad4697 >> + # same chips in WLCSP package with more pins >> + - items: >> + - const: adi,ad4695-wlcsp > > Usually we do not add compatibles for such differences. Programming > model is the same. Same for all other wlcsp. Unless something differs? OK, I will drop all of the -wlcsp stuff. It made the .dts validation catch more stuff, but wasn't being used in the driver because there really isn't a difference other than some of the pins not being there. > >> + - const: adi,ad4695 >> + - items: >> + - const: adi,ad4697-wlcsp >> + - const: adi,ad4697 >> + # same chips with higher max sample rate I suppose one could make the argument that the programming model is the same on these too, but the maximum sampling frequency does seem like an important bit of information so that you don't try to set the conversion trigger rate too high. >> + - items: >> + - const: adi,ad4696 > > Anyway, keep all fallbacked variants in one entry, so enum with const > fallback. > >> + - const: adi,ad4695 >> + - items: >> + - const: adi,ad4698 >> + - const: adi,ad4697 >> + # same chips with higher max sample rate in WLCSP package >> + - items: >> + - const: adi,ad4696-wlcsp >> + - const: adi,ad4696 > > That's wrong. ad4696 is compatible with 4695 as stated before. It is not > compatible with ad4695-wlcsp. > >> + - const: adi,ad4695-wlcsp >> + - const: adi,ad4695 >> + - items: >> + - const: adi,ad4698-wlcsp >> + - const: adi,ad4698 >> + - const: adi,ad4697-wlcsp >> + - const: adi,ad4697 >> + ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-13 13:57 ` David Lechner @ 2024-06-13 14:18 ` Krzysztof Kozlowski 2024-06-13 14:39 ` David Lechner 0 siblings, 1 reply; 18+ messages in thread From: Krzysztof Kozlowski @ 2024-06-13 14:18 UTC (permalink / raw) To: David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On 13/06/2024 15:57, David Lechner wrote: > >> >>> + - const: adi,ad4695 >>> + - items: >>> + - const: adi,ad4697-wlcsp >>> + - const: adi,ad4697 >>> + # same chips with higher max sample rate > > I suppose one could make the argument that the programming model is > the same on these too, but the maximum sampling frequency does seem > like an important bit of information so that you don't try to set > the conversion trigger rate too high. > which property is that? I don't see differences in the driver, so I don't get how these wlcsp compatibles allow you to control value of conversion trigger. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-13 14:18 ` Krzysztof Kozlowski @ 2024-06-13 14:39 ` David Lechner 2024-06-13 15:11 ` Nuno Sá 0 siblings, 1 reply; 18+ messages in thread From: David Lechner @ 2024-06-13 14:39 UTC (permalink / raw) To: Krzysztof Kozlowski, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: > On 13/06/2024 15:57, David Lechner wrote: >> >>> >>>> + - const: adi,ad4695 >>>> + - items: >>>> + - const: adi,ad4697-wlcsp >>>> + - const: adi,ad4697 >>>> + # same chips with higher max sample rate >> >> I suppose one could make the argument that the programming model is >> the same on these too, but the maximum sampling frequency does seem >> like an important bit of information so that you don't try to set >> the conversion trigger rate too high. >> > > which property is that? I don't see differences in the driver, so I > don't get how these wlcsp compatibles allow you to control value of > conversion trigger. This comment is unrelated to the package type (WLCSP or LFCSP). What I mean is that e.g. AD4695 and AD4696 are virtually identical other than the maximum allowable sample rate (500 kSPS or 1 MSPS). So my thinking was that it would make sense to have: compatible = "ad4695"; for the lower sample rate chip and compatible = "ad4696", "ad4695"; for the higher sample rate chip since ad4696 can do everything that ad4695 does plus a bit more. We haven't implemented buffered reads in the driver yet, so there isn't anything currently to be seen there. But when we do, we probably want to limit the allowable value for the sampling_frequency attribute based on which version of the chip is present. (I would like to get Jonathan's opinion of if this is something we actually need to do or not.) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-13 14:39 ` David Lechner @ 2024-06-13 15:11 ` Nuno Sá 2024-06-13 19:43 ` Rob Herring 0 siblings, 1 reply; 18+ messages in thread From: Nuno Sá @ 2024-06-13 15:11 UTC (permalink / raw) To: David Lechner, Krzysztof Kozlowski, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On Thu, 2024-06-13 at 09:39 -0500, David Lechner wrote: > On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: > > On 13/06/2024 15:57, David Lechner wrote: > > > > > > > > > > > > + - const: adi,ad4695 > > > > > + - items: > > > > > + - const: adi,ad4697-wlcsp > > > > > + - const: adi,ad4697 > > > > > + # same chips with higher max sample rate > > > > > > I suppose one could make the argument that the programming model is > > > the same on these too, but the maximum sampling frequency does seem > > > like an important bit of information so that you don't try to set > > > the conversion trigger rate too high. > > > > > > > which property is that? I don't see differences in the driver, so I > > don't get how these wlcsp compatibles allow you to control value of > > conversion trigger. > > This comment is unrelated to the package type (WLCSP or LFCSP). > > What I mean is that e.g. AD4695 and AD4696 are virtually identical > other than the maximum allowable sample rate (500 kSPS or 1 MSPS). > > So my thinking was that it would make sense to have: > > compatible = "ad4695"; > > for the lower sample rate chip and > > compatible = "ad4696", "ad4695"; > > for the higher sample rate chip since ad4696 can do everything > that ad4695 does plus a bit more. > IMO, that would make sense yes. If the higher sample rate chip fallsback, it will still work but not at full speed. The other way around is the one that we can't allow naturally. But possibly dumb question now... since both devices will be supported at the same time, do we actually care about having the fallback compatible? My understanding of the fallback story is that we may load a DTS in an older kernel where chip A is supported but chip B is not and it is ok for chip B to fallback to chip A. Since these devices will be supported at the same time, do we need to care? Unless out of tree stuff enters the equation? Or is there another usecase that I'm not aware about (or maybe it just makes sense to document properly...)? - Nuno Sá > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-13 15:11 ` Nuno Sá @ 2024-06-13 19:43 ` Rob Herring 2024-06-13 20:09 ` David Lechner 0 siblings, 1 reply; 18+ messages in thread From: Rob Herring @ 2024-06-13 19:43 UTC (permalink / raw) To: Nuno Sá Cc: David Lechner, Krzysztof Kozlowski, Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On Thu, Jun 13, 2024 at 05:11:48PM +0200, Nuno Sá wrote: > On Thu, 2024-06-13 at 09:39 -0500, David Lechner wrote: > > On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: > > > On 13/06/2024 15:57, David Lechner wrote: > > > > > > > > > > > > > > > + - const: adi,ad4695 > > > > > > + - items: > > > > > > + - const: adi,ad4697-wlcsp > > > > > > + - const: adi,ad4697 > > > > > > + # same chips with higher max sample rate > > > > > > > > I suppose one could make the argument that the programming model is > > > > the same on these too, but the maximum sampling frequency does seem > > > > like an important bit of information so that you don't try to set > > > > the conversion trigger rate too high. > > > > > > > > > > which property is that? I don't see differences in the driver, so I > > > don't get how these wlcsp compatibles allow you to control value of > > > conversion trigger. > > > > This comment is unrelated to the package type (WLCSP or LFCSP). > > > > What I mean is that e.g. AD4695 and AD4696 are virtually identical > > other than the maximum allowable sample rate (500 kSPS or 1 MSPS). > > > > So my thinking was that it would make sense to have: > > > > compatible = "ad4695"; > > > > for the lower sample rate chip and > > > > compatible = "ad4696", "ad4695"; > > > > for the higher sample rate chip since ad4696 can do everything > > that ad4695 does plus a bit more. > > > > IMO, that would make sense yes. If the higher sample rate chip fallsback, it will > still work but not at full speed. The other way around is the one that we can't allow > naturally. > > But possibly dumb question now... since both devices will be supported at the same > time, do we actually care about having the fallback compatible? My understanding of > the fallback story is that we may load a DTS in an older kernel where chip A is > supported but chip B is not and it is ok for chip B to fallback to chip A. Since > these devices will be supported at the same time, do we need to care? Unless out of > tree stuff enters the equation? Yeah, it doesn't really matter much in that case. > Or is there another usecase that I'm not aware about (or maybe it just makes sense to > document properly...)? Somewhat I guess. Perhaps if there's a 3rd chip with higher rate, then it will be more obvious what to do and we don't have to have this discussion again for it. :) Rob ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-13 19:43 ` Rob Herring @ 2024-06-13 20:09 ` David Lechner 2024-06-15 12:25 ` Jonathan Cameron 0 siblings, 1 reply; 18+ messages in thread From: David Lechner @ 2024-06-13 20:09 UTC (permalink / raw) To: Rob Herring, Nuno Sá Cc: Krzysztof Kozlowski, Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On 6/13/24 2:43 PM, Rob Herring wrote: > On Thu, Jun 13, 2024 at 05:11:48PM +0200, Nuno Sá wrote: >> On Thu, 2024-06-13 at 09:39 -0500, David Lechner wrote: >>> On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: >>>> On 13/06/2024 15:57, David Lechner wrote: >>>>> >>>>>> >>>>>>> + - const: adi,ad4695 >>>>>>> + - items: >>>>>>> + - const: adi,ad4697-wlcsp >>>>>>> + - const: adi,ad4697 >>>>>>> + # same chips with higher max sample rate >>>>> >>>>> I suppose one could make the argument that the programming model is >>>>> the same on these too, but the maximum sampling frequency does seem >>>>> like an important bit of information so that you don't try to set >>>>> the conversion trigger rate too high. >>>>> >>>> >>>> which property is that? I don't see differences in the driver, so I >>>> don't get how these wlcsp compatibles allow you to control value of >>>> conversion trigger. >>> >>> This comment is unrelated to the package type (WLCSP or LFCSP). >>> >>> What I mean is that e.g. AD4695 and AD4696 are virtually identical >>> other than the maximum allowable sample rate (500 kSPS or 1 MSPS). >>> >>> So my thinking was that it would make sense to have: >>> >>> compatible = "ad4695"; >>> >>> for the lower sample rate chip and >>> >>> compatible = "ad4696", "ad4695"; >>> >>> for the higher sample rate chip since ad4696 can do everything >>> that ad4695 does plus a bit more. >>> >> >> IMO, that would make sense yes. If the higher sample rate chip fallsback, it will >> still work but not at full speed. The other way around is the one that we can't allow >> naturally. >> >> But possibly dumb question now... since both devices will be supported at the same >> time, do we actually care about having the fallback compatible? My understanding of >> the fallback story is that we may load a DTS in an older kernel where chip A is >> supported but chip B is not and it is ok for chip B to fallback to chip A. Since >> these devices will be supported at the same time, do we need to care? Unless out of >> tree stuff enters the equation? > > Yeah, it doesn't really matter much in that case. > >> Or is there another usecase that I'm not aware about (or maybe it just makes sense to >> document properly...)? > > Somewhat I guess. Perhaps if there's a 3rd chip with higher rate, then > it will be more obvious what to do and we don't have to have this > discussion again for it. :) > > Rob It sounds like maybe the best thing to do here then is just keep it simple? Like this: compatible: enum: - adi,ad4695 - adi,ad4696 - adi,ad4697 - adi,ad4698 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-13 20:09 ` David Lechner @ 2024-06-15 12:25 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2024-06-15 12:25 UTC (permalink / raw) To: David Lechner Cc: Rob Herring, Nuno Sá, Krzysztof Kozlowski, Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On Thu, 13 Jun 2024 15:09:40 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 6/13/24 2:43 PM, Rob Herring wrote: > > On Thu, Jun 13, 2024 at 05:11:48PM +0200, Nuno Sá wrote: > >> On Thu, 2024-06-13 at 09:39 -0500, David Lechner wrote: > >>> On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: > >>>> On 13/06/2024 15:57, David Lechner wrote: > >>>>> > >>>>>> > >>>>>>> + - const: adi,ad4695 > >>>>>>> + - items: > >>>>>>> + - const: adi,ad4697-wlcsp > >>>>>>> + - const: adi,ad4697 > >>>>>>> + # same chips with higher max sample rate > >>>>> > >>>>> I suppose one could make the argument that the programming model is > >>>>> the same on these too, but the maximum sampling frequency does seem > >>>>> like an important bit of information so that you don't try to set > >>>>> the conversion trigger rate too high. > >>>>> > >>>> > >>>> which property is that? I don't see differences in the driver, so I > >>>> don't get how these wlcsp compatibles allow you to control value of > >>>> conversion trigger. > >>> > >>> This comment is unrelated to the package type (WLCSP or LFCSP). > >>> > >>> What I mean is that e.g. AD4695 and AD4696 are virtually identical > >>> other than the maximum allowable sample rate (500 kSPS or 1 MSPS). > >>> > >>> So my thinking was that it would make sense to have: > >>> > >>> compatible = "ad4695"; > >>> > >>> for the lower sample rate chip and > >>> > >>> compatible = "ad4696", "ad4695"; > >>> > >>> for the higher sample rate chip since ad4696 can do everything > >>> that ad4695 does plus a bit more. > >>> > >> > >> IMO, that would make sense yes. If the higher sample rate chip fallsback, it will > >> still work but not at full speed. The other way around is the one that we can't allow > >> naturally. > >> > >> But possibly dumb question now... since both devices will be supported at the same > >> time, do we actually care about having the fallback compatible? My understanding of > >> the fallback story is that we may load a DTS in an older kernel where chip A is > >> supported but chip B is not and it is ok for chip B to fallback to chip A. Since > >> these devices will be supported at the same time, do we need to care? Unless out of > >> tree stuff enters the equation? > > > > Yeah, it doesn't really matter much in that case. > > > >> Or is there another usecase that I'm not aware about (or maybe it just makes sense to > >> document properly...)? > > > > Somewhat I guess. Perhaps if there's a 3rd chip with higher rate, then > > it will be more obvious what to do and we don't have to have this > > discussion again for it. :) > > > > Rob > > It sounds like maybe the best thing to do here then is just keep it simple? > > Like this: > > compatible: > enum: > - adi,ad4695 > - adi,ad4696 > - adi,ad4697 > - adi,ad4698 > Quick comments late in discussion. Simply compatible list is fine as they are all coming together (though that may in theory not be true on some other OS!) I'm also fine with the fallback as you discussed so less capable chip is the one we fall back to. Definitely need to differentiate chips with different possible ranges of a parameter as that goes straight through to userspace as sampling_frequency_available and userspace is going to get confused if it gets told it can do things that silently don't work. This sort of thing is why I always want to see chip IDs listed even if we think they'll just work with another one. a) Documents hardware right and people expect to see the ID of the chip on the BoM b) Lets us deal with cases where we have missed subtle differences in capabilities because the driver didn't support them yet. So in conclusion either option you proposed is fine (as above, or fallback to the part we think is a subset of functionality.) Jonathan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-12 19:20 ` [PATCH 1/3] dt-bindings: iio: adc: add " David Lechner 2024-06-13 7:11 ` Krzysztof Kozlowski @ 2024-06-15 12:41 ` Jonathan Cameron 2024-06-15 15:23 ` David Lechner 1 sibling, 1 reply; 18+ messages in thread From: Jonathan Cameron @ 2024-06-15 12:41 UTC (permalink / raw) To: David Lechner Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On Wed, 12 Jun 2024 14:20:40 -0500 David Lechner <dlechner@baylibre.com> wrote: > Add device tree bindings for AD4695 and similar ADCs. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 297 +++++++++++++++++++++ > MAINTAINERS | 9 + > 2 files changed, 306 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml > new file mode 100644 > index 000000000000..8ff5bbbbef9f > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml > @@ -0,0 +1,297 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/adi,ad4695.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices Easy Drive Multiplexed SAR Analog to Digital Converters > + > +maintainers: > + - Michael Hennerich <Michael.Hennerich@analog.com> > + - Nuno Sá <nuno.sa@analog.com> > + > +description: | > + A family of similar multi-channel analog to digital converters with SPI bus. > + > + * https://www.analog.com/en/products/ad4695.html > + * https://www.analog.com/en/products/ad4696.html > + * https://www.analog.com/en/products/ad4697.html > + * https://www.analog.com/en/products/ad4698.html > + > +$ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + oneOf: > + - enum: > + - adi,ad4695 > + - adi,ad4697 > + # same chips in WLCSP package with more pins > + - items: > + - const: adi,ad4695-wlcsp > + - const: adi,ad4695 > + - items: > + - const: adi,ad4697-wlcsp > + - const: adi,ad4697 > + # same chips with higher max sample rate > + - items: > + - const: adi,ad4696 > + - const: adi,ad4695 > + - items: > + - const: adi,ad4698 > + - const: adi,ad4697 > + # same chips with higher max sample rate in WLCSP package > + - items: > + - const: adi,ad4696-wlcsp > + - const: adi,ad4696 > + - const: adi,ad4695-wlcsp > + - const: adi,ad4695 > + - items: > + - const: adi,ad4698-wlcsp > + - const: adi,ad4698 > + - const: adi,ad4697-wlcsp > + - const: adi,ad4697 > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 80000000 > + > + spi-cpol: true > + spi-cpha: true > + > + spi-rx-bus-width: > + minimum: 1 > + maximum: 4 > + > + avdd-supply: > + description: A 2.7 V to 5.5 V supply that powers the analog circuitry. I'm a cynic. Do we care about the supported voltages in this binding doc? Feels just somewhere we might make a mistake. > + > + ldo-in-supply: > + description: A 2.4 V to 5.5 V supply connected to the internal LDO input. > + > + vdd-supply: > + description: A 1.8V supply that powers the core circuitry. > + > + vio-supply: > + description: A 1.2V to 1.8V supply for the digital inputs and outputs. > + > + ref-supply: > + description: A 2.4 V to 5.1 V supply for the external reference voltage. > + > + refin-supply: > + description: A 2.4 V to 5.1 V supply for the internal reference buffer input. > + > + com-supply: > + description: Common voltage supply for pseudo-differential analog inputs. These last few have more info in them so definitely good to have the descriptions > + > + adi,no-ref-current-limit: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + When this flag is present, the REF Overvoltage Reduced Current protection > + is disabled. > + > + adi,no-ref-high-z: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + Enable this flag if the ref-supply requires Reference Input High-Z Mode > + to be disabled for proper operation. > + > + cnv-gpios: > + description: The Convert Input (CNV). If omitted, CNV is tied to SPI CS. > + maxItems: 1 > + > + reset-gpios: > + description: The Reset Input (RESET). Should be configured GPIO_ACTIVE_LOW. > + maxItems: 1 > + > + interrupts: > + minItems: 1 > + items: > + - description: > + Signal coming from the BSY_ALT_GP0 or GP3 pin that indicates a busy > + condition. > + - description: > + Signal coming from the BSY_ALT_GP0 or GP2 pin that indicates an alert > + condition. > + > + interrupt-names: > + minItems: 1 > + items: > + - const: busy > + - const: alert > + > + gpio-controller: true > + > + "#gpio-cells": > + const: 2 > + description: | > + The first cell is the GPn number: 0 to 3. > + The second cell takes standard GPIO flags. > + > + "#address-cells": > + const: 1 > + "#size-cells": > + const: 0 > + > +patternProperties: > + "^channel@[0-9a-f]$": > + type: object > + $ref: adc.yaml > + unevaluatedProperties: false > + description: > + Describes each individual channel. In addition the properties defined > + below, bipolar from adc.yaml is also supported. > + > + properties: > + reg: > + maximum: 15 > + description: Input pin number (INx). I'd drop this description as the pin pairing makes it messy. If you switch to diff-channels etc, just leave it as a index value not connected to the pin numbers. > + > + adi,pin-pairing: > + description: | > + The input pin pairing for the negative input. This can be: > + - REFGND, normally 0V (single-ended) > + - COM, normally V_REF/2, see com-supply (pseudo-differential) > + - For even numbered pins, the next odd numbered pin (differential) > + $ref: /schemas/types.yaml#/definitions/string > + enum: [refgnd, com, next] Next is full on differential, just provide both channels via diff-channels. You can constrain the particular combinations in the binding. Refcnd is normal single ended. Probably want to use the new single-channel property for that as we are mixing differential and single ended channels so reg is pretty much just an index. Hmm. For comm we haven't had done a recent binding for a chip with the option of pseudo differential that is per channel, they've been whole device only. That feels like it will be common enough we need to support it cleanly with a 'general' scheme. Problem is I know someone will have a chip with 2 vincom pins and selecting between them, so we can't just have pseudo-differential as a boolean and adc.yaml There are horrible solutions like a magic channel number that changes the meaning of diff-channels but that's ugly. Maybe pseudo-differential for now and we have to later we add pseudo-differential-comm = <0> etc? > + default: refgnd > + > + adi,no-high-z: > + $ref: /schemas/types.yaml#/definitions/flag > + description: | Do we need the | given not really formatted? > + Enable this flag if the input pin requires the Analog Input High-Z > + Mode to be disabled for proper operation. > + > + required: > + - reg > + > + allOf: > + # only even number pins can be paired with the next pin > + - if: > + properties: > + reg: > + not: > + multipleOf: 2 > + then: > + properties: > + adi,pin-pairing: > + enum: [refgnd, com] > + # bipolar mode is not supported with REFGND pairing > + - if: > + not: > + required: > + - adi,pin-pairing > + then: > + properties: > + bipolar: false > + else: > + if: > + properties: > + adi,pin-pairing: > + const: refgnd > + then: > + properties: > + bipolar: false > + > +required: > + - compatible > + - reg > + - avdd-supply > + - vio-supply > + > +allOf: > + - oneOf: > + - required: > + - ref-supply > + - required: > + - refin-supply > + > + - oneOf: > + - required: > + - ldo-in-supply > + - required: > + - vdd-supply > + > + # LFSCP package has fewer pins, so a few things are not valid in that case > + - if: > + properties: > + compatible: > + not: > + contains: > + pattern: -wlcsp$ > + then: > + properties: > + refin-supply: false > + spi-rx-bus-width: > + maximum: 2 > + > + # the internal reference buffer always requires high-z mode > + - if: > + required: > + - refin-supply > + then: > + properties: > + adi,no-ref-high-z: false > + > + # limit channels for 8-channel chips > + - if: > + properties: > + compatible: > + contains: > + const: adi,ad4697 > + then: > + patternProperties: > + "^channel@[0-7]$": > + properties: > + reg: > + maximum: 7 > + "^channel@[8-9a-f]$": false > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + adc@0 { > + compatible = "adi,ad4695"; > + reg = <0>; > + spi-cpol; > + spi-cpha; > + spi-max-frequency = <80000000>; > + avdd-supply = <&supply_2_5V>; > + vdd-supply = <&supply_1_8V>; > + vio-supply = <&supply_1_2V>; > + ref-supply = <&supply_5V>; > + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + /* Differential channel between IN0 and IN1. */ > + channel@0 { > + reg = <0>; > + adi,pin-pairing = "next"; > + bipolar; > + }; > + > + /* Single-ended channel between IN2 and REFGND. */ > + channel@2 { > + reg = <2>; > + }; > + > + /* Pseudo-differential channel between IN3 and COM. */ > + channel@f { > + reg = <3>; > + adi,pin-pairing = "com"; > + bipolar; > + }; > + }; > + }; ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-15 12:41 ` Jonathan Cameron @ 2024-06-15 15:23 ` David Lechner 2024-06-15 17:55 ` Jonathan Cameron 0 siblings, 1 reply; 18+ messages in thread From: David Lechner @ 2024-06-15 15:23 UTC (permalink / raw) To: Jonathan Cameron Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On 6/15/24 7:41 AM, Jonathan Cameron wrote: > On Wed, 12 Jun 2024 14:20:40 -0500 > David Lechner <dlechner@baylibre.com> wrote: > >> Add device tree bindings for AD4695 and similar ADCs. >> ... >> + >> + avdd-supply: >> + description: A 2.7 V to 5.5 V supply that powers the analog circuitry. > > I'm a cynic. Do we care about the supported voltages in this binding doc? > Feels just somewhere we might make a mistake. Not especially useful, I suppose. I'll clean these up a bit. > >> + >> + ldo-in-supply: >> + description: A 2.4 V to 5.5 V supply connected to the internal LDO input. >> + >> + vdd-supply: >> + description: A 1.8V supply that powers the core circuitry. >> + >> + vio-supply: >> + description: A 1.2V to 1.8V supply for the digital inputs and outputs. >> + >> + ref-supply: >> + description: A 2.4 V to 5.1 V supply for the external reference voltage. >> + >> + refin-supply: >> + description: A 2.4 V to 5.1 V supply for the internal reference buffer input. >> + >> + com-supply: >> + description: Common voltage supply for pseudo-differential analog inputs. > > These last few have more info in them so definitely good to have the descriptions > ... >> + >> +patternProperties: >> + "^channel@[0-9a-f]$": >> + type: object >> + $ref: adc.yaml >> + unevaluatedProperties: false >> + description: >> + Describes each individual channel. In addition the properties defined >> + below, bipolar from adc.yaml is also supported. >> + >> + properties: >> + reg: >> + maximum: 15 >> + description: Input pin number (INx). > > I'd drop this description as the pin pairing makes it messy. > If you switch to diff-channels etc, just leave it as a index value not > connected to the pin numbers. > >> + >> + adi,pin-pairing: >> + description: | >> + The input pin pairing for the negative input. This can be: >> + - REFGND, normally 0V (single-ended) >> + - COM, normally V_REF/2, see com-supply (pseudo-differential) >> + - For even numbered pins, the next odd numbered pin (differential) >> + $ref: /schemas/types.yaml#/definitions/string >> + enum: [refgnd, com, next] > > Next is full on differential, just provide both channels via > diff-channels. You can constrain the particular combinations in the binding. > > Refcnd is normal single ended. Probably want to use the new single-channel > property for that as we are mixing differential and single ended channels > so reg is pretty much just an index. > > Hmm. For comm we haven't had done a recent binding for a chip with the option > of pseudo differential that is per channel, they've been whole device only. > That feels like it will be common enough we need to support it cleanly > with a 'general' scheme. I think we have. :-) https://lore.kernel.org/linux-iio/adc6cba9-2e79-475f-9c24-039fe9d3345d@baylibre.com/T/#mcbc1ce3a2541db502bf7870b7ea8574626a46312 > > Problem is I know someone will have a chip with 2 vincom pins and selecting > between them, so we can't just have pseudo-differential as a boolean and adc.yaml > > There are horrible solutions like a magic channel number that changes the > meaning of diff-channels but that's ugly. > Maybe pseudo-differential for now and we have to later we add > pseudo-differential-comm = <0> etc? > I was trying to keep things simple with 1 property instead of 3, but we can drop adi,pin-pairing and use the standard diff-channels, single-channel and common-mode-channel properties. > >> + default: refgnd >> + >> + adi,no-high-z: >> + $ref: /schemas/types.yaml#/definitions/flag >> + description: | > > Do we need the | given not really formatted? No. Was probably copy/paste. > >> + Enable this flag if the input pin requires the Analog Input High-Z >> + Mode to be disabled for proper operation. >> + ... >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + >> + spi { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + adc@0 { >> + compatible = "adi,ad4695"; >> + reg = <0>; >> + spi-cpol; >> + spi-cpha; >> + spi-max-frequency = <80000000>; >> + avdd-supply = <&supply_2_5V>; >> + vdd-supply = <&supply_1_8V>; >> + vio-supply = <&supply_1_2V>; >> + ref-supply = <&supply_5V>; >> + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; >> + >> + #address-cells = <1>; >> + #size-cells = <0>; >> + Using the standard adc.yaml properties, these would now be: >> + /* Differential channel between IN0 and IN1. */ >> + channel@0 { >> + reg = <0>; diff-channels = <0>, <1>; >> + bipolar; >> + }; >> + >> + /* Single-ended channel between IN2 and REFGND. */ >> + channel@2 { >> + reg = <2>; single-channel = <2>; common-mode-channel = <0>; >> + }; >> + >> + /* Pseudo-differential channel between IN3 and COM. */ >> + channel@f { >> + reg = <3>; single-channel = <3>; common-mode-channel = <1>; >> + bipolar; >> + }; >> + }; >> + }; > And I will add a header file with macros for the common mode channel values. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-15 15:23 ` David Lechner @ 2024-06-15 17:55 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2024-06-15 17:55 UTC (permalink / raw) To: David Lechner Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc > >> + > >> + adi,pin-pairing: > >> + description: | > >> + The input pin pairing for the negative input. This can be: > >> + - REFGND, normally 0V (single-ended) > >> + - COM, normally V_REF/2, see com-supply (pseudo-differential) > >> + - For even numbered pins, the next odd numbered pin (differential) > >> + $ref: /schemas/types.yaml#/definitions/string > >> + enum: [refgnd, com, next] > > > > Next is full on differential, just provide both channels via > > diff-channels. You can constrain the particular combinations in the binding. > > > > Refcnd is normal single ended. Probably want to use the new single-channel > > property for that as we are mixing differential and single ended channels > > so reg is pretty much just an index. > > > > Hmm. For comm we haven't had done a recent binding for a chip with the option > > of pseudo differential that is per channel, they've been whole device only. > > That feels like it will be common enough we need to support it cleanly > > with a 'general' scheme. > > I think we have. :-) > > https://lore.kernel.org/linux-iio/adc6cba9-2e79-475f-9c24-039fe9d3345d@baylibre.com/T/#mcbc1ce3a2541db502bf7870b7ea8574626a46312 > My goldfish like memory strikes again. Had completely forgotten that :) > > > > Problem is I know someone will have a chip with 2 vincom pins and selecting > > between them, so we can't just have pseudo-differential as a boolean and adc.yaml > > > > There are horrible solutions like a magic channel number that changes the > > meaning of diff-channels but that's ugly. > > Maybe pseudo-differential for now and we have to later we add > > pseudo-differential-comm = <0> etc? > > > > I was trying to keep things simple with 1 property instead of 3, but we > can drop adi,pin-pairing and use the standard diff-channels, single-channel > and common-mode-channel properties. Sounds good. > > >> +examples: > >> + - | > >> + #include <dt-bindings/gpio/gpio.h> > >> + > >> + spi { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + adc@0 { > >> + compatible = "adi,ad4695"; > >> + reg = <0>; > >> + spi-cpol; > >> + spi-cpha; > >> + spi-max-frequency = <80000000>; > >> + avdd-supply = <&supply_2_5V>; > >> + vdd-supply = <&supply_1_8V>; > >> + vio-supply = <&supply_1_2V>; > >> + ref-supply = <&supply_5V>; > >> + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; > >> + > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > > Using the standard adc.yaml properties, these would now be: > > >> + /* Differential channel between IN0 and IN1. */ > >> + channel@0 { > >> + reg = <0>; > > diff-channels = <0>, <1>; > > >> + bipolar; > >> + }; > >> + > >> + /* Single-ended channel between IN2 and REFGND. */ > >> + channel@2 { > >> + reg = <2>; > > single-channel = <2>; > common-mode-channel = <0>; I wonder if we count ground ad default and so don't necessarily specify this? I don't really mind either way though. Maybe just default to 0 works. > > >> + }; > >> + > >> + /* Pseudo-differential channel between IN3 and COM. */ > >> + channel@f { > >> + reg = <3>; > > single-channel = <3>; > common-mode-channel = <1>; > > >> + bipolar; > >> + }; > >> + }; > >> + }; > > > > And I will add a header file with macros for the common mode > channel values. Great Jonathan > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] iio: adc: ad4695: Add driver for AD4695 and similar ADCs 2024-06-12 19:20 [PATCH 0/3] iio: adc: ad4695: new driver for AD4695 and similar ADCs David Lechner 2024-06-12 19:20 ` [PATCH 1/3] dt-bindings: iio: adc: add " David Lechner @ 2024-06-12 19:20 ` David Lechner 2024-06-12 19:40 ` David Lechner 2024-06-15 13:10 ` Jonathan Cameron 2024-06-12 19:20 ` [PATCH 3/3] Documentation: iio: Document ad4695 driver David Lechner 2 siblings, 2 replies; 18+ messages in thread From: David Lechner @ 2024-06-12 19:20 UTC (permalink / raw) To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: David Lechner, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc, Ramona Gradinariu This is a new driver for Analog Devices Inc. AD4695 and similar ADCs. The initial driver supports initializing the chip including configuring all possible LDO and reference voltage sources as well as any possible voltage input channel wiring configuration. Only the 4-wire SPI wiring mode where the CNV pin is tied to the CS pin is supported at this time. And reading sample data from the ADC can only be done in direct mode for now. Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com> Signed-off-by: David Lechner <dlechner@baylibre.com> --- MAINTAINERS | 1 + drivers/iio/adc/Kconfig | 11 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad4695.c | 804 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 817 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 8d15e4089d7c..611b7929e650 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1217,6 +1217,7 @@ L: linux-iio@vger.kernel.org S: Supported W: https://ez.analog.com/linux-software-drivers F: Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml +F: drivers/iio/adc/ad4695.c ANALOG DEVICES INC AD7091R DRIVER M: Marcelo Schmitt <marcelo.schmitt@analog.com> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 3d91015af6ea..26c3090ff967 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -36,6 +36,17 @@ config AD4130 To compile this driver as a module, choose M here: the module will be called ad4130. +config AD4695 + tristate "Analog Device AD4695 ADC Driver" + depends on SPI + select REGMAP_SPI + help + Say yes here to build support for Analog Devices AD4695 and similar + analog to digital converters (ADC). + + To compile this driver as a module, choose M here: the module will be + called ad4695. + config AD7091R tristate diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index 37ac689a0209..5c4d79d4f939 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o obj-$(CONFIG_AD4130) += ad4130.o +obj-$(CONFIG_AD4695) += ad4695.o obj-$(CONFIG_AD7091R) += ad7091r-base.o obj-$(CONFIG_AD7091R5) += ad7091r5.o obj-$(CONFIG_AD7091R8) += ad7091r8.o diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c new file mode 100644 index 000000000000..6e5a87817c86 --- /dev/null +++ b/drivers/iio/adc/ad4695.c @@ -0,0 +1,804 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * SPI ADC driver for Analog Devices Inc. AD4695 and similar chips + * + * https://www.analog.com/en/products/ad4695.html + * https://www.analog.com/en/products/ad4696.html + * https://www.analog.com/en/products/ad4697.html + * https://www.analog.com/en/products/ad4698.html + * + * Copyright 2024 Analog Devices Inc. + * Copyright 2024 BayLibre, SAS + */ + +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/compiler.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/gpio/consumer.h> +#include <linux/iio/iio.h> +#include <linux/property.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> +#include <linux/units.h> + +/* AD4695 registers */ +#define AD4695_REG_SPI_CONFIG_A 0x0000 +#define AD4695_REG_SPI_CONFIG_B 0x0001 +#define AD4695_REG_DEVICE_TYPE 0x0003 +#define AD4695_REG_SCRATCH_PAD 0x000A +#define AD4695_REG_VENDOR_L 0x000C +#define AD4695_REG_VENDOR_H 0x000D +#define AD4695_REG_LOOP_MODE 0x000E +#define AD4695_REG_SPI_CONFIG_C 0x0010 +#define AD4695_REG_SPI_STATUS 0x0011 +#define AD4695_REG_STATUS 0x0014 +#define AD4695_REG_ALERT_STATUS1 0x0015 +#define AD4695_REG_ALERT_STATUS2 0x0016 +#define AD4695_REG_CLAMP_STATUS 0x001A +#define AD4695_REG_SETUP 0x0020 +#define AD4695_REG_REF_CTRL 0x0021 +#define AD4695_REG_SEQ_CTRL 0x0022 +#define AD4695_REG_AC_CTRL 0x0023 +#define AD4695_REG_STD_SEQ_CONFIG 0x0024 +#define AD4695_REG_GPIO_CTRL 0x0026 +#define AD4695_REG_GP_MODE 0x0027 +#define AD4695_REG_TEMP_CTRL 0x0029 +#define AD4695_REG_CONFIG_IN(n) (0x0030 | (n)) +#define AD4695_REG_UPPER_IN(n) (0x0040 | (2 * (n))) +#define AD4695_REG_LOWER_IN(n) (0x0060 | (2 * (n))) +#define AD4695_REG_HYST_IN(n) (0x0080 | (2 * (n))) +#define AD4695_REG_OFFSET_IN(n) (0x00A0 | (2 * (n))) +#define AD4695_REG_GAIN_IN(n) (0x00C0 | (2 * (n))) +#define AD4695_REG_AS_SLOT(n) (0x0100 | (n)) +#define AD4695_REG_MAX 0x017F + +/* Conversion mode commands */ +#define AD4695_CMD_EXIT_CNV_MODE 0x0A +#define AD4695_CMD_TEMP_CHAN 0x0F +#define AD4695_CMD_VOLTAGE_CHAN(n) (0x10 | (n)) + +/* SPI_CONFIG_A */ +#define AD4695_SW_RST_MSB BIT(7) +#define AD4695_SW_RST_LSB BIT(0) + +/* SPI_CONFIG_B */ +#define AD4695_INST_MODE_MASK BIT(7) +#define AD4695_INST_MODE(x) FIELD_PREP(AD4695_INST_MODE_MASK, (x)) + +enum ad4695_instr_mode { + AD4695_INST_MODE_STREAM, + AD4695_INST_MODE_SINGLE, +}; + +/* Setup */ +#define AD4695_LDO_EN_MASK BIT(4) +#define AD4695_LDO_EN(x) FIELD_PREP(AD4695_LDO_EN_MASK, (((x) ? 1 : 0))) +#define AD4695_SPI_MODE_MASK BIT(2) +#define AD4695_SPI_MODE(x) FIELD_PREP(AD4695_SPI_MODE_MASK, (x)) +#define AD4695_SPI_CYC_CTRL_MASK BIT(1) +#define AD4695_SPI_CYC_CTRL(x) FIELD_PREP(AD4695_SPI_CYC_CTRL_MASK, (x)) + +/* REF_CTRL */ +#define AD4695_OV_MODE_MASK BIT(7) +#define AD4695_OV_MODE(x) FIELD_PREP(AD4695_OV_MODE_MASK, (x)) +#define AD4695_VREF_SET_MASK GENMASK(4, 2) +#define AD4695_VREF_SET(x) FIELD_PREP(AD4695_VREF_SET_MASK, (x)) +#define AD4695_REFHIZ_EN_MASK BIT(1) +#define AD4695_REFHIZ_EN(x) FIELD_PREP(AD4695_REFHIZ_EN_MASK, (((x) ? 1 : 0))) +#define AD4695_REFBUF_EN_MASK BIT(0) +#define AD4695_REFBUF_EN(x) FIELD_PREP(AD4695_REFBUF_EN_MASK, (((x) ? 1 : 0))) + +enum ad4695_ov_mode { + AD4695_OV_MODE_REDUCE_CURRENT, + AD4695_OV_MODE_DO_NOT_REDUCE_CURRENT, +}; + +/* SEQ_CTRL */ +#define AD4695_STD_SEQ_EN_MASK BIT(7) +#define AD4695_STD_SEQ_EN(x) FIELD_PREP(AD4695_STD_SEQ_EN_MASK, (((x) ? 1 : 0))) +#define AD4695_NUM_SLOTS_AS_MASK GENMASK(6, 0) +#define AD4695_NUM_SLOTS_AS(x) FIELD_PREP(AD4695_NUM_SLOTS_AS_MASK, (x)) + +/* CONFIG_INn */ +#define AD4695_IN_MODE_MASK BIT(6) +#define AD4695_IN_MODE(x) FIELD_PREP(AD4695_IN_MODE_MASK, (((x) ? 1 : 0))) +#define AD4695_IN_PAIR_MASK GENMASK(5, 4) +#define AD4695_IN_PAIR(x) FIELD_PREP(AD4695_IN_PAIR_MASK, (x)) +#define AD4695_AINHIGHZ_EN_MASK BIT(3) +#define AD4695_AINHIGHZ_EN(x) FIELD_PREP(AD4695_AINHIGHZ_EN_MASK, (((x) ? 1 : 0))) + +enum ad4695_in_pair { + AD4695_WITH_REFGND, + AD4695_WITH_COM, + AD4695_EVEN_ODD, +}; + +/* AS_SLOTn */ +#define AD4695_SLOT_INX_MASK GENMASK(3, 0) +#define AD4695_SLOT_INX(x) FIELD_PREP(AD4695_SLOT_INX_MASK, (x)) + +/* timing specs */ +#define AD4695_T_CONVERT_NS 415 +#define AD4695_T_WAKEUP_HW_MS 3 +#define AD4695_T_WAKEUP_SW_MS 3 +#define AD4695_T_REFBUF_MS 100 +#define AD4695_REG_ACCESS_SCLK_HZ (10 * MEGA) + +/* adi,pin-paring DT property lookup table */ +static const char * const ad4695_pin_pairing[] = { + [AD4695_WITH_REFGND] = "refgnd", + [AD4695_WITH_COM] = "com", + [AD4695_EVEN_ODD] = "next", +}; + +struct ad4695_chip_info { + const char *name; + int max_sample_rate; + u8 num_voltage_inputs; +}; + +struct ad4695_channel_config { + bool bipolar; + bool highz_en; + unsigned int channel; + enum ad4695_in_pair pin_pairing; +}; + +struct ad4695_state { + struct spi_device *spi; + struct regmap *regmap; + struct gpio_desc *reset_gpio; + struct ad4695_channel_config *channels_cfg; + const struct ad4695_chip_info *chip_info; + /* Reference voltage. */ + unsigned int vref_mv; + /* Common mode input pin voltage. */ + unsigned int com_mv; + /* raw data conversion data recived */ + u16 raw_data __aligned(IIO_DMA_MINALIGN); + /* Commands to send for single conversion */ + u16 cnv_cmd; + u8 cnv_cmd2; +}; + +static const struct regmap_range ad4695_regmap_rd_ranges[] = { + regmap_reg_range(AD4695_REG_SPI_CONFIG_A, AD4695_REG_SPI_CONFIG_B), + regmap_reg_range(AD4695_REG_DEVICE_TYPE, AD4695_REG_DEVICE_TYPE), + regmap_reg_range(AD4695_REG_SCRATCH_PAD, AD4695_REG_SCRATCH_PAD), + regmap_reg_range(AD4695_REG_VENDOR_L, AD4695_REG_LOOP_MODE), + regmap_reg_range(AD4695_REG_SPI_CONFIG_C, AD4695_REG_SPI_STATUS), + regmap_reg_range(AD4695_REG_STATUS, AD4695_REG_ALERT_STATUS2), + regmap_reg_range(AD4695_REG_CLAMP_STATUS, AD4695_REG_CLAMP_STATUS), + regmap_reg_range(AD4695_REG_SETUP, AD4695_REG_TEMP_CTRL), + regmap_reg_range(AD4695_REG_CONFIG_IN(0), AD4695_REG_MAX), +}; + +static const struct regmap_access_table ad4695_regmap_rd_table = { + .yes_ranges = ad4695_regmap_rd_ranges, + .n_yes_ranges = ARRAY_SIZE(ad4695_regmap_rd_ranges), +}; + +static const struct regmap_range ad4695_regmap_wr_ranges[] = { + regmap_reg_range(AD4695_REG_SPI_CONFIG_A, AD4695_REG_SPI_CONFIG_B), + regmap_reg_range(AD4695_REG_SCRATCH_PAD, AD4695_REG_SCRATCH_PAD), + regmap_reg_range(AD4695_REG_LOOP_MODE, AD4695_REG_LOOP_MODE), + regmap_reg_range(AD4695_REG_SPI_CONFIG_C, AD4695_REG_SPI_STATUS), + regmap_reg_range(AD4695_REG_SETUP, AD4695_REG_TEMP_CTRL), + regmap_reg_range(AD4695_REG_CONFIG_IN(0), AD4695_REG_MAX), +}; + +static const struct regmap_access_table ad4695_regmap_wr_table = { + .yes_ranges = ad4695_regmap_wr_ranges, + .n_yes_ranges = ARRAY_SIZE(ad4695_regmap_wr_ranges), +}; + +static const struct regmap_config ad4695_regmap_config = { + .name = "ad4695", + .reg_bits = 16, + .val_bits = 8, + .max_register = AD4695_REG_MAX, + .rd_table = &ad4695_regmap_rd_table, + .wr_table = &ad4695_regmap_wr_table, + .can_multi_write = true, +}; + +static const struct iio_chan_spec ad4695_channel_template = { + .type = IIO_VOLTAGE, + .indexed = 1, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_OFFSET), + .scan_type = { + .realbits = 16, + .storagebits = 16, + }, +}; + +static const struct iio_chan_spec ad4695_temp_channel_template = { + .address = AD4695_CMD_TEMP_CHAN, + .type = IIO_TEMP, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_OFFSET), + .scan_type = { + .sign = 's', + .realbits = 16, + .storagebits = 16, + }, +}; + +static const char * const ad4695_power_supplies[] = { + "avdd", "vio" +}; + +static const struct ad4695_chip_info ad4695_chip_info = { + .name = "ad4695", + .max_sample_rate = 500 * KILO, + .num_voltage_inputs = 16, +}; + +static const struct ad4695_chip_info ad4696_chip_info = { + .name = "ad4696", + .max_sample_rate = 1 * MEGA, + .num_voltage_inputs = 16, +}; + +static const struct ad4695_chip_info ad4697_chip_info = { + .name = "ad4697", + .max_sample_rate = 500 * KILO, + .num_voltage_inputs = 8, +}; + +static const struct ad4695_chip_info ad4698_chip_info = { + .name = "ad4698", + .max_sample_rate = 1 * MEGA, + .num_voltage_inputs = 8, +}; + +/* register convenience functions */ + +static int ad4695_soft_reset(struct ad4695_state *st) +{ + int ret; + + ret = regmap_set_bits(st->regmap, AD4695_REG_SPI_CONFIG_A, + AD4695_SW_RST_MSB | AD4695_SW_RST_LSB); + if (ret) + return ret; + + /* datasheet says we have to wait before communicating again */ + msleep(AD4695_T_WAKEUP_SW_MS); + + return 0; +} + +static int ad4695_set_instr_mode(struct ad4695_state *st, + enum ad4695_instr_mode mode) +{ + return regmap_update_bits(st->regmap, AD4695_REG_SPI_CONFIG_B, + AD4695_INST_MODE_MASK, AD4695_INST_MODE(mode)); +} + +static int ad4695_set_ldo_en(struct ad4695_state *st, bool enable) +{ + return regmap_update_bits(st->regmap, AD4695_REG_SETUP, + AD4695_LDO_EN_MASK, AD4695_LDO_EN(enable)); +} + +/** + * ad4695_set_single_cycle_mode - Set the device in single cycle mode + * @st: The AD4695 state + * @channel: The first channel to read + * + * As per the datasheet, to enable single cycle mode, we need to set + * STD_SEQ_EN=0, NUM_SLOTS_AS=0 and CYC_CTRL=1 (Table 15). Setting SPI_MODE=1 + * triggers the first conversion using the channel in AS_SLOT0. + * + * Context: can sleep, must be called with iio_device_claim_direct held + * Return: 0 on success, a negative error code on failure + */ +static int ad4695_set_single_cycle_mode(struct ad4695_state *st, + unsigned int channel) +{ + int ret; + + ret = regmap_clear_bits(st->regmap, AD4695_REG_SEQ_CTRL, + AD4695_STD_SEQ_EN_MASK | AD4695_NUM_SLOTS_AS_MASK); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD4695_REG_AS_SLOT(0), + AD4695_SLOT_INX(channel)); + if (ret) + return ret; + + return regmap_set_bits(st->regmap, AD4695_REG_SETUP, + AD4695_SPI_MODE_MASK | AD4695_SPI_CYC_CTRL_MASK); +} + +static int ad4695_set_ov_mode(struct ad4695_state *st, enum ad4695_ov_mode mode) +{ + return regmap_update_bits(st->regmap, AD4695_REG_REF_CTRL, + AD4695_OV_MODE_MASK, AD4695_OV_MODE(mode)); +} + +static int ad4695_set_ref_voltage(struct ad4695_state *st, int ref_voltage) +{ + u8 val; + + if (ref_voltage >= 2400 && ref_voltage <= 2750) + val = 0; + else if (ref_voltage > 2750 && ref_voltage <= 3250) + val = 1; + else if (ref_voltage > 3250 && ref_voltage <= 3750) + val = 2; + else if (ref_voltage > 3750 && ref_voltage <= 4500) + val = 3; + else if (ref_voltage > 4500 && ref_voltage <= 5100) + val = 4; + else + return -EINVAL; + + return regmap_update_bits(st->regmap, AD4695_REG_REF_CTRL, + AD4695_VREF_SET_MASK, AD4695_VREF_SET(val)); +} + +static int ad4695_set_refhiz_en(struct ad4695_state *st, bool enable) +{ + return regmap_update_bits(st->regmap, AD4695_REG_REF_CTRL, + AD4695_REFHIZ_EN_MASK, AD4695_REFHIZ_EN(enable)); +} + +static int ad4695_set_refbuf_en(struct ad4695_state *st, bool enable) +{ + return regmap_update_bits(st->regmap, AD4695_REG_REF_CTRL, + AD4695_REFBUF_EN_MASK, AD4695_REFBUF_EN(enable)); +} + +static int ad4695_write_chn_cfg(struct ad4695_state *st, + struct ad4695_channel_config *cfg) +{ + u32 mask = 0, val = 0; + + mask |= AD4695_IN_MODE_MASK; + val |= AD4695_IN_MODE(cfg->bipolar); + + mask |= AD4695_IN_PAIR_MASK; + val |= AD4695_IN_PAIR(cfg->pin_pairing); + + mask |= AD4695_AINHIGHZ_EN_MASK; + val |= AD4695_AINHIGHZ_EN(cfg->highz_en); + + return regmap_update_bits(st->regmap, AD4695_REG_CONFIG_IN(cfg->channel), + mask, val); +} + +/** + * ad4695_read_one_sample - Read a single sample using single-cycle mode + * @st: The AD4695 state + * @address: The address of the channel to read + * + * Upon return, the sample will be stored in the raw_data field of @st. + * + * Context: can sleep, must be called with iio_device_claim_direct held + * Return: 0 on success, a negative error code on failure + */ +static int ad4695_read_one_sample(struct ad4695_state *st, unsigned int address) +{ + struct spi_transfer xfer[2] = { }; + int ret; + + ret = ad4695_set_single_cycle_mode(st, address); + if (ret) + return ret; + + /* + * Setting the first channel to the temperature channel isn't supported + * in single-cycle mode, so we have to do an extra xfer to read the + * temperature. + */ + if (address == AD4695_CMD_TEMP_CHAN) { + /* We aren't reading, so we can make this a short xfer. */ + st->cnv_cmd2 = AD4695_CMD_TEMP_CHAN << 3; + xfer[0].bits_per_word = 8; + xfer[0].tx_buf = &st->cnv_cmd2; + xfer[0].len = 1; + xfer[0].cs_change = 1; + xfer[0].cs_change_delay.value = AD4695_T_CONVERT_NS; + xfer[0].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS; + + /* Then read the result and exit conversion mode. */ + st->cnv_cmd = AD4695_CMD_EXIT_CNV_MODE << 11; + xfer[1].bits_per_word = 16; + xfer[1].tx_buf = &st->cnv_cmd; + xfer[1].rx_buf = &st->raw_data; + xfer[1].len = 2; + + return spi_sync_transfer(st->spi, xfer, 2); + } + + /* + * The conversion has already been done and we just have to read the + * result and exit conversion mode. + */ + st->cnv_cmd = AD4695_CMD_EXIT_CNV_MODE << 11; + xfer[0].bits_per_word = 16; + xfer[0].tx_buf = &st->cnv_cmd; + xfer[0].rx_buf = &st->raw_data; + xfer[0].len = 2; + + return spi_sync_transfer(st->spi, xfer, 1); +} + +/* IIO implementation */ + +static int ad4695_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct ad4695_state *st = iio_priv(indio_dev); + struct ad4695_channel_config *cfg = &st->channels_cfg[chan->scan_index]; + u8 realbits = chan->scan_type.realbits; + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { + ret = ad4695_read_one_sample(st, chan->address); + if (ret) + return ret; + + if (chan->scan_type.sign == 's') + *val = sign_extend32(st->raw_data, realbits - 1); + else + *val = st->raw_data; + + return IIO_VAL_INT; + } + unreachable(); + case IIO_CHAN_INFO_SCALE: + switch (chan->type) { + case IIO_VOLTAGE: + *val = st->vref_mv; + *val2 = chan->scan_type.realbits; + return IIO_VAL_FRACTIONAL_LOG2; + case IIO_TEMP: + /* T_scale (°C) = raw * V_REF (mV) / (-1.8 mV/°C * 2^16) */ + *val = st->vref_mv * -556; + *val2 = 16; + return IIO_VAL_FRACTIONAL_LOG2; + default: + return -EINVAL; + } + case IIO_CHAN_INFO_OFFSET: + switch (chan->type) { + case IIO_VOLTAGE: + if (cfg->pin_pairing == AD4695_WITH_COM) + *val = st->com_mv * (1 << realbits) / st->vref_mv; + else + *val = 0; + + return IIO_VAL_INT; + case IIO_TEMP: + /* T_offset (°C) = -725 mV / (-1.8 mV/°C) */ + /* T_offset (raw) = T_offset (°C) * (-1.8 mV/°C) * 2^16 / V_REF (mV) */ + *val = -47513600; + *val2 = st->vref_mv; + return IIO_VAL_FRACTIONAL; + default: + return -EINVAL; + } + default: + return -EINVAL; + } +} + +static int ad4695_debugfs_reg_access(struct iio_dev *indio_dev, unsigned int reg, + unsigned int writeval, unsigned int *readval) +{ + struct ad4695_state *st = iio_priv(indio_dev); + + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { + if (readval) + return regmap_read(st->regmap, reg, readval); + + return regmap_write(st->regmap, reg, writeval); + } + + unreachable(); +} + +static const struct iio_info ad4695_info = { + .read_raw = &ad4695_read_raw, + .debugfs_reg_access = &ad4695_debugfs_reg_access, +}; + +static int ad4695_parse_channel_cfg(struct iio_dev *indio_dev) +{ + struct device *dev = indio_dev->dev.parent; + struct ad4695_state *st = iio_priv(indio_dev); + struct iio_chan_spec *iio_chan_arr; + struct ad4695_channel_config *chan_cfg_arr; + unsigned int channel, chan_idx = 0; + int ret; + + indio_dev->num_channels = device_get_child_node_count(dev); + if (!indio_dev->num_channels) + return dev_err_probe(dev, -ENODEV, "no channel children\n"); + + /* Extra channel for temperature. */ + indio_dev->num_channels++; + + iio_chan_arr = devm_kcalloc(dev, indio_dev->num_channels, + sizeof(*iio_chan_arr), GFP_KERNEL); + if (!iio_chan_arr) + return -ENOMEM; + + chan_cfg_arr = devm_kcalloc(dev, indio_dev->num_channels, + sizeof(*chan_cfg_arr), GFP_KERNEL); + if (!chan_cfg_arr) + return -ENOMEM; + + indio_dev->channels = iio_chan_arr; + st->channels_cfg = chan_cfg_arr; + + device_for_each_child_node_scoped(dev, child) { + struct ad4695_channel_config *chan_cfg = &st->channels_cfg[chan_idx]; + const char *pin_pairing_str; + + ret = fwnode_property_read_u32(child, "reg", &channel); + if (ret) + return dev_err_probe(dev, ret, "failed to read reg property (%s)\n", + fwnode_get_name(child)); + + if (channel >= st->chip_info->num_voltage_inputs) + return dev_err_probe(dev, -EINVAL, + "channel id out of range, maximum %d channels allowed (%s)\n", + st->chip_info->num_voltage_inputs, + fwnode_get_name(child)); + + iio_chan_arr[chan_idx] = ad4695_channel_template; + iio_chan_arr[chan_idx].scan_index = chan_idx; + iio_chan_arr[chan_idx].channel = channel; + iio_chan_arr[chan_idx].address = AD4695_CMD_VOLTAGE_CHAN(channel); + + chan_cfg->bipolar = fwnode_property_read_bool(child, "bipolar"); + if (chan_cfg->bipolar) + iio_chan_arr[chan_idx].scan_type.sign = 's'; + + chan_cfg->highz_en = !fwnode_property_read_bool(child, "adi,no-high-z"); + + ret = fwnode_property_read_string(child, "adi,pin-pairing", &pin_pairing_str); + if (ret && ret != -EINVAL) + return dev_err_probe(dev, ret, + "failed to read adi,pin-pairing property (%s)\n", + fwnode_get_name(child)); + + if (ret == -EINVAL) { + /* default when property omitted */ + chan_cfg->pin_pairing = AD4695_WITH_REFGND; + } else { + ret = sysfs_match_string(ad4695_pin_pairing, pin_pairing_str); + if (ret < 0) + return dev_err_probe(dev, ret, + "failed to match pin pairing: \"%s\" (%s)\n", + pin_pairing_str, fwnode_get_name(child)); + + chan_cfg->pin_pairing = ret; + } + + if (chan_cfg->bipolar && chan_cfg->pin_pairing == AD4695_WITH_REFGND) + return dev_err_probe(dev, -EINVAL, + "bipolar mode is not available for channels with the \"refgnd\" pin pairing assignment selected (%s).\n", + fwnode_get_name(child)); + + if (chan_cfg->pin_pairing == AD4695_EVEN_ODD) { + if (channel % 2 == 1) + return dev_err_probe(dev, -EINVAL, + "channels with \"next\" pin pairing assignment selected must have an even index (%s)\n", + fwnode_get_name(child)); + + iio_chan_arr[chan_idx].differential = 1; + iio_chan_arr[chan_idx].channel2 = channel + 1; + } + + chan_cfg->channel = channel; + + ret = ad4695_write_chn_cfg(st, chan_cfg); + if (ret) + return ret; + + chan_idx++; + } + + /* Temperature channel must be next scan index after voltage channels. */ + iio_chan_arr[chan_idx] = ad4695_temp_channel_template; + iio_chan_arr[chan_idx].scan_index = chan_idx; + + return 0; +} + +static int ad4695_probe(struct spi_device *spi) +{ + struct device *dev = &spi->dev; + struct ad4695_state *st; + struct iio_dev *indio_dev; + struct gpio_desc *cnv_gpio; + bool use_internal_ldo_supply; + bool use_internal_ref_buffer; + int ret; + + cnv_gpio = devm_gpiod_get_optional(dev, "cnv", GPIOD_OUT_LOW); + if (IS_ERR(cnv_gpio)) + return dev_err_probe(dev, PTR_ERR(cnv_gpio), "Failed to get CNV GPIO\n"); + + /* Driver currently requires CNV pin to be connected to SPI CS */ + if (cnv_gpio) + return dev_err_probe(dev, -ENODEV, "CNV GPIO is not supported\n"); + + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + st->spi = spi; + + st->chip_info = spi_get_device_match_data(spi); + if (!st->chip_info) + return -EINVAL; + + /* Registers cannot be read at the max allowable speed */ + spi->max_speed_hz = AD4695_REG_ACCESS_SCLK_HZ; + + st->regmap = devm_regmap_init_spi(spi, &ad4695_regmap_config); + if (IS_ERR(st->regmap)) + return dev_err_probe(dev, PTR_ERR(st->regmap), "Failed to initialize regmap\n"); + + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad4695_power_supplies), + ad4695_power_supplies); + if (ret) + return dev_err_probe(dev, ret, "Failed to enable power supplies\n"); + + /* If LDO_IN supply is present, then we are using internal LDO. */ + ret = devm_regulator_get_enable_optional(dev, "ldo-in"); + if (ret < 0 && ret != -ENODEV) + return dev_err_probe(dev, ret, "Failed to enable LDO_IN supply\n"); + + use_internal_ldo_supply = ret == 0; + + if (!use_internal_ldo_supply) { + /* Otherwise we need an external VDD supply. */ + ret = devm_regulator_get_enable(dev, "vdd"); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to enable VDD supply\n"); + } + + /* If REFIN supply is given, then we are using internal buffer */ + ret = devm_regulator_get_enable_read_voltage(dev, "refin"); + if (ret < 0 && ret != -ENODEV) + return dev_err_probe(dev, ret, "Failed to get REFIN voltage\n"); + + if (ret != -ENODEV) { + st->vref_mv = ret / 1000; + use_internal_ref_buffer = true; + } else { + /* Otherwise, we need an external reference. */ + ret = devm_regulator_get_enable_read_voltage(dev, "ref"); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to get REF voltage\n"); + + st->vref_mv = ret / 1000; + use_internal_ref_buffer = false; + } + + ret = devm_regulator_get_enable_read_voltage(dev, "com"); + if (ret < 0 && ret != -ENODEV) + return dev_err_probe(dev, ret, "Failed to get COM voltage\n"); + + st->com_mv = ret == -ENODEV ? 0 : ret / 1000; + + /* + * Reset the device using hardware reset if available or fall back to + * software reset. + */ + + st->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(st->reset_gpio)) + return PTR_ERR(st->reset_gpio); + + if (st->reset_gpio) { + gpiod_set_value(st->reset_gpio, 0); + /* datasheet says we have to wait before communicating */ + msleep(AD4695_T_WAKEUP_HW_MS); + } else { + ret = ad4695_soft_reset(st); + if (ret) + return ret; + } + + ret = ad4695_set_ldo_en(st, use_internal_ldo_supply); + if (ret) + return ret; + + /* configure reference supply */ + + if (device_property_present(dev, "adi,no-ref-current-limit")) { + ret = ad4695_set_ov_mode(st, AD4695_OV_MODE_DO_NOT_REDUCE_CURRENT); + if (ret) + return ret; + } + + if (device_property_present(dev, "adi,no-ref-high-z")) { + if (use_internal_ref_buffer) + return dev_err_probe(dev, -EINVAL, + "Cannot disable high-Z mode for internal reference buffer\n"); + + ret = ad4695_set_refhiz_en(st, false); + if (ret) + return ret; + } + + ad4695_set_ref_voltage(st, st->vref_mv); + if (ret) + return ret; + + if (use_internal_ref_buffer) { + ret = ad4695_set_refbuf_en(st, true); + if (ret) + return ret; + + /* Give the capacitor some time to charge up. */ + msleep(AD4695_T_REFBUF_MS); + } + + ret = ad4695_parse_channel_cfg(indio_dev); + if (ret) + return ret; + + ret = ad4695_set_instr_mode(st, AD4695_INST_MODE_SINGLE); + if (ret) + return ret; + + indio_dev->name = st->chip_info->name; + indio_dev->info = &ad4695_info; + indio_dev->modes = INDIO_DIRECT_MODE; + + return devm_iio_device_register(dev, indio_dev); +} + +static const struct spi_device_id ad4695_spi_id_table[] = { + { .name = "ad4695", .driver_data = (kernel_ulong_t)&ad4695_chip_info }, + { .name = "ad4696", .driver_data = (kernel_ulong_t)&ad4696_chip_info }, + { .name = "ad4697", .driver_data = (kernel_ulong_t)&ad4697_chip_info }, + { .name = "ad4698", .driver_data = (kernel_ulong_t)&ad4698_chip_info }, + { } +}; +MODULE_DEVICE_TABLE(spi, ad4695_spi_id_table); + +static const struct of_device_id ad4695_of_match_table[] = { + { .compatible = "adi,ad4695", .data = &ad4695_chip_info, }, + { .compatible = "adi,ad4696", .data = &ad4696_chip_info, }, + { .compatible = "adi,ad4697", .data = &ad4697_chip_info, }, + { .compatible = "adi,ad4698", .data = &ad4698_chip_info, }, + { } +}; +MODULE_DEVICE_TABLE(of, ad4695_of_match_table); + +static struct spi_driver ad4695_driver = { + .driver = { + .name = "ad4695", + .of_match_table = ad4695_of_match_table, + }, + .probe = ad4695_probe, + .id_table = ad4695_spi_id_table, +}; +module_spi_driver(ad4695_driver); + +MODULE_AUTHOR("Ramona Gradinariu <ramona.gradinariu@analog.com>"); +MODULE_AUTHOR("David Lechner <dlechner@baylibre.com>"); +MODULE_DESCRIPTION("Analog Devices AD4695 ADC driver"); +MODULE_LICENSE("GPL"); -- 2.45.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] iio: adc: ad4695: Add driver for AD4695 and similar ADCs 2024-06-12 19:20 ` [PATCH 2/3] iio: adc: ad4695: Add driver for " David Lechner @ 2024-06-12 19:40 ` David Lechner 2024-06-15 13:10 ` Jonathan Cameron 1 sibling, 0 replies; 18+ messages in thread From: David Lechner @ 2024-06-12 19:40 UTC (permalink / raw) To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc, Ramona Gradinariu On 6/12/24 2:20 PM, David Lechner wrote: > + ad4695_set_ref_voltage(st, st->vref_mv); > + if (ret) > + return ret; > + For some reason, no matter how many times I go over a patch, I always find something as soon as I hit send. This is missing ret = ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] iio: adc: ad4695: Add driver for AD4695 and similar ADCs 2024-06-12 19:20 ` [PATCH 2/3] iio: adc: ad4695: Add driver for " David Lechner 2024-06-12 19:40 ` David Lechner @ 2024-06-15 13:10 ` Jonathan Cameron 1 sibling, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2024-06-15 13:10 UTC (permalink / raw) To: David Lechner Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc, Ramona Gradinariu On Wed, 12 Jun 2024 14:20:41 -0500 David Lechner <dlechner@baylibre.com> wrote: > This is a new driver for Analog Devices Inc. AD4695 and similar ADCs. > The initial driver supports initializing the chip including configuring > all possible LDO and reference voltage sources as well as any possible > voltage input channel wiring configuration. > > Only the 4-wire SPI wiring mode where the CNV pin is tied to the CS pin > is supported at this time. And reading sample data from the ADC can only > be done in direct mode for now. > > Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com> > Signed-off-by: David Lechner <dlechner@baylibre.com> Hi Ramona / David, Comments inline. Big stuff is going to be around the binding so I've not commented much on that as it'll probably change in v2. other stuff all minor. Jonathan > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index 37ac689a0209..5c4d79d4f939 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -7,6 +7,7 @@ > obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o > obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o > obj-$(CONFIG_AD4130) += ad4130.o > +obj-$(CONFIG_AD4695) += ad4695.o > obj-$(CONFIG_AD7091R) += ad7091r-base.o > obj-$(CONFIG_AD7091R5) += ad7091r5.o > obj-$(CONFIG_AD7091R8) += ad7091r8.o > diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c > new file mode 100644 > index 000000000000..6e5a87817c86 > --- /dev/null > +++ b/drivers/iio/adc/ad4695.c > @@ -0,0 +1,804 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * SPI ADC driver for Analog Devices Inc. AD4695 and similar chips > + * > + * https://www.analog.com/en/products/ad4695.html > + * https://www.analog.com/en/products/ad4696.html > + * https://www.analog.com/en/products/ad4697.html > + * https://www.analog.com/en/products/ad4698.html > + * > + * Copyright 2024 Analog Devices Inc. > + * Copyright 2024 BayLibre, SAS > + */ > + > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <linux/compiler.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/gpio/consumer.h> > +#include <linux/iio/iio.h> > +#include <linux/property.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > +#include <linux/units.h> > + > +/* AD4695 registers */ > +#define AD4695_REG_SPI_CONFIG_A 0x0000 A nice way to handle register and field definitions is to use a few spaces to make the difference visible. #define AD4695_REG_SPI_CONFIG_A 0x0000 #define AD4695_SPI_CONFIG_A_SW_RST_MSB > +#define AD4695_REG_SPI_CONFIG_B 0x0001 > +#define AD4695_REG_DEVICE_TYPE 0x0003 > +#define AD4695_REG_SCRATCH_PAD 0x000A > +#define AD4695_REG_VENDOR_L 0x000C > +#define AD4695_REG_VENDOR_H 0x000D > +#define AD4695_REG_LOOP_MODE 0x000E > +#define AD4695_REG_SPI_CONFIG_C 0x0010 > +#define AD4695_REG_SPI_STATUS 0x0011 > +#define AD4695_REG_STATUS 0x0014 > +#define AD4695_REG_ALERT_STATUS1 0x0015 > +#define AD4695_REG_ALERT_STATUS2 0x0016 > +#define AD4695_REG_CLAMP_STATUS 0x001A > +#define AD4695_REG_SETUP 0x0020 > +#define AD4695_REG_REF_CTRL 0x0021 > +#define AD4695_REG_SEQ_CTRL 0x0022 > +#define AD4695_REG_AC_CTRL 0x0023 > +#define AD4695_REG_STD_SEQ_CONFIG 0x0024 > +#define AD4695_REG_GPIO_CTRL 0x0026 > +#define AD4695_REG_GP_MODE 0x0027 > +#define AD4695_REG_TEMP_CTRL 0x0029 > +#define AD4695_REG_CONFIG_IN(n) (0x0030 | (n)) > +#define AD4695_REG_UPPER_IN(n) (0x0040 | (2 * (n))) > +#define AD4695_REG_LOWER_IN(n) (0x0060 | (2 * (n))) > +#define AD4695_REG_HYST_IN(n) (0x0080 | (2 * (n))) > +#define AD4695_REG_OFFSET_IN(n) (0x00A0 | (2 * (n))) > +#define AD4695_REG_GAIN_IN(n) (0x00C0 | (2 * (n))) > +#define AD4695_REG_AS_SLOT(n) (0x0100 | (n)) > +#define AD4695_REG_MAX 0x017F > + > +/* Conversion mode commands */ > +#define AD4695_CMD_EXIT_CNV_MODE 0x0A > +#define AD4695_CMD_TEMP_CHAN 0x0F > +#define AD4695_CMD_VOLTAGE_CHAN(n) (0x10 | (n)) > + > +/* SPI_CONFIG_A */ > +#define AD4695_SW_RST_MSB BIT(7) > +#define AD4695_SW_RST_LSB BIT(0) Do these have separate usecases? Looks a bit like a magic value to me used to trigger a reset. If so just use one define with both bits set. > + > +/* SPI_CONFIG_B */ > +#define AD4695_INST_MODE_MASK BIT(7) I'm very keen on field defintions including the register so it's immediately obvious you at least got a plausible one. That does sometime make it useful to abreviate register names more. Also good to keep these next to the register address. Using a bit of white space can help to differentiate register fields from the register address. #define AD4695_SPI_CONFB_INST_MODE_MASK > +#define AD4695_INST_MODE(x) FIELD_PREP(AD4695_INST_MODE_MASK, (x)) I'd just call FIELD_PREP inline. It's a few more characters but it makes it immediately obvious this is filling the field rather than getting it which you can't tell from the short version. Same for all the other cases of this. Masks + values the field can take (where there is more than 0 or 1) should be enough. > + > +enum ad4695_instr_mode { > + AD4695_INST_MODE_STREAM, > + AD4695_INST_MODE_SINGLE, > +}; > + > +/* Setup */ > +#define AD4695_LDO_EN_MASK BIT(4) > +#define AD4695_LDO_EN(x) FIELD_PREP(AD4695_LDO_EN_MASK, (((x) ? 1 : 0))) > +#define AD4695_SPI_MODE_MASK BIT(2) > +#define AD4695_SPI_MODE(x) FIELD_PREP(AD4695_SPI_MODE_MASK, (x)) > +#define AD4695_SPI_CYC_CTRL_MASK BIT(1) > +#define AD4695_SPI_CYC_CTRL(x) FIELD_PREP(AD4695_SPI_CYC_CTRL_MASK, (x)) > + > +/* REF_CTRL */ > +#define AD4695_OV_MODE_MASK BIT(7) > +#define AD4695_OV_MODE(x) FIELD_PREP(AD4695_OV_MODE_MASK, (x)) > +#define AD4695_VREF_SET_MASK GENMASK(4, 2) > +#define AD4695_VREF_SET(x) FIELD_PREP(AD4695_VREF_SET_MASK, (x)) > +#define AD4695_REFHIZ_EN_MASK BIT(1) > +#define AD4695_REFHIZ_EN(x) FIELD_PREP(AD4695_REFHIZ_EN_MASK, (((x) ? 1 : 0))) > +#define AD4695_REFBUF_EN_MASK BIT(0) > +#define AD4695_REFBUF_EN(x) FIELD_PREP(AD4695_REFBUF_EN_MASK, (((x) ? 1 : 0))) > + > +enum ad4695_ov_mode { > + AD4695_OV_MODE_REDUCE_CURRENT, > + AD4695_OV_MODE_DO_NOT_REDUCE_CURRENT, > +}; > + > +/* SEQ_CTRL */ > +#define AD4695_STD_SEQ_EN_MASK BIT(7) > +#define AD4695_STD_SEQ_EN(x) FIELD_PREP(AD4695_STD_SEQ_EN_MASK, (((x) ? 1 : 0))) > +#define AD4695_NUM_SLOTS_AS_MASK GENMASK(6, 0) > +#define AD4695_NUM_SLOTS_AS(x) FIELD_PREP(AD4695_NUM_SLOTS_AS_MASK, (x)) > + > +/* CONFIG_INn */ > +#define AD4695_IN_MODE_MASK BIT(6) > +#define AD4695_IN_MODE(x) FIELD_PREP(AD4695_IN_MODE_MASK, (((x) ? 1 : 0))) > +#define AD4695_IN_PAIR_MASK GENMASK(5, 4) > +#define AD4695_IN_PAIR(x) FIELD_PREP(AD4695_IN_PAIR_MASK, (x)) > +#define AD4695_AINHIGHZ_EN_MASK BIT(3) > +#define AD4695_AINHIGHZ_EN(x) FIELD_PREP(AD4695_AINHIGHZ_EN_MASK, (((x) ? 1 : 0))) > + > +enum ad4695_in_pair { > + AD4695_WITH_REFGND, > + AD4695_WITH_COM, > + AD4695_EVEN_ODD, > +}; > + > +/* AS_SLOTn */ > +#define AD4695_SLOT_INX_MASK GENMASK(3, 0) > +#define AD4695_SLOT_INX(x) FIELD_PREP(AD4695_SLOT_INX_MASK, (x)) > +/* register convenience functions */ Drop 'code structure' comments. The bit rot badly as code evolves and the reader ought to be able to figure out the structure from the code. > + > +static int ad4695_soft_reset(struct ad4695_state *st) > +{ > + int ret; > + > + ret = regmap_set_bits(st->regmap, AD4695_REG_SPI_CONFIG_A, > + AD4695_SW_RST_MSB | AD4695_SW_RST_LSB); > + if (ret) > + return ret; > + > + /* datasheet says we have to wait before communicating again */ > + msleep(AD4695_T_WAKEUP_SW_MS); > + > + return 0; > +} > + > +static int ad4695_set_instr_mode(struct ad4695_state *st, > + enum ad4695_instr_mode mode) > +{ > + return regmap_update_bits(st->regmap, AD4695_REG_SPI_CONFIG_B, > + AD4695_INST_MODE_MASK, AD4695_INST_MODE(mode)); So far this wrapper isn't adding anything over just calling the regmap_update_bits directly. So I'd get rid of the wrapper > +} > + > +static int ad4695_set_ldo_en(struct ad4695_state *st, bool enable) > +{ > + return regmap_update_bits(st->regmap, AD4695_REG_SETUP, > + AD4695_LDO_EN_MASK, AD4695_LDO_EN(enable)); As above. This is pretty self explanatory as a regmap call so why wrap it? > +} > + > +/** > + * ad4695_set_single_cycle_mode - Set the device in single cycle mode > + * @st: The AD4695 state > + * @channel: The first channel to read > + * > + * As per the datasheet, to enable single cycle mode, we need to set > + * STD_SEQ_EN=0, NUM_SLOTS_AS=0 and CYC_CTRL=1 (Table 15). Setting SPI_MODE=1 > + * triggers the first conversion using the channel in AS_SLOT0. > + * > + * Context: can sleep, must be called with iio_device_claim_direct held > + * Return: 0 on success, a negative error code on failure > + */ > +static int ad4695_set_single_cycle_mode(struct ad4695_state *st, > + unsigned int channel) > +{ > + int ret; This sort of wrapper of more complex behaviour makes sense - particularly as it provides somewhere for documentation. So keep this one. > + > + ret = regmap_clear_bits(st->regmap, AD4695_REG_SEQ_CTRL, > + AD4695_STD_SEQ_EN_MASK | AD4695_NUM_SLOTS_AS_MASK); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD4695_REG_AS_SLOT(0), > + AD4695_SLOT_INX(channel)); > + if (ret) > + return ret; > + > + return regmap_set_bits(st->regmap, AD4695_REG_SETUP, > + AD4695_SPI_MODE_MASK | AD4695_SPI_CYC_CTRL_MASK); > +} > + > +static int ad4695_set_refhiz_en(struct ad4695_state *st, bool enable) > +{ > + return regmap_update_bits(st->regmap, AD4695_REG_REF_CTRL, > + AD4695_REFHIZ_EN_MASK, AD4695_REFHIZ_EN(enable)); These wrappers don't add that much in my opinion. I'd drop them and just call the regmap functions directly. > +} > + > +static int ad4695_set_refbuf_en(struct ad4695_state *st, bool enable) > +{ > + return regmap_update_bits(st->regmap, AD4695_REG_REF_CTRL, > + AD4695_REFBUF_EN_MASK, AD4695_REFBUF_EN(enable)); > +} > + > +static int ad4695_write_chn_cfg(struct ad4695_state *st, > + struct ad4695_channel_config *cfg) > +{ > + u32 mask = 0, val = 0; > + > + mask |= AD4695_IN_MODE_MASK; > + val |= AD4695_IN_MODE(cfg->bipolar); > + > + mask |= AD4695_IN_PAIR_MASK; > + val |= AD4695_IN_PAIR(cfg->pin_pairing); > + > + mask |= AD4695_AINHIGHZ_EN_MASK; > + val |= AD4695_AINHIGHZ_EN(cfg->highz_en); > + > + return regmap_update_bits(st->regmap, AD4695_REG_CONFIG_IN(cfg->channel), > + mask, val); > +} > + > + > +/* IIO implementation */ Drop that comment. It is highly likely to be come untrue over time as code gets moved around and brings little benefit. > + > +static int ad4695_debugfs_reg_access(struct iio_dev *indio_dev, unsigned int reg, > + unsigned int writeval, unsigned int *readval) > +{ > + struct ad4695_state *st = iio_priv(indio_dev); > + > + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { Bit odd given you don't support anything else yet, but I guess future proofing is fine. > + if (readval) > + return regmap_read(st->regmap, reg, readval); > + > + return regmap_write(st->regmap, reg, writeval); > + } > + > + unreachable(); > +} > + > +static const struct iio_info ad4695_info = { > + .read_raw = &ad4695_read_raw, > + .debugfs_reg_access = &ad4695_debugfs_reg_access, > +}; > + > +static int ad4695_parse_channel_cfg(struct iio_dev *indio_dev) > +{ > + struct device *dev = indio_dev->dev.parent; > + struct ad4695_state *st = iio_priv(indio_dev); > + struct iio_chan_spec *iio_chan_arr; > + struct ad4695_channel_config *chan_cfg_arr; > + unsigned int channel, chan_idx = 0; Trivial: Use a separate line for the one that gets initialised > + int ret; > + ... > +static int ad4695_probe(struct spi_device *spi) > +{ ... > + ret = ad4695_parse_channel_cfg(indio_dev); > + if (ret) > + return ret; > + > + ret = ad4695_set_instr_mode(st, AD4695_INST_MODE_SINGLE); I comment on these above. So far I'm not seeing an advantage to the wrappers. Maybe they will gain locking or something later but for now they aren't justified. > + if (ret) > + return ret; > + > + indio_dev->name = st->chip_info->name; > + indio_dev->info = &ad4695_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + return devm_iio_device_register(dev, indio_dev); > +} ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] Documentation: iio: Document ad4695 driver 2024-06-12 19:20 [PATCH 0/3] iio: adc: ad4695: new driver for AD4695 and similar ADCs David Lechner 2024-06-12 19:20 ` [PATCH 1/3] dt-bindings: iio: adc: add " David Lechner 2024-06-12 19:20 ` [PATCH 2/3] iio: adc: ad4695: Add driver for " David Lechner @ 2024-06-12 19:20 ` David Lechner 2024-06-15 13:12 ` Jonathan Cameron 2 siblings, 1 reply; 18+ messages in thread From: David Lechner @ 2024-06-12 19:20 UTC (permalink / raw) To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: David Lechner, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc The Analog Devices Inc. AD4695 (and similar chips) are complex ADCs that will benefit from a detailed driver documentation. This documents the current features supported by the driver. Signed-off-by: David Lechner <dlechner@baylibre.com> --- Documentation/iio/ad4695.rst | 145 +++++++++++++++++++++++++++++++++++++++++++ Documentation/iio/index.rst | 1 + MAINTAINERS | 1 + 3 files changed, 147 insertions(+) diff --git a/Documentation/iio/ad4695.rst b/Documentation/iio/ad4695.rst new file mode 100644 index 000000000000..6e142561524e --- /dev/null +++ b/Documentation/iio/ad4695.rst @@ -0,0 +1,145 @@ +.. SPDX-License-Identifier: GPL-2.0-only + +============= +AD4695 driver +============= + +ADC driver for Analog Devices Inc. AD4695 and similar devices. The module name +is ``ad4695``. + + +Supported devices +================= + +The following chips are supported by this driver: + +* `AD4695 <https://www.analog.com/AD4695>`_ +* `AD4696 <https://www.analog.com/AD4696>`_ +* `AD4697 <https://www.analog.com/AD4697>`_ +* `AD4698 <https://www.analog.com/AD4698>`_ + + +Supported features +================== + +SPI wiring modes +---------------- + +The driver currently supports the following SPI wiring configuration: + +4-wire mode +^^^^^^^^^^^ + +In this mode, CNV and CS are tied together and there is a single SDO line. + +.. code-block:: + + +-------------+ +-------------+ + | CS |<-+------| CS | + | CNV |<-+ | | + | ADC | | HOST | + | | | | + | SDI |<--------| SDO | + | SDO |-------->| SDI | + | SCLK |<--------| SCLK | + +-------------+ +-------------+ + +To use this mode, in the device tree, omit the ``cnv-gpios`` and +``spi-rx-bus-width`` properties. + +Channel configuration +--------------------- + +Since the chip supports multiple ways to configure each channel, this must be +described in the device tree based on what is actually wired up to the inputs. + +There are three typical configurations: + +Single-ended where a pin is used with the ``REFGND`` pin, pseudo-differential +where a pin is used with the ``COM`` pin and differential where two ``INx`` +pins are used as a pair + +Single-ended input +^^^^^^^^^^^^^^^^^^ + +Each ``INx`` pin can be used as a single-ended input in conjunction with the +``REFGND`` pin. The device tree will look like this: + +.. code-block:: + + channel@2 { + reg = <2>; + }; + +This will appear on the IIO bus as the ``voltage2`` channel. The processed value +(*raw × scale*) will be the voltage between the ``INx`` relative to ``REFGND``. +(Offset is always 0 when pairing with ``REFGND``.) + +Pseudo-differential input +^^^^^^^^^^^^^^^^^^^^^^^^^ + +Each ``INx`` pin can be used as a pseudo-differential input in conjunction with +the ``COM`` pin. The device tree will look like this: + +.. code-block:: + + com-supply = <&vref_div_2>; + + channel@3 { + reg = <3>; + adi,pin-pairing = "com"; + bipolar; + }; + +This will appear on the IIO bus as the ``voltage3`` channel. The processed value +(*(raw + offset) × scale*) will be the voltage measured on ``INx`` relative to +``REFGND``. (The offset is determined by the ``com-supply`` voltage.) + +Differential input +^^^^^^^^^^^^^^^^^^ + +An even-numbered ``INx`` pin and the following odd-numbered ``INx`` pin can be +used as a differential pair. The device tree for using ``IN0`` as the positive +input and ``IN1`` as the negative input will look like this: + +.. code-block:: + + channel@0 { + reg = <0>; + adi,pin-pairing = "next"; + bipolar; + }; + +This will appear on the IIO bus as the ``voltage0-voltage1`` channel. The +processed value (*raw × scale*) will be the voltage difference between the two +pins. (Offset is always 0 for differential channels.) + +VCC supply +---------- + +The chip supports being powered by an external LDO via the ``VCC`` input or an +internal LDO via the ``LDO_IN`` input. The driver looks at the device tree to +determine which is being used. If ``ldo-supply`` is present, then the internal +LDO is used. If ``vcc-supply`` is present, then the external LDO is used and +the internal LDO is disabled. + +Reference voltage +----------------- + +The chip supports an external reference voltage via the ``REF`` input or an +internal buffered reference voltage via the ``REFIN`` input. The driver looks +at the device tree to determine which is being used. If ``ref-supply`` is +present, then the external reference voltage is used and the internal buffer is +disabled. If ``refin-supply`` is present, then the internal buffered reference +voltage is used. + +Unimplemented features +---------------------- + +- Additional wiring modes +- Buffered reads +- Threshold events +- Oversampling +- Gain/offset calibration +- GPIO support +- CRC support diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst index 4c13bfa2865c..df69a76bf583 100644 --- a/Documentation/iio/index.rst +++ b/Documentation/iio/index.rst @@ -17,6 +17,7 @@ Industrial I/O Kernel Drivers .. toctree:: :maxdepth: 1 + ad4695 ad7944 adis16475 adis16480 diff --git a/MAINTAINERS b/MAINTAINERS index 611b7929e650..edd1a4e8f538 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1198,6 +1198,7 @@ L: linux-iio@vger.kernel.org S: Supported W: https://ez.analog.com/linux-software-drivers F: Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml +F: Documentation/iio/index.rst F: drivers/iio/dac/ad3552r.c ANALOG DEVICES INC AD4130 DRIVER -- 2.45.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] Documentation: iio: Document ad4695 driver 2024-06-12 19:20 ` [PATCH 3/3] Documentation: iio: Document ad4695 driver David Lechner @ 2024-06-15 13:12 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2024-06-15 13:12 UTC (permalink / raw) To: David Lechner Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On Wed, 12 Jun 2024 14:20:42 -0500 David Lechner <dlechner@baylibre.com> wrote: > The Analog Devices Inc. AD4695 (and similar chips) are complex ADCs that > will benefit from a detailed driver documentation. > > This documents the current features supported by the driver. > > Signed-off-by: David Lechner <dlechner@baylibre.com> LGTM. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2024-06-15 17:55 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-12 19:20 [PATCH 0/3] iio: adc: ad4695: new driver for AD4695 and similar ADCs David Lechner 2024-06-12 19:20 ` [PATCH 1/3] dt-bindings: iio: adc: add " David Lechner 2024-06-13 7:11 ` Krzysztof Kozlowski 2024-06-13 13:57 ` David Lechner 2024-06-13 14:18 ` Krzysztof Kozlowski 2024-06-13 14:39 ` David Lechner 2024-06-13 15:11 ` Nuno Sá 2024-06-13 19:43 ` Rob Herring 2024-06-13 20:09 ` David Lechner 2024-06-15 12:25 ` Jonathan Cameron 2024-06-15 12:41 ` Jonathan Cameron 2024-06-15 15:23 ` David Lechner 2024-06-15 17:55 ` Jonathan Cameron 2024-06-12 19:20 ` [PATCH 2/3] iio: adc: ad4695: Add driver for " David Lechner 2024-06-12 19:40 ` David Lechner 2024-06-15 13:10 ` Jonathan Cameron 2024-06-12 19:20 ` [PATCH 3/3] Documentation: iio: Document ad4695 driver David Lechner 2024-06-15 13:12 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).