Re: [PATCH 1/1] clk: Document of_clk_get_by_name() return values

[Jerome Brunet]

On mar. 15 sept. 2026 at 19:38, Dan Carpenter <error27@xxxxxxxxx> wrote:

> Callers should test the return from of_clk_get_by_name() with IS_ERR().
> The function returns a valid clock on success and an error pointer on
> failure; NULL is not a valid return value.
>
> Document this explicitly to prevent callers from treating NULL as a
> separate failure case.
>
> Assisted-by: ChatGPT:gpt-5
> Signed-off-by: Dan Carpenter <error27@xxxxxxxxx>
> ---
> There are a few other functions which look like they return NULL but
> never actually do.  This is one which has caused some confusion in
> the past.
>
>  drivers/clk/clk.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index f1756fe59372..a71bcf780de4 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -5430,6 +5430,9 @@ EXPORT_SYMBOL(of_clk_get);
>   * This function parses the clocks and clock-names properties,
>   * and uses them to look up the struct clk from the registered list of clock
>   * providers.
> + *
> + * Returns: A clock pointer on success or an error pointer on failure.  This
> + * function never returns NULL.

Thanks Dan. I was about to apply the change but it feels a bit strange
to document what a function never does.

Is this a documentation that should be added everywhere the return value
is to be tested with IS_ERR() ?

What about being more direct then:

"Returns: A clock pointer on success or an error pointer on failure.
 Caller should test the return value with IS_ERR()"

?

>   */
>  struct clk *of_clk_get_by_name(struct device_node *np, const char *name)
>  {
> -- 
> 2.53.0
>

-- 
Jerome