[PATCH 1/6] clk: Handle error pointers in clk_valid()

[Michal Suchánek]

On Sat, Mar 04, 2023 at 01:54:12PM -0600, Samuel Holland wrote:
> On 2/20/23 13:42, Michal Suchánek wrote:
> > On Mon, Feb 20, 2023 at 10:57:17AM -0500, Sean Anderson wrote:
> >>
> >> On 2/20/23 05:46, Michal Suchánek wrote:
> >>> On Sun, Feb 19, 2023 at 11:59:34PM -0600, Samuel Holland wrote:
> >>>> Some clk uclass functions, such as devm_clk_get() and clk_get_parent(),
> >>>> return error pointers. clk_valid() should not consider these pointers
> >>>> to be valid.
> >>>>
> >>>> Fixes: 8a1661f20e6c ("drivers: clk: Handle gracefully NULL pointers")
> >>>> Signed-off-by: Samuel Holland <samuel at sholland.org>
> >>>> ---
> >>>>
> >>>>   include/clk.h | 2 +-
> >>>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/include/clk.h b/include/clk.h
> >>>> index d91285235f7..4acb878ec6d 100644
> >>>> --- a/include/clk.h
> >>>> +++ b/include/clk.h
> >>>> @@ -671,7 +671,7 @@ static inline bool clk_dev_binded(struct clk *clk)
> >>>>    */
> >>>>   static inline bool clk_valid(struct clk *clk)
> >>>>   {
> >>>> -	return clk && !!clk->dev;
> >>>> +	return clk && !IS_ERR(clk) && !!clk->dev;
> >>>>   }
> >>>>   int soc_clk_dump(void);
> >>>
> >>> Maybe it would be better to avoid the error pointers instead, there
> >>> should not be that many places where they are returned.
> >>
> >> This not quite the right way to phrase this. Instead, I would ask: where
> >> are places where the return value is not checked correctly? It's perfectly
> > 
> > Pretty much everywhere where clk_get_parent is used outside of core code
> > absolutely no error checking is done.
> 
> There are 105 uses of ERR_PTR in drivers/clk, mostly in CCF clock
> registration functions. There is also devm_clk_get() with about 30
> callers. Those mostly seem to have reasonable error checking; the error
> pointer ends up as the driver probe function's return value via PTR_ERR.

Yes, there are too many. clk_valid should check both.

Reviewed-by: Michal Suchánek <msuchanek at suse.de>

Thanks

Michal