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.
