All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
@ 2017-12-23 23:40 ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-23 23:40 UTC (permalink / raw)
  To: netdev, ingrassia
  Cc: linus.luessing, khilman, linux-amlogic, jbrunet, narmstrong,
	peppe.cavallaro, alexandre.torgue, Martin Blumenstingl

Hi Dave,

please do not apply this series until it got a Tested-by from Emiliano.


Hi Emiliano,

you reported [0] that you couldn't get dwmac-meson8b to work on your
Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
I think I was able to find a fix: it consists of two patches (which you
find in this series)

Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
only partially test this (I could only check if the clocks were
calculated correctly when using a dummy 500002394Hz input clock instead
of MPLL2).

Could you please give this series a try and let me know about the
results?
You obviously still need your two "ARM: dts: meson8b" patches which
- add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
- enable Ethernet on the Odroid-C1

I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
fine (so let's hope that this also fixes your Meson8b issue :)).


changes since v1 at [1]:
- changed the subject of the cover-letter to indicate that this is all
  about the RGMII clock
- added PATCH #1 which ensures that we don't unnecessarily change the
  parent clocks in RMII mode (and also makes the code easier to
  understand)
- changed subject of PATCH #2 (formerly PATCH #1) to state that this
  is about the RGMII clock
- added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
- replaced PATCH #3 (formerly PATCH #2) with one that sets
  CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
  on Meson8b correctly


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html


Martin Blumenstingl (3):
  net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
  net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
  net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock

 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
 1 file changed, 27 insertions(+), 28 deletions(-)

-- 
2.15.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
@ 2017-12-23 23:40 ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-23 23:40 UTC (permalink / raw)
  To: linus-amlogic

Hi Dave,

please do not apply this series until it got a Tested-by from Emiliano.


Hi Emiliano,

you reported [0] that you couldn't get dwmac-meson8b to work on your
Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
I think I was able to find a fix: it consists of two patches (which you
find in this series)

Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
only partially test this (I could only check if the clocks were
calculated correctly when using a dummy 500002394Hz input clock instead
of MPLL2).

Could you please give this series a try and let me know about the
results?
You obviously still need your two "ARM: dts: meson8b" patches which
- add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
- enable Ethernet on the Odroid-C1

I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
fine (so let's hope that this also fixes your Meson8b issue :)).


changes since v1 at [1]:
- changed the subject of the cover-letter to indicate that this is all
  about the RGMII clock
- added PATCH #1 which ensures that we don't unnecessarily change the
  parent clocks in RMII mode (and also makes the code easier to
  understand)
- changed subject of PATCH #2 (formerly PATCH #1) to state that this
  is about the RGMII clock
- added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
- replaced PATCH #3 (formerly PATCH #2) with one that sets
  CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
  on Meson8b correctly


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html


Martin Blumenstingl (3):
  net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
  net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
  net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock

 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
 1 file changed, 27 insertions(+), 28 deletions(-)

-- 
2.15.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFT net-next v2 1/3] net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
  2017-12-23 23:40 ` Martin Blumenstingl
@ 2017-12-23 23:40   ` Martin Blumenstingl
  -1 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-23 23:40 UTC (permalink / raw)
  To: netdev, ingrassia
  Cc: linus.luessing, khilman, linux-amlogic, jbrunet, narmstrong,
	peppe.cavallaro, alexandre.torgue, Martin Blumenstingl

Neither the m25_div_clk nor the m250_div_clk or m250_mux_clk are used in
RMII mode. The m25_div_clk output is routed to the RGMII PHY's "RGMII
clock".
This means that we don't need to configure the clocks in RMII mode. The
driver however did this - with no effect since the clocks are not routed
to the PHY in RMII mode.

While here also rename meson8b_init_clk to meson8b_init_rgmii_clk to
make it easier to understand the code.

Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 46 ++++++++++------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 4404650b32c5..e1d5907e481c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -81,7 +81,7 @@ static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
 	writel(data, dwmac->regs + reg);
 }
 
-static int meson8b_init_clk(struct meson8b_dwmac *dwmac)
+static int meson8b_init_rgmii_clk(struct meson8b_dwmac *dwmac)
 {
 	struct clk_init_data init;
 	int i, ret;
@@ -176,7 +176,6 @@ static int meson8b_init_clk(struct meson8b_dwmac *dwmac)
 static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 {
 	int ret;
-	unsigned long clk_rate;
 	u8 tx_dly_val = 0;
 
 	switch (dwmac->phy_mode) {
@@ -191,9 +190,6 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		/* Generate a 25MHz clock for the PHY */
-		clk_rate = 25 * 1000 * 1000;
-
 		/* enable RGMII mode */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_RGMII_MODE,
 					PRG_ETH0_RGMII_MODE);
@@ -204,12 +200,24 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
 					tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
+
+		ret = clk_prepare_enable(dwmac->m25_div_clk);
+		if (ret) {
+			dev_err(&dwmac->pdev->dev, "failed to enable the PHY clock\n");
+			return ret;
+		}
+
+		/* Generate the 25MHz RGMII clock for the PHY */
+		ret = clk_set_rate(dwmac->m25_div_clk, 25 * 1000 * 1000);
+		if (ret) {
+			clk_disable_unprepare(dwmac->m25_div_clk);
+
+			dev_err(&dwmac->pdev->dev, "failed to set PHY clock\n");
+			return ret;
+		}
 		break;
 
 	case PHY_INTERFACE_MODE_RMII:
-		/* Use the rate of the mux clock for the internal RMII PHY */
-		clk_rate = clk_get_rate(dwmac->m250_mux_clk);
-
 		/* disable RGMII mode -> enables RMII mode */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_RGMII_MODE,
 					0);
@@ -231,20 +239,6 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 		return -EINVAL;
 	}
 
-	ret = clk_prepare_enable(dwmac->m25_div_clk);
-	if (ret) {
-		dev_err(&dwmac->pdev->dev, "failed to enable the PHY clock\n");
-		return ret;
-	}
-
-	ret = clk_set_rate(dwmac->m25_div_clk, clk_rate);
-	if (ret) {
-		clk_disable_unprepare(dwmac->m25_div_clk);
-
-		dev_err(&dwmac->pdev->dev, "failed to set PHY clock\n");
-		return ret;
-	}
-
 	/* enable TX_CLK and PHY_REF_CLK generator */
 	meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
 				PRG_ETH0_TX_AND_PHY_REF_CLK);
@@ -294,7 +288,7 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 				 &dwmac->tx_delay_ns))
 		dwmac->tx_delay_ns = 2;
 
-	ret = meson8b_init_clk(dwmac);
+	ret = meson8b_init_rgmii_clk(dwmac);
 	if (ret)
 		goto err_remove_config_dt;
 
@@ -311,7 +305,8 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 	return 0;
 
 err_clk_disable:
-	clk_disable_unprepare(dwmac->m25_div_clk);
+	if (phy_interface_mode_is_rgmii(dwmac->phy_mode))
+		clk_disable_unprepare(dwmac->m25_div_clk);
 err_remove_config_dt:
 	stmmac_remove_config_dt(pdev, plat_dat);
 
@@ -322,7 +317,8 @@ static int meson8b_dwmac_remove(struct platform_device *pdev)
 {
 	struct meson8b_dwmac *dwmac = get_stmmac_bsp_priv(&pdev->dev);
 
-	clk_disable_unprepare(dwmac->m25_div_clk);
+	if (phy_interface_mode_is_rgmii(dwmac->phy_mode))
+		clk_disable_unprepare(dwmac->m25_div_clk);
 
 	return stmmac_pltfr_remove(pdev);
 }
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFT net-next v2 1/3] net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
@ 2017-12-23 23:40   ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-23 23:40 UTC (permalink / raw)
  To: linus-amlogic

Neither the m25_div_clk nor the m250_div_clk or m250_mux_clk are used in
RMII mode. The m25_div_clk output is routed to the RGMII PHY's "RGMII
clock".
This means that we don't need to configure the clocks in RMII mode. The
driver however did this - with no effect since the clocks are not routed
to the PHY in RMII mode.

While here also rename meson8b_init_clk to meson8b_init_rgmii_clk to
make it easier to understand the code.

Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 46 ++++++++++------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 4404650b32c5..e1d5907e481c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -81,7 +81,7 @@ static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
 	writel(data, dwmac->regs + reg);
 }
 
-static int meson8b_init_clk(struct meson8b_dwmac *dwmac)
+static int meson8b_init_rgmii_clk(struct meson8b_dwmac *dwmac)
 {
 	struct clk_init_data init;
 	int i, ret;
@@ -176,7 +176,6 @@ static int meson8b_init_clk(struct meson8b_dwmac *dwmac)
 static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 {
 	int ret;
-	unsigned long clk_rate;
 	u8 tx_dly_val = 0;
 
 	switch (dwmac->phy_mode) {
@@ -191,9 +190,6 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		/* Generate a 25MHz clock for the PHY */
-		clk_rate = 25 * 1000 * 1000;
-
 		/* enable RGMII mode */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_RGMII_MODE,
 					PRG_ETH0_RGMII_MODE);
@@ -204,12 +200,24 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
 					tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
+
+		ret = clk_prepare_enable(dwmac->m25_div_clk);
+		if (ret) {
+			dev_err(&dwmac->pdev->dev, "failed to enable the PHY clock\n");
+			return ret;
+		}
+
+		/* Generate the 25MHz RGMII clock for the PHY */
+		ret = clk_set_rate(dwmac->m25_div_clk, 25 * 1000 * 1000);
+		if (ret) {
+			clk_disable_unprepare(dwmac->m25_div_clk);
+
+			dev_err(&dwmac->pdev->dev, "failed to set PHY clock\n");
+			return ret;
+		}
 		break;
 
 	case PHY_INTERFACE_MODE_RMII:
-		/* Use the rate of the mux clock for the internal RMII PHY */
-		clk_rate = clk_get_rate(dwmac->m250_mux_clk);
-
 		/* disable RGMII mode -> enables RMII mode */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_RGMII_MODE,
 					0);
@@ -231,20 +239,6 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 		return -EINVAL;
 	}
 
-	ret = clk_prepare_enable(dwmac->m25_div_clk);
-	if (ret) {
-		dev_err(&dwmac->pdev->dev, "failed to enable the PHY clock\n");
-		return ret;
-	}
-
-	ret = clk_set_rate(dwmac->m25_div_clk, clk_rate);
-	if (ret) {
-		clk_disable_unprepare(dwmac->m25_div_clk);
-
-		dev_err(&dwmac->pdev->dev, "failed to set PHY clock\n");
-		return ret;
-	}
-
 	/* enable TX_CLK and PHY_REF_CLK generator */
 	meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
 				PRG_ETH0_TX_AND_PHY_REF_CLK);
@@ -294,7 +288,7 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 				 &dwmac->tx_delay_ns))
 		dwmac->tx_delay_ns = 2;
 
-	ret = meson8b_init_clk(dwmac);
+	ret = meson8b_init_rgmii_clk(dwmac);
 	if (ret)
 		goto err_remove_config_dt;
 
@@ -311,7 +305,8 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 	return 0;
 
 err_clk_disable:
-	clk_disable_unprepare(dwmac->m25_div_clk);
+	if (phy_interface_mode_is_rgmii(dwmac->phy_mode))
+		clk_disable_unprepare(dwmac->m25_div_clk);
 err_remove_config_dt:
 	stmmac_remove_config_dt(pdev, plat_dat);
 
@@ -322,7 +317,8 @@ static int meson8b_dwmac_remove(struct platform_device *pdev)
 {
 	struct meson8b_dwmac *dwmac = get_stmmac_bsp_priv(&pdev->dev);
 
-	clk_disable_unprepare(dwmac->m25_div_clk);
+	if (phy_interface_mode_is_rgmii(dwmac->phy_mode))
+		clk_disable_unprepare(dwmac->m25_div_clk);
 
 	return stmmac_pltfr_remove(pdev);
 }
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFT net-next v2 2/3] net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
  2017-12-23 23:40 ` Martin Blumenstingl
@ 2017-12-23 23:40   ` Martin Blumenstingl
  -1 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-23 23:40 UTC (permalink / raw)
  To: netdev, ingrassia
  Cc: linus.luessing, khilman, linux-amlogic, jbrunet, narmstrong,
	peppe.cavallaro, alexandre.torgue, Martin Blumenstingl

Meson8b only supports MPLL2 as clock input. The rate of the MPLL2 clock
set by Odroid-C1's u-boot is close to 500MHz. The exact rate is
500002394Hz, which is calculated in drivers/clk/meson/clk-mpll.c
using the following formula:
DIV_ROUND_UP_ULL((u64)parent_rate * SDM_DEN, (SDM_DEN * n2) + sdm)
Odroid-C1's u-boot configures MPLL2 with the following values:
- SDM_DEN = 16384
- SDM = 1638
- N2 = 5

The 250MHz and 25MHz clocks inside dwmac-meson8b driver are derived
from the MPLL2 clock. Due to MPLL2 running slightly faster than 500MHz
the common clock framework chooses dividers which are too big to
generate the 250MHz and 25MHz clocks. Emiliano Ingrassia observed that
the divider for the 250MHz clock was set to 0x5 which results in a clock
rate of close to 100MHz instead of 250MHz. The divider for the 25MHz
clock is set to 0x0 (which means "divide by 5") so the resulting RGMII
clock is running at 20MHz (plus a few additional Hz). The RTL8211F PHY
on Odroid-C1 however fails to operate with a 20MHz RGMII clock.

Round the divider's clock rates to prevent this issue on Meson8b. This
means we'll now end up with a clock rate of 25000120Hz (= 25MHz plus
120Hz).
This has no effect on the Meson GX SoCs since there fclk_div2 is used as
input clock, which has a rate of 1000MHz (and thus is divisible cleanly
to 250MHz and 25MHz).

Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
Reported-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index e1d5907e481c..0da551c84fe8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -144,7 +144,9 @@ static int meson8b_init_rgmii_clk(struct meson8b_dwmac *dwmac)
 	dwmac->m250_div.shift = PRG_ETH0_CLK_M250_DIV_SHIFT;
 	dwmac->m250_div.width = PRG_ETH0_CLK_M250_DIV_WIDTH;
 	dwmac->m250_div.hw.init = &init;
-	dwmac->m250_div.flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO;
+	dwmac->m250_div.flags = CLK_DIVIDER_ONE_BASED |
+				CLK_DIVIDER_ALLOW_ZERO |
+				CLK_DIVIDER_ROUND_CLOSEST;
 
 	dwmac->m250_div_clk = devm_clk_register(dev, &dwmac->m250_div.hw);
 	if (WARN_ON(IS_ERR(dwmac->m250_div_clk)))
@@ -164,7 +166,8 @@ static int meson8b_init_rgmii_clk(struct meson8b_dwmac *dwmac)
 	dwmac->m25_div.width = PRG_ETH0_CLK_M25_DIV_WIDTH;
 	dwmac->m25_div.table = clk_25m_div_table;
 	dwmac->m25_div.hw.init = &init;
-	dwmac->m25_div.flags = CLK_DIVIDER_ALLOW_ZERO;
+	dwmac->m25_div.flags = CLK_DIVIDER_ALLOW_ZERO |
+				CLK_DIVIDER_ROUND_CLOSEST;
 
 	dwmac->m25_div_clk = devm_clk_register(dev, &dwmac->m25_div.hw);
 	if (WARN_ON(IS_ERR(dwmac->m25_div_clk)))
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFT net-next v2 2/3] net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
@ 2017-12-23 23:40   ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-23 23:40 UTC (permalink / raw)
  To: linus-amlogic

Meson8b only supports MPLL2 as clock input. The rate of the MPLL2 clock
set by Odroid-C1's u-boot is close to 500MHz. The exact rate is
500002394Hz, which is calculated in drivers/clk/meson/clk-mpll.c
using the following formula:
DIV_ROUND_UP_ULL((u64)parent_rate * SDM_DEN, (SDM_DEN * n2) + sdm)
Odroid-C1's u-boot configures MPLL2 with the following values:
- SDM_DEN = 16384
- SDM = 1638
- N2 = 5

The 250MHz and 25MHz clocks inside dwmac-meson8b driver are derived
from the MPLL2 clock. Due to MPLL2 running slightly faster than 500MHz
the common clock framework chooses dividers which are too big to
generate the 250MHz and 25MHz clocks. Emiliano Ingrassia observed that
the divider for the 250MHz clock was set to 0x5 which results in a clock
rate of close to 100MHz instead of 250MHz. The divider for the 25MHz
clock is set to 0x0 (which means "divide by 5") so the resulting RGMII
clock is running at 20MHz (plus a few additional Hz). The RTL8211F PHY
on Odroid-C1 however fails to operate with a 20MHz RGMII clock.

Round the divider's clock rates to prevent this issue on Meson8b. This
means we'll now end up with a clock rate of 25000120Hz (= 25MHz plus
120Hz).
This has no effect on the Meson GX SoCs since there fclk_div2 is used as
input clock, which has a rate of 1000MHz (and thus is divisible cleanly
to 250MHz and 25MHz).

Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
Reported-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index e1d5907e481c..0da551c84fe8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -144,7 +144,9 @@ static int meson8b_init_rgmii_clk(struct meson8b_dwmac *dwmac)
 	dwmac->m250_div.shift = PRG_ETH0_CLK_M250_DIV_SHIFT;
 	dwmac->m250_div.width = PRG_ETH0_CLK_M250_DIV_WIDTH;
 	dwmac->m250_div.hw.init = &init;
-	dwmac->m250_div.flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO;
+	dwmac->m250_div.flags = CLK_DIVIDER_ONE_BASED |
+				CLK_DIVIDER_ALLOW_ZERO |
+				CLK_DIVIDER_ROUND_CLOSEST;
 
 	dwmac->m250_div_clk = devm_clk_register(dev, &dwmac->m250_div.hw);
 	if (WARN_ON(IS_ERR(dwmac->m250_div_clk)))
@@ -164,7 +166,8 @@ static int meson8b_init_rgmii_clk(struct meson8b_dwmac *dwmac)
 	dwmac->m25_div.width = PRG_ETH0_CLK_M25_DIV_WIDTH;
 	dwmac->m25_div.table = clk_25m_div_table;
 	dwmac->m25_div.hw.init = &init;
-	dwmac->m25_div.flags = CLK_DIVIDER_ALLOW_ZERO;
+	dwmac->m25_div.flags = CLK_DIVIDER_ALLOW_ZERO |
+				CLK_DIVIDER_ROUND_CLOSEST;
 
 	dwmac->m25_div_clk = devm_clk_register(dev, &dwmac->m25_div.hw);
 	if (WARN_ON(IS_ERR(dwmac->m25_div_clk)))
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFT net-next v2 3/3] net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
  2017-12-23 23:40 ` Martin Blumenstingl
@ 2017-12-23 23:41   ` Martin Blumenstingl
  -1 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-23 23:41 UTC (permalink / raw)
  To: netdev, ingrassia
  Cc: linus.luessing, khilman, linux-amlogic, jbrunet, narmstrong,
	peppe.cavallaro, alexandre.torgue, Martin Blumenstingl

On Meson8b the only valid input clock is MPLL2. The bootloader
configures that to run at 500002394Hz which cannot be divided evenly
down to 25MHz using the m250_div and m25_div clocks. Currently the
common clock framework chooses a m250_div of 2 and a m25_div of 10,
which results in a RGMII clock of 25000120Hz (120Hz above the requested
25MHz).

Letting the common clock framework propagate the rate changes from the
m25_div clock to m250_div clock to m250_mux to it's parent allows us to
get the best possible parent clock rate. With this patch the common clock
framework calculates a rate of close-to-125MHz (124999851Hz to be exact)
for the MPLL2 clock (which is the mux input). Dividing that by 5 (using
only the m25_div) gives us a RGMII clock of 24999971Hz (which is only
29Hz off the requested 25MHz, compared to 120Hz from u-boot and the
vendor driver).

For now we only want to propagate rate changes to make sure that the mux
parent is not changed automatically by the common clock framework. This
is just to keep the number of side-effects from this patch at a minimum.

SoCs from the Meson GX series are not affected by this change because
the input clock is FCLK_DIV2 whose rate cannot be changed. Additionally
the GX SoCs don't need to use the "closest" divider since the parent
clock is a multiple of 250MHz.

Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
Suggested-by: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 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 0da551c84fe8..e542c8a14f2a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -116,7 +116,7 @@ static int meson8b_init_rgmii_clk(struct meson8b_dwmac *dwmac)
 	snprintf(clk_name, sizeof(clk_name), "%s#m250_sel", dev_name(dev));
 	init.name = clk_name;
 	init.ops = &clk_mux_ops;
-	init.flags = 0;
+	init.flags = CLK_SET_RATE_PARENT;
 	init.parent_names = mux_parent_names;
 	init.num_parents = MUX_CLK_NUM_PARENTS;
 
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFT net-next v2 3/3] net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
@ 2017-12-23 23:41   ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-23 23:41 UTC (permalink / raw)
  To: linus-amlogic

On Meson8b the only valid input clock is MPLL2. The bootloader
configures that to run at 500002394Hz which cannot be divided evenly
down to 25MHz using the m250_div and m25_div clocks. Currently the
common clock framework chooses a m250_div of 2 and a m25_div of 10,
which results in a RGMII clock of 25000120Hz (120Hz above the requested
25MHz).

Letting the common clock framework propagate the rate changes from the
m25_div clock to m250_div clock to m250_mux to it's parent allows us to
get the best possible parent clock rate. With this patch the common clock
framework calculates a rate of close-to-125MHz (124999851Hz to be exact)
for the MPLL2 clock (which is the mux input). Dividing that by 5 (using
only the m25_div) gives us a RGMII clock of 24999971Hz (which is only
29Hz off the requested 25MHz, compared to 120Hz from u-boot and the
vendor driver).

For now we only want to propagate rate changes to make sure that the mux
parent is not changed automatically by the common clock framework. This
is just to keep the number of side-effects from this patch at a minimum.

SoCs from the Meson GX series are not affected by this change because
the input clock is FCLK_DIV2 whose rate cannot be changed. Additionally
the GX SoCs don't need to use the "closest" divider since the parent
clock is a multiple of 250MHz.

Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
Suggested-by: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 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 0da551c84fe8..e542c8a14f2a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -116,7 +116,7 @@ static int meson8b_init_rgmii_clk(struct meson8b_dwmac *dwmac)
 	snprintf(clk_name, sizeof(clk_name), "%s#m250_sel", dev_name(dev));
 	init.name = clk_name;
 	init.ops = &clk_mux_ops;
-	init.flags = 0;
+	init.flags = CLK_SET_RATE_PARENT;
 	init.parent_names = mux_parent_names;
 	init.num_parents = MUX_CLK_NUM_PARENTS;
 
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
  2017-12-23 23:40 ` Martin Blumenstingl
@ 2017-12-28 16:16   ` Emiliano Ingrassia
  -1 siblings, 0 replies; 16+ messages in thread
From: Emiliano Ingrassia @ 2017-12-28 16:16 UTC (permalink / raw)
  To: Martin Blumenstingl, netdev
  Cc: linus.luessing, khilman, linux-amlogic, jbrunet, narmstrong,
	peppe.cavallaro, alexandre.torgue

Hi Martin, Hi Dave,

On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
> Hi Dave,
> 
> please do not apply this series until it got a Tested-by from Emiliano.
> 
> 
> Hi Emiliano,
> 
> you reported [0] that you couldn't get dwmac-meson8b to work on your
> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
> I think I was able to find a fix: it consists of two patches (which you
> find in this series)
> 
> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
> only partially test this (I could only check if the clocks were
> calculated correctly when using a dummy 500002394Hz input clock instead
> of MPLL2).
> 
> Could you please give this series a try and let me know about the
> results?
> You obviously still need your two "ARM: dts: meson8b" patches which
> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
> - enable Ethernet on the Odroid-C1
> 
> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
> fine (so let's hope that this also fixes your Meson8b issue :)).
> 
> 
> changes since v1 at [1]:
> - changed the subject of the cover-letter to indicate that this is all
>   about the RGMII clock
> - added PATCH #1 which ensures that we don't unnecessarily change the
>   parent clocks in RMII mode (and also makes the code easier to
>   understand)
> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
>   is about the RGMII clock
> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
> - replaced PATCH #3 (formerly PATCH #2) with one that sets
>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
>   on Meson8b correctly
>

Really thank you for your help and effort. I tried your patch but
unfortunately it didn't solve the problem.

Here is the new clk_summary:

xtal                            1            1    24000000          0 0
 sys_pll                        0            0  1200000000          0 0
  cpu_clk                       0            0  1200000000          0 0
 vid_pll                        0            0   732000000          0 0
 fixed_pll                      2            2  2550000000          0 0
  mpll2                         1            1   106250000          0 0
   c9410000.ethernet#m250_sel   1            1   106250000          0 0
    c9410000.ethernet#m250_div  1            1   106250000          0 0
     c9410000.ethernet#m25_div  1            1    21250000          0 0

which leads to a value of 0x70a1 in the prg0 ethernet register.
As you can see, something is changed but the RGMII clock is not at 25 MHz.
In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
clock for PHY" (see S805 manual), is 0.

Please, if you have other suggestions let me know.

Best regards,

Emiliano

> 
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
> 
> 
> Martin Blumenstingl (3):
>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
> 
>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
>  1 file changed, 27 insertions(+), 28 deletions(-)
> 
> -- 
> 2.15.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
@ 2017-12-28 16:16   ` Emiliano Ingrassia
  0 siblings, 0 replies; 16+ messages in thread
From: Emiliano Ingrassia @ 2017-12-28 16:16 UTC (permalink / raw)
  To: linus-amlogic

Hi Martin, Hi Dave,

On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
> Hi Dave,
> 
> please do not apply this series until it got a Tested-by from Emiliano.
> 
> 
> Hi Emiliano,
> 
> you reported [0] that you couldn't get dwmac-meson8b to work on your
> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
> I think I was able to find a fix: it consists of two patches (which you
> find in this series)
> 
> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
> only partially test this (I could only check if the clocks were
> calculated correctly when using a dummy 500002394Hz input clock instead
> of MPLL2).
> 
> Could you please give this series a try and let me know about the
> results?
> You obviously still need your two "ARM: dts: meson8b" patches which
> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
> - enable Ethernet on the Odroid-C1
> 
> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
> fine (so let's hope that this also fixes your Meson8b issue :)).
> 
> 
> changes since v1 at [1]:
> - changed the subject of the cover-letter to indicate that this is all
>   about the RGMII clock
> - added PATCH #1 which ensures that we don't unnecessarily change the
>   parent clocks in RMII mode (and also makes the code easier to
>   understand)
> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
>   is about the RGMII clock
> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
> - replaced PATCH #3 (formerly PATCH #2) with one that sets
>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
>   on Meson8b correctly
>

Really thank you for your help and effort. I tried your patch but
unfortunately it didn't solve the problem.

Here is the new clk_summary:

xtal                            1            1    24000000          0 0
 sys_pll                        0            0  1200000000          0 0
  cpu_clk                       0            0  1200000000          0 0
 vid_pll                        0            0   732000000          0 0
 fixed_pll                      2            2  2550000000          0 0
  mpll2                         1            1   106250000          0 0
   c9410000.ethernet#m250_sel   1            1   106250000          0 0
    c9410000.ethernet#m250_div  1            1   106250000          0 0
     c9410000.ethernet#m25_div  1            1    21250000          0 0

which leads to a value of 0x70a1 in the prg0 ethernet register.
As you can see, something is changed but the RGMII clock is not at 25 MHz.
In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
clock for PHY" (see S805 manual), is 0.

Please, if you have other suggestions let me know.

Best regards,

Emiliano

> 
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
> 
> 
> Martin Blumenstingl (3):
>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
> 
>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
>  1 file changed, 27 insertions(+), 28 deletions(-)
> 
> -- 
> 2.15.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
  2017-12-28 16:16   ` Emiliano Ingrassia
@ 2017-12-28 16:58     ` Martin Blumenstingl
  -1 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-28 16:58 UTC (permalink / raw)
  To: Emiliano Ingrassia
  Cc: netdev, linus.luessing, khilman, linux-amlogic, jbrunet,
	Neil Armstrong, peppe.cavallaro, alexandre.torgue

Hi Emiliano,

thank you for testing this!

On Thu, Dec 28, 2017 at 5:16 PM, Emiliano Ingrassia
<ingrassia@epigenesys.com> wrote:
> Hi Martin, Hi Dave,
>
> On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
>> Hi Dave,
>>
>> please do not apply this series until it got a Tested-by from Emiliano.
>>
>>
>> Hi Emiliano,
>>
>> you reported [0] that you couldn't get dwmac-meson8b to work on your
>> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
>> I think I was able to find a fix: it consists of two patches (which you
>> find in this series)
>>
>> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
>> only partially test this (I could only check if the clocks were
>> calculated correctly when using a dummy 500002394Hz input clock instead
>> of MPLL2).
>>
>> Could you please give this series a try and let me know about the
>> results?
>> You obviously still need your two "ARM: dts: meson8b" patches which
>> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
>> - enable Ethernet on the Odroid-C1
>>
>> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
>> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
>> fine (so let's hope that this also fixes your Meson8b issue :)).
>>
>>
>> changes since v1 at [1]:
>> - changed the subject of the cover-letter to indicate that this is all
>>   about the RGMII clock
>> - added PATCH #1 which ensures that we don't unnecessarily change the
>>   parent clocks in RMII mode (and also makes the code easier to
>>   understand)
>> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
>>   is about the RGMII clock
>> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
>> - replaced PATCH #3 (formerly PATCH #2) with one that sets
>>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
>>   on Meson8b correctly
>>
>
> Really thank you for your help and effort. I tried your patch but
> unfortunately it didn't solve the problem.
this is probably my fault: I forgot to mention that it requires a fix
for the 32-bit SoCs in the clock driver ("clk: meson: mpll: use 64-bit
maths in params_from_rate", see [0]) to work properly

>
> Here is the new clk_summary:
>
> xtal                            1            1    24000000          0 0
>  sys_pll                        0            0  1200000000          0 0
>   cpu_clk                       0            0  1200000000          0 0
>  vid_pll                        0            0   732000000          0 0
>  fixed_pll                      2            2  2550000000          0 0
>   mpll2                         1            1   106250000          0 0
>    c9410000.ethernet#m250_sel   1            1   106250000          0 0
>     c9410000.ethernet#m250_div  1            1   106250000          0 0
>      c9410000.ethernet#m25_div  1            1    21250000          0 0
>
> which leads to a value of 0x70a1 in the prg0 ethernet register.
> As you can see, something is changed but the RGMII clock is not at 25 MHz.
> In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
> clock for PHY" (see S805 manual), is 0.
assuming that the description in the datasheet is correct
after Kevin and Mike got updated information from Amlogic about the
PRG_ETHERNET0 register documenation (see [1]) we thought that bit 10
means "0 = divide by 5, 1 = divide by 10" (see [2]). I didn't question
this so far, but I'll test this on a newer SoC later (by forcing
m250_div to 125MHz, then m25_div will have register value 0 = divide
by 5)

if the description from the documentation is correct then we need to
replace m25_div with a fixed-factor clock (mult = 1, div = 5) and make
it a m25_en gate clock instead
the resulting clock path would look like this: mpll2 > m250_sel >
m250_div > fixed_factor > m25_en

> Please, if you have other suggestions let me know.
could you please re-test this with the patch from [0] applied? no
other changes should be needed!

> Best regards,
>
> Emiliano
>
>>
>> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
>> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
>>
>>
>> Martin Blumenstingl (3):
>>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
>>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
>>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
>>
>>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
>>  1 file changed, 27 insertions(+), 28 deletions(-)
>>
>> --
>> 2.15.1
>>

Regards
Martin


[0] https://patchwork.kernel.org/patch/10131677/
[1] https://www.mail-archive.com/netdev@vger.kernel.org/msg119290.html
[2] https://www.mail-archive.com/netdev@vger.kernel.org/msg119293.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
@ 2017-12-28 16:58     ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-28 16:58 UTC (permalink / raw)
  To: linus-amlogic

Hi Emiliano,

thank you for testing this!

On Thu, Dec 28, 2017 at 5:16 PM, Emiliano Ingrassia
<ingrassia@epigenesys.com> wrote:
> Hi Martin, Hi Dave,
>
> On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
>> Hi Dave,
>>
>> please do not apply this series until it got a Tested-by from Emiliano.
>>
>>
>> Hi Emiliano,
>>
>> you reported [0] that you couldn't get dwmac-meson8b to work on your
>> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
>> I think I was able to find a fix: it consists of two patches (which you
>> find in this series)
>>
>> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
>> only partially test this (I could only check if the clocks were
>> calculated correctly when using a dummy 500002394Hz input clock instead
>> of MPLL2).
>>
>> Could you please give this series a try and let me know about the
>> results?
>> You obviously still need your two "ARM: dts: meson8b" patches which
>> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
>> - enable Ethernet on the Odroid-C1
>>
>> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
>> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
>> fine (so let's hope that this also fixes your Meson8b issue :)).
>>
>>
>> changes since v1 at [1]:
>> - changed the subject of the cover-letter to indicate that this is all
>>   about the RGMII clock
>> - added PATCH #1 which ensures that we don't unnecessarily change the
>>   parent clocks in RMII mode (and also makes the code easier to
>>   understand)
>> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
>>   is about the RGMII clock
>> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
>> - replaced PATCH #3 (formerly PATCH #2) with one that sets
>>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
>>   on Meson8b correctly
>>
>
> Really thank you for your help and effort. I tried your patch but
> unfortunately it didn't solve the problem.
this is probably my fault: I forgot to mention that it requires a fix
for the 32-bit SoCs in the clock driver ("clk: meson: mpll: use 64-bit
maths in params_from_rate", see [0]) to work properly

>
> Here is the new clk_summary:
>
> xtal                            1            1    24000000          0 0
>  sys_pll                        0            0  1200000000          0 0
>   cpu_clk                       0            0  1200000000          0 0
>  vid_pll                        0            0   732000000          0 0
>  fixed_pll                      2            2  2550000000          0 0
>   mpll2                         1            1   106250000          0 0
>    c9410000.ethernet#m250_sel   1            1   106250000          0 0
>     c9410000.ethernet#m250_div  1            1   106250000          0 0
>      c9410000.ethernet#m25_div  1            1    21250000          0 0
>
> which leads to a value of 0x70a1 in the prg0 ethernet register.
> As you can see, something is changed but the RGMII clock is not at 25 MHz.
> In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
> clock for PHY" (see S805 manual), is 0.
assuming that the description in the datasheet is correct
after Kevin and Mike got updated information from Amlogic about the
PRG_ETHERNET0 register documenation (see [1]) we thought that bit 10
means "0 = divide by 5, 1 = divide by 10" (see [2]). I didn't question
this so far, but I'll test this on a newer SoC later (by forcing
m250_div to 125MHz, then m25_div will have register value 0 = divide
by 5)

if the description from the documentation is correct then we need to
replace m25_div with a fixed-factor clock (mult = 1, div = 5) and make
it a m25_en gate clock instead
the resulting clock path would look like this: mpll2 > m250_sel >
m250_div > fixed_factor > m25_en

> Please, if you have other suggestions let me know.
could you please re-test this with the patch from [0] applied? no
other changes should be needed!

> Best regards,
>
> Emiliano
>
>>
>> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
>> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
>>
>>
>> Martin Blumenstingl (3):
>>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
>>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
>>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
>>
>>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
>>  1 file changed, 27 insertions(+), 28 deletions(-)
>>
>> --
>> 2.15.1
>>

Regards
Martin


[0] https://patchwork.kernel.org/patch/10131677/
[1] https://www.mail-archive.com/netdev at vger.kernel.org/msg119290.html
[2] https://www.mail-archive.com/netdev at vger.kernel.org/msg119293.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
  2017-12-28 16:58     ` Martin Blumenstingl
@ 2017-12-28 17:51       ` Emiliano Ingrassia
  -1 siblings, 0 replies; 16+ messages in thread
From: Emiliano Ingrassia @ 2017-12-28 17:51 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: netdev, linus.luessing, khilman, linux-amlogic, jbrunet,
	Neil Armstrong, peppe.cavallaro, alexandre.torgue

Hi Martin,

thank you for the quick response!

On Thu, Dec 28, 2017 at 05:58:34PM +0100, Martin Blumenstingl wrote:
> Hi Emiliano,
> 
> thank you for testing this!
> 
> On Thu, Dec 28, 2017 at 5:16 PM, Emiliano Ingrassia
> <ingrassia@epigenesys.com> wrote:
> > Hi Martin, Hi Dave,
> >
> > On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
> >> Hi Dave,
> >>
> >> please do not apply this series until it got a Tested-by from Emiliano.
> >>
> >>
> >> Hi Emiliano,
> >>
> >> you reported [0] that you couldn't get dwmac-meson8b to work on your
> >> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
> >> I think I was able to find a fix: it consists of two patches (which you
> >> find in this series)
> >>
> >> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
> >> only partially test this (I could only check if the clocks were
> >> calculated correctly when using a dummy 500002394Hz input clock instead
> >> of MPLL2).
> >>
> >> Could you please give this series a try and let me know about the
> >> results?
> >> You obviously still need your two "ARM: dts: meson8b" patches which
> >> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
> >> - enable Ethernet on the Odroid-C1
> >>
> >> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
> >> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
> >> fine (so let's hope that this also fixes your Meson8b issue :)).
> >>
> >>
> >> changes since v1 at [1]:
> >> - changed the subject of the cover-letter to indicate that this is all
> >>   about the RGMII clock
> >> - added PATCH #1 which ensures that we don't unnecessarily change the
> >>   parent clocks in RMII mode (and also makes the code easier to
> >>   understand)
> >> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
> >>   is about the RGMII clock
> >> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
> >> - replaced PATCH #3 (formerly PATCH #2) with one that sets
> >>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
> >>   on Meson8b correctly
> >>
> >
> > Really thank you for your help and effort. I tried your patch but
> > unfortunately it didn't solve the problem.
> this is probably my fault: I forgot to mention that it requires a fix
> for the 32-bit SoCs in the clock driver ("clk: meson: mpll: use 64-bit
> maths in params_from_rate", see [0]) to work properly
>

Ok, with that patch applied I got:

xtal                               1            1    24000000          0 0
 sys_pll                           0            0  1200000000          0 0
  cpu_clk                          0            0  1200000000          0 0
 vid_pll                           0            0   732000000          0 0
 fixed_pll                         2            2  2550000000          0 0
  mpll2                            1            1   124999851          0 0
   c9410000.ethernet#m250_sel      1            1   124999851          0 0
    c9410000.ethernet#m250_div     1            1   124999851          0 0
     c9410000.ethernet#m25_div     1            1    24999971          0 0

which is equal to your result. However, the ethernet is still not working.
The prg0 register is set to 0x70A1.

A problem that I see with this solution is that MPLL2 is set to ~125 MHz.
The S805 SoC manual reports that bits 9-7 should contain a value x such
that: MPLL2 = 250 MHz * x (with x >= 1).
In our case, bits 9-7 are set to 1 which is incorrect.
I think that MPLL2 should be 250 MHz at least.

> >
> > Here is the new clk_summary:
> >
> > xtal                            1            1    24000000          0 0
> >  sys_pll                        0            0  1200000000          0 0
> >   cpu_clk                       0            0  1200000000          0 0
> >  vid_pll                        0            0   732000000          0 0
> >  fixed_pll                      2            2  2550000000          0 0
> >   mpll2                         1            1   106250000          0 0
> >    c9410000.ethernet#m250_sel   1            1   106250000          0 0
> >     c9410000.ethernet#m250_div  1            1   106250000          0 0
> >      c9410000.ethernet#m25_div  1            1    21250000          0 0
> >
> > which leads to a value of 0x70a1 in the prg0 ethernet register.
> > As you can see, something is changed but the RGMII clock is not at 25 MHz.
> > In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
> > clock for PHY" (see S805 manual), is 0.
> assuming that the description in the datasheet is correct
> after Kevin and Mike got updated information from Amlogic about the
> PRG_ETHERNET0 register documenation (see [1]) we thought that bit 10
> means "0 = divide by 5, 1 = divide by 10" (see [2]). I didn't question
> this so far, but I'll test this on a newer SoC later (by forcing
> m250_div to 125MHz, then m25_div will have register value 0 = divide
> by 5)
> 
> if the description from the documentation is correct then we need to
> replace m25_div with a fixed-factor clock (mult = 1, div = 5) and make
> it a m25_en gate clock instead
> the resulting clock path would look like this: mpll2 > m250_sel >
> m250_div > fixed_factor > m25_en
> 
> > Please, if you have other suggestions let me know.
> could you please re-test this with the patch from [0] applied? no
> other changes should be needed!
> 
> > Best regards,
> >
> > Emiliano
> >
> >>
> >> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
> >> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
> >>
> >>
> >> Martin Blumenstingl (3):
> >>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
> >>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
> >>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
> >>
> >>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
> >>  1 file changed, 27 insertions(+), 28 deletions(-)
> >>
> >> --
> >> 2.15.1
> >>
> 
> Regards
> Martin
> 

Thank you,

Emiliano

> 
> [0] https://patchwork.kernel.org/patch/10131677/
> [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg119290.html
> [2] https://www.mail-archive.com/netdev@vger.kernel.org/msg119293.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
@ 2017-12-28 17:51       ` Emiliano Ingrassia
  0 siblings, 0 replies; 16+ messages in thread
From: Emiliano Ingrassia @ 2017-12-28 17:51 UTC (permalink / raw)
  To: linus-amlogic

Hi Martin,

thank you for the quick response!

On Thu, Dec 28, 2017 at 05:58:34PM +0100, Martin Blumenstingl wrote:
> Hi Emiliano,
> 
> thank you for testing this!
> 
> On Thu, Dec 28, 2017 at 5:16 PM, Emiliano Ingrassia
> <ingrassia@epigenesys.com> wrote:
> > Hi Martin, Hi Dave,
> >
> > On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
> >> Hi Dave,
> >>
> >> please do not apply this series until it got a Tested-by from Emiliano.
> >>
> >>
> >> Hi Emiliano,
> >>
> >> you reported [0] that you couldn't get dwmac-meson8b to work on your
> >> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
> >> I think I was able to find a fix: it consists of two patches (which you
> >> find in this series)
> >>
> >> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
> >> only partially test this (I could only check if the clocks were
> >> calculated correctly when using a dummy 500002394Hz input clock instead
> >> of MPLL2).
> >>
> >> Could you please give this series a try and let me know about the
> >> results?
> >> You obviously still need your two "ARM: dts: meson8b" patches which
> >> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
> >> - enable Ethernet on the Odroid-C1
> >>
> >> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
> >> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
> >> fine (so let's hope that this also fixes your Meson8b issue :)).
> >>
> >>
> >> changes since v1 at [1]:
> >> - changed the subject of the cover-letter to indicate that this is all
> >>   about the RGMII clock
> >> - added PATCH #1 which ensures that we don't unnecessarily change the
> >>   parent clocks in RMII mode (and also makes the code easier to
> >>   understand)
> >> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
> >>   is about the RGMII clock
> >> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
> >> - replaced PATCH #3 (formerly PATCH #2) with one that sets
> >>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
> >>   on Meson8b correctly
> >>
> >
> > Really thank you for your help and effort. I tried your patch but
> > unfortunately it didn't solve the problem.
> this is probably my fault: I forgot to mention that it requires a fix
> for the 32-bit SoCs in the clock driver ("clk: meson: mpll: use 64-bit
> maths in params_from_rate", see [0]) to work properly
>

Ok, with that patch applied I got:

xtal                               1            1    24000000          0 0
 sys_pll                           0            0  1200000000          0 0
  cpu_clk                          0            0  1200000000          0 0
 vid_pll                           0            0   732000000          0 0
 fixed_pll                         2            2  2550000000          0 0
  mpll2                            1            1   124999851          0 0
   c9410000.ethernet#m250_sel      1            1   124999851          0 0
    c9410000.ethernet#m250_div     1            1   124999851          0 0
     c9410000.ethernet#m25_div     1            1    24999971          0 0

which is equal to your result. However, the ethernet is still not working.
The prg0 register is set to 0x70A1.

A problem that I see with this solution is that MPLL2 is set to ~125 MHz.
The S805 SoC manual reports that bits 9-7 should contain a value x such
that: MPLL2 = 250 MHz * x (with x >= 1).
In our case, bits 9-7 are set to 1 which is incorrect.
I think that MPLL2 should be 250 MHz at least.

> >
> > Here is the new clk_summary:
> >
> > xtal                            1            1    24000000          0 0
> >  sys_pll                        0            0  1200000000          0 0
> >   cpu_clk                       0            0  1200000000          0 0
> >  vid_pll                        0            0   732000000          0 0
> >  fixed_pll                      2            2  2550000000          0 0
> >   mpll2                         1            1   106250000          0 0
> >    c9410000.ethernet#m250_sel   1            1   106250000          0 0
> >     c9410000.ethernet#m250_div  1            1   106250000          0 0
> >      c9410000.ethernet#m25_div  1            1    21250000          0 0
> >
> > which leads to a value of 0x70a1 in the prg0 ethernet register.
> > As you can see, something is changed but the RGMII clock is not at 25 MHz.
> > In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
> > clock for PHY" (see S805 manual), is 0.
> assuming that the description in the datasheet is correct
> after Kevin and Mike got updated information from Amlogic about the
> PRG_ETHERNET0 register documenation (see [1]) we thought that bit 10
> means "0 = divide by 5, 1 = divide by 10" (see [2]). I didn't question
> this so far, but I'll test this on a newer SoC later (by forcing
> m250_div to 125MHz, then m25_div will have register value 0 = divide
> by 5)
> 
> if the description from the documentation is correct then we need to
> replace m25_div with a fixed-factor clock (mult = 1, div = 5) and make
> it a m25_en gate clock instead
> the resulting clock path would look like this: mpll2 > m250_sel >
> m250_div > fixed_factor > m25_en
> 
> > Please, if you have other suggestions let me know.
> could you please re-test this with the patch from [0] applied? no
> other changes should be needed!
> 
> > Best regards,
> >
> > Emiliano
> >
> >>
> >> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
> >> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
> >>
> >>
> >> Martin Blumenstingl (3):
> >>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
> >>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
> >>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
> >>
> >>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
> >>  1 file changed, 27 insertions(+), 28 deletions(-)
> >>
> >> --
> >> 2.15.1
> >>
> 
> Regards
> Martin
> 

Thank you,

Emiliano

> 
> [0] https://patchwork.kernel.org/patch/10131677/
> [1] https://www.mail-archive.com/netdev at vger.kernel.org/msg119290.html
> [2] https://www.mail-archive.com/netdev at vger.kernel.org/msg119293.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
  2017-12-28 17:51       ` Emiliano Ingrassia
@ 2017-12-28 19:28         ` Martin Blumenstingl
  -1 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-28 19:28 UTC (permalink / raw)
  To: Emiliano Ingrassia
  Cc: netdev, linus.luessing, khilman, linux-amlogic, jbrunet,
	Neil Armstrong, peppe.cavallaro, alexandre.torgue

Hi Emiliano,

On Thu, Dec 28, 2017 at 6:51 PM, Emiliano Ingrassia
<ingrassia@epigenesys.com> wrote:
> Hi Martin,
>
> thank you for the quick response!
>
> On Thu, Dec 28, 2017 at 05:58:34PM +0100, Martin Blumenstingl wrote:
>> Hi Emiliano,
>>
>> thank you for testing this!
>>
>> On Thu, Dec 28, 2017 at 5:16 PM, Emiliano Ingrassia
>> <ingrassia@epigenesys.com> wrote:
>> > Hi Martin, Hi Dave,
>> >
>> > On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
>> >> Hi Dave,
>> >>
>> >> please do not apply this series until it got a Tested-by from Emiliano.
>> >>
>> >>
>> >> Hi Emiliano,
>> >>
>> >> you reported [0] that you couldn't get dwmac-meson8b to work on your
>> >> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
>> >> I think I was able to find a fix: it consists of two patches (which you
>> >> find in this series)
>> >>
>> >> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
>> >> only partially test this (I could only check if the clocks were
>> >> calculated correctly when using a dummy 500002394Hz input clock instead
>> >> of MPLL2).
>> >>
>> >> Could you please give this series a try and let me know about the
>> >> results?
>> >> You obviously still need your two "ARM: dts: meson8b" patches which
>> >> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
>> >> - enable Ethernet on the Odroid-C1
>> >>
>> >> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
>> >> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
>> >> fine (so let's hope that this also fixes your Meson8b issue :)).
>> >>
>> >>
>> >> changes since v1 at [1]:
>> >> - changed the subject of the cover-letter to indicate that this is all
>> >>   about the RGMII clock
>> >> - added PATCH #1 which ensures that we don't unnecessarily change the
>> >>   parent clocks in RMII mode (and also makes the code easier to
>> >>   understand)
>> >> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
>> >>   is about the RGMII clock
>> >> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
>> >> - replaced PATCH #3 (formerly PATCH #2) with one that sets
>> >>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
>> >>   on Meson8b correctly
>> >>
>> >
>> > Really thank you for your help and effort. I tried your patch but
>> > unfortunately it didn't solve the problem.
>> this is probably my fault: I forgot to mention that it requires a fix
>> for the 32-bit SoCs in the clock driver ("clk: meson: mpll: use 64-bit
>> maths in params_from_rate", see [0]) to work properly
>>
>
> Ok, with that patch applied I got:
>
> xtal                               1            1    24000000          0 0
>  sys_pll                           0            0  1200000000          0 0
>   cpu_clk                          0            0  1200000000          0 0
>  vid_pll                           0            0   732000000          0 0
>  fixed_pll                         2            2  2550000000          0 0
>   mpll2                            1            1   124999851          0 0
>    c9410000.ethernet#m250_sel      1            1   124999851          0 0
>     c9410000.ethernet#m250_div     1            1   124999851          0 0
>      c9410000.ethernet#m25_div     1            1    24999971          0 0
in theory this looks good...!

> which is equal to your result. However, the ethernet is still not working.
OK, I'll send an updated version later (or tomorrow, depending on how
much time I have left today) which adds a fixed divider and converts
bit 10 to a gate
on GXBB and newer bit 10 is always true since the m250_div is only
3-bit wide (= max divider of 7, 0 is invalid according to the
datasheet). with a 1000MHz input clock (fclk_div2) m250_div will
divide this by 4 and m25_div divided this by 10.

> The prg0 register is set to 0x70A1.
>
> A problem that I see with this solution is that MPLL2 is set to ~125 MHz.
> The S805 SoC manual reports that bits 9-7 should contain a value x such
> that: MPLL2 = 250 MHz * x (with x >= 1).
> In our case, bits 9-7 are set to 1 which is incorrect.
> I think that MPLL2 should be 250 MHz at least.
when looking at the GXBB clock tree we need a fixed divide by 10.
this also means that the mpll2 clock will probably be set to ~250MHz,
the m250_div to 1 and with the fixed divider "10" we get close to our
desired 25MHz

>> >
>> > Here is the new clk_summary:
>> >
>> > xtal                            1            1    24000000          0 0
>> >  sys_pll                        0            0  1200000000          0 0
>> >   cpu_clk                       0            0  1200000000          0 0
>> >  vid_pll                        0            0   732000000          0 0
>> >  fixed_pll                      2            2  2550000000          0 0
>> >   mpll2                         1            1   106250000          0 0
>> >    c9410000.ethernet#m250_sel   1            1   106250000          0 0
>> >     c9410000.ethernet#m250_div  1            1   106250000          0 0
>> >      c9410000.ethernet#m25_div  1            1    21250000          0 0
>> >
>> > which leads to a value of 0x70a1 in the prg0 ethernet register.
>> > As you can see, something is changed but the RGMII clock is not at 25 MHz.
>> > In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
>> > clock for PHY" (see S805 manual), is 0.
>> assuming that the description in the datasheet is correct
>> after Kevin and Mike got updated information from Amlogic about the
>> PRG_ETHERNET0 register documenation (see [1]) we thought that bit 10
>> means "0 = divide by 5, 1 = divide by 10" (see [2]). I didn't question
>> this so far, but I'll test this on a newer SoC later (by forcing
>> m250_div to 125MHz, then m25_div will have register value 0 = divide
>> by 5)
>>
>> if the description from the documentation is correct then we need to
>> replace m25_div with a fixed-factor clock (mult = 1, div = 5) and make
>> it a m25_en gate clock instead
>> the resulting clock path would look like this: mpll2 > m250_sel >
>> m250_div > fixed_factor > m25_en
>>
>> > Please, if you have other suggestions let me know.
>> could you please re-test this with the patch from [0] applied? no
>> other changes should be needed!
>>
>> > Best regards,
>> >
>> > Emiliano
>> >
>> >>
>> >> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
>> >> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
>> >>
>> >>
>> >> Martin Blumenstingl (3):
>> >>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
>> >>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
>> >>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
>> >>
>> >>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
>> >>  1 file changed, 27 insertions(+), 28 deletions(-)
>> >>
>> >> --
>> >> 2.15.1
>> >>
>>
>> Regards
>> Martin
>>
>
> Thank you,
>
> Emiliano
>
>>
>> [0] https://patchwork.kernel.org/patch/10131677/
>> [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg119290.html
>> [2] https://www.mail-archive.com/netdev@vger.kernel.org/msg119293.html


Regards
Martin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b
@ 2017-12-28 19:28         ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2017-12-28 19:28 UTC (permalink / raw)
  To: linus-amlogic

Hi Emiliano,

On Thu, Dec 28, 2017 at 6:51 PM, Emiliano Ingrassia
<ingrassia@epigenesys.com> wrote:
> Hi Martin,
>
> thank you for the quick response!
>
> On Thu, Dec 28, 2017 at 05:58:34PM +0100, Martin Blumenstingl wrote:
>> Hi Emiliano,
>>
>> thank you for testing this!
>>
>> On Thu, Dec 28, 2017 at 5:16 PM, Emiliano Ingrassia
>> <ingrassia@epigenesys.com> wrote:
>> > Hi Martin, Hi Dave,
>> >
>> > On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
>> >> Hi Dave,
>> >>
>> >> please do not apply this series until it got a Tested-by from Emiliano.
>> >>
>> >>
>> >> Hi Emiliano,
>> >>
>> >> you reported [0] that you couldn't get dwmac-meson8b to work on your
>> >> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
>> >> I think I was able to find a fix: it consists of two patches (which you
>> >> find in this series)
>> >>
>> >> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
>> >> only partially test this (I could only check if the clocks were
>> >> calculated correctly when using a dummy 500002394Hz input clock instead
>> >> of MPLL2).
>> >>
>> >> Could you please give this series a try and let me know about the
>> >> results?
>> >> You obviously still need your two "ARM: dts: meson8b" patches which
>> >> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
>> >> - enable Ethernet on the Odroid-C1
>> >>
>> >> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
>> >> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
>> >> fine (so let's hope that this also fixes your Meson8b issue :)).
>> >>
>> >>
>> >> changes since v1 at [1]:
>> >> - changed the subject of the cover-letter to indicate that this is all
>> >>   about the RGMII clock
>> >> - added PATCH #1 which ensures that we don't unnecessarily change the
>> >>   parent clocks in RMII mode (and also makes the code easier to
>> >>   understand)
>> >> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
>> >>   is about the RGMII clock
>> >> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
>> >> - replaced PATCH #3 (formerly PATCH #2) with one that sets
>> >>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
>> >>   on Meson8b correctly
>> >>
>> >
>> > Really thank you for your help and effort. I tried your patch but
>> > unfortunately it didn't solve the problem.
>> this is probably my fault: I forgot to mention that it requires a fix
>> for the 32-bit SoCs in the clock driver ("clk: meson: mpll: use 64-bit
>> maths in params_from_rate", see [0]) to work properly
>>
>
> Ok, with that patch applied I got:
>
> xtal                               1            1    24000000          0 0
>  sys_pll                           0            0  1200000000          0 0
>   cpu_clk                          0            0  1200000000          0 0
>  vid_pll                           0            0   732000000          0 0
>  fixed_pll                         2            2  2550000000          0 0
>   mpll2                            1            1   124999851          0 0
>    c9410000.ethernet#m250_sel      1            1   124999851          0 0
>     c9410000.ethernet#m250_div     1            1   124999851          0 0
>      c9410000.ethernet#m25_div     1            1    24999971          0 0
in theory this looks good...!

> which is equal to your result. However, the ethernet is still not working.
OK, I'll send an updated version later (or tomorrow, depending on how
much time I have left today) which adds a fixed divider and converts
bit 10 to a gate
on GXBB and newer bit 10 is always true since the m250_div is only
3-bit wide (= max divider of 7, 0 is invalid according to the
datasheet). with a 1000MHz input clock (fclk_div2) m250_div will
divide this by 4 and m25_div divided this by 10.

> The prg0 register is set to 0x70A1.
>
> A problem that I see with this solution is that MPLL2 is set to ~125 MHz.
> The S805 SoC manual reports that bits 9-7 should contain a value x such
> that: MPLL2 = 250 MHz * x (with x >= 1).
> In our case, bits 9-7 are set to 1 which is incorrect.
> I think that MPLL2 should be 250 MHz at least.
when looking at the GXBB clock tree we need a fixed divide by 10.
this also means that the mpll2 clock will probably be set to ~250MHz,
the m250_div to 1 and with the fixed divider "10" we get close to our
desired 25MHz

>> >
>> > Here is the new clk_summary:
>> >
>> > xtal                            1            1    24000000          0 0
>> >  sys_pll                        0            0  1200000000          0 0
>> >   cpu_clk                       0            0  1200000000          0 0
>> >  vid_pll                        0            0   732000000          0 0
>> >  fixed_pll                      2            2  2550000000          0 0
>> >   mpll2                         1            1   106250000          0 0
>> >    c9410000.ethernet#m250_sel   1            1   106250000          0 0
>> >     c9410000.ethernet#m250_div  1            1   106250000          0 0
>> >      c9410000.ethernet#m25_div  1            1    21250000          0 0
>> >
>> > which leads to a value of 0x70a1 in the prg0 ethernet register.
>> > As you can see, something is changed but the RGMII clock is not at 25 MHz.
>> > In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
>> > clock for PHY" (see S805 manual), is 0.
>> assuming that the description in the datasheet is correct
>> after Kevin and Mike got updated information from Amlogic about the
>> PRG_ETHERNET0 register documenation (see [1]) we thought that bit 10
>> means "0 = divide by 5, 1 = divide by 10" (see [2]). I didn't question
>> this so far, but I'll test this on a newer SoC later (by forcing
>> m250_div to 125MHz, then m25_div will have register value 0 = divide
>> by 5)
>>
>> if the description from the documentation is correct then we need to
>> replace m25_div with a fixed-factor clock (mult = 1, div = 5) and make
>> it a m25_en gate clock instead
>> the resulting clock path would look like this: mpll2 > m250_sel >
>> m250_div > fixed_factor > m25_en
>>
>> > Please, if you have other suggestions let me know.
>> could you please re-test this with the patch from [0] applied? no
>> other changes should be needed!
>>
>> > Best regards,
>> >
>> > Emiliano
>> >
>> >>
>> >> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
>> >> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
>> >>
>> >>
>> >> Martin Blumenstingl (3):
>> >>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
>> >>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
>> >>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
>> >>
>> >>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 55 +++++++++++-----------
>> >>  1 file changed, 27 insertions(+), 28 deletions(-)
>> >>
>> >> --
>> >> 2.15.1
>> >>
>>
>> Regards
>> Martin
>>
>
> Thank you,
>
> Emiliano
>
>>
>> [0] https://patchwork.kernel.org/patch/10131677/
>> [1] https://www.mail-archive.com/netdev at vger.kernel.org/msg119290.html
>> [2] https://www.mail-archive.com/netdev at vger.kernel.org/msg119293.html


Regards
Martin

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-12-28 19:28 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-23 23:40 [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b Martin Blumenstingl
2017-12-23 23:40 ` Martin Blumenstingl
2017-12-23 23:40 ` [RFT net-next v2 1/3] net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode Martin Blumenstingl
2017-12-23 23:40   ` Martin Blumenstingl
2017-12-23 23:40 ` [RFT net-next v2 2/3] net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b Martin Blumenstingl
2017-12-23 23:40   ` Martin Blumenstingl
2017-12-23 23:41 ` [RFT net-next v2 3/3] net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock Martin Blumenstingl
2017-12-23 23:41   ` Martin Blumenstingl
2017-12-28 16:16 ` [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b Emiliano Ingrassia
2017-12-28 16:16   ` Emiliano Ingrassia
2017-12-28 16:58   ` Martin Blumenstingl
2017-12-28 16:58     ` Martin Blumenstingl
2017-12-28 17:51     ` Emiliano Ingrassia
2017-12-28 17:51       ` Emiliano Ingrassia
2017-12-28 19:28       ` Martin Blumenstingl
2017-12-28 19:28         ` Martin Blumenstingl

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.