All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>,
	 Hans Verkuil <hverkuil@xs4all.nl>,
	 Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	 Sakari Ailus <sakari.ailus@linux.intel.com>,
	 Umang Jain <umang.jain@ideasonboard.com>
Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Subject: [PATCH v5 04/10] media: subdev: Fix use of sd->enabled_streams in call_s_stream()
Date: Tue, 16 Apr 2024 16:55:07 +0300	[thread overview]
Message-ID: <20240416-enable-streams-impro-v5-4-bd5fcea49388@ideasonboard.com> (raw)
In-Reply-To: <20240416-enable-streams-impro-v5-0-bd5fcea49388@ideasonboard.com>

call_s_stream() uses sd->enabled_streams to track whether streaming has
already been enabled. However,
v4l2_subdev_enable/disable_streams_fallback(), which was the original
user of this field, already uses it, and
v4l2_subdev_enable/disable_streams_fallback() will call call_s_stream().

This leads to a conflict as both functions set the field. Afaics, both
functions set the field to the same value, so it won't cause a runtime
bug, but it's still wrong and if we, e.g., change how
v4l2_subdev_enable/disable_streams_fallback() operates we might easily
cause bugs.

Fix this by adding a new field, 's_stream_enabled', for
call_s_stream().

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 8 ++------
 include/media/v4l2-subdev.h           | 3 +++
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 606a909cd778..941cf7be22c3 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -421,12 +421,8 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable)
 	 * The .s_stream() operation must never be called to start or stop an
 	 * already started or stopped subdev. Catch offenders but don't return
 	 * an error yet to avoid regressions.
-	 *
-	 * As .s_stream() is mutually exclusive with the .enable_streams() and
-	 * .disable_streams() operation, we can use the enabled_streams field
-	 * to store the subdev streaming state.
 	 */
-	if (WARN_ON(!!sd->enabled_streams == !!enable))
+	if (WARN_ON(sd->s_stream_enabled == !!enable))
 		return 0;
 
 	ret = sd->ops->video->s_stream(sd, enable);
@@ -437,7 +433,7 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable)
 	}
 
 	if (!ret) {
-		sd->enabled_streams = enable ? BIT(0) : 0;
+		sd->s_stream_enabled = enable;
 
 		if (enable)
 			v4l2_subdev_enable_privacy_led(sd);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index a9e6b8146279..b3c3777db464 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1043,6 +1043,8 @@ struct v4l2_subdev_platform_data {
  *		     v4l2_subdev_enable_streams() and
  *		     v4l2_subdev_disable_streams() helper functions for fallback
  *		     cases.
+ * @s_stream_enabled: Tracks whether streaming has been enabled with s_stream.
+ *                    This is only for call_s_stream() internal use.
  *
  * Each instance of a subdev driver should create this struct, either
  * stand-alone or embedded in a larger struct.
@@ -1091,6 +1093,7 @@ struct v4l2_subdev {
 	 */
 	struct v4l2_subdev_state *active_state;
 	u64 enabled_streams;
+	bool s_stream_enabled;
 };
 
 

-- 
2.34.1


  parent reply	other threads:[~2024-04-16 13:55 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-16 13:55 [PATCH v5 00/10] media: subdev: Improve stream enable/disable machinery Tomi Valkeinen
2024-04-16 13:55 ` [PATCH v5 01/10] media: subdev: Add privacy led helpers Tomi Valkeinen
2024-04-16 13:55 ` [PATCH v5 02/10] media: subdev: Use v4l2_subdev_has_op() in v4l2_subdev_enable/disable_streams() Tomi Valkeinen
2024-04-16 13:55 ` [PATCH v5 03/10] media: subdev: Add checks for subdev features Tomi Valkeinen
2024-04-16 13:55 ` Tomi Valkeinen [this message]
2024-04-16 13:55 ` [PATCH v5 05/10] media: subdev: Improve v4l2_subdev_enable/disable_streams_fallback Tomi Valkeinen
2024-04-16 13:55 ` [PATCH v5 06/10] media: subdev: Add v4l2_subdev_is_streaming() Tomi Valkeinen
2024-04-16 13:55 ` [PATCH v5 07/10] media: subdev: Support privacy led in v4l2_subdev_enable/disable_streams() Tomi Valkeinen
2024-04-19 10:23   ` Laurent Pinchart
2024-04-16 13:55 ` [PATCH v5 08/10] media: subdev: Refactor v4l2_subdev_enable/disable_streams() Tomi Valkeinen
2024-04-24  8:41   ` Hans Verkuil
2024-04-24 13:08     ` Tomi Valkeinen
2024-04-16 13:55 ` [PATCH v5 09/10] media: subdev: Support single-stream case in v4l2_subdev_enable/disable_streams() Tomi Valkeinen
2024-04-19 15:36   ` Laurent Pinchart
2024-04-24 13:05     ` Tomi Valkeinen
2024-04-24 13:09       ` Laurent Pinchart
2024-04-24 13:11         ` Tomi Valkeinen
2024-04-24 13:17           ` Laurent Pinchart
2024-04-24 13:50             ` Tomi Valkeinen
2024-04-16 13:55 ` [PATCH v5 10/10] media: subdev: Support non-routing subdevs in v4l2_subdev_s_stream_helper() Tomi Valkeinen
2024-04-19 15:46   ` Laurent Pinchart
2024-04-17 14:00 ` [PATCH v5 00/10] media: subdev: Improve stream enable/disable machinery Umang Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240416-enable-streams-impro-v5-4-bd5fcea49388@ideasonboard.com \
    --to=tomi.valkeinen@ideasonboard.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=umang.jain@ideasonboard.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.