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

Hi!

quoted

On Tue 2020-05-12 23:10:56, Martin Blumenstingl wrote:

quoted

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html