All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] v4l: subdev: Warn if disabling streaming failed, return success
@ 2022-10-26  6:51 Sakari Ailus
  2022-10-26 11:54 ` Lad, Prabhakar
  2022-11-22  0:35 ` Laurent Pinchart
  0 siblings, 2 replies; 3+ messages in thread
From: Sakari Ailus @ 2022-10-26  6:51 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, Prabhakar Lad

Complain in the newly added s_stream video op wrapper if disabling
streaming failed. Also return zero in this case as there's nothing the
caller can do to return the error.

This way drivers also won't need to bother with printing error messages.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 15 +++++++++++++++
 include/media/v4l2-subdev.h           |  6 ++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 5c27bac772ea4..8a4ca2bd1584d 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -318,6 +318,20 @@ static int call_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad,
 	       sd->ops->pad->get_mbus_config(sd, pad, config);
 }
 
+static int call_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	int ret;
+
+	ret = sd->ops->video->s_stream(sd, enable);
+
+	if (!enable && ret < 0) {
+		dev_warn(sd->dev, "disabling streaming failed (%d)\n", ret);
+		return 0;
+	}
+
+	return ret;
+}
+
 #ifdef CONFIG_MEDIA_CONTROLLER
 /*
  * Create state-management wrapper for pad ops dealing with subdev state. The
@@ -377,6 +391,7 @@ static const struct v4l2_subdev_pad_ops v4l2_subdev_call_pad_wrappers = {
 static const struct v4l2_subdev_video_ops v4l2_subdev_call_video_wrappers = {
 	.g_frame_interval	= call_g_frame_interval,
 	.s_frame_interval	= call_s_frame_interval,
+	.s_stream		= call_s_stream,
 };
 
 const struct v4l2_subdev_ops v4l2_subdev_call_wrappers = {
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 54566d139da79..b15fa9930f30c 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -440,8 +440,10 @@ enum v4l2_subdev_pre_streamon_flags {
  * @g_input_status: get input status. Same as the status field in the
  *	&struct v4l2_input
  *
- * @s_stream: used to notify the driver that a video stream will start or has
- *	stopped.
+ * @s_stream: start (enabled == 1) or stop (enabled == 0) streaming on the
+ *	sub-device. Failure on stop will remove any resources acquired in
+ *	streaming start, while the error code is still returned by the driver.
+ *	Also see call_s_stream wrapper in v4l2-subdev.c.
  *
  * @g_pixelaspect: callback to return the pixelaspect ratio.
  *
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] v4l: subdev: Warn if disabling streaming failed, return success
  2022-10-26  6:51 [PATCH 1/1] v4l: subdev: Warn if disabling streaming failed, return success Sakari Ailus
@ 2022-10-26 11:54 ` Lad, Prabhakar
  2022-11-22  0:35 ` Laurent Pinchart
  1 sibling, 0 replies; 3+ messages in thread
From: Lad, Prabhakar @ 2022-10-26 11:54 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, laurent.pinchart

Hi Sakari,

Thank you for the patch.

On Wed, Oct 26, 2022 at 7:50 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Complain in the newly added s_stream video op wrapper if disabling
> streaming failed. Also return zero in this case as there's nothing the
> caller can do to return the error.
>
> This way drivers also won't need to bother with printing error messages.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 15 +++++++++++++++
>  include/media/v4l2-subdev.h           |  6 ++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
>

Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar


> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 5c27bac772ea4..8a4ca2bd1584d 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -318,6 +318,20 @@ static int call_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad,
>                sd->ops->pad->get_mbus_config(sd, pad, config);
>  }
>
> +static int call_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +       int ret;
> +
> +       ret = sd->ops->video->s_stream(sd, enable);
> +
> +       if (!enable && ret < 0) {
> +               dev_warn(sd->dev, "disabling streaming failed (%d)\n", ret);
> +               return 0;
> +       }
> +
> +       return ret;
> +}
> +
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  /*
>   * Create state-management wrapper for pad ops dealing with subdev state. The
> @@ -377,6 +391,7 @@ static const struct v4l2_subdev_pad_ops v4l2_subdev_call_pad_wrappers = {
>  static const struct v4l2_subdev_video_ops v4l2_subdev_call_video_wrappers = {
>         .g_frame_interval       = call_g_frame_interval,
>         .s_frame_interval       = call_s_frame_interval,
> +       .s_stream               = call_s_stream,
>  };
>
>  const struct v4l2_subdev_ops v4l2_subdev_call_wrappers = {
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 54566d139da79..b15fa9930f30c 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -440,8 +440,10 @@ enum v4l2_subdev_pre_streamon_flags {
>   * @g_input_status: get input status. Same as the status field in the
>   *     &struct v4l2_input
>   *
> - * @s_stream: used to notify the driver that a video stream will start or has
> - *     stopped.
> + * @s_stream: start (enabled == 1) or stop (enabled == 0) streaming on the
> + *     sub-device. Failure on stop will remove any resources acquired in
> + *     streaming start, while the error code is still returned by the driver.
> + *     Also see call_s_stream wrapper in v4l2-subdev.c.
>   *
>   * @g_pixelaspect: callback to return the pixelaspect ratio.
>   *
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] v4l: subdev: Warn if disabling streaming failed, return success
  2022-10-26  6:51 [PATCH 1/1] v4l: subdev: Warn if disabling streaming failed, return success Sakari Ailus
  2022-10-26 11:54 ` Lad, Prabhakar
@ 2022-11-22  0:35 ` Laurent Pinchart
  1 sibling, 0 replies; 3+ messages in thread
From: Laurent Pinchart @ 2022-11-22  0:35 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, Prabhakar Lad

Hi Sakari,

Thank you for the patch.

On Wed, Oct 26, 2022 at 09:51:23AM +0300, Sakari Ailus wrote:
> Complain in the newly added s_stream video op wrapper if disabling
> streaming failed. Also return zero in this case as there's nothing the
> caller can do to return the error.
> 
> This way drivers also won't need to bother with printing error messages.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 15 +++++++++++++++
>  include/media/v4l2-subdev.h           |  6 ++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 5c27bac772ea4..8a4ca2bd1584d 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -318,6 +318,20 @@ static int call_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad,
>  	       sd->ops->pad->get_mbus_config(sd, pad, config);
>  }
>  
> +static int call_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	int ret;
> +
> +	ret = sd->ops->video->s_stream(sd, enable);
> +
> +	if (!enable && ret < 0) {
> +		dev_warn(sd->dev, "disabling streaming failed (%d)\n", ret);
> +		return 0;
> +	}
> +
> +	return ret;
> +}
> +
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  /*
>   * Create state-management wrapper for pad ops dealing with subdev state. The
> @@ -377,6 +391,7 @@ static const struct v4l2_subdev_pad_ops v4l2_subdev_call_pad_wrappers = {
>  static const struct v4l2_subdev_video_ops v4l2_subdev_call_video_wrappers = {
>  	.g_frame_interval	= call_g_frame_interval,
>  	.s_frame_interval	= call_s_frame_interval,
> +	.s_stream		= call_s_stream,
>  };
>  
>  const struct v4l2_subdev_ops v4l2_subdev_call_wrappers = {
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 54566d139da79..b15fa9930f30c 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -440,8 +440,10 @@ enum v4l2_subdev_pre_streamon_flags {
>   * @g_input_status: get input status. Same as the status field in the
>   *	&struct v4l2_input
>   *
> - * @s_stream: used to notify the driver that a video stream will start or has
> - *	stopped.
> + * @s_stream: start (enabled == 1) or stop (enabled == 0) streaming on the
> + *	sub-device. Failure on stop will remove any resources acquired in
> + *	streaming start, while the error code is still returned by the driver.
> + *	Also see call_s_stream wrapper in v4l2-subdev.c.
>   *
>   * @g_pixelaspect: callback to return the pixelaspect ratio.
>   *

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-22  0:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-26  6:51 [PATCH 1/1] v4l: subdev: Warn if disabling streaming failed, return success Sakari Ailus
2022-10-26 11:54 ` Lad, Prabhakar
2022-11-22  0:35 ` Laurent Pinchart

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.