diff mbox

clk: imx6ull: use OSC clock during AXI rate change

Message ID 20180418124908.3079-1-stefan@agner.ch (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Stefan Agner April 18, 2018, 12:49 p.m. UTC
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(-)

Comments

Shawn Guo May 2, 2018, 7:38 a.m. UTC | #1
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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefan Agner May 7, 2018, 12:56 p.m. UTC | #2
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
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jacky Bai May 8, 2018, 7:32 a.m. UTC | #3
> 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
> >>
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefan Agner May 8, 2018, 1:20 p.m. UTC | #4
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
>> >>
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd May 8, 2018, 6:19 p.m. UTC | #5
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jacky Bai May 9, 2018, 1:26 a.m. UTC | #6
> 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
Stefan Agner May 9, 2018, 2:12 p.m. UTC | #7
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
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jacky Bai May 10, 2018, 1:53 a.m. UTC | #8
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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]);