linux-kernel - Re: [PATCH v2 1/5] dt-bindings: clock: add Amlogic T7 PLL clock controller

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: <f205415c-097f-47bf-8b18-097ea034df64@amlogic.com>
Date: Fri, 17 Jan 2025 16:04:51 +0800
From: Jian Hu <jian.hu@...ogic.com>
To: Jerome Brunet <jbrunet@...libre.com>, Rob Herring <robh@...nel.org>
Cc: Xianwei Zhao <xianwei.zhao@...ogic.com>, Chuan Liu
 <chuan.liu@...ogic.com>, Neil Armstrong <neil.armstrong@...aro.org>,
 Kevin Hilman <khilman@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
 Michael Turquette <mturquette@...libre.com>,
 Dmitry Rokosov <ddrokosov@...rdevices.ru>,
 devicetree <devicetree@...r.kernel.org>,
 linux-clk <linux-clk@...r.kernel.org>,
 linux-amlogic <linux-amlogic@...ts.infradead.org>,
 linux-kernel <linux-kernel@...r.kernel.org>,
 linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 1/5] dt-bindings: clock: add Amlogic T7 PLL clock
 controller

Hi, Jerome


Thanks for your review.


On 2025/1/14 1:50, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Fri 10 Jan 2025 at 09:54, Rob Herring <robh@...nel.org> wrote:
>
>> On Wed, Jan 08, 2025 at 05:40:21PM +0800, Jian Hu wrote:
>>> Add DT bindings for the PLL clock controller of the Amlogic T7 SoC family.
>>>
>>> Signed-off-by: Jian Hu <jian.hu@...ogic.com>
>>> ---
>>>   .../bindings/clock/amlogic,t7-pll-clkc.yaml   | 103 ++++++++++++++++++
>>>   .../dt-bindings/clock/amlogic,t7-pll-clkc.h   |  57 ++++++++++
>>>   2 files changed, 160 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/clock/amlogic,t7-pll-clkc.yaml
>>>   create mode 100644 include/dt-bindings/clock/amlogic,t7-pll-clkc.h
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/clock/amlogic,t7-pll-clkc.yaml
>>> b/Documentation/devicetree/bindings/clock/amlogic,t7-pll-clkc.yaml
>>> new file mode 100644
>>> index 000000000000..fd0323678d37
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/amlogic,t7-pll-clkc.yaml
>>> @@ -0,0 +1,103 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +# Copyright (C) 2024 Amlogic, Inc. All rights reserved
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/amlogic,t7-pll-clkc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Amlogic T7 PLL Clock Control Controller
>>> +
>>> +maintainers:
>>> +  - Neil Armstrong <neil.armstrong@...aro.org>
>>> +  - Jerome Brunet <jbrunet@...libre.com>
>>> +  - Jian Hu <jian.hu@...ogic.com>
>>> +  - Xianwei Zhao <xianwei.zhao@...ogic.com>
>>> +
>>> +if:
>> Move this after 'required' section.
>>
>> Generally we put 'if' under 'allOf' because we're likely to have another
>> if/then schema on the next compatible added. If you don't think this
>> binding will ever get used on another chip, then it is fine as-is.
>>
>>> +  properties:
>>> +    compatible:
>>> +      contains:
>>> +        const: amlogic,t7-pll-mclk
>>> +
>>> +then:
>>> +  properties:
>>> +    clocks:
>>> +      items:
>>> +        - description: mclk pll input oscillator gate
>>> +        - description: 24M oscillator input clock source for mclk_sel_0
>>> +        - description: fix 50Mhz input clock source for mclk_sel_0
> The rate is whatever the clock will actually be. Better not to mention
> it in this doc.


OK, I will remove the rate here.

>>> +
>>> +    clock-names:
>>> +      items:
> one being "input" and other suffixed "_in" looks really odd
>
>>> +        - const: input
>>> +        - const: mclk_in0
>>> +        - const: mclk_in1
> or just in0, in1, in2 if you are going with Rob's suggestion.
> Having "mclk_" in the top level would be confusing.


Ok, I will use in0/in1/in2 for them.

> --
> 2.47.1
>
> --
> Jerome