linux-kernel - Re: [PATCH v3 5/5] dt-bindings: iio: ad7192: Allow selection of clock modes

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]

Date:   Tue, 30 May 2023 18:22:06 +0100
From:   Conor Dooley <conor@...nel.org>
To:     fl.scratchpad@...il.com
Cc:     jic23@...nel.org, Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Alexandru Tachici <alexandru.tachici@...log.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v3 5/5] dt-bindings: iio: ad7192: Allow selection of
 clock modes

On Tue, May 30, 2023 at 09:53:11AM +0200, fl.scratchpad@...il.com wrote:
> From: Fabrizio Lamarque <fl.scratchpad@...il.com>
> 
> AD7192 supports external clock sources, generated by a digital clock
> source or a crystal oscillator, or internally generated clock option
> without external components.
> 
> Describe choice between internal and external clock, crystal or external
> oscillator, and internal clock output enable.
> 
> Signed-off-by: Fabrizio Lamarque <fl.scratchpad@...il.com>
> ---
>  .../bindings/iio/adc/adi,ad7192.yaml          | 27 ++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> index 16def2985ab4..f7ecfd65ad80 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> @@ -32,7 +32,8 @@ properties:
>  
>    clocks:
>      maxItems: 1
> -    description: phandle to the master clock (mclk)
> +    description: |
> +      Master clock (mclk). If not set, internal clock is used.
>  
>    clock-names:
>      items:
> @@ -50,6 +51,17 @@ properties:
>    vref-supply:
>      description: VRef voltage supply
>  
> +  adi,clock-xtal:
> +    description: |
> +      Select whether an external crystal oscillator or an external
> +      clock is applied as master (mclk) clock.
> +    type: boolean

Am I being daft, or are these the same thing? If they are not, and use
different input pins, I think it should be explained as it not clear.
Could you explain why we actually care that the source is a xtal versus
it being mclk, and why just having master clock is not sufficient?

> +  adi,int-clock-output-enable:
> +    description: |
> +      When internal clock is selected, this bit enables clock out pin.
> +    type: boolean

And this one makes you a clock provider, so the devices advocate
position would be that you know that this bit should be set if
"clocks" is not present and a consumer requests a clock.
I don't seem to have got the driver patches (at least not in this
mailbox), so I have got no information on how you've actually implemented
this.

Cheers,
Conor.

> +
>    adi,rejection-60-Hz-enable:
>      description: |
>        This bit enables a notch at 60 Hz when the first notch of the sinc
> @@ -84,11 +96,12 @@ properties:
>      description: see Documentation/devicetree/bindings/iio/adc/adc.yaml
>      type: boolean
>  
> +dependencies:
> +  adi,clock-xtal: ['clocks', 'clock-names']
> +
>  required:
>    - compatible
>    - reg
> -  - clocks
> -  - clock-names
>    - interrupts
>    - dvdd-supply
>    - avdd-supply
> @@ -98,6 +111,13 @@ required:
>  
>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      required:
> +        - clocks
> +        - clock-names
> +    then:
> +      properties:
> +        adi,int-clock-output-enable: false
>  
>  unevaluatedProperties: false
>  
> @@ -115,6 +135,7 @@ examples:
>              spi-cpha;
>              clocks = <&ad7192_mclk>;
>              clock-names = "mclk";
> +            adi,clock-xtal;
>              interrupts = <25 0x2>;
>              interrupt-parent = <&gpio>;
>              dvdd-supply = <&dvdd>;
> -- 
> 2.34.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)