All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>
To: alex.popov@linux.com
Cc: Gerhard Sittig <gsi@denx.de>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: Disabled LocalPlus Controller (LPC) clock on MPC512x
Date: Tue, 02 Dec 2014 11:47:59 +0100	[thread overview]
Message-ID: <547D98DF.9000708@sirius-es.it> (raw)
In-Reply-To: <5475BE5A.4090503@linux.com>

On 26/11/2014 12:49, Alexander Popov wrote:
> Hello.
>
Hi.

> My Freescale TWR-MPC5125 board instantly reboots if I touch
> any physical address on the LocalPlus Bus (LPB) for the first time
> when Linux has already booted.
>
> This effect is reproduced by using /dev/mem or loading a kernel module
> which works with any peripherals on LPB.
>
> It took me some time to find out that such crash is caused by
> clk_disable_unused() in drivers/clk/clk.c, which disables
> LocalPlus Controller (LPC) clock if I don't touch LPB addresses in the
> previous initcalls. So starting Linux with clk_ignore_unused bootparam
> or inserting dummy LPB reading to some initcall is a temporary fix.
>
> Is it correct to gate LPC clock?
For me yes, because it's physically present on SoC and could not be muxed.

> If yes, how to avoid the mentioned
> crashes properly?
>
> There's a piece of code in arch/powerpc/platforms/512x/clock-commonclk.c
> which is doubtful for me:
>
> /*
>  * pre-enable those "internal" clock items which never get
>  * claimed by any peripheral driver, to not have the clock
>  * subsystem disable them late at startup
>  */
> clk_prepare_enable(clks[MPC512x_CLK_DUMMY]);
> clk_prepare_enable(clks[MPC512x_CLK_E300]);    /* PowerPC CPU */
> clk_prepare_enable(clks[MPC512x_CLK_DDR]);    /* DRAM */
> clk_prepare_enable(clks[MPC512x_CLK_MEM]);    /* SRAM */
> clk_prepare_enable(clks[MPC512x_CLK_IPS]);    /* SoC periph */
> clk_prepare_enable(clks[MPC512x_CLK_LPC]);    /* boot media */
>
> Does it mean that these clocks should be registered with
> CLK_IGNORE_UNUSED flag?
>
Yes, in my opinion this groups of clocks may be considered as "always on",
but I don't think that MPC512x_CLK_LPC is an "internal" clock and then 
It could be enable if really used only.
In detail:
   - may be good to enable MPC512x_CLK_LPC only when localbus is enabled 
by the dts
   - if enabled, MPC512x_CLK_LPC have to setup with CLK_IGNORE_UNUSED 
flag because never get claimed by any driver.

I put in CC "Gerhard Sittig" also beacuse it might be interesting to 
know his point of view as the author of mpc512x common clock driver.

Regards,
Matteo

> Thanks a lot.
> Best regards,
> Alexander
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

WARNING: multiple messages have this Message-ID (diff)
From: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>
To: alex.popov@linux.com
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Gerhard Sittig <gsi@denx.de>
Subject: Re: Disabled LocalPlus Controller (LPC) clock on MPC512x
Date: Tue, 02 Dec 2014 11:47:59 +0100	[thread overview]
Message-ID: <547D98DF.9000708@sirius-es.it> (raw)
In-Reply-To: <5475BE5A.4090503@linux.com>

On 26/11/2014 12:49, Alexander Popov wrote:
> Hello.
>
Hi.

> My Freescale TWR-MPC5125 board instantly reboots if I touch
> any physical address on the LocalPlus Bus (LPB) for the first time
> when Linux has already booted.
>
> This effect is reproduced by using /dev/mem or loading a kernel module
> which works with any peripherals on LPB.
>
> It took me some time to find out that such crash is caused by
> clk_disable_unused() in drivers/clk/clk.c, which disables
> LocalPlus Controller (LPC) clock if I don't touch LPB addresses in the
> previous initcalls. So starting Linux with clk_ignore_unused bootparam
> or inserting dummy LPB reading to some initcall is a temporary fix.
>
> Is it correct to gate LPC clock?
For me yes, because it's physically present on SoC and could not be muxed.

> If yes, how to avoid the mentioned
> crashes properly?
>
> There's a piece of code in arch/powerpc/platforms/512x/clock-commonclk.c
> which is doubtful for me:
>
> /*
>  * pre-enable those "internal" clock items which never get
>  * claimed by any peripheral driver, to not have the clock
>  * subsystem disable them late at startup
>  */
> clk_prepare_enable(clks[MPC512x_CLK_DUMMY]);
> clk_prepare_enable(clks[MPC512x_CLK_E300]);    /* PowerPC CPU */
> clk_prepare_enable(clks[MPC512x_CLK_DDR]);    /* DRAM */
> clk_prepare_enable(clks[MPC512x_CLK_MEM]);    /* SRAM */
> clk_prepare_enable(clks[MPC512x_CLK_IPS]);    /* SoC periph */
> clk_prepare_enable(clks[MPC512x_CLK_LPC]);    /* boot media */
>
> Does it mean that these clocks should be registered with
> CLK_IGNORE_UNUSED flag?
>
Yes, in my opinion this groups of clocks may be considered as "always on",
but I don't think that MPC512x_CLK_LPC is an "internal" clock and then 
It could be enable if really used only.
In detail:
   - may be good to enable MPC512x_CLK_LPC only when localbus is enabled 
by the dts
   - if enabled, MPC512x_CLK_LPC have to setup with CLK_IGNORE_UNUSED 
flag because never get claimed by any driver.

I put in CC "Gerhard Sittig" also beacuse it might be interesting to 
know his point of view as the author of mpc512x common clock driver.

Regards,
Matteo

> Thanks a lot.
> Best regards,
> Alexander
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev


  reply	other threads:[~2014-12-02 10:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-26 11:49 Disabled LocalPlus Controller (LPC) clock on MPC512x Alexander Popov
2014-12-02 10:47 ` Matteo Facchinetti [this message]
2014-12-02 10:47   ` Matteo Facchinetti
2014-12-16 12:00   ` Alexander Popov
2014-12-16 12:00     ` Alexander Popov
2014-12-19  9:38     ` Matteo Facchinetti
2014-12-19  9:38       ` Matteo Facchinetti
2015-02-10 10:36       ` Alexander Popov
2015-02-10 10:36         ` Alexander Popov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=547D98DF.9000708@sirius-es.it \
    --to=matteo.facchinetti@sirius-es.it \
    --cc=alex.popov@linux.com \
    --cc=gsi@denx.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.