Re: [PATCH V6 1/3] dt-bindings: clock: document Amlogic S4 SoC PLL & peripheral clock controller

From: Jerome Brunet
Date: Fri Jan 20 2023 - 04:42:27 EST


On Mon 16 Jan 2023 at 17:31, Yu Tu <yu.tu@xxxxxxxxxxx> wrote:

> Hi Krzysztof,
> Thank you for your quick reply.
>
> On 2023/1/16 16:29, Krzysztof Kozlowski wrote:
>> [ EXTERNAL EMAIL ]
>> On 16/01/2023 08:42, Yu Tu wrote:
>>> Add the S4 PLL & peripheral clock controller dt-bindings in the s4 SoC
>>> family.
>>>
>>> Signed-off-by: Yu Tu <yu.tu@xxxxxxxxxxx>
>>> ---
>>> .../clock/amlogic,s4-peripherals-clkc.yaml | 104 ++++++++++++++
>>> .../bindings/clock/amlogic,s4-pll-clkc.yaml | 50 +++++++
>>> MAINTAINERS | 1 +
>>> .../clock/amlogic,s4-peripherals-clkc.h | 131 ++++++++++++++++++
>>> .../dt-bindings/clock/amlogic,s4-pll-clkc.h | 30 ++++
>>> 5 files changed, 316 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,s4-peripherals-clkc.yaml
>>> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,s4-pll-clkc.yaml
>>> create mode 100644 include/dt-bindings/clock/amlogic,s4-peripherals-clkc.h
>>> create mode 100644 include/dt-bindings/clock/amlogic,s4-pll-clkc.h
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/clock/amlogic,s4-peripherals-clkc.yaml
>>> b/Documentation/devicetree/bindings/clock/amlogic,s4-peripherals-clkc.yaml
>>> new file mode 100644
>>> index 000000000000..2deeff497754
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/amlogic,s4-peripherals-clkc.yaml
>>> @@ -0,0 +1,104 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/amlogic,s4-peripherals-clkc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Amlogic Meson S serials Peripherals Clock Controller
>>> +
>>> +maintainers:
>>> + - Neil Armstrong <neil.armstrong@xxxxxxxxxx>
>>> + - Jerome Brunet <jbrunet@xxxxxxxxxxxx>
>>> + - Yu Tu <yu.tu@xxxxxxxxxxx>
>>> +
>>> +properties:
>>> + compatible:
>>> + const: amlogic,s4-peripherals-clkc
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + clocks:
>>> + items:
>>> + - description: input fixed pll div2
>>> + - description: input fixed pll div2p5
>>> + - description: input fixed pll div3
>>> + - description: input fixed pll div4
>>> + - description: input fixed pll div5
>>> + - description: input fixed pll div7
>>> + - description: input hifi pll
>>> + - description: input gp0 pll
>>> + - description: input mpll0
>>> + - description: input mpll1
>>> + - description: input mpll2
>>> + - description: input mpll3
>>> + - description: input hdmi pll
>>> + - description: input oscillator (usually at 24MHz)
>>> + - description: input external 32kHz reference (optional)
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: fclk_div2
>>> + - const: fclk_div2p5
>>> + - const: fclk_div3
>>> + - const: fclk_div4
>>> + - const: fclk_div5
>>> + - const: fclk_div7
>>> + - const: hifi_pll
>>> + - const: gp0_pll
>>> + - const: mpll0
>>> + - const: mpll1
>>> + - const: mpll2
>>> + - const: mpll3
>>> + - const: hdmi_pll
>>> + - const: xtal
>>> + - const: ext_32k
>>> +
>>> + "#clock-cells":
>>> + const: 1
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - clocks
>>> + - clock-names
>>> + - "#clock-cells"
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/clock/amlogic,s4-peripherals-clkc.h>
>>> +
>>> + /* 32KHz reference crystal */
>>> + ext_32k: ref32k {
>>> + compatible = "fixed-clock";
>>> + #clock-cells = <0>;
>>> + clock-frequency = <32000>;
>>> + };
>> This wasn't here before. Drop it. It is trivial and it is not needed to
>> illustrate your device bindings. All clock bindings use it...
>>
>
> I'm fine with that. I don't know if Jerome agrees with that. Wait for
> him. See what he says.

This is a simple change related to your binding example.
Krzysztof is one of the DT maintainer. Please follow his recommendation.

>
>>> +
>>> + clkc_periphs: clock-controller@fe000000 {
>>> + compatible = "amlogic,s4-peripherals-clkc";
>>> + reg = <0xfe000000 0x49c>;
>>> + clocks = <&clkc_pll 3>,
>>> + <&clkc_pll 13>,
>>> + <&clkc_pll 5>,
>>> + <&clkc_pll 7>,
>>> + <&clkc_pll 9>,
>>> + <&clkc_pll 11>,
>>> + <&clkc_pll 17>,
>>> + <&clkc_pll 15>,
>>> + <&clkc_pll 25>,
>>> + <&clkc_pll 27>,
>>> + <&clkc_pll 29>,
>>> + <&clkc_pll 31>,
>>> + <&clkc_pll 20>,
>>> + <&xtal>,
>>> + <&ext_32k>;
>>> + clock-names = "fclk_div2", "fclk_div2p5", "fclk_div3", "fclk_div4",
>>> + "fclk_div5", "fclk_div7", "hifi_pll", "gp0_pll",
>>> + "mpll0", "mpll1", "mpll2", "mpll3", "hdmi_pll", "xtal",
>>> + "ext_32k";
>>> + #clock-cells = <1>;
>>> + };
>>> +...
>>> diff --git
>>> a/Documentation/devicetree/bindings/clock/amlogic,s4-pll-clkc.yaml
>>> b/Documentation/devicetree/bindings/clock/amlogic,s4-pll-clkc.yaml
>>> new file mode 100644
>>> index 000000000000..aeda4861cebe
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/amlogic,s4-pll-clkc.yaml
>>> @@ -0,0 +1,50 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/amlogic,s4-pll-clkc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Amlogic Meson S serials PLL Clock Controller
>>> +
>>> +maintainers:
>>> + - Neil Armstrong <neil.armstrong@xxxxxxxxxx>
>>> + - Jerome Brunet <jbrunet@xxxxxxxxxxxx>
>>> + - Yu Tu <yu.tu@xxxxxxxxxxx>
>>> +
>>> +properties:
>>> + compatible:
>>> + const: amlogic,s4-pll-clkc
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + clocks:
>>> + maxItems: 1
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: xtal
>>> +
>>> + "#clock-cells":
>>> + const: 1
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - clocks
>>> + - clock-names
>>> + - "#clock-cells"
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + clkc_pll: clock-controller@fe008000 {
>>> + compatible = "amlogic,s4-pll-clkc";
>>> + reg = <0xfe008000 0x1e8>;
>>> + clocks = <&xtal>;
>>> + clock-names = "xtal";
>>> + #clock-cells = <1>;
>>> + };
>>> +
>>> +...
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index f61eb221415b..26c82beeffda 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1897,6 +1897,7 @@ L: linux-amlogic@xxxxxxxxxxxxxxxxxxx
>>> S: Maintained
>>> F: Documentation/devicetree/bindings/clock/amlogic*
>>> F: drivers/clk/meson/
>>> +F: include/dt-bindings/clock/amlogic*
>>> F: include/dt-bindings/clock/gxbb*
>>> F: include/dt-bindings/clock/meson*
>>> diff --git a/include/dt-bindings/clock/amlogic,s4-peripherals-clkc.h
>>> b/include/dt-bindings/clock/amlogic,s4-peripherals-clkc.h
>>> new file mode 100644
>>> index 000000000000..bbec5094d5c3
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/amlogic,s4-peripherals-clkc.h
>>> @@ -0,0 +1,131 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> Unusual license... are you sure to license the bindings under GPLv4 or
>> GPLv5? Fine by me.
>>
>
> Yes.
>
>> Best regards,
>> Krzysztof
>>