Re: [PATCH] i2c: sprd: Delete i2c adapter in .remove's error path

[Andi Shyti <andi.shyti@kernel.org>]

Hi Uwe,

On Thu, Mar 09, 2023 at 01:04:18PM +0100, Uwe Kleine-König wrote:
> On Thu, Mar 09, 2023 at 01:17:22PM +0200, Andy Shevchenko wrote:
> > On Thu, Mar 9, 2023 at 11:58 AM Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> > >
> > > If pm runtime resume fails the .remove callback used to exit early. This
> > > resulted in an error message by the driver core but the device gets
> > > removed anyhow. This lets the registered i2c adapter stay around with an
> > > unbound parent device.
> > >
> > > So only skip clk disabling if resume failed, but do delete the adapter.
> > 
> > Still worrisome. I would disable clock independently, but the questions are:
> 
> Note that pm-runtime stuff disables the clk, so if resume failed, you
> have to assume the clk already being off.

this is because resume fails only of the clock fails to be
enabled and in that case we shouldn't worry about it...

> > 1) why the heck we need this dance with PM runtime for disabling clocks;
> > 2) why can't we use devm_clk_get_enabled() or so in the probe;
> 
> These questions are orthogonal to my patch, right?

... but (orthoginally to this patch) I guess Andy is asking what
happens to the clock when the remove is called through another
path. Maybe a next patch can use the devm_clk...() wrapper.

As for this one, I'm good with it:

Reviewed-by: Andi Shyti <andi.shyti@kernel.org>

Andi

> Runtime PM might delay suspend, so if you submit two transfers shortly
> after another, this might be more effective as the device isn't
> suspended in between. (Attention: half-baked knowledge)
> 
> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |