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 dariobin@libero.it Reviewed-by: Pratyush Yadav p.yadav@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