linux-kernel - Re: [RFC 1/2] dt-bindings: clk: versaclock5: Add load capacitance properties

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] [day] [month] [year] [list]

Message-ID: <7bad753d-a551-8810-7b12-5ec5ea9263d4@lucaceresoli.net>
Date:   Wed, 13 Jan 2021 15:39:35 +0100
From:   Luca Ceresoli <luca@...aceresoli.net>
To:     Adam Ford <aford173@...il.com>
Cc:     linux-clk <linux-clk@...r.kernel.org>,
        Adam Ford-BE <aford@...conembedded.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC 1/2] dt-bindings: clk: versaclock5: Add load capacitance
 properties

Hi Adam,

On 09/01/21 03:48, Adam Ford wrote:
> On Fri, Jan 8, 2021 at 4:49 PM Luca Ceresoli <luca@...aceresoli.net> wrote:
>>
>> Hi Adam,
>>
>> On 06/01/21 18:38, Adam Ford wrote:
>>> There are two registers which can set the load capacitance for
>>> XTAL1 and XTAL2. These are optional registers when using an
>>> external crystal.  Update the bindings to support them.
>>>
>>> Signed-off-by: Adam Ford <aford173@...il.com>
>>> ---
>>>  .../devicetree/bindings/clock/idt,versaclock5.yaml   | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
>>> index 2ac1131fd922..e5e55ffb266e 100644
>>> --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
>>> @@ -59,6 +59,18 @@ properties:
>>>      minItems: 1
>>>      maxItems: 2
>>>
>>> +  idt,xtal1-load-femtofarads:
>>
>> I wonder whether we should have a common, vendor independent property.
> 
> That would be nice.
> 
>> In mainline we have xtal-load-pf (ti,cdce925.txt bindings) which has no
>> vendor prefix. However I don't know how much common it is to need
> 
> rtc-pcf85063.c uses  quartz-load-femtofarads, so there is already some
> discrepancy.
> 
> Since the unit of measure here is femtofarads, using pF in the name seems wrong.
> We need to read the data as a u32, so femtofarads works better than
> pF, which would require a decimal point.
> 
>> different loads for x1 and x2. Any hardware engineer around?
> 
> I talked to a hardware engineer where I work, and he said it makes
> sense to keep them the same.  I only separated them because there are
> two registers, and I assumed there might be a reason to have X1 and X2
> be different, but I'm ok with reading one value and writing it to two
> different registers.

If both your HW engineer and the Renesas docs say setting different
values is not useful in real life, and other drivers don't set different
values as well, it looks like that is the reasonable way. I think it
also increases likelihood of establishing a unique property name to be
used for all future chips.

-- 
Luca