Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 30 May 2023 18:22:06 +0100
From:   Conor Dooley <conor@kernel.org>
To:     fl.scratchpad@gmail.com
Cc:     jic23@kernel.org, Lars-Peter Clausen <lars@metafoo.de>,
        Michael Hennerich <Michael.Hennerich@analog.com>,
        Alexandru Tachici <alexandru.tachici@analog.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>, linux-iio@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v3 5/5] dt-bindings: iio: ad7192: Allow selection of
 clock modes
Message-ID: <20230530-cannabis-headstone-883c5b891dd3@spud>
References: <20230530075311.400686-1-fl.scratchpad@gmail.com>
 <20230530075311.400686-6-fl.scratchpad@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="GOC40fJ/cBBzzJa4"
Content-Disposition: inline
In-Reply-To: <20230530075311.400686-6-fl.scratchpad@gmail.com>
Precedence: bulk


--GOC40fJ/cBBzzJa4
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, May 30, 2023 at 09:53:11AM +0200, fl.scratchpad@gmail.com wrote:
> From: Fabrizio Lamarque <fl.scratchpad@gmail.com>
>=20
> AD7192 supports external clock sources, generated by a digital clock
> source or a crystal oscillator, or internally generated clock option
> without external components.
>=20
> Describe choice between internal and external clock, crystal or external
> oscillator, and internal clock output enable.
>=20
> Signed-off-by: Fabrizio Lamarque <fl.scratchpad@gmail.com>
> ---
>  .../bindings/iio/adc/adi,ad7192.yaml          | 27 ++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>=20
> 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:
> =20
>    clocks:
>      maxItems: 1
> -    description: phandle to the master clock (mclk)
> +    description: |
> +      Master clock (mclk). If not set, internal clock is used.
> =20
>    clock-names:
>      items:
> @@ -50,6 +51,17 @@ properties:
>    vref-supply:
>      description: VRef voltage supply
> =20
> +  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
> =20
> +dependencies:
> +  adi,clock-xtal: ['clocks', 'clock-names']
> +
>  required:
>    - compatible
>    - reg
> -  - clocks
> -  - clock-names
>    - interrupts
>    - dvdd-supply
>    - avdd-supply
> @@ -98,6 +111,13 @@ required:
> =20
>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      required:
> +        - clocks
> +        - clock-names
> +    then:
> +      properties:
> +        adi,int-clock-output-enable: false
> =20
>  unevaluatedProperties: false
> =20
> @@ -115,6 +135,7 @@ examples:
>              spi-cpha;
>              clocks =3D <&ad7192_mclk>;
>              clock-names =3D "mclk";
> +            adi,clock-xtal;
>              interrupts =3D <25 0x2>;
>              interrupt-parent =3D <&gpio>;
>              dvdd-supply =3D <&dvdd>;
> --=20
> 2.34.1
>=20

--GOC40fJ/cBBzzJa4
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZHYwvgAKCRB4tDGHoIJi
0qkFAP9uZ1N8bFxZsXbgW8kQo1+vlYKpinjPWQHQw4vNab1QdAEAiMZn03LxwMx2
/4Q9ctSo9xBk9Le4GVYdC0z4WQRcWgs=
=KNs6
-----END PGP SIGNATURE-----

--GOC40fJ/cBBzzJa4--