[PATCH 0/2] clk: fix a bug in CCF which results in a clock being enabled twice.
Currently, ccf_clk_endisable() calls clk_enable() / clk_disable() directly depending on the request. However, this function is also referenced in clk->ops, which is also called in clk_enable() / clk_disable().
This caused a recursion and made the clock enabled twice if clk_enable() is invoked. So we have to call clk_disable() twice to disable the clock. It can be easily reproduced with few extra lines of debug code.
Fix that by calling clk->ops->enable/disable directly in ccf_clk_endisable() rather than using the framework functions.
Signed-off-by: Yang Xiwen forbidden405@outlook.com --- Yang Xiwen (2): clk: move clk_get_ops() to a common header clk: ccf: invoke ops provided by clk directly
drivers/clk/clk-uclass.c | 5 ----- drivers/clk/clk.c | 10 ++++++++-- include/clk.h | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) --- base-commit: 580eb31199be8a822e62f20965854a242f895d03 change-id: 20230724-clk_fix-1b7265d663a8
Best regards,
From: Yang Xiwen forbidden405@outlook.com
This allows it to be used by other source files.
Signed-off-by: Yang Xiwen forbidden405@outlook.com --- drivers/clk/clk-uclass.c | 5 ----- include/clk.h | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index dc3e9d6a26..5cc80e5e39 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -25,11 +25,6 @@ #include <linux/clk-provider.h> #include <linux/err.h>
-static inline const struct clk_ops *clk_dev_ops(struct udevice *dev) -{ - return (const struct clk_ops *)dev->driver->ops; -} - struct clk *dev_get_clk_ptr(struct udevice *dev) { return (struct clk *)dev_get_uclass_priv(dev); diff --git a/include/clk.h b/include/clk.h index d91285235f..bd3617e1e0 100644 --- a/include/clk.h +++ b/include/clk.h @@ -8,6 +8,7 @@ #ifndef _CLK_H_ #define _CLK_H_
+#include <dm/device.h> #include <dm/ofnode.h> #include <linux/err.h> #include <linux/errno.h> @@ -258,6 +259,17 @@ int clk_release_all(struct clk *clk, int count); */ void devm_clk_put(struct udevice *dev, struct clk *clk);
+/** + * clk_dev_ops - get ops of a clock + * @dev: clock device + * + * Return: ops of the clk + */ +static inline const struct clk_ops *clk_dev_ops(struct udevice *dev) +{ + return (const struct clk_ops *)dev->driver->ops; +} + #else
static inline int clk_get_by_phandle(struct udevice *dev, const @@ -315,6 +327,11 @@ static inline int clk_release_all(struct clk *clk, int count) static inline void devm_clk_put(struct udevice *dev, struct clk *clk) { } + +static inline const struct clk_ops *clk_dev_ops(struct udevice *dev) +{ + return ERR_PTR(-ENOSYS); +} #endif
/**
From: Yang Xiwen forbidden405@outlook.com
Invoking clk_enable() or clk_disable() in clk->ops context causes a recursion, results in a clock being enabled twice.
Signed-off-by: Yang Xiwen forbidden405@outlook.com --- drivers/clk/clk.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index a5a3461b66..bd455e44a4 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -10,7 +10,6 @@ #include <clk.h> #include <clk-uclass.h> #include <log.h> -#include <dm/device.h> #include <dm/uclass.h> #include <dm/lists.h> #include <dm/device-internal.h> @@ -115,11 +114,18 @@ int ccf_clk_set_parent(struct clk *clk, struct clk *parent) static int ccf_clk_endisable(struct clk *clk, bool enable) { struct clk *c; + const struct clk_ops *ops = clk_dev_ops(clk->dev); int err = clk_get_by_id(clk->id, &c);
if (err) return err; - return enable ? clk_enable(c) : clk_disable(c); + + if (enable && ops->enable) + return ops->enable(c); + else if (!enable && ops->disable) + return ops->disable(c); + + return 0; }
int ccf_clk_enable(struct clk *clk)
participants (1)
-
Yang Xiwen via B4 Relay