| Message ID | 1351089926-32161-3-git-send-email-lee.jones@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote: > The clock framework has changed somewhat and it's now better to > invoke clock_prepare_enable() and clk_disable_unprepare() rather > than the legacy clk_enable() and clk_disable() calls. This patch > converts the Nomadik Pin Control driver to the new framework. > > Signed-off-by: Lee Jones <lee.jones@linaro.org> (...) > - clk_enable(chip->clk); > + clk_prepare_enable(chip->clk); (...) > - clk_disable(chip->clk); > + clk_disable_unprepare(chip->clk); (Repeated for each occurence.) Is this *really* causing a regression? I mean the driver begin like this in nmk_gpio_probe(): clk = devm_clk_get(&dev->dev, NULL); if (IS_ERR(clk)) { ret = PTR_ERR(clk); goto out; } clk_prepare(clk); Then it leaves the clock prepared. So the clock is always prepared. You would only need to enable/disable it at times. And the semantics of the clk_enable/clk_disable call pair is such that it is fastpath and should be real quick, and that is exactly why we're using it repeatedly like that. Inserting clk_unprepare() effectively could make the whole driver a lot slower, so convince me on this one. ... I suspect the real bug (if there is one) must be in the clock implementation. Yours, Linus Walleij
On Wed, 24 Oct 2012, Linus Walleij wrote: > On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote: > > > The clock framework has changed somewhat and it's now better to > > invoke clock_prepare_enable() and clk_disable_unprepare() rather > > than the legacy clk_enable() and clk_disable() calls. This patch > > converts the Nomadik Pin Control driver to the new framework. > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > (...) > > - clk_enable(chip->clk); > > + clk_prepare_enable(chip->clk); > (...) > > - clk_disable(chip->clk); > > + clk_disable_unprepare(chip->clk); > > (Repeated for each occurence.) > > Is this *really* causing a regression? I mean the driver > begin like this in nmk_gpio_probe(): > > clk = devm_clk_get(&dev->dev, NULL); > if (IS_ERR(clk)) { > ret = PTR_ERR(clk); > goto out; > } > clk_prepare(clk); > > Then it leaves the clock prepared. So the clock is always > prepared. You would only need to enable/disable it at times. > > And the semantics of the clk_enable/clk_disable call pair > is such that it is fastpath and should be real quick, and that > is exactly why we're using it repeatedly like that. Inserting > clk_unprepare() effectively could make the whole driver a > lot slower, so convince me on this one. ... > > I suspect the real bug (if there is one) must be in the clock > implementation. This certainly doesn't fix the bug we spoke about. I believe Ulf is still working on that one. So do you want me to remove this patch?
On 10/25/2012 09:31 AM, Lee Jones wrote: > > This certainly doesn't fix the bug we spoke about. I believe Ulf > is still working on that one. > > So do you want me to remove this patch? > Yeah drop it for now. Yours, Linus Walleij
On Thu, 25 Oct 2012, Linus Walleij wrote: > On 10/25/2012 09:31 AM, Lee Jones wrote: > > > >This certainly doesn't fix the bug we spoke about. I believe Ulf > >is still working on that one. > > > >So do you want me to remove this patch? > > > > Yeah drop it for now. Actually, a quick question before I do: If it's better/faster to prepare the clock and keep it prepared while you do clk_enable/clk_disable, why don't we do that in all drivers? Why do we bother to prepare/unprepare each time if all it does is take up cycles?
On 25 October 2012 10:23, Lee Jones <lee.jones@linaro.org> wrote: > On Thu, 25 Oct 2012, Linus Walleij wrote: > >> On 10/25/2012 09:31 AM, Lee Jones wrote: >> > >> >This certainly doesn't fix the bug we spoke about. I believe Ulf >> >is still working on that one. >> > >> >So do you want me to remove this patch? >> > >> >> Yeah drop it for now. > > Actually, a quick question before I do: > > If it's better/faster to prepare the clock and keep it prepared > while you do clk_enable/clk_disable, why don't we do that in all > drivers? Why do we bother to prepare/unprepare each time if all > it does is take up cycles? > Depending on clock type, a clk_disable is actually not going to "gate" the clock, that might happen only in unprepare. This depends on if the clock is a fast or slow clock. To save as much power as possible, in general, we should do both disable and unprepare. Although it will be device driver dependent were it is most convenient to do this things. Sometimes it is possible to group them sometimes not. Kind regards Ulf Hansson
On Thu, 25 Oct 2012, Ulf Hansson wrote: > On 25 October 2012 10:23, Lee Jones <lee.jones@linaro.org> wrote: > > On Thu, 25 Oct 2012, Linus Walleij wrote: > > > >> On 10/25/2012 09:31 AM, Lee Jones wrote: > >> > > >> >This certainly doesn't fix the bug we spoke about. I believe Ulf > >> >is still working on that one. > >> > > >> >So do you want me to remove this patch? > >> > > >> > >> Yeah drop it for now. > > > > Actually, a quick question before I do: > > > > If it's better/faster to prepare the clock and keep it prepared > > while you do clk_enable/clk_disable, why don't we do that in all > > drivers? Why do we bother to prepare/unprepare each time if all > > it does is take up cycles? > > > > Depending on clock type, a clk_disable is actually not going to "gate" > the clock, that might happen only in unprepare. This depends on if the > clock is a fast or slow clock. > To save as much power as possible, in general, we should do both > disable and unprepare. Although it will be device driver dependent > were it is most convenient to do this things. > Sometimes it is possible to group them sometimes not. And in this case, it's better to ... ?
On Thu, Oct 25, 2012 at 10:23 AM, Lee Jones <lee.jones@linaro.org> wrote: > On Thu, 25 Oct 2012, Linus Walleij wrote: >> >> Yeah drop it for now. > > Actually, a quick question before I do: > > If it's better/faster to prepare the clock and keep it prepared > while you do clk_enable/clk_disable, It is generally faster that is why we call it fastpath. E.g. if the clock hardware can do this in IRQ context by just chaninging one bit in a quickly written register from 1->0 and then the clock goes off from some silicon. Whether it's "better" or not is a transcendental question, as it requires a ruler to measure betterness. > why don't we do that in all > drivers? Why do we bother to prepare/unprepare each time if all > it does is take up cycles? Usually to save power. Albeit saving power may be at odds with gaining the maximum performance and/or latency. So depending on the demands and use case the answer to whether or not you want to do this will be different. That's for the clock API. In the ux500 case specifically, you can drill down to the clock implementation and ask the question whether or not we want to do this for this instance of the pin controller in this case, I'll leave that for Ulf to answer... but remember that this driver is also used for the Nomadik NHK8815. Yours, Linus Walleij
On Thu, Oct 25, 2012 at 11:29 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > Depending on clock type, a clk_disable is actually not going to "gate" > the clock, that might happen only in unprepare. This depends on if the > clock is a fast or slow clock. Hm thats interesting. Now I need to drill down into this. So looking at it in this case: clk_disable() from the GPIO block of the pin controller will hit "gpio.0", "gpio.1" etc in drivers/clk/ux500/u8500_clk.c. These are PRCC (Programmable Clock Controller) clocks registered using clk_reg_prcc_pclk() from clk-prcc.c. pclk:s are using the clk_prcc_pclk_ops and these point to clk_prcc_pclk_enable()/clk_prcc_pclk_disable() for enable/disable respectively. These will just write a PRCC register. And prepare() and unprepare() are not implemented for this clock. So far we can conclude that clk_enable()/disable() will indeed achieve the desired effect of gating the clock to the GPIO block per se, so we are saving some power for sure. However the prepare()/unprepare() calls will of course also accumulate upwards and in e.g. the example of "gpio.0" and "gpio.1" the parent is "per1clk" which is the clock for the entire peripheral group. (At this point I can stick in a reminder of the idea to restructure the device tree in peripheral groups, because that exercise will certainly pay off the day we try to encode clocks in the device tree, I think the point can be clearly seen as we proceed...) This "per1clk" is registered using clk_req_prcmu_gate() from clk-prcmu.c. Looking at that clock type we find it's a plain software dummy for the clk_enable()/clk_disable() path. The real action is happening in the prepare()/unprepare() path. So to be able to shut down the entire peripheral cluster, indeed both functions need to be called. So in accordance with this we can see that the patch should be applied, in some form. However it is not removing the initial clk_prepare() so the entire patch will be pointless, the prepare count will currently always be > 0. I'll mangle Lee's patch a bit, hold on.. Yours, Linus Walleij
On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote: > The clock framework has changed somewhat and it's now better to > invoke clock_prepare_enable() and clk_disable_unprepare() rather > than the legacy clk_enable() and clk_disable() calls. This patch > converts the Nomadik Pin Control driver to the new framework. > > Signed-off-by: Lee Jones <lee.jones@linaro.org> I was convinced that this is a good change but no regression, so applied to the devel branch for 3.8. I also removed the initial clk_prepare() so the reference count may actually go down to 0 for the GPIO block and the peripheral cluster eventually gets relaxed. Thanks! Linus Walleij
On Thu, 25 Oct 2012, Linus Walleij wrote: > On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote: > > > The clock framework has changed somewhat and it's now better to > > invoke clock_prepare_enable() and clk_disable_unprepare() rather > > than the legacy clk_enable() and clk_disable() calls. This patch > > converts the Nomadik Pin Control driver to the new framework. > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > I was convinced that this is a good change but no regression, > so applied to the devel branch for 3.8. > > I also removed the initial clk_prepare() so the reference count > may actually go down to 0 for the GPIO block and the peripheral > cluster eventually gets relaxed. Nice. Thanks Linus.
On Thu, Oct 25, 2012 at 2:41 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote: > >> The clock framework has changed somewhat and it's now better to >> invoke clock_prepare_enable() and clk_disable_unprepare() rather >> than the legacy clk_enable() and clk_disable() calls. This patch >> converts the Nomadik Pin Control driver to the new framework. >> >> Signed-off-by: Lee Jones <lee.jones@linaro.org> > > I was convinced that this is a good change but no regression, > so applied to the devel branch for 3.8. > > I also removed the initial clk_prepare() so the reference count > may actually go down to 0 for the GPIO block and the peripheral > cluster eventually gets relaxed. Famous last words! The good news is that this actually works, and the refcount *does* go down to zero and gate off entire peripheral clusters. However that was not good because something vital in some peripheral cluster died and killed the system :-D Lee, could to to track down the reason and fix it so the patch can be applied? The only thing you need to do is to remove the superfluous clk_prepare() right after the devm_clk_get() that hogs each peripheral cluster. Probably some driver is needing a clk_get() or a clk_get_sys() is needs to be added somewhere to bring up some vital cluster, or there may be some out-of-tree driver needed to bring up the cluster properly I have no clue... Maybe some cluster just cannot be declocked like that. Yours, Linus Walleij
On Thu, 25 Oct 2012, Linus Walleij wrote: > On Thu, Oct 25, 2012 at 2:41 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Wed, Oct 24, 2012 at 4:45 PM, Lee Jones <lee.jones@linaro.org> wrote: > > > >> The clock framework has changed somewhat and it's now better to > >> invoke clock_prepare_enable() and clk_disable_unprepare() rather > >> than the legacy clk_enable() and clk_disable() calls. This patch > >> converts the Nomadik Pin Control driver to the new framework. > >> > >> Signed-off-by: Lee Jones <lee.jones@linaro.org> > > > > I was convinced that this is a good change but no regression, > > so applied to the devel branch for 3.8. > > > > I also removed the initial clk_prepare() so the reference count > > may actually go down to 0 for the GPIO block and the peripheral > > cluster eventually gets relaxed. > > Famous last words! > > The good news is that this actually works, and the refcount > *does* go down to zero and gate off entire peripheral > clusters. > > However that was not good because something vital in > some peripheral cluster died and killed the system :-D > > Lee, could to to track down the reason and fix it so the patch > can be applied? > > The only thing you need to do is to remove the superfluous > clk_prepare() right after the devm_clk_get() that hogs each > peripheral cluster. > > Probably some driver is needing a clk_get() or a clk_get_sys() is > needs to be added somewhere to bring up some vital cluster, > or there may be some out-of-tree driver needed to bring up the > cluster properly I have no clue... Maybe some cluster just > cannot be declocked like that. I leave work in 10 mins and won't be coding again for ~2.5 weeks. So if this is something you could squeeze in and fix-up, I'd be very grateful. Kind regards, Lee
On Thu, Oct 25, 2012 at 5:51 PM, Lee Jones <lee.jones@linaro.org> wrote: > On Thu, 25 Oct 2012, Linus Walleij wrote: >> Probably some driver is needing a clk_get() or a clk_get_sys() is >> needs to be added somewhere to bring up some vital cluster, >> or there may be some out-of-tree driver needed to bring up the >> cluster properly I have no clue... Maybe some cluster just >> cannot be declocked like that. > > I leave work in 10 mins and won't be coding again for ~2.5 weeks. > So if this is something you could squeeze in and fix-up, I'd be > very grateful. I'll try. It doesn't look right that a clk_prepare() in the pinctrl driver is saving the day for somebody else... Yours, Linus Walleij
On Thu, 25 Oct 2012, Linus Walleij wrote: > On Thu, Oct 25, 2012 at 5:51 PM, Lee Jones <lee.jones@linaro.org> wrote: > > On Thu, 25 Oct 2012, Linus Walleij wrote: > > >> Probably some driver is needing a clk_get() or a clk_get_sys() is > >> needs to be added somewhere to bring up some vital cluster, > >> or there may be some out-of-tree driver needed to bring up the > >> cluster properly I have no clue... Maybe some cluster just > >> cannot be declocked like that. > > > > I leave work in 10 mins and won't be coding again for ~2.5 weeks. > > So if this is something you could squeeze in and fix-up, I'd be > > very grateful. > > I'll try. It doesn't look right that a clk_prepare() in the pinctrl > driver is saving the day for somebody else... Hi Linus, Did you apply this in the end? Kind regards, Lee
On Wed, Nov 14, 2012 at 2:18 PM, Lee Jones <lee.jones@linaro.org> wrote: > On Thu, 25 Oct 2012, Linus Walleij wrote: > >> On Thu, Oct 25, 2012 at 5:51 PM, Lee Jones <lee.jones@linaro.org> wrote: >> > On Thu, 25 Oct 2012, Linus Walleij wrote: >> >> >> Probably some driver is needing a clk_get() or a clk_get_sys() is >> >> needs to be added somewhere to bring up some vital cluster, >> >> or there may be some out-of-tree driver needed to bring up the >> >> cluster properly I have no clue... Maybe some cluster just >> >> cannot be declocked like that. >> > >> > I leave work in 10 mins and won't be coding again for ~2.5 weeks. >> > So if this is something you could squeeze in and fix-up, I'd be >> > very grateful. >> >> I'll try. It doesn't look right that a clk_prepare() in the pinctrl >> driver is saving the day for somebody else... > > Hi Linus, > > Did you apply this in the end? No I can't. If I fix the bug in the patch (removing the surplus clk_prepare() in the probe function) the thing regresses the entire kernel, so I have no patch which actually is working. It needs to be root-caused and fixed properly... Yours, Linus Walleij
diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c index 01aea1c..570c753 100644 --- a/drivers/pinctrl/pinctrl-nomadik.c +++ b/drivers/pinctrl/pinctrl-nomadik.c @@ -448,7 +448,7 @@ static void nmk_gpio_glitch_slpm_init(unsigned int *slpm) if (!chip) break; - clk_enable(chip->clk); + clk_prepare_enable(chip->clk); slpm[i] = readl(chip->addr + NMK_GPIO_SLPC); writel(temp, chip->addr + NMK_GPIO_SLPC); @@ -467,7 +467,7 @@ static void nmk_gpio_glitch_slpm_restore(unsigned int *slpm) writel(slpm[i], chip->addr + NMK_GPIO_SLPC); - clk_disable(chip->clk); + clk_disable_unprepare(chip->clk); } } @@ -512,12 +512,12 @@ static int __nmk_config_pins(pin_cfg_t *cfgs, int num, bool sleep) break; } - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); spin_lock(&nmk_chip->lock); __nmk_config_pin(nmk_chip, pin % NMK_GPIO_PER_CHIP, cfgs[i], sleep, glitch ? slpm : NULL); spin_unlock(&nmk_chip->lock); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); } if (glitch) @@ -602,7 +602,7 @@ int nmk_gpio_set_slpm(int gpio, enum nmk_gpio_slpm mode) if (!nmk_chip) return -EINVAL; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); spin_lock_irqsave(&nmk_gpio_slpm_lock, flags); spin_lock(&nmk_chip->lock); @@ -610,7 +610,7 @@ int nmk_gpio_set_slpm(int gpio, enum nmk_gpio_slpm mode) spin_unlock(&nmk_chip->lock); spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; } @@ -637,11 +637,11 @@ int nmk_gpio_set_pull(int gpio, enum nmk_gpio_pull pull) if (!nmk_chip) return -EINVAL; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); spin_lock_irqsave(&nmk_chip->lock, flags); __nmk_gpio_set_pull(nmk_chip, gpio % NMK_GPIO_PER_CHIP, pull); spin_unlock_irqrestore(&nmk_chip->lock, flags); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; } @@ -665,11 +665,11 @@ int nmk_gpio_set_mode(int gpio, int gpio_mode) if (!nmk_chip) return -EINVAL; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); spin_lock_irqsave(&nmk_chip->lock, flags); __nmk_gpio_set_mode(nmk_chip, gpio % NMK_GPIO_PER_CHIP, gpio_mode); spin_unlock_irqrestore(&nmk_chip->lock, flags); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; } @@ -686,12 +686,12 @@ int nmk_gpio_get_mode(int gpio) bit = 1 << (gpio % NMK_GPIO_PER_CHIP); - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); afunc = readl(nmk_chip->addr + NMK_GPIO_AFSLA) & bit; bfunc = readl(nmk_chip->addr + NMK_GPIO_AFSLB) & bit; - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return (afunc ? NMK_GPIO_ALT_A : 0) | (bfunc ? NMK_GPIO_ALT_B : 0); } @@ -712,9 +712,9 @@ static void nmk_gpio_irq_ack(struct irq_data *d) if (!nmk_chip) return; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); writel(nmk_gpio_get_bitmask(d->hwirq), nmk_chip->addr + NMK_GPIO_IC); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); } enum nmk_gpio_irq_type { @@ -788,7 +788,7 @@ static int nmk_gpio_irq_maskunmask(struct irq_data *d, bool enable) if (!nmk_chip) return -EINVAL; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); spin_lock_irqsave(&nmk_gpio_slpm_lock, flags); spin_lock(&nmk_chip->lock); @@ -799,7 +799,7 @@ static int nmk_gpio_irq_maskunmask(struct irq_data *d, bool enable) spin_unlock(&nmk_chip->lock); spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; } @@ -825,7 +825,7 @@ static int nmk_gpio_irq_set_wake(struct irq_data *d, unsigned int on) return -EINVAL; bitmask = nmk_gpio_get_bitmask(d->hwirq); - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); spin_lock_irqsave(&nmk_gpio_slpm_lock, flags); spin_lock(&nmk_chip->lock); @@ -839,7 +839,7 @@ static int nmk_gpio_irq_set_wake(struct irq_data *d, unsigned int on) spin_unlock(&nmk_chip->lock); spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; } @@ -861,7 +861,7 @@ static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type) if (type & IRQ_TYPE_LEVEL_LOW) return -EINVAL; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); spin_lock_irqsave(&nmk_chip->lock, flags); if (enabled) @@ -885,7 +885,7 @@ static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type) __nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, true); spin_unlock_irqrestore(&nmk_chip->lock, flags); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; } @@ -894,7 +894,7 @@ static unsigned int nmk_gpio_irq_startup(struct irq_data *d) { struct nmk_gpio_chip *nmk_chip = irq_data_get_irq_chip_data(d); - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); nmk_gpio_irq_unmask(d); return 0; } @@ -904,7 +904,7 @@ static void nmk_gpio_irq_shutdown(struct irq_data *d) struct nmk_gpio_chip *nmk_chip = irq_data_get_irq_chip_data(d); nmk_gpio_irq_mask(d); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); } static struct irq_chip nmk_gpio_irq_chip = { @@ -943,9 +943,9 @@ static void nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc) struct nmk_gpio_chip *nmk_chip = irq_get_handler_data(irq); u32 status; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); status = readl(nmk_chip->addr + NMK_GPIO_IS); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); __nmk_gpio_irq_handler(irq, desc, status); } @@ -998,11 +998,11 @@ static int nmk_gpio_make_input(struct gpio_chip *chip, unsigned offset) struct nmk_gpio_chip *nmk_chip = container_of(chip, struct nmk_gpio_chip, chip); - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); writel(1 << offset, nmk_chip->addr + NMK_GPIO_DIRC); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; } @@ -1014,11 +1014,11 @@ static int nmk_gpio_get_input(struct gpio_chip *chip, unsigned offset) u32 bit = 1 << offset; int value; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); value = (readl(nmk_chip->addr + NMK_GPIO_DAT) & bit) != 0; - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return value; } @@ -1029,11 +1029,11 @@ static void nmk_gpio_set_output(struct gpio_chip *chip, unsigned offset, struct nmk_gpio_chip *nmk_chip = container_of(chip, struct nmk_gpio_chip, chip); - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); __nmk_gpio_set_output(nmk_chip, offset, val); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); } static int nmk_gpio_make_output(struct gpio_chip *chip, unsigned offset, @@ -1042,11 +1042,11 @@ static int nmk_gpio_make_output(struct gpio_chip *chip, unsigned offset, struct nmk_gpio_chip *nmk_chip = container_of(chip, struct nmk_gpio_chip, chip); - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); __nmk_gpio_make_output(nmk_chip, offset, val); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; } @@ -1080,7 +1080,7 @@ static void nmk_gpio_dbg_show_one(struct seq_file *s, struct gpio_chip *chip, [NMK_GPIO_ALT_C] = "altC", }; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); is_out = !!(readl(nmk_chip->addr + NMK_GPIO_DIR) & bit); pull = !(readl(nmk_chip->addr + NMK_GPIO_PDIS) & bit); mode = nmk_gpio_get_mode(gpio); @@ -1118,7 +1118,7 @@ static void nmk_gpio_dbg_show_one(struct seq_file *s, struct gpio_chip *chip, ? " wakeup" : ""); } } - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); } static void nmk_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip) @@ -1164,7 +1164,7 @@ void nmk_gpio_clocks_enable(void) if (!chip) continue; - clk_enable(chip->clk); + clk_prepare_enable(chip->clk); } } @@ -1178,7 +1178,7 @@ void nmk_gpio_clocks_disable(void) if (!chip) continue; - clk_disable(chip->clk); + clk_disable_unprepare(chip->clk); } } @@ -1201,14 +1201,14 @@ void nmk_gpio_wakeups_suspend(void) if (!chip) break; - clk_enable(chip->clk); + clk_prepare_enable(chip->clk); writel(chip->rwimsc & chip->real_wake, chip->addr + NMK_GPIO_RWIMSC); writel(chip->fwimsc & chip->real_wake, chip->addr + NMK_GPIO_FWIMSC); - clk_disable(chip->clk); + clk_disable_unprepare(chip->clk); } } @@ -1222,12 +1222,12 @@ void nmk_gpio_wakeups_resume(void) if (!chip) break; - clk_enable(chip->clk); + clk_prepare_enable(chip->clk); writel(chip->rwimsc, chip->addr + NMK_GPIO_RWIMSC); writel(chip->fwimsc, chip->addr + NMK_GPIO_FWIMSC); - clk_disable(chip->clk); + clk_disable_unprepare(chip->clk); } } @@ -1367,9 +1367,9 @@ static int __devinit nmk_gpio_probe(struct platform_device *dev) chip->dev = &dev->dev; chip->owner = THIS_MODULE; - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); #ifdef CONFIG_OF_GPIO chip->of_node = np; @@ -1580,7 +1580,7 @@ static int nmk_pmx_enable(struct pinctrl_dev *pctldev, unsigned function, nmk_chip = container_of(chip, struct nmk_gpio_chip, chip); dev_dbg(npct->dev, "setting pin %d to altsetting %d\n", g->pins[i], g->altsetting); - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); bit = g->pins[i] % NMK_GPIO_PER_CHIP; /* * If the pin is switching to altfunc, and there was an @@ -1593,7 +1593,7 @@ static int nmk_pmx_enable(struct pinctrl_dev *pctldev, unsigned function, __nmk_gpio_set_mode_safe(nmk_chip, bit, (g->altsetting & NMK_GPIO_ALT_C), glitch); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); /* * Call PRCM GPIOCR config function in case ALTC @@ -1657,11 +1657,11 @@ int nmk_gpio_request_enable(struct pinctrl_dev *pctldev, dev_dbg(npct->dev, "enable pin %u as GPIO\n", offset); - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); bit = offset % NMK_GPIO_PER_CHIP; /* There is no glitch when converting any pin to GPIO */ __nmk_gpio_set_mode(nmk_chip, bit, NMK_GPIO_ALT_GPIO); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; } @@ -1773,7 +1773,7 @@ int nmk_pin_config_set(struct pinctrl_dev *pctldev, output ? (val ? "high" : "low") : "", lowemi ? "on" : "off" ); - clk_enable(nmk_chip->clk); + clk_prepare_enable(nmk_chip->clk); bit = pin % NMK_GPIO_PER_CHIP; if (gpiomode) /* No glitch when going to GPIO mode */ @@ -1788,7 +1788,7 @@ int nmk_pin_config_set(struct pinctrl_dev *pctldev, __nmk_gpio_set_lowemi(nmk_chip, bit, lowemi); __nmk_gpio_set_slpm(nmk_chip, bit, slpm); - clk_disable(nmk_chip->clk); + clk_disable_unprepare(nmk_chip->clk); return 0; }
The clock framework has changed somewhat and it's now better to invoke clock_prepare_enable() and clk_disable_unprepare() rather than the legacy clk_enable() and clk_disable() calls. This patch converts the Nomadik Pin Control driver to the new framework. Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/pinctrl/pinctrl-nomadik.c | 96 ++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 48 deletions(-)