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

Samuel Holland samuel at sholland.org
Sat Mar 4 20:58:17 CET 2023


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.

Regards,
Samuel



More information about the U-Boot mailing list