| Message ID | 1377270227-1030-4-git-send-email-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 23, 2013 at 05:03:45PM +0200, Sylwester Nawrocki wrote: > This patch adds common __clk_get(), __clk_put() clkdev helpers which > replace their platform specific counterparts when the common clock > API is enabled. > > The owner module pointer field is added to struct clk so a reference > to the clock supplier module can be taken by the clock consumers. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> I'm mostly happy with this now. > +int __clk_get(struct clk *clk) > +{ > + if (clk && !try_module_get(clk->owner)) > + return 0; > + > + return 1; > +} > +EXPORT_SYMBOL(__clk_get); > + > +void __clk_put(struct clk *clk) > +{ > + if (WARN_ON_ONCE(IS_ERR(clk))) > + return; > + > + if (clk) > + module_put(clk->owner); > +} > +EXPORT_SYMBOL(__clk_put); Why are these exported? clkdev can only be built into the kernel, as can the common clk framework - they can't be modular. So why would a module wish to access these directly?
On 08/24/2013 01:13 AM, Russell King - ARM Linux wrote: > On Fri, Aug 23, 2013 at 05:03:45PM +0200, Sylwester Nawrocki wrote: >> > This patch adds common __clk_get(), __clk_put() clkdev helpers which >> > replace their platform specific counterparts when the common clock >> > API is enabled. >> > >> > The owner module pointer field is added to struct clk so a reference >> > to the clock supplier module can be taken by the clock consumers. >> > >> > Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com> >> > Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com> > > I'm mostly happy with this now. > >> > +int __clk_get(struct clk *clk) >> > +{ >> > + if (clk&& !try_module_get(clk->owner)) >> > + return 0; >> > + >> > + return 1; >> > +} >> > +EXPORT_SYMBOL(__clk_get); >> > + >> > +void __clk_put(struct clk *clk) >> > +{ >> > + if (WARN_ON_ONCE(IS_ERR(clk))) >> > + return; >> > + >> > + if (clk) >> > + module_put(clk->owner); >> > +} >> > +EXPORT_SYMBOL(__clk_put); > > Why are these exported? clkdev can only be built into the kernel, as can > the common clk framework - they can't be modular. So why would a module > wish to access these directly? I must have been mislead by the fact that some ARM sub-architecture exports those, have added them initially and then didn't think enough about it and left these in. Actually, at some point I noticed the exporting is not needed, but never did get around to remove it. Thanks. The updated series to follow.
diff --git a/arch/arm/include/asm/clkdev.h b/arch/arm/include/asm/clkdev.h index 80751c1..4e8a4b2 100644 --- a/arch/arm/include/asm/clkdev.h +++ b/arch/arm/include/asm/clkdev.h @@ -14,12 +14,14 @@ #include <linux/slab.h> +#ifndef CONFIG_COMMON_CLK #ifdef CONFIG_HAVE_MACH_CLKDEV #include <mach/clkdev.h> #else #define __clk_get(clk) ({ 1; }) #define __clk_put(clk) do { } while (0) #endif +#endif static inline struct clk_lookup_alloc *__clkdev_alloc(size_t size) { diff --git a/arch/blackfin/include/asm/clkdev.h b/arch/blackfin/include/asm/clkdev.h index 9053bed..7ac2436 100644 --- a/arch/blackfin/include/asm/clkdev.h +++ b/arch/blackfin/include/asm/clkdev.h @@ -8,7 +8,9 @@ static inline struct clk_lookup_alloc *__clkdev_alloc(size_t size) return kzalloc(size, GFP_KERNEL); } +#ifndef CONFIG_COMMON_CLK #define __clk_put(clk) #define __clk_get(clk) ({ 1; }) +#endif #endif diff --git a/arch/mips/include/asm/clkdev.h b/arch/mips/include/asm/clkdev.h index 2624754..1b3ad7b 100644 --- a/arch/mips/include/asm/clkdev.h +++ b/arch/mips/include/asm/clkdev.h @@ -14,8 +14,10 @@ #include <linux/slab.h> +#ifndef CONFIG_COMMON_CLK #define __clk_get(clk) ({ 1; }) #define __clk_put(clk) do { } while (0) +#endif static inline struct clk_lookup_alloc *__clkdev_alloc(size_t size) { diff --git a/arch/sh/include/asm/clkdev.h b/arch/sh/include/asm/clkdev.h index 6ba9186..c419014 100644 --- a/arch/sh/include/asm/clkdev.h +++ b/arch/sh/include/asm/clkdev.h @@ -25,7 +25,9 @@ static inline struct clk_lookup_alloc *__clkdev_alloc(size_t size) return kzalloc(size, GFP_KERNEL); } +#ifndef CONFIG_COMMON_CLK #define __clk_put(clk) #define __clk_get(clk) ({ 1; }) +#endif #endif /* __CLKDEV_H__ */ diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index eb3207e..c173f30 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1852,6 +1852,28 @@ void devm_clk_unregister(struct device *dev, struct clk *clk) } EXPORT_SYMBOL_GPL(devm_clk_unregister); +/* + * clkdev helpers + */ +int __clk_get(struct clk *clk) +{ + if (clk && !try_module_get(clk->owner)) + return 0; + + return 1; +} +EXPORT_SYMBOL(__clk_get); + +void __clk_put(struct clk *clk) +{ + if (WARN_ON_ONCE(IS_ERR(clk))) + return; + + if (clk) + module_put(clk->owner); +} +EXPORT_SYMBOL(__clk_put); + /*** clk rate change notifiers ***/ /** diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h index dd7adff..b7c0b58 100644 --- a/include/linux/clk-private.h +++ b/include/linux/clk-private.h @@ -25,10 +25,13 @@ #ifdef CONFIG_COMMON_CLK +struct module; + struct clk { const char *name; const struct clk_ops *ops; struct clk_hw *hw; + struct module *owner; struct clk *parent; const char **parent_names; struct clk **parents; diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 1ec14a7..2c42fde 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -133,6 +133,8 @@ struct clk_ops { void (*init)(struct clk_hw *hw); }; +struct module; + /** * struct clk_init_data - holds init data that's common to all clocks and is * shared between the clock provider and the common clock framework. diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h index a6a6f60..94bad77 100644 --- a/include/linux/clkdev.h +++ b/include/linux/clkdev.h @@ -43,4 +43,9 @@ int clk_add_alias(const char *, const char *, char *, struct device *); int clk_register_clkdev(struct clk *, const char *, const char *, ...); int clk_register_clkdevs(struct clk *, struct clk_lookup *, size_t); +#ifdef CONFIG_COMMON_CLK +int __clk_get(struct clk *clk); +void __clk_put(struct clk *clk); +#endif + #endif