On Mon, 2016-09-12 at 10:52 +0930, Joel Stanley wrote: > On Fri, Sep 9, 2016 at 6:56 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > > > > The newly added aspeed driver tries to check for a negative return > > value from a pinctrl function, but stores the intermediate value in > > a 'bool' variable, which cannot work: > > > > drivers/pinctrl/aspeed/pinctrl-aspeed.c: In function 'aspeed_sig_expr_set': > > drivers/pinctrl/aspeed/pinctrl-aspeed.c:192:11: error: comparison of constant '0' with boolean expression is always false [-Werror=bool-compare] > > > > This slightly reworks the logic to use an explicit comparison with zero > > before assigning to the temporary variable. > Thanks for finding this Arnd. > > Colin also found the issue. Thanks Colin! > > I think we should take this version of the fix. Linus, can you put > this in your tree please? +1 on each point above. I made a change to the loop body in response to feedback in one of the earlier iterations and unfortunately this looks to have slipped past me. I will look at adding a bit more static analysis to my patch submission checklist! > > > > > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx> > Acked-by: Joel Stanley <joel@xxxxxxxxx> Reviewed-by: Andrew Jeffery <andrew@xxxxxxxx> > > Cheers, > > Joel > > > > > --- > > drivers/pinctrl/aspeed/pinctrl-aspeed.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c > > index 7d461fc30d3c..0391f9f13f3e 100644 > > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c > > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c > > @@ -187,10 +187,10 @@ static bool aspeed_sig_expr_set(const struct aspeed_sig_expr *expr, > > continue; > > > > ret = regmap_update_bits(map, desc->reg, desc->mask, > > - pattern << __ffs(desc->mask)); > > + pattern << __ffs(desc->mask)) == 0; > > > > - if (ret < 0) > > - return false; > > + if (!ret) > > + return ret; > > } > > > > return aspeed_sig_expr_eval(expr, enable, map); > > -- > > 2.9.0 > >
Attachment:
signature.asc
Description: This is a digitally signed message part
