Re: [PATCH 6/7] dt-bindings: iio: adc: add adi,ad7606c-{16,18} compatible strings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 19, 2024 at 09:47:16AM +0300, 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
> 
> This DT adds support for 'channel@X' nodes'

I don't understand this sentence, suggest to drop it.


> 
> Signed-off-by: Alexandru Ardelean <aardelean@xxxxxxxxxxxx>
> ---
>  .../bindings/iio/adc/adi,ad7606.yaml          | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> index 69408cae3db9..f9e177de3f8c 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,6 +26,8 @@ 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:
> @@ -114,6 +118,30 @@ properties:
>        assumed that the pins are hardwired to VDD.
>      type: boolean
>  
> +patternProperties:
> +  "^channel@([0-9a-f])$":

[0-7]

> +    type: object
> +    $ref: adc.yaml
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel number.
> +        minimum: 0
> +        maximum: 7
> +
> +      diff-channel:
> +        description: Channel is bipolar differential.

There is diff-channels property, why do we need one more?

> +        type: boolean
> +
> +      bipolar:
> +        description: |
> +          Channel is bipolar single-ended. If 'diff-channel' is set, then
> +          the value of this property will be ignored.

Then provide here allOf:if:then which makes it false if diff-channel(s)
is present. And then drop entire property, because you duplicate what's
in adc.yaml.


> +        type: boolean

Blank line.

> +    required:
> +      - reg
> +
>  required:
>    - compatible
>    - reg
> @@ -170,6 +198,21 @@ allOf:
>          adi,conversion-start-gpios:
>            maxItems: 1
>  
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - adi,ad7606c-16
> +                - adi,ad7606c-18
> +    then:
> +      patternProperties:
> +        "^channel@([0-9a-f])$":
> +          properties:
> +            diff-channels: false
> +            bipolar: true

? Drop, no clue what you want to say here. But more important, you are
now adding channels to other variants. Split your commit between new
device and new properties for existing devices.


Best regards,
Krzysztof





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux