RE: [PATCH 3/3] i2c: rzv2m: Disable the operation of unit in case of error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Thursday, May 25, 2023 5:15 PM
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Cc: Wolfram Sang <wsa@xxxxxxxxxx>; Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx>; Philipp Zabel
> <p.zabel@xxxxxxxxxxxxxx>; linux-i2c@xxxxxxxxxxxxxxx; Geert Uytterhoeven
> <geert+renesas@xxxxxxxxx>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@xxxxxxxxxxxxxx>; linux-renesas-soc@xxxxxxxxxxxxxxx; Pavel Machek
> <pavel@xxxxxxx>
> Subject: Re: [PATCH 3/3] i2c: rzv2m: Disable the operation of unit in
> case of error
> 
> Hi Biju,
> 
> Thanks for your patch!
> 
> On Thu, May 25, 2023 at 3:51 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > The remove and suspend callbacks disable the operation of the unit.
> > Do the same in probe() in case of error.
> 
> Makes perfect sense.
> 
> > Reported-by: Pavel Machek <pavel@xxxxxxx>
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> > --- a/drivers/i2c/busses/i2c-rzv2m.c
> > +++ b/drivers/i2c/busses/i2c-rzv2m.c
> > @@ -454,8 +454,10 @@ static int rzv2m_i2c_probe(struct platform_device
> *pdev)
> >         platform_set_drvdata(pdev, priv);
> >
> >         ret = i2c_add_numbered_adapter(adap);
> > -       if (ret < 0)
> > +       if (ret < 0) {
> > +               BIT_CLRL(priv->base + IICB0CTL0, IICB0IICE);
> 
> This needs to be wrapped inside pm_runtime_resume_and_get()/
> pm_runtime_put(), like is done in the .suspend() callback.

OK will use a helper function to disable I2C and share the code
between probe error path, suspend and remove.

+static int rzv2m_i2c_disable(struct device *dev, struct rzv2m_i2c_priv *priv)
+{
+       int ret;
+
+       ret = pm_runtime_resume_and_get(dev);
+       if (ret < 0)
+               return ret;
+
+       BIT_CLRL(priv->base + IICB0CTL0, IICB0IICE);
+       pm_runtime_put(dev);
+
+       return 0;
+}

Cheers,
Biju

> 
> Note that this is also lacking from the .remove() callback.
> 
> >                 pm_runtime_disable(dev);
> > +       }
> >
> >         return ret;
> >  }
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> 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




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux