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@xxxxxxxxxxxxxx> 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@xxxxxxxxxx> 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/ |
