From aed6480fadede2b87103568aaa117a423a1c3fdc Mon Sep 17 00:00:00 2001 From: Nathan Barrett-Morrison Date: Mon, 15 May 2023 15:49:58 -0400 Subject: [PATCH 1/4] drivers: clk: Adjust temp var data type to properly match that of struct clk_ops In commit 5c5992cb90cf ("clk: Add debugging for return values"), a temporary storage variable was added around the ops->get_rate() call inside clk_get_rate(), so that the result could be passed through log_ret. This temporary variable was declared as an int, yet when we look in struct clk_ops, we can see this needs to be a ulong: ulong (*get_rate)(struct clk *clk); This was resulting in a signed to unsigned casting error on our builds, where a clock value of 0xABCDABCD was being incorrectly cast to 0xFFFFFFFFABCDABCD. Signed-off-by: Nathan Barrett-Morrison Reviewed-by: Sean Anderson Link: https://lore.kernel.org/r/20230515195005.1961495-1-nathan.morrison@timesys.com --- drivers/clk/clk-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index f186fcbcdb8..364ee92e7f1 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -477,7 +477,7 @@ void clk_free(struct clk *clk) ulong clk_get_rate(struct clk *clk) { const struct clk_ops *ops; - int ret; + ulong ret; debug("%s(clk=%p)\n", __func__, clk); if (!clk_valid(clk)) From b6a56f553318b4c0c8fb8b1ea05f2e15b2662ccb Mon Sep 17 00:00:00 2001 From: Eugen Hristev Date: Mon, 19 Jun 2023 13:47:52 +0300 Subject: [PATCH 2/4] clk: fix count parameter type for clk_release_all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second parameter for clk_release_all is used as an unsigned (which makes sense) but the function prototype declares it as an int. This causes warnings/error like such below: include/clk.h:422:48: error: conversion to ‘int’ from ‘unsigned int’ may change the sign of the result [-Werror=sign-conversion] 422 | return clk_release_all(bulk->clks, bulk->count); To fix this, changed the type of the count to `unsigned int` Fixes: 82a8a669b4f7 ("clk: add clk_release_all()") Signed-off-by: Eugen Hristev Reviewed-by: Xavier Drudis Ferran Reviewed-by: Sean Anderson Link: https://lore.kernel.org/r/20230619104752.278500-1-eugen.hristev@collabora.com --- drivers/clk/clk-uclass.c | 7 ++++--- include/clk.h | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 364ee92e7f1..dcbc3ee1a1e 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -422,12 +422,13 @@ int clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk) return clk_get_by_index_nodev(node, index, clk); } -int clk_release_all(struct clk *clk, int count) +int clk_release_all(struct clk *clk, unsigned int count) { - int i, ret; + unsigned int i; + int ret; for (i = 0; i < count; i++) { - debug("%s(clk[%d]=%p)\n", __func__, i, &clk[i]); + debug("%s(clk[%u]=%p)\n", __func__, i, &clk[i]); /* check if clock has been previously requested */ if (!clk[i].dev) diff --git a/include/clk.h b/include/clk.h index d91285235f7..a342297007b 100644 --- a/include/clk.h +++ b/include/clk.h @@ -243,7 +243,7 @@ static inline struct clk *devm_clk_get_optional(struct udevice *dev, * * Return: zero on success, or -ve error code. */ -int clk_release_all(struct clk *clk, int count); +int clk_release_all(struct clk *clk, unsigned int count); /** * devm_clk_put - "free" a managed clock source @@ -307,7 +307,7 @@ clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk) return -ENOSYS; } -static inline int clk_release_all(struct clk *clk, int count) +static inline int clk_release_all(struct clk *clk, unsigned int count) { return -ENOSYS; } From 0755db477fa7478e2659568bb7da038a9eaabb8d Mon Sep 17 00:00:00 2001 From: Maksim Kiselev Date: Wed, 6 Sep 2023 01:16:49 +0300 Subject: [PATCH 3/4] clk: use private clk struct in CLK_CCF's enable/disable functions In clk_enable()/clk_disable() functions, when CCF is activated, we must pass a private clk struct to enable()/disable() ops functions. Otherwise, the use of a container_of() construction within these ops should be banned. Because passing a non-private clk struct to container_of() results in an out of range error. At the moment, clk-mux, clk-fixed-factor, clk-gate and possibly other clocks use container_of() in their enable()/disable() functions. Therefore, for these functions to work correclty, private clk struct must be passed. Signed-off-by: Maksim Kiselev Reviewed-by: Sean Anderson Link: https://lore.kernel.org/r/20230905221649.3577929-1-bigunclemax@gmail.com --- drivers/clk/clk-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index dcbc3ee1a1e..3b5e3f9c86b 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -656,7 +656,7 @@ int clk_enable(struct clk *clk) } if (ops->enable) { - ret = ops->enable(clk); + ret = ops->enable(clkp ? clkp : clk); if (ret) { printf("Enable %s failed\n", clk->dev->name); return ret; @@ -713,7 +713,7 @@ int clk_disable(struct clk *clk) } if (ops->disable) { - ret = ops->disable(clk); + ret = ops->disable(clkp ? clkp : clk); if (ret) return ret; } From c4b52fda6924e92c6745351f32c4cafc36034170 Mon Sep 17 00:00:00 2001 From: Yang Xiwen Date: Fri, 18 Aug 2023 01:04:02 +0800 Subject: [PATCH 4/4] clk: also handle ENOENT in *_optional functions If the device does not specify any clocks in device tree, these functions will return PTR_ERR(-ENOENT). This is not the intended behavior and does not comply with linux kernel CCF. Fix that by returning NULL under such circumstances instead. Signed-off-by: Yang Xiwen Reviewed-by: Sean Anderson Link: https://lore.kernel.org/r/20230818-clk-fix-v1-3-49ec18f820bf@outlook.com --- include/clk.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/clk.h b/include/clk.h index a342297007b..249c0e0ab42 100644 --- a/include/clk.h +++ b/include/clk.h @@ -223,9 +223,11 @@ struct clk *devm_clk_get(struct udevice *dev, const char *id); static inline struct clk *devm_clk_get_optional(struct udevice *dev, const char *id) { + int ret; struct clk *clk = devm_clk_get(dev, id); - if (PTR_ERR(clk) == -ENODATA) + ret = PTR_ERR(clk); + if (ret == -ENODATA || ret == -ENOENT) return NULL; return clk; @@ -335,7 +337,7 @@ static inline int clk_get_by_name_optional(struct udevice *dev, int ret; ret = clk_get_by_name(dev, name, clk); - if (ret == -ENODATA) + if (ret == -ENODATA || ret == -ENOENT) return 0; return ret; @@ -359,7 +361,7 @@ static inline int clk_get_by_name_nodev_optional(ofnode node, const char *name, int ret; ret = clk_get_by_name_nodev(node, name, clk); - if (ret == -ENODATA) + if (ret == -ENODATA || ret == -ENOENT) return 0; return ret;