[PATCH 3/6] clk: Fix error handling in clk_get_parent()

[Michal Suchánek]

On Sat, Mar 04, 2023 at 01:58:17PM -0600, Samuel Holland wrote:
> On 2/20/23 04:39, Michal Suchánek wrote:
> > On Sun, Feb 19, 2023 at 11:59:36PM -0600, Samuel Holland wrote:
> >> Do not return both NULL and error pointers. The function is only
> >> documented as returning error pointers.
> >>
> >> Fixes: 8a1661f20e6c ("drivers: clk: Handle gracefully NULL pointers")
> >> Signed-off-by: Samuel Holland <samuel at sholland.org>
> >> ---
> >>
> >>  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 78299dbceb2..5bce976b060 100644
> >> --- a/drivers/clk/clk-uclass.c
> >> +++ b/drivers/clk/clk-uclass.c
> >> @@ -490,7 +490,7 @@ struct clk *clk_get_parent(struct clk *clk)
> >>  
> >>  	debug("%s(clk=%p)\n", __func__, clk);
> >>  	if (!clk_valid(clk))
> >> -		return NULL;
> >> +		return ERR_PTR(-ENODEV);
> >>  
> >>  	pdev = dev_get_parent(clk->dev);
> >>  	if (!pdev)
> > 
> > Do we really need this?
> > 
> > Who cares about the distinction?
> > 
> > Just returning NULL on any error makes the code so much simpler and
> > easier to understand.
> 
> I'm fine with returning only NULL, or only ERR_PTR(-ENODEV) as done
> later in the function, but returning both makes the error handling in
> callers more complicated for no benefit. It sounds like you would prefer
> I change the later ERR_PTR(-ENODEV) to NULL instead of this change? I
> can do that for v2.

I think NULL is easier to check, and so long as there is no meaningful
distinction between ERR_PTR and NULL we should converge towards NULL as
the single error return value.

Thanks

Michal