On 22/04/21 10:28PM, Dario Binacchi wrote: > As reported by Coverity Scan for Das U-Boot, the 'less-than-zero' > comparison of an unsigned value is never true. > > Signed-off-by: Dario Binacchi <dario...@libero.it> > Reviewed-by: Pratyush Yadav <p.ya...@ti.com> > > --- > > Changes in v2: > - Balance quote in commit message > - Add review tag > > drivers/pinctrl/pinctrl-single.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-single.c > b/drivers/pinctrl/pinctrl-single.c > index 48bdd0f6f5..7e1c5bb0a5 100644 > --- a/drivers/pinctrl/pinctrl-single.c > +++ b/drivers/pinctrl/pinctrl-single.c > @@ -295,7 +295,7 @@ static int single_configure_pins(struct udevice *dev, > func->npins = 0; > for (n = 0; n < count; n++, pins++) { > offset = fdt32_to_cpu(pins->reg); > - if (offset < 0 || offset > pdata->offset) { > + if (offset > pdata->offset) {
I went back and took a look at it again. The "offset < 0" comes from "reg < 0" in 9b884e79a6 (pinctrl: single: fix offset management, 2021-04-11), where reg is of type phys_addr_t. But phys_addr_t is also an unsigned value so I suppose the check was wrong to begin with. And fdt32_to_cpu() just does byte order conversions so it shouldn't return a negative value because there is no scope for failure. So again, the patch looks good to me, but some added context for anyone interested. > dev_err(dev, " invalid register offset 0x%x\n", > offset); > continue; > @@ -344,7 +344,7 @@ static int single_configure_bits(struct udevice *dev, > func->npins = 0; > for (n = 0; n < count; n++, pins++) { > offset = fdt32_to_cpu(pins->reg); > - if (offset < 0 || offset > pdata->offset) { > + if (offset > pdata->offset) { > dev_dbg(dev, " invalid register offset 0x%x\n", > offset); > continue; > -- > 2.17.1 > -- Regards, Pratyush Yadav Texas Instruments Inc.