Message ID | 20181011204042.GA17125@embeddedor.com |
---|---|
State | New |
Headers | show |
Series | pinctrl: rzn1: remove unnecessary unsigned integer comparison | expand |
Sorry for top-posting, Geert is maintainer for this driver. Yours, Linus Walleij On Thu, Oct 11, 2018 at 10:40 PM Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > > 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") > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > --- > drivers/pinctrl/pinctrl-rzn1.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pinctrl/pinctrl-rzn1.c b/drivers/pinctrl/pinctrl-rzn1.c > index ce05e3a..b5650d8 100644 > --- a/drivers/pinctrl/pinctrl-rzn1.c > +++ b/drivers/pinctrl/pinctrl-rzn1.c > @@ -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 >
On 10/12/18 11:17 AM, Linus Walleij wrote: > 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 <linus.walleij@linaro.org> (maintainer:PIN CONTROL SUBSYSTEM) linux-gpio@vger.kernel.org (open list:PIN CONTROL SUBSYSTEM) linux-kernel@vger.kernel.org (open list) Thanks -- Gustavo > Yours, > Linus Walleij > > On Thu, Oct 11, 2018 at 10:40 PM Gustavo A. R. Silva > <gustavo@embeddedor.com> wrote: >> >> 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") >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >> --- >> drivers/pinctrl/pinctrl-rzn1.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/pinctrl/pinctrl-rzn1.c b/drivers/pinctrl/pinctrl-rzn1.c >> index ce05e3a..b5650d8 100644 >> --- a/drivers/pinctrl/pinctrl-rzn1.c >> +++ b/drivers/pinctrl/pinctrl-rzn1.c >> @@ -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 >>
On Fri, Oct 12, 2018 at 11:28 AM Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > On 10/12/18 11:17 AM, Linus Walleij wrote: > > 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 <linus.walleij@linaro.org> (maintainer:PIN CONTROL SUBSYSTEM) > linux-gpio@vger.kernel.org (open list:PIN CONTROL SUBSYSTEM) > linux-kernel@vger.kernel.org (open list) Hm I bet it needs a MAINTAINERS update. Geert can you assume maintainership? Yours, Linus Walleij
Hi Gustavo, On Thu, Oct 11, 2018 at 10:42 PM Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > 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") > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> Thanks for your patch, which I somehow missed to review. Sorry for that. > --- a/drivers/pinctrl/pinctrl-rzn1.c > +++ b/drivers/pinctrl/pinctrl-rzn1.c > @@ -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"). > dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n", mdio); > ipctl->mdio_func[mdio] = func; Gr{oetje,eeting}s, Geert
diff --git a/drivers/pinctrl/pinctrl-rzn1.c b/drivers/pinctrl/pinctrl-rzn1.c index ce05e3a..b5650d8 100644 --- a/drivers/pinctrl/pinctrl-rzn1.c +++ b/drivers/pinctrl/pinctrl-rzn1.c @@ -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;
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") Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/pinctrl/pinctrl-rzn1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)