* [PATCH v2 0/4] iio: adc: ad4695: new driver for AD4695 and similar ADCs
@ 2024-06-17 19:53 David Lechner
2024-06-17 19:53 ` [PATCH v2 1/4] dt-bindings: iio: adc: add common-mode-channel dependency David Lechner
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: David Lechner @ 2024-06-17 19:53 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.
---
Changes in v2:
[PATCH 1/1]
* New patch
* Depends on recently applied patch
https://lore.kernel.org/linux-iio/20240607-ad4111-v7-1-97e3855900a0@analog.com/
[PATCH 1/2]
* Drop *-wlcsp compatible strings
* Don't use fallback compatible strings
* Reword supply descriptions
* Use standard channel properties instead of adi,pin-pairing
* Fix unnecessary | character
* Fix missing blank line
* Add header file with common mode channel macros
[PATCH 1/3]
* rework register definition macros
* remove code structure comments at top level
* remove simple wrapper functions around regmap functions
* rework channel fwnode parsing for DT changes
* fix missing return value check
[PATCH 1/4]
* Rework DT examples for DT bindings changes
* Add file to MAINTAINERS
* Link to v1: https://lore.kernel.org/r/20240612-iio-adc-ad4695-v1-0-6a4ed251fc86@baylibre.com
---
David Lechner (4):
dt-bindings: iio: adc: add common-mode-channel dependency
dt-bindings: iio: adc: add AD4695 and similar ADCs
iio: adc: ad4695: Add driver for AD4695 and similar ADCs
Documentation: iio: Document ad4695 driver
Documentation/devicetree/bindings/iio/adc/adc.yaml | 3 +
.../devicetree/bindings/iio/adc/adi,ad4695.yaml | 290 ++++++++
Documentation/iio/ad4695.rst | 153 +++++
Documentation/iio/index.rst | 1 +
MAINTAINERS | 12 +
drivers/iio/adc/Kconfig | 11 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4695.c | 753 +++++++++++++++++++++
include/dt-bindings/iio/adi,ad4695.h | 9 +
9 files changed, 1233 insertions(+)
---
base-commit: 9ab0746278aff96c1a36fcaad22bee9e9d3b3bf6
change-id: 20240517-iio-adc-ad4695-ef72b2a2cf88
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v2 1/4] dt-bindings: iio: adc: add common-mode-channel dependency 2024-06-17 19:53 [PATCH v2 0/4] iio: adc: ad4695: new driver for AD4695 and similar ADCs David Lechner @ 2024-06-17 19:53 ` David Lechner 2024-06-18 19:47 ` David Lechner 2024-06-17 19:53 ` [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs David Lechner ` (2 subsequent siblings) 3 siblings, 1 reply; 15+ messages in thread From: David Lechner @ 2024-06-17 19:53 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 common-mode-channel property is only used in conjunction with the single-channel property. This patch adds a dependency so that we will get a validation error if common-mode-channel is used without also specifying single-channel. Signed-off-by: David Lechner <dlechner@baylibre.com> --- v2 changes: * New patch * Depends on recently applied patch [1] [1]: https://lore.kernel.org/linux-iio/20240607-ad4111-v7-1-97e3855900a0@analog.com/ --- Documentation/devicetree/bindings/iio/adc/adc.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml index 8e7835cf36fd..5baef30104dd 100644 --- a/Documentation/devicetree/bindings/iio/adc/adc.yaml +++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml @@ -80,4 +80,7 @@ anyOf: - required: - reg +dependencies: + common-mode-channel: [single-channel] + additionalProperties: true -- 2.45.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: iio: adc: add common-mode-channel dependency 2024-06-17 19:53 ` [PATCH v2 1/4] dt-bindings: iio: adc: add common-mode-channel dependency David Lechner @ 2024-06-18 19:47 ` David Lechner 0 siblings, 0 replies; 15+ messages in thread From: David Lechner @ 2024-06-18 19:47 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 On 6/17/24 2:53 PM, David Lechner wrote: > The common-mode-channel property is only used in conjunction with the > single-channel property. This patch adds a dependency so that we will > get a validation error if common-mode-channel is used without also > specifying single-channel. It turns out, this might not be a valid assumption. See https://lore.kernel.org/linux-iio/20240617-iio-adc-ad4695-v2-1-63ef6583f25d@baylibre.com/T/#m7b921cf9a7deb4a4f02dd2f05668a3dd99232ee9 > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > > v2 changes: > * New patch > * Depends on recently applied patch [1] > > [1]: https://lore.kernel.org/linux-iio/20240607-ad4111-v7-1-97e3855900a0@analog.com/ > --- > Documentation/devicetree/bindings/iio/adc/adc.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml > index 8e7835cf36fd..5baef30104dd 100644 > --- a/Documentation/devicetree/bindings/iio/adc/adc.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml > @@ -80,4 +80,7 @@ anyOf: > - required: > - reg > > +dependencies: > + common-mode-channel: [single-channel] > + > additionalProperties: true > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-17 19:53 [PATCH v2 0/4] iio: adc: ad4695: new driver for AD4695 and similar ADCs David Lechner 2024-06-17 19:53 ` [PATCH v2 1/4] dt-bindings: iio: adc: add common-mode-channel dependency David Lechner @ 2024-06-17 19:53 ` David Lechner 2024-06-17 21:30 ` Rob Herring (Arm) 2024-06-18 19:29 ` David Lechner 2024-06-17 19:53 ` [PATCH v2 3/4] iio: adc: ad4695: Add driver for " David Lechner 2024-06-17 19:53 ` [PATCH v2 4/4] Documentation: iio: Document ad4695 driver David Lechner 3 siblings, 2 replies; 15+ messages in thread From: David Lechner @ 2024-06-17 19:53 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> --- v2 changes: * Drop *-wlcsp compatible strings * Don't use fallback compatible strings * Reword supply descriptions * Use standard channel properties instead of adi,pin-pairing * Fix unnecessary | character * Fix missing blank line * Add header file with common mode channel macros --- .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 290 +++++++++++++++++++++ MAINTAINERS | 10 + include/dt-bindings/iio/adi,ad4695.h | 9 + 3 files changed, 309 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..e5dafb1f6acf --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml @@ -0,0 +1,290 @@ +# 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: + enum: + - adi,ad4695 + - adi,ad4696 + - adi,ad4697 + - adi,ad4698 + + 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: Analog power supply. + + vio-supply: + description: I/O pin power supply. + + ldo-in-supply: + description: Internal LDO Input. Mutually exclusive with vdd-supply. + + vdd-supply: + description: Core power supply. Mutually exclusive with ldo-in-supply. + + ref-supply: + description: + External reference voltage. Mutually exclusive with refin-supply. + + refin-supply: + description: + Internal reference buffer input. Mutually exclusive with ref-supply. + + 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 + + diff-channels: + description: + Describes inputs used for differential channels. The first value must + be an even numbered input and the second value must be the next + consecutive odd numbered input. + items: + - minimum: 0 + maximum: 14 + multipleOf: 2 + - minimum: 1 + maximum: 15 + not: + multipleOf: 2 + + single-channel: + minimum: 0 + maximum: 15 + + common-mode-channel: + description: + Describes the common mode channel for single channels. 0 is REFGND + and 1 is COM. Macros are available for these values in + dt-bindings/iio/adi,ad4695.h. + minimum: 0 + maximum: 1 + default: 0 + + 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 + + oneOf: + - required: + - diff-channels + - required: + - single-channel + + allOf: + # bipolar mode can't be used with REFGND + - if: + required: + - single-channel + not: + required: + - common-mode-channel + then: + properties: + bipolar: false + - if: + required: + - common-mode-channel + properties: + common-mode-channel: + const: 0 + then: + properties: + bipolar: false + +required: + - compatible + - reg + - avdd-supply + - vio-supply + +allOf: + - oneOf: + - required: + - ldo-in-supply + - required: + - vdd-supply + + - oneOf: + - required: + - ref-supply + - required: + - refin-supply + + # 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: + enum: + - adi,ad4697 + - adi,ad4698 + then: + patternProperties: + "^channel@[0-7]$": + properties: + reg: + maximum: 7 + diff-channels: + items: + - maximum: 6 + - maximum: 7 + single-channel: + maximum: 7 + "^channel@[8-9a-f]$": false + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/iio/adi,ad4695.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 = <&power_supply>; + ldo-in-supply = <&power_supply>; + vio-supply = <&io_supply>; + refin-supply = <&supply_5V>; + com-supply = <&supply_2V5>; + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; + + #address-cells = <1>; + #size-cells = <0>; + + /* Differential channel between IN0 and IN1. */ + channel@0 { + reg = <0>; + diff-channels = <0>, <1>; + bipolar; + }; + + /* Single-ended channel between IN2 and REFGND. */ + channel@1 { + reg = <1>; + single-channel = <2>; + }; + + /* Pseudo-differential channel between IN3 and COM. */ + channel@2 { + reg = <2>; + single-channel = <3>; + common-mode-channel = <AD4695_COMMON_MODE_COM>; + bipolar; + }; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index c870bc6b8d63..19d4bc962c77 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1209,6 +1209,16 @@ 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 +F: include/dt-bindings/iio/adi,ad4695.h + ANALOG DEVICES INC AD7091R DRIVER M: Marcelo Schmitt <marcelo.schmitt@analog.com> L: linux-iio@vger.kernel.org diff --git a/include/dt-bindings/iio/adi,ad4695.h b/include/dt-bindings/iio/adi,ad4695.h new file mode 100644 index 000000000000..87a1b94af62c --- /dev/null +++ b/include/dt-bindings/iio/adi,ad4695.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ + +#ifndef _DT_BINDINGS_ADI_AD4695_H +#define _DT_BINDINGS_ADI_AD4695_H + +#define AD4695_COMMON_MODE_REFGND 0 +#define AD4695_COMMON_MODE_COM 1 + +#endif /* _DT_BINDINGS_ADI_AD4695_H */ -- 2.45.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-17 19:53 ` [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs David Lechner @ 2024-06-17 21:30 ` Rob Herring (Arm) 2024-06-17 22:06 ` David Lechner 2024-06-18 19:29 ` David Lechner 1 sibling, 1 reply; 15+ messages in thread From: Rob Herring (Arm) @ 2024-06-17 21:30 UTC (permalink / raw) To: David Lechner Cc: linux-iio, devicetree, Conor Dooley, Jonathan Corbet, Krzysztof Kozlowski, Jonathan Cameron, linux-kernel, linux-doc, Michael Hennerich, Nuno Sá On Mon, 17 Jun 2024 14:53:13 -0500, David Lechner wrote: > Add device tree bindings for AD4695 and similar ADCs. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > > v2 changes: > * Drop *-wlcsp compatible strings > * Don't use fallback compatible strings > * Reword supply descriptions > * Use standard channel properties instead of adi,pin-pairing > * Fix unnecessary | character > * Fix missing blank line > * Add header file with common mode channel macros > --- > .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 290 +++++++++++++++++++++ > MAINTAINERS | 10 + > include/dt-bindings/iio/adi,ad4695.h | 9 + > 3 files changed, 309 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml: single-channel: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml: common-mode-channel: missing type definition doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240617-iio-adc-ad4695-v2-2-63ef6583f25d@baylibre.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-17 21:30 ` Rob Herring (Arm) @ 2024-06-17 22:06 ` David Lechner 2024-06-18 18:00 ` Conor Dooley 0 siblings, 1 reply; 15+ messages in thread From: David Lechner @ 2024-06-17 22:06 UTC (permalink / raw) To: Rob Herring (Arm) Cc: linux-iio, devicetree, Conor Dooley, Jonathan Corbet, Krzysztof Kozlowski, Jonathan Cameron, linux-kernel, linux-doc, Michael Hennerich, Nuno Sá On 6/17/24 4:30 PM, Rob Herring (Arm) wrote: > > On Mon, 17 Jun 2024 14:53:13 -0500, David Lechner wrote: >> Add device tree bindings for AD4695 and similar ADCs. >> >> Signed-off-by: David Lechner <dlechner@baylibre.com> >> --- >> >> v2 changes: >> * Drop *-wlcsp compatible strings >> * Don't use fallback compatible strings >> * Reword supply descriptions >> * Use standard channel properties instead of adi,pin-pairing >> * Fix unnecessary | character >> * Fix missing blank line >> * Add header file with common mode channel macros >> --- >> .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 290 +++++++++++++++++++++ >> MAINTAINERS | 10 + >> include/dt-bindings/iio/adi,ad4695.h | 9 + >> 3 files changed, 309 insertions(+) >> > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml: single-channel: missing type definition > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml: common-mode-channel: missing type definition > > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240617-iio-adc-ad4695-v2-2-63ef6583f25d@baylibre.com > > The base for the series is generally the latest rc1. A different dependency > should be noted in *this* patch. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit after running the above command yourself. Note > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > your schema. However, it must be unset to test all examples with your schema. > I think the problem is that I don't have a well-known commit as the base-commit in my cover letter (oversight on my part). single-channel and common-mode-channel are recent additions to the common iio/adc.yaml so the types are defined there. make dt_binding_check did pass for me locally before sending the series. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-17 22:06 ` David Lechner @ 2024-06-18 18:00 ` Conor Dooley 2024-06-24 14:35 ` Rob Herring 0 siblings, 1 reply; 15+ messages in thread From: Conor Dooley @ 2024-06-18 18:00 UTC (permalink / raw) To: David Lechner Cc: Rob Herring (Arm), linux-iio, devicetree, Conor Dooley, Jonathan Corbet, Krzysztof Kozlowski, Jonathan Cameron, linux-kernel, linux-doc, Michael Hennerich, Nuno Sá [-- Attachment #1: Type: text/plain, Size: 2662 bytes --] On Mon, Jun 17, 2024 at 05:06:29PM -0500, David Lechner wrote: > On 6/17/24 4:30 PM, Rob Herring (Arm) wrote: > > > > On Mon, 17 Jun 2024 14:53:13 -0500, David Lechner wrote: > >> Add device tree bindings for AD4695 and similar ADCs. > >> > >> Signed-off-by: David Lechner <dlechner@baylibre.com> > >> --- > >> > >> v2 changes: > >> * Drop *-wlcsp compatible strings > >> * Don't use fallback compatible strings > >> * Reword supply descriptions > >> * Use standard channel properties instead of adi,pin-pairing > >> * Fix unnecessary | character > >> * Fix missing blank line > >> * Add header file with common mode channel macros > >> --- > >> .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 290 +++++++++++++++++++++ > >> MAINTAINERS | 10 + > >> include/dt-bindings/iio/adi,ad4695.h | 9 + > >> 3 files changed, 309 insertions(+) > >> > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > yamllint warnings/errors: > > > > dtschema/dtc warnings/errors: > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml: single-channel: missing type definition > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml: common-mode-channel: missing type definition > > > > doc reference errors (make refcheckdocs): > > > > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240617-iio-adc-ad4695-v2-2-63ef6583f25d@baylibre.com > > > > The base for the series is generally the latest rc1. A different dependency > > should be noted in *this* patch. > > > > If you already ran 'make dt_binding_check' and didn't see the above > > error(s), then make sure 'yamllint' is installed and dt-schema is up to > > date: > > > > pip3 install dtschema --upgrade > > > > Please check and re-submit after running the above command yourself. Note > > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > > your schema. However, it must be unset to test all examples with your schema. > > > > I think the problem is that I don't have a well-known commit as the > base-commit in my cover letter (oversight on my part). I think for his bot it needs, as written above, to be "in *this* patch". I'm not sure if that's to allow for manual review, or something the bot does automagically however. > single-channel and common-mode-channel are recent additions to the > common iio/adc.yaml so the types are defined there. > > make dt_binding_check did pass for me locally before sending the series. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-18 18:00 ` Conor Dooley @ 2024-06-24 14:35 ` Rob Herring 0 siblings, 0 replies; 15+ messages in thread From: Rob Herring @ 2024-06-24 14:35 UTC (permalink / raw) To: Conor Dooley Cc: David Lechner, linux-iio, devicetree, Conor Dooley, Jonathan Corbet, Krzysztof Kozlowski, Jonathan Cameron, linux-kernel, linux-doc, Michael Hennerich, Nuno Sá On Tue, Jun 18, 2024 at 07:00:03PM +0100, Conor Dooley wrote: > On Mon, Jun 17, 2024 at 05:06:29PM -0500, David Lechner wrote: > > On 6/17/24 4:30 PM, Rob Herring (Arm) wrote: > > > > > > On Mon, 17 Jun 2024 14:53:13 -0500, David Lechner wrote: > > >> Add device tree bindings for AD4695 and similar ADCs. > > >> > > >> Signed-off-by: David Lechner <dlechner@baylibre.com> > > >> --- > > >> > > >> v2 changes: > > >> * Drop *-wlcsp compatible strings > > >> * Don't use fallback compatible strings > > >> * Reword supply descriptions > > >> * Use standard channel properties instead of adi,pin-pairing > > >> * Fix unnecessary | character > > >> * Fix missing blank line > > >> * Add header file with common mode channel macros > > >> --- > > >> .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 290 +++++++++++++++++++++ > > >> MAINTAINERS | 10 + > > >> include/dt-bindings/iio/adi,ad4695.h | 9 + > > >> 3 files changed, 309 insertions(+) > > >> > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > > > yamllint warnings/errors: > > > > > > dtschema/dtc warnings/errors: > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml: single-channel: missing type definition > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml: common-mode-channel: missing type definition > > > > > > doc reference errors (make refcheckdocs): > > > > > > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240617-iio-adc-ad4695-v2-2-63ef6583f25d@baylibre.com > > > > > > The base for the series is generally the latest rc1. A different dependency > > > should be noted in *this* patch. > > > > > > If you already ran 'make dt_binding_check' and didn't see the above > > > error(s), then make sure 'yamllint' is installed and dt-schema is up to > > > date: > > > > > > pip3 install dtschema --upgrade > > > > > > Please check and re-submit after running the above command yourself. Note > > > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > > > your schema. However, it must be unset to test all examples with your schema. > > > > > > > I think the problem is that I don't have a well-known commit as the > > base-commit in my cover letter (oversight on my part). > > I think for his bot it needs, as written above, to be "in *this* patch". > I'm not sure if that's to allow for manual review, or something the bot > does automagically however. I used to do manual review before sending, but then I added co-maintainers that review patches before the bot emails got sent out. So now they are sent out as run without review and dependencies aren't dealt with at all. But yes, put something in this patch to say failures are expected because of a dependency. Rob ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-17 19:53 ` [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs David Lechner 2024-06-17 21:30 ` Rob Herring (Arm) @ 2024-06-18 19:29 ` David Lechner 2024-06-23 16:39 ` Jonathan Cameron 1 sibling, 1 reply; 15+ messages in thread From: David Lechner @ 2024-06-18 19:29 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 On 6/17/24 2:53 PM, David Lechner wrote: > Add device tree bindings for AD4695 and similar ADCs. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > ... > + > + 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 > + Since the interrupt can come from two different pins, it seems like we would need an extra property to specify this. Is there a standard way to do this? Otherwise I will add something like: adi,busy-on-gp3: $ref: /schemas/types.yaml#/definitions/flag description: When present, the busy interrupt is coming from the GP3 pin, otherwise the interrupt is coming from the BSY_ALT_GP0 pin. adi,alert-on-gp2: $ref: /schemas/types.yaml#/definitions/flag description: When present, the alert interrupt is coming from the GP2 pin, otherwise the interrupt is coming from the BSY_ALT_GP0 pin. > + > +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 > + > + diff-channels: > + description: > + Describes inputs used for differential channels. The first value must > + be an even numbered input and the second value must be the next > + consecutive odd numbered input. > + items: > + - minimum: 0 > + maximum: 14 > + multipleOf: 2 > + - minimum: 1 > + maximum: 15 > + not: > + multipleOf: 2 After some more testing, it turns out that I misunderstood the datasheet and this isn't actually fully differential, but rather pseudo-differential. So when pairing with the next pin, it is similar to pairing with the COM pin where the negative input pin is connected to a constant voltage source. > + > + single-channel: > + minimum: 0 > + maximum: 15 > + > + common-mode-channel: > + description: > + Describes the common mode channel for single channels. 0 is REFGND > + and 1 is COM. Macros are available for these values in > + dt-bindings/iio/adi,ad4695.h. > + minimum: 0 > + maximum: 1 > + default: 0 So I'm thinking the right thing to do here go back to using reg and the INx number and only have common-mode-channel (no diff-channels or single-channel). common-mode-channel will need to be changed to allow INx numbers in addition to COM and REFGND. This means that [PATCH v2 1/4] "dt-bindings: iio: adc: add common-mode-channel dependency" would be wrong since we would be using common-mode-channel without single-channel. It also means we will need an optional in1-supply: true for all odd numbered inputs. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-18 19:29 ` David Lechner @ 2024-06-23 16:39 ` Jonathan Cameron 2024-06-24 14:36 ` David Lechner 0 siblings, 1 reply; 15+ messages in thread From: Jonathan Cameron @ 2024-06-23 16:39 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 Tue, 18 Jun 2024 14:29:10 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 6/17/24 2:53 PM, David Lechner wrote: > > Add device tree bindings for AD4695 and similar ADCs. > > > > Signed-off-by: David Lechner <dlechner@baylibre.com> > > --- > > > ... > > > + > > + 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 > > + > > Since the interrupt can come from two different pins, it seems like we would > need an extra property to specify this. Is there a standard way to do this? > > Otherwise I will add something like: > > adi,busy-on-gp3: > $ref: /schemas/types.yaml#/definitions/flag > description: > When present, the busy interrupt is coming from the GP3 pin, otherwise > the interrupt is coming from the BSY_ALT_GP0 pin. > > adi,alert-on-gp2: > $ref: /schemas/types.yaml#/definitions/flag > description: > When present, the alert interrupt is coming from the GP2 pin, otherwise > the interrupt is coming from the BSY_ALT_GP0 pin. Cut and paste? Or it ends up on the same pin as the bsy? In which case that's a single interrupt and it's up to software to decide how to use. I'll guess it comes on GP1? > More interrupt names. We shouldn't restrict someone wiring all 4 if they want to - we'll just use 2 that we choose in the driver. interrupt-names minItems: 1 items: - const: busy-gp0 - const: busy-gp1 - const: alert-gp2 - cosnt: alert-gp1 T > > > + > > +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 > > + > > + diff-channels: > > + description: > > + Describes inputs used for differential channels. The first value must > > + be an even numbered input and the second value must be the next > > + consecutive odd numbered input. > > + items: > > + - minimum: 0 > > + maximum: 14 > > + multipleOf: 2 > > + - minimum: 1 > > + maximum: 15 > > + not: > > + multipleOf: 2 > > After some more testing, it turns out that I misunderstood the datasheet and > this isn't actually fully differential, but rather pseudo-differential. > > So when pairing with the next pin, it is similar to pairing with the COM pin > where the negative input pin is connected to a constant voltage source. Ok. I'm curious, how does it actually differ from a differential channel? What was that test? It doesn't cope with an actual differential pair and needs a stable value on the negative? > > > + > > + single-channel: > > + minimum: 0 > > + maximum: 15 > > + > > + common-mode-channel: > > + description: > > + Describes the common mode channel for single channels. 0 is REFGND > > + and 1 is COM. Macros are available for these values in > > + dt-bindings/iio/adi,ad4695.h. > > + minimum: 0 > > + maximum: 1 > > + default: 0 > > So I'm thinking the right thing to do here go back to using reg and the INx > number and only have common-mode-channel (no diff-channels or single-channel). > > common-mode-channel will need to be changed to allow INx numbers in addition > to COM and REFGND. > > This means that [PATCH v2 1/4] "dt-bindings: iio: adc: add common-mode-channel > dependency" would be wrong since we would be using common-mode-channel without > single-channel. > > It also means we will need an optional in1-supply: true for all odd numbered > inputs. Ok. I'm not totally sure I see how this comes together but will wait for v3 rather than trying to figure it out now. Jonathan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-23 16:39 ` Jonathan Cameron @ 2024-06-24 14:36 ` David Lechner 2024-06-24 17:52 ` Jonathan Cameron 0 siblings, 1 reply; 15+ messages in thread From: David Lechner @ 2024-06-24 14:36 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/23/24 11:39 AM, Jonathan Cameron wrote: > On Tue, 18 Jun 2024 14:29:10 -0500 > David Lechner <dlechner@baylibre.com> wrote: > >> On 6/17/24 2:53 PM, David Lechner wrote: >>> Add device tree bindings for AD4695 and similar ADCs. >>> >>> Signed-off-by: David Lechner <dlechner@baylibre.com> >>> --- >>> >> ... >> >>> + >>> + 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 >>> + >> >> Since the interrupt can come from two different pins, it seems like we would >> need an extra property to specify this. Is there a standard way to do this? >> >> Otherwise I will add something like: >> >> adi,busy-on-gp3: >> $ref: /schemas/types.yaml#/definitions/flag >> description: >> When present, the busy interrupt is coming from the GP3 pin, otherwise >> the interrupt is coming from the BSY_ALT_GP0 pin. >> >> adi,alert-on-gp2: >> $ref: /schemas/types.yaml#/definitions/flag >> description: >> When present, the alert interrupt is coming from the GP2 pin, otherwise >> the interrupt is coming from the BSY_ALT_GP0 pin. > Cut and paste? Or it ends up on the same pin as the bsy? In which case that's > a single interrupt and it's up to software to decide how to use. I'll guess > it comes on GP1? This is not a typo. The BSY_ALT_GP0 is a multi-purpose pin. The actual function of the pin isn't selected explicitly, but rather there is an order of priority (Table 25 in the datasheet). Also, there are two packages the chip can come in, LFCSP and WLCSP. The former only has GP0 and not GP1/2/3. My thinking was that if both interrupts are provided in the DT and neither adi,busy-on-gp3 or adi,alert-on-gp2 is given, then the driver would use a shared interrupt and only allow enabling one function alert or busy at a time. >> > > More interrupt names. We shouldn't restrict someone wiring all 4 if they want > to - we'll just use 2 that we choose in the driver. > > interrupt-names > minItems: 1 > items: > - const: busy-gp0 > - const: busy-gp1 > - const: alert-gp2 > - cosnt: alert-gp1 This would actually need to be: interrupt-names minItems: 1 items: - const: busy-gp0 - const: busy-gp3 - const: alert-gp0 - cosnt: alert-gp2 Or would it need to be this since there are two possible signals on the same pin rather than trying to mess with a shared interrupt? (Note: if both alert and busy are enabled at the same time on GP0, we will only get the alert signal and busy signal is masked). interrupt-names minItems: 1 items: - const: alert-busy-gp0 - const: busy-gp3 - cosnt: alert-gp2 > > T >> >>> + >>> +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 >>> + >>> + diff-channels: >>> + description: >>> + Describes inputs used for differential channels. The first value must >>> + be an even numbered input and the second value must be the next >>> + consecutive odd numbered input. >>> + items: >>> + - minimum: 0 >>> + maximum: 14 >>> + multipleOf: 2 >>> + - minimum: 1 >>> + maximum: 15 >>> + not: >>> + multipleOf: 2 >> >> After some more testing, it turns out that I misunderstood the datasheet and >> this isn't actually fully differential, but rather pseudo-differential. >> >> So when pairing with the next pin, it is similar to pairing with the COM pin >> where the negative input pin is connected to a constant voltage source. > > Ok. I'm curious, how does it actually differ from a differential channel? > What was that test? It doesn't cope with an actual differential pair and needs > a stable value on the negative? In my initial testing, since I was only doing a direct read, I was using constant voltages. But when I started working on buffered reads, then I saw noisy data when using a fully differential (antiphase) signal. This chip uses a multiplexer for channel so when an odd number pin is used as the positive input (paired with REFGND or COM), it goes through one multiplexer, but when an odd number pin is used as the negative input it goes through the other multiplexer - the same one as REFGND and COM. And burred in the middle of a paragraph on page 34 of 110 of the datasheet is the only place in the entire datasheet where it actually says this is a pseudo differential chip. > >> >>> + >>> + single-channel: >>> + minimum: 0 >>> + maximum: 15 >>> + >>> + common-mode-channel: >>> + description: >>> + Describes the common mode channel for single channels. 0 is REFGND >>> + and 1 is COM. Macros are available for these values in >>> + dt-bindings/iio/adi,ad4695.h. >>> + minimum: 0 >>> + maximum: 1 >>> + default: 0 >> >> So I'm thinking the right thing to do here go back to using reg and the INx >> number and only have common-mode-channel (no diff-channels or single-channel). >> >> common-mode-channel will need to be changed to allow INx numbers in addition >> to COM and REFGND. >> >> This means that [PATCH v2 1/4] "dt-bindings: iio: adc: add common-mode-channel >> dependency" would be wrong since we would be using common-mode-channel without >> single-channel. >> >> It also means we will need an optional in1-supply: true for all odd numbered >> inputs. > Ok. I'm not totally sure I see how this comes together but will wait for v3 rather > than trying to figure it out now. > > Jonathan > > It should end up like other pseudo differential chips we have done recently. I've got it worked out, so you'll be seeing it soon enough anyway. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs 2024-06-24 14:36 ` David Lechner @ 2024-06-24 17:52 ` Jonathan Cameron 0 siblings, 0 replies; 15+ messages in thread From: Jonathan Cameron @ 2024-06-24 17:52 UTC (permalink / raw) To: David Lechner Cc: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio, devicetree, linux-kernel, linux-doc On Mon, 24 Jun 2024 09:36:43 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 6/23/24 11:39 AM, Jonathan Cameron wrote: > > On Tue, 18 Jun 2024 14:29:10 -0500 > > David Lechner <dlechner@baylibre.com> wrote: > > > >> On 6/17/24 2:53 PM, David Lechner wrote: > >>> Add device tree bindings for AD4695 and similar ADCs. > >>> > >>> Signed-off-by: David Lechner <dlechner@baylibre.com> > >>> --- > >>> > >> ... > >> > >>> + > >>> + 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 > >>> + > >> > >> Since the interrupt can come from two different pins, it seems like we would > >> need an extra property to specify this. Is there a standard way to do this? > >> > >> Otherwise I will add something like: > >> > >> adi,busy-on-gp3: > >> $ref: /schemas/types.yaml#/definitions/flag > >> description: > >> When present, the busy interrupt is coming from the GP3 pin, otherwise > >> the interrupt is coming from the BSY_ALT_GP0 pin. > >> > >> adi,alert-on-gp2: > >> $ref: /schemas/types.yaml#/definitions/flag > >> description: > >> When present, the alert interrupt is coming from the GP2 pin, otherwise > >> the interrupt is coming from the BSY_ALT_GP0 pin. > > Cut and paste? Or it ends up on the same pin as the bsy? In which case that's > > a single interrupt and it's up to software to decide how to use. I'll guess > > it comes on GP1? > > This is not a typo. The BSY_ALT_GP0 is a multi-purpose pin. The actual function > of the pin isn't selected explicitly, but rather there is an order of priority > (Table 25 in the datasheet). > > Also, there are two packages the chip can come in, LFCSP and WLCSP. The former > only has GP0 and not GP1/2/3. > > My thinking was that if both interrupts are provided in the DT and neither > adi,busy-on-gp3 or adi,alert-on-gp2 is given, then the driver would use > a shared interrupt and only allow enabling one function alert or busy > at a time. Avoid the custom parameters They are not needed as software gets to decide how these pins are used (if I read you description correctly) Do it as 3 interrupts of which an combination could be supplied and the driver gets to figure out what to use them for. We are describing the wiring here for the interrupts, not what the driver is doing with them. So if only GP0 is provided then shared interrupt it is with either alert or busy. > > >> > > > > More interrupt names. We shouldn't restrict someone wiring all 4 if they want > > to - we'll just use 2 that we choose in the driver. > > > > interrupt-names > > minItems: 1 > > items: > > - const: busy-gp0 > > - const: busy-gp1 > > - const: alert-gp2 > > - cosnt: alert-gp1 > > This would actually need to be: > > interrupt-names > minItems: 1 > items: > - const: busy-gp0 > - const: busy-gp3 > - const: alert-gp0 > - cosnt: alert-gp2 > > Or would it need to be this since there are two possible signals on the > same pin rather than trying to mess with a shared interrupt? > (Note: if both alert and busy are enabled at the same time on GP0, we > will only get the alert signal and busy signal is masked). > > interrupt-names > minItems: 1 > items: > - const: alert-busy-gp0 > - const: busy-gp3 > - cosnt: alert-gp2 Don't describe the mode, just the pin in this case. There are other cases of this but normally they are called int1, int2 or something like that rather than gpX. They can have weird and complex constraints on what is possible to signal but that's a driver problem not a dt one. interrupt-names: minItems: 1 items: - const: gp0 - const: gp3 - const: gp2 I'm not sure I understand if there are other constraints, but if there are I think they are on what can be used at the same time, not what can be wired. > > > > > T > >> > >>> + > >>> +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 > >>> + > >>> + diff-channels: > >>> + description: > >>> + Describes inputs used for differential channels. The first value must > >>> + be an even numbered input and the second value must be the next > >>> + consecutive odd numbered input. > >>> + items: > >>> + - minimum: 0 > >>> + maximum: 14 > >>> + multipleOf: 2 > >>> + - minimum: 1 > >>> + maximum: 15 > >>> + not: > >>> + multipleOf: 2 > >> > >> After some more testing, it turns out that I misunderstood the datasheet and > >> this isn't actually fully differential, but rather pseudo-differential. > >> > >> So when pairing with the next pin, it is similar to pairing with the COM pin > >> where the negative input pin is connected to a constant voltage source. > > > > Ok. I'm curious, how does it actually differ from a differential channel? > > What was that test? It doesn't cope with an actual differential pair and needs > > a stable value on the negative? > > In my initial testing, since I was only doing a direct read, I was using > constant voltages. But when I started working on buffered reads, then I > saw noisy data when using a fully differential (antiphase) signal. > > This chip uses a multiplexer for channel so when an odd number pin is used > as the positive input (paired with REFGND or COM), it goes through one > multiplexer, but when an odd number pin is used as the negative input > it goes through the other multiplexer - the same one as REFGND and COM. > > And burred in the middle of a paragraph on page 34 of 110 of the datasheet > is the only place in the entire datasheet where it actually says this is > a pseudo differential chip. Fair enough. Sounds like the right test to me. I guess that second mux has a slow tracking buffer or similar. Good work tracking this down. > > > > >> > >>> + > >>> + single-channel: > >>> + minimum: 0 > >>> + maximum: 15 > >>> + > >>> + common-mode-channel: > >>> + description: > >>> + Describes the common mode channel for single channels. 0 is REFGND > >>> + and 1 is COM. Macros are available for these values in > >>> + dt-bindings/iio/adi,ad4695.h. > >>> + minimum: 0 > >>> + maximum: 1 > >>> + default: 0 > >> > >> So I'm thinking the right thing to do here go back to using reg and the INx > >> number and only have common-mode-channel (no diff-channels or single-channel). > >> > >> common-mode-channel will need to be changed to allow INx numbers in addition > >> to COM and REFGND. > >> > >> This means that [PATCH v2 1/4] "dt-bindings: iio: adc: add common-mode-channel > >> dependency" would be wrong since we would be using common-mode-channel without > >> single-channel. > >> > >> It also means we will need an optional in1-supply: true for all odd numbered > >> inputs. > > Ok. I'm not totally sure I see how this comes together but will wait for v3 rather > > than trying to figure it out now. > > > > Jonathan > > > > > > It should end up like other pseudo differential chips we have done recently. > I've got it worked out, so you'll be seeing it soon enough anyway. > Cool Jonathan ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/4] iio: adc: ad4695: Add driver for AD4695 and similar ADCs 2024-06-17 19:53 [PATCH v2 0/4] iio: adc: ad4695: new driver for AD4695 and similar ADCs David Lechner 2024-06-17 19:53 ` [PATCH v2 1/4] dt-bindings: iio: adc: add common-mode-channel dependency David Lechner 2024-06-17 19:53 ` [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs David Lechner @ 2024-06-17 19:53 ` David Lechner 2024-06-23 16:43 ` Jonathan Cameron 2024-06-17 19:53 ` [PATCH v2 4/4] Documentation: iio: Document ad4695 driver David Lechner 3 siblings, 1 reply; 15+ messages in thread From: David Lechner @ 2024-06-17 19:53 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> --- v2 changes: * rework register definition macros * remove code structure comments at top level * remove simple wrapper functions around regmap functions * rework channel fwnode parsing for DT changes * fix missing return value check --- MAINTAINERS | 1 + drivers/iio/adc/Kconfig | 11 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad4695.c | 753 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 766 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 19d4bc962c77..e7a338a3eaaa 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 F: include/dt-bindings/iio/adi,ad4695.h ANALOG DEVICES INC AD7091R DRIVER 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..dbe6175b780d --- /dev/null +++ b/drivers/iio/adc/ad4695.c @@ -0,0 +1,753 @@ +// 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_A_SW_RST (BIT(7) | BIT(0)) +#define AD4695_REG_SPI_CONFIG_B 0x0001 +#define AD4695_REG_SPI_CONFIG_B_INST_MODE BIT(7) +#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_CONFIG_C_MB_STRICT BIT(7) +#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_SETUP_LDO_EN BIT(4) +#define AD4695_REG_SETUP_SPI_MODE BIT(2) +#define AD4695_REG_SETUP_SPI_CYC_CTRL BIT(1) +#define AD4695_REG_REF_CTRL 0x0021 +#define AD4695_REG_REF_CTRL_OV_MODE BIT(7) +#define AD4695_REG_REF_CTRL_VREF_SET GENMASK(4, 2) +#define AD4695_REG_REF_CTRL_REFHIZ_EN BIT(1) +#define AD4695_REG_REF_CTRL_REFBUF_EN BIT(0) +#define AD4695_REG_SEQ_CTRL 0x0022 +#define AD4695_REG_SEQ_CTRL_STD_SEQ_EN BIT(7) +#define AD4695_REG_SEQ_CTRL_NUM_SLOTS_AS GENMASK(6, 0) +#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_CONFIG_IN_MODE BIT(6) +#define AD4695_REG_CONFIG_IN_PAIR GENMASK(5, 4) +#define AD4695_REG_CONFIG_IN_AINHIGHZ_EN BIT(3) +#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_AS_SLOT_INX GENMASK(3, 0) +#define AD4695_MAX_REG 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)) + +/* 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) + +enum ad4695_in_pair { + AD4695_IN_PAIR_REFGND, + AD4695_IN_PAIR_COM, + AD4695_IN_PAIR_EVEN_ODD, +}; + +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_MAX_REG), +}; + +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_MAX_REG), +}; + +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_MAX_REG, + .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, +}; + +/** + * 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_REG_SEQ_CTRL_STD_SEQ_EN | + AD4695_REG_SEQ_CTRL_NUM_SLOTS_AS); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD4695_REG_AS_SLOT(0), + FIELD_PREP(AD4695_REG_AS_SLOT_INX, channel)); + if (ret) + return ret; + + return regmap_set_bits(st->regmap, AD4695_REG_SETUP, + AD4695_REG_SETUP_SPI_MODE | + AD4695_REG_SETUP_SPI_CYC_CTRL); +} + +static int ad4695_set_ref_voltage(struct ad4695_state *st, int vref_mv) +{ + u8 val; + + if (vref_mv >= 2400 && vref_mv <= 2750) + val = 0; + else if (vref_mv > 2750 && vref_mv <= 3250) + val = 1; + else if (vref_mv > 3250 && vref_mv <= 3750) + val = 2; + else if (vref_mv > 3750 && vref_mv <= 4500) + val = 3; + else if (vref_mv > 4500 && vref_mv <= 5100) + val = 4; + else + return -EINVAL; + + return regmap_update_bits(st->regmap, AD4695_REG_REF_CTRL, + AD4695_REG_REF_CTRL_VREF_SET, + FIELD_PREP(AD4695_REG_REF_CTRL_VREF_SET, val)); +} + +static int ad4695_write_chn_cfg(struct ad4695_state *st, + struct ad4695_channel_config *cfg) +{ + u32 mask = 0, val = 0; + + mask |= AD4695_REG_CONFIG_IN_MODE; + val |= FIELD_PREP(AD4695_REG_CONFIG_IN_MODE, cfg->bipolar ? 1 : 0); + + mask |= AD4695_REG_CONFIG_IN_PAIR; + val |= FIELD_PREP(AD4695_REG_CONFIG_IN_PAIR, cfg->pin_pairing); + + mask |= AD4695_REG_CONFIG_IN_AINHIGHZ_EN; + val |= FIELD_PREP(AD4695_REG_CONFIG_IN_AINHIGHZ_EN, cfg->highz_en ? 1 : 0); + + 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); +} + +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_IN_PAIR_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 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]; + + iio_chan_arr[chan_idx] = ad4695_channel_template; + iio_chan_arr[chan_idx].scan_index = chan_idx; + + if (fwnode_property_present(child, "diff-channels")) { + u32 channels[2]; + + ret = fwnode_property_read_u32_array(child, + "diff-channels", + channels, + ARRAY_SIZE(channels)); + if (ret) + return dev_err_probe(dev, ret, + "failed to read diff-channels(%s)\n", + fwnode_get_name(child)); + + if (channels[0] >= st->chip_info->num_voltage_inputs || + channels[1] >= st->chip_info->num_voltage_inputs) + return dev_err_probe(dev, -EINVAL, + "diff-channel out of range (%s)\n", + fwnode_get_name(child)); + + if (channels[0] % 2 != 0 || + channels[1] != channels[0] + 1) + return dev_err_probe(dev, -EINVAL, + "diff-channel must be even/odd consecutive pair (%s)\n", + fwnode_get_name(child)); + + iio_chan_arr[chan_idx].differential = 1; + iio_chan_arr[chan_idx].channel = channels[0]; + iio_chan_arr[chan_idx].channel2 = channels[1]; + iio_chan_arr[chan_idx].address = + AD4695_CMD_VOLTAGE_CHAN(channels[0]); + + chan_cfg->channel = channels[0]; + chan_cfg->pin_pairing = AD4695_IN_PAIR_EVEN_ODD; + } else if (fwnode_property_present(child, "single-channel")) { + u32 val; + + ret = fwnode_property_read_u32(child, "single-channel", + &val); + if (ret) + return dev_err_probe(dev, ret, + "failed to read single-channel (%s)\n", + fwnode_get_name(child)); + + if (val >= st->chip_info->num_voltage_inputs) + return dev_err_probe(dev, -EINVAL, + "single-channel out of range (%s)\n", + fwnode_get_name(child)); + + iio_chan_arr[chan_idx].channel = val; + iio_chan_arr[chan_idx].address = + AD4695_CMD_VOLTAGE_CHAN(val); + + chan_cfg->channel = val; + + /* Default to REFGND if common-mode-channel is not specified. */ + val = AD4695_IN_PAIR_REFGND; + + ret = fwnode_property_read_u32(child, "common-mode-channel", + &val); + if (ret && ret != -EINVAL) + return dev_err_probe(dev, ret, + "failed to read common-mode-channel (%s)\n", + fwnode_get_name(child)); + + if (val > AD4695_IN_PAIR_COM) + return dev_err_probe(dev, -EINVAL, + "common-mode-channel out of range (%s)\n", + fwnode_get_name(child)); + + chan_cfg->pin_pairing = val; + } else { + return dev_err_probe(dev, -EINVAL, + "missing diff or single channel (%s)\n", + fwnode_get_name(child)); + } + + chan_cfg->bipolar = fwnode_property_read_bool(child, "bipolar"); + + iio_chan_arr[chan_idx].scan_type.sign = chan_cfg->bipolar ? 's' : 'u'; + + if (chan_cfg->bipolar && chan_cfg->pin_pairing == AD4695_IN_PAIR_REFGND) + return dev_err_probe(dev, -EINVAL, + "bipolar mode is not available for inputs paired with REFGND (%s).\n", + fwnode_get_name(child)); + + chan_cfg->highz_en = !fwnode_property_read_bool(child, "adi,no-high-z"); + + 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); + msleep(AD4695_T_WAKEUP_HW_MS); + } else { + ret = regmap_write(st->regmap, AD4695_REG_SPI_CONFIG_A, + AD4695_REG_SPI_CONFIG_A_SW_RST); + if (ret) + return ret; + + msleep(AD4695_T_WAKEUP_SW_MS); + } + + /* Needed for debugfs since it only access registers 1 byte at a time. */ + ret = regmap_set_bits(st->regmap, AD4695_REG_SPI_CONFIG_C, + AD4695_REG_SPI_CONFIG_C_MB_STRICT); + if (ret) + return ret; + + /* Disable internal LDO if it isn't needed. */ + ret = regmap_update_bits(st->regmap, AD4695_REG_SETUP, + AD4695_REG_SETUP_LDO_EN, + FIELD_PREP(AD4695_REG_SETUP_LDO_EN, + use_internal_ldo_supply ? 1 : 0)); + if (ret) + return ret; + + /* configure reference supply */ + + if (device_property_present(dev, "adi,no-ref-current-limit")) { + ret = regmap_set_bits(st->regmap, AD4695_REG_REF_CTRL, + AD4695_REG_REF_CTRL_OV_MODE); + 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 = regmap_clear_bits(st->regmap, AD4695_REG_REF_CTRL, + AD4695_REG_REF_CTRL_REFHIZ_EN); + if (ret) + return ret; + } + + ret = ad4695_set_ref_voltage(st, st->vref_mv); + if (ret) + return ret; + + if (use_internal_ref_buffer) { + ret = regmap_set_bits(st->regmap, AD4695_REG_REF_CTRL, + AD4695_REG_REF_CTRL_REFBUF_EN); + 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; + + 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] 15+ messages in thread
* Re: [PATCH v2 3/4] iio: adc: ad4695: Add driver for AD4695 and similar ADCs 2024-06-17 19:53 ` [PATCH v2 3/4] iio: adc: ad4695: Add driver for " David Lechner @ 2024-06-23 16:43 ` Jonathan Cameron 0 siblings, 0 replies; 15+ messages in thread From: Jonathan Cameron @ 2024-06-23 16:43 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 Mon, 17 Jun 2024 14:53:14 -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> I only took a quick look as it sounds like some of this will change. FWIW Looks good to me. Jonathan ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 4/4] Documentation: iio: Document ad4695 driver 2024-06-17 19:53 [PATCH v2 0/4] iio: adc: ad4695: new driver for AD4695 and similar ADCs David Lechner ` (2 preceding siblings ...) 2024-06-17 19:53 ` [PATCH v2 3/4] iio: adc: ad4695: Add driver for " David Lechner @ 2024-06-17 19:53 ` David Lechner 3 siblings, 0 replies; 15+ messages in thread From: David Lechner @ 2024-06-17 19:53 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> --- v2 changes: * Rework DT examples for DT bindings changes * Add file to MAINTAINERS --- Documentation/iio/ad4695.rst | 153 +++++++++++++++++++++++++++++++++++++++++++ Documentation/iio/index.rst | 1 + MAINTAINERS | 1 + 3 files changed, 155 insertions(+) diff --git a/Documentation/iio/ad4695.rst b/Documentation/iio/ad4695.rst new file mode 100644 index 000000000000..d5cde1b84d50 --- /dev/null +++ b/Documentation/iio/ad4695.rst @@ -0,0 +1,153 @@ +.. 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@0 { + reg = <0>; /* not related to the pin number */ + single-channel = <0>; /* IN0 */ + }; + +This will appear on the IIO bus as the ``voltage0`` channel. The processed value +(*raw × scale*) will be the voltage present on the ``IN0`` pin 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@1 { + reg = <1>; + single-channel = <1>; /* IN1 */ + common-mode-channel = <AD4695_COMMON_MODE_COM>; + bipolar; + }; + +This will appear on the IIO bus as the ``voltage1`` channel. The processed value +(*(raw + offset) × scale*) will be the voltage measured on the ``IN1`` pin +relative to ``REFGND``. (The offset is determined by the ``com-supply`` voltage.) + +The macro comes from: + +.. code-block:: + + #include <dt-bindings/iio/adi,ad4695.h> + +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 ``IN2`` as the positive +input and ``IN3`` as the negative input will look like this: + +.. code-block:: + + channel@2 { + reg = <2>; + diff-channels = <2>, <3>; /* IN2, IN3 */ + bipolar; + }; + +This will appear on the IIO bus as the ``voltage2-voltage3`` 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 e7a338a3eaaa..15f15b6013ce 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: Documentation/iio/ad4695.rst F: drivers/iio/adc/ad4695.c F: include/dt-bindings/iio/adi,ad4695.h -- 2.45.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-06-24 17:52 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-17 19:53 [PATCH v2 0/4] iio: adc: ad4695: new driver for AD4695 and similar ADCs David Lechner 2024-06-17 19:53 ` [PATCH v2 1/4] dt-bindings: iio: adc: add common-mode-channel dependency David Lechner 2024-06-18 19:47 ` David Lechner 2024-06-17 19:53 ` [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs David Lechner 2024-06-17 21:30 ` Rob Herring (Arm) 2024-06-17 22:06 ` David Lechner 2024-06-18 18:00 ` Conor Dooley 2024-06-24 14:35 ` Rob Herring 2024-06-18 19:29 ` David Lechner 2024-06-23 16:39 ` Jonathan Cameron 2024-06-24 14:36 ` David Lechner 2024-06-24 17:52 ` Jonathan Cameron 2024-06-17 19:53 ` [PATCH v2 3/4] iio: adc: ad4695: Add driver for " David Lechner 2024-06-23 16:43 ` Jonathan Cameron 2024-06-17 19:53 ` [PATCH v2 4/4] Documentation: iio: Document ad4695 driver David Lechner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.