[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+GgBR-H8W_YS3gPrrvxAWoQybjEb-p36pqxEatFbEAnuz2DvQ@mail.gmail.com>
Date: Sat, 7 Sep 2024 07:59:53 +0300
From: Alexandru Ardelean <aardelean@...libre.com>
To: David Lechner <dlechner@...libre.com>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, jic23@...nel.org, krzk+dt@...nel.org,
robh@...nel.org, lars@...afoo.de, michael.hennerich@...log.com,
gstols@...libre.com
Subject: Re: [PATCH v4 7/8] dt-bindings: iio: adc: add docs for
AD7606C-{16,18} parts
On Fri, Sep 6, 2024 at 12:54 AM David Lechner <dlechner@...libre.com> wrote:
>
> On 9/5/24 3:24 AM, Alexandru Ardelean wrote:
> > The driver will support the AD7606C-16 and AD7606C-18.
> > This change adds the compatible strings for these devices.
> >
> > The AD7606C-16,18 channels also support these (individually configurable)
> > types of channels:
> > - bipolar single-ended
> > - unipolar single-ended
> > - bipolar differential
> >
> > Signed-off-by: Alexandru Ardelean <aardelean@...libre.com>
> > ---
> > .../bindings/iio/adc/adi,ad7606.yaml | 109 ++++++++++++++++++
> > 1 file changed, 109 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > index 69408cae3db9..57537ab0ec82 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > @@ -14,6 +14,8 @@ description: |
> > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7605-4.pdf
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
> > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf
> > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf
> > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf
> > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
> >
> > properties:
> > @@ -24,11 +26,19 @@ properties:
> > - adi,ad7606-6
> > - adi,ad7606-8 # Referred to as AD7606 (without -8) in the datasheet
> > - adi,ad7606b
> > + - adi,ad7606c-16
> > + - adi,ad7606c-18
> > - adi,ad7616
> >
> > reg:
> > maxItems: 1
> >
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > spi-cpha: true
> >
> > spi-cpol: true
> > @@ -114,6 +124,46 @@ properties:
> > assumed that the pins are hardwired to VDD.
> > type: boolean
> >
> > +patternProperties:
> > + "^channel@[1-8]$":
> > + type: object
> > + $ref: adc.yaml
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + reg:
> > + description:
> > + The channel number, as specified in the datasheet (from 1 to 8).
> > + minimum: 1
> > + maximum: 8
> > +
> > + diff-channels:
> > + description:
> > + Each channel can be configured as a differential bipolar channel.
> > + The ADC uses the same positive and negative inputs for this.
> > + This property must be specified as 'reg' (or the channel number) for
> > + both positive and negative inputs (i.e. diff-channels = <reg reg>).
> > + items:
> > + minimum: 1
> > + maximum: 8
> > +
> > + bipolar:
> > + description:
> > + Each channel can be configured as a unipolar or bipolar single-ended.
> > + When this property is not specified, it's unipolar, so the ADC will
> > + have only the positive input wired.
> > + For this ADC the 'diff-channels' & 'bipolar' properties are mutually
> > + exclusive.
> > +
> > + required:
> > + - reg
> > +
> > + oneOf:
> > + - required:
> > + - diff-channels
> > + - required:
> > + - bipolar
>
> The datasheet (ad7606c-18.pdf) lists the following combinations:
>
> * Bipolar single-ended
> * Unipolar single-ended
> * Bipolar differential
>
> The logic in the oneOf: doesn't match this.
>
> This I think this would be sufficient:
>
> - if:
> required: [diff-channels]
> then:
> required: [bipolar]
So here, I am a bit vague.
This makes 'bipolar' mandatory if 'diff-channels' is mandatory, right?
But then 'bipolar' (on its own) becomes optional?
The way I understood the oneOf case is that:
1. if it's 'diff-channels' then it's specified 'bipolar differential'.
2. if it's 'bipolar' then it's specified as 'bipolar single-ended'
3. otherwise it's unipolar
4. oneOf enforces that at least 'diff-channels' or 'bipolar' is
specified if there is a channel node
>
> > +
> > required:
> > - compatible
> > - reg
> > @@ -170,6 +220,17 @@ allOf:
> > adi,conversion-start-gpios:
> > maxItems: 1
> >
> > + - if:
> > + not:
> > + properties:
> > + compatible:
> > + enum:
> > + - adi,ad7606c-16
> > + - adi,ad7606c-18
> > + then:
> > + patternProperties:
> > + "^channel@[1-8]$": false
> > +
> > unevaluatedProperties: false
> >
> > examples:
> > @@ -202,4 +263,52 @@ examples:
> > standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
> > };
> > };
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + spi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + adc@0 {
> > + compatible = "adi,ad7606c-18";
> > + reg = <0>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + spi-max-frequency = <1000000>;
> > + spi-cpol;
> > + spi-cpha;
> > +
> > + avcc-supply = <&adc_vref>;
> > + vdrive-supply = <&vdd_supply>;
> > +
> > + interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> > + interrupt-parent = <&gpio>;
> > +
> > + adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
> > + reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
> > + adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
> > + standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
> > +
> > + adi,sw-mode;
> > +
> > + channel@1 {
> > + reg = <1>;
> > + diff-channels = <1 1>;
>
> bipolar;
>
> > + };
> > +
> > + channel@3 {
> > + reg = <3>;
> > + bipolar;
> > + };
> > +
> > + channel@8 {
> > + reg = <8>;
> > + diff-channels = <8 8>;
>
> bipolar;
>
> > + };
> > +
> > + };
> > + };
> > ...
>
Powered by blists - more mailing lists