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]

Message-ID: <01dae949-4018-37f4-2dd9-cbecbd65b9a1@linaro.org>
Date:   Wed, 31 May 2023 21:24:00 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Fabrizio Lamarque <fl.scratchpad@...il.com>
Cc:     Conor Dooley <conor@...nel.org>, 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 31/05/2023 11:40, Fabrizio Lamarque wrote:
> On Wed, May 31, 2023 at 9:14 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@...aro.org> wrote:
>>
>> On 31/05/2023 08:59, Fabrizio Lamarque wrote:
>>> On Tue, May 30, 2023 at 7:22 PM Conor Dooley <conor@...nel.org> wrote:
>>>>
>>>> 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?
>>>
>>> I may revise the description as follows. Feel free to add your suggestions
>>> in case it is still not clear enough.
>>>
>>> "Select whether an external crystal oscillator between MCLK1 and MCLK2 or
>>> an external CMOS-compatible clock on MCLK2 is used as master clock".
>>>
>>> This is used to properly set CLK0 and CLK1 bits in the MODE register.
>>> I guess most applications would use an external crystal or internal clock.
>>> The external digital clock would allow synchronization of multiple ADCs,
>>
>> Description confuses me. Why would it matter what type of clock you have
>> as input - external crystal oscillator or external CMOS-compatible
>> clock? Later you refer to "internal", so maybe you meant here also
>> internal for one of the options?
> 
> The AD7192 needs to be configured according to the type of external
> clock that is
> applied on MCLK1/MCLK2 pins in order to activate the correct circuitry.
> 
> Here are some citations from the datasheet:
> 
> MCLK2 pin description:
> "The AD7192 has an internal 4.92 MHz clock. This internal clock can be
> made available
> on the MCLK2 pin. The clock for the AD7192 can be provided externally
> also in the form
> of a crystal or external clock. A crystal can be tied across the MCLK1
> and MCLK2 pins.
> Alternatively, the MCLK2 pin can be driven with a CMOS-compatible clock and the
> MCLK1 pin left unconnected."
> 
> Each of these clock modes have to be configured via AD7192 mode register.
> (Clock source configuration bits, mode register, CLK0 and CLK1).
> Here is their description from datasheet:
> 
> "Either the on-chip 4.92 MHz clock or an external clock can be used.
> The ability to
> use an external clock allows several AD7192 devices to be synchronized. Also,
> 50 Hz/60 Hz rejection is improved when an accurate external clock
> drives the AD7192."
> 
> The choice between internal clock, external crystal oscillator or
> external CMOS digital
> clock is a decision of the HW designer driven by noise rejection,
> synchronization, and
> cost requirements.
> 
> If possible, I kindly ask you suggestions on how to adjust the description
> so that it would be cleaner.

It's fine. I missed that part that first option requires feeding the
clock through two pins ("and").

Best regards,
Krzysztof