diff mbox series

[v12,23/30] media: subdev: Add for_each_active_route() macro

Message ID 20220727103639.581567-24-tomi.valkeinen@ideasonboard.com
State Superseded
Headers show
Series v4l: routing and streams support | expand

Commit Message

Tomi Valkeinen July 27, 2022, 10:36 a.m. UTC 
From: Jacopo Mondi <jacopo+renesas@jmondi.org>

Add a for_each_active_route() macro to replace the repeated pattern
of iterating on the active routes of a routing table.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 .clang-format                         |  1 +
 drivers/media/v4l2-core/v4l2-subdev.c | 20 ++++++++++++++++++++
 include/media/v4l2-subdev.h           | 13 +++++++++++++
 3 files changed, 34 insertions(+)

Comments

Tomi Valkeinen Aug. 1, 2022, 8:40 a.m. UTC | #1 
On 27/07/2022 13:36, Tomi Valkeinen wrote:

> @@ -1593,6 +1593,19 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>   				 const struct v4l2_subdev_krouting *routing,
>   				 enum v4l2_subdev_routing_restriction disallow);
>   
> +struct v4l2_subdev_route *
> +__v4l2_subdev_next_active_route(const struct v4l2_subdev_krouting *routing,
> +				struct v4l2_subdev_route *route);
> +
> +/**
> + * for_each_active_route - iterate on all active routes of a routing table
> + * @routing: The routing table
> + * @route: The route iterator
> + */
> +#define for_each_active_route(routing, route) \
> +	for ((route) = NULL;                  \
> +	     ((route) = __v4l2_subdev_next_active_route((routing), (route)));)
> +

By the way, now that we can do it, how do people feel about changing the
above (and other similar macros) to something like:

#define for_each_active_route(routing, route)        \
	for (struct v4l2_subdev_route *route = NULL; \
	     (route = __v4l2_subdev_next_active_route((routing), route));)

  Tomi
Sakari Ailus Aug. 1, 2022, 11:09 a.m. UTC | #2 
Moi,

On Mon, Aug 01, 2022 at 11:40:21AM +0300, Tomi Valkeinen wrote:
> On 27/07/2022 13:36, Tomi Valkeinen wrote:
> 
> > @@ -1593,6 +1593,19 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
> >   				 const struct v4l2_subdev_krouting *routing,
> >   				 enum v4l2_subdev_routing_restriction disallow);
> > +struct v4l2_subdev_route *
> > +__v4l2_subdev_next_active_route(const struct v4l2_subdev_krouting *routing,
> > +				struct v4l2_subdev_route *route);
> > +
> > +/**
> > + * for_each_active_route - iterate on all active routes of a routing table
> > + * @routing: The routing table
> > + * @route: The route iterator
> > + */
> > +#define for_each_active_route(routing, route) \
> > +	for ((route) = NULL;                  \
> > +	     ((route) = __v4l2_subdev_next_active_route((routing), (route)));)
> > +
> 
> By the way, now that we can do it, how do people feel about changing the
> above (and other similar macros) to something like:
> 
> #define for_each_active_route(routing, route)        \
> 	for (struct v4l2_subdev_route *route = NULL; \
> 	     (route = __v4l2_subdev_next_active_route((routing), route));)

Will this compile? :-)

Every such macro works that way and I'd think this is expected by the
developers in general. You also often need to break out of the loop and
keep that variable intact, this also supports doing it to usual way.

I.e. I'd keep it as-is.
diff mbox series

Patch

diff --git a/.clang-format b/.clang-format
index 9b87ea1fc16e..2629a5051bf8 100644
--- a/.clang-format
+++ b/.clang-format
@@ -190,6 +190,7 @@  ForEachMacros:
   - 'for_each_active_dev_scope'
   - 'for_each_active_drhd_unit'
   - 'for_each_active_iommu'
+  - 'for_each_active_route'
   - 'for_each_aggr_pgid'
   - 'for_each_available_child_of_node'
   - 'for_each_bench'
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index c9f3d1a0ede1..f3219c8a6317 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1734,6 +1734,26 @@  int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_routing_validate);
 
+struct v4l2_subdev_route *
+__v4l2_subdev_next_active_route(const struct v4l2_subdev_krouting *routing,
+				struct v4l2_subdev_route *route)
+{
+	if (route)
+		++route;
+	else
+		route = &routing->routes[0];
+
+	for (; route < routing->routes + routing->num_routes; ++route) {
+		if (!(route->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
+			continue;
+
+		return route;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(__v4l2_subdev_next_active_route);
+
 #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 a088ff1ecdc1..f749effadde2 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1593,6 +1593,19 @@  int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
 				 const struct v4l2_subdev_krouting *routing,
 				 enum v4l2_subdev_routing_restriction disallow);
 
+struct v4l2_subdev_route *
+__v4l2_subdev_next_active_route(const struct v4l2_subdev_krouting *routing,
+				struct v4l2_subdev_route *route);
+
+/**
+ * for_each_active_route - iterate on all active routes of a routing table
+ * @routing: The routing table
+ * @route: The route iterator
+ */
+#define for_each_active_route(routing, route) \
+	for ((route) = NULL;                  \
+	     ((route) = __v4l2_subdev_next_active_route((routing), (route)));)
+
 #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
 
 #endif /* CONFIG_MEDIA_CONTROLLER */