| Message ID | 20230525135108.240651-4-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [1/3] i2c: rzv2m: Drop extra space | expand |
Hi Biju, Thanks for your patch! On Thu, May 25, 2023 at 3:51 PM Biju Das <biju.das.jz@bp.renesas.com> 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@denx.de> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- 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. Note that this is also lacking from the .remove() callback. > pm_runtime_disable(dev); > + } > > return ret; > } Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: Thursday, May 25, 2023 5:15 PM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: Wolfram Sang <wsa@kernel.org>; Andy Shevchenko > <andriy.shevchenko@linux.intel.com>; Philipp Zabel > <p.zabel@pengutronix.de>; linux-i2c@vger.kernel.org; Geert Uytterhoeven > <geert+renesas@glider.be>; Prabhakar Mahadev Lad <prabhakar.mahadev- > lad.rj@bp.renesas.com>; linux-renesas-soc@vger.kernel.org; Pavel Machek > <pavel@denx.de> > 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@bp.renesas.com> > 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@denx.de> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- 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
diff --git a/drivers/i2c/busses/i2c-rzv2m.c b/drivers/i2c/busses/i2c-rzv2m.c index ccd337f2e8c3..3e9fc269e4fd 100644 --- 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); pm_runtime_disable(dev); + } return ret; }
The remove and suspend callbacks disable the operation of the unit. Do the same in probe() in case of error. Reported-by: Pavel Machek <pavel@denx.de> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/i2c/busses/i2c-rzv2m.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)