On 28/10/2021 12:03, Jacopo Mondi wrote:
Hi Tomi,
On Thu, Oct 28, 2021 at 11:32:12AM +0300, Tomi Valkeinen wrote:
On 17/10/2021 21:24, Jacopo Mondi wrote:
Add a for_each_active_route() macro to replace the repeated pattern
of iterating on the active routes of a routing table.
Replace the existing occurrences of such pattern in the codebase.
Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
---
drivers/media/i2c/ds90ub913.c | 8 ++------
drivers/media/i2c/ds90ub953.c | 7 ++-----
drivers/media/i2c/ds90ub960.c | 8 ++------
drivers/media/i2c/max9286.c | 10 ++--------
drivers/media/platform/ti-vpe/cal-video.c | 9 ++-------
drivers/media/v4l2-core/v4l2-subdev.c | 18 ++++++++++++++++++
include/media/v4l2-subdev.h | 11 +++++++++++
7 files changed, 39 insertions(+), 32 deletions(-)
...
+struct v4l2_subdev_route *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;
+}
Also, this must be exported. I'll add that. And probably better to have a
Does it ? I would rather have it in the header, as this is only
meant to be called by the for_each_active_route() macro, and not by
other users. However it seemed to be rather long to be defined as a
static inline function in the header, so I opted to move it to the c
file.
Yes, static inline is an option. The function is a bit long-ish, though,
as you mention.
To be honest, it's not clear to me what happens if a module calls the
for_each_active_route() macro that calls this non-exported function,
so you're probably correct.
The module cannot be loaded if it refers to a non-exported symbol.
However exporting the symbol makes it available globally, but I guess
Yes, thus the prefix is a good thing =).
that's not a big deal if it's clearly documented that drivers shall
not call this directly (or maybe we want it to be available globally,
why not...)
I'll see how long helper functions similar macros have as inline in
other parts of the kernel. Maybe static inline is fine.
But if not, we'll just need to document the helper function. I don't see
why we should say it shouldn't be called directly, though. But if that
is the case, we can prefix it with __.
Tomi
