diff mbox series

[1/3] dt-bindings: clock: add TWL6032 32K clocks

Message ID 20230819134147.456060-2-andreas@kemnade.info (mailing list archive)
State Changes Requested, archived
Headers show
Series ARM: omap: omap4-embt2ws: 32K clock for WLAN | expand

Commit Message

Andreas Kemnade Aug. 19, 2023, 1:41 p.m. UTC
To be able to be referenced from a future yaml-version of
mfd/twl-family.txt depending on toplevel compatible have a separate file
for the 6032

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 .../bindings/clock/ti,twl6032-clk.yaml        | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml

Comments

Rob Herring Aug. 21, 2023, 8:57 p.m. UTC | #1
On Sat, Aug 19, 2023 at 03:41:45PM +0200, Andreas Kemnade wrote:
> To be able to be referenced from a future yaml-version of
> mfd/twl-family.txt depending on toplevel compatible have a separate file
> for the 6032

Really, the parent needs to be done first...

> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  .../bindings/clock/ti,twl6032-clk.yaml        | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml b/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
> new file mode 100644
> index 0000000000000..aebd9f8d761a2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/ti,twl6032-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Clocks of the TWL6032 PMIC
> +
> +maintainers:
> +  - Andreas Kemnade <andreas@kemnade.info>
> +
> +description:
> +  The TWL6032 has some 32Khz clock outputs which can be controlled.

outputs? Seems like only 1 with no clock cells to specify which one.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,twl6032-clk32kaudio
> +      - ti,twl6032-clk32kg

Or is it 1 output per compatible? I hope not.

> +
> +  '#clock-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +  - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    twl {
> +        clk32kaudio {
> +            compatible = "ti,twl6032-clk32kaudio";
> +            #clock-cells = <0>;
> +        };

You don't need a child node to be a clock provider. Just add 
#clock-cells to the parent node.

Rob
Andreas Kemnade Aug. 23, 2023, 3:38 p.m. UTC | #2
On Mon, 21 Aug 2023 15:57:45 -0500
Rob Herring <robh@kernel.org> wrote:

> On Sat, Aug 19, 2023 at 03:41:45PM +0200, Andreas Kemnade wrote:
> > To be able to be referenced from a future yaml-version of
> > mfd/twl-family.txt depending on toplevel compatible have a separate
> > file for the 6032  
> 
> Really, the parent needs to be done first...
> 
well, for some other subdevices, a yaml is already in the tree
and Krzysztof recently added a R-By to another one.

But if the clocks should not have a node, then it is obvious.
What would be the route to conversion here: Is a conversion
of mfd/twl-family.txt without specifying subnodes ok for the first step,
maybe with additionalProperties: yes?


> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >  .../bindings/clock/ti,twl6032-clk.yaml        | 38
> > +++++++++++++++++++ 1 file changed, 38 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
> > b/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml new
> > file mode 100644 index 0000000000000..aebd9f8d761a2 --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/ti,twl6032-clk.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Clocks of the TWL6032 PMIC
> > +
> > +maintainers:
> > +  - Andreas Kemnade <andreas@kemnade.info>
> > +
> > +description:
> > +  The TWL6032 has some 32Khz clock outputs which can be
> > controlled.  
> 
> outputs? Seems like only 1 with no clock cells to specify which one.
> 
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ti,twl6032-clk32kaudio
> > +      - ti,twl6032-clk32kg  
> 
> Or is it 1 output per compatible? I hope not.
> 
yes, it is. It was inspired by the clk-palmas driver:
$ grep palmas.*32 arch/arm/boot/dts/ti/omap/omap5-*
arch/arm/boot/dts/ti/omap/omap5-board-common.dtsi:
clk32kgaudio: palmas_clk32k@1 {
arch/arm/boot/dts/ti/omap/omap5-board-common.dtsi:
	compatible = "ti,palmas-clk32kgaudio";

Well, we have the CLK_IGNORE_UNUSED, so if we use #clock-cells = 1,
an unused clock will not be touched by the kernel, right?

> > +
> > +  '#clock-cells':
> > +    const: 0
> > +
> > +required:
> > +  - compatible
> > +  - '#clock-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    twl {
> > +        clk32kaudio {
> > +            compatible = "ti,twl6032-clk32kaudio";
> > +            #clock-cells = <0>;
> > +        };  
> 
> You don't need a child node to be a clock provider. Just add 
> #clock-cells to the parent node.
> 
hmm, we have child nodes there for every subdevice in that family,
even if I doubt it is totally technically required.
So why should the clk device be an exception? 

Regards,
Andreas
Krzysztof Kozlowski Aug. 24, 2023, 7:09 a.m. UTC | #3
On 23/08/2023 17:38, Andreas Kemnade wrote:
> On Mon, 21 Aug 2023 15:57:45 -0500
> Rob Herring <robh@kernel.org> wrote:
> 
>> On Sat, Aug 19, 2023 at 03:41:45PM +0200, Andreas Kemnade wrote:
>>> To be able to be referenced from a future yaml-version of
>>> mfd/twl-family.txt depending on toplevel compatible have a separate
>>> file for the 6032  
>>
>> Really, the parent needs to be done first...
>>
> well, for some other subdevices, a yaml is already in the tree
> and Krzysztof recently added a R-By to another one.

Yep, but I am not checking every possible parent-child relationship. It
would not be even possible...

> 
> But if the clocks should not have a node, then it is obvious.
> What would be the route to conversion here: Is a conversion
> of mfd/twl-family.txt without specifying subnodes ok for the first step,
> maybe with additionalProperties: yes?

Yes.

> 
> 
>>> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
>>> ---
>>>  .../bindings/clock/ti,twl6032-clk.yaml        | 38
>>> +++++++++++++++++++ 1 file changed, 38 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
>>> b/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml new
>>> file mode 100644 index 0000000000000..aebd9f8d761a2 --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
>>> @@ -0,0 +1,38 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/ti,twl6032-clk.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Clocks of the TWL6032 PMIC
>>> +
>>> +maintainers:
>>> +  - Andreas Kemnade <andreas@kemnade.info>
>>> +
>>> +description:
>>> +  The TWL6032 has some 32Khz clock outputs which can be
>>> controlled.  
>>
>> outputs? Seems like only 1 with no clock cells to specify which one.
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,twl6032-clk32kaudio
>>> +      - ti,twl6032-clk32kg  
>>
>> Or is it 1 output per compatible? I hope not.
>>
> yes, it is. It was inspired by the clk-palmas driver:

Creating nodes for single clocks is rather antipattern. Also, many early
designs of drivers and bindings assumed mapping 1-to-1 between driver
and DT nodes. This is also considered an antipattern now.

> $ grep palmas.*32 arch/arm/boot/dts/ti/omap/omap5-*
> arch/arm/boot/dts/ti/omap/omap5-board-common.dtsi:
> clk32kgaudio: palmas_clk32k@1 {
> arch/arm/boot/dts/ti/omap/omap5-board-common.dtsi:
> 	compatible = "ti,palmas-clk32kgaudio";
> 
> Well, we have the CLK_IGNORE_UNUSED, so if we use #clock-cells = 1,
> an unused clock will not be touched by the kernel, right?

I don't understand what OS flag has anything to do with clock-cells...

> 
>>> +
>>> +  '#clock-cells':
>>> +    const: 0
>>> +
>>> +required:
>>> +  - compatible
>>> +  - '#clock-cells'
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    twl {
>>> +        clk32kaudio {
>>> +            compatible = "ti,twl6032-clk32kaudio";
>>> +            #clock-cells = <0>;
>>> +        };  
>>
>> You don't need a child node to be a clock provider. Just add 
>> #clock-cells to the parent node.
>>
> hmm, we have child nodes there for every subdevice in that family,
> even if I doubt it is totally technically required.
> So why should the clk device be an exception? 

There is no rule of having nodes for subdevices, thus there cannot be
such exception. The rule is nodes are created when needed, not to match
some consistency of Linux drivers.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml b/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
new file mode 100644
index 0000000000000..aebd9f8d761a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti,twl6032-clk.yaml
@@ -0,0 +1,38 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/ti,twl6032-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clocks of the TWL6032 PMIC
+
+maintainers:
+  - Andreas Kemnade <andreas@kemnade.info>
+
+description:
+  The TWL6032 has some 32Khz clock outputs which can be controlled.
+
+properties:
+  compatible:
+    enum:
+      - ti,twl6032-clk32kaudio
+      - ti,twl6032-clk32kg
+
+  '#clock-cells':
+    const: 0
+
+required:
+  - compatible
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    twl {
+        clk32kaudio {
+            compatible = "ti,twl6032-clk32kaudio";
+            #clock-cells = <0>;
+        };
+    };
+...