Re: [PATCH v6 4/6] dt-bindings: iio: adc: ad7192: Add clock provider

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 24 Jun 2024 15:49:39 +0300
Alisa-Dariana Roman <alisadariana@gmail.com> wrote:

> Internal clock of AD719X devices can be made available on MCLK2 pin. Add
> clock provider to support this functionality.
> 
> The clock source can be either provided externally or the internal clock
> is used. Pair of clocks and clock-names property is mutally exclusive
> with #clock-cells property.
> 
> Modify second example to showcase the mode where internal clock is used.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad7192.yaml   | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> index c3adc32684cf..384bff7e9bb7 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> @@ -42,13 +42,17 @@ properties:
>      description:
>        Optionally, either a crystal can be attached externally between MCLK1 and
>        MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2
> -      pin. If absent, internal 4.92MHz clock is used.
> +      pin. If absent, internal 4.92MHz clock is used, which can be made
> +      available on MCLK2 pin.
>  
>    clock-names:
>      enum:
>        - xtal
>        - mclk
>  
> +  "#clock-cells":
> +    const: 0
> +
>    interrupts:
>      maxItems: 1
>  
> @@ -169,6 +173,12 @@ allOf:
>        required:
>          - clocks
>          - clock-names
> +  - oneOf:
> +      - required:
> +          - clocks
> +          - clock-names
> +      - required:
> +          - "#clock-cells"

Just a heads up that in the next patch discussion we are considering if the
driver should fail to probe or not if the clock provider stuff isn't here.

This is a bit like io-channels-cells where we have listed it as required
for some devices but not others and left that at discretion of the the
binding writer as it often reflects likely rather than possible usecases.

Here though I think this is possibly a backwards compatibility break we
don't need to make.

Jonathan

>  
>  unevaluatedProperties: false
>  
> @@ -214,8 +224,7 @@ examples:
>              spi-max-frequency = <1000000>;
>              spi-cpol;
>              spi-cpha;
> -            clocks = <&ad7192_mclk>;
> -            clock-names = "mclk";
> +            #clock-cells = <0>;
>              interrupts = <25 0x2>;
>              interrupt-parent = <&gpio>;
>              aincom-supply = <&aincom>;