| Message ID | 20200601192124.172650-1-lkundrak@v3.sk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND,2] media: marvell-ccic: Add support for runtime PM | expand |
Cc += Sakari. I'm wondering if you'd mind looking at this mmp ccic patch too. Thank you Lubo On Mon, Jun 01, 2020 at 09:21:24PM +0200, Lubomir Rintel wrote: > On MMP3, the camera block lives on na separate power island. We want to > turn it off if the CCIC is not in use to conserve power. > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> > --- > drivers/media/platform/marvell-ccic/mcam-core.c | 3 +++ > drivers/media/platform/marvell-ccic/mmp-driver.c | 12 ++++++++---- > 2 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c > index 09775b6624c6b..c2cd1d461bd06 100644 > --- a/drivers/media/platform/marvell-ccic/mcam-core.c > +++ b/drivers/media/platform/marvell-ccic/mcam-core.c > @@ -24,6 +24,7 @@ > #include <linux/clk.h> > #include <linux/clk-provider.h> > #include <linux/videodev2.h> > +#include <linux/pm_runtime.h> > #include <media/v4l2-device.h> > #include <media/v4l2-ioctl.h> > #include <media/v4l2-ctrls.h> > @@ -901,6 +902,7 @@ static void mcam_clk_enable(struct mcam_camera *mcam) > { > unsigned int i; > > + pm_runtime_get_sync(mcam->dev); > for (i = 0; i < NR_MCAM_CLK; i++) { > if (!IS_ERR(mcam->clk[i])) > clk_prepare_enable(mcam->clk[i]); > @@ -915,6 +917,7 @@ static void mcam_clk_disable(struct mcam_camera *mcam) > if (!IS_ERR(mcam->clk[i])) > clk_disable_unprepare(mcam->clk[i]); > } > + pm_runtime_put(mcam->dev); > } > > /* ---------------------------------------------------------------------- */ > diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c > index 92b92255dac66..eec482d16805b 100644 > --- a/drivers/media/platform/marvell-ccic/mmp-driver.c > +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c > @@ -24,6 +24,7 @@ > #include <linux/list.h> > #include <linux/pm.h> > #include <linux/clk.h> > +#include <linux/pm_runtime.h> > > #include "mcam-core.h" > > @@ -313,10 +314,12 @@ static int mmpcam_probe(struct platform_device *pdev) > cam->irq = res->start; > ret = devm_request_irq(&pdev->dev, cam->irq, mmpcam_irq, IRQF_SHARED, > "mmp-camera", mcam); > - if (ret == 0) { > - mmpcam_add_device(cam); > - return 0; > - } > + if (ret) > + goto out; > + > + mmpcam_add_device(cam); > + pm_runtime_enable(&pdev->dev); > + return 0; > > out: > fwnode_handle_put(mcam->asd.match.fwnode); > @@ -332,6 +335,7 @@ static int mmpcam_remove(struct mmp_camera *cam) > > mmpcam_remove_device(cam); > mccic_shutdown(mcam); > + pm_runtime_force_suspend(mcam->dev); > return 0; > } > > -- > 2.26.2 >
Hi Lubomir, Thanks for the patch. On Mon, Jun 01, 2020 at 09:21:24PM +0200, Lubomir Rintel wrote: > On MMP3, the camera block lives on na separate power island. We want to s/on \Kn// > turn it off if the CCIC is not in use to conserve power. > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> > --- > drivers/media/platform/marvell-ccic/mcam-core.c | 3 +++ > drivers/media/platform/marvell-ccic/mmp-driver.c | 12 ++++++++---- > 2 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c > index 09775b6624c6b..c2cd1d461bd06 100644 > --- a/drivers/media/platform/marvell-ccic/mcam-core.c > +++ b/drivers/media/platform/marvell-ccic/mcam-core.c > @@ -24,6 +24,7 @@ > #include <linux/clk.h> > #include <linux/clk-provider.h> > #include <linux/videodev2.h> > +#include <linux/pm_runtime.h> > #include <media/v4l2-device.h> > #include <media/v4l2-ioctl.h> > #include <media/v4l2-ctrls.h> > @@ -901,6 +902,7 @@ static void mcam_clk_enable(struct mcam_camera *mcam) The clocks look like something that rather should be controlled by the device's runtime PM state. Could it be put behind suspend/resume callbacks? > { > unsigned int i; > > + pm_runtime_get_sync(mcam->dev); > for (i = 0; i < NR_MCAM_CLK; i++) { > if (!IS_ERR(mcam->clk[i])) > clk_prepare_enable(mcam->clk[i]); > @@ -915,6 +917,7 @@ static void mcam_clk_disable(struct mcam_camera *mcam) > if (!IS_ERR(mcam->clk[i])) > clk_disable_unprepare(mcam->clk[i]); > } > + pm_runtime_put(mcam->dev); > } > > /* ---------------------------------------------------------------------- */ > diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c > index 92b92255dac66..eec482d16805b 100644 > --- a/drivers/media/platform/marvell-ccic/mmp-driver.c > +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c > @@ -24,6 +24,7 @@ > #include <linux/list.h> > #include <linux/pm.h> > #include <linux/clk.h> > +#include <linux/pm_runtime.h> > > #include "mcam-core.h" > > @@ -313,10 +314,12 @@ static int mmpcam_probe(struct platform_device *pdev) > cam->irq = res->start; > ret = devm_request_irq(&pdev->dev, cam->irq, mmpcam_irq, IRQF_SHARED, > "mmp-camera", mcam); > - if (ret == 0) { > - mmpcam_add_device(cam); > - return 0; > - } > + if (ret) > + goto out; > + > + mmpcam_add_device(cam); > + pm_runtime_enable(&pdev->dev); If you enable runtime PM here, there's a chance the clocks have been prepared and enabled before this. How about moving registerin clocks after this instead? Looking at the current code, they are registered too early. > + return 0; > > out: > fwnode_handle_put(mcam->asd.match.fwnode); > @@ -332,6 +335,7 @@ static int mmpcam_remove(struct mmp_camera *cam) > > mmpcam_remove_device(cam); > mccic_shutdown(mcam); > + pm_runtime_force_suspend(mcam->dev); > return 0; > } >
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 09775b6624c6b..c2cd1d461bd06 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -24,6 +24,7 @@ #include <linux/clk.h> #include <linux/clk-provider.h> #include <linux/videodev2.h> +#include <linux/pm_runtime.h> #include <media/v4l2-device.h> #include <media/v4l2-ioctl.h> #include <media/v4l2-ctrls.h> @@ -901,6 +902,7 @@ static void mcam_clk_enable(struct mcam_camera *mcam) { unsigned int i; + pm_runtime_get_sync(mcam->dev); for (i = 0; i < NR_MCAM_CLK; i++) { if (!IS_ERR(mcam->clk[i])) clk_prepare_enable(mcam->clk[i]); @@ -915,6 +917,7 @@ static void mcam_clk_disable(struct mcam_camera *mcam) if (!IS_ERR(mcam->clk[i])) clk_disable_unprepare(mcam->clk[i]); } + pm_runtime_put(mcam->dev); } /* ---------------------------------------------------------------------- */ diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 92b92255dac66..eec482d16805b 100644 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -24,6 +24,7 @@ #include <linux/list.h> #include <linux/pm.h> #include <linux/clk.h> +#include <linux/pm_runtime.h> #include "mcam-core.h" @@ -313,10 +314,12 @@ static int mmpcam_probe(struct platform_device *pdev) cam->irq = res->start; ret = devm_request_irq(&pdev->dev, cam->irq, mmpcam_irq, IRQF_SHARED, "mmp-camera", mcam); - if (ret == 0) { - mmpcam_add_device(cam); - return 0; - } + if (ret) + goto out; + + mmpcam_add_device(cam); + pm_runtime_enable(&pdev->dev); + return 0; out: fwnode_handle_put(mcam->asd.match.fwnode); @@ -332,6 +335,7 @@ static int mmpcam_remove(struct mmp_camera *cam) mmpcam_remove_device(cam); mccic_shutdown(mcam); + pm_runtime_force_suspend(mcam->dev); return 0; }
On MMP3, the camera block lives on na separate power island. We want to turn it off if the CCIC is not in use to conserve power. Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- drivers/media/platform/marvell-ccic/mcam-core.c | 3 +++ drivers/media/platform/marvell-ccic/mmp-driver.c | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-)