2018-04-18 12:50:46

by Stefan Agner

[permalink] [raw]
Subject: [PATCH] clk: imx6ull: use OSC clock during AXI rate change

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 <[email protected]>
---
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



2018-05-02 07:38:48

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH] clk: imx6ull: use OSC clock during AXI rate change

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 <[email protected]>
> ---
> 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
>

2018-05-07 12:56:50

by Stefan Agner

[permalink] [raw]
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 :-(

--
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 <[email protected]>
>> ---
>> 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
>>

2018-05-08 07:33:04

by Jacky Bai

[permalink] [raw]
Subject: RE: [PATCH] clk: imx6ull: use OSC clock during AXI rate change

> 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 <[email protected]>
> >> ---
> >> 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
> >>

2018-05-08 13:20:45

by Stefan Agner

[permalink] [raw]
Subject: Re: [PATCH] clk: imx6ull: use OSC clock during AXI rate change

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 <[email protected]>
>> >> ---
>> >> 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
>> >>

2018-05-08 18:20:10

by Stephen Boyd

[permalink] [raw]
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.

2018-05-09 01:27:35

by Jacky Bai

[permalink] [raw]
Subject: RE: [PATCH] clk: imx6ull: use OSC clock during AXI rate change

> 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

2018-05-09 14:14:26

by Stefan Agner

[permalink] [raw]
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)

Both run at 396MHz in U-Boot, and both freeze and the same location.

--
Stefan

2018-05-10 01:54:20

by Jacky Bai

[permalink] [raw]
Subject: RE: [PATCH] clk: imx6ull: use OSC clock during AXI rate change

> 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