Hello,
with the help of Jianxin Pan (many thanks!) the meaning of the "new"
PRG_ETH1[19:16] register bits on Amlogic Meson G12A, G12B and SM1 SoCs
are finally known. These SoCs allow fine-tuning the RGMII RX delay in
200ps steps (contrary to what I have thought in the past [0] these are
not some "calibration" values).
The vendor u-boot has code to automatically detect the best RX/TX delay
settings. For now we keep it simple and add a device-tree property with
200ps precision to select the "right" RX delay for each board.
While here, deprecate the "amlogic,rx-delay-ns" property as it's not
used on any upstream .dts (yet). The driver is backwards compatible.
I have tested this on an X96 Air 4GB board (not upstream yet). Testing
with iperf3 gives 938 Mbits/sec in both directions (RX and TX). The
following network settings were used in the .dts (2ns TX delay
generated by the PHY, 800ps RX delay generated by the MAC as the PHY
only supports 0ns or 2ns RX delays):
&ext_mdio {
external_phy: ethernet-phy@0 {
/* Realtek RTL8211F (0x001cc916) */
reg = <0>;
eee-broken-1000t;
reset-assert-us = <10000>;
reset-deassert-us = <30000>;
reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <&gpio_intc>;
/* MAC_INTR on GPIOZ_14 */
interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
};
};
ðmac {
status = "okay";
pinctrl-0 = <ð_pins>, <ð_rgmii_pins>;
pinctrl-names = "default";
phy-mode = "rgmii-txid";
phy-handle = <&external_phy>;
amlogic,rgmii-rx-delay-ps = <800>;
};
To use the same settings from vendor u-boot (which in my case has broken
Ethernet) the following commands can be used:
mw.l 0xff634540 0x1621
mw.l 0xff634544 0x30000
phyreg w 0x0 0x1040
phyreg w 0x1f 0xd08
phyreg w 0x11 0x9
phyreg w 0x15 0x11
phyreg w 0x1f 0x0
phyreg w 0x0 0x9200
Also I have tested this on a X96 Max board without any .dts changes
to confirm that other boards with the same IP block still work fine
with these changes.
Changes since v1 at [1]:
- updated patch 1 by making it more clear when the RX delay is applied.
Thanks to Andrew for the suggestion!
- added a fix to enabling the timing-adjustment clock only when really
needed. Found by Andrew - thanks!
- added testing not about X96 Max
- v1 did not go to the netdev mailing list, v2 fixes this
[0] https://lore.kernel.org/netdev/CAFBinCATt4Hi9rigj52nMf3oygyFbnopZcsakGL=KyWnsjY3JA@mail.gmail.com/
[1] https://patchwork.kernel.org/project/linux-amlogic/list/?series=384279
Martin Blumenstingl (5):
dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
net: stmmac: dwmac-meson8b: fix enabling the timing-adjustment clock
net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A
.../bindings/net/amlogic,meson-dwmac.yaml | 61 +++++++++++-
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 92 +++++++++++++++----
2 files changed, 128 insertions(+), 25 deletions(-)
--
2.29.2
Newer SoCs starting with the Amlogic Meson G12A have more a precise
RGMII RX delay configuration register. This means more complexity in the
code. Extract the existing RGMII delay configuration code into a
separate function to make it easier to read/understand even when adding
more logic in the future.
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 03fce678b9f5..353fe0f53620 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -268,7 +268,7 @@ static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
return 0;
}
-static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
{
u32 tx_dly_config, rx_dly_config, delay_config;
int ret;
@@ -323,6 +323,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
delay_config);
+ return 0;
+}
+
+static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+{
+ int ret;
+
if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
/* only relevant for RMII mode -> disable in RGMII mode */
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
@@ -431,6 +438,10 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
goto err_remove_config_dt;
}
+ ret = meson8b_init_rgmii_delays(dwmac);
+ if (ret)
+ goto err_remove_config_dt;
+
ret = meson8b_init_rgmii_tx_clk(dwmac);
if (ret)
goto err_remove_config_dt;
--
2.29.2
Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX
delay logic. Instead of fine-tuning the delay in the nanoseconds range
it now allows tuning in 200 picosecond steps. This support comes with
new bits in the PRG_ETH1[19:16] register.
Add support for validating the RGMII RX delay as well as configuring the
register accordingly on these platforms.
Signed-off-by: Martin Blumenstingl <[email protected]>
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 61 +++++++++++++++----
1 file changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 353fe0f53620..2184b6c2c784 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -69,10 +69,21 @@
*/
#define PRG_ETH0_ADJ_SKEW GENMASK(24, 20)
+#define PRG_ETH1 0x4
+
+/* Defined for adding a delay to the input RX_CLK for better timing.
+ * Each step is 200ps. These bits are used with external RGMII PHYs
+ * because RGMII RX only has the small window. cfg_rxclk_dly can
+ * adjust the window between RX_CLK and RX_DATA and improve the stability
+ * of "rx data valid".
+ */
+#define PRG_ETH1_CFG_RXCLK_DLY GENMASK(19, 16)
+
struct meson8b_dwmac;
struct meson8b_dwmac_data {
int (*set_phy_mode)(struct meson8b_dwmac *dwmac);
+ bool has_prg_eth1_rgmii_rx_delay;
};
struct meson8b_dwmac {
@@ -270,30 +281,35 @@ static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
{
- u32 tx_dly_config, rx_dly_config, delay_config;
+ u32 tx_dly_config, rx_adj_config, cfg_rxclk_dly, delay_config;
int ret;
+ rx_adj_config = 0;
+ cfg_rxclk_dly = 0;
tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
dwmac->tx_delay_ns >> 1);
- if (dwmac->rx_delay_ps == 2000)
- rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
- else
- rx_dly_config = 0;
+ if (dwmac->data->has_prg_eth1_rgmii_rx_delay)
+ cfg_rxclk_dly = FIELD_PREP(PRG_ETH1_CFG_RXCLK_DLY,
+ dwmac->rx_delay_ps / 200);
+ else if (dwmac->rx_delay_ps == 2000)
+ rx_adj_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
- delay_config = tx_dly_config | rx_dly_config;
+ delay_config = tx_dly_config | rx_adj_config;
break;
case PHY_INTERFACE_MODE_RGMII_RXID:
delay_config = tx_dly_config;
+ cfg_rxclk_dly = 0;
break;
case PHY_INTERFACE_MODE_RGMII_TXID:
- delay_config = rx_dly_config;
+ delay_config = rx_adj_config;
break;
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RMII:
delay_config = 0;
+ cfg_rxclk_dly = 0;
break;
default:
dev_err(dwmac->dev, "unsupported phy-mode %s\n",
@@ -323,6 +339,9 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
delay_config);
+ meson8b_dwmac_mask_bits(dwmac, PRG_ETH1, PRG_ETH1_CFG_RXCLK_DLY,
+ cfg_rxclk_dly);
+
return 0;
}
@@ -424,11 +443,20 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
dwmac->rx_delay_ps *= 1000;
}
- if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
- dev_err(&pdev->dev,
- "The only allowed RX delays values are: 0ps, 2000ps");
- ret = -EINVAL;
- goto err_remove_config_dt;
+ if (dwmac->data->has_prg_eth1_rgmii_rx_delay) {
+ if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
+ dev_err(dwmac->dev,
+ "The only allowed RGMII RX delays values are: 0ps, 2000ps");
+ ret = -EINVAL;
+ goto err_remove_config_dt;
+ }
+ } else {
+ if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) {
+ dev_err(dwmac->dev,
+ "The RGMII RX delay range is 0..3000ps in 200ps steps");
+ ret = -EINVAL;
+ goto err_remove_config_dt;
+ }
}
dwmac->timing_adj_clk = devm_clk_get_optional(dwmac->dev,
@@ -470,10 +498,17 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
static const struct meson8b_dwmac_data meson8b_dwmac_data = {
.set_phy_mode = meson8b_set_phy_mode,
+ .has_prg_eth1_rgmii_rx_delay = false,
};
static const struct meson8b_dwmac_data meson_axg_dwmac_data = {
.set_phy_mode = meson_axg_set_phy_mode,
+ .has_prg_eth1_rgmii_rx_delay = false,
+};
+
+static const struct meson8b_dwmac_data meson_g12a_dwmac_data = {
+ .set_phy_mode = meson_axg_set_phy_mode,
+ .has_prg_eth1_rgmii_rx_delay = true,
};
static const struct of_device_id meson8b_dwmac_match[] = {
@@ -495,7 +530,7 @@ static const struct of_device_id meson8b_dwmac_match[] = {
},
{
.compatible = "amlogic,meson-g12a-dwmac",
- .data = &meson_axg_dwmac_data,
+ .data = &meson_g12a_dwmac_data,
},
{ }
};
--
2.29.2
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.
While here, also update the description of the RX delay to indicate
that:
- with "rgmii" or "rgmii-id" the RX delay should be specified
- with "rgmii-id" or "rgmii-rxid" the RX delay is added by the PHY so
any configuration on the MAC side is ignored
- with "rmii" the RX delay is not applicable and any configuration is
ignored
Signed-off-by: Martin Blumenstingl <[email protected]>
---
.../bindings/net/amlogic,meson-dwmac.yaml | 61 +++++++++++++++++--
1 file changed, 56 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
index 6b057b117aa0..62a1e92a645c 100644
--- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
@@ -74,17 +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
- 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".
+ picoseconds. When phy-mode is set to "rgmii" or "rgmii-id" 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 so any configuration here is
+ ignored. Also 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:
--
2.29.2
Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Parse the new
"amlogic,rgmii-rx-delay-ps" property or fall back to the old
"amlogic,rx-delay-ns".
Signed-off-by: Martin Blumenstingl <[email protected]>
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 22 ++++++++++++-------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index e27e2e7a53fd..03fce678b9f5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -83,7 +83,7 @@ struct meson8b_dwmac {
phy_interface_t phy_mode;
struct clk *rgmii_tx_clk;
u32 tx_delay_ns;
- u32 rx_delay_ns;
+ u32 rx_delay_ps;
struct clk *timing_adj_clk;
};
@@ -276,7 +276,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
dwmac->tx_delay_ns >> 1);
- if (dwmac->rx_delay_ns == 2)
+ if (dwmac->rx_delay_ps == 2000)
rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
else
rx_dly_config = 0;
@@ -406,14 +406,20 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
&dwmac->tx_delay_ns))
dwmac->tx_delay_ns = 2;
- /* use 0ns as fallback since this is what most boards actually use */
- if (of_property_read_u32(pdev->dev.of_node, "amlogic,rx-delay-ns",
- &dwmac->rx_delay_ns))
- dwmac->rx_delay_ns = 0;
+ /* RX delay defaults to 0ps since this is what many boards use */
+ if (of_property_read_u32(pdev->dev.of_node,
+ "amlogic,rgmii-rx-delay-ps",
+ &dwmac->rx_delay_ps)) {
+ if (!of_property_read_u32(pdev->dev.of_node,
+ "amlogic,rx-delay-ns",
+ &dwmac->rx_delay_ps))
+ /* convert ns to ps */
+ dwmac->rx_delay_ps *= 1000;
+ }
- if (dwmac->rx_delay_ns != 0 && dwmac->rx_delay_ns != 2) {
+ if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
dev_err(&pdev->dev,
- "The only allowed RX delays values are: 0ns, 2ns");
+ "The only allowed RX delays values are: 0ps, 2000ps");
ret = -EINVAL;
goto err_remove_config_dt;
}
--
2.29.2
The timing-adjustment clock only has to be enabled when a) there is a
2ns RX delay configured using device-tree and b) the phy-mode indicates
that the RX delay should be enabled.
Only enable the RX delay if both are true, instead of (by accident) also
enabling it when there's the 2ns RX delay configured but the phy-mode
incicates that the RX delay is not used.
Fixes: 9308c47640d515 ("net: stmmac: dwmac-meson8b: add support for the RX delay configuration")
Reported-by: Andrew Lunn <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index dc0b8b6d180d..e27e2e7a53fd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -301,7 +301,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
return -EINVAL;
}
- if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
+ if (delay_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");
--
2.29.2
On Sun, Nov 15, 2020 at 07:52:06PM +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.
>
> While here, also update the description of the RX delay to indicate
> that:
> - with "rgmii" or "rgmii-id" the RX delay should be specified
> - with "rgmii-id" or "rgmii-rxid" the RX delay is added by the PHY so
> any configuration on the MAC side is ignored
> - with "rmii" the RX delay is not applicable and any configuration is
> ignored
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Sun, Nov 15, 2020 at 07:52:07PM +0100, Martin Blumenstingl wrote:
> The timing-adjustment clock only has to be enabled when a) there is a
> 2ns RX delay configured using device-tree and b) the phy-mode indicates
> that the RX delay should be enabled.
>
> Only enable the RX delay if both are true, instead of (by accident) also
> enabling it when there's the 2ns RX delay configured but the phy-mode
> incicates that the RX delay is not used.
>
> Fixes: 9308c47640d515 ("net: stmmac: dwmac-meson8b: add support for the RX delay configuration")
> Reported-by: Andrew Lunn <[email protected]>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Sun, Nov 15, 2020 at 07:52:10PM +0100, Martin Blumenstingl wrote:
> Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX
> delay logic. Instead of fine-tuning the delay in the nanoseconds range
> it now allows tuning in 200 picosecond steps. This support comes with
> new bits in the PRG_ETH1[19:16] register.
>
> Add support for validating the RGMII RX delay as well as configuring the
> register accordingly on these platforms.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Sun, Nov 15, 2020 at 07:52:09PM +0100, Martin Blumenstingl wrote:
> Newer SoCs starting with the Amlogic Meson G12A have more a precise
> RGMII RX delay configuration register. This means more complexity in the
> code. Extract the existing RGMII delay configuration code into a
> separate function to make it easier to read/understand even when adding
> more logic in the future.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Sun, Nov 15, 2020 at 07:52:08PM +0100, Martin Blumenstingl wrote:
> Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
> delay register which allows picoseconds precision. Parse the new
> "amlogic,rgmii-rx-delay-ps" property or fall back to the old
> "amlogic,rx-delay-ns".
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On 11/15/20 10:52 AM, 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.
>
> While here, also update the description of the RX delay to indicate
> that:
> - with "rgmii" or "rgmii-id" the RX delay should be specified
> - with "rgmii-id" or "rgmii-rxid" the RX delay is added by the PHY so
> any configuration on the MAC side is ignored
> - with "rmii" the RX delay is not applicable and any configuration is
> ignored
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
On 11/15/20 10:52 AM, Martin Blumenstingl wrote:
> The timing-adjustment clock only has to be enabled when a) there is a
> 2ns RX delay configured using device-tree and b) the phy-mode indicates
> that the RX delay should be enabled.
>
> Only enable the RX delay if both are true, instead of (by accident) also
> enabling it when there's the 2ns RX delay configured but the phy-mode
> incicates that the RX delay is not used.
>
> Fixes: 9308c47640d515 ("net: stmmac: dwmac-meson8b: add support for the RX delay configuration")
> Reported-by: Andrew Lunn <[email protected]>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
On 11/15/20 10:52 AM, Martin Blumenstingl wrote:
> Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
> delay register which allows picoseconds precision. Parse the new
> "amlogic,rgmii-rx-delay-ps" property or fall back to the old
> "amlogic,rx-delay-ns".
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Maybe also issue a warning when the 'amlogic,rx-delay-ns' property is
found in addition to the 'amlogic,rgmii-rx-delay-ps'? Up to you how to
manage existing DTBs being deployed.
--
Florian
On 11/15/20 10:52 AM, Martin Blumenstingl wrote:
> Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX
> delay logic. Instead of fine-tuning the delay in the nanoseconds range
> it now allows tuning in 200 picosecond steps. This support comes with
> new bits in the PRG_ETH1[19:16] register.
>
> Add support for validating the RGMII RX delay as well as configuring the
> register accordingly on these platforms.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
On 11/15/20 10:52 AM, Martin Blumenstingl wrote:
> Newer SoCs starting with the Amlogic Meson G12A have more a precise
> RGMII RX delay configuration register. This means more complexity in the
> code. Extract the existing RGMII delay configuration code into a
> separate function to make it easier to read/understand even when adding
> more logic in the future.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
Hi Florian,
On Tue, Nov 17, 2020 at 7:36 PM Florian Fainelli <[email protected]> wrote:
>
> On 11/15/20 10:52 AM, Martin Blumenstingl wrote:
> > Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
> > delay register which allows picoseconds precision. Parse the new
> > "amlogic,rgmii-rx-delay-ps" property or fall back to the old
> > "amlogic,rx-delay-ns".
> >
> > Signed-off-by: Martin Blumenstingl <[email protected]>
>
> Reviewed-by: Florian Fainelli <[email protected]>
first of all: thanks for reviewing this (and the rest of the series)!
> Maybe also issue a warning when the 'amlogic,rx-delay-ns' property is
> found in addition to the 'amlogic,rgmii-rx-delay-ps'? Up to you how to
> manage existing DTBs being deployed.
none of the upstream DTBs uses amlogic,rx-delay-ns - and I am also not
aware of anything being in use "downstream".
I will add a sentence to the commit description when I re-send this
without RFC, something along those lines: "No upstream DTB uses the
old amlogic,rx-delay-ns (yet). Only include minimalistic logic to fall
back to the old property, without any special validation (for example:
old and new property are given at the same time)"
What do you think?
Best regards,
Martin
Hi Kevin,
On Sun, Nov 15, 2020 at 7:52 PM Martin Blumenstingl
<[email protected]> wrote:
[...]
> I have tested this on an X96 Air 4GB board (not upstream yet).
[...]
> Also I have tested this on a X96 Max board without any .dts changes
can you please add this series to your testing branch?
I am interested in feedback from Kernel CI for all the boards which
are there as well as any other testing bots
Thank you!
Best regards,
Martin
On Sun, Nov 15, 2020 at 07:52:06PM +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.
>
> While here, also update the description of the RX delay to indicate
> that:
> - with "rgmii" or "rgmii-id" the RX delay should be specified
> - with "rgmii-id" or "rgmii-rxid" the RX delay is added by the PHY so
> any configuration on the MAC side is ignored
> - with "rmii" the RX delay is not applicable and any configuration is
> ignored
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> .../bindings/net/amlogic,meson-dwmac.yaml | 61 +++++++++++++++++--
> 1 file changed, 56 insertions(+), 5 deletions(-)
Don't we have common properties for this now?
>
> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> index 6b057b117aa0..62a1e92a645c 100644
> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> @@ -74,17 +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
> - 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".
> + picoseconds. When phy-mode is set to "rgmii" or "rgmii-id" 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 so any configuration here is
> + ignored. Also 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:
> --
> 2.29.2
>
Hi Rob,
On Mon, Dec 7, 2020 at 8:17 PM Rob Herring <[email protected]> wrote:
>
> On Sun, Nov 15, 2020 at 07:52:06PM +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.
> >
> > While here, also update the description of the RX delay to indicate
> > that:
> > - with "rgmii" or "rgmii-id" the RX delay should be specified
> > - with "rgmii-id" or "rgmii-rxid" the RX delay is added by the PHY so
> > any configuration on the MAC side is ignored
> > - with "rmii" the RX delay is not applicable and any configuration is
> > ignored
> >
> > Signed-off-by: Martin Blumenstingl <[email protected]>
> > ---
> > .../bindings/net/amlogic,meson-dwmac.yaml | 61 +++++++++++++++++--
> > 1 file changed, 56 insertions(+), 5 deletions(-)
>
> Don't we have common properties for this now?
I did a quick:
$ grep -R rx-delay Documentation/devicetree/bindings/net/
I could find "rx-delay" without vendor prefix, but that's not using
any unit in the name (ns, ps, ...)
Please let me know if you aware of any "generic" property for the RX
delay in picosecond precision
Best regards,
Martin
On Sun, Dec 13, 2020 at 05:59:05PM +0100, Martin Blumenstingl wrote:
> Hi Rob,
>
> On Mon, Dec 7, 2020 at 8:17 PM Rob Herring <[email protected]> wrote:
> >
> > On Sun, Nov 15, 2020 at 07:52:06PM +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.
> > >
> > > While here, also update the description of the RX delay to indicate
> > > that:
> > > - with "rgmii" or "rgmii-id" the RX delay should be specified
> > > - with "rgmii-id" or "rgmii-rxid" the RX delay is added by the PHY so
> > > any configuration on the MAC side is ignored
> > > - with "rmii" the RX delay is not applicable and any configuration is
> > > ignored
> > >
> > > Signed-off-by: Martin Blumenstingl <[email protected]>
> > > ---
> > > .../bindings/net/amlogic,meson-dwmac.yaml | 61 +++++++++++++++++--
> > > 1 file changed, 56 insertions(+), 5 deletions(-)
> >
> > Don't we have common properties for this now?
> I did a quick:
> $ grep -R rx-delay Documentation/devicetree/bindings/net/
>
> I could find "rx-delay" without vendor prefix, but that's not using
> any unit in the name (ns, ps, ...)
> Please let me know if you aware of any "generic" property for the RX
> delay in picosecond precision
{rx,tx}-internal-delay-ps in ethernet-controller.yaml and
ethernet-phy.yaml.