* [PATCH 0/2] clk: misc bits for 3.8 @ 2012-12-04 21:00 Mike Turquette 2012-12-04 21:00 ` [PATCH 1/2] clk: introduce optional disable_unused callback Mike Turquette 2012-12-04 21:00 ` [PATCH 2/2] MAINTAINERS: bad email address for Mike Turquette Mike Turquette 0 siblings, 2 replies; 9+ messages in thread From: Mike Turquette @ 2012-12-04 21:00 UTC (permalink / raw) To: linux-arm-kernel Hi all, Unless there are any objections I plan to add these patches to the clk-next queue for 3.8. If there is discussion of the first patch then I'll hold off until it's ready. Thanks, Mike Mike Turquette (2): clk: introduce optional disable_unused callback MAINTAINERS: bad email address for Mike Turquette MAINTAINERS | 1 - drivers/clk/clk.c | 13 +++++++++++-- include/linux/clk-provider.h | 6 ++++++ 3 files changed, 17 insertions(+), 3 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] clk: introduce optional disable_unused callback 2012-12-04 21:00 [PATCH 0/2] clk: misc bits for 3.8 Mike Turquette @ 2012-12-04 21:00 ` Mike Turquette 2012-12-05 16:13 ` Ulf Hansson 2012-12-06 19:07 ` Linus Walleij 2012-12-04 21:00 ` [PATCH 2/2] MAINTAINERS: bad email address for Mike Turquette Mike Turquette 1 sibling, 2 replies; 9+ messages in thread From: Mike Turquette @ 2012-12-04 21:00 UTC (permalink / raw) To: linux-arm-kernel Some gate clocks have special needs which must be handled during the disable-unused clocks sequence. These needs might be driven by software due to the fact that we're disabling a clock outside of the normal clk_disable path and a clk's enable_count will not be accurate. On the other hand a specific hardware programming sequence might need to be followed for this corner case. This change is needed for the upcoming OMAP port to the common clock framework. Specifically, it is undesirable to treat the disable-unused path identically to the normal clk_disable path since other software layers are involved. In this case OMAP's clockdomain code throws WARNs and bails early due to the clock's enable_count being set to zero. A custom callback mitigates this problem nicely. Cc: Paul Walmsley <paul@pwsan.com> Signed-off-by: Mike Turquette <mturquette@linaro.org> --- drivers/clk/clk.c | 13 +++++++++++-- include/linux/clk-provider.h | 6 ++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 9955ad7..251e45d 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -219,8 +219,17 @@ static void clk_disable_unused_subtree(struct clk *clk) if (clk->flags & CLK_IGNORE_UNUSED) goto unlock_out; - if (__clk_is_enabled(clk) && clk->ops->disable) - clk->ops->disable(clk->hw); + /* + * some gate clocks have special needs during the disable-unused + * sequence. call .disable_unused if available, otherwise fall + * back to .disable + */ + if (__clk_is_enabled(clk)) { + if (clk->ops->disable_unused) + clk->ops->disable_unused(clk->hw); + else if (clk->ops->disable) + clk->ops->disable(clk->hw); + } unlock_out: spin_unlock_irqrestore(&enable_lock, flags); diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 3593a3c..1c94d18 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -57,6 +57,11 @@ struct clk_hw; * This function must not sleep. Optional, if this op is not * set then the enable count will be used. * + * @disable_unused: Disable the clock atomically. Only called from + * clk_disable_unused for gate clocks with special needs. + * Called with enable_lock held. This function must not + * sleep. + * * @recalc_rate Recalculate the rate of this clock, by querying hardware. The * parent rate is an input parameter. It is up to the caller to * ensure that the prepare_mutex is held across this call. @@ -106,6 +111,7 @@ struct clk_ops { int (*enable)(struct clk_hw *hw); void (*disable)(struct clk_hw *hw); int (*is_enabled)(struct clk_hw *hw); + void (*disable_unused)(struct clk_hw *hw); unsigned long (*recalc_rate)(struct clk_hw *hw, unsigned long parent_rate); long (*round_rate)(struct clk_hw *hw, unsigned long, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/2] clk: introduce optional disable_unused callback 2012-12-04 21:00 ` [PATCH 1/2] clk: introduce optional disable_unused callback Mike Turquette @ 2012-12-05 16:13 ` Ulf Hansson 2012-12-05 18:00 ` Mike Turquette 2012-12-06 19:07 ` Linus Walleij 1 sibling, 1 reply; 9+ messages in thread From: Ulf Hansson @ 2012-12-05 16:13 UTC (permalink / raw) To: linux-arm-kernel On 4 December 2012 22:00, Mike Turquette <mturquette@linaro.org> wrote: > Some gate clocks have special needs which must be handled during the > disable-unused clocks sequence. These needs might be driven by software > due to the fact that we're disabling a clock outside of the normal > clk_disable path and a clk's enable_count will not be accurate. On the > other hand a specific hardware programming sequence might need to be > followed for this corner case. > > This change is needed for the upcoming OMAP port to the common clock > framework. Specifically, it is undesirable to treat the disable-unused > path identically to the normal clk_disable path since other software > layers are involved. In this case OMAP's clockdomain code throws WARNs > and bails early due to the clock's enable_count being set to zero. A > custom callback mitigates this problem nicely. > > Cc: Paul Walmsley <paul@pwsan.com> > Signed-off-by: Mike Turquette <mturquette@linaro.org> > --- > drivers/clk/clk.c | 13 +++++++++++-- > include/linux/clk-provider.h | 6 ++++++ > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 9955ad7..251e45d 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -219,8 +219,17 @@ static void clk_disable_unused_subtree(struct clk *clk) > if (clk->flags & CLK_IGNORE_UNUSED) > goto unlock_out; > > - if (__clk_is_enabled(clk) && clk->ops->disable) > - clk->ops->disable(clk->hw); > + /* > + * some gate clocks have special needs during the disable-unused > + * sequence. call .disable_unused if available, otherwise fall > + * back to .disable > + */ > + if (__clk_is_enabled(clk)) { > + if (clk->ops->disable_unused) > + clk->ops->disable_unused(clk->hw); > + else if (clk->ops->disable) > + clk->ops->disable(clk->hw); > + } > > unlock_out: > spin_unlock_irqrestore(&enable_lock, flags); > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index 3593a3c..1c94d18 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -57,6 +57,11 @@ struct clk_hw; > * This function must not sleep. Optional, if this op is not > * set then the enable count will be used. > * > + * @disable_unused: Disable the clock atomically. Only called from > + * clk_disable_unused for gate clocks with special needs. > + * Called with enable_lock held. This function must not > + * sleep. > + * > * @recalc_rate Recalculate the rate of this clock, by querying hardware. The > * parent rate is an input parameter. It is up to the caller to > * ensure that the prepare_mutex is held across this call. > @@ -106,6 +111,7 @@ struct clk_ops { > int (*enable)(struct clk_hw *hw); > void (*disable)(struct clk_hw *hw); > int (*is_enabled)(struct clk_hw *hw); > + void (*disable_unused)(struct clk_hw *hw); > unsigned long (*recalc_rate)(struct clk_hw *hw, > unsigned long parent_rate); > long (*round_rate)(struct clk_hw *hw, unsigned long, > -- > 1.7.9.5 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel Is there a need for updating the clk documentation around this? Anyway, you have my: Acked-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Ulf Hansson ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] clk: introduce optional disable_unused callback 2012-12-05 16:13 ` Ulf Hansson @ 2012-12-05 18:00 ` Mike Turquette 0 siblings, 0 replies; 9+ messages in thread From: Mike Turquette @ 2012-12-05 18:00 UTC (permalink / raw) To: linux-arm-kernel On Wed, Dec 5, 2012 at 8:13 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 4 December 2012 22:00, Mike Turquette <mturquette@linaro.org> wrote: >> Some gate clocks have special needs which must be handled during the >> disable-unused clocks sequence. These needs might be driven by software >> due to the fact that we're disabling a clock outside of the normal >> clk_disable path and a clk's enable_count will not be accurate. On the >> other hand a specific hardware programming sequence might need to be >> followed for this corner case. >> >> This change is needed for the upcoming OMAP port to the common clock >> framework. Specifically, it is undesirable to treat the disable-unused >> path identically to the normal clk_disable path since other software >> layers are involved. In this case OMAP's clockdomain code throws WARNs >> and bails early due to the clock's enable_count being set to zero. A >> custom callback mitigates this problem nicely. >> >> Cc: Paul Walmsley <paul@pwsan.com> >> Signed-off-by: Mike Turquette <mturquette@linaro.org> >> --- >> drivers/clk/clk.c | 13 +++++++++++-- >> include/linux/clk-provider.h | 6 ++++++ >> 2 files changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c >> index 9955ad7..251e45d 100644 >> --- a/drivers/clk/clk.c >> +++ b/drivers/clk/clk.c >> @@ -219,8 +219,17 @@ static void clk_disable_unused_subtree(struct clk *clk) >> if (clk->flags & CLK_IGNORE_UNUSED) >> goto unlock_out; >> >> - if (__clk_is_enabled(clk) && clk->ops->disable) >> - clk->ops->disable(clk->hw); >> + /* >> + * some gate clocks have special needs during the disable-unused >> + * sequence. call .disable_unused if available, otherwise fall >> + * back to .disable >> + */ >> + if (__clk_is_enabled(clk)) { >> + if (clk->ops->disable_unused) >> + clk->ops->disable_unused(clk->hw); >> + else if (clk->ops->disable) >> + clk->ops->disable(clk->hw); >> + } >> >> unlock_out: >> spin_unlock_irqrestore(&enable_lock, flags); >> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h >> index 3593a3c..1c94d18 100644 >> --- a/include/linux/clk-provider.h >> +++ b/include/linux/clk-provider.h >> @@ -57,6 +57,11 @@ struct clk_hw; >> * This function must not sleep. Optional, if this op is not >> * set then the enable count will be used. >> * >> + * @disable_unused: Disable the clock atomically. Only called from >> + * clk_disable_unused for gate clocks with special needs. >> + * Called with enable_lock held. This function must not >> + * sleep. >> + * >> * @recalc_rate Recalculate the rate of this clock, by querying hardware. The >> * parent rate is an input parameter. It is up to the caller to >> * ensure that the prepare_mutex is held across this call. >> @@ -106,6 +111,7 @@ struct clk_ops { >> int (*enable)(struct clk_hw *hw); >> void (*disable)(struct clk_hw *hw); >> int (*is_enabled)(struct clk_hw *hw); >> + void (*disable_unused)(struct clk_hw *hw); >> unsigned long (*recalc_rate)(struct clk_hw *hw, >> unsigned long parent_rate); >> long (*round_rate)(struct clk_hw *hw, unsigned long, >> -- >> 1.7.9.5 >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel at lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > Is there a need for updating the clk documentation around this? > I knew I was forgetting something. The documentation needs a general cleanup effort and this callback should not go undocumented for long. > Anyway, you have my: > > Acked-by: Ulf Hansson <ulf.hansson@linaro.org> > Thanks! Regards, Mike > > Kind regards > Ulf Hansson ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] clk: introduce optional disable_unused callback 2012-12-04 21:00 ` [PATCH 1/2] clk: introduce optional disable_unused callback Mike Turquette 2012-12-05 16:13 ` Ulf Hansson @ 2012-12-06 19:07 ` Linus Walleij 2012-12-06 21:58 ` Ulf Hansson 1 sibling, 1 reply; 9+ messages in thread From: Linus Walleij @ 2012-12-06 19:07 UTC (permalink / raw) To: linux-arm-kernel On Tue, Dec 4, 2012 at 10:00 PM, Mike Turquette <mturquette@linaro.org> wrote: > Some gate clocks have special needs which must be handled during the > disable-unused clocks sequence. These needs might be driven by software > due to the fact that we're disabling a clock outside of the normal > clk_disable path and a clk's enable_count will not be accurate. On the > other hand a specific hardware programming sequence might need to be > followed for this corner case. > > This change is needed for the upcoming OMAP port to the common clock > framework. Specifically, it is undesirable to treat the disable-unused > path identically to the normal clk_disable path since other software > layers are involved. In this case OMAP's clockdomain code throws WARNs > and bails early due to the clock's enable_count being set to zero. A > custom callback mitigates this problem nicely. > > Cc: Paul Walmsley <paul@pwsan.com> > Signed-off-by: Mike Turquette <mturquette@linaro.org> This semantic change: > - if (__clk_is_enabled(clk) && clk->ops->disable) > - clk->ops->disable(clk->hw); > + /* > + * some gate clocks have special needs during the disable-unused > + * sequence. call .disable_unused if available, otherwise fall > + * back to .disable > + */ > + if (__clk_is_enabled(clk)) { > + if (clk->ops->disable_unused) > + clk->ops->disable_unused(clk->hw); > + else if (clk->ops->disable) > + clk->ops->disable(clk->hw); > + } Means that you should probably go into all drivers and set their .disable_unused to point to the clk_disable() implentation. Something like the below (untested, Signed-off-by: Linus Walleij <linus.walleij@linaro.org> diff --git a/drivers/clk/clk-u300.c b/drivers/clk/clk-u300.c index a15f792..504abde 100644 --- a/drivers/clk/clk-u300.c +++ b/drivers/clk/clk-u300.c @@ -340,6 +340,7 @@ static const struct clk_ops syscon_clk_ops = { .unprepare = syscon_clk_unprepare, .enable = syscon_clk_enable, .disable = syscon_clk_disable, + .disable_unused = syscon_clk_disable, .is_enabled = syscon_clk_is_enabled, .recalc_rate = syscon_clk_recalc_rate, .round_rate = syscon_clk_round_rate, diff --git a/drivers/clk/ux500/clk-prcc.c b/drivers/clk/ux500/clk-prcc.c index 7eee7f7..1d3ff60 100644 --- a/drivers/clk/ux500/clk-prcc.c +++ b/drivers/clk/ux500/clk-prcc.c @@ -84,12 +84,14 @@ static int clk_prcc_is_enabled(struct clk_hw *hw) static struct clk_ops clk_prcc_pclk_ops = { .enable = clk_prcc_pclk_enable, .disable = clk_prcc_pclk_disable, + .disable_unused = clk_prcc_pclk_disable, .is_enabled = clk_prcc_is_enabled, }; static struct clk_ops clk_prcc_kclk_ops = { .enable = clk_prcc_kclk_enable, .disable = clk_prcc_kclk_disable, + .disable_unused = clk_prcc_kclk_disable, .is_enabled = clk_prcc_is_enabled, }; diff --git a/drivers/clk/ux500/clk-prcmu.c b/drivers/clk/ux500/clk-prcmu.c index 74faa7e..00498f2 100644 --- a/drivers/clk/ux500/clk-prcmu.c +++ b/drivers/clk/ux500/clk-prcmu.c @@ -172,6 +172,7 @@ static struct clk_ops clk_prcmu_scalable_ops = { .unprepare = clk_prcmu_unprepare, .enable = clk_prcmu_enable, .disable = clk_prcmu_disable, + .disable_unused = clk_prcmu_disable, .is_enabled = clk_prcmu_is_enabled, .recalc_rate = clk_prcmu_recalc_rate, .round_rate = clk_prcmu_round_rate, @@ -183,6 +184,7 @@ static struct clk_ops clk_prcmu_gate_ops = { .unprepare = clk_prcmu_unprepare, .enable = clk_prcmu_enable, .disable = clk_prcmu_disable, + .disable_unused = clk_prcmu_disable, .is_enabled = clk_prcmu_is_enabled, .recalc_rate = clk_prcmu_recalc_rate, }; @@ -204,6 +206,7 @@ static struct clk_ops clk_prcmu_opp_gate_ops = { .unprepare = clk_prcmu_opp_unprepare, .enable = clk_prcmu_enable, .disable = clk_prcmu_disable, + .disable_unused = clk_prcmu_disable, .is_enabled = clk_prcmu_is_enabled, .recalc_rate = clk_prcmu_recalc_rate, }; @@ -213,6 +216,7 @@ static struct clk_ops clk_prcmu_opp_volt_scalable_ops = { .unprepare = clk_prcmu_opp_volt_unprepare, .enable = clk_prcmu_enable, .disable = clk_prcmu_disable, + .disable_unused = clk_prcmu_disable, .is_enabled = clk_prcmu_is_enabled, .recalc_rate = clk_prcmu_recalc_rate, .round_rate = clk_prcmu_round_rate, Yours, Linus Walleij ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/2] clk: introduce optional disable_unused callback 2012-12-06 19:07 ` Linus Walleij @ 2012-12-06 21:58 ` Ulf Hansson 2012-12-06 22:05 ` Mike Turquette 0 siblings, 1 reply; 9+ messages in thread From: Ulf Hansson @ 2012-12-06 21:58 UTC (permalink / raw) To: linux-arm-kernel On 6 December 2012 20:07, Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Dec 4, 2012 at 10:00 PM, Mike Turquette <mturquette@linaro.org> wrote: > >> Some gate clocks have special needs which must be handled during the >> disable-unused clocks sequence. These needs might be driven by software >> due to the fact that we're disabling a clock outside of the normal >> clk_disable path and a clk's enable_count will not be accurate. On the >> other hand a specific hardware programming sequence might need to be >> followed for this corner case. >> >> This change is needed for the upcoming OMAP port to the common clock >> framework. Specifically, it is undesirable to treat the disable-unused >> path identically to the normal clk_disable path since other software >> layers are involved. In this case OMAP's clockdomain code throws WARNs >> and bails early due to the clock's enable_count being set to zero. A >> custom callback mitigates this problem nicely. >> >> Cc: Paul Walmsley <paul@pwsan.com> >> Signed-off-by: Mike Turquette <mturquette@linaro.org> > > This semantic change: > >> - if (__clk_is_enabled(clk) && clk->ops->disable) >> - clk->ops->disable(clk->hw); >> + /* >> + * some gate clocks have special needs during the disable-unused >> + * sequence. call .disable_unused if available, otherwise fall >> + * back to .disable >> + */ >> + if (__clk_is_enabled(clk)) { >> + if (clk->ops->disable_unused) >> + clk->ops->disable_unused(clk->hw); >> + else if (clk->ops->disable) >> + clk->ops->disable(clk->hw); >> + } > > Means that you should probably go into all drivers and set their > .disable_unused to point to the clk_disable() implentation. Something > like the below (untested, Signed-off-by: Linus Walleij > <linus.walleij@linaro.org> > > diff --git a/drivers/clk/clk-u300.c b/drivers/clk/clk-u300.c > index a15f792..504abde 100644 > --- a/drivers/clk/clk-u300.c > +++ b/drivers/clk/clk-u300.c > @@ -340,6 +340,7 @@ static const struct clk_ops syscon_clk_ops = { > .unprepare = syscon_clk_unprepare, > .enable = syscon_clk_enable, > .disable = syscon_clk_disable, > + .disable_unused = syscon_clk_disable, > .is_enabled = syscon_clk_is_enabled, > .recalc_rate = syscon_clk_recalc_rate, > .round_rate = syscon_clk_round_rate, > diff --git a/drivers/clk/ux500/clk-prcc.c b/drivers/clk/ux500/clk-prcc.c > index 7eee7f7..1d3ff60 100644 > --- a/drivers/clk/ux500/clk-prcc.c > +++ b/drivers/clk/ux500/clk-prcc.c > @@ -84,12 +84,14 @@ static int clk_prcc_is_enabled(struct clk_hw *hw) > static struct clk_ops clk_prcc_pclk_ops = { > .enable = clk_prcc_pclk_enable, > .disable = clk_prcc_pclk_disable, > + .disable_unused = clk_prcc_pclk_disable, > .is_enabled = clk_prcc_is_enabled, > }; > > static struct clk_ops clk_prcc_kclk_ops = { > .enable = clk_prcc_kclk_enable, > .disable = clk_prcc_kclk_disable, > + .disable_unused = clk_prcc_kclk_disable, > .is_enabled = clk_prcc_is_enabled, > }; > > diff --git a/drivers/clk/ux500/clk-prcmu.c b/drivers/clk/ux500/clk-prcmu.c > index 74faa7e..00498f2 100644 > --- a/drivers/clk/ux500/clk-prcmu.c > +++ b/drivers/clk/ux500/clk-prcmu.c > @@ -172,6 +172,7 @@ static struct clk_ops clk_prcmu_scalable_ops = { > .unprepare = clk_prcmu_unprepare, > .enable = clk_prcmu_enable, > .disable = clk_prcmu_disable, > + .disable_unused = clk_prcmu_disable, > .is_enabled = clk_prcmu_is_enabled, > .recalc_rate = clk_prcmu_recalc_rate, > .round_rate = clk_prcmu_round_rate, > @@ -183,6 +184,7 @@ static struct clk_ops clk_prcmu_gate_ops = { > .unprepare = clk_prcmu_unprepare, > .enable = clk_prcmu_enable, > .disable = clk_prcmu_disable, > + .disable_unused = clk_prcmu_disable, > .is_enabled = clk_prcmu_is_enabled, > .recalc_rate = clk_prcmu_recalc_rate, > }; > @@ -204,6 +206,7 @@ static struct clk_ops clk_prcmu_opp_gate_ops = { > .unprepare = clk_prcmu_opp_unprepare, > .enable = clk_prcmu_enable, > .disable = clk_prcmu_disable, > + .disable_unused = clk_prcmu_disable, > .is_enabled = clk_prcmu_is_enabled, > .recalc_rate = clk_prcmu_recalc_rate, > }; > @@ -213,6 +216,7 @@ static struct clk_ops clk_prcmu_opp_volt_scalable_ops = { > .unprepare = clk_prcmu_opp_volt_unprepare, > .enable = clk_prcmu_enable, > .disable = clk_prcmu_disable, > + .disable_unused = clk_prcmu_disable, > .is_enabled = clk_prcmu_is_enabled, > .recalc_rate = clk_prcmu_recalc_rate, > .round_rate = clk_prcmu_round_rate, > > > Yours, > Linus Walleij > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel I am not sure such change is needed. It kind of depends on what Mike decides to put in the clk documentation. :-) According to commit msg, it seems like the .disable_unused function will be optional and must only be implemented for those clks that has has special issues to consider. Kind regards Ulf Hansson ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] clk: introduce optional disable_unused callback 2012-12-06 21:58 ` Ulf Hansson @ 2012-12-06 22:05 ` Mike Turquette 2012-12-07 8:11 ` Linus Walleij 0 siblings, 1 reply; 9+ messages in thread From: Mike Turquette @ 2012-12-06 22:05 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 6, 2012 at 1:58 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 6 December 2012 20:07, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Tue, Dec 4, 2012 at 10:00 PM, Mike Turquette <mturquette@linaro.org> wrote: >> >>> Some gate clocks have special needs which must be handled during the >>> disable-unused clocks sequence. These needs might be driven by software >>> due to the fact that we're disabling a clock outside of the normal >>> clk_disable path and a clk's enable_count will not be accurate. On the >>> other hand a specific hardware programming sequence might need to be >>> followed for this corner case. >>> >>> This change is needed for the upcoming OMAP port to the common clock >>> framework. Specifically, it is undesirable to treat the disable-unused >>> path identically to the normal clk_disable path since other software >>> layers are involved. In this case OMAP's clockdomain code throws WARNs >>> and bails early due to the clock's enable_count being set to zero. A >>> custom callback mitigates this problem nicely. >>> >>> Cc: Paul Walmsley <paul@pwsan.com> >>> Signed-off-by: Mike Turquette <mturquette@linaro.org> >> >> This semantic change: >> >>> - if (__clk_is_enabled(clk) && clk->ops->disable) >>> - clk->ops->disable(clk->hw); >>> + /* >>> + * some gate clocks have special needs during the disable-unused >>> + * sequence. call .disable_unused if available, otherwise fall >>> + * back to .disable >>> + */ >>> + if (__clk_is_enabled(clk)) { >>> + if (clk->ops->disable_unused) >>> + clk->ops->disable_unused(clk->hw); >>> + else if (clk->ops->disable) >>> + clk->ops->disable(clk->hw); >>> + } >> >> Means that you should probably go into all drivers and set their >> .disable_unused to point to the clk_disable() implentation. Something >> like the below (untested, Signed-off-by: Linus Walleij >> <linus.walleij@linaro.org> >> >> diff --git a/drivers/clk/clk-u300.c b/drivers/clk/clk-u300.c >> index a15f792..504abde 100644 >> --- a/drivers/clk/clk-u300.c >> +++ b/drivers/clk/clk-u300.c >> @@ -340,6 +340,7 @@ static const struct clk_ops syscon_clk_ops = { >> .unprepare = syscon_clk_unprepare, >> .enable = syscon_clk_enable, >> .disable = syscon_clk_disable, >> + .disable_unused = syscon_clk_disable, >> .is_enabled = syscon_clk_is_enabled, >> .recalc_rate = syscon_clk_recalc_rate, >> .round_rate = syscon_clk_round_rate, >> diff --git a/drivers/clk/ux500/clk-prcc.c b/drivers/clk/ux500/clk-prcc.c >> index 7eee7f7..1d3ff60 100644 >> --- a/drivers/clk/ux500/clk-prcc.c >> +++ b/drivers/clk/ux500/clk-prcc.c >> @@ -84,12 +84,14 @@ static int clk_prcc_is_enabled(struct clk_hw *hw) >> static struct clk_ops clk_prcc_pclk_ops = { >> .enable = clk_prcc_pclk_enable, >> .disable = clk_prcc_pclk_disable, >> + .disable_unused = clk_prcc_pclk_disable, >> .is_enabled = clk_prcc_is_enabled, >> }; >> >> static struct clk_ops clk_prcc_kclk_ops = { >> .enable = clk_prcc_kclk_enable, >> .disable = clk_prcc_kclk_disable, >> + .disable_unused = clk_prcc_kclk_disable, >> .is_enabled = clk_prcc_is_enabled, >> }; >> >> diff --git a/drivers/clk/ux500/clk-prcmu.c b/drivers/clk/ux500/clk-prcmu.c >> index 74faa7e..00498f2 100644 >> --- a/drivers/clk/ux500/clk-prcmu.c >> +++ b/drivers/clk/ux500/clk-prcmu.c >> @@ -172,6 +172,7 @@ static struct clk_ops clk_prcmu_scalable_ops = { >> .unprepare = clk_prcmu_unprepare, >> .enable = clk_prcmu_enable, >> .disable = clk_prcmu_disable, >> + .disable_unused = clk_prcmu_disable, >> .is_enabled = clk_prcmu_is_enabled, >> .recalc_rate = clk_prcmu_recalc_rate, >> .round_rate = clk_prcmu_round_rate, >> @@ -183,6 +184,7 @@ static struct clk_ops clk_prcmu_gate_ops = { >> .unprepare = clk_prcmu_unprepare, >> .enable = clk_prcmu_enable, >> .disable = clk_prcmu_disable, >> + .disable_unused = clk_prcmu_disable, >> .is_enabled = clk_prcmu_is_enabled, >> .recalc_rate = clk_prcmu_recalc_rate, >> }; >> @@ -204,6 +206,7 @@ static struct clk_ops clk_prcmu_opp_gate_ops = { >> .unprepare = clk_prcmu_opp_unprepare, >> .enable = clk_prcmu_enable, >> .disable = clk_prcmu_disable, >> + .disable_unused = clk_prcmu_disable, >> .is_enabled = clk_prcmu_is_enabled, >> .recalc_rate = clk_prcmu_recalc_rate, >> }; >> @@ -213,6 +216,7 @@ static struct clk_ops clk_prcmu_opp_volt_scalable_ops = { >> .unprepare = clk_prcmu_opp_volt_unprepare, >> .enable = clk_prcmu_enable, >> .disable = clk_prcmu_disable, >> + .disable_unused = clk_prcmu_disable, >> .is_enabled = clk_prcmu_is_enabled, >> .recalc_rate = clk_prcmu_recalc_rate, >> .round_rate = clk_prcmu_round_rate, >> >> >> Yours, >> Linus Walleij >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel at lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > I am not sure such change is needed. It kind of depends on what Mike > decides to put in the clk documentation. :-) > > According to commit msg, it seems like the .disable_unused function > will be optional and must only be implemented for those clks that has > has special issues to consider. > That is correct. I don't want people to read the above code while porting their platform and perceive that .disable_unused is mandatory in any way. Using the same function in both pointers defeats the purpose of the new optional callback. Regards, Mike > Kind regards > Ulf Hansson ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] clk: introduce optional disable_unused callback 2012-12-06 22:05 ` Mike Turquette @ 2012-12-07 8:11 ` Linus Walleij 0 siblings, 0 replies; 9+ messages in thread From: Linus Walleij @ 2012-12-07 8:11 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 6, 2012 at 11:05 PM, Mike Turquette <mturquette@linaro.org> wrote: > On Thu, Dec 6, 2012 at 1:58 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> According to commit msg, it seems like the .disable_unused function >> will be optional and must only be implemented for those clks that has >> has special issues to consider. >> > > That is correct. I don't want people to read the above code while > porting their platform and perceive that .disable_unused is mandatory > in any way. Using the same function in both pointers defeats the > purpose of the new optional callback. Aha I get it... Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] MAINTAINERS: bad email address for Mike Turquette 2012-12-04 21:00 [PATCH 0/2] clk: misc bits for 3.8 Mike Turquette 2012-12-04 21:00 ` [PATCH 1/2] clk: introduce optional disable_unused callback Mike Turquette @ 2012-12-04 21:00 ` Mike Turquette 1 sibling, 0 replies; 9+ messages in thread From: Mike Turquette @ 2012-12-04 21:00 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Mike Turquette <mturquette@linaro.org> --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 1fa9074..6d7295f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1986,7 +1986,6 @@ F: fs/coda/ F: include/linux/coda*.h COMMON CLK FRAMEWORK -M: Mike Turquette <mturquette@ti.com> M: Mike Turquette <mturquette@linaro.org> L: linux-arm-kernel at lists.infradead.org (same as CLK API & CLKDEV) T: git git://git.linaro.org/people/mturquette/linux.git -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-12-07 8:11 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-12-04 21:00 [PATCH 0/2] clk: misc bits for 3.8 Mike Turquette 2012-12-04 21:00 ` [PATCH 1/2] clk: introduce optional disable_unused callback Mike Turquette 2012-12-05 16:13 ` Ulf Hansson 2012-12-05 18:00 ` Mike Turquette 2012-12-06 19:07 ` Linus Walleij 2012-12-06 21:58 ` Ulf Hansson 2012-12-06 22:05 ` Mike Turquette 2012-12-07 8:11 ` Linus Walleij 2012-12-04 21:00 ` [PATCH 2/2] MAINTAINERS: bad email address for Mike Turquette Mike Turquette
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).