[PATCH] clk: Reject positive return values from init callbacks

yong.liu posted 1 patch 1 day, 9 hours ago
drivers/clk/clk.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] clk: Reject positive return values from init callbacks
Posted by yong.liu 1 day, 9 hours ago
Clock init callbacks must return zero on success or a negative error code
on failure. If a callback returns a positive value, __clk_core_init()
already takes its failure path and __clk_register() releases the clock
before returning ERR_PTR(ret).

However, a positive value encoded by ERR_PTR() is not recognized by
IS_ERR(). Consequently, clk_register() returns an invalid pointer that
passes the caller's error check, while clk_hw_register() and
of_clk_hw_register() report success through PTR_ERR_OR_ZERO() despite
registration having failed.

Convert positive init return values to -EINVAL before taking the failure
path. This makes the registration failure detectable through the public
registration interfaces while preserving valid negative error codes.

Signed-off-by: yong.liu <binary_world@163.com>
---
 drivers/clk/clk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -4053,6 +4053,8 @@ static int __clk_core_init(struct clk_core *core)
 	 */
 	if (core->ops->init) {
 		ret = core->ops->init(core->hw);
+		if (ret > 0)
+			ret = -EINVAL;
 		if (ret)
 			goto out;
 	}
Re: [PATCH] clk: Reject positive return values from init callbacks
Posted by Jerome Brunet 1 day, 4 hours ago
On mer. 23 sept. 2026 at 03:58, yong.liu <binary_world@163.com> wrote:

> Clock init callbacks must return zero on success or a negative error code
> on failure. If a callback returns a positive value, __clk_core_init()
> already takes its failure path and __clk_register() releases the clock
> before returning ERR_PTR(ret).

If a provider returns a positive value it is bug in the provider and it should
be fixed. The contract is clear:
- negative value on errors
- 0 on success

Positive values just means nothing.

I don't think the framework should try to defend against every silly
things a provider can try.

>
> However, a positive value encoded by ERR_PTR() is not recognized by
> IS_ERR(). Consequently, clk_register() returns an invalid pointer that
> passes the caller's error check, while clk_hw_register() and
> of_clk_hw_register() report success through PTR_ERR_OR_ZERO() despite
> registration having failed.
>
> Convert positive init return values to -EINVAL before taking the failure
> path. This makes the registration failure detectable through the public
> registration interfaces while preserving valid negative error codes.
>
> Signed-off-by: yong.liu <binary_world@163.com>
> ---
>  drivers/clk/clk.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -4053,6 +4053,8 @@ static int __clk_core_init(struct clk_core *core)
>  	 */
>  	if (core->ops->init) {
>  		ret = core->ops->init(core->hw);
> +		if (ret > 0)
> +			ret = -EINVAL;

That would at the very least be a WARN_ON(), but again, I don't think
this is the kind of checks we should add.

>  		if (ret)
>  			goto out;
>  	}
>

-- 
Jerome