[v3,1/8] dt-bindings: net: meson-dwmac: Add the amlogic, rx-delay-ns property
diff mbox series

Message ID 20200512211103.530674-2-martin.blumenstingl@googlemail.com
State Accepted
Commit 7af4c8451d80d0a8622483c27ab141a7c1a94573
Headers show
Series
  • dwmac-meson8b Ethernet RX delay configuration
Related show

Commit Message

Martin Blumenstingl May 12, 2020, 9:10 p.m. UTC
The PRG_ETHERNET registers on Meson8b and newer SoCs can add an RX
delay. Add a property with the known supported values so it can be
configured according to the board layout.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../bindings/net/amlogic,meson-dwmac.yaml           | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Pavel Machek May 24, 2020, 9:28 p.m. UTC | #1
On Tue 2020-05-12 23:10:56, Martin Blumenstingl wrote:
> The PRG_ETHERNET registers on Meson8b and newer SoCs can add an RX
> delay. Add a property with the known supported values so it can be
> configured according to the board layout.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../bindings/net/amlogic,meson-dwmac.yaml           | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> index ae91aa9d8616..66074314e57a 100644
> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> @@ -67,6 +67,19 @@ allOf:
>              PHY and MAC are adding a delay).
>              Any configuration is ignored when the phy-mode is set to "rmii".
>  
> +        amlogic,rx-delay-ns:
> +          enum:

Is it likely other MACs will need rx-delay property, too? Should we get rid of the amlogic,
prefix?
										Pavel
Florian Fainelli May 24, 2020, 10:05 p.m. UTC | #2
On 5/24/2020 2:28 PM, Pavel Machek wrote:
> On Tue 2020-05-12 23:10:56, Martin Blumenstingl wrote:
>> The PRG_ETHERNET registers on Meson8b and newer SoCs can add an RX
>> delay. Add a property with the known supported values so it can be
>> configured according to the board layout.
>>
>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>>  .../bindings/net/amlogic,meson-dwmac.yaml           | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
>> index ae91aa9d8616..66074314e57a 100644
>> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
>> @@ -67,6 +67,19 @@ allOf:
>>              PHY and MAC are adding a delay).
>>              Any configuration is ignored when the phy-mode is set to "rmii".
>>  
>> +        amlogic,rx-delay-ns:
>> +          enum:
> 
> Is it likely other MACs will need rx-delay property, too? Should we get rid of the amlogic,
> prefix?

Yes, there are several MAC bindings that already define a delay property:

Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml:
     allwinner,rx-delay-ps:
Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml:
     allwinner,rx-delay-ps:
Documentation/devicetree/bindings/net/apm-xgene-enet.txt:- rx-delay:
Delay value for RGMII bridge RX clock.
Documentation/devicetree/bindings/net/apm-xgene-enet.txt:       rx-delay
= <2>;
Documentation/devicetree/bindings/net/cavium-pip.txt:- rx-delay: Delay
value for RGMII receive clock. Optional. Disabled if 0.
Documentation/devicetree/bindings/net/mediatek-dwmac.txt:-
mediatek,rx-delay-ps: RX clock delay macro value. Default is 0.
Documentation/devicetree/bindings/net/mediatek-dwmac.txt:
mediatek,rx-delay-ps = <1530>;

standardizing on rx-delay-ps and tx-delay-ps would make sense since that
is the lowest resolution and the property would be correctly named with
an unit in the name.
Pavel Machek May 25, 2020, 9:07 a.m. UTC | #3
Hi!

> > On Tue 2020-05-12 23:10:56, Martin Blumenstingl wrote:
> >> The PRG_ETHERNET registers on Meson8b and newer SoCs can add an RX
> >> delay. Add a property with the known supported values so it can be
> >> configured according to the board layout.
> >>
> >> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> >> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> >> ---
> >>  .../bindings/net/amlogic,meson-dwmac.yaml           | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> >> index ae91aa9d8616..66074314e57a 100644
> >> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> >> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> >> @@ -67,6 +67,19 @@ allOf:
> >>              PHY and MAC are adding a delay).
> >>              Any configuration is ignored when the phy-mode is set to "rmii".
> >>  
> >> +        amlogic,rx-delay-ns:
> >> +          enum:
> > 
> > Is it likely other MACs will need rx-delay property, too? Should we get rid of the amlogic,
> > prefix?
> 
> Yes, there are several MAC bindings that already define a delay property:
> 
> Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml:
>      allwinner,rx-delay-ps:
> Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml:
>      allwinner,rx-delay-ps:
> Documentation/devicetree/bindings/net/apm-xgene-enet.txt:- rx-delay:
> Delay value for RGMII bridge RX clock.
> Documentation/devicetree/bindings/net/apm-xgene-enet.txt:       rx-delay
> = <2>;
> Documentation/devicetree/bindings/net/cavium-pip.txt:- rx-delay: Delay
> value for RGMII receive clock. Optional. Disabled if 0.
> Documentation/devicetree/bindings/net/mediatek-dwmac.txt:-
> mediatek,rx-delay-ps: RX clock delay macro value. Default is 0.
> Documentation/devicetree/bindings/net/mediatek-dwmac.txt:
> mediatek,rx-delay-ps = <1530>;
> 
> standardizing on rx-delay-ps and tx-delay-ps would make sense since that
> is the lowest resolution and the property would be correctly named with
> an unit in the name.

Seems like similar patch is already being reviewed from Dan Murphy (?)
from TI.

Best regards,
								Pavel
Andrew Lunn May 25, 2020, 1:57 p.m. UTC | #4
> > standardizing on rx-delay-ps and tx-delay-ps would make sense since that
> > is the lowest resolution and the property would be correctly named with
> > an unit in the name.
> 
> Seems like similar patch is already being reviewed from Dan Murphy (?)
> from TI.

Dan is working on the PHY side. But there is probably code which can
be shared.

One question to consider, do we want the same properties names for MAC
and PHY, or do we want to make them different, to avoid confusion?

	   Andrew
Pavel Machek May 25, 2020, 8:17 p.m. UTC | #5
On Mon 2020-05-25 15:57:28, Andrew Lunn wrote:
> > > standardizing on rx-delay-ps and tx-delay-ps would make sense since that
> > > is the lowest resolution and the property would be correctly named with
> > > an unit in the name.
> > 
> > Seems like similar patch is already being reviewed from Dan Murphy (?)
> > from TI.
> 
> Dan is working on the PHY side. But there is probably code which can
> be shared.
> 
> One question to consider, do we want the same properties names for MAC
> and PHY, or do we want to make them different, to avoid confusion?

We have same properties accross different hardware (compatible, reg),
so same property between MAC and PHY seems to make sense.

									Pavel

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
index ae91aa9d8616..66074314e57a 100644
--- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
@@ -67,6 +67,19 @@  allOf:
             PHY and MAC are adding a delay).
             Any configuration is ignored when the phy-mode is set to "rmii".
 
+        amlogic,rx-delay-ns:
+          enum:
+            - 0
+            - 2
+          default: 0
+          description:
+            The internal RGMII RX clock delay (provided by this IP block) in
+            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
+            should be explicitly configured. When the phy-mode is set to
+            either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
+            provided by the PHY. Any configuration is ignored when the
+            phy-mode is set to "rmii".
+
 properties:
   compatible:
     additionalItems: true