Patchwork ARM: imx35: don't disable the uart clock when DEBUG_LL is in use

login
register
mail settings
Submitter Uwe Kleine-König
Date July 31, 2012, 1:44 p.m.
Message ID <1343742279-29570-1-git-send-email-u.kleine-koenig@pengutronix.de>
Download mbox | patch
Permalink /patch/174265/
State New
Headers show

Comments

Uwe Kleine-König - July 31, 2012, 1:44 p.m.
Otherwise printch et al. and printk with earlyprintk and keep_bootcon
becomes unfunctional when the clk framework disables all unused clocks.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/mach-imx/clk-imx35.c |    4 ++++
 1 file changed, 4 insertions(+)
Sascha Hauer - July 31, 2012, 5:43 p.m.
On Tue, Jul 31, 2012 at 03:44:39PM +0200, Uwe Kleine-König wrote:
> Otherwise printch et al. and printk with earlyprintk and keep_bootcon
> becomes unfunctional when the clk framework disables all unused clocks.

Why don't you use the regular console? Letting the clock framework
disable the clocks was pretty much intentional.

Sascha

> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  arch/arm/mach-imx/clk-imx35.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c
> index c6422fb..83d614c 100644
> --- a/arch/arm/mach-imx/clk-imx35.c
> +++ b/arch/arm/mach-imx/clk-imx35.c
> @@ -271,6 +271,10 @@ int __init mx35_clocks_init()
>  	 */
>  	clk_prepare_enable(clk[scc_gate]);
>  
> +	/* Assert that the UART clock keeps running if DEBUG_LL is in use */
> +	if (IS_ENABLED(CONFIG_DEBUG_IMX31_IMX35_UART))
> +		clk_prepare_enable(clk[uart1_gate]);
> +
>  	imx_print_silicon_rev("i.MX35", mx35_revision());
>  
>  #ifdef CONFIG_MXC_USE_EPIT
> -- 
> 1.7.10.4
> 
>
Uwe Kleine-König - July 31, 2012, 6:32 p.m.
Hello Sascha,

On Tue, Jul 31, 2012 at 07:43:58PM +0200, Sascha Hauer wrote:
> On Tue, Jul 31, 2012 at 03:44:39PM +0200, Uwe Kleine-König wrote:
> > Otherwise printch et al. and printk with earlyprintk and keep_bootcon
> > becomes unfunctional when the clk framework disables all unused clocks.
> 
> Why don't you use the regular console? Letting the clock framework
> disable the clocks was pretty much intentional.
I don't question that disabling unused clocks is sensible. But usually
if you enable DEBUG_LL you want to use it even after the clock framework
disabled all (apparently) unused clocks. And in my case the regular
console didn't work yet and so I relied on earlyprintk.
Even if the problem proved to be the result of my stupidity this patch
asserts that earlyprintk just works as it should be for debug aids.

Best regards
Uwe
Uwe Kleine-König - Aug. 6, 2012, 2:24 p.m.
On Tue, Jul 31, 2012 at 08:32:06PM +0200, Uwe Kleine-König wrote:
> Hello Sascha,
> 
> On Tue, Jul 31, 2012 at 07:43:58PM +0200, Sascha Hauer wrote:
> > On Tue, Jul 31, 2012 at 03:44:39PM +0200, Uwe Kleine-König wrote:
> > > Otherwise printch et al. and printk with earlyprintk and keep_bootcon
> > > becomes unfunctional when the clk framework disables all unused clocks.
> > 
> > Why don't you use the regular console? Letting the clock framework
> > disable the clocks was pretty much intentional.
> I don't question that disabling unused clocks is sensible. But usually
> if you enable DEBUG_LL you want to use it even after the clock framework
> disabled all (apparently) unused clocks. And in my case the regular
> console didn't work yet and so I relied on earlyprintk.
> Even if the problem proved to be the result of my stupidity this patch
> asserts that earlyprintk just works as it should be for debug aids.
ping
Sascha Hauer - Aug. 6, 2012, 2:35 p.m.
On Mon, Aug 06, 2012 at 04:24:06PM +0200, Uwe Kleine-König wrote:
> On Tue, Jul 31, 2012 at 08:32:06PM +0200, Uwe Kleine-König wrote:
> > Hello Sascha,
> > 
> > On Tue, Jul 31, 2012 at 07:43:58PM +0200, Sascha Hauer wrote:
> > > On Tue, Jul 31, 2012 at 03:44:39PM +0200, Uwe Kleine-König wrote:
> > > > Otherwise printch et al. and printk with earlyprintk and keep_bootcon
> > > > becomes unfunctional when the clk framework disables all unused clocks.
> > > 
> > > Why don't you use the regular console? Letting the clock framework
> > > disable the clocks was pretty much intentional.
> > I don't question that disabling unused clocks is sensible. But usually
> > if you enable DEBUG_LL you want to use it even after the clock framework
> > disabled all (apparently) unused clocks. And in my case the regular
> > console didn't work yet and so I relied on earlyprintk.
> > Even if the problem proved to be the result of my stupidity this patch
> > asserts that earlyprintk just works as it should be for debug aids.
> ping

Honestly, I don't think this is a good idea. You fixed one UART on one
SoC, so ARRAY_SIZE(i.MX1, i.MX21, i.MX25, i.MX27, i.MX31, i.MX35,
i.MX51, i.MX53, i.MX6) * NUM_UARTS remain broken for this usecase. IMO
DEBUG_LL is to get some stuff out of an otherwise silent board, nothing
more.

Sascha

Patch

diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c
index c6422fb..83d614c 100644
--- a/arch/arm/mach-imx/clk-imx35.c
+++ b/arch/arm/mach-imx/clk-imx35.c
@@ -271,6 +271,10 @@  int __init mx35_clocks_init()
 	 */
 	clk_prepare_enable(clk[scc_gate]);
 
+	/* Assert that the UART clock keeps running if DEBUG_LL is in use */
+	if (IS_ENABLED(CONFIG_DEBUG_IMX31_IMX35_UART))
+		clk_prepare_enable(clk[uart1_gate]);
+
 	imx_print_silicon_rev("i.MX35", mx35_revision());
 
 #ifdef CONFIG_MXC_USE_EPIT