| Message ID | 20221003121852.616745-9-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | v4l: routing and streams support | expand |
On 03.10.2022 15:18, Tomi Valkeinen wrote: >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(+) > >diff --git a/.clang-format b/.clang-format >index 1247d54f9e49..31f39ae78f7b 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 3ae4f39a50e4..1049c07d2e49 100644 >--- a/drivers/media/v4l2-core/v4l2-subdev.c >+++ b/drivers/media/v4l2-core/v4l2-subdev.c >@@ -1212,6 +1212,26 @@ int v4l2_subdev_set_routing(struct v4l2_subdev *sd, > } > EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing); > >+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 7962e6572bda..89e58208e330 100644 >--- a/include/media/v4l2-subdev.h >+++ b/include/media/v4l2-subdev.h >@@ -1435,6 +1435,19 @@ int v4l2_subdev_set_routing(struct v4l2_subdev *sd, > struct v4l2_subdev_state *state, > const struct v4l2_subdev_krouting *routing); > >+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)));) Hi, shouldn't it be something like: for ((route) = NULL; (route) ; (route) = __v4l2_subdev_next_active_route((routing), (route))) Thanks, Dafna >+ > #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ > > #endif /* CONFIG_MEDIA_CONTROLLER */ >-- >2.34.1 >
On 09/10/2022 08:38, Dafna Hirschfeld wrote: > On 03.10.2022 15:18, Tomi Valkeinen wrote: >> 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(+) >> >> diff --git a/.clang-format b/.clang-format >> index 1247d54f9e49..31f39ae78f7b 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 3ae4f39a50e4..1049c07d2e49 100644 >> --- a/drivers/media/v4l2-core/v4l2-subdev.c >> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >> @@ -1212,6 +1212,26 @@ int v4l2_subdev_set_routing(struct v4l2_subdev >> *sd, >> } >> EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing); >> >> +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 7962e6572bda..89e58208e330 100644 >> --- a/include/media/v4l2-subdev.h >> +++ b/include/media/v4l2-subdev.h >> @@ -1435,6 +1435,19 @@ int v4l2_subdev_set_routing(struct v4l2_subdev >> *sd, >> struct v4l2_subdev_state *state, >> const struct v4l2_subdev_krouting *routing); >> >> +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)));) > Hi, shouldn't it be something like: > for ((route) = NULL; (route) ; (route) = > __v4l2_subdev_next_active_route((routing), (route))) What you suggest would never do anything: you initialize route to NULL, and then check if the route is !NULL. I also find the current version a bit "interesting", but afaics, it works correctly. Tomi
diff --git a/.clang-format b/.clang-format index 1247d54f9e49..31f39ae78f7b 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 3ae4f39a50e4..1049c07d2e49 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -1212,6 +1212,26 @@ int v4l2_subdev_set_routing(struct v4l2_subdev *sd, } EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing); +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 7962e6572bda..89e58208e330 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1435,6 +1435,19 @@ int v4l2_subdev_set_routing(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, const struct v4l2_subdev_krouting *routing); +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 */