Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Deprecate the old
"amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps"
property.
For older SoCs the only known supported values were 0ns and 2ns. The new
SoCs have 200ps precision and support RGMII RX delays between 0ps and
3000ps.
Signed-off-by: Martin Blumenstingl <[email protected]>
---
.../bindings/net/amlogic,meson-dwmac.yaml | 52 ++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
index 6b057b117aa0..bafde1194193 100644
--- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
@@ -74,18 +74,68 @@ allOf:
Any configuration is ignored when the phy-mode is set to "rmii".
amlogic,rx-delay-ns:
+ deprecated: true
enum:
- 0
- 2
default: 0
+ description:
+ The internal RGMII RX clock delay in nanoseconds. Deprecated, use
+ amlogic,rgmii-rx-delay-ps instead.
+
+ amlogic,rgmii-rx-delay-ps:
+ 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
+ picoseconds. 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".
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - amlogic,meson8b-dwmac
+ - amlogic,meson8m2-dwmac
+ - amlogic,meson-gxbb-dwmac
+ - amlogic,meson-axg-dwmac
+ then:
+ properties:
+ amlogic,rgmii-rx-delay-ps:
+ enum:
+ - 0
+ - 2000
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - amlogic,meson-g12a-dwmac
+ then:
+ properties:
+ amlogic,rgmii-rx-delay-ps:
+ enum:
+ - 0
+ - 200
+ - 400
+ - 600
+ - 800
+ - 1000
+ - 1200
+ - 1400
+ - 1600
+ - 1800
+ - 2000
+ - 2200
+ - 2400
+ - 2600
+ - 2800
+ - 3000
+
properties:
compatible:
additionalItems: true
--
2.29.2
On Sat, Nov 14, 2020 at 09:01:01PM +0100, Martin Blumenstingl wrote:
> Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
> delay register which allows picoseconds precision. Deprecate the old
> "amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps"
> property.
>
> For older SoCs the only known supported values were 0ns and 2ns. The new
> SoCs have 200ps precision and support RGMII RX delays between 0ps and
> 3000ps.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> .../bindings/net/amlogic,meson-dwmac.yaml | 52 ++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> index 6b057b117aa0..bafde1194193 100644
> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> @@ -74,18 +74,68 @@ allOf:
> Any configuration is ignored when the phy-mode is set to "rmii".
>
> amlogic,rx-delay-ns:
> + deprecated: true
> enum:
> - 0
> - 2
> default: 0
> + description:
> + The internal RGMII RX clock delay in nanoseconds. Deprecated, use
> + amlogic,rgmii-rx-delay-ps instead.
> +
> + amlogic,rgmii-rx-delay-ps:
> + 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
> + picoseconds. 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".
Hi Martin
I don't think the wording matches what the driver is actually doing:
if (dwmac->rx_delay_ns == 2)
rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
else
rx_dly_config = 0;
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
delay_config = tx_dly_config | rx_dly_config;
break;
case PHY_INTERFACE_MODE_RGMII_RXID:
delay_config = tx_dly_config;
break;
case PHY_INTERFACE_MODE_RGMII_TXID:
delay_config = rx_dly_config;
break;
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RMII:
delay_config = 0;
break;
So rx_delay is used for both rgmii and rgmii-txid. The binding says
nothing about rgmii-txid.
And while i'm looking at the code, i wonder about this:
if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
if (!dwmac->timing_adj_clk) {
dev_err(dwmac->dev,
"The timing-adjustment clock is mandatory for the RX delay re-timing\n");
return -EINVAL;
}
/* The timing adjustment logic is driven by a separate clock */
ret = meson8b_devm_clk_prepare_enable(dwmac,
dwmac->timing_adj_clk);
if (ret) {
dev_err(dwmac->dev,
"Failed to enable the timing-adjustment clock\n");
return ret;
}
}
It looks like it can be that rx_dly_config & PRG_ETH0_ADJ_ENABLE is
true, so the clock is enabled, but delay_config does not contain
rx_delay_config, so it is pointless turning it on.
Andrew
Hi Andrew,
On Sat, Nov 14, 2020 at 11:32 PM Andrew Lunn <[email protected]> wrote:
[...]
> > + amlogic,rgmii-rx-delay-ps:
> > + 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
> > + picoseconds. 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".
>
> Hi Martin
>
> I don't think the wording matches what the driver is actually doing:
>
> if (dwmac->rx_delay_ns == 2)
> rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
> else
> rx_dly_config = 0;
>
> switch (dwmac->phy_mode) {
> case PHY_INTERFACE_MODE_RGMII:
> delay_config = tx_dly_config | rx_dly_config;
> break;
> case PHY_INTERFACE_MODE_RGMII_RXID:
> delay_config = tx_dly_config;
> break;
> case PHY_INTERFACE_MODE_RGMII_TXID:
> delay_config = rx_dly_config;
> break;
> case PHY_INTERFACE_MODE_RGMII_ID:
> case PHY_INTERFACE_MODE_RMII:
> delay_config = 0;
> break;
>
> So rx_delay is used for both rgmii and rgmii-txid. The binding says
> nothing about rgmii-txid.
interesting point here. it's been like this before this patch. still I
would like to understand what the proper way to fix it is so I can
also include a fix for it:
1. should rgmii-txid not add any RX delay on the MAC side? that would
mean for my board I will switch to phy-mode rgmii so the MAC applies
both the RX and TX delays
2. update the documentation to clarify that rgmii-txid would also add
the RX delay on the MAC side
> And while i'm looking at the code, i wonder about this:
>
> if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
> if (!dwmac->timing_adj_clk) {
> dev_err(dwmac->dev,
> "The timing-adjustment clock is mandatory for the RX delay re-timing\n");
> return -EINVAL;
> }
>
> /* The timing adjustment logic is driven by a separate clock */
> ret = meson8b_devm_clk_prepare_enable(dwmac,
> dwmac->timing_adj_clk);
> if (ret) {
> dev_err(dwmac->dev,
> "Failed to enable the timing-adjustment clock\n");
> return ret;
> }
> }
>
> It looks like it can be that rx_dly_config & PRG_ETH0_ADJ_ENABLE is
> true, so the clock is enabled, but delay_config does not contain
> rx_delay_config, so it is pointless turning it on.
that is a good point and also a bug with one of the previous patches
I'll include a patch fixing this in v2
Best regards,
Martin
On Sun, Nov 15, 2020 at 10:22:06AM +0100, Martin Blumenstingl wrote:
> Hi Andrew,
>
> On Sat, Nov 14, 2020 at 11:32 PM Andrew Lunn <[email protected]> wrote:
> [...]
> > > + amlogic,rgmii-rx-delay-ps:
> > > + 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
> > > + picoseconds. 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".
> >
> > Hi Martin
> >
> > I don't think the wording matches what the driver is actually doing:
> >
> > if (dwmac->rx_delay_ns == 2)
> > rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
> > else
> > rx_dly_config = 0;
> >
> > switch (dwmac->phy_mode) {
> > case PHY_INTERFACE_MODE_RGMII:
> > delay_config = tx_dly_config | rx_dly_config;
> > break;
> > case PHY_INTERFACE_MODE_RGMII_RXID:
> > delay_config = tx_dly_config;
> > break;
> > case PHY_INTERFACE_MODE_RGMII_TXID:
> > delay_config = rx_dly_config;
> > break;
> > case PHY_INTERFACE_MODE_RGMII_ID:
> > case PHY_INTERFACE_MODE_RMII:
> > delay_config = 0;
> > break;
> >
> > So rx_delay is used for both rgmii and rgmii-txid. The binding says
> > nothing about rgmii-txid.
> interesting point here. it's been like this before this patch. still I
> would like to understand what the proper way to fix it is so I can
> also include a fix for it:
> 1. should rgmii-txid not add any RX delay on the MAC side? that would
> mean for my board I will switch to phy-mode rgmii so the MAC applies
> both the RX and TX delays
> 2. update the documentation to clarify that rgmii-txid would also add
> the RX delay on the MAC side
Hi Martin
I would fix the documentation.
> that is a good point and also a bug with one of the previous patches
> I'll include a patch fixing this in v2
Thanks for looking at these.
Andrew