| Message ID | 20090223160349.GB27688@n2100.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > Sent: Monday, February 23, 2009 10:04 AM > To: Woodruff, Richard > Ack? Ack. > diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c > index 08baa18..b2d9e1f 100644 > --- a/arch/arm/plat-omap/clock.c > +++ b/arch/arm/plat-omap/clock.c > @@ -144,12 +144,14 @@ int clk_set_parent(struct clk *clk, struct clk *parent) > return ret; > > spin_lock_irqsave(&clockfw_lock, flags); > - if (arch_clock->clk_set_parent) > - ret = arch_clock->clk_set_parent(clk, parent); > - if (ret == 0) { > - if (clk->recalc) > - clk->rate = clk->recalc(clk); > - propagate_rate(clk); > + if (clk->usecount == 0) { > + if (arch_clock->clk_set_parent) > + ret = arch_clock->clk_set_parent(clk, parent); > + if (ret == 0) { > + if (clk->recalc) > + clk->rate = clk->recalc(clk); > + propagate_rate(clk); > + } > } > spin_unlock_irqrestore(&clockfw_lock, flags); > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Russell, On Mon, 23 Feb 2009, Russell King - ARM Linux wrote: > On Thu, Feb 19, 2009 at 06:50:52PM -0600, Woodruff, Richard wrote: > > The historic usage of this has been against single use leaf clocks (1st instance of gptimer). When it was used it did: > > clk_get() > > clk_set_parent() > > clk_enable() > > > > This usage was ok for that. Use on a disabled clock is needed. > > > > If there are multiple users on the clock or it is enabled there are problems. > > It sounds to me like clk_set_parent() needs this then to prevent enabled > clocks being reparented. By doing so, we no longer need to disable/enable > the clock within the OMAP2/3 set_parent function since we are now > guaranteed that the clock will be disabled. > > Ack? One suggestion on the patch: > diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c > index 08baa18..b2d9e1f 100644 > --- a/arch/arm/plat-omap/clock.c > +++ b/arch/arm/plat-omap/clock.c > @@ -144,12 +144,14 @@ int clk_set_parent(struct clk *clk, struct clk *parent) > return ret; > > spin_lock_irqsave(&clockfw_lock, flags); > - if (arch_clock->clk_set_parent) > - ret = arch_clock->clk_set_parent(clk, parent); > - if (ret == 0) { > - if (clk->recalc) > - clk->rate = clk->recalc(clk); > - propagate_rate(clk); > + if (clk->usecount == 0) { > + if (arch_clock->clk_set_parent) > + ret = arch_clock->clk_set_parent(clk, parent); > + if (ret == 0) { > + if (clk->recalc) > + clk->rate = clk->recalc(clk); > + propagate_rate(clk); > + } > } > spin_unlock_irqrestore(&clockfw_lock, flags); Suggest returning a more specific error than -EINVAL: if (clk->usecount > 0) return -EBUSY; - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 02, 2009 at 04:02:14PM -0700, Paul Walmsley wrote: > Suggest returning a more specific error than -EINVAL: > > if (clk->usecount > 0) > return -EBUSY; Done. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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-omap2/clock.c b/arch/arm/mach-omap2/clock.c index 40cb65b..f49bce9 100644 --- a/arch/arm/mach-omap2/clock.c +++ b/arch/arm/mach-omap2/clock.c @@ -807,9 +807,6 @@ int omap2_clk_set_parent(struct clk *clk, struct clk *new_parent) if (!parent_div) return -EINVAL; - if (clk->usecount > 0) - _omap2_clk_disable(clk); - /* Set new source value (previous dividers if any in effect) */ v = __raw_readl(clk->clksel_reg); v &= ~clk->clksel_mask; @@ -819,9 +816,6 @@ int omap2_clk_set_parent(struct clk *clk, struct clk *new_parent) _omap2xxx_clk_commit(clk); - if (clk->usecount > 0) - _omap2_clk_enable(clk); - clk_reparent(clk, new_parent); /* CLKSEL clocks follow their parents' rates, divided by a divisor */ diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c index 08baa18..b2d9e1f 100644 --- a/arch/arm/plat-omap/clock.c +++ b/arch/arm/plat-omap/clock.c @@ -144,12 +144,14 @@ int clk_set_parent(struct clk *clk, struct clk *parent) return ret; spin_lock_irqsave(&clockfw_lock, flags); - if (arch_clock->clk_set_parent) - ret = arch_clock->clk_set_parent(clk, parent); - if (ret == 0) { - if (clk->recalc) - clk->rate = clk->recalc(clk); - propagate_rate(clk); + if (clk->usecount == 0) { + if (arch_clock->clk_set_parent) + ret = arch_clock->clk_set_parent(clk, parent); + if (ret == 0) { + if (clk->recalc) + clk->rate = clk->recalc(clk); + propagate_rate(clk); + } } spin_unlock_irqrestore(&clockfw_lock, flags);