[RFC 1/8] RFC: CLK: OMAP: Add basic infrastructure for OMAP clocks
Mike Turquette
mturquette at linaro.org
Fri Mar 22 16:01:38 EDT 2013
Quoting Tero Kristo (2013-03-22 11:39:45)
> On Fri, 2013-03-22 at 09:47 -0700, Mike Turquette wrote:
> > Quoting Tero Kristo (2013-03-22 01:39:08)
> > > On Thu, 2013-03-21 at 11:50 -0700, Mike Turquette wrote:
> > > > Quoting Tero Kristo (2013-03-21 10:35:40)
> > > > > This patch adds basic infrastructure support for registering clocks
> > > > > under common clock framework. This patch is done in preparation for
> > > > > moving clock data from arch/arm/mach-omap2/ folder under /drivers/clk/omap.
> > > > >
> > > > > Signed-off-by: Tero Kristo <t-kristo at ti.com>
> > > > > Cc: Mike Turquette <mturquette at linaro.org>
> > > >
> > > > Thanks for taking a crack at this Tero! This is a great step towards
> > > > getting rid of clk-private.h.
> > >
> > > Hi Mike,
> > >
> > > Thanks for the quick comments.
> > >
> > > >
> > > > Regarding the long-term roadmap of the omap clock code:
> > > >
> > > > In general all of the changes to the omap clock code for using the
> > > > common struct clk have been very incremental. We still have the old
> > > > legacy struct clk definition, the name of that struct was changed to
> > > > clk_hw_omap, but it is still a fairly large, monolithic structure. Are
> > > > there plans to break the clock types up into smaller, more focused clock
> > > > types? E.g. get rid of struct dpll_data and have a dedicated dpll clock
> > > > type.
> > >
> > > I think this sounds like a fair approach to me, currently the struct is
> > > huge and we register almost everything under it. This RFC set only tries
> > > to tackle with the most immediate problem, removing the dependency to
> > > clk-private.h and moving the clock data out from mach-omap2.
> > >
> > > >
> > > > The question above is not reason to block this patch since it is a
> > > > fairly large refactoring effort, but it is something to consider.
> > > >
> > > > Some comments below.
> > > >
> > > > <snip>
> > > >
> > > > > +struct omap_clk {
> > > > > + u16 cpu;
> > > > > + const char *dev_id;
> > > > > + const char *con_id;
> > > > > + struct omap_init_clk *clk;
> > > > > +};
> > > > > +
> > > > > +#define CLK(dev, con, ck, cp) \
> > > > > + { \
> > > > > + .cpu = cp, \
> > > > > + .dev_id = dev, \
> > > > > + .con_id = con, \
> > > > > + .clk = ck, \
> > > > > + }
> > > > > +
> > > >
> > > > IS omap_clk and CLK really necessary today? I thought that the move to
> > > > separate clocks out by tables got rid of this macro/structure?
> > >
> > > Well, it looks like we have a few clocks which actually use same clock
> > > nodes, like dpll_mpu_ck is declared twice with different dev / con ids
> > > here. It is possible to get rid of this in most cases and incorporate
> > > the data from omap_clk to the omap_init_clk. Maybe for the special cases
> > > we can just add a static clk registration in the code.
> > >
> > > >
> > > > > +#define OMAP_CLK_FIXED_RATE(_name, _flags, _rate, _ignore) \
> > > > > + static struct omap_init_clk _name __initdata = { \
> > > > > + .name = #_name, \
> > > > > + .clk_flags = _flags, \
> > > > > + .rate = _rate, \
> > > > > + .clk_register = omap_clk_register_fixed_rate, \
> > > > > + };
> > > > > +
> > > >
> > > > These macros are unreadable. They were originally born out of the nasty
> > > > clk-private.h stuff which included forward declarations of struct clk
> > > > and all sorts of ugliness. I know it saves you LoC to use them now but
> > > > I really prefer to use designated initializers for the sake of
> > > > readability. Do you plan to convert the OMAP clock code back to how it
> > > > was, pre-macros?
> > >
> > > Yea, I think we should do this eventually once we also split the
> > > different clock types to their own init structs. Then we can get rid of
> > > these macros at the same point. However, I decided not to do this at
> > > this step, as this simple data conversion allows us to convert also
> > > omap2 / omap3 clocks rather easily, for which we don't have auto
> > > generation available.
> > >
> >
> > Cool. As long as there is a plan to clean this stuff up in the future
> > then I'm happy with this incremental approach.
> >
> > Hopefully I'll have time to test this out on my omap hardware next week.
>
> Just a quick note, you need the clock init order patch from Rajendra
> with this set, a working patch set is available in my branch here:
>
Yes, I know. I plan to take that patch and this series and remove
clk-private.h and see what happens.
Regards,
Mike
> git://gitorious.org/~kristo/omap-pm/omap-pm-work.git
> branch: linux-3.8-omap4-clk-move
>
> ... thats on top of 3.8.
>
> -Tero
More information about the linux-arm-kernel
mailing list