[EXT] Re: i.MX8MM Clock errors and caam failure on 5.10-rc4
Adam Ford
aford173 at gmail.com
Thu Dec 3 19:07:12 EST 2020
On Mon, Nov 30, 2020 at 8:44 PM Andy Duan <fugang.duan at nxp.com> wrote:
>
> From: Adam Ford <aford173 at gmail.com> Sent: Monday, November 30, 2020 7:29 PM
> > On Mon, Nov 30, 2020 at 2:32 AM Aisheng Dong <aisheng.dong at nxp.com>
> > wrote:
> > >
> > > Hi Fugang & Horia,
> > >
> > > > From: Adam Ford <aford173 at gmail.com>
> > > > Sent: Sunday, November 22, 2020 6:43 AM
> > > >
> > > > I have a board which uses Bluetooth on UART1 similar to the 8mm-evk.
> > > > The serial port for the BT needs to run at 80MHz because I have the
> > > > baud rate of the serial part set to 4M.
> > > >
> > > > I thought I'd give the 5.10-RC's a try. I found some issues that
> > > > concern me, but before I go back to bisect, I thought I'd ask if
> > > > people are aware of it and/or have ideas. The BT is failing to
> > > > operate as is the caam engine, and it seems to be related to some clocking
> > issues.
> > > >
> > > > Failed to get clock for /timer at 306a0000
> > > > Failed to initialize '/timer at 306a0000': -22
> > > > clk: failed to reparent uart1 to sys_pll1_80m: -16
> > > > clk: failed to reparent uart3 to sys_pll1_80m: -16
> > > > caam 30900000.caam: Failed to get clk 'ipg': -2
> > > > caam 30900000.caam: Failed to request all necessary clocks
> > > > caam: probe of 30900000.caam failed with error -2
> > > >
> >
> > I upgraded my version of ATF to the imx_5.4.47_2.2.0 release and the latest
> > upstream U-Boot. With those updates, the timer and caam errors went away,
> > however I am still seeing the following:
> >
> > clk: failed to reparent uart1 to sys_pll1_80m: -16
> > clk: failed to reparent uart3 to sys_pll1_80m: -16
> >
> > Is there something else missing?
>
> Adam, below patch prevent enabled leaf clock to be parented.
> So you should keep uart clock is disabled during clock scan and init in of_clk_init().
> You can refer to the fixes at imx/clk.c:
> https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/clk/imx/clk.c?h=imx_5.4.47_2.2.0
Thanks for the pointer! From what I can tell, all the UART clocks are
being initialized instead of just the clocks being used by the
earlycon, earlyprintk and sdio.
As a test, I modified imx_keep_uart_clocks to always be 0 and that
made the issue go away, and I can re-parent the UART clock as I need.
Looking at NXP's branch, it appears as if they're trying to only
enable the clock associated to the stdio port, but the code seems more
complicated.
As a compromise, I'd like to propose something like:
diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c
index 47882c51cb85..a2e1d70ab782 100644
--- a/drivers/clk/imx/clk.c
+++ b/drivers/clk/imx/clk.c
@@ -163,12 +163,17 @@ __setup_param("earlyprintk", imx_keep_uart_earlyprintk,
void imx_register_uart_clocks(struct clk ** const clks[])
{
+ struct clk *uart_clk;
if (imx_keep_uart_clocks) {
int i;
imx_uart_clocks = clks;
- for (i = 0; imx_uart_clocks[i]; i++)
+ for (i = 0; imx_uart_clocks[i]; i++) {
+ uart_clk = of_clk_get(of_stdout, i++);
+ if (IS_ERR(uart_clk))
+ continue;
clk_prepare_enable(*imx_uart_clocks[i]);
+ }
}
}
This should scan through the node to see if the clock is associated
with stdout, and only then will it enable the clock. This won't fix
the issue if earlycon points to a different UART.
As a test, I attempted to re-parent the uart2 (stdout) to a 80MHz PLL
as well as UART1 and UART3. I expect UART2 to fail for the purposes
of this test to prove the check for stdout is doing what I expect.
The UART associated to the sdio node returns with the busy error as I hoped:
[ 0.179709] clk: failed to reparent uart2 to sys_pll1_80m: -16
However, the rest of the errors are gone.
Looking at the clk_summary, it appears as if the UART1 and UART3 were
able to reparent and UART1 was already being used, so it returned the
busy flag.
# cat /sys/kernel/debug/clk/clk_summary |grep uart
uart3 0 0 0 80000000
0 0 50000
uart3_root_clk 0 0 0 80000000
0 0 50000
uart1 1 1 0 80000000
0 0 50000
uart1_root_clk 2 2 0 80000000
0 0 50000
uart4 0 0 0 24000000
0 0 50000
uart4_root_clk 0 0 0 24000000
0 0 50000
uart2 1 1 0 24000000
0 0 50000
uart2_root_clk 4 4 0 24000000
0 0 50000
If that seems reasonable to people, I'll push it as a real patch.
adam
>
> commit 9461f7b33d11cbbf5ce79c3c03d0da9d42dfce92
> Author: Jerome Brunet <jbrunet at baylibre.com>
> Date: Tue Jun 19 15:40:51 2018 +0200
>
> clk: fix CLK_SET_RATE_GATE with clock rate protection
>
> CLK_SET_RATE_GATE should prevent any operation which may result in a rate
> change or glitch while the clock is prepared/enabled.
>
> IOW, the following sequence is not allowed anymore with CLK_SET_RATE_GATE:
> * clk_get()
> * clk_prepare_enable()
> * clk_get_rate()
> * clk_set_rate()
>
> At the moment this is enforced on the leaf clock of the operation, not
> along the tree. This problematic because, if a PLL has the CLK_RATE_GATE,
> it won't be enforced if the clk_set_rate() is called on its child clocks.
>
> Using clock rate protection, we can now enforce CLK_SET_RATE_GATE along the
> clock tree
>
> Acked-by: Linus Walleij <linus.walleij at linaro.org>
> Tested-by: Quentin Schulz <quentin.schulz at free-electrons.com>
> Tested-by: Maxime Ripard <maxime.ripard at free-electrons.com>
> Signed-off-by: Jerome Brunet <jbrunet at baylibre.com>
> Signed-off-by: Michael Turquette <mturquette at baylibre.com>
> Link: lkml.kernel.org/r/20180619134051.16726-3-jbrunet at baylibre.com
>
> >
> > > > I jumped ahead to linux-next to see if there were any upstream fixes
> > > > implemented but it throws the same errors.
> > > >
> > > > If anyone has any ideas, I'd like to try them. If not, I'll spend
> > > > some time bisecting.
> > >
> > > Are you aware of this issue?
> > > Any suggestions?
> > >
> > > Regards
> > > Aisheng
> > >
> > > >
> > > > adam
More information about the linux-arm-kernel
mailing list