[PATCH] clk: cdce9xx: Remove always true test
Andrew Goodbody
andrew.goodbody at linaro.org
Fri Jul 25 12:31:06 CEST 2025
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 at 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
More information about the U-Boot
mailing list