diff mbox series

[v14,22/34] media: subdev: add v4l2_subdev_set_routing helper()

Message ID 20220831141357.1396081-23-tomi.valkeinen@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series [v14,01/34] media: Documentation: mc: add definitions for stream and pipeline | expand

Commit Message

Tomi Valkeinen Aug. 31, 2022, 2:13 p.m. UTC 
Add a helper function to set the subdev routing. The helper can be used
from subdev driver's set_routing op to store the routing table.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 28 +++++++++++++++++++++++++++
 include/media/v4l2-subdev.h           | 16 +++++++++++++++
 2 files changed, 44 insertions(+)

Comments

Sakari Ailus Sept. 25, 2022, 11:26 a.m. UTC | #1 
Moi,

On Wed, Aug 31, 2022 at 05:13:45PM +0300, Tomi Valkeinen wrote:
> Add a helper function to set the subdev routing. The helper can be used
> from subdev driver's set_routing op to store the routing table.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 28 +++++++++++++++++++++++++++
>  include/media/v4l2-subdev.h           | 16 +++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 1ce9a7dc0c6e..f3f872c72180 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1180,6 +1180,34 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt);
>  
> +int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
> +			    struct v4l2_subdev_state *state,
> +			    const struct v4l2_subdev_krouting *routing)
> +{
> +	struct v4l2_subdev_krouting *dst = &state->routing;
> +	const struct v4l2_subdev_krouting *src = routing;
> +	struct v4l2_subdev_krouting new_routing = { 0 };
> +
> +	lockdep_assert_held(state->lock);
> +
> +	if (src->num_routes > 0) {
> +		new_routing.routes = kmemdup(src->routes,
> +			src->num_routes * sizeof(*src->routes),

This can overflow.

> +			GFP_KERNEL);

Apart from that:

		new_routing.routes =
			kmemdup(src->routes,
				src->num_routes * sizeof(*src->routes),
				GFP_KERNEL);

> +
> +		if (!new_routing.routes)
> +			return -ENOMEM;
> +	}
> +
> +	new_routing.num_routes = src->num_routes;
> +
> +	kfree(dst->routes);
> +	*dst = new_routing;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>  
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 2d1509556ce0..b29003de8b0a 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1412,6 +1412,22 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd)
>  int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
>  			struct v4l2_subdev_format *format);
>  
> +/**
> + * v4l2_subdev_set_routing() - Set given routing to subdev state
> + * @sd: The subdevice
> + * @state: The subdevice state
> + * @routing: Routing that will be copied to subdev state
> + *
> + * This will release old routing table (if any) from the state, allocate
> + * enough space for the given routing, and copy the routing.
> + *
> + * This can be used from the subdev driver's set_routing op, after validating
> + * the routing.
> + */
> +int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
> +			    struct v4l2_subdev_state *state,
> +			    const struct v4l2_subdev_krouting *routing);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>  
>  #endif /* CONFIG_MEDIA_CONTROLLER */
Tomi Valkeinen Sept. 26, 2022, 5:56 a.m. UTC | #2 
On 25/09/2022 14:26, Sakari Ailus wrote:
> Moi,
> 
> On Wed, Aug 31, 2022 at 05:13:45PM +0300, Tomi Valkeinen wrote:
>> Add a helper function to set the subdev routing. The helper can be used
>> from subdev driver's set_routing op to store the routing table.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>   drivers/media/v4l2-core/v4l2-subdev.c | 28 +++++++++++++++++++++++++++
>>   include/media/v4l2-subdev.h           | 16 +++++++++++++++
>>   2 files changed, 44 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index 1ce9a7dc0c6e..f3f872c72180 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -1180,6 +1180,34 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
>>   }
>>   EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt);
>>   
>> +int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
>> +			    struct v4l2_subdev_state *state,
>> +			    const struct v4l2_subdev_krouting *routing)
>> +{
>> +	struct v4l2_subdev_krouting *dst = &state->routing;
>> +	const struct v4l2_subdev_krouting *src = routing;
>> +	struct v4l2_subdev_krouting new_routing = { 0 };
>> +
>> +	lockdep_assert_held(state->lock);
>> +
>> +	if (src->num_routes > 0) {
>> +		new_routing.routes = kmemdup(src->routes,
>> +			src->num_routes * sizeof(*src->routes),
> 
> This can overflow.

Right. In practice it won't happen as the num_routes has been limited to 
256, but no harm to check for overflow here.

  Tomi
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 1ce9a7dc0c6e..f3f872c72180 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1180,6 +1180,34 @@  int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt);
 
+int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_state *state,
+			    const struct v4l2_subdev_krouting *routing)
+{
+	struct v4l2_subdev_krouting *dst = &state->routing;
+	const struct v4l2_subdev_krouting *src = routing;
+	struct v4l2_subdev_krouting new_routing = { 0 };
+
+	lockdep_assert_held(state->lock);
+
+	if (src->num_routes > 0) {
+		new_routing.routes = kmemdup(src->routes,
+			src->num_routes * sizeof(*src->routes),
+			GFP_KERNEL);
+
+		if (!new_routing.routes)
+			return -ENOMEM;
+	}
+
+	new_routing.num_routes = src->num_routes;
+
+	kfree(dst->routes);
+	*dst = new_routing;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing);
+
 #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
 
 #endif /* CONFIG_MEDIA_CONTROLLER */
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 2d1509556ce0..b29003de8b0a 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1412,6 +1412,22 @@  v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd)
 int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
 			struct v4l2_subdev_format *format);
 
+/**
+ * v4l2_subdev_set_routing() - Set given routing to subdev state
+ * @sd: The subdevice
+ * @state: The subdevice state
+ * @routing: Routing that will be copied to subdev state
+ *
+ * This will release old routing table (if any) from the state, allocate
+ * enough space for the given routing, and copy the routing.
+ *
+ * This can be used from the subdev driver's set_routing op, after validating
+ * the routing.
+ */
+int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_state *state,
+			    const struct v4l2_subdev_krouting *routing);
+
 #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
 
 #endif /* CONFIG_MEDIA_CONTROLLER */