Hi Tomi,

Thank you for the patch.

On Mon, May 24, 2021 at 01:44:02PM +0300, Tomi Valkeinen wrote:

This function seems to be ised in v4l2_subdev_link_validate() only,
let's make it static for now.

+{
+	int ret;
+
+	routing->which = state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;

I think we could simplify this if we mandate subdev drivers to embed an
instance of v4l2_subdev_state for the active state. We could then drop
the .get_routing() operation. We could actually drop the .get_fmt()
operation too, but that would be a long term goal. As .get_routing() is
new, I'd rather not introduce it, and push drivers that need routing
support to use v4l2_subdev_state.

+	routing->routes = NULL;
+	routing->num_routes = 0;
+
+	ret = v4l2_subdev_call(sd, pad, get_routing, state, routing);
+	if (ret == 0)
+		return 0;
+	if (ret != -ENOSPC)
+		return ret;
+
+	routing->routes = kvmalloc_array(routing->num_routes,
+					 sizeof(*routing->routes), GFP_KERNEL);
+	if (!routing->routes)
+		return -ENOMEM;
+
+	ret = v4l2_subdev_call(sd, pad, get_routing, state, routing);
+	if (ret) {
+		kvfree(routing->routes);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_get_routing);
+
+void v4l2_subdev_free_routing(struct v4l2_subdev_krouting *routing)
+{
+	kvfree(routing->routes);
+	routing->routes = NULL;
+	routing->num_routes = 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_free_routing);
+
+int v4l2_subdev_cpy_routing(struct v4l2_subdev_krouting *dst,
+			    const struct v4l2_subdev_krouting *src)
+{
+	if (dst->num_routes < src->num_routes) {
+		dst->num_routes = src->num_routes;
+		return -ENOSPC;
+	}
+
+	memcpy(dst->routes, src->routes,
+	       src->num_routes * sizeof(*src->routes));
+	dst->num_routes = src->num_routes;
+	dst->which = src->which;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_cpy_routing);

These two functions could also be made internal if we mandate usage of
v4l2_subdev_state.

+
+int v4l2_subdev_dup_routing(struct v4l2_subdev_krouting *dst,
+			    const struct v4l2_subdev_krouting *src)
+{
+	v4l2_subdev_free_routing(dst);
+
+	if (src->num_routes == 0) {
+		dst->which = src->which;
+		return 0;
+	}
+
+	dst->routes = kvmalloc_array(src->num_routes, sizeof(*src->routes),
+				     GFP_KERNEL);
+	if (!dst->routes)
+		return -ENOMEM;
+
+	memcpy(dst->routes, src->routes,
+	       src->num_routes * sizeof(*src->routes));
+	dst->num_routes = src->num_routes;
+	dst->which = src->which;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_dup_routing);
+
+bool v4l2_subdev_has_route(struct v4l2_subdev_krouting *routing,

The first argument can be const.

Maybe "routing table" (here and where applicable) ? Up to you.

+ *
+ * @routing: The routing to be freed
+ *
+ * Frees the routing data in @routing.
+ */
+void v4l2_subdev_free_routing(struct v4l2_subdev_krouting *routing);
+
+/**
+ * v4l2_subdev_cpy_routing() - Copy the routing
+ *
+ * @dst: The destination routing
+ * @src: The source routing
+ *
+ * Copies routing from @src to @dst without allocating space. If @dst does not
+ * have enough space, set dst->num_routes to the required number of routes, and
+ * return -ENOSPC.
+ *
+ * Can be used in subdevice's v4l2_subdev_pad_ops.get_routing() callback.
+ */
+int v4l2_subdev_cpy_routing(struct v4l2_subdev_krouting *dst,
+			    const struct v4l2_subdev_krouting *src);
+
+/**
+ * v4l2_subdev_dup_routing() - Duplicate the routing
+ *
+ * @dst: The destination routing
+ * @src: The source routing
+ *
+ * Makes a duplicate of the routing from @src to @dst by allocating enough
+ * memory and making a copy of the routing.
+ *
+ * Can be used in subdevice's v4l2_subdev_pad_ops.set_routing() callback
+ * to store the given routing.
+ *
+ * Must be freed with v4l2_subdev_free_routing after use.
+ */
+int v4l2_subdev_dup_routing(struct v4l2_subdev_krouting *dst,
+			    const struct v4l2_subdev_krouting *src);
+
+/**
+ * v4l2_subdev_has_route() - Check if there is a route between two pads

s/a route/an active route/

+ *
+ * @routing: The subdevice's routing
+ * @pad0: First pad
+ * @pad1: Second pad
+ *
+ * Returns whether there is a route between @pad0 and @pad1 of the same

Same here. Maybe we want to also mention "a direct route", as the
function doesn't return true when given pads B and C when there are
direct routes from A to B and A to C.

+ * subdevice according to the given routing.
+ */
+bool v4l2_subdev_has_route(struct v4l2_subdev_krouting *routing,
+			   unsigned int pad0, unsigned int pad1);
+
 #endif

-- 
Regards,

Laurent Pinchart