On Mon, May 25, 2020 at 01:45:07PM +0200, Tomasz Figa wrote:
> On Fri, May 22, 2020 at 11:27 AM Dongchun Zhu <dongchun.zhu@xxxxxxxxxxxx> wrote:
> >
> > Hi Tomasz,
> >
> > Thanks for the review. My replies are as below.
> >
> > On Thu, 2020-05-21 at 19:51 +0000, Tomasz Figa wrote:
> > > Hi Dongchun, Sakari,
> > >
> > > On Mon, May 18, 2020 at 09:27:31PM +0800, Dongchun Zhu wrote:
> [snip]
> > > > + pm_runtime_enable(dev);
> > > > + if (!pm_runtime_enabled(dev)) {
> > > > + ret = dw9768_runtime_resume(dev);
> > > > + if (ret < 0) {
> > > > + dev_err(dev, "failed to power on: %d\n", ret);
> > > > + goto entity_cleanup;
> > > > + }
> > > > + }
> > > > +
> > > > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > > > + if (ret < 0)
> > > > + goto entity_cleanup;
> > > > +
> > > > + return 0;
> > > > +
> > > > +entity_cleanup:
> > >
> > > Need to power off if the code above powered on.
> > >
> >
> > Thanks for the reminder.
> > If there is something wrong with runtime PM, actuator is to be powered
> > on via dw9768_runtime_resume() API.
> > When actuator sub-device is powered on completely and async registered
> > successfully, we shall power off it afterwards.
> >
>
> The code above calls dw9768_runtime_resume() if
> !pm_runtime_enabled(dev), but the clean-up code below the
> entity_cleanup label doesn't have the corresponding
> dw9768_runtime_suspend() call.
Yes, an example on using runtime PM can be found in e.g. ov8856 driver.
The open / release callbacks seem fine though. Sensors just don't need
them as they have the streaming state (and s_stream).
--
Sakari Ailus
