Hi Todor, On Wed, Jul 25, 2018 at 01:01:31PM +0300, Todor Tomov wrote: > Hi Sakari, > > Thank you for review. > > On 24.07.2018 15:49, Sakari Ailus wrote: > > Hi Todor, > > > > On Mon, Jul 23, 2018 at 02:02:35PM +0300, Todor Tomov wrote: > >> There is a PM domain for each of the VFE hardware modules. Add > >> support for basic runtime PM support to be able to control the > >> PM domains. When a PM domain needs to be powered on - a device > >> link is created. When a PM domain needs to be powered off - > >> its device link is removed. This allows separate and > >> independent control of the PM domains. > >> > >> Suspend/Resume is still not supported. > >> > >> Signed-off-by: Todor Tomov <todor.tomov@xxxxxxxxxx> > >> --- > >> drivers/media/platform/qcom/camss/camss-csid.c | 4 ++ > >> drivers/media/platform/qcom/camss/camss-csiphy.c | 5 ++ > >> drivers/media/platform/qcom/camss/camss-ispif.c | 19 ++++++- > >> drivers/media/platform/qcom/camss/camss-vfe.c | 13 +++++ > >> drivers/media/platform/qcom/camss/camss.c | 63 ++++++++++++++++++++++++ > >> drivers/media/platform/qcom/camss/camss.h | 11 +++++ > >> 6 files changed, 113 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c > >> index 627ef44..ea2b0ba 100644 > >> --- a/drivers/media/platform/qcom/camss/camss-csid.c > >> +++ b/drivers/media/platform/qcom/camss/camss-csid.c > >> @@ -13,6 +13,7 @@ > >> #include <linux/kernel.h> > >> #include <linux/of.h> > >> #include <linux/platform_device.h> > >> +#include <linux/pm_runtime.h> > >> #include <linux/regulator/consumer.h> > >> #include <media/media-entity.h> > >> #include <media/v4l2-device.h> > >> @@ -316,6 +317,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) > >> if (on) { > >> u32 hw_version; > >> > >> + pm_runtime_get_sync(dev); > >> + > >> ret = regulator_enable(csid->vdda); > > > > Shouldn't the regulator be enabled in the runtime_resume callback instead? > > Ideally - yes, but it becomes more complex (different pipelines are possible > and we have only one callback) so (at least for now) I have left it as it is > and stated in the commit message that suspend/resume is still not supported. Ack. -- Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx