Skip to contentSkip to conversation

pinctrl: rzn1: remove unnecessary unsigned integer comparison

PATCH In review Gustavo A. R. Silva 3 participants 5 messages Oct 11, 2018 – Mar 24, 2021

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pinctrl: rzn1: remove unnecessary unsigned integer comparison

There is no need to compare ipctl->mdio_func[mdio] with => 0, because such comparison is always true. Notice that *mdio_func* is of type u32 (32 bits, unsigned).

Fix this by removing such comparison.

Addresses-Coverity-ID: 1474166 ("Unsigned compared against 0")

---
 drivers/pinctrl/pinctrl-rzn1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pinctrl/pinctrl-rzn1.c +1 -1
index ce05e3a..b5650d8 100644
@@ -195,7 +195,7 @@ static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value)
 static void rzn1_pinctrl_mdio_select(struct rzn1_pinctrl *ipctl, int mdio,
 				     u32 func)
 {
-	if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != func)
+	if (ipctl->mdio_func[mdio] != func)
 		dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n", mdio);
 	ipctl->mdio_func[mdio] = func;
 
Signature · 1 line
2.7.4
#1 lore raw

Sorry for top-posting, Geert is maintainer for this driver.

Yours,
Linus Walleij

Quoted 29 lines · Gustavo A. R. Silva

There is no need to compare ipctl->mdio_func[mdio] with => 0, because such comparison is always true. Notice that *mdio_func* is of type u32 (32 bits, unsigned).

Fix this by removing such comparison.

Addresses-Coverity-ID: 1474166 ("Unsigned compared against 0")

---
 drivers/pinctrl/pinctrl-rzn1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pinctrl/pinctrl-rzn1.c +1 -2
index ce05e3a..b5650d8 100644
@@ -195,7 +195,7 @@ static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value)
 static void rzn1_pinctrl_mdio_select(struct rzn1_pinctrl *ipctl, int mdio,
                                     u32 func)
 {
-       if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != func)
+       if (ipctl->mdio_func[mdio] != func)
                dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n", mdio);
        ipctl->mdio_func[mdio] = func;
 
--

2.7.4

#2 lore raw
Quoted 2 lines · Linus Walleij

Sorry for top-posting, Geert is maintainer for this driver.

Oh okay.

This is what I get when I run the get_maintainer scripts on the rzn1 driver:

$ scripts/get_maintainer.pl --nokeywords --nogit --nogit-fallback drivers/pinctrl/pinctrl-rzn1.c
Linus Walleij <[email protected]> (maintainer:PIN CONTROL SUBSYSTEM)
[email protected] (open list:PIN CONTROL SUBSYSTEM)
[email protected] (open list)

Thanks
--
Gustavo

Quoted 34 lines

Yours,
Linus Walleij

Quoted 29 lines · Gustavo A. R. Silva

There is no need to compare ipctl->mdio_func[mdio] with => 0, because such comparison is always true. Notice that *mdio_func* is of type u32 (32 bits, unsigned).

Fix this by removing such comparison.

Addresses-Coverity-ID: 1474166 ("Unsigned compared against 0")

---
 drivers/pinctrl/pinctrl-rzn1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pinctrl/pinctrl-rzn1.c +1 -2
index ce05e3a..b5650d8 100644
@@ -195,7 +195,7 @@ static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value)
 static void rzn1_pinctrl_mdio_select(struct rzn1_pinctrl *ipctl, int mdio,
                                     u32 func)
 {
-       if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != func)
+       if (ipctl->mdio_func[mdio] != func)
                dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n", mdio);
        ipctl->mdio_func[mdio] = func;
 
--

2.7.4

#3 lore raw
Quoted 4 lines · Gustavo A. R. Silva
Quoted 2 lines · Linus Walleij

Sorry for top-posting, Geert is maintainer for this driver.

Quoted 8 lines

Oh okay.

This is what I get when I run the get_maintainer scripts on the rzn1 driver:

$ scripts/get_maintainer.pl --nokeywords --nogit --nogit-fallback drivers/pinctrl/pinctrl-rzn1.c
Linus Walleij <[email protected]> (maintainer:PIN CONTROL SUBSYSTEM)
[email protected] (open list:PIN CONTROL SUBSYSTEM)
[email protected] (open list)

Hm I bet it needs a MAINTAINERS update.
Geert can you assume maintainership?

Yours,
Linus Walleij

#4 lore raw
In reply to Gustavo A. R. Silva

Hi Gustavo,

Quoted 8 lines · Gustavo A. R. Silva

There is no need to compare ipctl->mdio_func[mdio] with => 0, because such comparison is always true. Notice that *mdio_func* is of type u32 (32 bits, unsigned).

Fix this by removing such comparison.

Addresses-Coverity-ID: 1474166 ("Unsigned compared against 0")

Thanks for your patch, which I somehow missed to review. Sorry for that.

drivers/pinctrl/pinctrl-rzn1.c static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value) · line 198

Quoted 8 lines
drivers/pinctrl/pinctrl-rzn1.c +1 -1
@@ -195,7 +195,7 @@ static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value)
 static void rzn1_pinctrl_mdio_select(struct rzn1_pinctrl *ipctl, int mdio,
                                     u32 func)
 {
-       if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != func)
+       if (ipctl->mdio_func[mdio] != func)

Note that mdio_func is initialized to -1, hence the check. But the check indeed didn't work this way, so mdio_func was changed later to int in commit 8deaaa46d2de91e3 ("pinctrl: rzn1: Fix check for used MDIO bus").

drivers/pinctrl/pinctrl-rzn1.c static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value) · line 200

Quoted 2 lines
drivers/pinctrl/pinctrl-rzn1.c +0 -0
                dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n", mdio);
        ipctl->mdio_func[mdio] = func;

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds