Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201219135036.3216017-1-martin.blumenstingl@googlemail.com>
In-Reply-To: <20201219135036.3216017-1-martin.blumenstingl@googlemail.com>
From:   Thomas Graichen <thomas.graichen@googlemail.com>
Date:   Mon, 21 Dec 2020 20:50:14 +0100
Message-ID: <CAOUEw12Ldi9Kv2Gd=OCEgeVa+jv_FM1HqGRQBiEYCo==8PcrWA@mail.gmail.com>
Subject: Re: [PATCH] net: stmmac: dwmac-meson8b: ignore the second clock input
To:     Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc:     netdev@vger.kernel.org, linux-amlogic@lists.infradead.org,
        davem@davemloft.net, kuba@kernel.org, khilman@baylibre.com,
        jbrunet@baylibre.com, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Dec 19, 2020 at 2:52 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> 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 <thomas.graichen@gmail.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Tested-by: thomas graichen <thomas.graichen@gmail.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 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
>