Hi Laurent, > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Sent: Sunday, September 29, 2024 8:01 PM > To: G.N. Zhou (OSS) <guoniu.zhou@xxxxxxxxxxx> > Cc: rmfrfs@xxxxxxxxx; martink@xxxxxxxxx; kernel@xxxxxxx; > mchehab@xxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; > kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; imx@xxxxxxxxxxxxxxx; linux- > media@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] media: nxp: imx8mq-mipi-csi2: Fix CSI clocks always enabled > issue > > Hi Guoniu, > > Thank you for the patch. > > On Sun, Sep 29, 2024 at 06:16:35PM +0800, guoniu.zhou@xxxxxxxxxxx wrote: > > From: "Guoniu.zhou" <guoniu.zhou@xxxxxxx> > > > > 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 ? Sure. > > > Signed-off-by: Guoniu.zhou <guoniu.zhou@xxxxxxx> > > --- > > 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) > > -- > Regards, > > Laurent Pinchart