linux-kernel - Re: [PATCH v1 01/15] dt-bindings: iio: adc: ad7768-1: add synchronization over SPI property

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dc7f6461-6fce-4dbd-9be4-f7814053e7dc@baylibre.com>
Date: Tue, 7 Jan 2025 17:35:08 -0600
From: David Lechner <dlechner@...libre.com>
To: Jonathan Santos <Jonathan.Santos@...log.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, jic23@...nel.org,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 marcelo.schmitt1@...il.com
Subject: Re: [PATCH v1 01/15] dt-bindings: iio: adc: ad7768-1: add
 synchronization over SPI property

On 1/7/25 9:24 AM, Jonathan Santos wrote:
> Add adi,sync-in-spi property to enable synchronization over SPI.
> This should be used in the case when the GPIO cannot provide a
> pulse synchronous with the base MCLK signal.
> 
> User can choose between SPI, GPIO synchronization or neither of them,
> but only if a external pulse can be provided, for example, by another
> device in a multidevice setup.
> 

While we are fixing up these bindings, we could add some more trivial things,
like power supplies.

Also, the interrupt property could use a description since the chip has multiple
output pins. I assume it means the /DRDY pin?

> Signed-off-by: Jonathan Santos <Jonathan.Santos@...log.com>
> ---
>  .../bindings/iio/adc/adi,ad7768-1.yaml        | 24 ++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> index 3ce59d4d065f..55cec27bfe60 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> @@ -47,6 +47,15 @@ properties:
>        in any way, for example if the filter decimation rate changes.
>        As the line is active low, it should be marked GPIO_ACTIVE_LOW.
>  
> +  adi,sync-in-spi:

If this is saying that SYNC_OUT is connected to SYNC_IN, then I think the name
should be something like adi,sync-in-sync-out. SPI seems irrelevant here since
we should just be describing how things are wired up, not how it is being used.

But if we also need to consider the case where SYNC_OUT of one chip is connected
to SYNC_IN of another chip, we might want to consider using trigger-source
bindings instead (recently standardized in dtschema).

> +    description:
> +      Enables synchronization of multiple devices over SPI. This property is
> +      used when a signal synchronous to the base MCLK signal cannot be provided
> +      via GPIO. It requires the SYNC_OUT pin to be connected to the SYNC_IN pin
> +      on the ADC. In the case of multiple devices, the SYNC_OUT pin of one device
> +      should be routed to the SYNC_IN pins of the other devices.
> +    type: boolean
> +
>    reset-gpios:
>      maxItems: 1
>  
> @@ -65,7 +74,6 @@ required:
>    - vref-supply
>    - spi-cpol
>    - spi-cpha
> -  - adi,sync-in-gpios
>  
>  patternProperties:
>    "^channel@([0-9]|1[0-5])$":
> @@ -89,6 +97,20 @@ patternProperties:
>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
>  
> +  # adi,sync-in-gpios and adi,sync-in-spi are mutually exclusive (neither is also valid)
> +  - if:
> +      required:
> +        - adi,sync-in-gpios
> +    then:
> +      properties:
> +        adi,sync-in-spi: false
> +  - if:
> +      required:
> +        - adi,sync-in-spi
> +    then:
> +      properties:
> +        adi,sync-in-gpios: false

I think this can be simplified to using oneOf: to provide XOR validation

> +
>  unevaluatedProperties: false
>  
>  examples: