2020-12-19 13:55:01

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH] net: stmmac: dwmac-meson8b: ignore the second clock input

The dwmac glue registers on Amlogic Meson8b and newer SoCs has two clock
inputs:
- Meson8b and Meson8m2: MPLL2 and MPLL2 (the same parent is wired to
both inputs)
- GXBB, GXL, GXM, AXG, G12A, G12B, SM1: FCLK_DIV2 and MPLL2

All known vendor kernels and u-boots are using the first input only. We
let the common clock framework automatically choose the "right" parent.
For some boards this causes a problem though, specificially with G12A and
newer SoCs. The clock input is used for generating the 125MHz RGMII TX
clock. For the two input clocks this means on G12A:
- FCLK_DIV2: 999999985Hz / 8 = 124999998.125Hz
- MPLL2: 499999993Hz / 4 = 124999998.25Hz

In theory MPLL2 is the "better" clock input because it's gets us 0.125Hz
closer to the requested frequency than FCLK_DIV2. In reality however
there is a resource conflict because MPLL2 is needed to generate some of
the audio clocks. dwmac-meson8b probes first and sets up the clock tree
with MPLL2. This works fine until the audio driver comes and "steals"
the MPLL2 clocks and configures it with it's own rate (294909637Hz). The
common clock framework happily changes the MPLL2 rate but does not
reconfigure our RGMII TX clock tree, which then ends up at 73727409Hz,
which is more than 40% off the requested 125MHz.

Don't use the second clock input for now to force the common clock
framework to always select the first parent. This mimics the behavior
from the vendor driver and fixes the clock resource conflict with the
audio driver on G12A boards. Once the common clock framework can handle
this situation this change can be reverted again.

Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
Reported-by: Thomas Graichen <[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 459ae715b33d..f184b00f5116 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -135,7 +135,7 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac *dwmac)
struct device *dev = dwmac->dev;
static const struct clk_parent_data mux_parents[] = {
{ .fw_name = "clkin0", },
- { .fw_name = "clkin1", },
+ { .index = -1, },
};
static const struct clk_div_table div_table[] = {
{ .div = 2, .val = 2, },
--
2.29.2


2020-12-21 19:53:29

by Thomas Graichen

[permalink] [raw]
Subject: Re: [PATCH] net: stmmac: dwmac-meson8b: ignore the second clock input

On Sat, Dec 19, 2020 at 2:52 PM Martin Blumenstingl
<[email protected]> wrote:
>
> The dwmac glue registers on Amlogic Meson8b and newer SoCs has two clock
> inputs:
> - Meson8b and Meson8m2: MPLL2 and MPLL2 (the same parent is wired to
> both inputs)
> - GXBB, GXL, GXM, AXG, G12A, G12B, SM1: FCLK_DIV2 and MPLL2
>
> All known vendor kernels and u-boots are using the first input only. We
> let the common clock framework automatically choose the "right" parent.
> For some boards this causes a problem though, specificially with G12A and
> newer SoCs. The clock input is used for generating the 125MHz RGMII TX
> clock. For the two input clocks this means on G12A:
> - FCLK_DIV2: 999999985Hz / 8 = 124999998.125Hz
> - MPLL2: 499999993Hz / 4 = 124999998.25Hz
>
> In theory MPLL2 is the "better" clock input because it's gets us 0.125Hz
> closer to the requested frequency than FCLK_DIV2. In reality however
> there is a resource conflict because MPLL2 is needed to generate some of
> the audio clocks. dwmac-meson8b probes first and sets up the clock tree
> with MPLL2. This works fine until the audio driver comes and "steals"
> the MPLL2 clocks and configures it with it's own rate (294909637Hz). The
> common clock framework happily changes the MPLL2 rate but does not
> reconfigure our RGMII TX clock tree, which then ends up at 73727409Hz,
> which is more than 40% off the requested 125MHz.
>
> Don't use the second clock input for now to force the common clock
> framework to always select the first parent. This mimics the behavior
> from the vendor driver and fixes the clock resource conflict with the
> audio driver on G12A boards. Once the common clock framework can handle
> this situation this change can be reverted again.
>
> Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
> Reported-by: Thomas Graichen <[email protected]>
> Signed-off-by: Martin Blumenstingl <[email protected]>

Tested-by: thomas graichen <[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 459ae715b33d..f184b00f5116 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> @@ -135,7 +135,7 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac *dwmac)
> struct device *dev = dwmac->dev;
> static const struct clk_parent_data mux_parents[] = {
> { .fw_name = "clkin0", },
> - { .fw_name = "clkin1", },
> + { .index = -1, },
> };
> static const struct clk_div_table div_table[] = {
> { .div = 2, .val = 2, },
> --
> 2.29.2
>

2020-12-23 02:51:52

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH] net: stmmac: dwmac-meson8b: ignore the second clock input

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Sat, 19 Dec 2020 14:50:36 +0100 you wrote:
> The dwmac glue registers on Amlogic Meson8b and newer SoCs has two clock
> inputs:
> - Meson8b and Meson8m2: MPLL2 and MPLL2 (the same parent is wired to
> both inputs)
> - GXBB, GXL, GXM, AXG, G12A, G12B, SM1: FCLK_DIV2 and MPLL2
>
> All known vendor kernels and u-boots are using the first input only. We
> let the common clock framework automatically choose the "right" parent.
> For some boards this causes a problem though, specificially with G12A and
> newer SoCs. The clock input is used for generating the 125MHz RGMII TX
> clock. For the two input clocks this means on G12A:
> - FCLK_DIV2: 999999985Hz / 8 = 124999998.125Hz
> - MPLL2: 499999993Hz / 4 = 124999998.25Hz
>
> [...]

Here is the summary with links:
- net: stmmac: dwmac-meson8b: ignore the second clock input
https://git.kernel.org/netdev/net/c/f87777a3c30c

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html