netdev - Re: [PATCH 1/3] dt: adin: document clk-out property

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: <bc0e507b-338b-8a86-1a7b-8055e2cf9a3a@solid-run.com>
Date:   Sun, 10 Apr 2022 21:41:31 +0300
From:   Josua Mayer <josua@...id-run.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        netdev@...r.kernel.org
Cc:     alvaro.karsz@...id-run.com,
        Michael Hennerich <michael.hennerich@...log.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Alexandru Ardelean <alexandru.ardelean@...log.com>
Subject: Re: [PATCH 1/3] dt: adin: document clk-out property

\o/

Thank you for the quick reply!

Am 10.04.22 um 17:21 schrieb Krzysztof Kozlowski:
> On 10/04/2022 12:46, Josua Mayer wrote:
>> The ADIN1300 supports generating certain clocks on its GP_CLK pin.
>> Add a DT property to specify the frequency.
>>
>> Due to the complexity of the clock configuration register, for now only
>> 125MHz is documented.
> Thank you for your patch. There is something to discuss/improve.
>
> Adjust subject prefix to the subsystem (dt-bindings, not dt, missing net).
Ack. So something like
dt-bindings: net: adin: document clk-out property
>> Signed-off-by: Josua Mayer <josua@...id-run.com>
>> ---
>>   Documentation/devicetree/bindings/net/adi,adin.yaml | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
>> index 1129f2b58e98..4e421bf5193d 100644
>> --- a/Documentation/devicetree/bindings/net/adi,adin.yaml
>> +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
>> @@ -36,6 +36,11 @@ properties:
>>       enum: [ 4, 8, 12, 16, 20, 24 ]
>>       default: 8
>>   
>> +  adi,clk-out-frequency:
> Use types defined by the dtschema, so "adi,clk-out-hz". Then no need for
> type/ref.
That sounds useful, I was not aware. The only inspiration I used was the 
at803x driver ...
It seemed natural to share the property name as it serves the same 
purpose here.
>> +    description: Clock output frequency in Hertz.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [125000000]
> If only one value, then "const: 125000000", but why do you need such
> value in DT if it is always the same?
Yes yes yes.
 From my understanding of the adin1300 data-sheet, it can provide either 
a 25MHz clock or a 125MHz clock on its GP_CLK pin. So for the context of 
this feature we would have two options. However because we found the 
documentation very confusing we skipped the 25MHz option.

Actually my statement above omits some of the options.
- There are actually two 125MHz clocks, the first called "recovered" and 
the second "free running".
- One can let the phy choose the rate based on its internal state.
This is indicated on page 73 of the datasheet
(https://www.analog.com/media/en/technical-documentation/data-sheets/adin1300.pdf)

Because of this confusion we wanted to for now only allow selecting the 
free-running 125MHz clock.

Sincerely
Josua Mayer