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

[Andreas Kemnade]

On Mon, 21 Aug 2023 15:57:45 -0500
Rob Herring <robh@xxxxxxxxxx> 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@xxxxxxxxxxxx>
> > ---
> >  .../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@xxxxxxxxxxxx>
> > +
> > +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