| Message ID | 20220314103941.46021-4-jacopo@jmondi.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx: imx-mipi-csis: Additional cleanups | expand |
Hi Jacopo, Thank you for the patch. On Mon, Mar 14, 2022 at 11:39:39AM +0100, Jacopo Mondi wrote: > The s_stream() operation implementation was locked to avoid races with > the mipi_csis_log_status() which access the platform registers and needs > the interface to be powered up. > > As powering is now handled by runtime_pm which is refcounted, it is > not necessary to manually lock s_stream() anymore. The lock is also used to protect against races between .s_stream() and .set_fmt() both accessing the active format. I don't think we can drop it. > As the error path is now simplified, we can inline it. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > drivers/media/platform/imx/imx-mipi-csis.c | 23 ++++++---------------- > 1 file changed, 6 insertions(+), 17 deletions(-) > > diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c > index b76bb129a416..4a6152c13d52 100644 > --- a/drivers/media/platform/imx/imx-mipi-csis.c > +++ b/drivers/media/platform/imx/imx-mipi-csis.c > @@ -912,16 +912,12 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable) > int ret; > > if (!enable) { > - mutex_lock(&csis->lock); > - > v4l2_subdev_call(csis->src_sd, video, s_stream, 0); > > mipi_csis_stop_stream(csis); > if (csis->debug.enable) > mipi_csis_log_counters(csis, true); > > - mutex_unlock(&csis->lock); > - > pm_runtime_put(csis->dev); > > return 0; > @@ -937,25 +933,18 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable) > if (ret < 0) > return ret; > > - mutex_lock(&csis->lock); > - > mipi_csis_start_stream(csis); > ret = v4l2_subdev_call(csis->src_sd, video, s_stream, 1); > - if (ret < 0) > - goto out; > + if (ret < 0) { > + mipi_csis_stop_stream(csis); > + pm_runtime_put(csis->dev); > > - mipi_csis_log_counters(csis, true); > + return ret; > + } > > - mutex_unlock(&csis->lock); > + mipi_csis_log_counters(csis, true); > > return 0; > - > -out: > - mipi_csis_stop_stream(csis); > - mutex_unlock(&csis->lock); > - pm_runtime_put(csis->dev); > - > - return ret; > } > > static struct v4l2_mbus_framefmt *
diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c index b76bb129a416..4a6152c13d52 100644 --- a/drivers/media/platform/imx/imx-mipi-csis.c +++ b/drivers/media/platform/imx/imx-mipi-csis.c @@ -912,16 +912,12 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable) int ret; if (!enable) { - mutex_lock(&csis->lock); - v4l2_subdev_call(csis->src_sd, video, s_stream, 0); mipi_csis_stop_stream(csis); if (csis->debug.enable) mipi_csis_log_counters(csis, true); - mutex_unlock(&csis->lock); - pm_runtime_put(csis->dev); return 0; @@ -937,25 +933,18 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable) if (ret < 0) return ret; - mutex_lock(&csis->lock); - mipi_csis_start_stream(csis); ret = v4l2_subdev_call(csis->src_sd, video, s_stream, 1); - if (ret < 0) - goto out; + if (ret < 0) { + mipi_csis_stop_stream(csis); + pm_runtime_put(csis->dev); - mipi_csis_log_counters(csis, true); + return ret; + } - mutex_unlock(&csis->lock); + mipi_csis_log_counters(csis, true); return 0; - -out: - mipi_csis_stop_stream(csis); - mutex_unlock(&csis->lock); - pm_runtime_put(csis->dev); - - return ret; } static struct v4l2_mbus_framefmt *
The s_stream() operation implementation was locked to avoid races with the mipi_csis_log_status() which access the platform registers and needs the interface to be powered up. As powering is now handled by runtime_pm which is refcounted, it is not necessary to manually lock s_stream() anymore. As the error path is now simplified, we can inline it. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- drivers/media/platform/imx/imx-mipi-csis.c | 23 ++++++---------------- 1 file changed, 6 insertions(+), 17 deletions(-)