From a1c8d531c694e193dc48ef2428877d5ea2e35134 Mon Sep 17 00:00:00 2001 From: Jerome Brunet Date: Thu, 23 Jul 2026 15:00:51 +0200 Subject: [PATCH] clk: fix self-consuming provider module pinning clk_hw_get_clk() lets a provider get a struct clk for one of its own struct clk_hw. When a struct clk is created, the module usage count of the provider is unconditionally increased. For a self-consuming provider, this means it pins itself and the module can never be unloaded. Increasing the module usage count should only be done when the consumer lives in a different module from the provider. Use THIS_MODULE to capture caller's module and increase the module usage count accordingly. It is OK for consumer-only APIs such as clk_get() or of_clk_get() to pass a NULL owner. As a result, any provider module will get pinned, same as before. Fixes: 30d6f8c15d2c ("clk: add api to get clk consumer from clk_hw") Signed-off-by: Jerome Brunet Signed-off-by: Brian Masney --- drivers/clk/clk.c | 46 ++++++++++++++++++++---------------- drivers/clk/clk.h | 5 ++-- drivers/clk/clkdev.c | 4 ++-- include/linux/clk-provider.h | 22 ++++++++++++++--- 4 files changed, 50 insertions(+), 27 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index fef87167a60b..7fd5bc2a31f6 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -166,6 +166,7 @@ struct clk { unsigned long max_rate; unsigned int exclusive_count; struct hlist_node clks_node; + bool pinning; }; /*** runtime pm ***/ @@ -4237,13 +4238,15 @@ static void free_clk(struct clk *clk) * @hw: clk_hw associated with the clk being consumed * @dev_id: string describing device name * @con_id: connection ID string on device + * @owner: reference to the module creating the clock * * This is the main function used to create a clk pointer for use by clk * consumers. It connects a consumer to the clk_core and clk_hw structures * used by the framework and clk provider respectively. */ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw, - const char *dev_id, const char *con_id) + const char *dev_id, const char *con_id, + struct module *owner) { struct clk *clk; struct clk_core *core; @@ -4258,7 +4261,13 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw, return clk; clk->dev = dev; - if (!try_module_get(core->owner)) { + /* + * Pin the provider module only when the consumer lives in a different + * module. A provider getting a clk from its own clk_hw would otherwise + * pin itself and could never be unloaded. + */ + clk->pinning = owner != core->owner; + if (clk->pinning && !try_module_get(core->owner)) { free_clk(clk); return ERR_PTR(-ENOENT); } @@ -4269,24 +4278,19 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw, return clk; } -/** - * clk_hw_get_clk - get clk consumer given an clk_hw - * @hw: clk_hw associated with the clk being consumed - * @con_id: connection ID string on device - * - * Returns: new clk consumer - * This is the function to be used by providers which need - * to get a consumer clk and act on the clock element - * Calls to this function must be balanced with calls clk_put() +/* + * Internal helper backing the clk_hw_get_clk() macro, which passes the caller's + * module via THIS_MODULE. */ -struct clk *clk_hw_get_clk(struct clk_hw *hw, const char *con_id) +struct clk *__clk_hw_get_clk(struct clk_hw *hw, const char *con_id, + struct module *owner) { struct device *dev = hw->core->dev; const char *name = dev ? dev_name(dev) : NULL; - return clk_hw_create_clk(dev, hw, name, con_id); + return clk_hw_create_clk(dev, hw, name, con_id, owner); } -EXPORT_SYMBOL(clk_hw_get_clk); +EXPORT_SYMBOL(__clk_hw_get_clk); static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist) { @@ -4450,8 +4454,9 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw) INIT_HLIST_HEAD(&core->clks); /* - * Don't call clk_hw_create_clk() here because that would pin the - * provider module to itself and prevent it from ever being removed. + * Don't call clk_hw_create_clk() here because it would systematically + * add a consumer to the hw clock and all clocks would appear to have + * consumer in the clock summary. */ hw->clk = alloc_clk(core, NULL, NULL); if (IS_ERR(hw->clk)) { @@ -4808,7 +4813,7 @@ struct clk *devm_clk_hw_get_clk(struct device *dev, struct clk_hw *hw, if (!clkp) return ERR_PTR(-ENOMEM); - clk = clk_hw_get_clk(hw, con_id); + clk = __clk_hw_get_clk(hw, con_id, dev->driver->owner); if (!IS_ERR(clk)) { *clkp = clk; devres_add(dev, clkp); @@ -4855,7 +4860,8 @@ void __clk_put(struct clk *clk) owner = clk->core->owner; kref_put(&clk->core->ref, __clk_release); - module_put(owner); + if (clk->pinning) + module_put(owner); free_clk(clk); } @@ -5376,7 +5382,7 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec) { struct clk_hw *hw = of_clk_get_hw_from_clkspec(clkspec); - return clk_hw_create_clk(NULL, hw, NULL, __func__); + return clk_hw_create_clk(NULL, hw, NULL, __func__, NULL); } EXPORT_SYMBOL_GPL(of_clk_get_from_provider); @@ -5403,7 +5409,7 @@ static struct clk *__of_clk_get(struct device_node *np, { struct clk_hw *hw = of_clk_get_hw(np, index, con_id); - return clk_hw_create_clk(NULL, hw, dev_id, con_id); + return clk_hw_create_clk(NULL, hw, dev_id, con_id, NULL); } struct clk *of_clk_get(struct device_node *np, int index) diff --git a/drivers/clk/clk.h b/drivers/clk/clk.h index 2d801900cad5..5009380863a9 100644 --- a/drivers/clk/clk.h +++ b/drivers/clk/clk.h @@ -23,13 +23,14 @@ struct clk_hw *clk_find_hw(const char *dev_id, const char *con_id); #ifdef CONFIG_COMMON_CLK struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw, - const char *dev_id, const char *con_id); + const char *dev_id, const char *con_id, + struct module *owner); void __clk_put(struct clk *clk); #else /* All these casts to avoid ifdefs in clkdev... */ static inline struct clk * clk_hw_create_clk(struct device *dev, struct clk_hw *hw, const char *dev_id, - const char *con_id) + const char *con_id, struct module *owner) { return (struct clk *)hw; } diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index abaa0f9e0083..919fc680ae4d 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -88,7 +88,7 @@ static struct clk *__clk_get_sys(struct device *dev, const char *dev_id, { struct clk_hw *hw = clk_find_hw(dev_id, con_id); - return clk_hw_create_clk(dev, hw, dev_id, con_id); + return clk_hw_create_clk(dev, hw, dev_id, con_id, NULL); } struct clk *clk_get_sys(const char *dev_id, const char *con_id) @@ -105,7 +105,7 @@ struct clk *clk_get(struct device *dev, const char *con_id) if (dev && dev->of_node) { hw = of_clk_get_hw(dev->of_node, 0, con_id); if (!IS_ERR(hw) || PTR_ERR(hw) == -EPROBE_DEFER) - return clk_hw_create_clk(dev, hw, dev_id, con_id); + return clk_hw_create_clk(dev, hw, dev_id, con_id, NULL); } return __clk_get_sys(dev, dev_id, con_id); diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index dc8e161f9c2c..58902e8dfd68 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -50,6 +50,7 @@ struct clk; struct clk_hw; struct clk_core; struct dentry; +struct module; /** * struct clk_rate_request - Structure encoding the clk constraints that @@ -351,8 +352,9 @@ struct clk_init_data { * @core: pointer to the struct clk_core instance that points back to this * struct clk_hw instance * - * @clk: pointer to the per-user struct clk instance that can be used to call - * into the clk API + * @clk: pointer to the per-user struct clk instance. This will be removed at + * some point in the future, Please consider the field obsolete and do not use + * it. Use clk_hw_get_clk() to get a per-user struct clk from a struct clk_hw. * * @init: pointer to struct clk_init_data that contains the init data shared * with the common clock framework. This pointer will be set to NULL once @@ -1474,10 +1476,24 @@ static inline struct clk_hw *__clk_get_hw(struct clk *clk) } #endif -struct clk *clk_hw_get_clk(struct clk_hw *hw, const char *con_id); +struct clk *__clk_hw_get_clk(struct clk_hw *hw, const char *con_id, + struct module *owner); struct clk *devm_clk_hw_get_clk(struct device *dev, struct clk_hw *hw, const char *con_id); +/** + * clk_hw_get_clk - get clk consumer given a clk_hw + * @hw: clk_hw associated with the clk being consumed + * @con_id: connection ID string on device + * + * Return: new clk consumer + * This is the function to be used by providers which need + * to get a consumer clk and act on the clock element + * Calls to this function must be balanced with calls to clk_put() + */ +#define clk_hw_get_clk(hw, con_id) \ + __clk_hw_get_clk((hw), (con_id), THIS_MODULE) + unsigned int clk_hw_get_num_parents(const struct clk_hw *hw); struct clk_hw *clk_hw_get_parent(const struct clk_hw *hw); struct clk_hw *clk_hw_get_parent_by_index(const struct clk_hw *hw,