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