| Message ID | 20240929101635.1648234-1-guoniu.zhou@oss.nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: nxp: imx8mq-mipi-csi2: Fix CSI clocks always enabled issue | expand |
Hi Guoniu, Thank you for the patch. On Sun, Sep 29, 2024 at 06:16:35PM +0800, guoniu.zhou@oss.nxp.com wrote: > From: "Guoniu.zhou" <guoniu.zhou@nxp.com> > > Fix CSI clocks always enabled issue after run system suspend/resume. > > The clocks for CSI will be enabled in imx8mq_mipi_csi_pm_resume() > when system resume and the state of CSI will be set to ST_POWERED. > It not only cause clock always enabled issue but also system hang > issue on iMX8ULP platform since imx8mq_mipi_csi_pm_suspend() will > access CSI registers if run system suspend/resume again, but the > CSI power domain is not active. > > In order to fix this issue, using pm_runtime_force_suspend/resume > instead of calling imx8mq_mipi_csi_pm_suspend/resume directly. I think you can simplify suspend/resume handling in the driver by dropping the system suspend/resume handlers. At system suspend time the device should already be stopped, because the top-level driver (the CSI bridge in the i.MX8MQ) should have stopped the pipeline. Similarly, at system resume time, there should be no need to restart streaming, as the top-level driver will control that. I'll send a patch and CC you, could you give it a try ? > Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com> > --- > drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c > index d4a6c5532969..15029bb81b35 100644 > --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c > +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c > @@ -697,8 +697,11 @@ static int imx8mq_mipi_csi_suspend(struct device *dev) > { > struct v4l2_subdev *sd = dev_get_drvdata(dev); > struct csi_state *state = mipi_sd_to_csi2_state(sd); > + int ret; > > - imx8mq_mipi_csi_pm_suspend(dev); > + ret = pm_runtime_force_suspend(dev); > + if (ret < 0) > + return ret; > > state->state |= ST_SUSPENDED; > > @@ -713,7 +716,7 @@ static int imx8mq_mipi_csi_resume(struct device *dev) > if (!(state->state & ST_SUSPENDED)) > return 0; > > - return imx8mq_mipi_csi_pm_resume(dev); > + return pm_runtime_force_resume(dev); > } > > static int imx8mq_mipi_csi_runtime_suspend(struct device *dev)
diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c index d4a6c5532969..15029bb81b35 100644 --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c @@ -697,8 +697,11 @@ static int imx8mq_mipi_csi_suspend(struct device *dev) { struct v4l2_subdev *sd = dev_get_drvdata(dev); struct csi_state *state = mipi_sd_to_csi2_state(sd); + int ret; - imx8mq_mipi_csi_pm_suspend(dev); + ret = pm_runtime_force_suspend(dev); + if (ret < 0) + return ret; state->state |= ST_SUSPENDED; @@ -713,7 +716,7 @@ static int imx8mq_mipi_csi_resume(struct device *dev) if (!(state->state & ST_SUSPENDED)) return 0; - return imx8mq_mipi_csi_pm_resume(dev); + return pm_runtime_force_resume(dev); } static int imx8mq_mipi_csi_runtime_suspend(struct device *dev)