[PATCH] dt-bindings: clock: Add binding for TI clksel

[PATCH] dt-bindings: clock: Add binding for TI clksel

[Tony Lindgren]

In order to prepare for fixing lots of devicetree unique_unit_address
warnings for the TI clock nodes, let's add a binding for the clksel
clocks. This allows us to move the overlapping devicetree clocks to be
children of the related clksel nodes. And then we need the reg property
only for the parent clksel node.

We want to set #clock-cells = <1> in case we ever start parsing the clkcsel
clocks directly using a clksel driver rather than using the existing
component clock drivers and child nodes. We also need to update the clock
drivers to get the IO address from the parent clksel node before updating
the dts files. These will be posted as separate patches.

Cc: Tero Kristo <kristo@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 .../bindings/clock/ti/ti,clksel.yaml          | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml

diff --git a/Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml b/Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml
new file mode 100644
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/ti/ti,clksel.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Binding for TI clksel clock
+
+maintainers:
+  - Tony Lindgren <tony@atomide.com>
+
+description: |
+  The TI CLKSEL clocks consist of consist of a mux, divider, multiplier and
+  gate bits.
+
+properties:
+  compatible:
+    const: ti,clksel
+
+  reg:
+    maxItems: 1
+    description: The CLKSEL register
+
+  "#clock-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+
+additionalProperties: true
+
+examples:
+  - |
+    clksel_gfx_fclk: clock@52c {
+      compatible = "ti,clksel";
+      reg = <0x25c 0x4>;
+      #clock-cells = <1>;
+    };
+...
-- 
2.34.1

Re: [PATCH] dt-bindings: clock: Add binding for TI clksel

[Rob Herring]

On Fri, Dec 17, 2021 at 01:36:40PM +0200, Tony Lindgren wrote:
> In order to prepare for fixing lots of devicetree unique_unit_address
> warnings for the TI clock nodes, let's add a binding for the clksel
> clocks. This allows us to move the overlapping devicetree clocks to be
> children of the related clksel nodes. And then we need the reg property
> only for the parent clksel node.
> 
> We want to set #clock-cells = <1> in case we ever start parsing the clkcsel
> clocks directly using a clksel driver rather than using the existing
> component clock drivers and child nodes. We also need to update the clock
> drivers to get the IO address from the parent clksel node before updating
> the dts files. These will be posted as separate patches.
> 
> Cc: Tero Kristo <kristo@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  .../bindings/clock/ti/ti,clksel.yaml          | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml b/Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/ti/ti,clksel.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Binding for TI clksel clock
> +
> +maintainers:
> +  - Tony Lindgren <tony@atomide.com>
> +
> +description: |
> +  The TI CLKSEL clocks consist of consist of a mux, divider, multiplier and
> +  gate bits.
> +
> +properties:
> +  compatible:
> +    const: ti,clksel
> +
> +  reg:
> +    maxItems: 1
> +    description: The CLKSEL register
> +
> +  "#clock-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +
> +additionalProperties: true

Like what properties?

true is only used for common, incomplete schemas referenced by device 
schemas.

> +
> +examples:
> +  - |
> +    clksel_gfx_fclk: clock@52c {
> +      compatible = "ti,clksel";
> +      reg = <0x25c 0x4>;
> +      #clock-cells = <1>;
> +    };
> +...
> -- 
> 2.34.1
> 

Re: [PATCH] dt-bindings: clock: Add binding for TI clksel

[Tony Lindgren]

Hi,

* Rob Herring <robh@kernel.org> [211221 18:13]:
> On Fri, Dec 17, 2021 at 01:36:40PM +0200, Tony Lindgren wrote:
> > +additionalProperties: true
> 
> Like what properties?
> 
> true is only used for common, incomplete schemas referenced by device 
> schemas.

There is a collection of the current component clock child nodes for each
clksel instance. I got warnings with "additionalProperties: false", but
maybe the child clock nodes need to be somehow specified in the binding?

For example, below is a sample patch for am335x CLKSEL_GFX_FCLK using a
clksel parent node with the child nodes moved to fix warnings for
unique_unit_address. It also has clock-output-names property added to
avoid the node naming warnings. For the other clksel instances, they can
be a collection of dividers, multipliers, gates and muxes.

Regards,

Tony

8< --------
diff --git a/arch/arm/boot/dts/am33xx-clocks.dtsi b/arch/arm/boot/dts/am33xx-clocks.dtsi
--- a/arch/arm/boot/dts/am33xx-clocks.dtsi
+++ b/arch/arm/boot/dts/am33xx-clocks.dtsi
@@ -494,20 +494,27 @@ mmc_clk: mmc_clk {
 		clock-div = <2>;
 	};
 
-	gfx_fclk_clksel_ck: gfx_fclk_clksel_ck@52c {
-		#clock-cells = <0>;
-		compatible = "ti,mux-clock";
-		clocks = <&dpll_core_m4_ck>, <&dpll_per_m2_ck>;
-		ti,bit-shift = <1>;
-		reg = <0x052c>;
-	};
+	clock@52c {
+		compatible = "ti,clksel";
+		reg = <0x52c>;
+		#clock-cells = <1>;
+		#address-cells = <0>;
+
+		gfx_fclk_clksel_ck: clock-gfx-fclk-clksel {
+			#clock-cells = <0>;
+			compatible = "ti,mux-clock";
+			clock-output-names = "gfx_fclk_clksel_ck";
+			clocks = <&dpll_core_m4_ck>, <&dpll_per_m2_ck>;
+			ti,bit-shift = <1>;
+		};
 
-	gfx_fck_div_ck: gfx_fck_div_ck@52c {
-		#clock-cells = <0>;
-		compatible = "ti,divider-clock";
-		clocks = <&gfx_fclk_clksel_ck>;
-		reg = <0x052c>;
-		ti,max-div = <2>;
+		gfx_fck_div_ck: clock-gfx-fck-div {
+			#clock-cells = <0>;
+			compatible = "ti,divider-clock";
+			clock-output-names = "gfx_fck_div_ck";
+			clocks = <&gfx_fclk_clksel_ck>;
+			ti,max-div = <2>;
+		};
 	};
 
 	sysclkout_pre_ck: sysclkout_pre_ck@700 {

Re: [PATCH] dt-bindings: clock: Add binding for TI clksel

[Rob Herring]

On Wed, Dec 22, 2021 at 12:24 AM Tony Lindgren <tony@atomide.com> wrote:
>
> Hi,
>
> * Rob Herring <robh@kernel.org> [211221 18:13]:
> > On Fri, Dec 17, 2021 at 01:36:40PM +0200, Tony Lindgren wrote:
> > > +additionalProperties: true
> >
> > Like what properties?
> >
> > true is only used for common, incomplete schemas referenced by device
> > schemas.
>
> There is a collection of the current component clock child nodes for each
> clksel instance. I got warnings with "additionalProperties: false", but
> maybe the child clock nodes need to be somehow specified in the binding?

If everything else is a child node, then you can do:

additionalProperties:
  type: object

>
> For example, below is a sample patch for am335x CLKSEL_GFX_FCLK using a
> clksel parent node with the child nodes moved to fix warnings for
> unique_unit_address. It also has clock-output-names property added to
> avoid the node naming warnings. For the other clksel instances, they can
> be a collection of dividers, multipliers, gates and muxes.
>
> Regards,
>
> Tony
>
> 8< --------
> diff --git a/arch/arm/boot/dts/am33xx-clocks.dtsi b/arch/arm/boot/dts/am33xx-clocks.dtsi
> --- a/arch/arm/boot/dts/am33xx-clocks.dtsi
> +++ b/arch/arm/boot/dts/am33xx-clocks.dtsi
> @@ -494,20 +494,27 @@ mmc_clk: mmc_clk {
>                 clock-div = <2>;
>         };
>
> -       gfx_fclk_clksel_ck: gfx_fclk_clksel_ck@52c {
> -               #clock-cells = <0>;
> -               compatible = "ti,mux-clock";
> -               clocks = <&dpll_core_m4_ck>, <&dpll_per_m2_ck>;
> -               ti,bit-shift = <1>;
> -               reg = <0x052c>;
> -       };
> +       clock@52c {
> +               compatible = "ti,clksel";
> +               reg = <0x52c>;
> +               #clock-cells = <1>;
> +               #address-cells = <0>;
> +
> +               gfx_fclk_clksel_ck: clock-gfx-fclk-clksel {
> +                       #clock-cells = <0>;
> +                       compatible = "ti,mux-clock";
> +                       clock-output-names = "gfx_fclk_clksel_ck";
> +                       clocks = <&dpll_core_m4_ck>, <&dpll_per_m2_ck>;
> +                       ti,bit-shift = <1>;
> +               };
>
> -       gfx_fck_div_ck: gfx_fck_div_ck@52c {
> -               #clock-cells = <0>;
> -               compatible = "ti,divider-clock";
> -               clocks = <&gfx_fclk_clksel_ck>;
> -               reg = <0x052c>;
> -               ti,max-div = <2>;
> +               gfx_fck_div_ck: clock-gfx-fck-div {
> +                       #clock-cells = <0>;
> +                       compatible = "ti,divider-clock";
> +                       clock-output-names = "gfx_fck_div_ck";
> +                       clocks = <&gfx_fclk_clksel_ck>;
> +                       ti,max-div = <2>;
> +               };
>         };
>
>         sysclkout_pre_ck: sysclkout_pre_ck@700 {

Re: [PATCH] dt-bindings: clock: Add binding for TI clksel

[Tony Lindgren]

* Rob Herring <robh@kernel.org> [220104 22:06]:
> On Wed, Dec 22, 2021 at 12:24 AM Tony Lindgren <tony@atomide.com> wrote:
> >
> > Hi,
> >
> > * Rob Herring <robh@kernel.org> [211221 18:13]:
> > > On Fri, Dec 17, 2021 at 01:36:40PM +0200, Tony Lindgren wrote:
> > > > +additionalProperties: true
> > >
> > > Like what properties?
> > >
> > > true is only used for common, incomplete schemas referenced by device
> > > schemas.
> >
> > There is a collection of the current component clock child nodes for each
> > clksel instance. I got warnings with "additionalProperties: false", but
> > maybe the child clock nodes need to be somehow specified in the binding?
> 
> If everything else is a child node, then you can do:
> 
> additionalProperties:
>   type: object

OK thanks will do, the additional properties are always child nodes.

Regards,

Tony