On Friday 17 May 2013, Shawn Guo wrote:
> On Thu, May 16, 2013 at 12:29:28PM +0200, Arnd Bergmann wrote:> > On Thursday 16 May 2013 14:10:46 Jingchang Lu wrote:> > > +config SOC_MVF600> > > > Shouldn't that 'depends on ARCH_MULTI_V7'?> > > SOC_MVF600 is added as a sub-item of ARCH_MXC which already handles the> ARCH_MULTI_* dependency.
Ah, makes sense.
> > Can you describe how much Vybrid is actually like MXC? Do you actually> > use most of the mach-imx code?> > > Jingchang mentioned some IP blocks shared between IMX and Vybrid. Right> now, mvf600 reuses mxc_restart() and some amount of clk code> clk-pllv3.c, clk-gate2.c clk.c etc. in arch/arm/mach-imx.
Ok.
> > Actually I think you should move that driver to drivers/clk and use> > of_clk_init(NULL) to initialize it.> > > The mvf600 clock driver uses a lot of base clk support from mach-imx,> and can not be moved into drivers/clk as a single driver. Right now, in> IMX clock drivers, we call of_clk_init() to only register fixed rate> clocks, since all the other clocks are not represented in device tree.
What are your plans for this in the long run?
> > It would be nice if we could integrate that into the watchdog driver,> > so that driver just registers a pm_restart function when it gets loaded.> > It is a rather small driver, so it would not hurt to always load it.> > > Sound like a good idea. We will consider it as another cleanup task for> mach-imx.
Ok
> > What is the mscm? Shouldn't the boot loader have set this up correctly?> > If you can remove that code from the kernel, you can use the default> > irqchip_init call.> > > Yeah, and if we find another way for l2x0_of_init() call.
Good point. That is something we need to do anyway.
> > > +static const char *mvf600_dt_compat[] __initdata = {> > > + "fsl,mvf600",> > > + NULL,> > > +};> > > +> > > +DT_MACHINE_START(VYBRID_VF6XX, "Freescale Vybrid MVF600 (Device Tree)")> > > + .init_irq = mvf600_init_irq,> > > + .init_time = mvf600_init_time,> > > + .init_machine = mvf600_init_machine,> > > + .dt_compat = mvf600_dt_compat,> > > + .restart = mxc_restart,> > > +MACHINE_END> > > > If we can do all of the above, we can actually remove the entire machine> > descriptor here, since all its members are NULL.> > > Yeah, we understand the goal, and this will be the goal of mach-imx> cleanup.
Ok. I guess we just won't be there for 3.11 then.
Arnd

On Fri, May 17, 2013 at 02:29:52PM +0200, Arnd Bergmann wrote:
> > > Actually I think you should move that driver to drivers/clk and use> > > of_clk_init(NULL) to initialize it.> > > > > The mvf600 clock driver uses a lot of base clk support from mach-imx,> > and can not be moved into drivers/clk as a single driver. Right now, in> > IMX clock drivers, we call of_clk_init() to only register fixed rate> > clocks, since all the other clocks are not represented in device tree.> > What are your plans for this in the long run?>
We can move all the IMX clock drivers into drivers/clk at some point
when necessary. But I do not have a plan to register all the clocks
by merely calling of_clk_init(), because doing that would mean we have
to represent all these clocks in device tree. For imx6q example, it's
about 200 ~ 300 nodes addition to DTB. Device tree maintainers are
against to the idea. They are perfectly fine with having clock driver
in kernel to represent/register these SoC internal clocks to clk
framework.
Shawn

On Friday 17 May 2013 21:06:05 Shawn Guo wrote:
> On Fri, May 17, 2013 at 02:29:52PM +0200, Arnd Bergmann wrote:> > > > Actually I think you should move that driver to drivers/clk and use> > > > of_clk_init(NULL) to initialize it.> > > > > > > The mvf600 clock driver uses a lot of base clk support from mach-imx,> > > and can not be moved into drivers/clk as a single driver. Right now, in> > > IMX clock drivers, we call of_clk_init() to only register fixed rate> > > clocks, since all the other clocks are not represented in device tree.> > > > What are your plans for this in the long run?> > > We can move all the IMX clock drivers into drivers/clk at some point> when necessary. But I do not have a plan to register all the clocks> by merely calling of_clk_init(), because doing that would mean we have> to represent all these clocks in device tree. For imx6q example, it's> about 200 ~ 300 nodes addition to DTB. Device tree maintainers are> against to the idea. They are perfectly fine with having clock driver> in kernel to represent/register these SoC internal clocks to clk> framework.
Can't we move the driver to drivers/clk and have it initialized through
of_clk_init() without representing all clocks in the DT?
For all I can tell, CLK_OF_DECLARE() requires only the clock provider
to be described in DT, but not the actual clocks.
Arnd

On Fri, May 17, 2013 at 03:17:31PM +0200, Arnd Bergmann wrote:
> Can't we move the driver to drivers/clk and have it initialized through> of_clk_init() without representing all clocks in the DT?> > For all I can tell, CLK_OF_DECLARE() requires only the clock provider> to be described in DT, but not the actual clocks.
Clock provider is actually a "clk". For fixed rate clock example,
of_fixed_factor_clk_setup() firstly parses DT to get all the
necessary data and then calls clk_register_fixed_factor() to register
the clk to framework, lastly calls of_clk_add_provider() to register the
clk as a provider.
Shawn

On Fri, May 17, 2013 at 03:17:31PM +0200, Arnd Bergmann wrote:
> On Friday 17 May 2013 21:06:05 Shawn Guo wrote:> > On Fri, May 17, 2013 at 02:29:52PM +0200, Arnd Bergmann wrote:> > > > > Actually I think you should move that driver to drivers/clk and use> > > > > of_clk_init(NULL) to initialize it.> > > > > > > > > The mvf600 clock driver uses a lot of base clk support from mach-imx,> > > > and can not be moved into drivers/clk as a single driver. Right now, in> > > > IMX clock drivers, we call of_clk_init() to only register fixed rate> > > > clocks, since all the other clocks are not represented in device tree.> > > > > > What are your plans for this in the long run?> > > > > We can move all the IMX clock drivers into drivers/clk at some point> > when necessary. But I do not have a plan to register all the clocks> > by merely calling of_clk_init(), because doing that would mean we have> > to represent all these clocks in device tree. For imx6q example, it's> > about 200 ~ 300 nodes addition to DTB. Device tree maintainers are> > against to the idea. They are perfectly fine with having clock driver> > in kernel to represent/register these SoC internal clocks to clk> > framework.> > Can't we move the driver to drivers/clk and have it initialized through> of_clk_init() without representing all clocks in the DT?
Sorry. I just get aware of that I misunderstood your comment. Yes,
declaring the clock initialization function with CLK_OF_DECLARE(),
we can use of_clk_init() to have them invoked properly. I just sent a
patch to change imx6 clock driver to do that.
Shawn