* [PATCH 0/2] Add support for AD4000 series @ 2024-03-22 22:04 Marcelo Schmitt 2024-03-22 22:05 ` [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 Marcelo Schmitt 2024-03-22 22:05 ` [PATCH 2/2] iio: adc: Add support for AD4000 Marcelo Schmitt 0 siblings, 2 replies; 14+ messages in thread From: Marcelo Schmitt @ 2024-03-22 22:04 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1 Cc: linux-iio, devicetree, linux-kernel This series adds support for high-speed, high-precision AD4000 series of SAR ADCs. Most uncommon things about this set are: 1) These devices have the same SPI (Strange Peripheral Interface) as AD7944 devices, which has been documented in ad7944.rst [1]. The device tree description for SPI connections and mode can be the same as of ad7944 adi,spi-mode [2]. Because ad4000 driver does not currently support daisy-chain mode, I simplified things a little bit. If having a more complete doc is preferred, I'm fine changing to that. [1]: https://lore.kernel.org/linux-iio/20240313-mainline-ad7944-doc-v1-2-7860416726e4@baylibre.com/ [2]: https://lore.kernel.org/linux-iio/20240304-ad7944-mainline-v5-1-f0a38cea8901@baylibre.com/ 2) Differently from AD7944, AD4000 devices have a configuration register to toggle some features. For instance, turbo mode is set through configuration register rather than an external pin. This simplifies hardware connections, but then requires software interface. So, additional ABI being proposed in sysfs-bus-iio-adc-ad4000. The one I'm most in doubt about is span_compression_en which affects the in_voltageY_scale attribute. That might be instead supported by providing _scale_available and allowing write to _scale. Anyway, let me know how bad those look like :) Thanks, Marcelo Marcelo Schmitt (2): dt-bindings: iio: adc: Add AD4000 iio: adc: Add support for AD4000 .../ABI/testing/sysfs-bus-iio-adc-ad4000 | 36 + .../bindings/iio/adc/adi,ad4000.yaml | 151 ++++ MAINTAINERS | 9 + drivers/iio/adc/Kconfig | 12 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad4000.c | 666 ++++++++++++++++++ 6 files changed, 875 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml create mode 100644 drivers/iio/adc/ad4000.c -- 2.43.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 2024-03-22 22:04 [PATCH 0/2] Add support for AD4000 series Marcelo Schmitt @ 2024-03-22 22:05 ` Marcelo Schmitt 2024-03-22 23:28 ` Rob Herring 2024-03-23 18:44 ` Jonathan Cameron 2024-03-22 22:05 ` [PATCH 2/2] iio: adc: Add support for AD4000 Marcelo Schmitt 1 sibling, 2 replies; 14+ messages in thread From: Marcelo Schmitt @ 2024-03-22 22:05 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1 Cc: linux-iio, devicetree, linux-kernel Add device tree documentation for AD4000 series of ADC devices. Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> --- Pasting relevant comment from cover letter here to aid reviewers. These devices have the same SPI (Strange Peripheral Interface) as AD7944 devices, which has been documented in ad7944.rst [1]. The device tree description for SPI connections and mode can be the same as of ad7944 adi,spi-mode [2]. Because ad4000 driver does not currently support daisy-chain mode, I simplified things a little bit. If having a more complete doc is preferred, I'm fine changing to that. [1]: https://lore.kernel.org/linux-iio/20240313-mainline-ad7944-doc-v1-2-7860416726e4@baylibre.com/ [2]: https://lore.kernel.org/linux-iio/20240304-ad7944-mainline-v5-1-f0a38cea8901@baylibre.com/ .../bindings/iio/adc/adi,ad4000.yaml | 151 ++++++++++++++++++ MAINTAINERS | 7 + 2 files changed, 158 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file mode 100644 index 000000000000..9e3d6a3920ea --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml @@ -0,0 +1,151 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices AD4000 ADC device driver + +maintainers: + - Marcelo Schmitt <marcelo.schmitt@analog.com> + +description: | + Analog Devices AD4000 family of Analog to Digital Converters with SPI support. + Specifications can be found at: + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf + +properties: + compatible: + enum: + - adi,ad4000 + - adi,ad4001 + - adi,ad4002 + - adi,ad4003 + - adi,ad4004 + - adi,ad4005 + - adi,ad4006 + - adi,ad4007 + - adi,ad4008 + - adi,ad4010 + - adi,ad4011 + - adi,ad4020 + - adi,ad4021 + - adi,ad4022 + - adi,adaq4001 + - adi,adaq4003 + + reg: true + spi-max-frequency: true + + vref-supply: + description: Phandle to the regulator for ADC reference voltage. + + adi,gain-milli: + description: | + The hardware gain applied to the ADC input (in milli units). + The gain provided by the ADC input scaler is defined by the hardware + connections between chip pins OUT+, R1K-, R1K1-, R1K+, R1K1+, and OUT-. + If not present, default to 1000 (no actual gain applied). + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [454, 909, 1000, 1900] + default: 1000 + + adi,spi-cs-mode: + type: boolean + description: | + This property indicates the SPI wiring configuration. + + When this property is omitted, it indicates that the device SDI pin is + connected to SPI controller CS line and device CNV pin has been connected + to a GPIO. Datasheets call this "4-wire mode". + + When this property is present, the driver must assume standard SPI + connections which, for these devices, consists of connecting the + controller CS line to device CNV pin. This configuration is + (misleadingly) called "3-wire mode" in datasheets. + + cnv-gpios: + description: The GPIO connected to the CNV pin. + maxItems: 1 + +patternProperties: + "^channel@([0-1])$": + $ref: adc.yaml + type: object + description: Represents the external channel connected to the ADC. + + properties: + reg: + maxItems: 1 + + diff-channels: true + + required: + - reg + + additionalProperties: false + +required: + - compatible + - reg + - vref-supply + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + + - if: + properties: + adi,spi-cs-mode: false + then: + required: + - cnv-gpios + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + spi { + #address-cells = <1>; + #size-cells = <0>; + /* Example for a AD4000 devices */ + adc@0 { + compatible = "adi,ad4020"; + reg = <0>; + spi-max-frequency = <71000000>; + vref-supply = <&vref>; + cnv-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH>; + #address-cells = <1>; + #size-cells = <0>; + channel@0 { + reg = <0>; + diff-channels = <0 1>; + }; + }; + }; + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + /* Example for a ADAQ4000 devices */ + adc@0 { + compatible = "adi,adaq4003"; + reg = <0>; + spi-max-frequency = <80000000>; + vref-supply = <&vref>; + adi,spi-cs-mode; + adi,gain-milli = <454>; + #address-cells = <1>; + #size-cells = <0>; + channel@0 { + reg = <0>; + diff-channels = <0 1>; + }; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 2662ec49b297..3ca90f842298 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1135,6 +1135,13 @@ W: https://ez.analog.com/linux-software-drivers F: Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml F: drivers/iio/dac/ad3552r.c +ANALOG DEVICES INC AD4000 DRIVER +M: Marcelo Schmitt <marcelo.schmitt@analog.com> +L: linux-iio@vger.kernel.org +S: Supported +W: https://ez.analog.com/linux-software-drivers +F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml + ANALOG DEVICES INC AD4130 DRIVER M: Cosmin Tanislav <cosmin.tanislav@analog.com> L: linux-iio@vger.kernel.org -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 2024-03-22 22:05 ` [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 Marcelo Schmitt @ 2024-03-22 23:28 ` Rob Herring 2024-03-23 3:29 ` Marcelo Schmitt 2024-03-23 18:44 ` Jonathan Cameron 1 sibling, 1 reply; 14+ messages in thread From: Rob Herring @ 2024-03-22 23:28 UTC (permalink / raw) To: Marcelo Schmitt Cc: krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1, robh+dt, devicetree, lars, linux-iio, jic23, Michael.Hennerich, linux-kernel On Fri, 22 Mar 2024 19:05:08 -0300, Marcelo Schmitt wrote: > Add device tree documentation for AD4000 series of ADC devices. > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > --- > Pasting relevant comment from cover letter here to aid reviewers. > > These devices have the same SPI (Strange Peripheral Interface) as AD7944 > devices, which has been documented in ad7944.rst [1]. > The device tree description for SPI connections and mode can be the same as of > ad7944 adi,spi-mode [2]. > Because ad4000 driver does not currently support daisy-chain mode, I simplified > things a little bit. If having a more complete doc is preferred, I'm fine > changing to that. > > [1]: https://lore.kernel.org/linux-iio/20240313-mainline-ad7944-doc-v1-2-7860416726e4@baylibre.com/ > [2]: https://lore.kernel.org/linux-iio/20240304-ad7944-mainline-v5-1-f0a38cea8901@baylibre.com/ > > .../bindings/iio/adc/adi,ad4000.yaml | 151 ++++++++++++++++++ > MAINTAINERS | 7 + > 2 files changed, 158 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected) from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected) from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/81665b5f0d37d593e6d299528de8d68da8574077.1711131830.git.marcelo.schmitt@analog.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] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 2024-03-22 23:28 ` Rob Herring @ 2024-03-23 3:29 ` Marcelo Schmitt 2024-03-23 10:58 ` Krzysztof Kozlowski 0 siblings, 1 reply; 14+ messages in thread From: Marcelo Schmitt @ 2024-03-23 3:29 UTC (permalink / raw) To: Rob Herring Cc: Marcelo Schmitt, krzysztof.kozlowski+dt, conor+dt, robh+dt, devicetree, lars, linux-iio, jic23, Michael.Hennerich, linux-kernel > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected) > from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml# > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected) > from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml# > ok, adding proper #address-cells and #size-cells fixes the warning. '#address-cells': const: 1 '#size-cells': const: 0 I'm assuming missing those in v1 doesn't hurt review so will wait for some feedback before sending a v2. > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/81665b5f0d37d593e6d299528de8d68da8574077.1711131830.git.marcelo.schmitt@analog.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] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 2024-03-23 3:29 ` Marcelo Schmitt @ 2024-03-23 10:58 ` Krzysztof Kozlowski 0 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2024-03-23 10:58 UTC (permalink / raw) To: Marcelo Schmitt, Rob Herring Cc: Marcelo Schmitt, krzysztof.kozlowski+dt, conor+dt, robh+dt, devicetree, lars, linux-iio, jic23, Michael.Hennerich, linux-kernel On 23/03/2024 04:29, Marcelo Schmitt wrote: >> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' >> on your patch (DT_CHECKER_FLAGS is new in v5.13): >> >> yamllint warnings/errors: >> >> dtschema/dtc warnings/errors: >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected) >> from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml# >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4000.example.dtb: adc@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells' were unexpected) >> from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml# >> > > ok, adding proper #address-cells and #size-cells fixes the warning. > > '#address-cells': > const: 1 > > '#size-cells': > const: 0 > > I'm assuming missing those in v1 doesn't hurt review so will wait for some > feedback before sending a v2. Hurts in a way it is a proof you did not test your binding before sending. Performing review on untested code might be a waste of reviewers time. Please test your code before sending it. I am not going to perform review of untested code. It does not look like you tested the bindings, at least after quick look. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Maybe you need to update your dtschema and yamllint. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 2024-03-22 22:05 ` [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 Marcelo Schmitt 2024-03-22 23:28 ` Rob Herring @ 2024-03-23 18:44 ` Jonathan Cameron 2024-03-23 20:18 ` David Lechner 1 sibling, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2024-03-23 18:44 UTC (permalink / raw) To: Marcelo Schmitt Cc: lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1, linux-iio, devicetree, linux-kernel, David Lechner On Fri, 22 Mar 2024 19:05:08 -0300 Marcelo Schmitt <marcelo.schmitt@analog.com> wrote: > Add device tree documentation for AD4000 series of ADC devices. > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > --- > Pasting relevant comment from cover letter here to aid reviewers. > > These devices have the same SPI (Strange Peripheral Interface) as AD7944 > devices, which has been documented in ad7944.rst [1]. > The device tree description for SPI connections and mode can be the same as of > ad7944 adi,spi-mode [2]. > Because ad4000 driver does not currently support daisy-chain mode, I simplified > things a little bit. If having a more complete doc is preferred, I'm fine > changing to that. > > [1]: https://lore.kernel.org/linux-iio/20240313-mainline-ad7944-doc-v1-2-7860416726e4@baylibre.com/ > [2]: https://lore.kernel.org/linux-iio/20240304-ad7944-mainline-v5-1-f0a38cea8901@baylibre.com/ > > .../bindings/iio/adc/adi,ad4000.yaml | 151 ++++++++++++++++++ > MAINTAINERS | 7 + > 2 files changed, 158 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml > new file mode 100644 > index 000000000000..9e3d6a3920ea > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml > @@ -0,0 +1,151 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices AD4000 ADC device driver > + > +maintainers: > + - Marcelo Schmitt <marcelo.schmitt@analog.com> > + > +description: | > + Analog Devices AD4000 family of Analog to Digital Converters with SPI support. > + Specifications can be found at: > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf > + > +properties: > + compatible: > + enum: > + - adi,ad4000 > + - adi,ad4001 > + - adi,ad4002 > + - adi,ad4003 > + - adi,ad4004 > + - adi,ad4005 > + - adi,ad4006 > + - adi,ad4007 > + - adi,ad4008 > + - adi,ad4010 > + - adi,ad4011 > + - adi,ad4020 > + - adi,ad4021 > + - adi,ad4022 > + - adi,adaq4001 > + - adi,adaq4003 > + > + reg: true > + spi-max-frequency: true > + > + vref-supply: > + description: Phandle to the regulator for ADC reference voltage. > + > + adi,gain-milli: > + description: | > + The hardware gain applied to the ADC input (in milli units). > + The gain provided by the ADC input scaler is defined by the hardware > + connections between chip pins OUT+, R1K-, R1K1-, R1K+, R1K1+, and OUT-. > + If not present, default to 1000 (no actual gain applied). > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [454, 909, 1000, 1900] > + default: 1000 > + > + adi,spi-cs-mode: We've just merged a driver for the ad7944 and bindings which has a similar 3-wire-mode. Please share the approach used in that binding. Whilst it seems we don't have the other mode here, I think we still want to use a similar enum. +CC David to take a look at this one given he went through long discussions on how to deal with it for the driver he was working on so probably remembers the reasoning etc better than I do :) Jonathan > + type: boolean > + description: | > + This property indicates the SPI wiring configuration. > + > + When this property is omitted, it indicates that the device SDI pin is > + connected to SPI controller CS line and device CNV pin has been connected > + to a GPIO. Datasheets call this "4-wire mode". > + > + When this property is present, the driver must assume standard SPI > + connections which, for these devices, consists of connecting the > + controller CS line to device CNV pin. This configuration is > + (misleadingly) called "3-wire mode" in datasheets. > + > + cnv-gpios: > + description: The GPIO connected to the CNV pin. > + maxItems: 1 > + > +patternProperties: > + "^channel@([0-1])$": > + $ref: adc.yaml > + type: object > + description: Represents the external channel connected to the ADC. > + > + properties: > + reg: > + maxItems: 1 > + > + diff-channels: true > + > + required: > + - reg > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + - vref-supply > + > +allOf: > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > + - if: > + properties: > + adi,spi-cs-mode: false > + then: > + required: > + - cnv-gpios > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + /* Example for a AD4000 devices */ > + adc@0 { > + compatible = "adi,ad4020"; > + reg = <0>; > + spi-max-frequency = <71000000>; > + vref-supply = <&vref>; > + cnv-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH>; > + #address-cells = <1>; > + #size-cells = <0>; > + channel@0 { > + reg = <0>; > + diff-channels = <0 1>; > + }; > + }; > + }; > + - | > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + /* Example for a ADAQ4000 devices */ > + adc@0 { > + compatible = "adi,adaq4003"; > + reg = <0>; > + spi-max-frequency = <80000000>; > + vref-supply = <&vref>; > + adi,spi-cs-mode; > + adi,gain-milli = <454>; > + #address-cells = <1>; > + #size-cells = <0>; > + channel@0 { > + reg = <0>; > + diff-channels = <0 1>; > + }; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 2662ec49b297..3ca90f842298 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1135,6 +1135,13 @@ W: https://ez.analog.com/linux-software-drivers > F: Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > F: drivers/iio/dac/ad3552r.c > > +ANALOG DEVICES INC AD4000 DRIVER > +M: Marcelo Schmitt <marcelo.schmitt@analog.com> > +L: linux-iio@vger.kernel.org > +S: Supported > +W: https://ez.analog.com/linux-software-drivers > +F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml > + > ANALOG DEVICES INC AD4130 DRIVER > M: Cosmin Tanislav <cosmin.tanislav@analog.com> > L: linux-iio@vger.kernel.org ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 2024-03-23 18:44 ` Jonathan Cameron @ 2024-03-23 20:18 ` David Lechner 2024-03-23 20:38 ` David Lechner 0 siblings, 1 reply; 14+ messages in thread From: David Lechner @ 2024-03-23 20:18 UTC (permalink / raw) To: Jonathan Cameron Cc: Marcelo Schmitt, lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1, linux-iio, devicetree, linux-kernel On Sat, Mar 23, 2024 at 1:45 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Fri, 22 Mar 2024 19:05:08 -0300 > Marcelo Schmitt <marcelo.schmitt@analog.com> wrote: > > > Add device tree documentation for AD4000 series of ADC devices. > > > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf > > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > > --- > > Pasting relevant comment from cover letter here to aid reviewers. > > > > These devices have the same SPI (Strange Peripheral Interface) as AD7944 > > devices, which has been documented in ad7944.rst [1]. > > The device tree description for SPI connections and mode can be the same as of > > ad7944 adi,spi-mode [2]. > > Because ad4000 driver does not currently support daisy-chain mode, I simplified > > things a little bit. If having a more complete doc is preferred, I'm fine > > changing to that. Yes, having a complete binding is always preferred [1]. Bindings should never omit anything just because it isn't implemented in the driver. [1]: https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bindings.html ... > > + > > + adi,spi-cs-mode: > > We've just merged a driver for the ad7944 and bindings which has a > similar 3-wire-mode. Please share the approach used in that binding. > Whilst it seems we don't have the other mode here, I think we still want > to use a similar enum. The ad40xx chips actually do have the same daisy chain mode. So the exact same property and all enum values apply. > +CC David to take a look at this one given he went through long > discussions on how to deal with it for the driver he was working on > so probably remembers the reasoning etc better than I do :) > In addition to the SPI wiring modes, the proposed bindings are also missing power supplies and the busy interrupt. Also, since the ADAQ chips are quite different from the AD chips, it would be very helpful for reviewers (and git history) to split out adding those chips to the DT bindings and driver into separate patches. This way we can clearly see which features only apply to the ADAQ chips. Here is what I would consider a reasonably complete binding for the AD40XX chips (excluding ADAQ for now as I suggested). --- # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 --- $id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# title: Analog Devices AD4000 and similar Analog to Digital Converters maintainers: - Marcelo Schmitt <marcelo.schmitt@analog.com> description: | Analog Devices AD4000 family of Analog to Digital Converters with SPI support. Specifications can be found at: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf $ref: /schemas/spi/spi-peripheral-props.yaml# properties: compatible: enum: - adi,ad4000 - adi,ad4001 - adi,ad4002 - adi,ad4003 - adi,ad4004 - adi,ad4005 - adi,ad4006 - adi,ad4007 - adi,ad4008 - adi,ad4010 - adi,ad4011 - adi,ad4020 - adi,ad4021 - adi,ad4022 reg: maxItems: 1 spi-max-frequency: maximum: 102040816 # for VIO > 2.7 V, 81300813 for VIO > 1.7 V spi-cpha: true adi,spi-mode: $ref: /schemas/types.yaml#/definitions/string enum: [ single, chain ] description: | This property indicates the SPI wiring configuration. When this property is omitted, it is assumed that the device is using what the datasheet calls "4-wire mode". This is the conventional SPI mode used when there are multiple devices on the same bus. In this mode, the CNV line is used to initiate the conversion and the SDI line is connected to CS on the SPI controller. When this property is present, it indicates that the device is using one of the following alternative wiring configurations: * single: The datasheet calls this "3-wire mode". (NOTE: The datasheet's definition of 3-wire mode is NOT at all related to the standard spi-3wire property!) This mode is often used when the ADC is the only device on the bus. In this mode, SDI is tied to VIO, and the CNV line can be connected to the CS line of the SPI controller or to a GPIO, in which case the CS line of the controller is unused. * chain: The datasheet calls this "chain mode". This mode is used to save on wiring when multiple ADCs are used. In this mode, the SDI line of one chip is tied to the SDO of the next chip in the chain and the SDI of the last chip in the chain is tied to GND. Only the first chip in the chain is connected to the SPI bus. The CNV line of all chips are tied together. The CS line of the SPI controller can be used as the CNV line only if it is active high. '#daisy-chained-devices': true vdd-supply: description: A 1.8V supply that powers the chip (VDD). vio-supply: description: A 1.8V to 5V supply for the digital inputs and outputs (VIO). ref-supply: description: A 2.5 to 5V supply for the external reference voltage (REF). cnv-gpios: description: The Convert Input (CNV). This input has multiple functions. It initiates the conversions and selects the SPI mode of the device (chain or CS). In 'single' mode, this property is omitted if the CNV pin is connected to the CS line of the SPI controller. maxItems: 1 interrupts: description: The SDO pin can also function as a busy indicator. This node should be connected to an interrupt that is triggered when the SDO line goes low while the SDI line is high and the CNV line is low ('single' mode) or the SDI line is low and the CNV line is high ('multi' mode); or when the SDO line goes high while the SDI and CNV lines are high (chain mode), maxItems: 1 required: - compatible - reg - vdd-supply - vio-supply - ref-supply allOf: # in '4-wire' mode, cnv-gpios is required, for other modes it is optional - if: not: required: - adi,spi-mode then: required: - cnv-gpios # chain mode has lower SCLK max rate - if: required: - adi,spi-mode properties: adi,spi-mode: const: chain then: properties: spi-max-frequency: maximum: 50000000 # for VIO > 2.7 V, 40000000 for VIO > 1.7 V required: - '#daisy-chained-devices' else: properties: '#daisy-chained-devices': false unevaluatedProperties: false examples: - | #include <dt-bindings/gpio/gpio.h> spi { #address-cells = <1>; #size-cells = <0>; adc@0 { compatible = "adi,ad4020"; reg = <0>; spi-max-frequency = <71000000>; vdd-supply = <&supply_1_8V>; vio-supply = <&supply_1_8V>; ref-supply = <&supply_5V>; cnv-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH>; }; }; ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 2024-03-23 20:18 ` David Lechner @ 2024-03-23 20:38 ` David Lechner 2024-03-23 21:35 ` Marcelo Schmitt 0 siblings, 1 reply; 14+ messages in thread From: David Lechner @ 2024-03-23 20:38 UTC (permalink / raw) To: Jonathan Cameron Cc: Marcelo Schmitt, lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1, linux-iio, devicetree, linux-kernel On Sat, Mar 23, 2024 at 3:18 PM David Lechner <dlechner@baylibre.com> wrote: ... > Here is what I would consider a reasonably complete binding for the > AD40XX chips (excluding ADAQ for now as I suggested). I missed one... I also think it makes sense for the High-Z mode selection to be a DT property since needing to enable it or disable it depends entirely on what is connected to the analog input pins. --- adi,high-z-input: type: boolean description: High-Z mode allows the amplifier and RC filter in front of the ADC to be chosen based on the signal bandwidth of interest, rather than the settling requirements of the switched capacitor SAR ADC inputs. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 2024-03-23 20:38 ` David Lechner @ 2024-03-23 21:35 ` Marcelo Schmitt 0 siblings, 0 replies; 14+ messages in thread From: Marcelo Schmitt @ 2024-03-23 21:35 UTC (permalink / raw) To: David Lechner Cc: Jonathan Cameron, Marcelo Schmitt, lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux-iio, devicetree, linux-kernel On 03/23, David Lechner wrote: > On Sat, Mar 23, 2024 at 3:18 PM David Lechner <dlechner@baylibre.com> wrote: > > ... > > > Here is what I would consider a reasonably complete binding for the > > AD40XX chips (excluding ADAQ for now as I suggested). > > I missed one... > > I also think it makes sense for the High-Z mode selection to be a DT > property since needing to enable it or disable it depends entirely on > what is connected to the analog input pins. > > --- > > adi,high-z-input: > type: boolean > description: > High-Z mode allows the amplifier and RC filter in front of the ADC to be > chosen based on the signal bandwidth of interest, rather than the settling > requirements of the switched capacitor SAR ADC inputs. ok, will do the suggested changes, including provide AD and ADAQ in separate patches. Thanks, Marcelo ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] iio: adc: Add support for AD4000 2024-03-22 22:04 [PATCH 0/2] Add support for AD4000 series Marcelo Schmitt 2024-03-22 22:05 ` [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 Marcelo Schmitt @ 2024-03-22 22:05 ` Marcelo Schmitt 2024-03-23 19:12 ` Jonathan Cameron 2024-03-23 21:53 ` David Lechner 1 sibling, 2 replies; 14+ messages in thread From: Marcelo Schmitt @ 2024-03-22 22:05 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1 Cc: linux-iio, devicetree, linux-kernel Add support for AD4000 series of high accuracy, high speed, low power, successive aproximation register (SAR) ADCs. Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> --- Pasting relevant comment from cover letter here to aid reviewers. Differently from AD7944, AD4000 devices have a configuration register to toggle some features. For instance, turbo mode is set through configuration register rather than an external pin. This simplifies hardware connections, but then requires software interface. So, additional ABI being proposed in sysfs-bus-iio-adc-ad4000. The one I'm most in doubt about is span_compression_en which affects the in_voltageY_scale attribute. That might be instead supported by providing _scale_available and allowing write to _scale. .../ABI/testing/sysfs-bus-iio-adc-ad4000 | 36 + MAINTAINERS | 2 + drivers/iio/adc/Kconfig | 12 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad4000.c | 666 ++++++++++++++++++ 5 files changed, 717 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 create mode 100644 drivers/iio/adc/ad4000.c diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 new file mode 100644 index 000000000000..98fb7539ad6d --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 @@ -0,0 +1,36 @@ +What: /sys/bus/iio/devices/iio:deviceX/turbo_en +KernelVersion: 6.9 +Contact: linux-iio@vger.kernel.org +Description: + This attribute is used to enable/disable turbo mode allowing + slower SPI clock rates (at a minimum SCK rate of 75 MHz) to + achieve the maximum throughput of 2 MSPS. + +What: /sys/bus/iio/devices/iio:deviceX/span_compression_en +KernelVersion: 6.9 +Contact: linux-iio@vger.kernel.org +Description: + This attribute is used to enable/disable the input span + compression feature that reduces the ADC input range by 10% from + the top and bottom of the range while still accessing all + available ADC codes. Enabling span compression causes a + decrease in ADC scale which is reflected in the channel + in_voltageY_scale attribute. + +What: /sys/bus/iio/devices/iio:deviceX/status_bits_en +KernelVersion: 6.9 +Contact: linux-iio@vger.kernel.org +Description: + This attribute is used to make the chip append a 6-bit status + word at the end of conversion results. The 6 status bits contain + the configuration register fields for OV clamp flag, span + compression, high-z mode, and turbo mode. + +What: /sys/bus/iio/devices/iio:deviceX/high_impedance_en +KernelVersion: 6.9 +Contact: linux-iio@vger.kernel.org +Description: + This attribute is used to enable/disable high impedance mode + (high-z) which reduces nonlinear charge kickback to the ADC + input. + diff --git a/MAINTAINERS b/MAINTAINERS index 3ca90f842298..2ae98c700ff0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1140,7 +1140,9 @@ M: Marcelo Schmitt <marcelo.schmitt@analog.com> L: linux-iio@vger.kernel.org S: Supported W: https://ez.analog.com/linux-software-drivers +F: Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml +F: drivers/iio/adc/ad4000.c ANALOG DEVICES INC AD4130 DRIVER M: Cosmin Tanislav <cosmin.tanislav@analog.com> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 0d9282fa67f5..15db35f00ef0 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -21,6 +21,18 @@ config AD_SIGMA_DELTA select IIO_BUFFER select IIO_TRIGGERED_BUFFER +config AD4000 + tristate "Analog Devices AD4000 ADC Driver" + depends on SPI + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER + help + Say yes here to build support for Analog Devices AD4000 high speed + SPI analog to digital converters (ADC). + + To compile this driver as a module, choose M here: the module will be + called ad4000. + config AD4130 tristate "Analog Device AD4130 ADC Driver" depends on SPI diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index b3c434722364..f535d617ae89 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -6,6 +6,7 @@ # When adding new entries keep the list in alphabetical order obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o +obj-$(CONFIG_AD4000) += ad4000.o obj-$(CONFIG_AD4130) += ad4130.o obj-$(CONFIG_AD7091R) += ad7091r-base.o obj-$(CONFIG_AD7091R5) += ad7091r5.o diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c new file mode 100644 index 000000000000..f77104755979 --- /dev/null +++ b/drivers/iio/adc/ad4000.c @@ -0,0 +1,666 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * AD4000 SPI ADC driver + * + * Copyright 2024 Analog Devices Inc. + */ +#include <asm/unaligned.h> +#include <linux/bits.h> +#include <linux/bitfield.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/math.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/gpio/consumer.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> +#include <linux/sysfs.h> +#include <linux/units.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> +#include <linux/iio/buffer.h> +#include <linux/iio/trigger.h> +#include <linux/iio/triggered_buffer.h> +#include <linux/iio/trigger_consumer.h> + +#define AD400X_READ_COMMAND 0x54 +#define AD400X_WRITE_COMMAND 0x14 + +#define AD4000_CONFIG_REG_MSK 0xFF + +/* AD4000 Configuration Register programmable bits */ +#define AD4000_STATUS BIT(4) /* Status bits output */ +#define AD4000_SPAN_COMP BIT(3) /* Input span compression */ +#define AD4000_HIGHZ BIT(2) /* High impedance mode */ +#define AD4000_TURBO BIT(1) /* Turbo mode */ + +#define AD4000_16BIT_MSK GENMASK(31, 16) +#define AD4000_18BIT_MSK GENMASK(31, 14) +#define AD4000_20BIT_MSK GENMASK(31, 12) + +#define AD4000_CHANNEL(_sign, _real_bits) \ + { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .scan_type = { \ + .sign = _sign, \ + .realbits = _real_bits, \ + .storagebits = 32, \ + .endianness = IIO_BE, \ + }, \ + } \ + +enum ad4000_ids { + ID_AD4000, + ID_AD4001, + ID_AD4002, + ID_AD4003, + ID_AD4004, + ID_AD4005, + ID_AD4006, + ID_AD4007, + ID_AD4008, + ID_AD4010, + ID_AD4011, + ID_AD4020, + ID_AD4021, + ID_AD4022, + ID_ADAQ4001, + ID_ADAQ4003, +}; + +struct ad4000_chip_info { + const char *dev_name; + struct iio_chan_spec chan_spec; +}; + +static const struct ad4000_chip_info ad4000_chips[] = { + [ID_AD4000] = { + .dev_name = "ad4000", + .chan_spec = AD4000_CHANNEL('u', 16), + }, + [ID_AD4001] = { + .dev_name = "ad4001", + .chan_spec = AD4000_CHANNEL('s', 16), + }, + [ID_AD4002] = { + .dev_name = "ad4002", + .chan_spec = AD4000_CHANNEL('u', 18), + }, + [ID_AD4003] = { + .dev_name = "ad4003", + .chan_spec = AD4000_CHANNEL('s', 18), + }, + [ID_AD4004] = { + .dev_name = "ad4004", + .chan_spec = AD4000_CHANNEL('u', 16), + }, + [ID_AD4005] = { + .dev_name = "ad4005", + .chan_spec = AD4000_CHANNEL('s', 16), + }, + [ID_AD4006] = { + .dev_name = "ad4006", + .chan_spec = AD4000_CHANNEL('u', 18), + }, + [ID_AD4007] = { + .dev_name = "ad4007", + .chan_spec = AD4000_CHANNEL('s', 18), + }, + [ID_AD4008] = { + .dev_name = "ad4008", + .chan_spec = AD4000_CHANNEL('u', 16), + }, + [ID_AD4010] = { + .dev_name = "ad4010", + .chan_spec = AD4000_CHANNEL('u', 18), + }, + [ID_AD4011] = { + .dev_name = "ad4011", + .chan_spec = AD4000_CHANNEL('s', 18), + }, + [ID_AD4020] = { + .dev_name = "ad4020", + .chan_spec = AD4000_CHANNEL('s', 20), + }, + [ID_AD4021] = { + .dev_name = "ad4021", + .chan_spec = AD4000_CHANNEL('s', 20), + }, + [ID_AD4022] = { + .dev_name = "ad4022", + .chan_spec = AD4000_CHANNEL('s', 20), + }, + [ID_ADAQ4001] = { + .dev_name = "adaq4001", + .chan_spec = AD4000_CHANNEL('s', 16), + }, + [ID_ADAQ4003] = { + .dev_name = "adaq4003", + .chan_spec = AD4000_CHANNEL('s', 18), + }, +}; + +enum ad4000_gains { + AD4000_0454_GAIN = 0, + AD4000_0909_GAIN = 1, + AD4000_1_GAIN = 2, + AD4000_1900_GAIN = 3, + AD4000_GAIN_LEN +}; + +/* + * Gains stored and computed as fractions to avoid introducing rounding erros. + */ +static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = { + [AD4000_0454_GAIN] = { 227, 500 }, + [AD4000_0909_GAIN] = { 909, 1000 }, + [AD4000_1_GAIN] = { 1, 1 }, + [AD4000_1900_GAIN] = { 19, 10 }, +}; + +struct ad4000_state { + struct spi_device *spi; + struct gpio_desc *cnv_gpio; + int vref; + bool status_bits; + bool span_comp; + bool turbo_mode; + bool high_z_mode; + + enum ad4000_gains pin_gain; + int scale_tbl[AD4000_GAIN_LEN][2]; + + /* + * DMA (thus cache coherency maintenance) requires the + * transfer buffers to live in their own cache lines. + */ + union { + struct { + u8 sample_buf[4]; + s64 timestamp; + } scan; + u8 d8[2]; + } data __aligned(IIO_DMA_MINALIGN); +}; + +static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits) +{ + int val, val2, tmp0, tmp1, i; + u64 tmp2; + + val2 = scale_bits; + for (i = 0; i < AD4000_GAIN_LEN; i++) { + val = st->vref / 1000; + /* Multiply by MILLI here to avoid losing precision */ + val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI, + ad4000_gains_frac[i][0]); + /* Would multiply by NANO here but we already multiplied by MILLI */ + tmp2 = shift_right((u64)val * MICRO, val2); + tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1); + st->scale_tbl[i][0] = tmp0; /* Integer part */ + st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */ + } +} + +static int ad4000_write_reg(struct ad4000_state *st, uint8_t val) +{ + struct spi_transfer t = { + .tx_buf = st->data.d8, + .len = 2, + }; + struct spi_message m; + + put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val, st->data.d8); + + spi_message_init(&m); + spi_message_add_tail(&t, &m); + + return spi_sync(st->spi, &m); +} + +static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val) +{ + struct spi_transfer t = {0}; + struct spi_message m; + int ret; + + st->data.d8[0] = AD400X_READ_COMMAND; + + t.rx_buf = st->data.d8; + t.tx_buf = st->data.d8; + t.len = 2; + + spi_message_init_with_transfers(&m, &t, 1); + + ret = spi_sync(st->spi, &m); + if (ret < 0) + return ret; + + *val = FIELD_GET(AD4000_CONFIG_REG_MSK, get_unaligned_be16(st->data.d8)); + + return ret; +} + +static int ad4000_read_sample(struct ad4000_state *st, uint32_t *val) +{ + struct spi_transfer t = {0}; + struct spi_message m; + int ret; + + t.rx_buf = &st->data.scan.sample_buf; + t.len = 4; + t.delay.value = 60; + t.delay.unit = SPI_DELAY_UNIT_NSECS; + + spi_message_init_with_transfers(&m, &t, 1); + + if (st->cnv_gpio) + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH); + + ret = spi_sync(st->spi, &m); + if (ret < 0) + return ret; + + if (st->cnv_gpio) + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW); + + *val = get_unaligned_be32(&st->data.scan.sample_buf); + + return 0; +} + +static int ad4000_single_conversion(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, int *val) +{ + struct ad4000_state *st = iio_priv(indio_dev); + u32 sample; + int ret; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = ad4000_read_sample(st, &sample); + + iio_device_release_direct_mode(indio_dev); + + if (ret) + return ret; + + switch (chan->scan_type.realbits) { + case 16: + sample = FIELD_GET(AD4000_16BIT_MSK, sample); + break; + case 18: + sample = FIELD_GET(AD4000_18BIT_MSK, sample); + break; + case 20: + sample = FIELD_GET(AD4000_20BIT_MSK, sample); + break; + default: + return -EINVAL; + } + + if (chan->scan_type.sign == 's') + *val = sign_extend32(sample, chan->scan_type.realbits - 1); + + return IIO_VAL_INT; +} + +static int ad4000_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long info) +{ + struct ad4000_state *st = iio_priv(indio_dev); + + switch (info) { + case IIO_CHAN_INFO_RAW: + return ad4000_single_conversion(indio_dev, chan, val); + case IIO_CHAN_INFO_SCALE: + *val = st->scale_tbl[st->pin_gain][0]; + *val2 = st->scale_tbl[st->pin_gain][1]; + if (st->span_comp) + *val2 = DIV_ROUND_CLOSEST(*val2 * 4, 5); + return IIO_VAL_INT_PLUS_NANO; + default: + break; + } + + return -EINVAL; +} + +static ssize_t ad4000_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr); + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct ad4000_state *st = iio_priv(indio_dev); + + switch ((u32)this_attr->address) { + case AD4000_STATUS: + return sysfs_emit(buf, "%d\n", st->status_bits); + case AD4000_SPAN_COMP: + return sysfs_emit(buf, "%d\n", st->span_comp); + case AD4000_HIGHZ: + return sysfs_emit(buf, "%d\n", st->high_z_mode); + case AD4000_TURBO: + return sysfs_emit(buf, "%d\n", st->turbo_mode); + default: + return -EINVAL; + } +} + +static ssize_t ad4000_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct ad4000_state *st = iio_priv(indio_dev); + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr); + unsigned int reg_val; + bool val; + int ret; + + ret = kstrtobool(buf, &val); + if (ret < 0) + return ret; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = ad4000_read_reg(st, ®_val); + if (ret < 0) + goto err_release; + + switch ((u32)this_attr->address) { + case AD4000_STATUS: + reg_val &= ~AD4000_STATUS; + reg_val |= FIELD_PREP(AD4000_STATUS, val); + ret = ad4000_write_reg(st, reg_val); + if (ret < 0) + goto err_release; + + st->status_bits = val; + break; + case AD4000_SPAN_COMP: + reg_val &= ~AD4000_SPAN_COMP; + reg_val |= FIELD_PREP(AD4000_SPAN_COMP, val); + ret = ad4000_write_reg(st, reg_val); + if (ret < 0) + goto err_release; + + st->span_comp = val; + break; + case AD4000_HIGHZ: + reg_val &= ~AD4000_HIGHZ; + reg_val |= FIELD_PREP(AD4000_HIGHZ, val); + ret = ad4000_write_reg(st, reg_val); + if (ret < 0) + goto err_release; + + st->high_z_mode = val; + break; + case AD4000_TURBO: + reg_val &= ~AD4000_TURBO; + reg_val |= FIELD_PREP(AD4000_TURBO, val); + ret = ad4000_write_reg(st, reg_val); + if (ret < 0) + goto err_release; + + st->turbo_mode = val; + break; + default: + ret = -EINVAL; + goto err_release; + } + +err_release: + iio_device_release_direct_mode(indio_dev); + return ret ? ret : len; +} + +static IIO_DEVICE_ATTR(status_bits_en, 0644, ad4000_show, ad4000_store, + AD4000_STATUS); + +static IIO_DEVICE_ATTR(span_compression_en, 0644, ad4000_show, ad4000_store, + AD4000_SPAN_COMP); + +static IIO_DEVICE_ATTR(high_impedance_en, 0644, ad4000_show, ad4000_store, + AD4000_HIGHZ); + +static IIO_DEVICE_ATTR(turbo_en, 0644, ad4000_show, ad4000_store, + AD4000_TURBO); + +static struct attribute *ad4000_attributes[] = { + &iio_dev_attr_status_bits_en.dev_attr.attr, + &iio_dev_attr_span_compression_en.dev_attr.attr, + &iio_dev_attr_high_impedance_en.dev_attr.attr, + &iio_dev_attr_turbo_en.dev_attr.attr, + NULL +}; + +static const struct attribute_group ad4000_attribute_group = { + .attrs = ad4000_attributes, +}; + +static irqreturn_t ad4000_trigger_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct ad4000_state *st = iio_priv(indio_dev); + struct spi_transfer t = {0}; + struct spi_message m; + int ret; + + t.rx_buf = &st->data.scan.sample_buf; + t.len = 4; + t.delay.value = 60; + t.delay.unit = SPI_DELAY_UNIT_NSECS; + + spi_message_init_with_transfers(&m, &t, 1); + + if (st->cnv_gpio) + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH); + + ret = spi_sync(st->spi, &m); + if (ret < 0) + goto err_out; + + if (st->cnv_gpio) + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW); + + iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan, + iio_get_time_ns(indio_dev)); + +err_out: + iio_trigger_notify_done(indio_dev->trig); + return IRQ_HANDLED; +} + +static const struct iio_info ad4000_info = { + .read_raw = &ad4000_read_raw, + .attrs = &ad4000_attribute_group, +}; + +static void ad4000_regulator_disable(void *reg) +{ + regulator_disable(reg); +} + +static int ad4000_probe(struct spi_device *spi) +{ + const struct ad4000_chip_info *chip; + struct regulator *vref_reg; + struct iio_dev *indio_dev; + struct ad4000_state *st; + int ret; + + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + chip = (const struct ad4000_chip_info *)device_get_match_data(&spi->dev); + if (!chip) + return -EINVAL; + + st = iio_priv(indio_dev); + st->spi = spi; + + vref_reg = devm_regulator_get(&spi->dev, "vref"); + if (IS_ERR(vref_reg)) + return dev_err_probe(&spi->dev, PTR_ERR(vref_reg), + "Failed to get vref regulator\n"); + + ret = regulator_enable(vref_reg); + if (ret < 0) + return dev_err_probe(&spi->dev, ret, + "Failed to enable voltage regulator\n"); + + ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg); + if (ret) + return dev_err_probe(&spi->dev, ret, + "Failed to add regulator disable action\n"); + + st->vref = regulator_get_voltage(vref_reg); + if (st->vref < 0) + return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n"); + + if (!device_property_present(&spi->dev, "adi,spi-cs-mode")) { + st->cnv_gpio = devm_gpiod_get(&spi->dev, "cnv", GPIOD_OUT_HIGH); + if (IS_ERR(st->cnv_gpio)) { + if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio), + "Failed to get CNV GPIO"); + } + } + + indio_dev->name = chip->dev_name; + indio_dev->info = &ad4000_info; + indio_dev->channels = &chip->chan_spec; + indio_dev->num_channels = 1; + + if (device_property_present(&spi->dev, "adi,gain-milli")) { + u32 val; + + ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val); + if (ret) + return ret; + + switch (val) { + case 454: + st->pin_gain = AD4000_0454_GAIN; + break; + case 909: + st->pin_gain = AD4000_0909_GAIN; + break; + case 1000: + st->pin_gain = AD4000_1_GAIN; + break; + case 1900: + st->pin_gain = AD4000_1900_GAIN; + break; + default: + return dev_err_probe(&spi->dev, -EINVAL, + "Invalid firmware provided gain\n"); + } + } else { + st->pin_gain = AD4000_1_GAIN; + } + + /* + * ADCs that output twos complement code have one less bit to express + * voltage magnitude. + */ + if (chip->chan_spec.scan_type.sign == 's') + ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1); + else + ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits); + + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, + &iio_pollfunc_store_time, + &ad4000_trigger_handler, NULL); + if (ret) + return ret; + + return devm_iio_device_register(&spi->dev, indio_dev); +} + +static const struct spi_device_id ad4000_id[] = { + { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] }, + { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] }, + { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] }, + { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] }, + { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] }, + { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] }, + { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] }, + { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] }, + { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] }, + { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] }, + { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] }, + { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] }, + { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] }, + { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] }, + { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] }, + { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] }, + { } +}; +MODULE_DEVICE_TABLE(spi, ad4000_id); + +static const struct of_device_id ad4000_of_match[] = { + { .compatible = "adi,ad4000", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4000] }, + { .compatible = "adi,ad4001", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4001] }, + { .compatible = "adi,ad4002", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4002] }, + { .compatible = "adi,ad4003", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4003] }, + { .compatible = "adi,ad4004", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4004] }, + { .compatible = "adi,ad4005", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4005] }, + { .compatible = "adi,ad4006", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4006] }, + { .compatible = "adi,ad4007", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4007] }, + { .compatible = "adi,ad4008", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4008] }, + { .compatible = "adi,ad4010", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4010] }, + { .compatible = "adi,ad4011", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4011] }, + { .compatible = "adi,ad4020", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4020] }, + { .compatible = "adi,ad4021", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4021] }, + { .compatible = "adi,ad4022", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4022] }, + { .compatible = "adi,adaq4001", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_ADAQ4001] }, + { .compatible = "adi,adaq4003", + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_ADAQ4003] }, + { } +}; +MODULE_DEVICE_TABLE(of, ad4000_of_match); + +static struct spi_driver ad4000_driver = { + .driver = { + .name = "ad4000", + .of_match_table = ad4000_of_match, + }, + .probe = ad4000_probe, + .id_table = ad4000_id, +}; +module_spi_driver(ad4000_driver); + +MODULE_AUTHOR("Mircea Caprioru <mircea.caprioru@analog.com>"); +MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt@analog.com>"); +MODULE_DESCRIPTION("Analog Devices AD4000 ADC driver"); +MODULE_LICENSE("GPL"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: adc: Add support for AD4000 2024-03-22 22:05 ` [PATCH 2/2] iio: adc: Add support for AD4000 Marcelo Schmitt @ 2024-03-23 19:12 ` Jonathan Cameron 2024-03-23 21:53 ` David Lechner 1 sibling, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2024-03-23 19:12 UTC (permalink / raw) To: Marcelo Schmitt Cc: lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1, linux-iio, devicetree, linux-kernel On Fri, 22 Mar 2024 19:05:34 -0300 Marcelo Schmitt <marcelo.schmitt@analog.com> wrote: > Add support for AD4000 series of high accuracy, high speed, low power, > successive aproximation register (SAR) ADCs. > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > --- > Pasting relevant comment from cover letter here to aid reviewers. > > Differently from AD7944, AD4000 devices have a configuration register to > toggle some features. For instance, turbo mode is set through configuration > register rather than an external pin. This simplifies hardware connections, > but then requires software interface. So, additional ABI being proposed > in sysfs-bus-iio-adc-ad4000. The one I'm most in doubt about is > span_compression_en which affects the in_voltageY_scale attribute. > That might be instead supported by providing _scale_available and allowing write > to _scale. Yes. That's what I suggested inline before reading this properly. Much prefer that as standard tooling will know how to use it. Various comments inline. In particularly look at the spi helpers. It's unusual to see a driver do so much manual handling of transfers. Jonathan > > .../ABI/testing/sysfs-bus-iio-adc-ad4000 | 36 + > MAINTAINERS | 2 + > drivers/iio/adc/Kconfig | 12 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ad4000.c | 666 ++++++++++++++++++ > 5 files changed, 717 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 > create mode 100644 drivers/iio/adc/ad4000.c > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 > new file mode 100644 > index 000000000000..98fb7539ad6d > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 > @@ -0,0 +1,36 @@ > +What: /sys/bus/iio/devices/iio:deviceX/turbo_en > +KernelVersion: 6.9 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to enable/disable turbo mode allowing > + slower SPI clock rates (at a minimum SCK rate of 75 MHz) to > + achieve the maximum throughput of 2 MSPS. When would we turn this on or off from userspace? From this brief description it sounds like either we are trying to run very fast and the SPI bus can't clock quick enough for the non turbo mode. In which case detect that we need it and then turn it on. Right now I have no idea why I'd ever turn this off. Who doesn't want to press the turbo button? > + > +What: /sys/bus/iio/devices/iio:deviceX/span_compression_en > +KernelVersion: 6.9 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to enable/disable the input span > + compression feature that reduces the ADC input range by 10% from > + the top and bottom of the range while still accessing all > + available ADC codes. Enabling span compression causes a > + decrease in ADC scale which is reflected in the channel > + in_voltageY_scale attribute. Can you not control it via in_voltageY_scale then? > + > +What: /sys/bus/iio/devices/iio:deviceX/status_bits_en > +KernelVersion: 6.9 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to make the chip append a 6-bit status > + word at the end of conversion results. The 6 status bits contain > + the configuration register fields for OV clamp flag, span > + compression, high-z mode, and turbo mode. Why would I want this? Which of these isn't something I control? This sounds like a debug feature we shouldn't enable. > + > +What: /sys/bus/iio/devices/iio:deviceX/high_impedance_en > +KernelVersion: 6.9 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to enable/disable high impedance mode > + (high-z) which reduces nonlinear charge kickback to the ADC > + input. For this one, do we have precendence in other drivers? I'm find with a control just not sure if this the write ABI to use. > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index b3c434722364..f535d617ae89 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -6,6 +6,7 @@ > # When adding new entries keep the list in alphabetical order > obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o > obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o > +obj-$(CONFIG_AD4000) += ad4000.o > obj-$(CONFIG_AD4130) += ad4130.o > obj-$(CONFIG_AD7091R) += ad7091r-base.o > obj-$(CONFIG_AD7091R5) += ad7091r5.o > diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c > new file mode 100644 > index 000000000000..f77104755979 > --- /dev/null > +++ b/drivers/iio/adc/ad4000.c > @@ -0,0 +1,666 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * AD4000 SPI ADC driver > + * > + * Copyright 2024 Analog Devices Inc. > + */ > +#include <asm/unaligned.h> > +#include <linux/bits.h> > +#include <linux/bitfield.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/kernel.h> > +#include <linux/math.h> > +#include <linux/module.h> > +#include <linux/of.h> Why? Probably want mod_devicetable.h > +#include <linux/gpio/consumer.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > +#include <linux/sysfs.h> > +#include <linux/units.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/trigger.h> You aren't providing a trigger so shouldn't need this one I think. > +#include <linux/iio/triggered_buffer.h> > +#include <linux/iio/trigger_consumer.h> > + > +#define AD400X_READ_COMMAND 0x54 > +#define AD400X_WRITE_COMMAND 0x14 > + > +#define AD4000_CONFIG_REG_MSK 0xFF > + > +/* AD4000 Configuration Register programmable bits */ > +#define AD4000_STATUS BIT(4) /* Status bits output */ > +#define AD4000_SPAN_COMP BIT(3) /* Input span compression */ > +#define AD4000_HIGHZ BIT(2) /* High impedance mode */ > +#define AD4000_TURBO BIT(1) /* Turbo mode */ > + > +#define AD4000_16BIT_MSK GENMASK(31, 16) > +#define AD4000_18BIT_MSK GENMASK(31, 14) > +#define AD4000_20BIT_MSK GENMASK(31, 12) > + > +#define AD4000_CHANNEL(_sign, _real_bits) \ > + { \ > + .type = IIO_VOLTAGE, \ > + .indexed = 1, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_SCALE), \ > + .scan_type = { \ > + .sign = _sign, \ > + .realbits = _real_bits, \ > + .storagebits = 32, \ Why store a 16 bit value in 32 bits? Obviously will make code more complex to handling it differently but the memory saving could be significant if the buffer is large. > + .endianness = IIO_BE, \ > + }, \ > + } \ > + > + > +static int ad4000_write_reg(struct ad4000_state *st, uint8_t val) > +{ > + struct spi_transfer t = { > + .tx_buf = st->data.d8, > + .len = 2, > + }; > + struct spi_message m; > + > + put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val, st->data.d8); > + > + spi_message_init(&m); > + spi_message_add_tail(&t, &m); Why can't use init_with_transfers? Though maybe spi_write() is enough here and in other cases. + look at spi_write_then_read() and see if that's appropriate for others. > + > + return spi_sync(st->spi, &m); > +} > + > +static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val) > +{ > + struct spi_transfer t = {0}; > + struct spi_message m; > + int ret; > + > + st->data.d8[0] = AD400X_READ_COMMAND; > + > + t.rx_buf = st->data.d8; > + t.tx_buf = st->data.d8; > + t.len = 2; As below on structure initialization. > + > + spi_message_init_with_transfers(&m, &t, 1); > + > + ret = spi_sync(st->spi, &m); > + if (ret < 0) > + return ret; > + > + *val = FIELD_GET(AD4000_CONFIG_REG_MSK, get_unaligned_be16(st->data.d8)); > + > + return ret; > +} > + > +static int ad4000_read_sample(struct ad4000_state *st, uint32_t *val) > +{ > + struct spi_transfer t = {0}; > + struct spi_message m; > + int ret; > + > + t.rx_buf = &st->data.scan.sample_buf; > + t.len = 4; > + t.delay.value = 60; > + t.delay.unit = SPI_DELAY_UNIT_NSECS; Similar to below, use c99 style structure initialization. > + > + spi_message_init_with_transfers(&m, &t, 1); > + > + if (st->cnv_gpio) > + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH); > + > + ret = spi_sync(st->spi, &m); > + if (ret < 0) > + return ret; > + > + if (st->cnv_gpio) > + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW); > + > + *val = get_unaligned_be32(&st->data.scan.sample_buf); > + > + return 0; > +} > + > +static int ad4000_single_conversion(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, int *val) > +{ > + struct ad4000_state *st = iio_priv(indio_dev); > + u32 sample; > + int ret; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = ad4000_read_sample(st, &sample); > + > + iio_device_release_direct_mode(indio_dev); > + > + if (ret) > + return ret; Scoped version works here too to avoid the need for delaying the error check. (see below) > + > + switch (chan->scan_type.realbits) { > + case 16: > + sample = FIELD_GET(AD4000_16BIT_MSK, sample); > + break; > + case 18: > + sample = FIELD_GET(AD4000_18BIT_MSK, sample); > + break; > + case 20: > + sample = FIELD_GET(AD4000_20BIT_MSK, sample); > + break; > + default: > + return -EINVAL; > + } > + > + if (chan->scan_type.sign == 's') > + *val = sign_extend32(sample, chan->scan_type.realbits - 1); > + > + return IIO_VAL_INT; > +} > +static ssize_t ad4000_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + struct iio_dev *indio_dev = dev_to_iio_dev(dev); > + struct ad4000_state *st = iio_priv(indio_dev); > + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr); > + unsigned int reg_val; > + bool val; > + int ret; > + > + ret = kstrtobool(buf, &val); > + if (ret < 0) > + return ret; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = ad4000_read_reg(st, ®_val); > + if (ret < 0) > + goto err_release; > + > + switch ((u32)this_attr->address) { > + case AD4000_STATUS: > + reg_val &= ~AD4000_STATUS; > + reg_val |= FIELD_PREP(AD4000_STATUS, val); > + ret = ad4000_write_reg(st, reg_val); > + if (ret < 0) > + goto err_release; > + > + st->status_bits = val; > + break; > + case AD4000_SPAN_COMP: > + reg_val &= ~AD4000_SPAN_COMP; > + reg_val |= FIELD_PREP(AD4000_SPAN_COMP, val); > + ret = ad4000_write_reg(st, reg_val); > + if (ret < 0) > + goto err_release; > + > + st->span_comp = val; > + break; > + case AD4000_HIGHZ: > + reg_val &= ~AD4000_HIGHZ; > + reg_val |= FIELD_PREP(AD4000_HIGHZ, val); > + ret = ad4000_write_reg(st, reg_val); > + if (ret < 0) > + goto err_release; > + > + st->high_z_mode = val; > + break; > + case AD4000_TURBO: > + reg_val &= ~AD4000_TURBO; > + reg_val |= FIELD_PREP(AD4000_TURBO, val); > + ret = ad4000_write_reg(st, reg_val); > + if (ret < 0) > + goto err_release; > + > + st->turbo_mode = val; > + break; > + default: > + ret = -EINVAL; > + goto err_release; > + } > + > +err_release: > + iio_device_release_direct_mode(indio_dev); The scoped cleanup.h based version of this is no upstream and would clean# this code up a lot by allowing early returns See iio_device_claim_direct_scoped() > + return ret ? ret : len; > +} > +static irqreturn_t ad4000_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct ad4000_state *st = iio_priv(indio_dev); > + struct spi_transfer t = {0}; Spaces around the 0, though technically I think you just need {}; > + struct spi_message m; > + int ret; > + > + t.rx_buf = &st->data.scan.sample_buf; > + t.len = 4; > + t.delay.value = 60; > + t.delay.unit = SPI_DELAY_UNIT_NSECS; Better still. struct spi_transfer t = { .rx_buf = &... etc }; > + > + spi_message_init_with_transfers(&m, &t, 1); > + > + if (st->cnv_gpio) > + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH); > + > + ret = spi_sync(st->spi, &m); > + if (ret < 0) > + goto err_out; > + > + if (st->cnv_gpio) > + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW); > + > + iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan, > + iio_get_time_ns(indio_dev)); > + > +err_out: > + iio_trigger_notify_done(indio_dev->trig); > + return IRQ_HANDLED; > +} > + > +static const struct iio_info ad4000_info = { > + .read_raw = &ad4000_read_raw, > + .attrs = &ad4000_attribute_group, > +}; > + > +static void ad4000_regulator_disable(void *reg) > +{ > + regulator_disable(reg); > +} > + > +static int ad4000_probe(struct spi_device *spi) > +{ > + const struct ad4000_chip_info *chip; > + struct regulator *vref_reg; > + struct iio_dev *indio_dev; > + struct ad4000_state *st; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + chip = (const struct ad4000_chip_info *)device_get_match_data(&spi->dev); Shouldn't need the cast. It's casting const void * to another const pointer which the c spec allows to be done implicitly > + if (!chip) > + return -EINVAL; > + > + st = iio_priv(indio_dev); > + st->spi = spi; > + > + vref_reg = devm_regulator_get(&spi->dev, "vref"); I'm guessing there are other power supplies? You should enable them as well and given you don't use the voltage of the others you can just use the calls to get them enabled. > + if (IS_ERR(vref_reg)) > + return dev_err_probe(&spi->dev, PTR_ERR(vref_reg), > + "Failed to get vref regulator\n"); > + > + ret = regulator_enable(vref_reg); > + if (ret < 0) > + return dev_err_probe(&spi->dev, ret, > + "Failed to enable voltage regulator\n"); > + > + ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg); > + if (ret) > + return dev_err_probe(&spi->dev, ret, > + "Failed to add regulator disable action\n"); > + > + st->vref = regulator_get_voltage(vref_reg); > + if (st->vref < 0) > + return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n"); > + > + if (!device_property_present(&spi->dev, "adi,spi-cs-mode")) { > + st->cnv_gpio = devm_gpiod_get(&spi->dev, "cnv", GPIOD_OUT_HIGH); > + if (IS_ERR(st->cnv_gpio)) { > + if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + > + return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio), > + "Failed to get CNV GPIO"); > + } > + } > + > + indio_dev->name = chip->dev_name; > + indio_dev->info = &ad4000_info; > + indio_dev->channels = &chip->chan_spec; > + indio_dev->num_channels = 1; > + > + if (device_property_present(&spi->dev, "adi,gain-milli")) { > + u32 val; > + > + ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val); > + if (ret) > + return ret; > + > + switch (val) { > + case 454: > + st->pin_gain = AD4000_0454_GAIN; > + break; > + case 909: > + st->pin_gain = AD4000_0909_GAIN; > + break; > + case 1000: > + st->pin_gain = AD4000_1_GAIN; > + break; > + case 1900: > + st->pin_gain = AD4000_1900_GAIN; > + break; > + default: > + return dev_err_probe(&spi->dev, -EINVAL, > + "Invalid firmware provided gain\n"); > + } > + } else { > + st->pin_gain = AD4000_1_GAIN; For defaults, a nice pattern is to just set them before the attempt to read the property and don't update them if the property isn't available. st->pin_gain = AD4000_1_GAIN; if (device_property_present(&spi->dev, "adi,gain-milli")) { ... } > + } > +static const struct spi_device_id ad4000_id[] = { > + { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] }, > + { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] }, > + { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] }, > + { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] }, > + { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] }, > + { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] }, > + { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] }, > + { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] }, > + { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] }, > + { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] }, > + { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] }, > + { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] }, > + { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] }, > + { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] }, > + { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] }, > + { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] }, > + { } > +}; > +MODULE_DEVICE_TABLE(spi, ad4000_id); > + > +static const struct of_device_id ad4000_of_match[] = { > + { .compatible = "adi,ad4000", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4000] }, Why the casts? Shouldn't need them as data is a const void * and you are casting from another const pointer. > + { .compatible = "adi,ad4001", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4001] }, > + { .compatible = "adi,ad4002", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4002] }, > + { .compatible = "adi,ad4003", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4003] }, > + { .compatible = "adi,ad4004", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4004] }, > + { .compatible = "adi,ad4005", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4005] }, > + { .compatible = "adi,ad4006", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4006] }, > + { .compatible = "adi,ad4007", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4007] }, > + { .compatible = "adi,ad4008", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4008] }, > + { .compatible = "adi,ad4010", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4010] }, > + { .compatible = "adi,ad4011", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4011] }, > + { .compatible = "adi,ad4020", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4020] }, > + { .compatible = "adi,ad4021", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4021] }, > + { .compatible = "adi,ad4022", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_AD4022] }, > + { .compatible = "adi,adaq4001", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_ADAQ4001] }, > + { .compatible = "adi,adaq4003", > + .data = (struct ad4000_chip_info *)&ad4000_chips[ID_ADAQ4003] }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ad4000_of_match); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: adc: Add support for AD4000 2024-03-22 22:05 ` [PATCH 2/2] iio: adc: Add support for AD4000 Marcelo Schmitt 2024-03-23 19:12 ` Jonathan Cameron @ 2024-03-23 21:53 ` David Lechner 2024-03-24 12:45 ` Jonathan Cameron 2024-03-25 13:35 ` David Lechner 1 sibling, 2 replies; 14+ messages in thread From: David Lechner @ 2024-03-23 21:53 UTC (permalink / raw) To: Marcelo Schmitt Cc: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1, linux-iio, devicetree, linux-kernel On Fri, Mar 22, 2024 at 5:06 PM Marcelo Schmitt <marcelo.schmitt@analog.com> wrote: > > Add support for AD4000 series of high accuracy, high speed, low power, > successive aproximation register (SAR) ADCs. > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > --- > Pasting relevant comment from cover letter here to aid reviewers. > > Differently from AD7944, AD4000 devices have a configuration register to > toggle some features. For instance, turbo mode is set through configuration > register rather than an external pin. This simplifies hardware connections, > but then requires software interface. So, additional ABI being proposed > in sysfs-bus-iio-adc-ad4000. The one I'm most in doubt about is > span_compression_en which affects the in_voltageY_scale attribute. > That might be instead supported by providing _scale_available and allowing write > to _scale. > > .../ABI/testing/sysfs-bus-iio-adc-ad4000 | 36 + > MAINTAINERS | 2 + > drivers/iio/adc/Kconfig | 12 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ad4000.c | 666 ++++++++++++++++++ > 5 files changed, 717 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 > create mode 100644 drivers/iio/adc/ad4000.c > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 > new file mode 100644 > index 000000000000..98fb7539ad6d > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 > @@ -0,0 +1,36 @@ > +What: /sys/bus/iio/devices/iio:deviceX/turbo_en > +KernelVersion: 6.9 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to enable/disable turbo mode allowing > + slower SPI clock rates (at a minimum SCK rate of 75 MHz) to > + achieve the maximum throughput of 2 MSPS. > + > +What: /sys/bus/iio/devices/iio:deviceX/span_compression_en > +KernelVersion: 6.9 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to enable/disable the input span > + compression feature that reduces the ADC input range by 10% from > + the top and bottom of the range while still accessing all > + available ADC codes. Enabling span compression causes a > + decrease in ADC scale which is reflected in the channel > + in_voltageY_scale attribute. > + > +What: /sys/bus/iio/devices/iio:deviceX/status_bits_en > +KernelVersion: 6.9 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to make the chip append a 6-bit status > + word at the end of conversion results. The 6 status bits contain > + the configuration register fields for OV clamp flag, span > + compression, high-z mode, and turbo mode. > + I agree with Jonathan that the three attributes above are not needed (for the reasons he mentioned). > +What: /sys/bus/iio/devices/iio:deviceX/high_impedance_en > +KernelVersion: 6.9 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to enable/disable high impedance mode > + (high-z) which reduces nonlinear charge kickback to the ADC > + input. > + As I mentioned in the DT patch, this one seems like it should be a DT property, not a runtime setting. > diff --git a/MAINTAINERS b/MAINTAINERS > index 3ca90f842298..2ae98c700ff0 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1140,7 +1140,9 @@ M: Marcelo Schmitt <marcelo.schmitt@analog.com> > L: linux-iio@vger.kernel.org > S: Supported > W: https://ez.analog.com/linux-software-drivers > +F: Documentation/ABI/testing/sysfs-bus-iio-adc-ad4000 > F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml > +F: drivers/iio/adc/ad4000.c > > ANALOG DEVICES INC AD4130 DRIVER > M: Cosmin Tanislav <cosmin.tanislav@analog.com> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 0d9282fa67f5..15db35f00ef0 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -21,6 +21,18 @@ config AD_SIGMA_DELTA > select IIO_BUFFER > select IIO_TRIGGERED_BUFFER > > +config AD4000 > + tristate "Analog Devices AD4000 ADC Driver" > + depends on SPI > + select IIO_BUFFER > + select IIO_TRIGGERED_BUFFER > + help > + Say yes here to build support for Analog Devices AD4000 high speed > + SPI analog to digital converters (ADC). > + > + To compile this driver as a module, choose M here: the module will be > + called ad4000. > + > config AD4130 > tristate "Analog Device AD4130 ADC Driver" > depends on SPI > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index b3c434722364..f535d617ae89 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -6,6 +6,7 @@ > # When adding new entries keep the list in alphabetical order > obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o > obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o > +obj-$(CONFIG_AD4000) += ad4000.o > obj-$(CONFIG_AD4130) += ad4130.o > obj-$(CONFIG_AD7091R) += ad7091r-base.o > obj-$(CONFIG_AD7091R5) += ad7091r5.o > diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c > new file mode 100644 > index 000000000000..f77104755979 > --- /dev/null > +++ b/drivers/iio/adc/ad4000.c > @@ -0,0 +1,666 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * AD4000 SPI ADC driver > + * > + * Copyright 2024 Analog Devices Inc. > + */ > +#include <asm/unaligned.h> > +#include <linux/bits.h> > +#include <linux/bitfield.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/kernel.h> > +#include <linux/math.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/gpio/consumer.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > +#include <linux/sysfs.h> > +#include <linux/units.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/trigger.h> > +#include <linux/iio/triggered_buffer.h> > +#include <linux/iio/trigger_consumer.h> > + > +#define AD400X_READ_COMMAND 0x54 > +#define AD400X_WRITE_COMMAND 0x14 > + > +#define AD4000_CONFIG_REG_MSK 0xFF > + > +/* AD4000 Configuration Register programmable bits */ > +#define AD4000_STATUS BIT(4) /* Status bits output */ > +#define AD4000_SPAN_COMP BIT(3) /* Input span compression */ > +#define AD4000_HIGHZ BIT(2) /* High impedance mode */ > +#define AD4000_TURBO BIT(1) /* Turbo mode */ > + > +#define AD4000_16BIT_MSK GENMASK(31, 16) > +#define AD4000_18BIT_MSK GENMASK(31, 14) > +#define AD4000_20BIT_MSK GENMASK(31, 12) > + > +#define AD4000_CHANNEL(_sign, _real_bits) \ > + { \ > + .type = IIO_VOLTAGE, \ > + .indexed = 1, \ Some chips are fully differential and some are pseudo-differential. For the fully differential chips, I would expect .differential = 1, As discussed on other recent drivers, psudo-differential are .differential = 0 though. > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_SCALE), \ > + .scan_type = { \ > + .sign = _sign, \ > + .realbits = _real_bits, \ > + .storagebits = 32, \ > + .endianness = IIO_BE, \ I think this should be IIO_CPU and we should make use of .bits_per_word in the SPI xfers like we do in the ad7944 driver. Otherwise, I think we might have difficutly to achieve max sample rate for the 18 and 20-bit chips once we get SPI offload support added. > + }, \ > + } \ > + > +enum ad4000_ids { > + ID_AD4000, > + ID_AD4001, > + ID_AD4002, > + ID_AD4003, > + ID_AD4004, > + ID_AD4005, > + ID_AD4006, > + ID_AD4007, > + ID_AD4008, > + ID_AD4010, > + ID_AD4011, > + ID_AD4020, > + ID_AD4021, > + ID_AD4022, > + ID_ADAQ4001, > + ID_ADAQ4003, > +}; IMHO, these types of enums just make more work (noise) for people reading the code and don't add anything useful. > + > +struct ad4000_chip_info { > + const char *dev_name; > + struct iio_chan_spec chan_spec; > +}; > + > +static const struct ad4000_chip_info ad4000_chips[] = { > + [ID_AD4000] = { > + .dev_name = "ad4000", > + .chan_spec = AD4000_CHANNEL('u', 16), > + }, > + [ID_AD4001] = { > + .dev_name = "ad4001", > + .chan_spec = AD4000_CHANNEL('s', 16), > + }, > + [ID_AD4002] = { > + .dev_name = "ad4002", > + .chan_spec = AD4000_CHANNEL('u', 18), > + }, > + [ID_AD4003] = { > + .dev_name = "ad4003", > + .chan_spec = AD4000_CHANNEL('s', 18), > + }, > + [ID_AD4004] = { > + .dev_name = "ad4004", > + .chan_spec = AD4000_CHANNEL('u', 16), > + }, > + [ID_AD4005] = { > + .dev_name = "ad4005", > + .chan_spec = AD4000_CHANNEL('s', 16), > + }, > + [ID_AD4006] = { > + .dev_name = "ad4006", > + .chan_spec = AD4000_CHANNEL('u', 18), > + }, > + [ID_AD4007] = { > + .dev_name = "ad4007", > + .chan_spec = AD4000_CHANNEL('s', 18), > + }, > + [ID_AD4008] = { > + .dev_name = "ad4008", > + .chan_spec = AD4000_CHANNEL('u', 16), > + }, > + [ID_AD4010] = { > + .dev_name = "ad4010", > + .chan_spec = AD4000_CHANNEL('u', 18), > + }, > + [ID_AD4011] = { > + .dev_name = "ad4011", > + .chan_spec = AD4000_CHANNEL('s', 18), > + }, > + [ID_AD4020] = { > + .dev_name = "ad4020", > + .chan_spec = AD4000_CHANNEL('s', 20), > + }, > + [ID_AD4021] = { > + .dev_name = "ad4021", > + .chan_spec = AD4000_CHANNEL('s', 20), > + }, > + [ID_AD4022] = { > + .dev_name = "ad4022", > + .chan_spec = AD4000_CHANNEL('s', 20), > + }, > + [ID_ADAQ4001] = { > + .dev_name = "adaq4001", > + .chan_spec = AD4000_CHANNEL('s', 16), > + }, > + [ID_ADAQ4003] = { > + .dev_name = "adaq4003", > + .chan_spec = AD4000_CHANNEL('s', 18), > + }, > +}; As mentioned in my DT bindings reply, I think it would be helpful for reviewers (and git history) to move adding ADAQ support to a separate patch since they have some significant differences from the AD parts. > + > +enum ad4000_gains { > + AD4000_0454_GAIN = 0, > + AD4000_0909_GAIN = 1, > + AD4000_1_GAIN = 2, > + AD4000_1900_GAIN = 3, > + AD4000_GAIN_LEN > +}; > + > +/* > + * Gains stored and computed as fractions to avoid introducing rounding erros. spelling: errors > + */ > +static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = { > + [AD4000_0454_GAIN] = { 227, 500 }, > + [AD4000_0909_GAIN] = { 909, 1000 }, > + [AD4000_1_GAIN] = { 1, 1 }, > + [AD4000_1900_GAIN] = { 19, 10 }, > +}; > + > +struct ad4000_state { > + struct spi_device *spi; > + struct gpio_desc *cnv_gpio; > + int vref; > + bool status_bits; > + bool span_comp; > + bool turbo_mode; > + bool high_z_mode; > + > + enum ad4000_gains pin_gain; > + int scale_tbl[AD4000_GAIN_LEN][2]; > + > + /* > + * DMA (thus cache coherency maintenance) requires the > + * transfer buffers to live in their own cache lines. > + */ > + union { > + struct { > + u8 sample_buf[4]; > + s64 timestamp; Usually we see __aligned(8) applied to the timestamp (I'm guessing some archs need it?) > + } scan; > + u8 d8[2]; > + } data __aligned(IIO_DMA_MINALIGN); > +}; > + > +static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits) > +{ > + int val, val2, tmp0, tmp1, i; > + u64 tmp2; > + > + val2 = scale_bits; > + for (i = 0; i < AD4000_GAIN_LEN; i++) { > + val = st->vref / 1000; > + /* Multiply by MILLI here to avoid losing precision */ > + val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI, > + ad4000_gains_frac[i][0]); > + /* Would multiply by NANO here but we already multiplied by MILLI */ > + tmp2 = shift_right((u64)val * MICRO, val2); > + tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1); > + st->scale_tbl[i][0] = tmp0; /* Integer part */ > + st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */ > + } > +} > + > +static int ad4000_write_reg(struct ad4000_state *st, uint8_t val) > +{ > + struct spi_transfer t = { > + .tx_buf = st->data.d8, > + .len = 2, > + }; > + struct spi_message m; > + > + put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val, st->data.d8); > + > + spi_message_init(&m); > + spi_message_add_tail(&t, &m); > + > + return spi_sync(st->spi, &m); > +} > + > +static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val) > +{ > + struct spi_transfer t = {0}; > + struct spi_message m; > + int ret; > + > + st->data.d8[0] = AD400X_READ_COMMAND; > + > + t.rx_buf = st->data.d8; > + t.tx_buf = st->data.d8; > + t.len = 2; > + > + spi_message_init_with_transfers(&m, &t, 1); > + > + ret = spi_sync(st->spi, &m); > + if (ret < 0) > + return ret; > + > + *val = FIELD_GET(AD4000_CONFIG_REG_MSK, get_unaligned_be16(st->data.d8)); > + > + return ret; > +} > + > +static int ad4000_read_sample(struct ad4000_state *st, uint32_t *val) As with the ad7944 driver, I would expect different handling for the different SPI wiring modes. This looks like it only works with 4-wire mode. > +{ > + struct spi_transfer t = {0}; > + struct spi_message m; > + int ret; > + > + t.rx_buf = &st->data.scan.sample_buf; > + t.len = 4; > + t.delay.value = 60; > + t.delay.unit = SPI_DELAY_UNIT_NSECS; > + > + spi_message_init_with_transfers(&m, &t, 1); > + > + if (st->cnv_gpio) > + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH); > + > + ret = spi_sync(st->spi, &m); > + if (ret < 0) > + return ret; > + > + if (st->cnv_gpio) > + gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW); > + > + *val = get_unaligned_be32(&st->data.scan.sample_buf); > + > + return 0; > +} > + > +static int ad4000_single_conversion(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, int *val) > +{ > + struct ad4000_state *st = iio_priv(indio_dev); > + u32 sample; > + int ret; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = ad4000_read_sample(st, &sample); > + > + iio_device_release_direct_mode(indio_dev); > + > + if (ret) > + return ret; > + > + switch (chan->scan_type.realbits) { > + case 16: > + sample = FIELD_GET(AD4000_16BIT_MSK, sample); > + break; > + case 18: > + sample = FIELD_GET(AD4000_18BIT_MSK, sample); > + break; > + case 20: > + sample = FIELD_GET(AD4000_20BIT_MSK, sample); > + break; > + default: > + return -EINVAL; > + } > + > + if (chan->scan_type.sign == 's') > + *val = sign_extend32(sample, chan->scan_type.realbits - 1); > + > + return IIO_VAL_INT; > +} > + > +static int ad4000_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long info) > +{ > + struct ad4000_state *st = iio_priv(indio_dev); > + > + switch (info) { > + case IIO_CHAN_INFO_RAW: > + return ad4000_single_conversion(indio_dev, chan, val); > + case IIO_CHAN_INFO_SCALE: > + *val = st->scale_tbl[st->pin_gain][0]; > + *val2 = st->scale_tbl[st->pin_gain][1]; > + if (st->span_comp) > + *val2 = DIV_ROUND_CLOSEST(*val2 * 4, 5); > + return IIO_VAL_INT_PLUS_NANO; > + default: > + break; > + } > + > + return -EINVAL; > +} > + ... > +static irqreturn_t ad4000_trigger_handler(int irq, void *p) I would expect different handling for the different SPI wiring modes here too. > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct ad4000_state *st = iio_priv(indio_dev); > + struct spi_transfer t = {0}; > + struct spi_message m; > + int ret; > + > + t.rx_buf = &st->data.scan.sample_buf; > + t.len = 4; > + t.delay.value = 60; > + t.delay.unit = SPI_DELAY_UNIT_NSECS; > + > + spi_message_init_with_transfers(&m, &t, 1); > + > + if (st->cnv_gpio) > + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH); > + > + ret = spi_sync(st->spi, &m); > + if (ret < 0) > + goto err_out; > + > + if (st->cnv_gpio) > + gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW); > + > + iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan, > + iio_get_time_ns(indio_dev)); > + > +err_out: > + iio_trigger_notify_done(indio_dev->trig); > + return IRQ_HANDLED; > +} > + > +static const struct iio_info ad4000_info = { > + .read_raw = &ad4000_read_raw, > + .attrs = &ad4000_attribute_group, > +}; > + > +static void ad4000_regulator_disable(void *reg) > +{ > + regulator_disable(reg); > +} > + > +static int ad4000_probe(struct spi_device *spi) > +{ > + const struct ad4000_chip_info *chip; > + struct regulator *vref_reg; > + struct iio_dev *indio_dev; > + struct ad4000_state *st; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + chip = (const struct ad4000_chip_info *)device_get_match_data(&spi->dev); Should this be spi_get_device_match_data()? Also don't need the cast here. > + if (!chip) > + return -EINVAL; > + > + st = iio_priv(indio_dev); > + st->spi = spi; > + > + vref_reg = devm_regulator_get(&spi->dev, "vref"); This should to be devm_regulator_get_optional(), otherwise it can return a "dummy" regulator if one is missing in the devicetree which will fail when getting the voltage. > + if (IS_ERR(vref_reg)) > + return dev_err_probe(&spi->dev, PTR_ERR(vref_reg), > + "Failed to get vref regulator\n"); > + > + ret = regulator_enable(vref_reg); > + if (ret < 0) > + return dev_err_probe(&spi->dev, ret, > + "Failed to enable voltage regulator\n"); > + > + ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg); > + if (ret) > + return dev_err_probe(&spi->dev, ret, > + "Failed to add regulator disable action\n"); > + > + st->vref = regulator_get_voltage(vref_reg); > + if (st->vref < 0) > + return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n"); > + > + if (!device_property_present(&spi->dev, "adi,spi-cs-mode")) { > + st->cnv_gpio = devm_gpiod_get(&spi->dev, "cnv", GPIOD_OUT_HIGH); > + if (IS_ERR(st->cnv_gpio)) { > + if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + > + return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio), > + "Failed to get CNV GPIO"); > + } > + } Since the DT bindings should be the same for the SPI wirting modes, we should have the same property and logic as the ad7944 driver here. > + > + indio_dev->name = chip->dev_name; > + indio_dev->info = &ad4000_info; > + indio_dev->channels = &chip->chan_spec; > + indio_dev->num_channels = 1; > + > + if (device_property_present(&spi->dev, "adi,gain-milli")) { > + u32 val; > + > + ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val); > + if (ret) > + return ret; > + > + switch (val) { > + case 454: > + st->pin_gain = AD4000_0454_GAIN; > + break; > + case 909: > + st->pin_gain = AD4000_0909_GAIN; > + break; > + case 1000: > + st->pin_gain = AD4000_1_GAIN; > + break; > + case 1900: > + st->pin_gain = AD4000_1900_GAIN; > + break; > + default: > + return dev_err_probe(&spi->dev, -EINVAL, > + "Invalid firmware provided gain\n"); > + } > + } else { > + st->pin_gain = AD4000_1_GAIN; > + } > + > + /* > + * ADCs that output twos complement code have one less bit to express > + * voltage magnitude. > + */ > + if (chip->chan_spec.scan_type.sign == 's') > + ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1); > + else > + ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits); > + AFAICT, the gain stuff only applies to ADAQ chips, so it seems odd to call this for all chips (and have the scale attribute report this for chips that don't support it). > + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, > + &iio_pollfunc_store_time, > + &ad4000_trigger_handler, NULL); > + if (ret) > + return ret; > + > + return devm_iio_device_register(&spi->dev, indio_dev); > +} > + ... ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: adc: Add support for AD4000 2024-03-23 21:53 ` David Lechner @ 2024-03-24 12:45 ` Jonathan Cameron 2024-03-25 13:35 ` David Lechner 1 sibling, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2024-03-24 12:45 UTC (permalink / raw) To: David Lechner Cc: Marcelo Schmitt, lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1, linux-iio, devicetree, linux-kernel On Sat, 23 Mar 2024 16:53:17 -0500 > > + > > + /* > > + * DMA (thus cache coherency maintenance) requires the > > + * transfer buffers to live in their own cache lines. > > + */ > > + union { > > + struct { > > + u8 sample_buf[4]; > > + s64 timestamp; > > Usually we see __aligned(8) applied to the timestamp (I'm guessing > some archs need it?) > Good spot. Yes, x86_32 is the one that we most commonly refer to for this. It aligns s64 to only 32 bits whereas IIO ABI is always naturally aligned. > > + } scan; > > + u8 d8[2]; > > + } data __aligned(IIO_DMA_MINALIGN); > > +}; ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] iio: adc: Add support for AD4000 2024-03-23 21:53 ` David Lechner 2024-03-24 12:45 ` Jonathan Cameron @ 2024-03-25 13:35 ` David Lechner 1 sibling, 0 replies; 14+ messages in thread From: David Lechner @ 2024-03-25 13:35 UTC (permalink / raw) To: Marcelo Schmitt Cc: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt, conor+dt, marcelo.schmitt1, linux-iio, devicetree, linux-kernel On Sat, Mar 23, 2024 at 4:53 PM David Lechner <dlechner@baylibre.com> wrote: > > On Fri, Mar 22, 2024 at 5:06 PM Marcelo Schmitt > <marcelo.schmitt@analog.com> wrote: > > ... > > + > > + vref_reg = devm_regulator_get(&spi->dev, "vref"); > > This should to be devm_regulator_get_optional(), otherwise it can > return a "dummy" regulator if one is missing in the devicetree which > will fail when getting the voltage. > > > + if (IS_ERR(vref_reg)) > > + return dev_err_probe(&spi->dev, PTR_ERR(vref_reg), > > + "Failed to get vref regulator\n"); > > + > > + ret = regulator_enable(vref_reg); > > + if (ret < 0) > > + return dev_err_probe(&spi->dev, ret, > > + "Failed to enable voltage regulator\n"); > > + > > + ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg); > > + if (ret) > > + return dev_err_probe(&spi->dev, ret, > > + "Failed to add regulator disable action\n"); > > + > > + st->vref = regulator_get_voltage(vref_reg); > > + if (st->vref < 0) > > + return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n"); > > + > > + if (!device_property_present(&spi->dev, "adi,spi-cs-mode")) { > > + st->cnv_gpio = devm_gpiod_get(&spi->dev, "cnv", GPIOD_OUT_HIGH); > > + if (IS_ERR(st->cnv_gpio)) { > > + if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER) > > + return -EPROBE_DEFER; > > + > > + return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio), > > + "Failed to get CNV GPIO"); > > + } > > + } > In a review for a different patch, Jonathan said he would prefer devm_regulator_get() and failing in regulator_get_voltage() rather than using devm_regulator_get_optional() so I think the same would apply here and my suggestion should be overruled. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-03-25 13:35 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-22 22:04 [PATCH 0/2] Add support for AD4000 series Marcelo Schmitt 2024-03-22 22:05 ` [PATCH 1/2] dt-bindings: iio: adc: Add AD4000 Marcelo Schmitt 2024-03-22 23:28 ` Rob Herring 2024-03-23 3:29 ` Marcelo Schmitt 2024-03-23 10:58 ` Krzysztof Kozlowski 2024-03-23 18:44 ` Jonathan Cameron 2024-03-23 20:18 ` David Lechner 2024-03-23 20:38 ` David Lechner 2024-03-23 21:35 ` Marcelo Schmitt 2024-03-22 22:05 ` [PATCH 2/2] iio: adc: Add support for AD4000 Marcelo Schmitt 2024-03-23 19:12 ` Jonathan Cameron 2024-03-23 21:53 ` David Lechner 2024-03-24 12:45 ` Jonathan Cameron 2024-03-25 13:35 ` David Lechner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox