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. Best regards, Tomasz