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

[Jerome Brunet <jbrunet@baylibre.com>]

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