diff mbox series

[RFC,v2,02/11] dt-bindings: net: dwmac-meson: Document the "timing-adjustment" clock

Message ID 20200429201644.1144546-3-martin.blumenstingl@googlemail.com (mailing list archive)
State Superseded
Headers show
Series dwmac-meson8b Ethernet RX delay configuration | expand

Commit Message

Martin Blumenstingl April 29, 2020, 8:16 p.m. UTC
The PRG_ETHERNET registers can add an RX delay in RGMII mode. This
requires an internal re-timing circuit whose input clock is called
"timing adjustment clock". Document this clock input so the clock can be
enabled as needed.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../devicetree/bindings/net/amlogic,meson-dwmac.yaml   | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Andrew Lunn May 1, 2020, 3:27 p.m. UTC | #1
On Wed, Apr 29, 2020 at 10:16:35PM +0200, Martin Blumenstingl wrote:
> The PRG_ETHERNET registers can add an RX delay in RGMII mode. This
> requires an internal re-timing circuit whose input clock is called
> "timing adjustment clock". Document this clock input so the clock can be
> enabled as needed.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Rob Herring (Arm) May 1, 2020, 9:09 p.m. UTC | #2
On Wed, 29 Apr 2020 22:16:35 +0200, Martin Blumenstingl wrote:
> The PRG_ETHERNET registers can add an RX delay in RGMII mode. This
> requires an internal re-timing circuit whose input clock is called
> "timing adjustment clock". Document this clock input so the clock can be
> enabled as needed.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../devicetree/bindings/net/amlogic,meson-dwmac.yaml   | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: Additional items are not allowed ([4294967295] was unexpected)
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: [[4294967295], [4294967295], [4294967295], [4294967295]] is too long
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: Additional items are not allowed ([4294967295] was unexpected)
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: [[4294967295], [4294967295], [4294967295], [4294967295]] is too long

See https://patchwork.ozlabs.org/patch/1279646

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.
Martin Blumenstingl May 1, 2020, 9:53 p.m. UTC | #3
Hi Rob,

On Fri, May 1, 2020 at 11:09 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, 29 Apr 2020 22:16:35 +0200, Martin Blumenstingl wrote:
> > The PRG_ETHERNET registers can add an RX delay in RGMII mode. This
> > requires an internal re-timing circuit whose input clock is called
> > "timing adjustment clock". Document this clock input so the clock can be
> > enabled as needed.
> >
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > ---
> >  .../devicetree/bindings/net/amlogic,meson-dwmac.yaml   | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: Additional items are not allowed ([4294967295] was unexpected)
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: [[4294967295], [4294967295], [4294967295], [4294967295]] is too long
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: Additional items are not allowed ([4294967295] was unexpected)
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: [[4294967295], [4294967295], [4294967295], [4294967295]] is too long
I am seeing this on my own build machine as well, but only for the .yaml example
The .dts example does not emit this warning

Also I don't see what's wrong with my way of describing the new,
optional clock and it's clock-name
Can you please point me in the right direction here?


Thank you!
Martin
Martin Blumenstingl May 10, 2020, 10:34 p.m. UTC | #4
Hello Rob,

On Fri, May 1, 2020 at 11:53 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Rob,
>
> On Fri, May 1, 2020 at 11:09 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, 29 Apr 2020 22:16:35 +0200, Martin Blumenstingl wrote:
> > > The PRG_ETHERNET registers can add an RX delay in RGMII mode. This
> > > requires an internal re-timing circuit whose input clock is called
> > > "timing adjustment clock". Document this clock input so the clock can be
> > > enabled as needed.
> > >
> > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > > ---
> > >  .../devicetree/bindings/net/amlogic,meson-dwmac.yaml   | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: Additional items are not allowed ([4294967295] was unexpected)
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: [[4294967295], [4294967295], [4294967295], [4294967295]] is too long
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: Additional items are not allowed ([4294967295] was unexpected)
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: ethernet@c9410000: clocks: [[4294967295], [4294967295], [4294967295], [4294967295]] is too long
> I am seeing this on my own build machine as well, but only for the .yaml example
> The .dts example does not emit this warning
I found out what's going on here:
- I built these patches against the net-next tree (including dt_binding_check)
- and against linux-next (also including dt_binding_check)

Your tree contains commit f22531438ff42c ("dt-bindings: net: dwmac:
increase 'maxItems' for 'clocks', 'clock-names' properties") [0].
The net-next tree doesn't have that commit but linux-next does.
So when I run dt_binding_check with this series applied on top of
linux-next all warnings/errors are gone.
However when I run dt_binding_check with this series applied on top of
net-next I get the same errors as you.
The reason is that the additional patch in your tree increases the
maximum number of clocks from three to five. With this patch the
Amlogic DWMAC glue needs (up to) four clock inputs.

I have to re-send this series anyways due to a bug in another patch.
Please let me know how to make your bot happy when when I re-send the patches.


Thank you!
Martin


[0] https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/commit/?h=dt/next&id=f22531438ff42ce568f81e346428461c71dea9e2
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
index 8d851f59d9f2..2bc0e8b0d25b 100644
--- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
@@ -40,18 +40,22 @@  allOf:
     then:
       properties:
         clocks:
+          minItems: 3
+          maxItems: 4
           items:
             - description: GMAC main clock
             - description: First parent clock of the internal mux
             - description: Second parent clock of the internal mux
+            - description: The clock which drives the timing adjustment logic
 
         clock-names:
           minItems: 3
-          maxItems: 3
+          maxItems: 4
           items:
             - const: stmmaceth
             - const: clkin0
             - const: clkin1
+            - const: timing-adjustment
 
         amlogic,tx-delay-ns:
           $ref: /schemas/types.yaml#definitions/uint32
@@ -120,7 +124,7 @@  examples:
          reg = <0xc9410000 0x10000>, <0xc8834540 0x8>;
          interrupts = <8>;
          interrupt-names = "macirq";
-         clocks = <&clk_eth>, <&clkc_fclk_div2>, <&clk_mpll2>;
-         clock-names = "stmmaceth", "clkin0", "clkin1";
+         clocks = <&clk_eth>, <&clk_fclk_div2>, <&clk_mpll2>, <&clk_fclk_div2>;
+         clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment";
          phy-mode = "rgmii";
     };