Patchwork [09/19] clock-imx35: fix reboot in internal boot mode

login
register
mail settings
Submitter Eric Benard
Date Dec. 13, 2011, 6:31 a.m.
Message ID <1323757911-25217-9-git-send-email-eric@eukrea.com>
Download mbox | patch
Permalink /patch/131012/
State New
Headers show

Comments

Eric Benard - Dec. 13, 2011, 6:31 a.m.
commit 8d75a2620dc3e33ce504044c375c443ed7ed4128 disable IIM
clock after reading silicon revision which will prevent
reboot in internal boot mode (see comment a few line before)

Signed-off-by: Eric Bénard <eric@eukrea.com>
Cc: Jason Liu <jason.hui@linaro.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
---
 arch/arm/mach-imx/clock-imx35.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
Sascha Hauer - Dec. 13, 2011, 10 a.m.
On Tue, Dec 13, 2011 at 07:31:41AM +0100, Eric Bénard wrote:
> commit 8d75a2620dc3e33ce504044c375c443ed7ed4128 disable IIM
> clock after reading silicon revision which will prevent
> reboot in internal boot mode (see comment a few line before)

I'm a bit unsure. The fix you suggest is the least intrusive, but
a proper fix would be to move the mentioned block below the
imx_print_silicon_rev, like this:

	clk_enable(&iim_clk);
	imx_print_silicon_rev("i.MX35", mx35_revision());
	clk_disable(&iim_clk);

	/*
	 * Check if we came up in internal boot mode. If yes, we need
	 * some
	 * extra clocks turned on, otherwise the MX35 boot ROM code will
	 * hang after a watchdog reset.
	 */
	if (!(__raw_readl(CCM_BASE + CCM_RCSR) & (3 << 10))) {
		clk_enable(&iim_clk);
		clk_enable(&uart1_clk);
		clk_enable(&scc_clk);
	}

This would also get the clk enable counters right.

Sascha

> 
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> Cc: Jason Liu <jason.hui@linaro.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> ---
>  arch/arm/mach-imx/clock-imx35.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c
> index 8116f11..b49a732 100644
> --- a/arch/arm/mach-imx/clock-imx35.c
> +++ b/arch/arm/mach-imx/clock-imx35.c
> @@ -538,7 +538,10 @@ int __init mx35_clocks_init()
>  
>  	clk_enable(&iim_clk);
>  	imx_print_silicon_rev("i.MX35", mx35_revision());
> -	clk_disable(&iim_clk);
> +	/* Don't disable IIM clock if we came up in internal boot mode */
> +	if (__raw_readl(CCM_BASE + CCM_RCSR) & (3 << 10)) {
> +		clk_disable(&iim_clk);
> +	}
>  
>  #ifdef CONFIG_MXC_USE_EPIT
>  	epit_timer_init(&epit1_clk,
> -- 
> 1.7.6.4
> 
>
Sergei Shtylyov - Dec. 13, 2011, 10:52 a.m.
Hello.

On 13-12-2011 10:31, Eric Bénard wrote:

> commit 8d75a2620dc3e33ce504044c375c443ed7ed4128

    Please also specify that commit's summary in parens.

 > disable IIM

    Disables?

> clock after reading silicon revision which will prevent
> reboot in internal boot mode (see comment a few line before)

> Signed-off-by: Eric Bénard<eric@eukrea.com>
> Cc: Jason Liu<jason.hui@linaro.org>
> Cc: Sascha Hauer<kernel@pengutronix.de>
> ---
>   arch/arm/mach-imx/clock-imx35.c |    5 ++++-
>   1 files changed, 4 insertions(+), 1 deletions(-)

> diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c
> index 8116f11..b49a732 100644
> --- a/arch/arm/mach-imx/clock-imx35.c
> +++ b/arch/arm/mach-imx/clock-imx35.c
> @@ -538,7 +538,10 @@ int __init mx35_clocks_init()
>
>   	clk_enable(&iim_clk);
>   	imx_print_silicon_rev("i.MX35", mx35_revision());
> -	clk_disable(&iim_clk);
> +	/* Don't disable IIM clock if we came up in internal boot mode */
> +	if (__raw_readl(CCM_BASE + CCM_RCSR) & (3 << 10)) {
> +		clk_disable(&iim_clk);
> +	}

    {} not needed -- scripts/checkpatch.pl should have warned you.

WBR, Sergei
Eric Benard - Dec. 13, 2011, 1:41 p.m.
Hi,

Le 13/12/2011 11:52, Sergei Shtylyov a écrit :
 > {} not needed -- scripts/checkpatch.pl should have warned you.
 >
it failed to do so :
$ ./scripts/checkpatch.pl 0009-clock-imx35-fix-reboot-in-internal-boot-mode.patch
total: 0 errors, 0 warnings, 11 lines checked

0009-clock-imx35-fix-reboot-in-internal-boot-mode.patch has no obvious style 
problems and is ready for submission.

Eric
Eric Benard - Dec. 13, 2011, 1:47 p.m.
Le 13/12/2011 11:00, Sascha Hauer a écrit :
> On Tue, Dec 13, 2011 at 07:31:41AM +0100, Eric Bénard wrote:
>> commit 8d75a2620dc3e33ce504044c375c443ed7ed4128 disable IIM
>> clock after reading silicon revision which will prevent
>> reboot in internal boot mode (see comment a few line before)
>
> I'm a bit unsure. The fix you suggest is the least intrusive, but
> a proper fix would be to move the mentioned block below the
> imx_print_silicon_rev, like this:
>
> 	clk_enable(&iim_clk);
> 	imx_print_silicon_rev("i.MX35", mx35_revision());
> 	clk_disable(&iim_clk);
>
> 	/*
> 	 * Check if we came up in internal boot mode. If yes, we need
> 	 * some
> 	 * extra clocks turned on, otherwise the MX35 boot ROM code will
> 	 * hang after a watchdog reset.
> 	 */
> 	if (!(__raw_readl(CCM_BASE + CCM_RCSR)&  (3<<  10))) {
> 		clk_enable(&iim_clk);
> 		clk_enable(&uart1_clk);
> 		clk_enable(&scc_clk);
> 	}
>
> This would also get the clk enable counters right.
>
OK that's cleaner.
While fixing the patch it seems comment & code are not synced as scc_clk is 
0x3 << 2 and the code was setting 0x3 << 4 which corresponds to sdma_clk (and 
in my configuration, reboot works without any of these clocks forced). I chose 
to have code synced with comment for v2 of the patch.

Eric

Patch

diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c
index 8116f11..b49a732 100644
--- a/arch/arm/mach-imx/clock-imx35.c
+++ b/arch/arm/mach-imx/clock-imx35.c
@@ -538,7 +538,10 @@  int __init mx35_clocks_init()
 
 	clk_enable(&iim_clk);
 	imx_print_silicon_rev("i.MX35", mx35_revision());
-	clk_disable(&iim_clk);
+	/* Don't disable IIM clock if we came up in internal boot mode */
+	if (__raw_readl(CCM_BASE + CCM_RCSR) & (3 << 10)) {
+		clk_disable(&iim_clk);
+	}
 
 #ifdef CONFIG_MXC_USE_EPIT
 	epit_timer_init(&epit1_clk,