| Message ID | 1340879846-12900-5-git-send-email-pgaikwad@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 20120628-16:07, Prashant Gaikwad wrote: > Add Tegra platform specific clock structure clk_tegra and > some helper functions for generic clock framework. > > struct clk_tegra is the single strcture used for all types of > clocks. reset and cfg_ex ops moved to clk_tegra from clk_ops. > > Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com> Hi Prashant, I'm happy to see Tegra getting ported over to the common clk framework. <snip> > +void tegra_clk_add(struct clk *clk) > +{ > + struct clk_tegra *c = to_clk_tegra(__clk_get_hw(clk)); > + > + mutex_lock(&clock_list_lock); > + list_add(&c->node, &clocks); > + mutex_unlock(&clock_list_lock); > +} > + > +struct clk *tegra_get_clock_by_name(const char *name) > +{ > + struct clk_tegra *c; > + struct clk *ret = NULL; > + mutex_lock(&clock_list_lock); > + list_for_each_entry(c, &clocks, node) { > + if (strcmp(__clk_get_name(c->hw.clk), name) == 0) { > + ret = c->hw.clk; > + break; > + } > + } > + mutex_unlock(&clock_list_lock); > + return ret; > +} > + Are you planning to continue using an internal list of struct clk_tegra? OMAP had a similar list for the legacy clock framework but that was since removed and now lookups are done solely through clkdev. <snip> > +void tegra_periph_reset_deassert(struct clk *c) > +{ > + struct clk_tegra *clk = to_clk_tegra(__clk_get_hw(c)); > + BUG_ON(!clk->reset); > + clk->reset(__clk_get_hw(c), false); > +} > +EXPORT_SYMBOL(tegra_periph_reset_deassert); > + > +void tegra_periph_reset_assert(struct clk *c) > +{ > + struct clk_tegra *clk = to_clk_tegra(__clk_get_hw(c)); > + BUG_ON(!clk->reset); > + clk->reset(__clk_get_hw(c), true); > +} > +EXPORT_SYMBOL(tegra_periph_reset_assert); > + > +/* Several extended clock configuration bits (e.g., clock routing, clock > + * phase control) are included in PLL and peripheral clock source > + * registers. */ > +int tegra_clk_cfg_ex(struct clk *c, enum tegra_clk_ex_param p, u32 setting) > +{ > + int ret = 0; > + struct clk_tegra *clk = to_clk_tegra(__clk_get_hw(c)); > + > + if (!clk->clk_cfg_ex) { > + ret = -ENOSYS; > + goto out; > + } > + ret = clk->clk_cfg_ex(__clk_get_hw(c), p, setting); > + > +out: > + return ret; > +} We had some discussions in the past on your clock reset and external line request operations which you've had to put into struct clk_tegra. Do you need to expose those ops to code in drivers/*? I consider that a reasonable litmus test to start considering if something should be moved into the generic clk.h api. <snip> > +enum clk_state { > + UNINITIALIZED = 0, > + ON, > + OFF, > +}; How is clk_state used? Might it be helpful to you if that was in the generic code in drivers/clk/clk.c? <snip> > +struct clk_tegra { > + /* node for master clocks list */ > + struct list_head node; /* node for list of all clocks */ > + struct clk_lookup lookup; > + struct clk_hw hw; > + > + bool set; > + unsigned long fixed_rate; > + unsigned long max_rate; > + unsigned long min_rate; > + u32 flags; > + const char *name; > + > + enum clk_state state; > + u32 div; > + u32 mul; > + > + u32 reg; > + u32 reg_shift; > + > + struct list_head shared_bus_list; > + > + union { > + struct { > + unsigned int clk_num; > + } periph; > + struct { > + unsigned long input_min; > + unsigned long input_max; > + unsigned long cf_min; > + unsigned long cf_max; > + unsigned long vco_min; > + unsigned long vco_max; > + const struct clk_pll_freq_table *freq_table; > + int lock_delay; > + unsigned long fixed_rate; > + } pll; > + struct { > + u32 sel; > + u32 reg_mask; > + } mux; > + struct { > + struct clk *main; > + struct clk *backup; > + } cpu; > + struct { > + struct list_head node; > + bool enabled; > + unsigned long rate; > + } shared_bus_user; > + } u; > + > + void (*reset)(struct clk_hw *, bool); > + int (*clk_cfg_ex)(struct clk_hw *, enum tegra_clk_ex_param, u32); > +}; I know this is an initial effort but I hope in time that struct clk_tegra can be broken up into separate clk types. It would be even better if some of the Tegra clocks code be converted over to the common clk types in drivers/clk/clk-*.c Regards, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 28 June 2012 11:58 PM, Mike Turquette wrote: > On 20120628-16:07, Prashant Gaikwad wrote: >> Add Tegra platform specific clock structure clk_tegra and >> some helper functions for generic clock framework. >> >> struct clk_tegra is the single strcture used for all types of >> clocks. reset and cfg_ex ops moved to clk_tegra from clk_ops. >> >> Signed-off-by: Prashant Gaikwad<pgaikwad@nvidia.com> > Hi Prashant, > > I'm happy to see Tegra getting ported over to the common clk framework. Thanks Mike!! It will help to get in-line with recent developments. > <snip> >> +void tegra_clk_add(struct clk *clk) >> +{ >> + struct clk_tegra *c = to_clk_tegra(__clk_get_hw(clk)); >> + >> + mutex_lock(&clock_list_lock); >> + list_add(&c->node,&clocks); >> + mutex_unlock(&clock_list_lock); >> +} >> + >> +struct clk *tegra_get_clock_by_name(const char *name) >> +{ >> + struct clk_tegra *c; >> + struct clk *ret = NULL; >> + mutex_lock(&clock_list_lock); >> + list_for_each_entry(c,&clocks, node) { >> + if (strcmp(__clk_get_name(c->hw.clk), name) == 0) { >> + ret = c->hw.clk; >> + break; >> + } >> + } >> + mutex_unlock(&clock_list_lock); >> + return ret; >> +} >> + > Are you planning to continue using an internal list of struct clk_tegra? > OMAP had a similar list for the legacy clock framework but that was > since removed and now lookups are done solely through clkdev. No, will move to clkdev later. > <snip> >> +void tegra_periph_reset_deassert(struct clk *c) >> +{ >> + struct clk_tegra *clk = to_clk_tegra(__clk_get_hw(c)); >> + BUG_ON(!clk->reset); >> + clk->reset(__clk_get_hw(c), false); >> +} >> +EXPORT_SYMBOL(tegra_periph_reset_deassert); >> + >> +void tegra_periph_reset_assert(struct clk *c) >> +{ >> + struct clk_tegra *clk = to_clk_tegra(__clk_get_hw(c)); >> + BUG_ON(!clk->reset); >> + clk->reset(__clk_get_hw(c), true); >> +} >> +EXPORT_SYMBOL(tegra_periph_reset_assert); >> + >> +/* Several extended clock configuration bits (e.g., clock routing, clock >> + * phase control) are included in PLL and peripheral clock source >> + * registers. */ >> +int tegra_clk_cfg_ex(struct clk *c, enum tegra_clk_ex_param p, u32 setting) >> +{ >> + int ret = 0; >> + struct clk_tegra *clk = to_clk_tegra(__clk_get_hw(c)); >> + >> + if (!clk->clk_cfg_ex) { >> + ret = -ENOSYS; >> + goto out; >> + } >> + ret = clk->clk_cfg_ex(__clk_get_hw(c), p, setting); >> + >> +out: >> + return ret; >> +} > We had some discussions in the past on your clock reset and external > line request operations which you've had to put into struct clk_tegra. > > Do you need to expose those ops to code in drivers/*? I consider that a > reasonable litmus test to start considering if something should be moved > into the generic clk.h api. Yes, we need these ops in drivers. Peter has sent a patch proposing to move these ops to generic clk. In addition, we also need mechanism/ops to change rate and parent from clk_ops implementation. There was some discussion but I do not know the latest status. > <snip> >> +enum clk_state { >> + UNINITIALIZED = 0, >> + ON, >> + OFF, >> +}; > How is clk_state used? Might it be helpful to you if that was in the > generic code in drivers/clk/clk.c? clk_state is used to store the clock state. I am planning to remove this since is_enabled ops should work. > <snip> >> +struct clk_tegra { >> + /* node for master clocks list */ >> + struct list_head node; /* node for list of all clocks */ >> + struct clk_lookup lookup; >> + struct clk_hw hw; >> + >> + bool set; >> + unsigned long fixed_rate; >> + unsigned long max_rate; >> + unsigned long min_rate; >> + u32 flags; >> + const char *name; >> + >> + enum clk_state state; >> + u32 div; >> + u32 mul; >> + >> + u32 reg; >> + u32 reg_shift; >> + >> + struct list_head shared_bus_list; >> + >> + union { >> + struct { >> + unsigned int clk_num; >> + } periph; >> + struct { >> + unsigned long input_min; >> + unsigned long input_max; >> + unsigned long cf_min; >> + unsigned long cf_max; >> + unsigned long vco_min; >> + unsigned long vco_max; >> + const struct clk_pll_freq_table *freq_table; >> + int lock_delay; >> + unsigned long fixed_rate; >> + } pll; >> + struct { >> + u32 sel; >> + u32 reg_mask; >> + } mux; >> + struct { >> + struct clk *main; >> + struct clk *backup; >> + } cpu; >> + struct { >> + struct list_head node; >> + bool enabled; >> + unsigned long rate; >> + } shared_bus_user; >> + } u; >> + >> + void (*reset)(struct clk_hw *, bool); >> + int (*clk_cfg_ex)(struct clk_hw *, enum tegra_clk_ex_param, u32); >> +}; > I know this is an initial effort but I hope in time that struct > clk_tegra can be broken up into separate clk types. It would be even > better if some of the Tegra clocks code be converted over to the common > clk types in drivers/clk/clk-*.c Yes, that is next in my action list. One approach I am working on is to model the clock as Mux -> Divider -> Gate by which we can use some of common clock types. But this will take time. > Regards, > Mike -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 29, 2012 at 1:34 AM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: > On Thursday 28 June 2012 11:58 PM, Mike Turquette wrote: >> We had some discussions in the past on your clock reset and external >> line request operations which you've had to put into struct clk_tegra. >> >> Do you need to expose those ops to code in drivers/*? I consider that a >> reasonable litmus test to start considering if something should be moved >> into the generic clk.h api. > > > Yes, we need these ops in drivers. Peter has sent a patch proposing to move > these ops to generic clk. > > In addition, we also need mechanism/ops to change rate and parent from > clk_ops implementation. There was some discussion but I do not know the > latest status. > Hi Prashant, OK, that is good to know. I haven't forgotten about this topic. I'm still trying to think of the best way to expose less-common operations to drivers... Based on Stephen's feedback in patch 0 I'll be waiting for another version of this series before taking into clk-next. Thanks, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/02/2012 06:09 PM, Turquette, Mike wrote: > On Fri, Jun 29, 2012 at 1:34 AM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: >> On Thursday 28 June 2012 11:58 PM, Mike Turquette wrote: >>> We had some discussions in the past on your clock reset and external >>> line request operations which you've had to put into struct clk_tegra. >>> >>> Do you need to expose those ops to code in drivers/*? I consider that a >>> reasonable litmus test to start considering if something should be moved >>> into the generic clk.h api. >> >> >> Yes, we need these ops in drivers. Peter has sent a patch proposing to move >> these ops to generic clk. >> >> In addition, we also need mechanism/ops to change rate and parent from >> clk_ops implementation. There was some discussion but I do not know the >> latest status. >> > > Hi Prashant, > > OK, that is good to know. I haven't forgotten about this topic. I'm > still trying to think of the best way to expose less-common operations > to drivers... > > Based on Stephen's feedback in patch 0 I'll be waiting for another > version of this series before taking into clk-next. Oh, does this series depend on stuff in clk-next? I was intending to take it through the Tegra tree, since it definitely depends on (in complex ways through context if nothing else due to to large split/rename patches) a bunch of commits in Tegra's for-3.6/common-clk branch. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 2, 2012 at 6:31 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 07/02/2012 06:09 PM, Turquette, Mike wrote: >> On Fri, Jun 29, 2012 at 1:34 AM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: >>> On Thursday 28 June 2012 11:58 PM, Mike Turquette wrote: >>>> We had some discussions in the past on your clock reset and external >>>> line request operations which you've had to put into struct clk_tegra. >>>> >>>> Do you need to expose those ops to code in drivers/*? I consider that a >>>> reasonable litmus test to start considering if something should be moved >>>> into the generic clk.h api. >>> >>> >>> Yes, we need these ops in drivers. Peter has sent a patch proposing to move >>> these ops to generic clk. >>> >>> In addition, we also need mechanism/ops to change rate and parent from >>> clk_ops implementation. There was some discussion but I do not know the >>> latest status. >>> >> >> Hi Prashant, >> >> OK, that is good to know. I haven't forgotten about this topic. I'm >> still trying to think of the best way to expose less-common operations >> to drivers... >> >> Based on Stephen's feedback in patch 0 I'll be waiting for another >> version of this series before taking into clk-next. > > Oh, does this series depend on stuff in clk-next? I was intending to > take it through the Tegra tree, since it definitely depends on (in > complex ways through context if nothing else due to to large > split/rename patches) a bunch of commits in Tegra's for-3.6/common-clk > branch. Oops. I just completely zoned out and went into "clk-next maintenance mode". Yeah you had already mentioned that this was for the Tegra tree. Nothing to see here, move along! Regards, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-tegra/clock.c b/arch/arm/mach-tegra/clock.c index 58f981c..ef9b494 100644 --- a/arch/arm/mach-tegra/clock.c +++ b/arch/arm/mach-tegra/clock.c @@ -1,6 +1,7 @@ /* * * Copyright (C) 2010 Google, Inc. + * Copyright (c) 2012 NVIDIA CORPORATION. All rights reserved. * * Author: * Colin Cross <ccross@google.com> @@ -62,6 +63,7 @@ static DEFINE_MUTEX(clock_list_lock); static LIST_HEAD(clocks); +#ifndef CONFIG_COMMON_CLK struct clk *tegra_get_clock_by_name(const char *name) { struct clk *c; @@ -668,5 +670,127 @@ err_out: debugfs_remove_recursive(clk_debugfs_root); return err; } - #endif +#else + +void tegra_clk_add(struct clk *clk) +{ + struct clk_tegra *c = to_clk_tegra(__clk_get_hw(clk)); + + mutex_lock(&clock_list_lock); + list_add(&c->node, &clocks); + mutex_unlock(&clock_list_lock); +} + +struct clk *tegra_get_clock_by_name(const char *name) +{ + struct clk_tegra *c; + struct clk *ret = NULL; + mutex_lock(&clock_list_lock); + list_for_each_entry(c, &clocks, node) { + if (strcmp(__clk_get_name(c->hw.clk), name) == 0) { + ret = c->hw.clk; + break; + } + } + mutex_unlock(&clock_list_lock); + return ret; +} + +static int tegra_clk_init_one_from_table(struct tegra_clk_init_table *table) +{ + struct clk *c; + struct clk *p; + struct clk *parent; + + int ret = 0; + + c = tegra_get_clock_by_name(table->name); + + if (!c) { + pr_warn("Unable to initialize clock %s\n", + table->name); + return -ENODEV; + } + + parent = clk_get_parent(c); + + if (table->parent) { + p = tegra_get_clock_by_name(table->parent); + if (!p) { + pr_warn("Unable to find parent %s of clock %s\n", + table->parent, table->name); + return -ENODEV; + } + + if (parent != p) { + ret = clk_set_parent(c, p); + if (ret) { + pr_warn("Unable to set parent %s of clock %s: %d\n", + table->parent, table->name, ret); + return -EINVAL; + } + } + } + + if (table->rate && table->rate != clk_get_rate(c)) { + ret = clk_set_rate(c, table->rate); + if (ret) { + pr_warn("Unable to set clock %s to rate %lu: %d\n", + table->name, table->rate, ret); + return -EINVAL; + } + } + + if (table->enabled) { + ret = clk_prepare_enable(c); + if (ret) { + pr_warn("Unable to enable clock %s: %d\n", + table->name, ret); + return -EINVAL; + } + } + + return 0; +} + +void tegra_clk_init_from_table(struct tegra_clk_init_table *table) +{ + for (; table->name; table++) + tegra_clk_init_one_from_table(table); +} + +void tegra_periph_reset_deassert(struct clk *c) +{ + struct clk_tegra *clk = to_clk_tegra(__clk_get_hw(c)); + BUG_ON(!clk->reset); + clk->reset(__clk_get_hw(c), false); +} +EXPORT_SYMBOL(tegra_periph_reset_deassert); + +void tegra_periph_reset_assert(struct clk *c) +{ + struct clk_tegra *clk = to_clk_tegra(__clk_get_hw(c)); + BUG_ON(!clk->reset); + clk->reset(__clk_get_hw(c), true); +} +EXPORT_SYMBOL(tegra_periph_reset_assert); + +/* Several extended clock configuration bits (e.g., clock routing, clock + * phase control) are included in PLL and peripheral clock source + * registers. */ +int tegra_clk_cfg_ex(struct clk *c, enum tegra_clk_ex_param p, u32 setting) +{ + int ret = 0; + struct clk_tegra *clk = to_clk_tegra(__clk_get_hw(c)); + + if (!clk->clk_cfg_ex) { + ret = -ENOSYS; + goto out; + } + ret = clk->clk_cfg_ex(__clk_get_hw(c), p, setting); + +out: + return ret; +} +#endif /* !CONFIG_COMMON_CLK */ diff --git a/arch/arm/mach-tegra/clock.h b/arch/arm/mach-tegra/clock.h index bc30065..f4d32ba 100644 --- a/arch/arm/mach-tegra/clock.h +++ b/arch/arm/mach-tegra/clock.h @@ -2,6 +2,7 @@ * arch/arm/mach-tegra/include/mach/clock.h * * Copyright (C) 2010 Google, Inc. + * Copyright (c) 2012 NVIDIA CORPORATION. All rights reserved. * * Author: * Colin Cross <ccross@google.com> @@ -20,6 +21,7 @@ #ifndef __MACH_TEGRA_CLOCK_H #define __MACH_TEGRA_CLOCK_H +#include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/list.h> #include <linux/spinlock.h> @@ -54,6 +56,11 @@ struct clk; +#ifdef CONFIG_COMMON_CLK +struct clk_tegra; +#define to_clk_tegra(_hw) container_of(_hw, struct clk_tegra, hw) +#endif + struct clk_mux_sel { struct clk *input; u32 value; @@ -68,6 +75,13 @@ struct clk_pll_freq_table { u8 cpcon; }; +enum clk_state { + UNINITIALIZED = 0, + ON, + OFF, +}; + +#ifndef CONFIG_COMMON_CLK struct clk_ops { void (*init)(struct clk *); int (*enable)(struct clk *); @@ -80,12 +94,6 @@ struct clk_ops { enum tegra_clk_ex_param, u32); }; -enum clk_state { - UNINITIALIZED = 0, - ON, - OFF, -}; - struct clk { /* node for master clocks list */ struct list_head node; /* node for list of all clocks */ @@ -147,6 +155,65 @@ struct clk { spinlock_t spinlock; }; +#else + +struct clk_tegra { + /* node for master clocks list */ + struct list_head node; /* node for list of all clocks */ + struct clk_lookup lookup; + struct clk_hw hw; + + bool set; + unsigned long fixed_rate; + unsigned long max_rate; + unsigned long min_rate; + u32 flags; + const char *name; + + enum clk_state state; + u32 div; + u32 mul; + + u32 reg; + u32 reg_shift; + + struct list_head shared_bus_list; + + union { + struct { + unsigned int clk_num; + } periph; + struct { + unsigned long input_min; + unsigned long input_max; + unsigned long cf_min; + unsigned long cf_max; + unsigned long vco_min; + unsigned long vco_max; + const struct clk_pll_freq_table *freq_table; + int lock_delay; + unsigned long fixed_rate; + } pll; + struct { + u32 sel; + u32 reg_mask; + } mux; + struct { + struct clk *main; + struct clk *backup; + } cpu; + struct { + struct list_head node; + bool enabled; + unsigned long rate; + } shared_bus_user; + } u; + + void (*reset)(struct clk_hw *, bool); + int (*clk_cfg_ex)(struct clk_hw *, enum tegra_clk_ex_param, u32); +}; +#endif /* !CONFIG_COMMON_CLK */ + struct clk_duplicate { const char *name; struct clk_lookup lookup; @@ -159,13 +226,16 @@ struct tegra_clk_init_table { bool enabled; }; +#ifndef CONFIG_COMMON_CLK +void clk_init(struct clk *clk); +unsigned long clk_get_rate_locked(struct clk *c); +int clk_set_rate_locked(struct clk *c, unsigned long rate); +int clk_reparent(struct clk *c, struct clk *parent); +#endif /* !CONFIG_COMMON_CLK */ + void tegra2_init_clocks(void); void tegra30_init_clocks(void); -void clk_init(struct clk *clk); struct clk *tegra_get_clock_by_name(const char *name); -int clk_reparent(struct clk *c, struct clk *parent); void tegra_clk_init_from_table(struct tegra_clk_init_table *table); -unsigned long clk_get_rate_locked(struct clk *c); -int clk_set_rate_locked(struct clk *c, unsigned long rate); #endif diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c index 204a5c8..c91a1b2 100644 --- a/arch/arm/mach-tegra/common.c +++ b/arch/arm/mach-tegra/common.c @@ -149,6 +149,8 @@ void __init tegra30_init_early(void) void __init tegra_init_late(void) { +#ifndef CONFIG_COMMON_CLK tegra_clk_debugfs_init(); +#endif tegra_powergate_debugfs_init(); } diff --git a/arch/arm/mach-tegra/include/mach/clk.h b/arch/arm/mach-tegra/include/mach/clk.h index d97e403..95f3a54 100644 --- a/arch/arm/mach-tegra/include/mach/clk.h +++ b/arch/arm/mach-tegra/include/mach/clk.h @@ -34,7 +34,10 @@ enum tegra_clk_ex_param { void tegra_periph_reset_deassert(struct clk *c); void tegra_periph_reset_assert(struct clk *c); +#ifndef CONFIG_COMMON_CLK unsigned long clk_get_rate_all_locked(struct clk *c); +#endif + void tegra2_sdmmc_tap_delay(struct clk *c, int delay); int tegra_clk_cfg_ex(struct clk *c, enum tegra_clk_ex_param p, u32 setting);
Add Tegra platform specific clock structure clk_tegra and some helper functions for generic clock framework. struct clk_tegra is the single strcture used for all types of clocks. reset and cfg_ex ops moved to clk_tegra from clk_ops. Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com> --- arch/arm/mach-tegra/clock.c | 126 +++++++++++++++++++++++++++++++- arch/arm/mach-tegra/clock.h | 90 ++++++++++++++++++++--- arch/arm/mach-tegra/common.c | 2 + arch/arm/mach-tegra/include/mach/clk.h | 3 + 4 files changed, 210 insertions(+), 11 deletions(-)