linux-kernel - Re: [PATCH 1/2] dt-bindings: clock: Add bindings for Renesas ProXO

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]

Date:   Fri, 28 Oct 2022 19:55:59 -0400
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Alex Helms <alexander.helms.jy@...esas.com>,
        Rob Herring <robh@...nel.org>
Cc:     linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-clk@...r.kernel.org, sboyd@...nel.org,
        mturquette@...libre.com, geert+renesas@...der.be
Subject: Re: [PATCH 1/2] dt-bindings: clock: Add bindings for Renesas ProXO

On 22/09/2022 14:06, Alex Helms wrote:
> On 9/15/2022 5:08 AM, Rob Herring wrote:
>> On Wed, Sep 14, 2022 at 02:18:08PM -0700, Alex Helms wrote:
>>> Add dt bindings for the Renesas ProXO oscillator.
>>>
>>> Signed-off-by: Alex Helms <alexander.helms.jy@...esas.com>

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.

>>> ---
>>>  .../bindings/clock/renesas,proxo.yaml         | 50 +++++++++++++++++++
>>>  MAINTAINERS                                   |  5 ++
>>>  2 files changed, 55 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/renesas,proxo.yaml b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>>> new file mode 100644
>>> index 000000000..2ae131431
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml
>>> @@ -0,0 +1,50 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fclock%2Frenesas%2Cversaclock7.yaml%23&amp;data=05%7C01%7Calexander.helms.jy%40renesas.com%7C34ce960b2542459b61d408da97130b50%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637988405289843575%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=izvR%2FXbUNVXknaODSnMmcetPnn%2Ff3Jk%2F6RuBdZ7uSa0%3D&amp;reserved=0
>>> +$schema: https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Calexander.helms.jy%40renesas.com%7C34ce960b2542459b61d408da97130b50%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637988405289843575%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=dQ7nZtgfTE5DrXB1iavJuOhSWmGl4tYI1m6iZpkPAhA%3D&amp;reserved=0
>>> +
>>> +title: Renesas ProXO Oscillator Device Tree Bindings
>>> +
>>> +maintainers:
>>> +  - Alex Helms <alexander.helms.jy@...esas.com>
>>> +
>>> +description: |
>>
>> Don't need '|' if no formatting to preserve.
>>
>>> +  Renesas ProXO is a family of programmable ultra-low phase noise 
>>> +  quartz-based oscillators.
>>
>> Are they all the same from a s/w standpoint. If not you need specific 
>> compatibles for the differences. 
> 
> At this time only the XP variant is supported and this is reflected in
> the "compatible" string.

You should document everything, if possible, not what is supported by
some version of some system/driver/firmware.

> 
>>
>>> +
>>> +properties:
>>> +  '#clock-cells':
>>> +    const: 0
>>> +
>>> +  compatible:
>>> +    enum:
>>> +      - renesas,proxo-xp
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  renesas,xtal:
>>> +    description: Internal crystal frequency, default is 50000000 (50MHz)
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>
>> Use 'clock-frequency'.
> 
> I think "crystal-frequency" would make more sense. The parts come
> programmed to a factory clock frequency which is different than the
> internal crystal frequency.

We have a standard property and it should be used, when applicable.
Don't encode here frequency for something else than the driving clock.

Best regards,
Krzysztof