[PATCH 1/4] clk: k210: Fix checking if ulongs are less than 0

[Sean Anderson]

On 7/27/21 4:15 AM, Damien Le Moal wrote:
> On 2021/07/27 12:51, Sean Anderson wrote:
>> Some clock functions return ulong but still have "negative" errors. To deal
>> with this, cast the relevant arguments to long.
>>
>> Fixes: 609bd60b94 ("clk: k210: Rewrite to remove CCF")
>> Reported-by: Coverity Scan <scan-admin at coverity.com>
>> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
>> ---
>>
>>   drivers/clk/clk_kendryte.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/clk_kendryte.c b/drivers/clk/clk_kendryte.c
>> index 3148756968..37bd624eca 100644
>> --- a/drivers/clk/clk_kendryte.c
>> +++ b/drivers/clk/clk_kendryte.c
>> @@ -439,7 +439,7 @@ static const struct k210_clk_params k210_clks[] = {
>>   #ifdef CONFIG_CLK_K210_SET_RATE
>>   static int k210_pll_enable(struct k210_clk_priv *priv, int id);
>>   static int k210_pll_disable(struct k210_clk_priv *priv, int id);
>> -static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, ulong rate_in);
>> +static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, long rate_in);
>>   
>>   /*
>>    * The PLL included with the Kendryte K210 appears to be a True Circuits, Inc.
>> @@ -841,7 +841,7 @@ TEST_STATIC int k210_pll_calc_config(u32 rate, u32 rate_in,
>>   }
>>   
>>   static ulong k210_pll_set_rate(struct k210_clk_priv *priv, int id, ulong rate,
> 
> Shouldn't this one return a long, in case of error ? It seems that the commit
> messages hints at such a change, but you are changing the argument type instead.
> A little confusing. What am I missing ?

Perhaps they should return long, but these are basically matching the
prototypes in include/clk-uclass.h. And there, get_rate and set_rate
take ulong arguments and return ulongs.

> 
>> -			       ulong rate_in)
>> +			       long rate_in)
>>   {
>>   	int err;
>>   	const struct k210_pll_params *pll = &k210_plls[id];
>> @@ -890,7 +890,7 @@ static ulong k210_pll_set_rate(struct k210_clk_priv *priv, int id, ulong rate,
>>   #endif /* CONFIG_CLK_K210_SET_RATE */
>>   
>>   static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id,
> 
> Same here ?
> 
>> -			       ulong rate_in)
>> +			       long rate_in)
> 
> I would assume that these functions are called if the rate_in argument is
> correct, so I do not really understand why the argument type needs to be changed...

Hm, I suppose the better patch would be to check the return of
get_rate and set_rate when we call them. I think my intent here was to
allow subsequent functions to be no-ops in case of error, but it looks
like I act on these values directly. Will fix in v2.

--Sean

> 
>>   {
>>   	u64 r, f, od;
>>   	u32 reg = readl(priv->base + k210_plls[id].off);
>>
> 
>