Em Tue, 19 Jan 2021 09:56:31 +0300
Dan Carpenter <[email protected]> escreveu:
> On Thu, Jan 14, 2021 at 09:56:41AM -0800, Joe Perches wrote:
> > On Thu, 2021-01-14 at 18:35 +0100, Mauro Carvalho Chehab wrote:
> > > Instead of using 1/0 for true/false, change the type to boolean
> > > and change the returned value.
> > []
> > > diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.c
> > > b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
> > []
> > > @@ -326,24 +326,24 @@ static int hi3670_phy_set_params(struct hi3670_priv
> > > *priv)
> > > return ret;
> > > }
> > >
> > > -static int hi3670_is_abbclk_seleted(struct hi3670_priv *priv)
> > > +static bool hi3670_is_abbclk_seleted(struct hi3670_priv *priv)
> >
> > Presumably this should be "selected" not "seleted"
True. I'll fix it.
> >
> > > {
> > > u32 reg;
> > >
> > > if (!priv->sctrl) {
> > > dev_err(priv->dev, "priv->sctrl is null!\n");
> > > - return 1;
> > > + return true;
> > > }
> > >
> > > if (regmap_read(priv->sctrl, SCTRL_SCDEEPSLEEPED, ®)) {
> > > dev_err(priv->dev, "SCTRL_SCDEEPSLEEPED read failed!\n");
> > > - return 1;
> > > + return true;
> > > }
> > >
> > > if ((reg & USB_CLK_SELECTED) == 0)
> > > - return 1;
> > > + return true;
> > >
> > > - return 0;
> > > + return false;
> > > }
> >
> > if (foo)
> > return true;
> > return false;
> >
> > should generally be consolidated into a single test.
>
> I quite prefer the original format, but you're right about the return
> looking reversed. Using "return !!(reg & USB_CLK_SELECTED);" is
> especially problematic. I like !! but Linus has commented a couple
> times that he doesn't like !!.
Me to. "return !!(reg & USB_CLK_SELECTED)" just obfuscates the
source code, imho for no good reason. It sounds a lot easier to
read something like:
if (cond1)
return true;
if (cond2)
return true;
if (cond3)
return true;
return false;
Yet, I agree that the logic here seems to be inverted.
>
> regards,
> dan carpenter
>
>
Thanks,
Mauro
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
