Re: pm_runtime_resume_and_get in .remove callbacks

[Uwe Kleine-König]

Hello Rafael,

On Wed, Jul 20, 2022 at 12:19:09PM +0200, Rafael J. Wysocki wrote:
> On Wed, Jul 20, 2022 at 8:06 AM Uwe Kleine-König
> <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
> > On Wed, Jul 13, 2022 at 07:47:39PM +0200, Rafael J. Wysocki wrote:
> > > (1) Use pm_runtime_get_sync() instead of pm_runtime_resume_and_get()
> > > and don't check its return value,
> > >
> > > or if that is not viable, because something really can run if and only
> > > if the device is operational,
> > >
> > > (2) do something like
> > >
> > > ret = pm_runtime_resume_and_get(i2c_dev->dev);
> > > i2c_del_adapter(&i2c_dev->adap);
> > > if (ret >= 0)
> > >         clk_disable_unprepare(i2c_dev->clk);
> > >
> > > pm_runtime_put_noidle(i2c_dev->dev);
> > > pm_runtime_disable(i2c_dev->dev);
> >
> > Why would you not disable the clk if the resume failed?
> 
> I thought that it might lead to problems if the device that failed to
> resume was expected to be accessible.
> 
> If that's not the case, you can simply do (1).
> 
> > Is it an option to not call one of the resume variants at all and only
> > call pm_runtime_disable()?
> 
> That depends on whether or not you need to manipulate the hardware in
> the del/disable part.  If you need to access it there, it is better to
> resume I think.  Otherwise, you don't have to do anything, but then
> the next probe needs to be prepared for finding the device in the
> suspended state.

OK, thanks for your time. I think I understood it good enough to tackle
some of the problems I identified.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature