| Message ID | 20200512211103.530674-2-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 7af4c8451d80d0a8622483c27ab141a7c1a94573 |
| Headers | show |
| Series | dwmac-meson8b Ethernet RX delay configuration | expand |
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
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.
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
> > 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
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
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