From mboxrd@z Thu Jan 1 00:00:00 1970 From: mturquette@linaro.org (Mike Turquette) Date: Fri, 22 Mar 2013 13:01:38 -0700 Subject: [RFC 1/8] RFC: CLK: OMAP: Add basic infrastructure for OMAP clocks In-Reply-To: <1363977585.15426.207.camel@sokoban> References: <1363887347-4686-1-git-send-email-t-kristo@ti.com> <1363887347-4686-2-git-send-email-t-kristo@ti.com> <20130321185051.834.97551@quantum> <1363941548.15426.181.camel@sokoban> <20130322164756.834.65517@quantum> <1363977585.15426.207.camel@sokoban> Message-ID: <20130322200138.834.40123@quantum> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 > > > > > Cc: Mike Turquette > > > > > > > > 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. > > > > > > > > > > > > > > > > > +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