Quoting Krzysztof Kozlowski (2024-02-15 23:12:29)
On 16/02/2024 00:12, Stephen Boyd wrote:
Quoting Krzysztof Kozlowski (2024-02-08 08:37:10)
None of the implementations of the get() and get_hw() callbacks of "struct of_clk_provider" modify the contents of received of_phandle_args pointer. They treat it as read-only variable used to find the clock to return. Make obvious that implementations are not supposed to modify the of_phandle_args, by making it a pointer to const.
Signed-off-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org
This will almost certainly break the build once it is merged to linux-next. What's your plan to merge this?
First problem is that it might not apply... I prepared it on next to be sure all subsystems are updated.
The idea is to get reviews and acks and then:
- Maybe it applies cleanly to your tree meaning there will be no
conflicts with other trees, 2. If not, then I can keep rebasing it and it should be applied after rc1.
The struct clk based version is probably not going to be used in any new code. If you split the patch up and converted the struct clk based ones first then that would probably apply without breaking anything, because new code should only be using the struct clk_hw version.
The struct clk_hw version could be done in two steps. Introduce another get_hw callback with the const signature, and then update the world to use that callback, finally remove the old callback. We could call this callback 'get_clk_hw'. This is probably more work than it's worth though, but at least this way we don't have to worry about applying after rc1.
Or perhaps we need to cast everything and use macros? It would be bad if the callback actually did something with the clkspec and we cast it to const, but your patch shows that nobody is doing that. We would get rid of this macro garbage once everything is converted.
---8<--- diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 2253c154a824..8e5ed16a97a0 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -4818,7 +4818,7 @@ struct of_clk_provider { struct list_head link;
struct device_node *node; - struct clk *(*get)(struct of_phandle_args *clkspec, void *data); + struct clk *(*get)(const struct of_phandle_args *clkspec, void *data); struct clk_hw *(*get_hw)(struct of_phandle_args *clkspec, void *data); void *data; }; @@ -4880,8 +4880,8 @@ EXPORT_SYMBOL_GPL(of_clk_hw_onecell_get); * * This function is *deprecated*. Use of_clk_add_hw_provider() instead. */ -int of_clk_add_provider(struct device_node *np, - struct clk *(*clk_src_get)(struct of_phandle_args *clkspec, +int _of_clk_add_provider(struct device_node *np, + struct clk *(*clk_src_get)(const struct of_phandle_args *clkspec, void *data), void *data) { @@ -4914,7 +4914,7 @@ int of_clk_add_provider(struct device_node *np,
return ret; } -EXPORT_SYMBOL_GPL(of_clk_add_provider); +EXPORT_SYMBOL_GPL(_of_clk_add_provider);
/** * of_clk_add_hw_provider() - Register a clock provider for a node diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 1293c38ddb7f..bfc660fa7c8f 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -1531,10 +1531,11 @@ struct clk_hw_onecell_data { }
#ifdef CONFIG_OF -int of_clk_add_provider(struct device_node *np, - struct clk *(*clk_src_get)(struct of_phandle_args *args, +int _of_clk_add_provider(struct device_node *np, + struct clk *(*clk_src_get)(const struct of_phandle_args *args, void *data), void *data); + int of_clk_add_hw_provider(struct device_node *np, struct clk_hw *(*get)(struct of_phandle_args *clkspec, void *data), @@ -1559,8 +1560,8 @@ int of_clk_detect_critical(struct device_node *np, int index,
#else /* !CONFIG_OF */
-static inline int of_clk_add_provider(struct device_node *np, - struct clk *(*clk_src_get)(struct of_phandle_args *args, +static inline int _of_clk_add_provider(struct device_node *np, + struct clk *(*clk_src_get)(const struct of_phandle_args *args, void *data), void *data) { @@ -1614,6 +1615,12 @@ static inline int of_clk_detect_critical(struct device_node *np, int index, } #endif /* CONFIG_OF */
+typedef struct clk *(*clk_src_get_fn)(const struct of_phandle_args *args, void *data); + +#define of_clk_add_provider(np, get, data) ({ \ + _of_clk_add_provider(np, (clk_src_get_fn)(get), data); \ +}) + void clk_gate_restore_context(struct clk_hw *hw);
#endif /* CLK_PROVIDER_H */