[PATCH] clk: cdce9xx: Remove always true test

[PATCH] clk: cdce9xx: Remove always true test

[Andrew Goodbody]

The function dev_read_u32_default does not return an error and the
variable 'val' is unsigned so testing for >= 0 will always be true so
remove this test.

This issue was found by Smatch.

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---
 drivers/clk/clk-cdce9xx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/clk-cdce9xx.c b/drivers/clk/clk-cdce9xx.c
index e5f74e714d5..492a196604a 100644
--- a/drivers/clk/clk-cdce9xx.c
+++ b/drivers/clk/clk-cdce9xx.c
@@ -117,8 +117,7 @@ static int cdce9xx_clk_probe(struct udevice *dev)
 	data->xtal_rate = clk_get_rate(&clk);
 
 	val = dev_read_u32_default(dev, "xtal-load-pf", -1);
-	if (val >= 0)
-		cdce9xx_reg_write(dev, CDCE9XX_REG_XCSEL, val << 3);
+	cdce9xx_reg_write(dev, CDCE9XX_REG_XCSEL, val << 3);
 
 	return 0;
 }

---
base-commit: bd0ade7d090a334b3986936d63a34001d99722ad
change-id: 20250723-cdce9xx_clk-9e1008c78db7

Best regards,
-- 
Andrew Goodbody <andrew.goodbody@linaro.org>

Re: [PATCH] clk: cdce9xx: Remove always true test

[Quentin Schulz]

Hi Andrew,

On 7/23/25 5:17 PM, Andrew Goodbody wrote:
> The function dev_read_u32_default does not return an error and the
> variable 'val' is unsigned so testing for >= 0 will always be true so
> remove this test.
> 
> This issue was found by Smatch.
> 
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> ---
>   drivers/clk/clk-cdce9xx.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-cdce9xx.c b/drivers/clk/clk-cdce9xx.c
> index e5f74e714d5..492a196604a 100644
> --- a/drivers/clk/clk-cdce9xx.c
> +++ b/drivers/clk/clk-cdce9xx.c
> @@ -117,8 +117,7 @@ static int cdce9xx_clk_probe(struct udevice *dev)
>   	data->xtal_rate = clk_get_rate(&clk);
>   
>   	val = dev_read_u32_default(dev, "xtal-load-pf", -1);
> -	if (val >= 0)
> -		cdce9xx_reg_write(dev, CDCE9XX_REG_XCSEL, val << 3);
> +	cdce9xx_reg_write(dev, CDCE9XX_REG_XCSEL, val << 3);

I think the original code was broken?

I assume the intended behavior was to return -1 if the property isn't 
present. Obviously, dev_read_u32_default having a -1 as default won't 
return a -1...

So I think we should migrate to dev_read_u32 instead and check the 
return code and do the write based on that?

Something like:

if (!dev_read_u32(dev, "xtal-load-pf", &val))
     cdce9xx_reg_write(dev, CDCE9XX_REG_XCSEL, val << 3);

maybe? (Not tested nor compiled)

Cheers,
Quentin

Re: [PATCH] clk: cdce9xx: Remove always true test

[Andrew Goodbody]

On 23/07/2025 18:03, Quentin Schulz wrote:
> Hi Andrew,
> 
> On 7/23/25 5:17 PM, Andrew Goodbody wrote:
>> The function dev_read_u32_default does not return an error and the
>> variable 'val' is unsigned so testing for >= 0 will always be true so
>> remove this test.
>>
>> This issue was found by Smatch.
>>
>> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
>> ---
>>   drivers/clk/clk-cdce9xx.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/clk-cdce9xx.c b/drivers/clk/clk-cdce9xx.c
>> index e5f74e714d5..492a196604a 100644
>> --- a/drivers/clk/clk-cdce9xx.c
>> +++ b/drivers/clk/clk-cdce9xx.c
>> @@ -117,8 +117,7 @@ static int cdce9xx_clk_probe(struct udevice *dev)
>>       data->xtal_rate = clk_get_rate(&clk);
>>       val = dev_read_u32_default(dev, "xtal-load-pf", -1);
>> -    if (val >= 0)
>> -        cdce9xx_reg_write(dev, CDCE9XX_REG_XCSEL, val << 3);
>> +    cdce9xx_reg_write(dev, CDCE9XX_REG_XCSEL, val << 3);
> 
> I think the original code was broken?
> 
> I assume the intended behavior was to return -1 if the property isn't 
> present. Obviously, dev_read_u32_default having a -1 as default won't 
> return a -1...
> 
> So I think we should migrate to dev_read_u32 instead and check the 
> return code and do the write based on that?
> 
> Something like:
> 
> if (!dev_read_u32(dev, "xtal-load-pf", &val))
>      cdce9xx_reg_write(dev, CDCE9XX_REG_XCSEL, val << 3);
> 
> maybe? (Not tested nor compiled)
> 
> Cheers,
> Quentin

Fine by me. V2 coming.

Andrew