| Message ID | 20180418124908.3079-1-stefan@agner.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jacky, Do you see this problem on i.MX6 ULL? What's your take on Stefan's fix? Shawn On Wed, Apr 18, 2018 at 02:49:08PM +0200, Stefan Agner wrote: > On i.MX6 ULL using PLL3 seems to cause a freeze when setting > the parent to IMX6UL_CLK_PLL3_USB_OTG. This only seems to appear > since commit 6f9575e55632 ("clk: imx: Add CLK_IS_CRITICAL flag > for busy divider and busy mux"), probably because the clock is > now forced to be on. > > Fixes: 6f9575e55632("clk: imx: Add CLK_IS_CRITICAL flag for busy divider and busy mux") > Signed-off-by: Stefan Agner <stefan@agner.ch> > --- > This addresses a regression ssen on v4.17-rc1 where the kernel > boots during clock initialization, see also: > https://patchwork.kernel.org/patch/10295927/ > > drivers/clk/imx/clk-imx6ul.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c > index 114ecbb94ec5..12320118f8de 100644 > --- a/drivers/clk/imx/clk-imx6ul.c > +++ b/drivers/clk/imx/clk-imx6ul.c > @@ -464,7 +464,7 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) > clk_set_rate(clks[IMX6UL_CLK_AHB], 99000000); > > /* Change periph_pre clock to pll2_bus to adjust AXI rate to 264MHz */ > - clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], clks[IMX6UL_CLK_PLL3_USB_OTG]); > + clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], clks[IMX6UL_CLK_OSC]); > clk_set_parent(clks[IMX6UL_CLK_PERIPH], clks[IMX6UL_CLK_PERIPH_CLK2]); > clk_set_parent(clks[IMX6UL_CLK_PERIPH_PRE], clks[IMX6UL_CLK_PLL2_BUS]); > clk_set_parent(clks[IMX6UL_CLK_PERIPH], clks[IMX6UL_CLK_PERIPH_PRE]); > -- > 2.17.0 >
Hi Jacky, On 02.05.2018 09:38, Shawn Guo wrote: > Hi Jacky, > > Do you see this problem on i.MX6 ULL? What's your take on Stefan's fix? Any comment to this? It is 4.17.0-rc4 is out and i.MX 6ULL is still broken :-( -- Stefan > > Shawn > > On Wed, Apr 18, 2018 at 02:49:08PM +0200, Stefan Agner wrote: >> On i.MX6 ULL using PLL3 seems to cause a freeze when setting >> the parent to IMX6UL_CLK_PLL3_USB_OTG. This only seems to appear >> since commit 6f9575e55632 ("clk: imx: Add CLK_IS_CRITICAL flag >> for busy divider and busy mux"), probably because the clock is >> now forced to be on. >> >> Fixes: 6f9575e55632("clk: imx: Add CLK_IS_CRITICAL flag for busy divider and busy mux") >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> --- >> This addresses a regression ssen on v4.17-rc1 where the kernel >> boots during clock initialization, see also: >> https://patchwork.kernel.org/patch/10295927/ >> >> drivers/clk/imx/clk-imx6ul.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c >> index 114ecbb94ec5..12320118f8de 100644 >> --- a/drivers/clk/imx/clk-imx6ul.c >> +++ b/drivers/clk/imx/clk-imx6ul.c >> @@ -464,7 +464,7 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) >> clk_set_rate(clks[IMX6UL_CLK_AHB], 99000000); >> >> /* Change periph_pre clock to pll2_bus to adjust AXI rate to 264MHz */ >> - clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], clks[IMX6UL_CLK_PLL3_USB_OTG]); >> + clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], clks[IMX6UL_CLK_OSC]); >> clk_set_parent(clks[IMX6UL_CLK_PERIPH], clks[IMX6UL_CLK_PERIPH_CLK2]); >> clk_set_parent(clks[IMX6UL_CLK_PERIPH_PRE], clks[IMX6UL_CLK_PLL2_BUS]); >> clk_set_parent(clks[IMX6UL_CLK_PERIPH], clks[IMX6UL_CLK_PERIPH_PRE]); >> -- >> 2.17.0 >>
> Subject: Re: [PATCH] clk: imx6ull: use OSC clock during AXI rate change > > Hi Jacky, > > On 02.05.2018 09:38, Shawn Guo wrote: > > Hi Jacky, > > > > Do you see this problem on i.MX6 ULL? What's your take on Stefan's fix? > > Any comment to this? > > It is 4.17.0-rc4 is out and i.MX 6ULL is still broken :-( > Hi Stefan, I have tried two 6ULL board, I don't meet such issue. System can boot up successfully with commit 6f9575e55632 included. Anyway, the change in this patch is ok for me. it is no harm to the BUS clock change flow. Jacky > -- > Stefan > > > > > Shawn > > > > On Wed, Apr 18, 2018 at 02:49:08PM +0200, Stefan Agner wrote: > >> On i.MX6 ULL using PLL3 seems to cause a freeze when setting the > >> parent to IMX6UL_CLK_PLL3_USB_OTG. This only seems to appear since > >> commit 6f9575e55632 ("clk: imx: Add CLK_IS_CRITICAL flag for busy > >> divider and busy mux"), probably because the clock is now forced to > >> be on. > >> > >> Fixes: 6f9575e55632("clk: imx: Add CLK_IS_CRITICAL flag for busy > >> divider and busy mux") > >> Signed-off-by: Stefan Agner <stefan@agner.ch> > >> --- > >> This addresses a regression ssen on v4.17-rc1 where the kernel boots > >> during clock initialization, see also: > >> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa > >> > tchwork.kernel.org%2Fpatch%2F10295927%2F&data=02%7C01%7Cping.bai% > 40nx > >> > p.com%7C023287ec65034c4db45f08d5b419effb%7C686ea1d3bc2b4c6fa92cd9 > 9c5c > >> > 301635%7C0%7C0%7C636612945852594725&sdata=U0ZGid9ZBey0FXfId2dhZb > hVl8p > >> CcjTiexG3JHYwCA4%3D&reserved=0 > >> > >> drivers/clk/imx/clk-imx6ul.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/clk/imx/clk-imx6ul.c > >> b/drivers/clk/imx/clk-imx6ul.c index 114ecbb94ec5..12320118f8de > >> 100644 > >> --- a/drivers/clk/imx/clk-imx6ul.c > >> +++ b/drivers/clk/imx/clk-imx6ul.c > >> @@ -464,7 +464,7 @@ static void __init imx6ul_clocks_init(struct > device_node *ccm_node) > >> clk_set_rate(clks[IMX6UL_CLK_AHB], 99000000); > >> > >> /* Change periph_pre clock to pll2_bus to adjust AXI rate to 264MHz > */ > >> - clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], > clks[IMX6UL_CLK_PLL3_USB_OTG]); > >> + clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], > >> +clks[IMX6UL_CLK_OSC]); > >> clk_set_parent(clks[IMX6UL_CLK_PERIPH], > clks[IMX6UL_CLK_PERIPH_CLK2]); > >> clk_set_parent(clks[IMX6UL_CLK_PERIPH_PRE], > clks[IMX6UL_CLK_PLL2_BUS]); > >> clk_set_parent(clks[IMX6UL_CLK_PERIPH], > >> clks[IMX6UL_CLK_PERIPH_PRE]); > >> -- > >> 2.17.0 > >>
On 08.05.2018 09:32, Jacky Bai wrote: >> Subject: Re: [PATCH] clk: imx6ull: use OSC clock during AXI rate change >> >> Hi Jacky, >> >> On 02.05.2018 09:38, Shawn Guo wrote: >> > Hi Jacky, >> > >> > Do you see this problem on i.MX6 ULL? What's your take on Stefan's fix? >> >> Any comment to this? >> >> It is 4.17.0-rc4 is out and i.MX 6ULL is still broken :-( >> > Hi Stefan, > > I have tried two 6ULL board, I don't meet such issue. System can boot > up successfully with commit 6f9575e55632 included. > Anyway, the change in this patch is ok for me. it is no harm to the > BUS clock change flow. Hm, that is interesting, maybe it is because we use a different SKU? Or maybe bootloader? I tested here with a 800 MHz clocked variant on a Colibri iMX6ULL using U-Boot 2016.11. -- Stefan > > Jacky >> -- >> Stefan >> >> > >> > Shawn >> > >> > On Wed, Apr 18, 2018 at 02:49:08PM +0200, Stefan Agner wrote: >> >> On i.MX6 ULL using PLL3 seems to cause a freeze when setting the >> >> parent to IMX6UL_CLK_PLL3_USB_OTG. This only seems to appear since >> >> commit 6f9575e55632 ("clk: imx: Add CLK_IS_CRITICAL flag for busy >> >> divider and busy mux"), probably because the clock is now forced to >> >> be on. >> >> >> >> Fixes: 6f9575e55632("clk: imx: Add CLK_IS_CRITICAL flag for busy >> >> divider and busy mux") >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> >> --- >> >> This addresses a regression ssen on v4.17-rc1 where the kernel boots >> >> during clock initialization, see also: >> >> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa >> >> >> tchwork.kernel.org%2Fpatch%2F10295927%2F&data=02%7C01%7Cping.bai% >> 40nx >> >> >> p.com%7C023287ec65034c4db45f08d5b419effb%7C686ea1d3bc2b4c6fa92cd9 >> 9c5c >> >> >> 301635%7C0%7C0%7C636612945852594725&sdata=U0ZGid9ZBey0FXfId2dhZb >> hVl8p >> >> CcjTiexG3JHYwCA4%3D&reserved=0 >> >> >> >> drivers/clk/imx/clk-imx6ul.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/clk/imx/clk-imx6ul.c >> >> b/drivers/clk/imx/clk-imx6ul.c index 114ecbb94ec5..12320118f8de >> >> 100644 >> >> --- a/drivers/clk/imx/clk-imx6ul.c >> >> +++ b/drivers/clk/imx/clk-imx6ul.c >> >> @@ -464,7 +464,7 @@ static void __init imx6ul_clocks_init(struct >> device_node *ccm_node) >> >> clk_set_rate(clks[IMX6UL_CLK_AHB], 99000000); >> >> >> >> /* Change periph_pre clock to pll2_bus to adjust AXI rate to 264MHz >> */ >> >> - clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], >> clks[IMX6UL_CLK_PLL3_USB_OTG]); >> >> + clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], >> >> +clks[IMX6UL_CLK_OSC]); >> >> clk_set_parent(clks[IMX6UL_CLK_PERIPH], >> clks[IMX6UL_CLK_PERIPH_CLK2]); >> >> clk_set_parent(clks[IMX6UL_CLK_PERIPH_PRE], >> clks[IMX6UL_CLK_PLL2_BUS]); >> >> clk_set_parent(clks[IMX6UL_CLK_PERIPH], >> >> clks[IMX6UL_CLK_PERIPH_PRE]); >> >> -- >> >> 2.17.0 >> >>
Quoting Stefan Agner (2018-05-08 06:20:03) > On 08.05.2018 09:32, Jacky Bai wrote: > > > > I have tried two 6ULL board, I don't meet such issue. System can boot > > up successfully with commit 6f9575e55632 included. > > Anyway, the change in this patch is ok for me. it is no harm to the > > BUS clock change flow. > > Hm, that is interesting, maybe it is because we use a different SKU? Or > maybe bootloader? > > I tested here with a 800 MHz clocked variant on a Colibri iMX6ULL using > U-Boot 2016.11. > I'll throw this into fixes today because it fixes something to be useful. It's still interesting to see what's different between the two setups though.
> Subject: Re: [PATCH] clk: imx6ull: use OSC clock during AXI rate change > > Quoting Stefan Agner (2018-05-08 06:20:03) > > On 08.05.2018 09:32, Jacky Bai wrote: > > > > > > I have tried two 6ULL board, I don't meet such issue. System can > > > boot up successfully with commit 6f9575e55632 included. > > > Anyway, the change in this patch is ok for me. it is no harm to the > > > BUS clock change flow. > > > > Hm, that is interesting, maybe it is because we use a different SKU? > > Or maybe bootloader? > > > > I tested here with a 800 MHz clocked variant on a Colibri iMX6ULL > > using U-Boot 2016.11. > > > > I'll throw this into fixes today because it fixes something to be useful. It's still > interesting to see what's different between the two setups though. Thanks. I will find some 800MHz parts and have a try with different bootloader later. Will let you know, when I have some results. Jacky
On 09.05.2018 03:26, Jacky Bai wrote: >> Subject: Re: [PATCH] clk: imx6ull: use OSC clock during AXI rate change >> >> Quoting Stefan Agner (2018-05-08 06:20:03) >> > On 08.05.2018 09:32, Jacky Bai wrote: >> > > >> > > I have tried two 6ULL board, I don't meet such issue. System can >> > > boot up successfully with commit 6f9575e55632 included. >> > > Anyway, the change in this patch is ok for me. it is no harm to the >> > > BUS clock change flow. >> > >> > Hm, that is interesting, maybe it is because we use a different SKU? >> > Or maybe bootloader? >> > >> > I tested here with a 800 MHz clocked variant on a Colibri iMX6ULL >> > using U-Boot 2016.11. >> > >> >> I'll throw this into fixes today because it fixes something to be useful. It's still >> interesting to see what's different between the two setups though. > > Thanks. > > I will find some 800MHz parts and have a try with different bootloader later. > Will let you know, when I have some results. > FWIW, I tried two SKUs here: MCIMX6Z2DVM05AA (528Mhz) MCIMX6Z2CVM08AA (800MHz) Both run at 396MHz in U-Boot, and both freeze and the same location. -- Stefan
> Subject: Re: [PATCH] clk: imx6ull: use OSC clock during AXI rate change > > On 09.05.2018 03:26, Jacky Bai wrote: > >> Subject: Re: [PATCH] clk: imx6ull: use OSC clock during AXI rate > >> change > >> > >> Quoting Stefan Agner (2018-05-08 06:20:03) > >> > On 08.05.2018 09:32, Jacky Bai wrote: > >> > > > >> > > I have tried two 6ULL board, I don't meet such issue. System can > >> > > boot up successfully with commit 6f9575e55632 included. > >> > > Anyway, the change in this patch is ok for me. it is no harm to > >> > > the BUS clock change flow. > >> > > >> > Hm, that is interesting, maybe it is because we use a different SKU? > >> > Or maybe bootloader? > >> > > >> > I tested here with a 800 MHz clocked variant on a Colibri iMX6ULL > >> > using U-Boot 2016.11. > >> > > >> > >> I'll throw this into fixes today because it fixes something to be > >> useful. It's still interesting to see what's different between the two setups > though. > > > > Thanks. > > > > I will find some 800MHz parts and have a try with different bootloader later. > > Will let you know, when I have some results. > > > > FWIW, I tried two SKUs here: > MCIMX6Z2DVM05AA (528Mhz) > MCIMX6Z2CVM08AA (800MHz) > MCIMX6Z or MCIMX6Y ? I think i.MX6ULL should be 6Y. Jacky > Both run at 396MHz in U-Boot, and both freeze and the same location. > > -- > Stefan
diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c index 114ecbb94ec5..12320118f8de 100644 --- a/drivers/clk/imx/clk-imx6ul.c +++ b/drivers/clk/imx/clk-imx6ul.c @@ -464,7 +464,7 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) clk_set_rate(clks[IMX6UL_CLK_AHB], 99000000); /* Change periph_pre clock to pll2_bus to adjust AXI rate to 264MHz */ - clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], clks[IMX6UL_CLK_PLL3_USB_OTG]); + clk_set_parent(clks[IMX6UL_CLK_PERIPH_CLK2_SEL], clks[IMX6UL_CLK_OSC]); clk_set_parent(clks[IMX6UL_CLK_PERIPH], clks[IMX6UL_CLK_PERIPH_CLK2]); clk_set_parent(clks[IMX6UL_CLK_PERIPH_PRE], clks[IMX6UL_CLK_PLL2_BUS]); clk_set_parent(clks[IMX6UL_CLK_PERIPH], clks[IMX6UL_CLK_PERIPH_PRE]);
On i.MX6 ULL using PLL3 seems to cause a freeze when setting the parent to IMX6UL_CLK_PLL3_USB_OTG. This only seems to appear since commit 6f9575e55632 ("clk: imx: Add CLK_IS_CRITICAL flag for busy divider and busy mux"), probably because the clock is now forced to be on. Fixes: 6f9575e55632("clk: imx: Add CLK_IS_CRITICAL flag for busy divider and busy mux") Signed-off-by: Stefan Agner <stefan@agner.ch> --- This addresses a regression ssen on v4.17-rc1 where the kernel boots during clock initialization, see also: https://patchwork.kernel.org/patch/10295927/ drivers/clk/imx/clk-imx6ul.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)