Use guard() to simplify mutex locking. No functional change.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
leave as it without cleanup goto branch because there are two path to
update stream_count.
And it will be replaced soon at
Use new v4l2_subdev_pad_ops.enable_streams(disalbe_stream) replace
deprecated s_stream interface.
https://lore.kernel.org/imx/20250821-95_cam-v3-18-c9286fbb34b9@nxp.com/
---
drivers/staging/media/imx/imx6-mipi-csi2.c | 54 +++++++++++-------------------
1 file changed, 19 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index e1b4b7fb53131ce9515b9441d8fc420e85d3e993..762f19ffd0858c952027afa8e0f36fc87246e1ea 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -412,21 +412,17 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
struct csi2_dev *csi2 = sd_to_dev(sd);
int i, ret = 0;
- mutex_lock(&csi2->lock);
+ guard(mutex)(&csi2->lock);
- if (!csi2->src_sd) {
- ret = -EPIPE;
- goto out;
- }
+ if (!csi2->src_sd)
+ return -EPIPE;
for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
if (csi2->sink_linked[i])
break;
}
- if (i >= CSI2_NUM_SRC_PADS) {
- ret = -EPIPE;
- goto out;
- }
+ if (i >= CSI2_NUM_SRC_PADS)
+ return -EPIPE;
/*
* enable/disable streaming only if stream_count is
@@ -441,14 +437,12 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
else
csi2_stop(csi2);
if (ret)
- goto out;
+ return ret;
update_count:
csi2->stream_count += enable ? 1 : -1;
if (csi2->stream_count < 0)
csi2->stream_count = 0;
-out:
- mutex_unlock(&csi2->lock);
return ret;
}
@@ -466,32 +460,28 @@ static int csi2_link_setup(struct media_entity *entity,
remote_sd = media_entity_to_v4l2_subdev(remote->entity);
- mutex_lock(&csi2->lock);
+ guard(mutex)(&csi2->lock);
if (local->flags & MEDIA_PAD_FL_SOURCE) {
if (flags & MEDIA_LNK_FL_ENABLED) {
- if (csi2->sink_linked[local->index - 1]) {
- ret = -EBUSY;
- goto out;
- }
+ if (csi2->sink_linked[local->index - 1])
+ return -EBUSY;
+
csi2->sink_linked[local->index - 1] = true;
} else {
csi2->sink_linked[local->index - 1] = false;
}
} else {
if (flags & MEDIA_LNK_FL_ENABLED) {
- if (csi2->src_sd) {
- ret = -EBUSY;
- goto out;
- }
+ if (csi2->src_sd)
+ return -EBUSY;
+
csi2->src_sd = remote_sd;
} else {
csi2->src_sd = NULL;
}
}
-out:
- mutex_unlock(&csi2->lock);
return ret;
}
@@ -512,14 +502,12 @@ static int csi2_get_fmt(struct v4l2_subdev *sd,
struct csi2_dev *csi2 = sd_to_dev(sd);
struct v4l2_mbus_framefmt *fmt;
- mutex_lock(&csi2->lock);
+ guard(mutex)(&csi2->lock);
fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
sdformat->format = *fmt;
- mutex_unlock(&csi2->lock);
-
return 0;
}
@@ -529,17 +517,14 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
{
struct csi2_dev *csi2 = sd_to_dev(sd);
struct v4l2_mbus_framefmt *fmt;
- int ret = 0;
if (sdformat->pad >= CSI2_NUM_PADS)
return -EINVAL;
- mutex_lock(&csi2->lock);
+ guard(mutex)(&csi2->lock);
- if (csi2->stream_count > 0) {
- ret = -EBUSY;
- goto out;
- }
+ if (csi2->stream_count > 0)
+ return -EBUSY;
/* Output pads mirror active input pad, no limits on input pads */
if (sdformat->pad != CSI2_SINK_PAD)
@@ -548,9 +533,8 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
*fmt = sdformat->format;
-out:
- mutex_unlock(&csi2->lock);
- return ret;
+
+ return 0;
}
static int csi2_registered(struct v4l2_subdev *sd)
--
2.34.1
Hi Frank,
Thank you for the patch.
On Fri, Jan 16, 2026 at 11:17:59AM -0500, Frank Li wrote:
> Use guard() to simplify mutex locking. No functional change.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> leave as it without cleanup goto branch because there are two path to
> update stream_count.
>
> And it will be replaced soon at
>
> Use new v4l2_subdev_pad_ops.enable_streams(disalbe_stream) replace
> deprecated s_stream interface.
>
> https://lore.kernel.org/imx/20250821-95_cam-v3-18-c9286fbb34b9@nxp.com/
> ---
> drivers/staging/media/imx/imx6-mipi-csi2.c | 54 +++++++++++-------------------
> 1 file changed, 19 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index e1b4b7fb53131ce9515b9441d8fc420e85d3e993..762f19ffd0858c952027afa8e0f36fc87246e1ea 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -412,21 +412,17 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
> struct csi2_dev *csi2 = sd_to_dev(sd);
> int i, ret = 0;
>
> - mutex_lock(&csi2->lock);
> + guard(mutex)(&csi2->lock);
>
> - if (!csi2->src_sd) {
> - ret = -EPIPE;
> - goto out;
> - }
> + if (!csi2->src_sd)
> + return -EPIPE;
>
> for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
> if (csi2->sink_linked[i])
> break;
> }
> - if (i >= CSI2_NUM_SRC_PADS) {
> - ret = -EPIPE;
> - goto out;
> - }
> + if (i >= CSI2_NUM_SRC_PADS)
> + return -EPIPE;
>
> /*
> * enable/disable streaming only if stream_count is
> @@ -441,14 +437,12 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
> else
> csi2_stop(csi2);
> if (ret)
> - goto out;
> + return ret;
>
> update_count:
> csi2->stream_count += enable ? 1 : -1;
> if (csi2->stream_count < 0)
> csi2->stream_count = 0;
> -out:
> - mutex_unlock(&csi2->lock);
> return ret;
> }
>
> @@ -466,32 +460,28 @@ static int csi2_link_setup(struct media_entity *entity,
>
> remote_sd = media_entity_to_v4l2_subdev(remote->entity);
>
> - mutex_lock(&csi2->lock);
> + guard(mutex)(&csi2->lock);
>
> if (local->flags & MEDIA_PAD_FL_SOURCE) {
> if (flags & MEDIA_LNK_FL_ENABLED) {
> - if (csi2->sink_linked[local->index - 1]) {
> - ret = -EBUSY;
> - goto out;
> - }
> + if (csi2->sink_linked[local->index - 1])
> + return -EBUSY;
> +
> csi2->sink_linked[local->index - 1] = true;
> } else {
> csi2->sink_linked[local->index - 1] = false;
> }
> } else {
> if (flags & MEDIA_LNK_FL_ENABLED) {
> - if (csi2->src_sd) {
> - ret = -EBUSY;
> - goto out;
> - }
> + if (csi2->src_sd)
> + return -EBUSY;
> +
> csi2->src_sd = remote_sd;
> } else {
> csi2->src_sd = NULL;
> }
> }
>
> -out:
> - mutex_unlock(&csi2->lock);
> return ret;
You can
return 0;
here and drop the local ret variable. With that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> }
>
> @@ -512,14 +502,12 @@ static int csi2_get_fmt(struct v4l2_subdev *sd,
> struct csi2_dev *csi2 = sd_to_dev(sd);
> struct v4l2_mbus_framefmt *fmt;
>
> - mutex_lock(&csi2->lock);
> + guard(mutex)(&csi2->lock);
>
> fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
>
> sdformat->format = *fmt;
>
> - mutex_unlock(&csi2->lock);
> -
> return 0;
> }
>
> @@ -529,17 +517,14 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
> {
> struct csi2_dev *csi2 = sd_to_dev(sd);
> struct v4l2_mbus_framefmt *fmt;
> - int ret = 0;
>
> if (sdformat->pad >= CSI2_NUM_PADS)
> return -EINVAL;
>
> - mutex_lock(&csi2->lock);
> + guard(mutex)(&csi2->lock);
>
> - if (csi2->stream_count > 0) {
> - ret = -EBUSY;
> - goto out;
> - }
> + if (csi2->stream_count > 0)
> + return -EBUSY;
>
> /* Output pads mirror active input pad, no limits on input pads */
> if (sdformat->pad != CSI2_SINK_PAD)
> @@ -548,9 +533,8 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
> fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
>
> *fmt = sdformat->format;
> -out:
> - mutex_unlock(&csi2->lock);
> - return ret;
> +
> + return 0;
> }
>
> static int csi2_registered(struct v4l2_subdev *sd)
--
Regards,
Laurent Pinchart
© 2016 - 2026 Red Hat, Inc.