| Message ID | 1316730422-20027-5-git-send-email-mturquette@ti.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote: > From: Jeremy Kerr <jeremy.kerr@canonical.com> > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > Signed-off-by: Jamie Iles <jamie@jamieiles.com> > Signed-off-by: Mike Turquette <mturquette@ti.com> > --- > Changes since v1: > Add copyright header > Fold in Jamie's patch for set-to-disable clks > Use BIT macro instead of shift > > drivers/clk/Kconfig | 4 ++ > drivers/clk/Makefile | 1 + > drivers/clk/clk-gate.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/clk.h | 13 ++++++++ > 4 files changed, 96 insertions(+), 0 deletions(-) > create mode 100644 drivers/clk/clk-gate.c > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index d8313d7..a78967c 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -12,3 +12,7 @@ config GENERIC_CLK > config GENERIC_CLK_FIXED > bool > depends on GENERIC_CLK > + > +config GENERIC_CLK_GATE > + bool > + depends on GENERIC_CLK I see zero documentation on what a "gated clock" is supposed to be or how it works, and there are zero comments in the code. It's kind of hard to review that way, and even harder to use. g.
On Sat, Sep 24, 2011 at 9:02 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote: >> From: Jeremy Kerr <jeremy.kerr@canonical.com> >> >> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> >> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> >> Signed-off-by: Jamie Iles <jamie@jamieiles.com> >> Signed-off-by: Mike Turquette <mturquette@ti.com> >> --- >> Changes since v1: >> Add copyright header >> Fold in Jamie's patch for set-to-disable clks >> Use BIT macro instead of shift >> >> drivers/clk/Kconfig | 4 ++ >> drivers/clk/Makefile | 1 + >> drivers/clk/clk-gate.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/clk.h | 13 ++++++++ >> 4 files changed, 96 insertions(+), 0 deletions(-) >> create mode 100644 drivers/clk/clk-gate.c >> >> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig >> index d8313d7..a78967c 100644 >> --- a/drivers/clk/Kconfig >> +++ b/drivers/clk/Kconfig >> @@ -12,3 +12,7 @@ config GENERIC_CLK >> config GENERIC_CLK_FIXED >> bool >> depends on GENERIC_CLK >> + >> +config GENERIC_CLK_GATE >> + bool >> + depends on GENERIC_CLK > > I see zero documentation on what a "gated clock" is supposed to be or > how it works, and there are zero comments in the code. It's kind of > hard to review that way, and even harder to use. Will add Documentation and re-post. Thanks, Mike > g. >
Mike, On 09/22/2011 05:26 PM, Mike Turquette wrote: > From: Jeremy Kerr <jeremy.kerr@canonical.com> > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > Signed-off-by: Jamie Iles <jamie@jamieiles.com> > Signed-off-by: Mike Turquette <mturquette@ti.com> > --- > Changes since v1: > Add copyright header > Fold in Jamie's patch for set-to-disable clks > Use BIT macro instead of shift > > drivers/clk/Kconfig | 4 ++ > drivers/clk/Makefile | 1 + > drivers/clk/clk-gate.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/clk.h | 13 ++++++++ > 4 files changed, 96 insertions(+), 0 deletions(-) > create mode 100644 drivers/clk/clk-gate.c > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index d8313d7..a78967c 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -12,3 +12,7 @@ config GENERIC_CLK > config GENERIC_CLK_FIXED > bool > depends on GENERIC_CLK > + > +config GENERIC_CLK_GATE > + bool > + depends on GENERIC_CLK > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index 9a3325a..d186446 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -2,3 +2,4 @@ > obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o > obj-$(CONFIG_GENERIC_CLK) += clk.o > obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o > +obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > new file mode 100644 > index 0000000..a1d8e79 > --- /dev/null > +++ b/drivers/clk/clk-gate.c > @@ -0,0 +1,78 @@ > +/* > + * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@canonical.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Simple clk gate implementation > + */ > + > +#include <linux/clk.h> > +#include <linux/module.h> > +#include <asm/io.h> use linux/io.h > + > +#define to_clk_gate(clk) container_of(clk, struct clk_gate, hw) > + > +static unsigned long clk_gate_get_rate(struct clk_hw *clk) > +{ > + return clk_get_rate(clk_get_parent(clk->clk)); > +} > + > +static void clk_gate_set_bit(struct clk_hw *clk) > +{ > + struct clk_gate *gate = to_clk_gate(clk); > + u32 reg; > + > + reg = __raw_readl(gate->reg); > + reg |= BIT(gate->bit_idx); > + __raw_writel(reg, gate->reg); Don't these read-mod-writes need a spinlock around it? It's possible to have an enable bits and dividers in the same register. If you did a set_rate and while doing an enable/disable, there would be a problem. Also, it may be 2 different clocks in the same register, so the spinlock needs to be shared and not per clock. > +} > + > +static void clk_gate_clear_bit(struct clk_hw *clk) > +{ > + struct clk_gate *gate = to_clk_gate(clk); > + u32 reg; > + > + reg = __raw_readl(gate->reg); > + reg &= ~BIT(gate->bit_idx); > + __raw_writel(reg, gate->reg); > +} > + > +static int clk_gate_enable_set(struct clk_hw *clk) > +{ > + clk_gate_set_bit(clk); > + > + return 0; > +} > + > +static void clk_gate_disable_clear(struct clk_hw *clk) > +{ > + clk_gate_clear_bit(clk); > +} > + > +struct clk_hw_ops clk_gate_set_enable_ops = { const? > + .recalc_rate = clk_gate_get_rate, > + .enable = clk_gate_enable_set, > + .disable = clk_gate_disable_clear, > +}; > +EXPORT_SYMBOL_GPL(clk_gate_set_enable_ops); > + > +static int clk_gate_enable_clear(struct clk_hw *clk) > +{ > + clk_gate_clear_bit(clk); > + > + return 0; > +} > + > +static void clk_gate_disable_set(struct clk_hw *clk) > +{ > + clk_gate_set_bit(clk); > +} Are these wrapper functions really needed? Just assign set_bit and clear_bit functions directly to the ops structs. Only the ops struct name is exposed to the user. > + > +struct clk_hw_ops clk_gate_set_disable_ops = { const? Rob
Hi Rob, On Mon, Sep 26, 2011 at 01:33:08PM -0500, Rob Herring wrote: > Mike, > > On 09/22/2011 05:26 PM, Mike Turquette wrote: > > From: Jeremy Kerr <jeremy.kerr@canonical.com> > > > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > > Signed-off-by: Jamie Iles <jamie@jamieiles.com> > > Signed-off-by: Mike Turquette <mturquette@ti.com> > > --- > > Changes since v1: > > Add copyright header > > Fold in Jamie's patch for set-to-disable clks > > Use BIT macro instead of shift > > > > drivers/clk/Kconfig | 4 ++ > > drivers/clk/Makefile | 1 + > > drivers/clk/clk-gate.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/clk.h | 13 ++++++++ > > 4 files changed, 96 insertions(+), 0 deletions(-) > > create mode 100644 drivers/clk/clk-gate.c > > > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > > index d8313d7..a78967c 100644 > > --- a/drivers/clk/Kconfig > > +++ b/drivers/clk/Kconfig > > @@ -12,3 +12,7 @@ config GENERIC_CLK > > config GENERIC_CLK_FIXED > > bool > > depends on GENERIC_CLK > > + > > +config GENERIC_CLK_GATE > > + bool > > + depends on GENERIC_CLK > > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > > index 9a3325a..d186446 100644 > > --- a/drivers/clk/Makefile > > +++ b/drivers/clk/Makefile > > @@ -2,3 +2,4 @@ > > obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o > > obj-$(CONFIG_GENERIC_CLK) += clk.o > > obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o > > +obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o > > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > > new file mode 100644 > > index 0000000..a1d8e79 > > --- /dev/null > > +++ b/drivers/clk/clk-gate.c > > @@ -0,0 +1,78 @@ > > +/* > > + * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@canonical.com> > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + * Simple clk gate implementation > > + */ > > + > > +#include <linux/clk.h> > > +#include <linux/module.h> > > +#include <asm/io.h> > > use linux/io.h > > > + > > +#define to_clk_gate(clk) container_of(clk, struct clk_gate, hw) > > + > > +static unsigned long clk_gate_get_rate(struct clk_hw *clk) > > +{ > > + return clk_get_rate(clk_get_parent(clk->clk)); > > +} > > + > > +static void clk_gate_set_bit(struct clk_hw *clk) > > +{ > > + struct clk_gate *gate = to_clk_gate(clk); > > + u32 reg; > > + > > + reg = __raw_readl(gate->reg); > > + reg |= BIT(gate->bit_idx); > > + __raw_writel(reg, gate->reg); > > Don't these read-mod-writes need a spinlock around it? > > It's possible to have an enable bits and dividers in the same register. > If you did a set_rate and while doing an enable/disable, there would be > a problem. Also, it may be 2 different clocks in the same register, so > the spinlock needs to be shared and not per clock. Well the prepare lock will be held here and I believe that would be sufficient. > > +} > > + > > +static void clk_gate_clear_bit(struct clk_hw *clk) > > +{ > > + struct clk_gate *gate = to_clk_gate(clk); > > + u32 reg; > > + > > + reg = __raw_readl(gate->reg); > > + reg &= ~BIT(gate->bit_idx); > > + __raw_writel(reg, gate->reg); > > +} > > + > > +static int clk_gate_enable_set(struct clk_hw *clk) > > +{ > > + clk_gate_set_bit(clk); > > + > > + return 0; > > +} > > + > > +static void clk_gate_disable_clear(struct clk_hw *clk) > > +{ > > + clk_gate_clear_bit(clk); > > +} > > + > > +struct clk_hw_ops clk_gate_set_enable_ops = { > > const? Yup. > > + .recalc_rate = clk_gate_get_rate, > > + .enable = clk_gate_enable_set, > > + .disable = clk_gate_disable_clear, > > +}; > > +EXPORT_SYMBOL_GPL(clk_gate_set_enable_ops); > > + > > +static int clk_gate_enable_clear(struct clk_hw *clk) > > +{ > > + clk_gate_clear_bit(clk); > > + > > + return 0; > > +} > > + > > +static void clk_gate_disable_set(struct clk_hw *clk) > > +{ > > + clk_gate_set_bit(clk); > > +} > > Are these wrapper functions really needed? Just assign set_bit and > clear_bit functions directly to the ops structs. Only the ops struct > name is exposed to the user. I used the wrappers because the .enable method has to return an int, but the disable needs to return void. It's either that or open code the set/clear in each. > > + > > +struct clk_hw_ops clk_gate_set_disable_ops = { > > const? Yes. Jamie
On 09/26/2011 01:40 PM, Jamie Iles wrote: > Hi Rob, > > On Mon, Sep 26, 2011 at 01:33:08PM -0500, Rob Herring wrote: >> Mike, >> >> On 09/22/2011 05:26 PM, Mike Turquette wrote: >>> From: Jeremy Kerr <jeremy.kerr@canonical.com> >>> >>> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> >>> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> >>> Signed-off-by: Jamie Iles <jamie@jamieiles.com> >>> Signed-off-by: Mike Turquette <mturquette@ti.com> >>> --- >>> Changes since v1: >>> Add copyright header >>> Fold in Jamie's patch for set-to-disable clks >>> Use BIT macro instead of shift >>> >>> drivers/clk/Kconfig | 4 ++ >>> drivers/clk/Makefile | 1 + >>> drivers/clk/clk-gate.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ >>> include/linux/clk.h | 13 ++++++++ >>> 4 files changed, 96 insertions(+), 0 deletions(-) >>> create mode 100644 drivers/clk/clk-gate.c >>> >>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig >>> index d8313d7..a78967c 100644 >>> --- a/drivers/clk/Kconfig >>> +++ b/drivers/clk/Kconfig >>> @@ -12,3 +12,7 @@ config GENERIC_CLK >>> config GENERIC_CLK_FIXED >>> bool >>> depends on GENERIC_CLK >>> + >>> +config GENERIC_CLK_GATE >>> + bool >>> + depends on GENERIC_CLK >>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile >>> index 9a3325a..d186446 100644 >>> --- a/drivers/clk/Makefile >>> +++ b/drivers/clk/Makefile >>> @@ -2,3 +2,4 @@ >>> obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o >>> obj-$(CONFIG_GENERIC_CLK) += clk.o >>> obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o >>> +obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o >>> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c >>> new file mode 100644 >>> index 0000000..a1d8e79 >>> --- /dev/null >>> +++ b/drivers/clk/clk-gate.c >>> @@ -0,0 +1,78 @@ >>> +/* >>> + * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@canonical.com> >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + * >>> + * Simple clk gate implementation >>> + */ >>> + >>> +#include <linux/clk.h> >>> +#include <linux/module.h> >>> +#include <asm/io.h> >> >> use linux/io.h >> >>> + >>> +#define to_clk_gate(clk) container_of(clk, struct clk_gate, hw) >>> + >>> +static unsigned long clk_gate_get_rate(struct clk_hw *clk) >>> +{ >>> + return clk_get_rate(clk_get_parent(clk->clk)); >>> +} >>> + >>> +static void clk_gate_set_bit(struct clk_hw *clk) >>> +{ >>> + struct clk_gate *gate = to_clk_gate(clk); >>> + u32 reg; >>> + >>> + reg = __raw_readl(gate->reg); >>> + reg |= BIT(gate->bit_idx); >>> + __raw_writel(reg, gate->reg); >> >> Don't these read-mod-writes need a spinlock around it? >> >> It's possible to have an enable bits and dividers in the same register. >> If you did a set_rate and while doing an enable/disable, there would be >> a problem. Also, it may be 2 different clocks in the same register, so >> the spinlock needs to be shared and not per clock. > > Well the prepare lock will be held here and I believe that would be > sufficient. No, the enable spinlock is protecting enable/disable. But set_rate is protected by the prepare mutex. So you clearly don't need locking if you have a register of only 1 bit enables. If you have a register accessed by both enable/disable and prepare/unprepare/set_rate, then you need some protection. > >>> +} >>> + >>> +static void clk_gate_clear_bit(struct clk_hw *clk) >>> +{ >>> + struct clk_gate *gate = to_clk_gate(clk); >>> + u32 reg; >>> + >>> + reg = __raw_readl(gate->reg); >>> + reg &= ~BIT(gate->bit_idx); >>> + __raw_writel(reg, gate->reg); >>> +} >>> + >>> +static int clk_gate_enable_set(struct clk_hw *clk) >>> +{ >>> + clk_gate_set_bit(clk); >>> + >>> + return 0; >>> +} >>> + >>> +static void clk_gate_disable_clear(struct clk_hw *clk) >>> +{ >>> + clk_gate_clear_bit(clk); >>> +} >>> + >>> +struct clk_hw_ops clk_gate_set_enable_ops = { >> >> const? > > Yup. > >>> + .recalc_rate = clk_gate_get_rate, >>> + .enable = clk_gate_enable_set, >>> + .disable = clk_gate_disable_clear, >>> +}; >>> +EXPORT_SYMBOL_GPL(clk_gate_set_enable_ops); >>> + >>> +static int clk_gate_enable_clear(struct clk_hw *clk) >>> +{ >>> + clk_gate_clear_bit(clk); >>> + >>> + return 0; >>> +} >>> + >>> +static void clk_gate_disable_set(struct clk_hw *clk) >>> +{ >>> + clk_gate_set_bit(clk); >>> +} >> >> Are these wrapper functions really needed? Just assign set_bit and >> clear_bit functions directly to the ops structs. Only the ops struct >> name is exposed to the user. > > I used the wrappers because the .enable method has to return an int, but > the disable needs to return void. It's either that or open code the > set/clear in each. Okay. I missed that detail... Rob
On Mon, Sep 26, 2011 at 02:10:32PM -0500, Rob Herring wrote: > On 09/26/2011 01:40 PM, Jamie Iles wrote: > > On Mon, Sep 26, 2011 at 01:33:08PM -0500, Rob Herring wrote: > >>> +static void clk_gate_set_bit(struct clk_hw *clk) > >>> +{ > >>> + struct clk_gate *gate = to_clk_gate(clk); > >>> + u32 reg; > >>> + > >>> + reg = __raw_readl(gate->reg); > >>> + reg |= BIT(gate->bit_idx); > >>> + __raw_writel(reg, gate->reg); > >> > >> Don't these read-mod-writes need a spinlock around it? > >> > >> It's possible to have an enable bits and dividers in the same register. > >> If you did a set_rate and while doing an enable/disable, there would be > >> a problem. Also, it may be 2 different clocks in the same register, so > >> the spinlock needs to be shared and not per clock. > > > > Well the prepare lock will be held here and I believe that would be > > sufficient. > > No, the enable spinlock is protecting enable/disable. But set_rate is > protected by the prepare mutex. So you clearly don't need locking if you > have a register of only 1 bit enables. If you have a register accessed > by both enable/disable and prepare/unprepare/set_rate, then you need > some protection. OK fair point, but I would guess that if you had a clock like this then you probably wouldn't use this simple gated clock would you? (speaking from my world where we have quite simple clocks ;-)) Jamie
On Mon, Sep 26, 2011 at 12:37 PM, Jamie Iles <jamie@jamieiles.com> wrote: > On Mon, Sep 26, 2011 at 02:10:32PM -0500, Rob Herring wrote: >> On 09/26/2011 01:40 PM, Jamie Iles wrote: >> > On Mon, Sep 26, 2011 at 01:33:08PM -0500, Rob Herring wrote: >> >>> +static void clk_gate_set_bit(struct clk_hw *clk) >> >>> +{ >> >>> + struct clk_gate *gate = to_clk_gate(clk); >> >>> + u32 reg; >> >>> + >> >>> + reg = __raw_readl(gate->reg); >> >>> + reg |= BIT(gate->bit_idx); >> >>> + __raw_writel(reg, gate->reg); >> >> >> >> Don't these read-mod-writes need a spinlock around it? >> >> >> >> It's possible to have an enable bits and dividers in the same register. >> >> If you did a set_rate and while doing an enable/disable, there would be >> >> a problem. Also, it may be 2 different clocks in the same register, so >> >> the spinlock needs to be shared and not per clock. >> > >> > Well the prepare lock will be held here and I believe that would be >> > sufficient. >> >> No, the enable spinlock is protecting enable/disable. But set_rate is >> protected by the prepare mutex. So you clearly don't need locking if you >> have a register of only 1 bit enables. If you have a register accessed >> by both enable/disable and prepare/unprepare/set_rate, then you need >> some protection. > > OK fair point, but I would guess that if you had a clock like this then > you probably wouldn't use this simple gated clock would you? (speaking > from my world where we have quite simple clocks ;-)) I think it is a safe assumption that if a register controls both enable/disable and some programmable divider then, 1) those controls are probably for the same clock 2) that clock won't be using the cookie-cutter gated-clock implementation anyways Rob, do you feel these assumptions are OK and locking can remain the same in this patch? Regards, Mike > Jamie >
On 09/26/2011 05:37 PM, Turquette, Mike wrote: > On Mon, Sep 26, 2011 at 12:37 PM, Jamie Iles <jamie@jamieiles.com> wrote: >> On Mon, Sep 26, 2011 at 02:10:32PM -0500, Rob Herring wrote: >>> On 09/26/2011 01:40 PM, Jamie Iles wrote: >>>> On Mon, Sep 26, 2011 at 01:33:08PM -0500, Rob Herring wrote: >>>>>> +static void clk_gate_set_bit(struct clk_hw *clk) >>>>>> +{ >>>>>> + struct clk_gate *gate = to_clk_gate(clk); >>>>>> + u32 reg; >>>>>> + >>>>>> + reg = __raw_readl(gate->reg); >>>>>> + reg |= BIT(gate->bit_idx); >>>>>> + __raw_writel(reg, gate->reg); >>>>> >>>>> Don't these read-mod-writes need a spinlock around it? >>>>> >>>>> It's possible to have an enable bits and dividers in the same register. >>>>> If you did a set_rate and while doing an enable/disable, there would be >>>>> a problem. Also, it may be 2 different clocks in the same register, so >>>>> the spinlock needs to be shared and not per clock. >>>> >>>> Well the prepare lock will be held here and I believe that would be >>>> sufficient. >>> >>> No, the enable spinlock is protecting enable/disable. But set_rate is >>> protected by the prepare mutex. So you clearly don't need locking if you >>> have a register of only 1 bit enables. If you have a register accessed >>> by both enable/disable and prepare/unprepare/set_rate, then you need >>> some protection. >> >> OK fair point, but I would guess that if you had a clock like this then >> you probably wouldn't use this simple gated clock would you? (speaking >> from my world where we have quite simple clocks ;-)) > > I think it is a safe assumption that if a register controls both > enable/disable and some programmable divider then, > > 1) those controls are probably for the same clock > 2) that clock won't be using the cookie-cutter gated-clock > implementation anyways By definition of simple gated clock, the other bits have to be for another clock. The restriction is that all the other bits can only be clock gate bits. > > Rob, do you feel these assumptions are OK and locking can remain the > same in this patch? Perhaps it is rare enough that it is not worth it use generic code in this case. If so, the documentation should be clear about this constraint. It is not something anyone will have hit before because everyone used a single global lock. Now with the api being split between 2 locks, this adds a new complexity. I think the simple gated clock code should be usable for any clock controlled by a single bit in a 32-bit register independent of other things in that register. One example is MX1 in (mach-imx/clock-imx1.c). The CSCR register has single bit enable for clk16m while hclk and clk48m have dividers in that register. Rob
On 09/26/2011 04:30 PM, Rob Herring wrote: > On 09/26/2011 05:37 PM, Turquette, Mike wrote: >> On Mon, Sep 26, 2011 at 12:37 PM, Jamie Iles<jamie@jamieiles.com> wrote: >>> On Mon, Sep 26, 2011 at 02:10:32PM -0500, Rob Herring wrote: >>>> On 09/26/2011 01:40 PM, Jamie Iles wrote: >>>>> On Mon, Sep 26, 2011 at 01:33:08PM -0500, Rob Herring wrote: >>>>>>> +static void clk_gate_set_bit(struct clk_hw *clk) >>>>>>> +{ >>>>>>> + struct clk_gate *gate = to_clk_gate(clk); >>>>>>> + u32 reg; >>>>>>> + >>>>>>> + reg = __raw_readl(gate->reg); >>>>>>> + reg |= BIT(gate->bit_idx); >>>>>>> + __raw_writel(reg, gate->reg); >>>>>> >>>>>> Don't these read-mod-writes need a spinlock around it? >>>>>> >>>>>> It's possible to have an enable bits and dividers in the same register. >>>>>> If you did a set_rate and while doing an enable/disable, there would be >>>>>> a problem. Also, it may be 2 different clocks in the same register, so >>>>>> the spinlock needs to be shared and not per clock. >>>>> >>>>> Well the prepare lock will be held here and I believe that would be >>>>> sufficient. >>>> >>>> No, the enable spinlock is protecting enable/disable. But set_rate is >>>> protected by the prepare mutex. So you clearly don't need locking if you >>>> have a register of only 1 bit enables. If you have a register accessed >>>> by both enable/disable and prepare/unprepare/set_rate, then you need >>>> some protection. >>> >>> OK fair point, but I would guess that if you had a clock like this then >>> you probably wouldn't use this simple gated clock would you? (speaking >>> from my world where we have quite simple clocks ;-)) >> >> I think it is a safe assumption that if a register controls both >> enable/disable and some programmable divider then, >> >> 1) those controls are probably for the same clock >> 2) that clock won't be using the cookie-cutter gated-clock >> implementation anyways > > By definition of simple gated clock, the other bits have to be for > another clock. The restriction is that all the other bits can only be > clock gate bits. > >> >> Rob, do you feel these assumptions are OK and locking can remain the >> same in this patch? > > Perhaps it is rare enough that it is not worth it use generic code in > this case. If so, the documentation should be clear about this > constraint. It is not something anyone will have hit before because > everyone used a single global lock. Now with the api being split between > 2 locks, this adds a new complexity. I kinda agree with Rob on this. There are very few, if any, such simple clocks on MSM chips. It's very easy to a SoC clock developer to accidentally use these simple clocks without realizing the point that Rob brings up. > I think the simple gated clock code should be usable for any clock > controlled by a single bit in a 32-bit register independent of other > things in that register. To take care of the scenario Rob bring up, the prepare/unprepare and enable/disable code will have to grab a per-tree register-lock before accessing any registers. The prepare/unprepare code should obviously be written to hold this register-lock for as small of a duration as possible. For example, if the prepare code is doing voltage increase, the register-lock should be grabber _after_ the voltage is increased. At least, this is approximately how the MSM clock code can be mapped onto this generic framework. I think we should just go ahead and implement the per-tree register lock so that the generic clock implementations are more useful. The lock will really be held only for a very short time and hence shouldn't matter that there is a single lock for all the clocks in a tree. Thomas, Did you get a chance to send out your patches with support for per-tree locking? I would really like to see that as part of the first patch series that gets pulled in. Thanks, Saravana
On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote: > From: Jeremy Kerr <jeremy.kerr@canonical.com> > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > Signed-off-by: Jamie Iles <jamie@jamieiles.com> > Signed-off-by: Mike Turquette <mturquette@ti.com> > --- > Changes since v1: > Add copyright header > Fold in Jamie's patch for set-to-disable clks > Use BIT macro instead of shift > > drivers/clk/Kconfig | 4 ++ > drivers/clk/Makefile | 1 + > drivers/clk/clk-gate.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/clk.h | 13 ++++++++ > 4 files changed, 96 insertions(+), 0 deletions(-) > create mode 100644 drivers/clk/clk-gate.c I feel hard to tell the tree the clk parent, at register/init time. For the simple gate clk, the only way is to set .get_parent. But normally, for clk without any divider we set .get_parent to NULL. Maybe we can put .parent to struct clk_hw? Thanks Richard > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index d8313d7..a78967c 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -12,3 +12,7 @@ config GENERIC_CLK > config GENERIC_CLK_FIXED > bool > depends on GENERIC_CLK > + > +config GENERIC_CLK_GATE > + bool > + depends on GENERIC_CLK > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index 9a3325a..d186446 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -2,3 +2,4 @@ > obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o > obj-$(CONFIG_GENERIC_CLK) += clk.o > obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o > +obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > new file mode 100644 > index 0000000..a1d8e79 > --- /dev/null > +++ b/drivers/clk/clk-gate.c > @@ -0,0 +1,78 @@ > +/* > + * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@canonical.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Simple clk gate implementation > + */ > + > +#include <linux/clk.h> > +#include <linux/module.h> > +#include <asm/io.h> > + > +#define to_clk_gate(clk) container_of(clk, struct clk_gate, hw) > + > +static unsigned long clk_gate_get_rate(struct clk_hw *clk) > +{ > + return clk_get_rate(clk_get_parent(clk->clk)); > +} > + > +static void clk_gate_set_bit(struct clk_hw *clk) > +{ > + struct clk_gate *gate = to_clk_gate(clk); > + u32 reg; > + > + reg = __raw_readl(gate->reg); > + reg |= BIT(gate->bit_idx); > + __raw_writel(reg, gate->reg); > +} > + > +static void clk_gate_clear_bit(struct clk_hw *clk) > +{ > + struct clk_gate *gate = to_clk_gate(clk); > + u32 reg; > + > + reg = __raw_readl(gate->reg); > + reg &= ~BIT(gate->bit_idx); > + __raw_writel(reg, gate->reg); > +} > + > +static int clk_gate_enable_set(struct clk_hw *clk) > +{ > + clk_gate_set_bit(clk); > + > + return 0; > +} > + > +static void clk_gate_disable_clear(struct clk_hw *clk) > +{ > + clk_gate_clear_bit(clk); > +} > + > +struct clk_hw_ops clk_gate_set_enable_ops = { > + .recalc_rate = clk_gate_get_rate, > + .enable = clk_gate_enable_set, > + .disable = clk_gate_disable_clear, > +}; > +EXPORT_SYMBOL_GPL(clk_gate_set_enable_ops); > + > +static int clk_gate_enable_clear(struct clk_hw *clk) > +{ > + clk_gate_clear_bit(clk); > + > + return 0; > +} > + > +static void clk_gate_disable_set(struct clk_hw *clk) > +{ > + clk_gate_set_bit(clk); > +} > + > +struct clk_hw_ops clk_gate_set_disable_ops = { > + .recalc_rate = clk_gate_get_rate, > + .enable = clk_gate_enable_clear, > + .disable = clk_gate_disable_set, > +}; > +EXPORT_SYMBOL_GPL(clk_gate_set_disable_ops); > diff --git a/include/linux/clk.h b/include/linux/clk.h > index 1903dd8..626fd0d 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -124,6 +124,19 @@ extern struct clk_hw_ops clk_fixed_ops; > > #endif /* CONFIG_GENERIC_CLK_FIXED */ > > +#ifdef CONFIG_GENERIC_CLK_GATE > + > +struct clk_gate { > + struct clk_hw hw; > + void __iomem *reg; > + u8 bit_idx; > +}; > + > +extern struct clk_hw_ops clk_gate_set_enable_ops; > +extern struct clk_hw_ops clk_gate_set_disable_ops; > + > +#endif /* CONFIG_GENERIC_CLK_GATE */ > + > /** > * clk_register - register and initialize a new clock > * > -- > 1.7.4.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Tue, Oct 11, 2011 at 11:46 PM, Richard Zhao <richard.zhao@freescale.com> wrote: > On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote: >> From: Jeremy Kerr <jeremy.kerr@canonical.com> >> >> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> >> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> >> Signed-off-by: Jamie Iles <jamie@jamieiles.com> >> Signed-off-by: Mike Turquette <mturquette@ti.com> >> --- >> Changes since v1: >> Add copyright header >> Fold in Jamie's patch for set-to-disable clks >> Use BIT macro instead of shift >> >> drivers/clk/Kconfig | 4 ++ >> drivers/clk/Makefile | 1 + >> drivers/clk/clk-gate.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/clk.h | 13 ++++++++ >> 4 files changed, 96 insertions(+), 0 deletions(-) >> create mode 100644 drivers/clk/clk-gate.c > > I feel hard to tell the tree the clk parent, at register/init time. For the > simple gate clk, the only way is to set .get_parent. But normally, for clk > without any divider we set .get_parent to NULL. Maybe we can put .parent to > struct clk_hw? For non-mux clocks, whose parent is *always* going to be the same, you should create a duplicate .parent in the clk_hw_* structure and then have .get_parent return clk_hw_*->parent. This is analogous to the way clk_hw_fixed returns clk_hw_fixed->rate when .recalc is called on it. Regards, Mike > Thanks > Richard >> >> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig >> index d8313d7..a78967c 100644 >> --- a/drivers/clk/Kconfig >> +++ b/drivers/clk/Kconfig >> @@ -12,3 +12,7 @@ config GENERIC_CLK >> config GENERIC_CLK_FIXED >> bool >> depends on GENERIC_CLK >> + >> +config GENERIC_CLK_GATE >> + bool >> + depends on GENERIC_CLK >> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile >> index 9a3325a..d186446 100644 >> --- a/drivers/clk/Makefile >> +++ b/drivers/clk/Makefile >> @@ -2,3 +2,4 @@ >> obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o >> obj-$(CONFIG_GENERIC_CLK) += clk.o >> obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o >> +obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o >> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c >> new file mode 100644 >> index 0000000..a1d8e79 >> --- /dev/null >> +++ b/drivers/clk/clk-gate.c >> @@ -0,0 +1,78 @@ >> +/* >> + * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@canonical.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * Simple clk gate implementation >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/module.h> >> +#include <asm/io.h> >> + >> +#define to_clk_gate(clk) container_of(clk, struct clk_gate, hw) >> + >> +static unsigned long clk_gate_get_rate(struct clk_hw *clk) >> +{ >> + return clk_get_rate(clk_get_parent(clk->clk)); >> +} >> + >> +static void clk_gate_set_bit(struct clk_hw *clk) >> +{ >> + struct clk_gate *gate = to_clk_gate(clk); >> + u32 reg; >> + >> + reg = __raw_readl(gate->reg); >> + reg |= BIT(gate->bit_idx); >> + __raw_writel(reg, gate->reg); >> +} >> + >> +static void clk_gate_clear_bit(struct clk_hw *clk) >> +{ >> + struct clk_gate *gate = to_clk_gate(clk); >> + u32 reg; >> + >> + reg = __raw_readl(gate->reg); >> + reg &= ~BIT(gate->bit_idx); >> + __raw_writel(reg, gate->reg); >> +} >> + >> +static int clk_gate_enable_set(struct clk_hw *clk) >> +{ >> + clk_gate_set_bit(clk); >> + >> + return 0; >> +} >> + >> +static void clk_gate_disable_clear(struct clk_hw *clk) >> +{ >> + clk_gate_clear_bit(clk); >> +} >> + >> +struct clk_hw_ops clk_gate_set_enable_ops = { >> + .recalc_rate = clk_gate_get_rate, >> + .enable = clk_gate_enable_set, >> + .disable = clk_gate_disable_clear, >> +}; >> +EXPORT_SYMBOL_GPL(clk_gate_set_enable_ops); >> + >> +static int clk_gate_enable_clear(struct clk_hw *clk) >> +{ >> + clk_gate_clear_bit(clk); >> + >> + return 0; >> +} >> + >> +static void clk_gate_disable_set(struct clk_hw *clk) >> +{ >> + clk_gate_set_bit(clk); >> +} >> + >> +struct clk_hw_ops clk_gate_set_disable_ops = { >> + .recalc_rate = clk_gate_get_rate, >> + .enable = clk_gate_enable_clear, >> + .disable = clk_gate_disable_set, >> +}; >> +EXPORT_SYMBOL_GPL(clk_gate_set_disable_ops); >> diff --git a/include/linux/clk.h b/include/linux/clk.h >> index 1903dd8..626fd0d 100644 >> --- a/include/linux/clk.h >> +++ b/include/linux/clk.h >> @@ -124,6 +124,19 @@ extern struct clk_hw_ops clk_fixed_ops; >> >> #endif /* CONFIG_GENERIC_CLK_FIXED */ >> >> +#ifdef CONFIG_GENERIC_CLK_GATE >> + >> +struct clk_gate { >> + struct clk_hw hw; >> + void __iomem *reg; >> + u8 bit_idx; >> +}; >> + >> +extern struct clk_hw_ops clk_gate_set_enable_ops; >> +extern struct clk_hw_ops clk_gate_set_disable_ops; >> + >> +#endif /* CONFIG_GENERIC_CLK_GATE */ >> + >> /** >> * clk_register - register and initialize a new clock >> * >> -- >> 1.7.4.1 >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> > >
On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote: > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > new file mode 100644 > index 0000000..a1d8e79 > --- /dev/null > +++ b/drivers/clk/clk-gate.c > @@ -0,0 +1,78 @@ > +/* > + * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@canonical.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Simple clk gate implementation > + */ > + > +#include <linux/clk.h> > +#include <linux/module.h> > +#include <asm/io.h> linux/io.h please.
On Thu, Oct 13, 2011 at 7:45 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote: >> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c >> new file mode 100644 >> index 0000000..a1d8e79 >> --- /dev/null >> +++ b/drivers/clk/clk-gate.c >> @@ -0,0 +1,78 @@ >> +/* >> + * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@canonical.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * Simple clk gate implementation >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/module.h> >> +#include <asm/io.h> > > linux/io.h please. > Will roll into v3. Thanks for reviewing, Mike
On Wed, Oct 12, 2011 at 07:59:19AM -0700, Turquette, Mike wrote: > On Tue, Oct 11, 2011 at 11:46 PM, Richard Zhao > <richard.zhao@freescale.com> wrote: > > On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote: > >> From: Jeremy Kerr <jeremy.kerr@canonical.com> > >> > >> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > >> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > >> Signed-off-by: Jamie Iles <jamie@jamieiles.com> > >> Signed-off-by: Mike Turquette <mturquette@ti.com> > >> --- > >> Changes since v1: > >> Add copyright header > >> Fold in Jamie's patch for set-to-disable clks > >> Use BIT macro instead of shift > >> > >> drivers/clk/Kconfig | 4 ++ > >> drivers/clk/Makefile | 1 + > >> drivers/clk/clk-gate.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ > >> include/linux/clk.h | 13 ++++++++ > >> 4 files changed, 96 insertions(+), 0 deletions(-) > >> create mode 100644 drivers/clk/clk-gate.c > > > > I feel hard to tell the tree the clk parent, at register/init time. For the > > simple gate clk, the only way is to set .get_parent. But normally, for clk > > without any divider we set .get_parent to NULL. Maybe we can put .parent to > > struct clk_hw? > > For non-mux clocks, whose parent is *always* going to be the same, you > should create a duplicate .parent in the clk_hw_* structure and then > have .get_parent return clk_hw_*->parent. Maybe I do not understand what you mean here, but I think there is something missing in the gate. > > This is analogous to the way clk_hw_fixed returns clk_hw_fixed->rate > when .recalc is called on it. > > >> + > >> +static unsigned long clk_gate_get_rate(struct clk_hw *clk) > >> +{ > >> + return clk_get_rate(clk_get_parent(clk->clk)); > >> +} clk_get_parent goes down to clk_gate_set_enable_ops.get_parent below... > >> + > >> + > >> +struct clk_hw_ops clk_gate_set_enable_ops = { > >> + .recalc_rate = clk_gate_get_rate, > >> + .enable = clk_gate_enable_set, > >> + .disable = clk_gate_disable_clear, > >> +}; ...but this does not have a get_parent pointer, so clk_get_parent() for a gate always returns NULL which means that a gate never has a valid rate. Sascha
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index d8313d7..a78967c 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -12,3 +12,7 @@ config GENERIC_CLK config GENERIC_CLK_FIXED bool depends on GENERIC_CLK + +config GENERIC_CLK_GATE + bool + depends on GENERIC_CLK diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 9a3325a..d186446 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o obj-$(CONFIG_GENERIC_CLK) += clk.o obj-$(CONFIG_GENERIC_CLK_FIXED) += clk-fixed.o +obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c new file mode 100644 index 0000000..a1d8e79 --- /dev/null +++ b/drivers/clk/clk-gate.c @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@canonical.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Simple clk gate implementation + */ + +#include <linux/clk.h> +#include <linux/module.h> +#include <asm/io.h> + +#define to_clk_gate(clk) container_of(clk, struct clk_gate, hw) + +static unsigned long clk_gate_get_rate(struct clk_hw *clk) +{ + return clk_get_rate(clk_get_parent(clk->clk)); +} + +static void clk_gate_set_bit(struct clk_hw *clk) +{ + struct clk_gate *gate = to_clk_gate(clk); + u32 reg; + + reg = __raw_readl(gate->reg); + reg |= BIT(gate->bit_idx); + __raw_writel(reg, gate->reg); +} + +static void clk_gate_clear_bit(struct clk_hw *clk) +{ + struct clk_gate *gate = to_clk_gate(clk); + u32 reg; + + reg = __raw_readl(gate->reg); + reg &= ~BIT(gate->bit_idx); + __raw_writel(reg, gate->reg); +} + +static int clk_gate_enable_set(struct clk_hw *clk) +{ + clk_gate_set_bit(clk); + + return 0; +} + +static void clk_gate_disable_clear(struct clk_hw *clk) +{ + clk_gate_clear_bit(clk); +} + +struct clk_hw_ops clk_gate_set_enable_ops = { + .recalc_rate = clk_gate_get_rate, + .enable = clk_gate_enable_set, + .disable = clk_gate_disable_clear, +}; +EXPORT_SYMBOL_GPL(clk_gate_set_enable_ops); + +static int clk_gate_enable_clear(struct clk_hw *clk) +{ + clk_gate_clear_bit(clk); + + return 0; +} + +static void clk_gate_disable_set(struct clk_hw *clk) +{ + clk_gate_set_bit(clk); +} + +struct clk_hw_ops clk_gate_set_disable_ops = { + .recalc_rate = clk_gate_get_rate, + .enable = clk_gate_enable_clear, + .disable = clk_gate_disable_set, +}; +EXPORT_SYMBOL_GPL(clk_gate_set_disable_ops); diff --git a/include/linux/clk.h b/include/linux/clk.h index 1903dd8..626fd0d 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -124,6 +124,19 @@ extern struct clk_hw_ops clk_fixed_ops; #endif /* CONFIG_GENERIC_CLK_FIXED */ +#ifdef CONFIG_GENERIC_CLK_GATE + +struct clk_gate { + struct clk_hw hw; + void __iomem *reg; + u8 bit_idx; +}; + +extern struct clk_hw_ops clk_gate_set_enable_ops; +extern struct clk_hw_ops clk_gate_set_disable_ops; + +#endif /* CONFIG_GENERIC_CLK_GATE */ + /** * clk_register - register and initialize a new clock *