[PATCH] ARM:IMX6Q:use pll2_pfd2_396m as the enfc_sel's parent

[PATCH] ARM:IMX6Q:use pll2_pfd2_396m as the enfc_sel's parent

[Huang Shijie]

The gpmi-nand driver can support the ONFI nand chip's EDO (extra data out)
mode in the asynchrounous mode. In the asynchrounous mode 5, the gpmi
needs 100MHz clock for the IO. But with the pll2_pfd0_352m, we can not
get the 100MHz clock.

So choose pll2_pfd2_396m as enfc_sel's parent.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 arch/arm/mach-imx/clk-imx6q.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 4233d9e..58705b6 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -440,6 +440,13 @@ int __init mx6q_clocks_init(void)
 	clk_register_clkdev(clk[ahb], "ahb", NULL);
 	clk_register_clkdev(clk[cko1], "cko1", NULL);
 
+	/*
+	 * The gpmi needs 100MHz frequency in the EDO/Sync mode,
+	 * We can not get the 100MHz from the pll2_pfd0_352m.
+	 * So choose pll2_pfd2_396m as enfc_sel's parent.
+	 */
+	clk_set_parent(clk[enfc_sel], clk[pll2_pfd2_396m]);
+
 	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
 		clk_prepare_enable(clk[clks_init_on[i]]);
 
-- 
1.7.0.4

[PATCH] ARM:IMX6Q:use pll2_pfd2_396m as the enfc_sel's parent

[Shawn Guo]

On Mon, Sep 10, 2012 at 03:17:56PM +0800, Huang Shijie wrote:
> The gpmi-nand driver can support the ONFI nand chip's EDO (extra data out)
> mode in the asynchrounous mode. In the asynchrounous mode 5, the gpmi
> needs 100MHz clock for the IO. But with the pll2_pfd0_352m, we can not
> get the 100MHz clock.
> 
> So choose pll2_pfd2_396m as enfc_sel's parent.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>

Applied, thanks.

But we really expect that clock framework can get improved to have such
case handled in clk_set_rate() call.

Shawn

> ---
>  arch/arm/mach-imx/clk-imx6q.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> index 4233d9e..58705b6 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -440,6 +440,13 @@ int __init mx6q_clocks_init(void)
>  	clk_register_clkdev(clk[ahb], "ahb", NULL);
>  	clk_register_clkdev(clk[cko1], "cko1", NULL);
>  
> +	/*
> +	 * The gpmi needs 100MHz frequency in the EDO/Sync mode,
> +	 * We can not get the 100MHz from the pll2_pfd0_352m.
> +	 * So choose pll2_pfd2_396m as enfc_sel's parent.
> +	 */
> +	clk_set_parent(clk[enfc_sel], clk[pll2_pfd2_396m]);
> +
>  	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
>  		clk_prepare_enable(clk[clks_init_on[i]]);
>  
> -- 
> 1.7.0.4
> 
> 

[PATCH] ARM:IMX6Q:use pll2_pfd2_396m as the enfc_sel's parent

[Sascha Hauer]

On Wed, Sep 19, 2012 at 01:33:50PM +0800, Shawn Guo wrote:
> On Mon, Sep 10, 2012 at 03:17:56PM +0800, Huang Shijie wrote:
> > The gpmi-nand driver can support the ONFI nand chip's EDO (extra data out)
> > mode in the asynchrounous mode. In the asynchrounous mode 5, the gpmi
> > needs 100MHz clock for the IO. But with the pll2_pfd0_352m, we can not
> > get the 100MHz clock.
> > 
> > So choose pll2_pfd2_396m as enfc_sel's parent.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> 
> Applied, thanks.
> 
> But we really expect that clock framework can get improved to have such
> case handled in clk_set_rate() call.

How do you want to archieve that? It would be possible to teach the
clock framework to iterate over the possible parents and see what gives
the best result, but how do we teach the clock framework that there are
other constraints, like for example 'use this clock in low power mode
and that one otherwise'?

We have a similar problem with the IPU: When the LDB is used, the IPU
pixel clock has to be switched to the LDB clock, even if other clocks
would result in a more accurate frequency.

That's a topic for a long long discussion, I think even longer than it
took to introduce the clock framework ;)

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH] ARM:IMX6Q:use pll2_pfd2_396m as the enfc_sel's parent

[Shawn Guo]

On Wed, Sep 19, 2012 at 09:31:30AM +0200, Sascha Hauer wrote:
> On Wed, Sep 19, 2012 at 01:33:50PM +0800, Shawn Guo wrote:
> > On Mon, Sep 10, 2012 at 03:17:56PM +0800, Huang Shijie wrote:
> > > The gpmi-nand driver can support the ONFI nand chip's EDO (extra data out)
> > > mode in the asynchrounous mode. In the asynchrounous mode 5, the gpmi
> > > needs 100MHz clock for the IO. But with the pll2_pfd0_352m, we can not
> > > get the 100MHz clock.
> > > 
> > > So choose pll2_pfd2_396m as enfc_sel's parent.
> > > 
> > > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > 
> > Applied, thanks.
> > 
> > But we really expect that clock framework can get improved to have such
> > case handled in clk_set_rate() call.
> 
> How do you want to archieve that?

Actually, I'm relying on Mike for that, as he said a patch for that
is under hacking [1].

> It would be possible to teach the
> clock framework to iterate over the possible parents and see what gives
> the best result, but how do we teach the clock framework that there are
> other constraints, like for example 'use this clock in low power mode
> and that one otherwise'?
> 
I have no idea, and I'm not even sure Mike's patch will address such
constraints.  Mike?

Shawn

> We have a similar problem with the IPU: When the LDB is used, the IPU
> pixel clock has to be switched to the LDB clock, even if other clocks
> would result in a more accurate frequency.
> 
> That's a topic for a long long discussion, I think even longer than it
> took to introduce the clock framework ;)
> 

[1] http://thread.gmane.org/gmane.linux.ports.tegra/6196/focus=6198

[PATCH] ARM:IMX6Q:use pll2_pfd2_396m as the enfc_sel's parent

[Mike Turquette]

On Wed, Sep 19, 2012 at 6:44 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Wed, Sep 19, 2012 at 09:31:30AM +0200, Sascha Hauer wrote:
>> On Wed, Sep 19, 2012 at 01:33:50PM +0800, Shawn Guo wrote:
>> > On Mon, Sep 10, 2012 at 03:17:56PM +0800, Huang Shijie wrote:
>> > > The gpmi-nand driver can support the ONFI nand chip's EDO (extra data out)
>> > > mode in the asynchrounous mode. In the asynchrounous mode 5, the gpmi
>> > > needs 100MHz clock for the IO. But with the pll2_pfd0_352m, we can not
>> > > get the 100MHz clock.
>> > >
>> > > So choose pll2_pfd2_396m as enfc_sel's parent.
>> > >
>> > > Signed-off-by: Huang Shijie <b32955@freescale.com>
>> >
>> > Applied, thanks.
>> >
>> > But we really expect that clock framework can get improved to have such
>> > case handled in clk_set_rate() call.
>>
>> How do you want to archieve that?
>
> Actually, I'm relying on Mike for that, as he said a patch for that
> is under hacking [1].
>

Hmm, I need to re-prioritize rebasing this patch...

>> It would be possible to teach the
>> clock framework to iterate over the possible parents and see what gives
>> the best result, but how do we teach the clock framework that there are
>> other constraints, like for example 'use this clock in low power mode
>> and that one otherwise'?
>>
> I have no idea, and I'm not even sure Mike's patch will address such
> constraints.  Mike?
>

I'm not sure about the "low power clock configuration" case, but I
have some local code for the OMAP clock port which is in a "clk-sets"
branch.  This code looks at requested clock rates and matches them to
a table which affects many clocks.  Essentially the idea is that an
operating point (OPP) is a combination of both voltage for a rail and
a snapshot of clock configuration for many clocks in a subtree.
Instead of relying on the algorithmic methods for programming a clock
rate (such as in the clk-divider.c implementation or the many
platform-specific PLL .set_rate implementations) this implementation
uses a limited set of predefined allowed clock rates and somewhat
simplifies the concept of programming many clocks for predefined
use-cases.  Unfortunately the code is in terrible shape and not ready
for public review, but I will prioritize it.  This code goes along
nicely with my next crack at solving the generic DVFS problem.

This approach might help the "low power clock configuration" case, but
time will only tell.  I will take into consideration not only a table
listing valid frequencies, but also valid parents which should help
your case.

Regards,
Mike

> Shawn
>
>> We have a similar problem with the IPU: When the LDB is used, the IPU
>> pixel clock has to be switched to the LDB clock, even if other clocks
>> would result in a more accurate frequency.
>>
>> That's a topic for a long long discussion, I think even longer than it
>> took to introduce the clock framework ;)
>>
>
> [1] http://thread.gmane.org/gmane.linux.ports.tegra/6196/focus=6198