On 02/05/2017 04:48 PM, Laurent Pinchart wrote:

Hi Steve,

On Tuesday 24 Jan 2017 18:07:55 Steve Longerbeam wrote:

quoted

On 01/24/2017 04:02 AM, Philipp Zabel wrote:

quoted

On Fri, 2017-01-20 at 15:03 +0100, Hans Verkuil wrote:

quoted

quoted

+
+int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct v4l2_mbus_config
*cfg)
+{
+	struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
+	struct media_pad *pad;
+	int ret;
+
+	if (vidsw->active == -1) {
+		dev_err(sd->dev, "no configuration for inactive mux\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Retrieve media bus configuration from the entity connected to the
+	 * active input
+	 */
+	pad = media_entity_remote_pad(&vidsw->pads[vidsw->active]);
+	if (pad) {
+		sd = media_entity_to_v4l2_subdev(pad->entity);
+		ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
+		if (ret == -ENOIOCTLCMD)
+			pad = NULL;
+		else if (ret < 0) {
+			dev_err(sd->dev, "failed to get source 

configuration\n");

quoted

quoted

quoted

quoted

+			return ret;
+		}
+	}
+	if (!pad) {
+		/* Mirror the input side on the output side */
+		cfg->type = vidsw->endpoint[vidsw->active].bus_type;
+		if (cfg->type == V4L2_MBUS_PARALLEL ||
+		    cfg->type == V4L2_MBUS_BT656)
+			cfg->flags = vidsw->endpoint[vidsw-

active].bus.parallel.flags;

quoted

quoted

quoted

+	}
+
+	return 0;
+}

I am not certain this op is needed at all. In the current kernel this op
is only used by soc_camera, pxa_camera and omap3isp (somewhat dubious).
Normally this information should come from the device tree and there
should be no need for this op.

My (tentative) long-term plan was to get rid of this op.

If you don't need it, then I recommend it is removed.

Hi Hans, the imx-media driver was only calling g_mbus_config to the camera
sensor, and it was doing that to determine the sensor's bus type. This info
was already available from parsing a v4l2_of_endpoint from the sensor node.
So it was simple to remove the g_mbus_config calls, and instead rely on the
parsed sensor v4l2_of_endpoint.

That's not a good point. The imx-media driver must not parse the sensor DT 
node as it is not aware of what bindings the sensor is compatible with. 
Information must instead be queried from the sensor subdev at runtime, through 
the g_mbus_config() operation.

Of course, if you can get the information from the imx-media DT node, that's 
certainly an option. It's only information provided by the sensor driver that 
you have no choice but query using a subdev operation.

Shouldn't this come from the imx-media DT node? BTW, why is omap3isp using this?

The reason I am suspicious about this op is that it came from soc-camera and
predates the DT. The contents of v4l2_mbus_config seems very much like a HW
description to me, i.e. something that belongs in the DT.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html