diff mbox series

[v2,1/2] dt-bindings: iio: adc: Add AD4000

Message ID 7c877c865f0b7da28d9f1f177b3b2692b0ae20b9.1712585500.git.marcelo.schmitt@analog.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for AD4000 series | expand

Commit Message

Marcelo Schmitt April 8, 2024, 2:31 p.m. UTC
Add device tree documentation for AD4000 family 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>
---
 .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 208 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml

Comments

David Lechner April 9, 2024, 2:57 a.m. UTC | #1
On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
<marcelo.schmitt@analog.com> wrote:
>
> Add device tree documentation for AD4000 family 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
>

Suggested-by: David Lechner <dlechner@baylibre.com>

(if you still use mostly my suggestions in the end)

> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
>  .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 208 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..ca06afb5149e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> @@ -0,0 +1,201 @@
> +# 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
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.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
> +      - adi,adaq4001
> +      - adi,adaq4003
> +
> +  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 ]

It sounds like there are more possible wiring configurations for these
chips that I thought when suggesting reusing this binding from AD7944
so we probably need more options here. (see my reply to the cover
letter for the complete context of these remarks)

We identified A) an additional wiring configuration where SDI of the
ADC chip is wired to SDO of the SPI controller and B) a potential need
to pin mux between wiring modes to work around SPI controller
limitations perhaps we could omit the adi,spi-mode property and just
use the standard pinctrl properties.

  pinctrl-names:
    description: |
      Names for possible ways the SDI line of the controller is wired.

      * default: The SDI line of the ADC is connected to the SDO line of the
        SPI controller.  CNV line of the ADC is connected to CS of the SPI
        controller.
      * 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!)  In this mode, SDI is tied to VIO, and the CNV line
        can be connected to the CS line of the SPI controller (typical) or to a
        GPIO, in which case the CS line of the controller is unused.  The SDO
        line of the SPI controller is not connected.
      * multi: The datasheet calls this "4-wire mode" and is used when multiple
        chips are connected in parallel.  In this mode, the ADC SDI line is tied
        to the CS line on the SPI controller and the CNV line is connected to
        a GPIO.  The SDO line of the SPI controller is not connected.
      * 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.

      If one name is specified, it is assumed the chip is hard-wired in this
      configuration.

      If two names are specified, it is assumed that a pinmux can switch between
      the two wiring configurations.  The first is the default mode for reading
      and writing registers on the chip and the second is the mode for reading
      the conversion data from the chip.
    oneOf:
      - items:
          - enum:
            - default
            - single
            - multi
            - chain
      - items:
          - const: default
          - enum:
            - single
            - multi
            - chain

  pinctrl-0:
    maxItems: 1

  pinctrl-1:
    maxItems: 1


> +    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 5.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. If 'single' mode is selected and this GPIO
> +      is provided, it must be active low.

Since the conversion is triggered on the low to high transition of
CNV, I think it only makes sense to have it active high and not active
low.

> +    maxItems: 1
> +
> +  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.
> +
> +  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

Same suggestion as in V1 - we should make it clear that this property
only applies to ADAQ chips (in the description and also a -if: for the
bindings validator). Also, looking at the datasheet, it looks like
there are a lot more pins on the ADAQ chips, so I think there are more
properties missing here.

Some trivial ones:

vs-pos-supply (VS+ pin, 0 to 11V supply) and vs-neg-supply (VS- pin,
-11 to 0V supply)

pd-amp-gpios (active low) and pd-ref-gpios (active low) for optional
runtime power management.

Also the datasheet says the ADAQ chips supports "Single-ended to
differential conversion". So it seems like we might need some extra
properties to describe that case (a flag for indicating single-ended
wiring and an optional voltage supply to describe what is connected to
the negative input if it isn't tied to GND)
Marcelo Schmitt April 9, 2024, 3:30 p.m. UTC | #2
On 04/08, David Lechner wrote:
> On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> <marcelo.schmitt@analog.com> wrote:
> >
> > Add device tree documentation for AD4000 family 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
> >
> 
> Suggested-by: David Lechner <dlechner@baylibre.com>
> 
> (if you still use mostly my suggestions in the end)

Yes, it's been of great help. Will include the tag in future ad4000 DT patches.

> 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> >  .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++++++++++++++
> >  MAINTAINERS                                   |   7 +
> >  2 files changed, 208 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..ca06afb5149e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > @@ -0,0 +1,201 @@
> > +# 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
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.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
> > +      - adi,adaq4001
> > +      - adi,adaq4003
> > +
> > +  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 ]
> 
> It sounds like there are more possible wiring configurations for these
> chips that I thought when suggesting reusing this binding from AD7944
> so we probably need more options here. (see my reply to the cover
> letter for the complete context of these remarks)
> 
> We identified A) an additional wiring configuration where SDI of the
> ADC chip is wired to SDO of the SPI controller and B) a potential need
> to pin mux between wiring modes to work around SPI controller
> limitations perhaps we could omit the adi,spi-mode property and just
> use the standard pinctrl properties.
> 
>   pinctrl-names:
>     description: |
>       Names for possible ways the SDI line of the controller is wired.
> 
>       * default: The SDI line of the ADC is connected to the SDO line of the
>         SPI controller.  CNV line of the ADC is connected to CS of the SPI
>         controller.
Not sure if should be DT, but maybe also point out that in default mode the
SPI controller must be capable of keeping ADC SDI (controller SDO) line high
during ADC conversions.

>       * 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!)  In this mode, SDI is tied to VIO, and the CNV line
>         can be connected to the CS line of the SPI controller (typical) or to a
>         GPIO, in which case the CS line of the controller is unused.  The SDO
>         line of the SPI controller is not connected.
>       * multi: The datasheet calls this "4-wire mode" and is used when multiple
>         chips are connected in parallel.  In this mode, the ADC SDI line is tied
>         to the CS line on the SPI controller and the CNV line is connected to
>         a GPIO.  The SDO line of the SPI controller is not connected.
>       * 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.
> 
>       If one name is specified, it is assumed the chip is hard-wired in this
>       configuration.
> 
>       If two names are specified, it is assumed that a pinmux can switch between
>       the two wiring configurations.  The first is the default mode for reading
>       and writing registers on the chip and the second is the mode for reading
>       the conversion data from the chip.
>     oneOf:
>       - items:
>           - enum:
>             - default
>             - single
>             - multi
>             - chain
>       - items:
>           - const: default
>           - enum:
>             - single
>             - multi
>             - chain
> 
>   pinctrl-0:
>     maxItems: 1
> 
>   pinctrl-1:
>     maxItems: 1
> 
> 
> > +    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 5.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. If 'single' mode is selected and this GPIO
> > +      is provided, it must be active low.
> 
> Since the conversion is triggered on the low to high transition of
> CNV, I think it only makes sense to have it active high and not active
> low.

The idea was to use the GPIO as a replacement for the controller CS when
in "3-wire"/single mode so we could have simpler handling of SPI transfers.
But if changing transfer to avoid latency then this might not simplify anything
anymore. Will probably drop this last line.

> 
> > +    maxItems: 1
> > +
> > +  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.
> > +
> > +  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
> 
> Same suggestion as in V1 - we should make it clear that this property
> only applies to ADAQ chips (in the description and also a -if: for the
> bindings validator). Also, looking at the datasheet, it looks like
> there are a lot more pins on the ADAQ chips, so I think there are more
> properties missing here.
> 
> Some trivial ones:
> 
> vs-pos-supply (VS+ pin, 0 to 11V supply) and vs-neg-supply (VS- pin,
> -11 to 0V supply)
> 
> pd-amp-gpios (active low) and pd-ref-gpios (active low) for optional
> runtime power management.

Ok, will have closer look to these and other pins described in the datasheet and
include them here too.

> 
> Also the datasheet says the ADAQ chips supports "Single-ended to
> differential conversion". So it seems like we might need some extra
> properties to describe that case (a flag for indicating single-ended
> wiring and an optional voltage supply to describe what is connected to
> the negative input if it isn't tied to GND)

Yes, the differential ADCs also support "Single-ended to differential conversion".
Will provide support those too.
Jonathan Cameron April 13, 2024, 4:14 p.m. UTC | #3
On Tue, 9 Apr 2024 12:30:09 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 04/08, David Lechner wrote:
> > On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > <marcelo.schmitt@analog.com> wrote:  
> > >
> > > Add device tree documentation for AD4000 family 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
> > >  
> > 
> > Suggested-by: David Lechner <dlechner@baylibre.com>
> > 
> > (if you still use mostly my suggestions in the end)  
> 
> Yes, it's been of great help. Will include the tag in future ad4000 DT patches.
> 
> >   
> > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > ---
> > >  .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++++++++++++++
> > >  MAINTAINERS                                   |   7 +
> > >  2 files changed, 208 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..ca06afb5149e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > @@ -0,0 +1,201 @@
> > > +# 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
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.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
> > > +      - adi,adaq4001
> > > +      - adi,adaq4003
> > > +
> > > +  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 ]  
> > 
> > It sounds like there are more possible wiring configurations for these
> > chips that I thought when suggesting reusing this binding from AD7944
> > so we probably need more options here. (see my reply to the cover
> > letter for the complete context of these remarks)
> > 
> > We identified A) an additional wiring configuration where SDI of the
> > ADC chip is wired to SDO of the SPI controller and B) a potential need
> > to pin mux between wiring modes to work around SPI controller
> > limitations perhaps we could omit the adi,spi-mode property and just
> > use the standard pinctrl properties.
> > 
> >   pinctrl-names:

I'm lost on how pinctrl makes sense here.
Yes you are changing the modes of the pins, but not in a conventional sense
of some register that is being updated to say now use them like this.
The mode is dependent on the timing sequence of how the pins are used.
Otherwise looking at it a different way it's an external wiring thing we
aren't controlling it at all.  Is pinctrl suitable for that?
I always thought of it as a way to change configurations of SoC pins.

A pointer to some precendence in another driver for using it like this
would go some way towards convincing me.

Jonathan


> >     description: |
> >       Names for possible ways the SDI line of the controller is wired.
> > 
> >       * default: The SDI line of the ADC is connected to the SDO line of the
> >         SPI controller.  CNV line of the ADC is connected to CS of the SPI
> >         controller.  
> Not sure if should be DT, but maybe also point out that in default mode the
> SPI controller must be capable of keeping ADC SDI (controller SDO) line high
> during ADC conversions.
> 
> >       * 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!)  In this mode, SDI is tied to VIO, and the CNV line
> >         can be connected to the CS line of the SPI controller (typical) or to a
> >         GPIO, in which case the CS line of the controller is unused.  The SDO
> >         line of the SPI controller is not connected.
> >       * multi: The datasheet calls this "4-wire mode" and is used when multiple
> >         chips are connected in parallel.  In this mode, the ADC SDI line is tied
> >         to the CS line on the SPI controller and the CNV line is connected to
> >         a GPIO.  The SDO line of the SPI controller is not connected.
> >       * 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.
> > 
> >       If one name is specified, it is assumed the chip is hard-wired in this
> >       configuration.
> > 
> >       If two names are specified, it is assumed that a pinmux can switch between
> >       the two wiring configurations.  The first is the default mode for reading
> >       and writing registers on the chip and the second is the mode for reading
> >       the conversion data from the chip.
> >     oneOf:
> >       - items:
> >           - enum:
> >             - default
> >             - single
> >             - multi
> >             - chain
> >       - items:
> >           - const: default
> >           - enum:
> >             - single
> >             - multi
> >             - chain
> > 
> >   pinctrl-0:
> >     maxItems: 1
> > 
> >   pinctrl-1:
> >     maxItems: 1
David Lechner April 13, 2024, 5:33 p.m. UTC | #4
On 4/13/24 11:14 AM, Jonathan Cameron wrote:
> On Tue, 9 Apr 2024 12:30:09 -0300
> Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> 
>> On 04/08, David Lechner wrote:
>>> On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
>>> <marcelo.schmitt@analog.com> wrote:  
>>>>

...

>>>> +
>>>> +  adi,spi-mode:
>>>> +    $ref: /schemas/types.yaml#/definitions/string
>>>> +    enum: [ single, chain ]  
>>>
>>> It sounds like there are more possible wiring configurations for these
>>> chips that I thought when suggesting reusing this binding from AD7944
>>> so we probably need more options here. (see my reply to the cover
>>> letter for the complete context of these remarks)
>>>
>>> We identified A) an additional wiring configuration where SDI of the
>>> ADC chip is wired to SDO of the SPI controller and B) a potential need
>>> to pin mux between wiring modes to work around SPI controller
>>> limitations perhaps we could omit the adi,spi-mode property and just
>>> use the standard pinctrl properties.
>>>
>>>   pinctrl-names:
> 
> I'm lost on how pinctrl makes sense here.
> Yes you are changing the modes of the pins, but not in a conventional sense
> of some register that is being updated to say now use them like this.
> The mode is dependent on the timing sequence of how the pins are used.
> Otherwise looking at it a different way it's an external wiring thing we
> aren't controlling it at all.  Is pinctrl suitable for that?
> I always thought of it as a way to change configurations of SoC pins.

Yes, this is exactly what I think we need here.

To write to the register, the chip has to be wired like this ("default"):

                                         +-------------+
     +-----------------------------------| SDO         |
     |                                   |             |
     |              +--------------------| CS          |
     |              v                    |             |
     |    +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

But to read sample data, the chip has to be wired in one of these
3 configurations:


3-wire mode ("single"):

                                         +-------------+
                    +--------------------| CS          |
                    v                    |             |
    VIO   +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

4-wire mode ("multi"):
                                         +-------------+
     +-----------------------------------| CS          |
     |                                   |             |
     |              +--------------------| GPIO        |
     |              v                    |             |
     |    +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

Chain mode ("chain"):

                                         +-------------+
                    +--------------------| CS          |
                    v                    |             |
          +--------------------+         |     HOST    |
          |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
     |    |        SCK         |         |             |
    GND   +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+


If we want to be able to both write the register and read data,
some reconfiguration is needed. It might be possible to read data
using the register-write wiring configuration, but that only
works if SDO can be set to the correct state *before* the
CS line changes. This is not something that I think most SPI
controllers can do (e.g. Marcelo mentioned in the cover letter
that RPi always returns SDO to low after every xfer while
the AXI SPI Engine leaves SDO wherever it was last).

> 
> A pointer to some precendence in another driver for using it like this
> would go some way towards convincing me.
> 
> Jonathan
> 


I didn't find much precedence for something like this, but I
found devicetree/bindings/net/mediatek-bluetooth.txt that uses
pinctrl to pull a UART Rx pin low for a bootstrap mode which
sounds very similar to what we need to do here (pull the SPI
controller SDO pin high or low for 3-wire or chain mode).

For example, if we wanted to use 3-wire mode for reading
data, we would set the pinctrl to "default" to write the
register to configure the chip during driver probe. Then
to read data, we would change the pinctrl to "single" before
doing the SPI xfer to ensure that the ADC SDI pin is pulled
high independent of what the SDO line of the SPI controller
is currently doing.
Jonathan Cameron April 14, 2024, 6:09 p.m. UTC | #5
On Sat, 13 Apr 2024 12:33:54 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 4/13/24 11:14 AM, Jonathan Cameron wrote:
> > On Tue, 9 Apr 2024 12:30:09 -0300
> > Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> >   
> >> On 04/08, David Lechner wrote:  
> >>> On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> >>> <marcelo.schmitt@analog.com> wrote:    
> >>>>  
> 
> ...
> 
> >>>> +
> >>>> +  adi,spi-mode:
> >>>> +    $ref: /schemas/types.yaml#/definitions/string
> >>>> +    enum: [ single, chain ]    
> >>>
> >>> It sounds like there are more possible wiring configurations for these
> >>> chips that I thought when suggesting reusing this binding from AD7944
> >>> so we probably need more options here. (see my reply to the cover
> >>> letter for the complete context of these remarks)
> >>>
> >>> We identified A) an additional wiring configuration where SDI of the
> >>> ADC chip is wired to SDO of the SPI controller and B) a potential need
> >>> to pin mux between wiring modes to work around SPI controller
> >>> limitations perhaps we could omit the adi,spi-mode property and just
> >>> use the standard pinctrl properties.
> >>>
> >>>   pinctrl-names:  
> > 
> > I'm lost on how pinctrl makes sense here.
> > Yes you are changing the modes of the pins, but not in a conventional sense
> > of some register that is being updated to say now use them like this.
> > The mode is dependent on the timing sequence of how the pins are used.
> > Otherwise looking at it a different way it's an external wiring thing we
> > aren't controlling it at all.  Is pinctrl suitable for that?
> > I always thought of it as a way to change configurations of SoC pins.  
> 
> Yes, this is exactly what I think we need here.
> 
> To write to the register, the chip has to be wired like this ("default"):
> 
>                                          +-------------+
>      +-----------------------------------| SDO         |
>      |                                   |             |
>      |              +--------------------| CS          |
>      |              v                    |             |
>      |    +--------------------+         |     HOST    |
>      |    |        CNV         |         |             |
>      +--->| SDI   AD7944   SDO |-------->| SDI         |
>           |        SCK         |         |             |
>           +--------------------+         |             |
>                     ^                    |             |
>                     +--------------------| SCLK        |
>                                          +-------------+
> 
> But to read sample data, the chip has to be wired in one of these
> 3 configurations:
> 
> 
> 3-wire mode ("single"):
> 
>                                          +-------------+
>                     +--------------------| CS          |
>                     v                    |             |
>     VIO   +--------------------+         |     HOST    |
>      |    |        CNV         |         |             |
>      +--->| SDI   AD7944   SDO |-------->| SDI         |
>           |        SCK         |         |             |
>           +--------------------+         |             |
>                     ^                    |             |
>                     +--------------------| SCLK        |
>                                          +-------------+
> 
> 4-wire mode ("multi"):
>                                          +-------------+
>      +-----------------------------------| CS          |
>      |                                   |             |
>      |              +--------------------| GPIO        |
>      |              v                    |             |
>      |    +--------------------+         |     HOST    |
>      |    |        CNV         |         |             |
>      +--->| SDI   AD7944   SDO |-------->| SDI         |
>           |        SCK         |         |             |
>           +--------------------+         |             |
>                     ^                    |             |
>                     +--------------------| SCLK        |
>                                          +-------------+
> 
> Chain mode ("chain"):
> 
>                                          +-------------+
>                     +--------------------| CS          |
>                     v                    |             |
>           +--------------------+         |     HOST    |
>           |        CNV         |         |             |
>      +--->| SDI   AD7944   SDO |-------->| SDI         |
>      |    |        SCK         |         |             |
>     GND   +--------------------+         |             |
>                     ^                    |             |
>                     +--------------------| SCLK        |
>                                          +-------------+
> 
> 
> If we want to be able to both write the register and read data,
> some reconfiguration is needed. It might be possible to read data
> using the register-write wiring configuration, but that only
> works if SDO can be set to the correct state *before* the
> CS line changes. This is not something that I think most SPI
> controllers can do (e.g. Marcelo mentioned in the cover letter
> that RPi always returns SDO to low after every xfer while
> the AXI SPI Engine leaves SDO wherever it was last).
> 
> > 
> > A pointer to some precendence in another driver for using it like this
> > would go some way towards convincing me.
> > 
> > Jonathan
> >   
> 
> 
> I didn't find much precedence for something like this, but I
> found devicetree/bindings/net/mediatek-bluetooth.txt that uses
> pinctrl to pull a UART Rx pin low for a bootstrap mode which
> sounds very similar to what we need to do here (pull the SPI
> controller SDO pin high or low for 3-wire or chain mode).
> 
> For example, if we wanted to use 3-wire mode for reading
> data, we would set the pinctrl to "default" to write the
> register to configure the chip during driver probe. Then
> to read data, we would change the pinctrl to "single" before
> doing the SPI xfer to ensure that the ADC SDI pin is pulled
> high independent of what the SDO line of the SPI controller
> is currently doing.

Ah ok.  This is implying that we are switching to a controllable
mode to pull that pin high (or I guess one where it is always
high).  I'm not sure if that's more common than an SPI controller
where you can set the 'don't' care state to high or low.
I assume we can't get away with just setting the output buffer
to all 1s because it won't hold that state between transfers?

Feels like that could be rolled into the SPI subsystem with
any necessary transitions handled there (maybe)

Just to check, this isn't just a case of a missing pull up
resistor to drag that line correctly when it isn't actively
driven (combined with all 1s in the write buffer) etc?

Jonathan


> 
> 
>
Marcelo Schmitt April 16, 2024, 9:46 p.m. UTC | #6
So, I have been trying to make this work, though I haven't been successful so
far, and I don't really think using pinctrl is a good solution for this either.

Comments inline.

On 04/14, Jonathan Cameron wrote:
> On Sat, 13 Apr 2024 12:33:54 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > On 4/13/24 11:14 AM, Jonathan Cameron wrote:
> > > On Tue, 9 Apr 2024 12:30:09 -0300
> > > Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> > >   
> > >> On 04/08, David Lechner wrote:  
> > >>> On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > >>> <marcelo.schmitt@analog.com> wrote:    
> > >>>>  
> > 
> > ...
> > 

...

> > >>>
> > >>> We identified A) an additional wiring configuration where SDI of the
> > >>> ADC chip is wired to SDO of the SPI controller and B) a potential need
> > >>> to pin mux between wiring modes to work around SPI controller
> > >>> limitations perhaps we could omit the adi,spi-mode property and just
> > >>> use the standard pinctrl properties.
> > >>>
> > >>>   pinctrl-names:  
> > > 
> > > I'm lost on how pinctrl makes sense here.
> > > Yes you are changing the modes of the pins, but not in a conventional sense
> > > of some register that is being updated to say now use them like this.
> > > The mode is dependent on the timing sequence of how the pins are used.
> > > Otherwise looking at it a different way it's an external wiring thing we
> > > aren't controlling it at all.  Is pinctrl suitable for that?
> > > I always thought of it as a way to change configurations of SoC pins.  
> > 
> > Yes, this is exactly what I think we need here.
> > 
> > To write to the register, the chip has to be wired like this ("default"):
> > 
> >                                          +-------------+
> >      +-----------------------------------| SDO         |
> >      |                                   |             |
> >      |              +--------------------| CS          |
> >      |              v                    |             |
> >      |    +--------------------+         |     HOST    |
> >      |    |        CNV         |         |             |
> >      +--->| SDI   AD7944   SDO |-------->| SDI         |
> >           |        SCK         |         |             |
> >           +--------------------+         |             |
> >                     ^                    |             |
> >                     +--------------------| SCLK        |
> >                                          +-------------+
> > 
> > But to read sample data, the chip has to be wired in one of these
> > 3 configurations:
> > 
> > 
> > 3-wire mode ("single"):
> > 
> >                                          +-------------+
> >                     +--------------------| CS          |
> >                     v                    |             |
> >     VIO   +--------------------+         |     HOST    |
> >      |    |        CNV         |         |             |
> >      +--->| SDI   AD7944   SDO |-------->| SDI         |
> >           |        SCK         |         |             |
> >           +--------------------+         |             |
> >                     ^                    |             |
> >                     +--------------------| SCLK        |
> >                                          +-------------+
> > 

3-wire mode like setup can be achieved if the SPI controller is capable of
keeping the ADC SDI line high (at VIO level) during ADC sampling, but keeping
controller SDO high throughout the entire transfer is not a thing all SPI
controllers can do (RPi's can't).
If the ADC is hard wired connected as in the 3-wire diagram then the user
can't write/read the configuration register. Same applies to "4-wire" mode
where controller CS is connected to ADC SDI.
The whole point of having pinctrl configurations was to make it possible
to both read/write config register and do ADC sampling if the SPI controller
can't keep ADC SDI at VIO. I don't think pinctrl can solve this problem though.

...

> > 
> > If we want to be able to both write the register and read data,
> > some reconfiguration is needed. It might be possible to read data
> > using the register-write wiring configuration, but that only
> > works if SDO can be set to the correct state *before* the
> > CS line changes. This is not something that I think most SPI
> > controllers can do (e.g. Marcelo mentioned in the cover letter
> > that RPi always returns SDO to low after every xfer while
> > the AXI SPI Engine leaves SDO wherever it was last).
> > 
> > > 
> > > A pointer to some precendence in another driver for using it like this
> > > would go some way towards convincing me.
> > > 
> > > Jonathan
> > >   
> > 
> > 
> > I didn't find much precedence for something like this, but I
> > found devicetree/bindings/net/mediatek-bluetooth.txt that uses
> > pinctrl to pull a UART Rx pin low for a bootstrap mode which
> > sounds very similar to what we need to do here (pull the SPI
> > controller SDO pin high or low for 3-wire or chain mode).
> > 

The pinctrl configuration for this ADC would not be meant to change once after
boot as it looks to be the usual use case for pinctrl (including mediatek-bluetooth.txt).

Also, no suitable mux for the "3-wire" mode in
Documentation/devicetree/bindings/pinctrl/xlnx,pinctrl-zynq.yaml
to do it like Documentation/devicetree/bindings/net/mediatek-bluetooth.txt.
The zynq pinctrl driver (drivers/pinctrl/pinctrl-zynq.c) would have to be
updated to add the new mux function in 
static const struct zynq_pinmux_function zynq_pmux_functions[] = {
	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
...
	DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_single, 1),
	DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_multi, 2),
though this is not really a thing that's on zynq, but one that is related to
these ADCs so I'm not sure it should go there.

> > For example, if we wanted to use 3-wire mode for reading
> > data, we would set the pinctrl to "default" to write the
> > register to configure the chip during driver probe. Then
> > to read data, we would change the pinctrl to "single" before
> > doing the SPI xfer to ensure that the ADC SDI pin is pulled
> > high independent of what the SDO line of the SPI controller
> > is currently doing.

No, the pin configuration for this ADCs would be expected to change unrestricted
amount of times. The pin configuration would have to change every time a sample
read is made after a register access transfers and vice-versa. If we decide
to support span compression, every change to _scale would lead to pinctrl
configuration change.

At pin level, we would want to rise SPI controller SDO line to VIO but then
the new SDO pin config would conflict with SPI pin group config.

I included pinctrl properties in my test dts to make use of pinctrl framework.
Yet, that doesn't really alternate SPI line configuration we are using because
the SPI function that is in the PS (FPGA's Processing System) is not what we are
interfacing when using spi-engine. Copy of my test dts at end of email.

Currently, the SPI controller we are using to test with these exotic ADCs
is the spi-engine which is implemented in the FPGA as an IP block which
owns control of the bus lines (SPI, SDO, CS, ...). To alternate the
configuration of SPI lines (pull SDO (ADC SDI) up to VIO, connect controller CS
to ADC SDI, etc.) I think it should be done in the HDL project. I don't think
it's a good idea to hijack spi-engine lines through pinctrl.

> 
> Ah ok.  This is implying that we are switching to a controllable
> mode to pull that pin high (or I guess one where it is always
> high).  I'm not sure if that's more common than an SPI controller
> where you can set the 'don't' care state to high or low.
> I assume we can't get away with just setting the output buffer
> to all 1s because it won't hold that state between transfers?

I tried sending the tx buffer filled with 1s when testing this with a RPi4 but
it brought the controller SDO (ADC SDI) line low between each 8 bits of transfer
(attaching an image (yellow line is SCLK, green lines is controller SDO)).
Anyway, can we have any guaranties with respect to controller SDO line behaviour
when its not clocking out data?

> 
> Feels like that could be rolled into the SPI subsystem with
> any necessary transitions handled there (maybe)

To me, this sounds more reasonable than pinctrl.
Yeah, if we can set a don't' care state for the SDO line then that should be
enough for these ADCs.
Otherwise, can we really do anything if the controller can't keep SDO high?

> 
> Just to check, this isn't just a case of a missing pull up
> resistor to drag that line correctly when it isn't actively
> driven (combined with all 1s in the write buffer) etc?

When using spi-engine, the controller SDO is connected to ADC SDI, controller
CS to ADC CNV and, for reg access, it works as conventional SPI.
spi-engine leaves the SDO line the state it was after last transfer so it we
can make it idle high during sample read. No hardware pull-up needed.

Marcelo

> 
> Jonathan
> 
> 

The device tree source file I was using for testing with the ADC with the
changes for using pinctrl. Didn't really work.

// SPDX-License-Identifier: GPL-2.0
/*
 * Analog Devices ADAQ4003
 * https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad400x
 * https://wiki.analog.com/resources/eval/user-guides/ad400x
 *
 * hdl_project: <pulsar_adc_pmdz/zed>
 * board_revision: <>
 *
 * Copyright (C) 2016-2023 Analog Devices Inc.
 */
/dts-v1/;

#include "zynq-zed.dtsi"
#include "zynq-zed-adv7511.dtsi"
#include <dt-bindings/pinctrl/pinctrl-zynq.h>

/ {
	adc_vref: regulator-vref {
		compatible = "regulator-fixed";
		regulator-name = "EVAL 5V Vref";
		regulator-min-microvolt = <5000000>;
		regulator-max-microvolt = <5000000>;
		regulator-always-on;
	};

	adc_vdd: regulator-vdd {
		compatible = "regulator-fixed";
		regulator-name = "Eval VDD supply";
		regulator-min-microvolt = <1800000>;
		regulator-max-microvolt = <1800000>;
		regulator-always-on;
	};

	adc_vio: regulator-vio {
		compatible = "regulator-fixed";
		regulator-name = "Eval VIO supply";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
		regulator-always-on;
	};
};

&pinctrl0 {
	/* Restore conventional SPI pin configuration */
	pinctrl_spi_default: default_config {
		mux {
			/* Are these the ones used by spi-engine? */
			groups = "spi0_0_grp";
			function = "spi0";
		};
		conf {
			groups = "spi0_0_grp";
			power-source = <IO_STANDARD_LVCMOS33>;
		};
		conf-spi-sdo {
			pins = "MIO17"; /* SPI0 SDO? */
			bias-disable;
		};
	};

	/* Pull-up SPI SDO (ADC SDI) to VIO */
	pinctrl_spi_single: single_config {
		conf-spi-sdo {
			pins = "MIO17"; /* Conflicts with SPI0 pin group */
			bias-pull-up;
		};
	};
};

&fpga_axi {
	rx_dma: rx-dmac@44a30000 {
		compatible = "adi,axi-dmac-1.00.a";
		reg = <0x44a30000 0x1000>;
		#dma-cells = <1>;
		interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&clkc 17>;

		adi,channels {
			#size-cells = <0>;
			#address-cells = <1>;

			dma-channel@0 {
				reg = <0>;
				adi,source-bus-width = <32>;
				adi,source-bus-type = <1>;
				adi,destination-bus-width = <64>;
				adi,destination-bus-type = <0>;
			};
		};
	};

	axi_pwm_gen: axi-pwm-gen@ {
		compatible = "adi,axi-pwmgen";
		reg = <0x44b00000 0x1000>;
		label = "cnv";
		#pwm-cells = <2>;
		clocks = <&spi_clk>;
	};

	spi_clk: axi-clkgen@44a70000 {
		compatible = "adi,axi-clkgen-2.00.a";
		reg = <0x44a70000 0x10000>;
		#clock-cells = <0>;
		clocks = <&clkc 15>, <&clkc 15>;
		clock-names = "s_axi_aclk", "clkin1";
		clock-output-names = "spi_clk";
	};

	axi_spi_engine_0: spi@44a00000 {
		compatible = "adi,axi-spi-engine-1.00.a";
		reg = <0x44a00000 0x1000>;
		interrupt-parent = <&intc>;
		interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&clkc 15 &spi_clk>;
		clock-names = "s_axi_aclk", "spi_clk";
		num-cs = <1>;

		#address-cells = <0x1>;
		#size-cells = <0x0>;

		adaq4003: adc@0 {
			compatible = "adi,adaq4003";
			reg = <0>;
			spi-max-frequency = <102000000>;
			spi-cpha;
			pinctrl-names = "default", "single";
			pinctrl-0 = <&pinctrl_spi_default>;
			pinctrl-1 = <&pinctrl_spi_single>;
			vdd-supply = <&adc_vdd>;
			vio-supply = <&adc_vio>;
			ref-supply = <&adc_vref>;
			adi,high-z-input;
			adi,gain-milli = <454>;
		};
	};
};
Jonathan Cameron April 20, 2024, 2:17 p.m. UTC | #7
On Tue, 16 Apr 2024 18:46:11 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> So, I have been trying to make this work, though I haven't been successful so
> far, and I don't really think using pinctrl is a good solution for this either.
> 
> Comments inline.
> 
> On 04/14, Jonathan Cameron wrote:
> > On Sat, 13 Apr 2024 12:33:54 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> >   
> > > On 4/13/24 11:14 AM, Jonathan Cameron wrote:  
> > > > On Tue, 9 Apr 2024 12:30:09 -0300
> > > > Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> > > >     
> > > >> On 04/08, David Lechner wrote:    
> > > >>> On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > > >>> <marcelo.schmitt@analog.com> wrote:      
> > > >>>>    
> > > 
> > > ...
> > >   
> 
> ...
> 
> > > >>>
> > > >>> We identified A) an additional wiring configuration where SDI of the
> > > >>> ADC chip is wired to SDO of the SPI controller and B) a potential need
> > > >>> to pin mux between wiring modes to work around SPI controller
> > > >>> limitations perhaps we could omit the adi,spi-mode property and just
> > > >>> use the standard pinctrl properties.
> > > >>>
> > > >>>   pinctrl-names:    
> > > > 
> > > > I'm lost on how pinctrl makes sense here.
> > > > Yes you are changing the modes of the pins, but not in a conventional sense
> > > > of some register that is being updated to say now use them like this.
> > > > The mode is dependent on the timing sequence of how the pins are used.
> > > > Otherwise looking at it a different way it's an external wiring thing we
> > > > aren't controlling it at all.  Is pinctrl suitable for that?
> > > > I always thought of it as a way to change configurations of SoC pins.    
> > > 
> > > Yes, this is exactly what I think we need here.
> > > 
> > > To write to the register, the chip has to be wired like this ("default"):
> > > 
> > >                                          +-------------+
> > >      +-----------------------------------| SDO         |
> > >      |                                   |             |
> > >      |              +--------------------| CS          |
> > >      |              v                    |             |
> > >      |    +--------------------+         |     HOST    |
> > >      |    |        CNV         |         |             |
> > >      +--->| SDI   AD7944   SDO |-------->| SDI         |
> > >           |        SCK         |         |             |
> > >           +--------------------+         |             |
> > >                     ^                    |             |
> > >                     +--------------------| SCLK        |
> > >                                          +-------------+
> > > 
> > > But to read sample data, the chip has to be wired in one of these
> > > 3 configurations:
> > > 
> > > 
> > > 3-wire mode ("single"):
> > > 
> > >                                          +-------------+
> > >                     +--------------------| CS          |
> > >                     v                    |             |
> > >     VIO   +--------------------+         |     HOST    |
> > >      |    |        CNV         |         |             |
> > >      +--->| SDI   AD7944   SDO |-------->| SDI         |
> > >           |        SCK         |         |             |
> > >           +--------------------+         |             |
> > >                     ^                    |             |
> > >                     +--------------------| SCLK        |
> > >                                          +-------------+
> > >   
> 
> 3-wire mode like setup can be achieved if the SPI controller is capable of
> keeping the ADC SDI line high (at VIO level) during ADC sampling, but keeping
> controller SDO high throughout the entire transfer is not a thing all SPI
> controllers can do (RPi's can't).
> If the ADC is hard wired connected as in the 3-wire diagram then the user
> can't write/read the configuration register. Same applies to "4-wire" mode
> where controller CS is connected to ADC SDI.
> The whole point of having pinctrl configurations was to make it possible
> to both read/write config register and do ADC sampling if the SPI controller
> can't keep ADC SDI at VIO. I don't think pinctrl can solve this problem though.
> 
> ...
> 
> > > 
> > > If we want to be able to both write the register and read data,
> > > some reconfiguration is needed. It might be possible to read data
> > > using the register-write wiring configuration, but that only
> > > works if SDO can be set to the correct state *before* the
> > > CS line changes. This is not something that I think most SPI
> > > controllers can do (e.g. Marcelo mentioned in the cover letter
> > > that RPi always returns SDO to low after every xfer while
> > > the AXI SPI Engine leaves SDO wherever it was last).
> > >   
> > > > 
> > > > A pointer to some precendence in another driver for using it like this
> > > > would go some way towards convincing me.
> > > > 
> > > > Jonathan
> > > >     
> > > 
> > > 
> > > I didn't find much precedence for something like this, but I
> > > found devicetree/bindings/net/mediatek-bluetooth.txt that uses
> > > pinctrl to pull a UART Rx pin low for a bootstrap mode which
> > > sounds very similar to what we need to do here (pull the SPI
> > > controller SDO pin high or low for 3-wire or chain mode).
> > >   
> 
> The pinctrl configuration for this ADC would not be meant to change once after
> boot as it looks to be the usual use case for pinctrl (including mediatek-bluetooth.txt).
> 
> Also, no suitable mux for the "3-wire" mode in
> Documentation/devicetree/bindings/pinctrl/xlnx,pinctrl-zynq.yaml
> to do it like Documentation/devicetree/bindings/net/mediatek-bluetooth.txt.
> The zynq pinctrl driver (drivers/pinctrl/pinctrl-zynq.c) would have to be
> updated to add the new mux function in 
> static const struct zynq_pinmux_function zynq_pmux_functions[] = {
> 	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
> ...
> 	DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_single, 1),
> 	DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_multi, 2),
> though this is not really a thing that's on zynq, but one that is related to
> these ADCs so I'm not sure it should go there.


I'd argue we are after a specific SPI controller setup for this.
A controller driver would need modifying to make it work.

> 
> > > For example, if we wanted to use 3-wire mode for reading
> > > data, we would set the pinctrl to "default" to write the
> > > register to configure the chip during driver probe. Then
> > > to read data, we would change the pinctrl to "single" before
> > > doing the SPI xfer to ensure that the ADC SDI pin is pulled
> > > high independent of what the SDO line of the SPI controller
> > > is currently doing.  
> 
> No, the pin configuration for this ADCs would be expected to change unrestricted
> amount of times. The pin configuration would have to change every time a sample
> read is made after a register access transfers and vice-versa. If we decide
> to support span compression, every change to _scale would lead to pinctrl
> configuration change.
> 
> At pin level, we would want to rise SPI controller SDO line to VIO but then
> the new SDO pin config would conflict with SPI pin group config.
> 
> I included pinctrl properties in my test dts to make use of pinctrl framework.
> Yet, that doesn't really alternate SPI line configuration we are using because
> the SPI function that is in the PS (FPGA's Processing System) is not what we are
> interfacing when using spi-engine. Copy of my test dts at end of email.
> 
> Currently, the SPI controller we are using to test with these exotic ADCs
> is the spi-engine which is implemented in the FPGA as an IP block which
> owns control of the bus lines (SPI, SDO, CS, ...). To alternate the
> configuration of SPI lines (pull SDO (ADC SDI) up to VIO, connect controller CS
> to ADC SDI, etc.) I think it should be done in the HDL project. I don't think
> it's a good idea to hijack spi-engine lines through pinctrl.

Such functionality would need to be pushed to the spi controller driver
which could know if there was any need to do anything like this, or if there
was simply a register to set.

> 
> > 
> > Ah ok.  This is implying that we are switching to a controllable
> > mode to pull that pin high (or I guess one where it is always
> > high).  I'm not sure if that's more common than an SPI controller
> > where you can set the 'don't' care state to high or low.
> > I assume we can't get away with just setting the output buffer
> > to all 1s because it won't hold that state between transfers?  
> 
> I tried sending the tx buffer filled with 1s when testing this with a RPi4 but
> it brought the controller SDO (ADC SDI) line low between each 8 bits of transfer
> (attaching an image (yellow line is SCLK, green lines is controller SDO)).

Pity - thought that was overly optimistic.

> Anyway, can we have any guaranties with respect to controller SDO line behaviour
> when its not clocking out data?


> 
> > 
> > Feels like that could be rolled into the SPI subsystem with
> > any necessary transitions handled there (maybe)  
> 
> To me, this sounds more reasonable than pinctrl.
> Yeah, if we can set a don't' care state for the SDO line then that should be
> enough for these ADCs.
> Otherwise, can we really do anything if the controller can't keep SDO high?

There is one similar (ish) entry already.
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/spi/spi.h#L29
#define	SPI_3WIRE_HIZ		_BITUL(15)	/* high impedance turnaround */
in that it is controlling state in what I think would normally be a don't care state.

I think we could have an
SPI_SDO_DONT_CARE_HIGH (naming to be improved upon ;) 
that a driver could advertise support for and the spi device could request.

https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/panel/panel-tpo-tpg110.c#L429

Implement that in the spi-gpio driver as a PoC probably and in your SPI copntoller
driver.

Ultimately if the controller really isn't capable (including dances through pin
mode changes if necessary) then the ADC won't work in this wiring with that host
controller.

I'd propose something along these lines and see whether Mark + any other active
SPI folk think it is reasonable or not?

> 
> > 
> > Just to check, this isn't just a case of a missing pull up
> > resistor to drag that line correctly when it isn't actively
> > driven (combined with all 1s in the write buffer) etc?  
> 
> When using spi-engine, the controller SDO is connected to ADC SDI, controller
> CS to ADC CNV and, for reg access, it works as conventional SPI.
> spi-engine leaves the SDO line the state it was after last transfer so it we
> can make it idle high during sample read. No hardware pull-up needed.

Fair enough. No multi master support I guess (that is a bit obscure for
SPI).  A little ugly that it's dependent on the last access - so you would need
to do a dummy access if the normal last bit was wrong level?

Jonathan

> 
> Marcelo
> 
> > 
> > Jonathan
> > 
> >   
> 
> The device tree source file I was using for testing with the ADC with the
> changes for using pinctrl. Didn't really work.
> 
> // SPDX-License-Identifier: GPL-2.0
> /*
>  * Analog Devices ADAQ4003
>  * https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad400x
>  * https://wiki.analog.com/resources/eval/user-guides/ad400x
>  *
>  * hdl_project: <pulsar_adc_pmdz/zed>
>  * board_revision: <>
>  *
>  * Copyright (C) 2016-2023 Analog Devices Inc.
>  */
> /dts-v1/;
> 
> #include "zynq-zed.dtsi"
> #include "zynq-zed-adv7511.dtsi"
> #include <dt-bindings/pinctrl/pinctrl-zynq.h>
> 
> / {
> 	adc_vref: regulator-vref {
> 		compatible = "regulator-fixed";
> 		regulator-name = "EVAL 5V Vref";
> 		regulator-min-microvolt = <5000000>;
> 		regulator-max-microvolt = <5000000>;
> 		regulator-always-on;
> 	};
> 
> 	adc_vdd: regulator-vdd {
> 		compatible = "regulator-fixed";
> 		regulator-name = "Eval VDD supply";
> 		regulator-min-microvolt = <1800000>;
> 		regulator-max-microvolt = <1800000>;
> 		regulator-always-on;
> 	};
> 
> 	adc_vio: regulator-vio {
> 		compatible = "regulator-fixed";
> 		regulator-name = "Eval VIO supply";
> 		regulator-min-microvolt = <3300000>;
> 		regulator-max-microvolt = <3300000>;
> 		regulator-always-on;
> 	};
> };
> 
> &pinctrl0 {
> 	/* Restore conventional SPI pin configuration */
> 	pinctrl_spi_default: default_config {
> 		mux {
> 			/* Are these the ones used by spi-engine? */
> 			groups = "spi0_0_grp";
> 			function = "spi0";
> 		};
> 		conf {
> 			groups = "spi0_0_grp";
> 			power-source = <IO_STANDARD_LVCMOS33>;
> 		};
> 		conf-spi-sdo {
> 			pins = "MIO17"; /* SPI0 SDO? */
> 			bias-disable;
> 		};
> 	};
> 
> 	/* Pull-up SPI SDO (ADC SDI) to VIO */
> 	pinctrl_spi_single: single_config {
> 		conf-spi-sdo {
> 			pins = "MIO17"; /* Conflicts with SPI0 pin group */
> 			bias-pull-up;
> 		};
> 	};
> };
> 
> &fpga_axi {
> 	rx_dma: rx-dmac@44a30000 {
> 		compatible = "adi,axi-dmac-1.00.a";
> 		reg = <0x44a30000 0x1000>;
> 		#dma-cells = <1>;
> 		interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
> 		clocks = <&clkc 17>;
> 
> 		adi,channels {
> 			#size-cells = <0>;
> 			#address-cells = <1>;
> 
> 			dma-channel@0 {
> 				reg = <0>;
> 				adi,source-bus-width = <32>;
> 				adi,source-bus-type = <1>;
> 				adi,destination-bus-width = <64>;
> 				adi,destination-bus-type = <0>;
> 			};
> 		};
> 	};
> 
> 	axi_pwm_gen: axi-pwm-gen@ {
> 		compatible = "adi,axi-pwmgen";
> 		reg = <0x44b00000 0x1000>;
> 		label = "cnv";
> 		#pwm-cells = <2>;
> 		clocks = <&spi_clk>;
> 	};
> 
> 	spi_clk: axi-clkgen@44a70000 {
> 		compatible = "adi,axi-clkgen-2.00.a";
> 		reg = <0x44a70000 0x10000>;
> 		#clock-cells = <0>;
> 		clocks = <&clkc 15>, <&clkc 15>;
> 		clock-names = "s_axi_aclk", "clkin1";
> 		clock-output-names = "spi_clk";
> 	};
> 
> 	axi_spi_engine_0: spi@44a00000 {
> 		compatible = "adi,axi-spi-engine-1.00.a";
> 		reg = <0x44a00000 0x1000>;
> 		interrupt-parent = <&intc>;
> 		interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> 		clocks = <&clkc 15 &spi_clk>;
> 		clock-names = "s_axi_aclk", "spi_clk";
> 		num-cs = <1>;
> 
> 		#address-cells = <0x1>;
> 		#size-cells = <0x0>;
> 
> 		adaq4003: adc@0 {
> 			compatible = "adi,adaq4003";
> 			reg = <0>;
> 			spi-max-frequency = <102000000>;
> 			spi-cpha;
> 			pinctrl-names = "default", "single";
> 			pinctrl-0 = <&pinctrl_spi_default>;
> 			pinctrl-1 = <&pinctrl_spi_single>;
> 			vdd-supply = <&adc_vdd>;
> 			vio-supply = <&adc_vio>;
> 			ref-supply = <&adc_vref>;
> 			adi,high-z-input;
> 			adi,gain-milli = <454>;
> 		};
> 	};
> };
Marcelo Schmitt April 21, 2024, 10:38 p.m. UTC | #8
On 04/20, Jonathan Cameron wrote:
> On Tue, 16 Apr 2024 18:46:11 -0300
> Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> 
> > So, I have been trying to make this work, though I haven't been successful so
> > far, and I don't really think using pinctrl is a good solution for this either.
> > 
> > Comments inline.
> > 
> > On 04/14, Jonathan Cameron wrote:
> > > On Sat, 13 Apr 2024 12:33:54 -0500
> > > David Lechner <dlechner@baylibre.com> wrote:
> > >   
> > > > On 4/13/24 11:14 AM, Jonathan Cameron wrote:  
> > > > > On Tue, 9 Apr 2024 12:30:09 -0300
> > > > > Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> > > > >     
> > > > >> On 04/08, David Lechner wrote:    
> > > > >>> On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > > > >>> <marcelo.schmitt@analog.com> wrote:      
> > > > >>>>    
> > > > 
> > > > ...
> > > >   
> > 
> > ...
> > 

...

> > 
> > The pinctrl configuration for this ADC would not be meant to change once after
> > boot as it looks to be the usual use case for pinctrl (including mediatek-bluetooth.txt).
> > 
> > Also, no suitable mux for the "3-wire" mode in
> > Documentation/devicetree/bindings/pinctrl/xlnx,pinctrl-zynq.yaml
> > to do it like Documentation/devicetree/bindings/net/mediatek-bluetooth.txt.
> > The zynq pinctrl driver (drivers/pinctrl/pinctrl-zynq.c) would have to be
> > updated to add the new mux function in 
> > static const struct zynq_pinmux_function zynq_pmux_functions[] = {
> > 	DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
> > ...
> > 	DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_single, 1),
> > 	DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_multi, 2),
> > though this is not really a thing that's on zynq, but one that is related to
> > these ADCs so I'm not sure it should go there.
> 
> 
> I'd argue we are after a specific SPI controller setup for this.
> A controller driver would need modifying to make it work.

Ack, makes sense to me.

> 
> > 
> > > > For example, if we wanted to use 3-wire mode for reading
> > > > data, we would set the pinctrl to "default" to write the
> > > > register to configure the chip during driver probe. Then
> > > > to read data, we would change the pinctrl to "single" before
> > > > doing the SPI xfer to ensure that the ADC SDI pin is pulled
> > > > high independent of what the SDO line of the SPI controller
> > > > is currently doing.  
> > 
> > No, the pin configuration for this ADCs would be expected to change unrestricted
> > amount of times. The pin configuration would have to change every time a sample
> > read is made after a register access transfers and vice-versa. If we decide
> > to support span compression, every change to _scale would lead to pinctrl
> > configuration change.
> > 
> > At pin level, we would want to rise SPI controller SDO line to VIO but then
> > the new SDO pin config would conflict with SPI pin group config.
> > 
> > I included pinctrl properties in my test dts to make use of pinctrl framework.
> > Yet, that doesn't really alternate SPI line configuration we are using because
> > the SPI function that is in the PS (FPGA's Processing System) is not what we are
> > interfacing when using spi-engine. Copy of my test dts at end of email.
> > 
> > Currently, the SPI controller we are using to test with these exotic ADCs
> > is the spi-engine which is implemented in the FPGA as an IP block which
> > owns control of the bus lines (SPI, SDO, CS, ...). To alternate the
> > configuration of SPI lines (pull SDO (ADC SDI) up to VIO, connect controller CS
> > to ADC SDI, etc.) I think it should be done in the HDL project. I don't think
> > it's a good idea to hijack spi-engine lines through pinctrl.
> 
> Such functionality would need to be pushed to the spi controller driver
> which could know if there was any need to do anything like this, or if there
> was simply a register to set.
> 

Ack.

> > 
> > > 
> > > Ah ok.  This is implying that we are switching to a controllable
> > > mode to pull that pin high (or I guess one where it is always
> > > high).  I'm not sure if that's more common than an SPI controller
> > > where you can set the 'don't' care state to high or low.
> > > I assume we can't get away with just setting the output buffer
> > > to all 1s because it won't hold that state between transfers?  
> > 
> > I tried sending the tx buffer filled with 1s when testing this with a RPi4 but
> > it brought the controller SDO (ADC SDI) line low between each 8 bits of transfer
> > (attaching an image (yellow line is SCLK, green lines is controller SDO)).
> 
> Pity - thought that was overly optimistic.
> 
> > Anyway, can we have any guaranties with respect to controller SDO line behaviour
> > when its not clocking out data?
> 
> 
> > 
> > > 
> > > Feels like that could be rolled into the SPI subsystem with
> > > any necessary transitions handled there (maybe)  
> > 
> > To me, this sounds more reasonable than pinctrl.
> > Yeah, if we can set a don't' care state for the SDO line then that should be
> > enough for these ADCs.
> > Otherwise, can we really do anything if the controller can't keep SDO high?
> 
> There is one similar (ish) entry already.
> https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/spi/spi.h#L29
> #define	SPI_3WIRE_HIZ		_BITUL(15)	/* high impedance turnaround */
> in that it is controlling state in what I think would normally be a don't care state.
> 
> I think we could have an
> SPI_SDO_DONT_CARE_HIGH (naming to be improved upon ;) 
> that a driver could advertise support for and the spi device could request.
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/panel/panel-tpo-tpg110.c#L429
> 
> Implement that in the spi-gpio driver as a PoC probably and in your SPI copntoller
> driver.
> 
> Ultimately if the controller really isn't capable (including dances through pin
> mode changes if necessary) then the ADC won't work in this wiring with that host
> controller.
> 
> I'd propose something along these lines and see whether Mark + any other active
> SPI folk think it is reasonable or not?

Sounds promising. Will try implement something like that.

> 
> > 
> > > 
> > > Just to check, this isn't just a case of a missing pull up
> > > resistor to drag that line correctly when it isn't actively
> > > driven (combined with all 1s in the write buffer) etc?  
> > 
> > When using spi-engine, the controller SDO is connected to ADC SDI, controller
> > CS to ADC CNV and, for reg access, it works as conventional SPI.
> > spi-engine leaves the SDO line the state it was after last transfer so it we
> > can make it idle high during sample read. No hardware pull-up needed.
> 
> Fair enough. No multi master support I guess (that is a bit obscure for
> SPI).  A little ugly that it's dependent on the last access - so you would need
> to do a dummy access if the normal last bit was wrong level?

Seems I've been lucky with it but yes, we would need a dummy transfer to put
controller SDO line in the desired state. I'm thinking it should not be
difficult to make spi-engine SDO line always idle high (or idle in a
pre-configured state). I'll talk with the guys in the HDL team and what can be
done about it.

Thanks,
Marcelo

> 
> Jonathan
> 
> > 
> > Marcelo
> > 
> > > 
> > > Jonathan
> > > 
> > >   
> > 
> > The device tree source file I was using for testing with the ADC with the
> > changes for using pinctrl. Didn't really work.
> > 
> > // SPDX-License-Identifier: GPL-2.0
> > /*
> >  * Analog Devices ADAQ4003
> >  * https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad400x
> >  * https://wiki.analog.com/resources/eval/user-guides/ad400x
> >  *
> >  * hdl_project: <pulsar_adc_pmdz/zed>
> >  * board_revision: <>
> >  *
> >  * Copyright (C) 2016-2023 Analog Devices Inc.
> >  */
> > /dts-v1/;
> > 
> > #include "zynq-zed.dtsi"
> > #include "zynq-zed-adv7511.dtsi"
> > #include <dt-bindings/pinctrl/pinctrl-zynq.h>
> > 
> > / {
> > 	adc_vref: regulator-vref {
> > 		compatible = "regulator-fixed";
> > 		regulator-name = "EVAL 5V Vref";
> > 		regulator-min-microvolt = <5000000>;
> > 		regulator-max-microvolt = <5000000>;
> > 		regulator-always-on;
> > 	};
> > 
> > 	adc_vdd: regulator-vdd {
> > 		compatible = "regulator-fixed";
> > 		regulator-name = "Eval VDD supply";
> > 		regulator-min-microvolt = <1800000>;
> > 		regulator-max-microvolt = <1800000>;
> > 		regulator-always-on;
> > 	};
> > 
> > 	adc_vio: regulator-vio {
> > 		compatible = "regulator-fixed";
> > 		regulator-name = "Eval VIO supply";
> > 		regulator-min-microvolt = <3300000>;
> > 		regulator-max-microvolt = <3300000>;
> > 		regulator-always-on;
> > 	};
> > };
> > 
> > &pinctrl0 {
> > 	/* Restore conventional SPI pin configuration */
> > 	pinctrl_spi_default: default_config {
> > 		mux {
> > 			/* Are these the ones used by spi-engine? */
> > 			groups = "spi0_0_grp";
> > 			function = "spi0";
> > 		};
> > 		conf {
> > 			groups = "spi0_0_grp";
> > 			power-source = <IO_STANDARD_LVCMOS33>;
> > 		};
> > 		conf-spi-sdo {
> > 			pins = "MIO17"; /* SPI0 SDO? */
> > 			bias-disable;
> > 		};
> > 	};
> > 
> > 	/* Pull-up SPI SDO (ADC SDI) to VIO */
> > 	pinctrl_spi_single: single_config {
> > 		conf-spi-sdo {
> > 			pins = "MIO17"; /* Conflicts with SPI0 pin group */
> > 			bias-pull-up;
> > 		};
> > 	};
> > };
> > 
> > &fpga_axi {
> > 	rx_dma: rx-dmac@44a30000 {
> > 		compatible = "adi,axi-dmac-1.00.a";
> > 		reg = <0x44a30000 0x1000>;
> > 		#dma-cells = <1>;
> > 		interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
> > 		clocks = <&clkc 17>;
> > 
> > 		adi,channels {
> > 			#size-cells = <0>;
> > 			#address-cells = <1>;
> > 
> > 			dma-channel@0 {
> > 				reg = <0>;
> > 				adi,source-bus-width = <32>;
> > 				adi,source-bus-type = <1>;
> > 				adi,destination-bus-width = <64>;
> > 				adi,destination-bus-type = <0>;
> > 			};
> > 		};
> > 	};
> > 
> > 	axi_pwm_gen: axi-pwm-gen@ {
> > 		compatible = "adi,axi-pwmgen";
> > 		reg = <0x44b00000 0x1000>;
> > 		label = "cnv";
> > 		#pwm-cells = <2>;
> > 		clocks = <&spi_clk>;
> > 	};
> > 
> > 	spi_clk: axi-clkgen@44a70000 {
> > 		compatible = "adi,axi-clkgen-2.00.a";
> > 		reg = <0x44a70000 0x10000>;
> > 		#clock-cells = <0>;
> > 		clocks = <&clkc 15>, <&clkc 15>;
> > 		clock-names = "s_axi_aclk", "clkin1";
> > 		clock-output-names = "spi_clk";
> > 	};
> > 
> > 	axi_spi_engine_0: spi@44a00000 {
> > 		compatible = "adi,axi-spi-engine-1.00.a";
> > 		reg = <0x44a00000 0x1000>;
> > 		interrupt-parent = <&intc>;
> > 		interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> > 		clocks = <&clkc 15 &spi_clk>;
> > 		clock-names = "s_axi_aclk", "spi_clk";
> > 		num-cs = <1>;
> > 
> > 		#address-cells = <0x1>;
> > 		#size-cells = <0x0>;
> > 
> > 		adaq4003: adc@0 {
> > 			compatible = "adi,adaq4003";
> > 			reg = <0>;
> > 			spi-max-frequency = <102000000>;
> > 			spi-cpha;
> > 			pinctrl-names = "default", "single";
> > 			pinctrl-0 = <&pinctrl_spi_default>;
> > 			pinctrl-1 = <&pinctrl_spi_single>;
> > 			vdd-supply = <&adc_vdd>;
> > 			vio-supply = <&adc_vio>;
> > 			ref-supply = <&adc_vref>;
> > 			adi,high-z-input;
> > 			adi,gain-milli = <454>;
> > 		};
> > 	};
> > };
>
diff mbox series

Patch

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..ca06afb5149e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
@@ -0,0 +1,201 @@ 
+# 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
+    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.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
+      - adi,adaq4001
+      - adi,adaq4003
+
+  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 5.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. If 'single' mode is selected and this GPIO
+      is provided, it must be active low.
+    maxItems: 1
+
+  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.
+
+  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
+
+  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
+  - spi-cpha
+  - 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>;
+        /* Example for a AD devices */
+        adc@0 {
+            compatible = "adi,ad4020";
+            reg = <0>;
+            spi-cpha;
+            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>;
+        };
+    };
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        /* Example for a ADAQ devices */
+        adc@0 {
+            compatible = "adi,adaq4003";
+            reg = <0>;
+            spi-cpha;
+            adi,spi-mode = "single";
+            spi-max-frequency = <80000000>;
+            vdd-supply = <&supply_1_8V>;
+            vio-supply = <&supply_1_8V>;
+            ref-supply = <&supply_5V>;
+            adi,high-z-input;
+            adi,gain-milli = <454>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a7287cf44869..5dfe118a5dd3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1159,6 +1159,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