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
