| Message ID | 20230817090810.203900-2-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Fix Versa3 clock mapping | expand |
On Thu, Aug 17, 2023 at 10:08:08AM +0100, Biju Das wrote: > Document clock-output-names property and fix the "assigned-clock-rates" > for each clock output in the example based on Table 3. ("Output Source") > in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}). > > While at it, replace clocks phandle in the example from x1_x2->x1 as > X2 is a different 32768 kHz crystal. > > Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be> > Closes: https://lore.kernel.org/all/CAMuHMdUHD+bEco=WYTYWsTAyRt3dTQQt4Xpaejss0Y2ZpLCMNg@mail.gmail.com/ > Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock generator bindings") > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Acked-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > --- > v1->v2: > * Updated commit description to make it clear it fixes > "assigned-clock-rates" in the example based on 5P35023 datasheet. > --- > .../devicetree/bindings/clock/renesas,5p35023.yaml | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > index 839648e753d4..db8d01b291dd 100644 > --- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > +++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > @@ -49,6 +49,9 @@ properties: > $ref: /schemas/types.yaml#/definitions/uint8-array > maxItems: 37 > > + clock-output-names: > + maxItems: 6 > + > required: > - compatible > - reg > @@ -68,7 +71,7 @@ examples: > reg = <0x68>; > #clock-cells = <1>; > > - clocks = <&x1_x2>; > + clocks = <&x1>; > > renesas,settings = [ > 80 00 11 19 4c 02 23 7f 83 19 08 a9 5f 25 24 bf > @@ -76,11 +79,14 @@ examples: > 80 b0 45 c4 95 > ]; > > + clock-output-names = "ref", "se1", "se2", "se3", > + "diff1", "diff2"; > + > assigned-clocks = <&versa3 0>, <&versa3 1>, > <&versa3 2>, <&versa3 3>, > <&versa3 4>, <&versa3 5>; > - assigned-clock-rates = <12288000>, <25000000>, > - <12000000>, <11289600>, > - <11289600>, <24000000>; > + assigned-clock-rates = <24000000>, <11289600>, > + <11289600>, <12000000>, > + <25000000>, <12288000>; > }; > }; > -- > 2.25.1 >
Hi Biju, On Thu, Aug 17, 2023 at 11:08 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Document clock-output-names property and fix the "assigned-clock-rates" > for each clock output in the example based on Table 3. ("Output Source") > in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}). > > While at it, replace clocks phandle in the example from x1_x2->x1 as > X2 is a different 32768 kHz crystal. > > Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be> > Closes: https://lore.kernel.org/all/CAMuHMdUHD+bEco=WYTYWsTAyRt3dTQQt4Xpaejss0Y2ZpLCMNg@mail.gmail.com/ > Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock generator bindings") > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v1->v2: > * Updated commit description to make it clear it fixes > "assigned-clock-rates" in the example based on 5P35023 datasheet. Thanks for your patch! > --- > .../devicetree/bindings/clock/renesas,5p35023.yaml | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > index 839648e753d4..db8d01b291dd 100644 > --- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > +++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > @@ -49,6 +49,9 @@ properties: > $ref: /schemas/types.yaml#/definitions/uint8-array > maxItems: 37 > > + clock-output-names: > + maxItems: 6 > + Why do you need clock-output-names? The clock output names should be created by the driver (taking into account the instance number, so it works with multiple instances). Gr{oetje,eeting}s, Geert
Hi Geert Uytterhoeven, Thanks for the feedback. > Subject: Re: [PATCH v2 1/3] dt-bindings: clock: versaclock3: Document > clock-output-names > > Hi Biju, > > On Thu, Aug 17, 2023 at 11:08 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > Document clock-output-names property and fix the "assigned-clock-rates" > > for each clock output in the example based on Table 3. ("Output > > Source") in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}). > > > > While at it, replace clocks phandle in the example from x1_x2->x1 as > > X2 is a different 32768 kHz crystal. > > > > Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Closes: > > Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock > > generator bindings") > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v1->v2: > > * Updated commit description to make it clear it fixes > > "assigned-clock-rates" in the example based on 5P35023 datasheet. > > Thanks for your patch! > > --- > > .../devicetree/bindings/clock/renesas,5p35023.yaml | 14 > > ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > > b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > > index 839648e753d4..db8d01b291dd 100644 > > --- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > > +++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > > @@ -49,6 +49,9 @@ properties: > > $ref: /schemas/types.yaml#/definitions/uint8-array > > maxItems: 37 > > > > + clock-output-names: > > + maxItems: 6 > > + > > Why do you need clock-output-names? I thought it will be useful information for a user, by looking at the example the name of clock-output-name and corresponding assigned-clocks and assigned-clock-rates. See below, from this one can understand the relation between index and actual clock output. clock-output-names = "ref", "se1", "se2", "se3", "diff1", "diff2"; assigned-clocks = <&versa3 0>, <&versa3 1>, <&versa3 2>, <&versa3 3>, <&versa3 4>, <&versa3 5>; assigned-clock-rates = <24000000>, <11289600>, <11289600>, <12000000>, <25000000>, <12288000>; > The clock output names should be created by the driver (taking into account > the instance number, so it works with multiple instances). OK, so shall I remove it from bindings then? Cheers, Biju
Hi Biju, On Mon, Aug 21, 2023 at 10:11 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH v2 1/3] dt-bindings: clock: versaclock3: Document > > clock-output-names > > On Thu, Aug 17, 2023 at 11:08 AM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > Document clock-output-names property and fix the "assigned-clock-rates" > > > for each clock output in the example based on Table 3. ("Output > > > Source") in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}). > > > > > > While at it, replace clocks phandle in the example from x1_x2->x1 as > > > X2 is a different 32768 kHz crystal. > > > > > > Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > Closes: > > > Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock > > > generator bindings") > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > v1->v2: > > > * Updated commit description to make it clear it fixes > > > "assigned-clock-rates" in the example based on 5P35023 datasheet. > > > > Thanks for your patch! > > > --- > > > .../devicetree/bindings/clock/renesas,5p35023.yaml | 14 > > > ++++++++++---- > > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > > > b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > > > index 839648e753d4..db8d01b291dd 100644 > > > --- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > > > +++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml > > > @@ -49,6 +49,9 @@ properties: > > > $ref: /schemas/types.yaml#/definitions/uint8-array > > > maxItems: 37 > > > > > > + clock-output-names: > > > + maxItems: 6 > > > + > > > > Why do you need clock-output-names? > > I thought it will be useful information for a user, by looking at the example the name of clock-output-name and corresponding assigned-clocks and assigned-clock-rates. > > See below, from this one can understand the relation between index and actual clock output. > > clock-output-names = "ref", "se1", "se2", "se3", > "diff1", "diff2"; > > assigned-clocks = <&versa3 0>, <&versa3 1>, > <&versa3 2>, <&versa3 3>, > <&versa3 4>, <&versa3 5>; > assigned-clock-rates = <24000000>, <11289600>, > <11289600>, <12000000>, > <25000000>, <12288000>; > > > The clock output names should be created by the driver (taking into account > > the instance number, so it works with multiple instances). > > OK, so shall I remove it from bindings then? I think so, as it is not needed. What is still missing (contrary to the Closes tag) is the mapping from clock IDs to actual clock outputs. Gr{oetje,eeting}s, Geert
diff --git a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml index 839648e753d4..db8d01b291dd 100644 --- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml +++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml @@ -49,6 +49,9 @@ properties: $ref: /schemas/types.yaml#/definitions/uint8-array maxItems: 37 + clock-output-names: + maxItems: 6 + required: - compatible - reg @@ -68,7 +71,7 @@ examples: reg = <0x68>; #clock-cells = <1>; - clocks = <&x1_x2>; + clocks = <&x1>; renesas,settings = [ 80 00 11 19 4c 02 23 7f 83 19 08 a9 5f 25 24 bf @@ -76,11 +79,14 @@ examples: 80 b0 45 c4 95 ]; + clock-output-names = "ref", "se1", "se2", "se3", + "diff1", "diff2"; + assigned-clocks = <&versa3 0>, <&versa3 1>, <&versa3 2>, <&versa3 3>, <&versa3 4>, <&versa3 5>; - assigned-clock-rates = <12288000>, <25000000>, - <12000000>, <11289600>, - <11289600>, <24000000>; + assigned-clock-rates = <24000000>, <11289600>, + <11289600>, <12000000>, + <25000000>, <12288000>; }; };
Document clock-output-names property and fix the "assigned-clock-rates" for each clock output in the example based on Table 3. ("Output Source") in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}). While at it, replace clocks phandle in the example from x1_x2->x1 as X2 is a different 32768 kHz crystal. Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be> Closes: https://lore.kernel.org/all/CAMuHMdUHD+bEco=WYTYWsTAyRt3dTQQt4Xpaejss0Y2ZpLCMNg@mail.gmail.com/ Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock generator bindings") Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v1->v2: * Updated commit description to make it clear it fixes "assigned-clock-rates" in the example based on 5P35023 datasheet. --- .../devicetree/bindings/clock/renesas,5p35023.yaml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)