* [RFC 0/8]: omap4: clk: move clock data under drivers/clk @ 2013-03-21 17:35 Tero Kristo [not found] ` <1363887347-4686-2-git-send-email-t-kristo@ti.com> 2013-03-22 5:28 ` [RFC 0/8]: omap4: clk: move clock data under drivers/clk Rajendra Nayak 0 siblings, 2 replies; 8+ messages in thread From: Tero Kristo @ 2013-03-21 17:35 UTC (permalink / raw) To: linux-arm-kernel Hi, This is an RFC version of the clock data move under drivers/clk. Tested under 3.8 and boots fine, but don't try this out unless you are experimental sort (I quickly tried with 3.9-rc3 and it failed to boot with that.) The approach taken here has minimal impact on the clock data and should make it easy to transfer clock data for omap2 / omap3 and omap5 also. omap4 was only done as a proof of concept. Any comments appreciated. -Tero ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <1363887347-4686-2-git-send-email-t-kristo@ti.com>]
* [RFC 1/8] RFC: CLK: OMAP: Add basic infrastructure for OMAP clocks [not found] ` <1363887347-4686-2-git-send-email-t-kristo@ti.com> @ 2013-03-21 18:50 ` Mike Turquette 2013-03-22 8:39 ` Tero Kristo 0 siblings, 1 reply; 8+ messages in thread From: Mike Turquette @ 2013-03-21 18:50 UTC (permalink / raw) To: linux-arm-kernel 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@ti.com> > Cc: Mike Turquette <mturquette@linaro.org> Thanks for taking a crack at this Tero! This is a great step towards getting rid of clk-private.h. 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. 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? > +#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? Regards, Mike ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 1/8] RFC: CLK: OMAP: Add basic infrastructure for OMAP clocks 2013-03-21 18:50 ` [RFC 1/8] RFC: CLK: OMAP: Add basic infrastructure for OMAP clocks Mike Turquette @ 2013-03-22 8:39 ` Tero Kristo 2013-03-22 16:47 ` Mike Turquette 0 siblings, 1 reply; 8+ messages in thread From: Tero Kristo @ 2013-03-22 8:39 UTC (permalink / raw) To: linux-arm-kernel 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@ti.com> > > Cc: Mike Turquette <mturquette@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. -Tero ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 1/8] RFC: CLK: OMAP: Add basic infrastructure for OMAP clocks 2013-03-22 8:39 ` Tero Kristo @ 2013-03-22 16:47 ` Mike Turquette 2013-03-22 18:39 ` Tero Kristo 0 siblings, 1 reply; 8+ messages in thread From: Mike Turquette @ 2013-03-22 16:47 UTC (permalink / raw) To: linux-arm-kernel 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@ti.com> > > > Cc: Mike Turquette <mturquette@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. Regards, Mike > -Tero ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 1/8] RFC: CLK: OMAP: Add basic infrastructure for OMAP clocks 2013-03-22 16:47 ` Mike Turquette @ 2013-03-22 18:39 ` Tero Kristo 2013-03-22 20:01 ` Mike Turquette 0 siblings, 1 reply; 8+ messages in thread From: Tero Kristo @ 2013-03-22 18:39 UTC (permalink / raw) To: linux-arm-kernel 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@ti.com> > > > > Cc: Mike Turquette <mturquette@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: git://gitorious.org/~kristo/omap-pm/omap-pm-work.git branch: linux-3.8-omap4-clk-move ... thats on top of 3.8. -Tero ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 1/8] RFC: CLK: OMAP: Add basic infrastructure for OMAP clocks 2013-03-22 18:39 ` Tero Kristo @ 2013-03-22 20:01 ` Mike Turquette 0 siblings, 0 replies; 8+ messages in thread From: Mike Turquette @ 2013-03-22 20:01 UTC (permalink / raw) To: linux-arm-kernel 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@ti.com> > > > > > Cc: Mike Turquette <mturquette@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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 0/8]: omap4: clk: move clock data under drivers/clk 2013-03-21 17:35 [RFC 0/8]: omap4: clk: move clock data under drivers/clk Tero Kristo [not found] ` <1363887347-4686-2-git-send-email-t-kristo@ti.com> @ 2013-03-22 5:28 ` Rajendra Nayak 2013-03-22 9:32 ` Tero Kristo 1 sibling, 1 reply; 8+ messages in thread From: Rajendra Nayak @ 2013-03-22 5:28 UTC (permalink / raw) To: linux-arm-kernel Tero, On Thursday 21 March 2013 11:05 PM, Tero Kristo wrote: > Hi, > > This is an RFC version of the clock data move under drivers/clk. > Tested under 3.8 and boots fine, but don't try this out unless > you are experimental sort (I quickly tried with 3.9-rc3 and it failed to > boot with that.) > > The approach taken here has minimal impact on the clock data > and should make it easy to transfer clock data for omap2 / omap3 and > omap5 also. omap4 was only done as a proof of concept. > > Any comments appreciated. I strangely only see the cover letter delivered to me and no patches. Do you have a branch with the patches which you can share? regards, Rajendra > > -Tero > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 0/8]: omap4: clk: move clock data under drivers/clk 2013-03-22 5:28 ` [RFC 0/8]: omap4: clk: move clock data under drivers/clk Rajendra Nayak @ 2013-03-22 9:32 ` Tero Kristo 0 siblings, 0 replies; 8+ messages in thread From: Tero Kristo @ 2013-03-22 9:32 UTC (permalink / raw) To: linux-arm-kernel On Fri, 2013-03-22 at 10:58 +0530, Rajendra Nayak wrote: > Tero, > > On Thursday 21 March 2013 11:05 PM, Tero Kristo wrote: > > Hi, > > > > This is an RFC version of the clock data move under drivers/clk. > > Tested under 3.8 and boots fine, but don't try this out unless > > you are experimental sort (I quickly tried with 3.9-rc3 and it failed to > > boot with that.) > > > > The approach taken here has minimal impact on the clock data > > and should make it easy to transfer clock data for omap2 / omap3 and > > omap5 also. omap4 was only done as a proof of concept. > > > > Any comments appreciated. > > I strangely only see the cover letter delivered to me and no > patches. Do you have a branch with the patches which you can > share? Keerthy had a similar question. It looks like TI people have had problems with mailing list subscriptions as of late, and I am actually also getting moderator bounces for my emails from linux-arm-kernel list atm. Anyway, I just pushed a branch here: git://gitorious.org/~kristo/omap-pm/omap-pm-work.git branch: linux-3.8-omap4-clk-move -Tero ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-03-22 20:01 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-21 17:35 [RFC 0/8]: omap4: clk: move clock data under drivers/clk Tero Kristo [not found] ` <1363887347-4686-2-git-send-email-t-kristo@ti.com> 2013-03-21 18:50 ` [RFC 1/8] RFC: CLK: OMAP: Add basic infrastructure for OMAP clocks Mike Turquette 2013-03-22 8:39 ` Tero Kristo 2013-03-22 16:47 ` Mike Turquette 2013-03-22 18:39 ` Tero Kristo 2013-03-22 20:01 ` Mike Turquette 2013-03-22 5:28 ` [RFC 0/8]: omap4: clk: move clock data under drivers/clk Rajendra Nayak 2013-03-22 9:32 ` Tero Kristo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).