Hi Hans,

On Wednesday 19 January 2011 08:23:18 Hans Verkuil wrote:

On Wednesday, January 19, 2011 00:50:35 Laurent Pinchart wrote:

quoted

On Wednesday 19 January 2011 00:05:10 Hans Verkuil wrote:

quoted

On Tuesday, January 18, 2011 23:18:42 Martin Hostettler wrote:

[snip]

quoted

quoted

quoted

+#ifdef CONFIG_VIDEO_ADV_DEBUG
+static long mt9m032_ioctl(struct v4l2_subdev *sd, unsigned int cmd,
void *arg) +{
+	if (cmd == VIDIOC_DBG_G_REGISTER || cmd == VIDIOC_DBG_S_REGISTER) {
+		struct v4l2_dbg_register *p = arg;
+
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+
+		if (cmd == VIDIOC_DBG_G_REGISTER)
+			return v4l2_subdev_call(sd, core, g_register, p);
+		else
+			return v4l2_subdev_call(sd, core, s_register, p);
+	} else {
+		return -ENOIOCTLCMD;
+	}
+}

Huh? Ah, I get it. This is for when the user uses the subdev's device
node directly. This is not good, the v4l2 framework should do
translate this to g/s_register.

Agreed.

quoted

The same should be done for g_chip_ident, I guess.

I don't think we need g_chip_ident for subdev nodes, do we ?

Why not? It makes the use of v4l2-dbg a bit easier if it is there. If you
provide g/s_register, then you should provide this one as well.

Because v4l2_dbg_register::match is used to address a specific subdevice. When 
issuing the VIDIOC_DBG_[GS]_REGISTER ioctls on a subdev device node, the field 
isn't needed anymore.

I though of another one that should be handled in the framework:
VIDIOC_LOG_STATUS.

That definitely should be handled as well.

-- 
Regards,

Laurent Pinchart