[RFC PATCH 0/6] Add VIDIOC_DBG_G_CHIP_NAME ioctl.

[RFC PATCH 0/6] Add VIDIOC_DBG_G_CHIP_NAME ioctl.

[Hans Verkuil]

This is a patch series implementing this proposal:

http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/61539

It keeps the old CHIP_IDENT around for now. My plan is to start removing
support for VIDIOC_G_CHIP_IDENT and the match types V4L2_CHIP_MATCH_I2C_DRIVER,
V4L2_CHIP_MATCH_I2C_ADDR and V4L2_CHIP_MATCH_AC97 for 3.11. Once it's all
removed we can also ditch the v4l2-chip-ident.h header.

These debugging ioctls have always been a bit problematic, but sub-devices
are a much better fit and greatly simplify the implementation.

Comments are welcome!

Regards,

	Hans

[RFC PATCH 1/6] v4l2-common: remove obsolete check for ' at the end of a driver name.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

During the transition to sub-devices several years ago non-subdev drivers
had a ' at the end of their driver name to tell them apart from already
converted drivers. This check was a left-over from that time and can be
removed.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-common.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index aa044f4..46d2b9b 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -251,9 +251,6 @@ int v4l2_chip_match_i2c_client(struct i2c_client *c, const struct v4l2_dbg_match
 		if (c->driver == NULL || c->driver->driver.name == NULL)
 			return 0;
 		len = strlen(c->driver->driver.name);
-		/* legacy drivers have a ' suffix, don't try to match that */
-		if (len && c->driver->driver.name[len - 1] == '\'')
-			len--;
 		return len && !strncmp(c->driver->driver.name, match->name, len);
 	case V4L2_CHIP_MATCH_I2C_ADDR:
 		return c->addr == match->addr;
-- 
1.7.10.4

[RFC PATCH 2/6] v4l2: add new VIDIOC_DBG_G_CHIP_NAME ioctl.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Simplify the debugging ioctls by creating the VIDIOC_DBG_G_CHIP_NAME ioctl.
This will eventually replace VIDIOC_DBG_G_CHIP_IDENT. Chip matching is done
by the name or index of subdevices or an index to a bridge chip. Most of this
can all be done automatically, so most drivers just need to provide get/set
register ops.

In particular, it is now possible to get/set subdev registers without
requiring assistance of the bridge driver.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-common.c |    5 +-
 drivers/media/v4l2-core/v4l2-dev.c    |    5 +-
 drivers/media/v4l2-core/v4l2-ioctl.c  |  115 +++++++++++++++++++++++++++++++--
 include/media/v4l2-ioctl.h            |    3 +
 include/uapi/linux/videodev2.h        |   34 +++++++---
 5 files changed, 146 insertions(+), 16 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 46d2b9b..537be23 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -230,7 +230,7 @@ EXPORT_SYMBOL(v4l2_ctrl_next);
 int v4l2_chip_match_host(const struct v4l2_dbg_match *match)
 {
 	switch (match->type) {
-	case V4L2_CHIP_MATCH_HOST:
+	case V4L2_CHIP_MATCH_BRIDGE:
 		return match->addr == 0;
 	default:
 		return 0;
@@ -254,6 +254,9 @@ int v4l2_chip_match_i2c_client(struct i2c_client *c, const struct v4l2_dbg_match
 		return len && !strncmp(c->driver->driver.name, match->name, len);
 	case V4L2_CHIP_MATCH_I2C_ADDR:
 		return c->addr == match->addr;
+	case V4L2_CHIP_MATCH_SUBDEV_IDX:
+	case V4L2_CHIP_MATCH_SUBDEV_NAME:
+		return 1;
 	default:
 		return 0;
 	}
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index de1e9ab..c0c651d 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -591,9 +591,10 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
 	SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
 	SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
+	set_bit(_IOC_NR(VIDIOC_DBG_G_CHIP_NAME), valid_ioctls);
 #ifdef CONFIG_VIDEO_ADV_DEBUG
-	SET_VALID_IOCTL(ops, VIDIOC_DBG_G_REGISTER, vidioc_g_register);
-	SET_VALID_IOCTL(ops, VIDIOC_DBG_S_REGISTER, vidioc_s_register);
+	set_bit(_IOC_NR(VIDIOC_DBG_G_REGISTER), valid_ioctls);
+	set_bit(_IOC_NR(VIDIOC_DBG_S_REGISTER), valid_ioctls);
 #endif
 	SET_VALID_IOCTL(ops, VIDIOC_DBG_G_CHIP_IDENT, vidioc_g_chip_ident);
 	/* yes, really vidioc_subscribe_event */
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index aa6e7c7..923ec95 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -622,7 +622,8 @@ static void v4l_print_dbg_chip_ident(const void *arg, bool write_only)
 	const struct v4l2_dbg_chip_ident *p = arg;
 
 	pr_cont("type=%u, ", p->match.type);
-	if (p->match.type == V4L2_CHIP_MATCH_I2C_DRIVER)
+	if (p->match.type == V4L2_CHIP_MATCH_I2C_DRIVER ||
+	    p->match.type == V4L2_CHIP_MATCH_SUBDEV_NAME)
 		pr_cont("name=%s, ", p->match.name);
 	else
 		pr_cont("addr=%u, ", p->match.addr);
@@ -630,12 +631,27 @@ static void v4l_print_dbg_chip_ident(const void *arg, bool write_only)
 			p->ident, p->revision);
 }
 
+static void v4l_print_dbg_chip_name(const void *arg, bool write_only)
+{
+	const struct v4l2_dbg_chip_name *p = arg;
+
+	pr_cont("type=%u, ", p->match.type);
+	if (p->match.type == V4L2_CHIP_MATCH_I2C_DRIVER ||
+	    p->match.type == V4L2_CHIP_MATCH_SUBDEV_NAME)
+		pr_cont("name=%.*s, ",
+				(int)sizeof(p->match.name), p->match.name);
+	else
+		pr_cont("addr=%u, ", p->match.addr);
+	pr_cont("name=%.*s\n", (int)sizeof(p->name), p->name);
+}
+
 static void v4l_print_dbg_register(const void *arg, bool write_only)
 {
 	const struct v4l2_dbg_register *p = arg;
 
 	pr_cont("type=%u, ", p->match.type);
-	if (p->match.type == V4L2_CHIP_MATCH_I2C_DRIVER)
+	if (p->match.type == V4L2_CHIP_MATCH_I2C_DRIVER ||
+	    p->match.type == V4L2_CHIP_MATCH_SUBDEV_NAME)
 		pr_cont("name=%s, ", p->match.name);
 	else
 		pr_cont("addr=%u, ", p->match.addr);
@@ -1787,15 +1803,38 @@ static int v4l_log_status(const struct v4l2_ioctl_ops *ops,
 	return ret;
 }
 
+static bool v4l_dbg_found_match(const struct v4l2_dbg_match *match,
+		struct v4l2_subdev *sd, int idx)
+{
+	if (match->type == V4L2_CHIP_MATCH_SUBDEV_IDX)
+		return match->addr == idx;
+	return !strcmp(match->name, sd->name);
+}
+
 static int v4l_dbg_g_register(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	struct v4l2_dbg_register *p = arg;
+	struct video_device *vfd = video_devdata(file);
+	struct v4l2_subdev *sd;
+	int idx = 0;
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
-	return ops->vidioc_g_register(file, fh, p);
+	if (p->match.type == V4L2_CHIP_MATCH_SUBDEV_IDX ||
+	    p->match.type == V4L2_CHIP_MATCH_SUBDEV_NAME) {
+		if (vfd->v4l2_dev == NULL)
+			return -EINVAL;
+		v4l2_device_for_each_subdev(sd, vfd->v4l2_dev) {
+			if (v4l_dbg_found_match(&p->match, sd, idx++))
+				return v4l2_subdev_call(sd, core, g_register, p);
+		}
+		return -EINVAL;
+	}
+	if (ops->vidioc_g_register)
+		return ops->vidioc_g_register(file, fh, p);
+	return -EINVAL;
 #else
 	return -ENOTTY;
 #endif
@@ -1806,10 +1845,25 @@ static int v4l_dbg_s_register(const struct v4l2_ioctl_ops *ops,
 {
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	struct v4l2_dbg_register *p = arg;
+	struct video_device *vfd = video_devdata(file);
+	struct v4l2_subdev *sd;
+	int idx = 0;
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
-	return ops->vidioc_s_register(file, fh, p);
+	if (p->match.type == V4L2_CHIP_MATCH_SUBDEV_IDX ||
+	    p->match.type == V4L2_CHIP_MATCH_SUBDEV_NAME) {
+		if (vfd->v4l2_dev == NULL)
+			return -EINVAL;
+		v4l2_device_for_each_subdev(sd, vfd->v4l2_dev) {
+			if (v4l_dbg_found_match(&p->match, sd, idx++))
+				return v4l2_subdev_call(sd, core, s_register, p);
+		}
+		return -EINVAL;
+	}
+	if (ops->vidioc_s_register)
+		return ops->vidioc_s_register(file, fh, p);
+	return -EINVAL;
 #else
 	return -ENOTTY;
 #endif
@@ -1822,9 +1876,61 @@ static int v4l_dbg_g_chip_ident(const struct v4l2_ioctl_ops *ops,
 
 	p->ident = V4L2_IDENT_NONE;
 	p->revision = 0;
+	if (p->match.type == V4L2_CHIP_MATCH_SUBDEV_NAME ||
+	    p->match.type == V4L2_CHIP_MATCH_SUBDEV_IDX)
+		return -EINVAL;
 	return ops->vidioc_g_chip_ident(file, fh, p);
 }
 
+static int v4l_dbg_g_chip_name(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh, void *arg)
+{
+	struct video_device *vfd = video_devdata(file);
+	struct v4l2_dbg_chip_name *p = arg;
+	struct v4l2_subdev *sd;
+	int idx = 0;
+
+	switch (p->match.type) {
+	case V4L2_CHIP_MATCH_BRIDGE:
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+		if (ops->vidioc_s_register)
+			p->flags |= V4L2_CHIP_FL_WRITABLE;
+		if (ops->vidioc_g_register)
+			p->flags |= V4L2_CHIP_FL_READABLE;
+#endif
+		if (ops->vidioc_g_chip_name)
+			return ops->vidioc_g_chip_name(file, fh, arg);
+		if (p->match.addr)
+			return -EINVAL;
+		if (vfd->v4l2_dev)
+			strlcpy(p->name, vfd->v4l2_dev->name, sizeof(p->name));
+		else if (vfd->parent)
+			strlcpy(p->name, vfd->parent->driver->name, sizeof(p->name));
+		else
+			strlcpy(p->name, "bridge", sizeof(p->name));
+		return 0;
+
+	case V4L2_CHIP_MATCH_SUBDEV_IDX:
+	case V4L2_CHIP_MATCH_SUBDEV_NAME:
+		if (vfd->v4l2_dev == NULL)
+			break;
+		v4l2_device_for_each_subdev(sd, vfd->v4l2_dev) {
+			if (v4l_dbg_found_match(&p->match, sd, idx++)) {
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+				if (sd->ops->core && sd->ops->core->s_register)
+					p->flags |= V4L2_CHIP_FL_WRITABLE;
+				if (sd->ops->core && sd->ops->core->g_register)
+					p->flags |= V4L2_CHIP_FL_READABLE;
+#endif
+				strlcpy(p->name, sd->name, sizeof(p->name));
+				return 0;
+			}
+		}
+		break;
+	}
+	return -EINVAL;
+}
+
 static int v4l_dqevent(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
@@ -2043,6 +2149,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
 	IOCTL_INFO_STD(VIDIOC_QUERY_DV_TIMINGS, vidioc_query_dv_timings, v4l_print_dv_timings, 0),
 	IOCTL_INFO_STD(VIDIOC_DV_TIMINGS_CAP, vidioc_dv_timings_cap, v4l_print_dv_timings_cap, INFO_FL_CLEAR(v4l2_dv_timings_cap, type)),
 	IOCTL_INFO_FNC(VIDIOC_ENUM_FREQ_BANDS, v4l_enum_freq_bands, v4l_print_freq_band, 0),
+	IOCTL_INFO_FNC(VIDIOC_DBG_G_CHIP_NAME, v4l_dbg_g_chip_name, v4l_print_dbg_chip_name, INFO_FL_CLEAR(v4l2_dbg_chip_name, match)),
 };
 #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
 
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 4118ad1..ad44cbf 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -247,6 +247,9 @@ struct v4l2_ioctl_ops {
 	int (*vidioc_g_chip_ident)     (struct file *file, void *fh,
 					struct v4l2_dbg_chip_ident *chip);
 
+	int (*vidioc_g_chip_name)      (struct file *file, void *fh,
+					struct v4l2_dbg_chip_name *chip);
+
 	int (*vidioc_enum_framesizes)   (struct file *file, void *fh,
 					 struct v4l2_frmsizeenum *fsize);
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index b5f5cdd..6bea1ab 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1855,10 +1855,13 @@ struct v4l2_event_subscription {
 
 /* VIDIOC_DBG_G_REGISTER and VIDIOC_DBG_S_REGISTER */
 
-#define V4L2_CHIP_MATCH_HOST       0  /* Match against chip ID on host (0 for the host) */
-#define V4L2_CHIP_MATCH_I2C_DRIVER 1  /* Match against I2C driver name */
-#define V4L2_CHIP_MATCH_I2C_ADDR   2  /* Match against I2C 7-bit address */
-#define V4L2_CHIP_MATCH_AC97       3  /* Match against anciliary AC97 chip */
+#define V4L2_CHIP_MATCH_BRIDGE      0  /* Match against chip ID on the bridge (0 for the bridge) */
+#define V4L2_CHIP_MATCH_HOST V4L2_CHIP_MATCH_BRIDGE
+#define V4L2_CHIP_MATCH_I2C_DRIVER  1  /* Match against I2C driver name */
+#define V4L2_CHIP_MATCH_I2C_ADDR    2  /* Match against I2C 7-bit address */
+#define V4L2_CHIP_MATCH_AC97        3  /* Match against anciliary AC97 chip */
+#define V4L2_CHIP_MATCH_SUBDEV_NAME 4  /* Match against subdev name */
+#define V4L2_CHIP_MATCH_SUBDEV_IDX  5  /* Match against subdev index */
 
 struct v4l2_dbg_match {
 	__u32 type; /* Match type */
@@ -1882,6 +1885,17 @@ struct v4l2_dbg_chip_ident {
 	__u32 revision;    /* chip revision, chip specific */
 } __attribute__ ((packed));
 
+#define V4L2_CHIP_FL_READABLE (1 << 0)
+#define V4L2_CHIP_FL_WRITABLE (1 << 1)
+
+/* VIDIOC_DBG_G_CHIP_NAME */
+struct v4l2_dbg_chip_name {
+	struct v4l2_dbg_match match;
+	char name[32];
+	__u32 flags;
+	__u32 reserved[8];
+} __attribute__ ((packed));
+
 /**
  * struct v4l2_create_buffers - VIDIOC_CREATE_BUFS argument
  * @index:	on return, index of the first created buffer
@@ -1959,15 +1973,12 @@ struct v4l2_create_buffers {
 #define VIDIOC_G_EXT_CTRLS	_IOWR('V', 71, struct v4l2_ext_controls)
 #define VIDIOC_S_EXT_CTRLS	_IOWR('V', 72, struct v4l2_ext_controls)
 #define VIDIOC_TRY_EXT_CTRLS	_IOWR('V', 73, struct v4l2_ext_controls)
-#if 1
 #define VIDIOC_ENUM_FRAMESIZES	_IOWR('V', 74, struct v4l2_frmsizeenum)
 #define VIDIOC_ENUM_FRAMEINTERVALS _IOWR('V', 75, struct v4l2_frmivalenum)
 #define VIDIOC_G_ENC_INDEX       _IOR('V', 76, struct v4l2_enc_idx)
 #define VIDIOC_ENCODER_CMD      _IOWR('V', 77, struct v4l2_encoder_cmd)
 #define VIDIOC_TRY_ENCODER_CMD  _IOWR('V', 78, struct v4l2_encoder_cmd)
-#endif
 
-#if 1
 /* Experimental, meant for debugging, testing and internal use.
    Only implemented if CONFIG_VIDEO_ADV_DEBUG is defined.
    You must be root to use these ioctls. Never use these in applications! */
@@ -1975,9 +1986,10 @@ struct v4l2_create_buffers {
 #define	VIDIOC_DBG_G_REGISTER 	_IOWR('V', 80, struct v4l2_dbg_register)
 
 /* Experimental, meant for debugging, testing and internal use.
-   Never use this ioctl in applications! */
+   Never use this ioctl in applications!
+   Note: this ioctl is deprecated in favor of VIDIOC_DBG_G_CHIP_NAME and
+   will go away in the future. */
 #define VIDIOC_DBG_G_CHIP_IDENT _IOWR('V', 81, struct v4l2_dbg_chip_ident)
-#endif
 
 #define VIDIOC_S_HW_FREQ_SEEK	 _IOW('V', 82, struct v4l2_hw_freq_seek)
 
@@ -2017,6 +2029,10 @@ struct v4l2_create_buffers {
    versions. */
 #define VIDIOC_ENUM_FREQ_BANDS	_IOWR('V', 101, struct v4l2_frequency_band)
 
+/* Experimental, meant for debugging, testing and internal use.
+   Never use these in applications! */
+#define VIDIOC_DBG_G_CHIP_NAME  _IOWR('V', 102, struct v4l2_dbg_chip_name)
+
 /* Reminder: when adding new ioctls please add support for them to
    drivers/media/video/v4l2-compat-ioctl32.c as well! */
 
-- 
1.7.10.4

Re: [RFC PATCH 2/6] v4l2: add new VIDIOC_DBG_G_CHIP_NAME ioctl.

[Laurent Pinchart]

Hi Hans,

On Monday 18 March 2013 17:38:16 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Simplify the debugging ioctls by creating the VIDIOC_DBG_G_CHIP_NAME ioctl.
> This will eventually replace VIDIOC_DBG_G_CHIP_IDENT. Chip matching is done
> by the name or index of subdevices or an index to a bridge chip. Most of
> this can all be done automatically, so most drivers just need to provide
> get/set register ops.
> 
> In particular, it is now possible to get/set subdev registers without
> requiring assistance of the bridge driver.

My biggest question is why don't we use the media controller API to get the 
information provided by this new ioctl ?

> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c |    5 +-
>  drivers/media/v4l2-core/v4l2-dev.c    |    5 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c  |  115 ++++++++++++++++++++++++++++--
>  include/media/v4l2-ioctl.h            |    3 +
>  include/uapi/linux/videodev2.h        |   34 +++++++---
>  5 files changed, 146 insertions(+), 16 deletions(-)

[snip]

> diff --git a/drivers/media/v4l2-core/v4l2-dev.c
> b/drivers/media/v4l2-core/v4l2-dev.c index de1e9ab..c0c651d 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -591,9 +591,10 @@ static void determine_valid_ioctls(struct video_device

[snip]

> +static int v4l_dbg_g_chip_name(const struct v4l2_ioctl_ops *ops,
> +				struct file *file, void *fh, void *arg)

As this is a debug ioctl that should never be used in application, I would 
like to guard the whole implementation with #ifdef CONFIG_VIDEO_ADV_DEBUG. 
This will make sure that no applications will abuse it, as it won't be 
available in distro kernels.

> +{
> +	struct video_device *vfd = video_devdata(file);
> +	struct v4l2_dbg_chip_name *p = arg;
> +	struct v4l2_subdev *sd;
> +	int idx = 0;
> +
> +	switch (p->match.type) {
> +	case V4L2_CHIP_MATCH_BRIDGE:
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +		if (ops->vidioc_s_register)
> +			p->flags |= V4L2_CHIP_FL_WRITABLE;
> +		if (ops->vidioc_g_register)
> +			p->flags |= V4L2_CHIP_FL_READABLE;
> +#endif
> +		if (ops->vidioc_g_chip_name)
> +			return ops->vidioc_g_chip_name(file, fh, arg);
> +		if (p->match.addr)
> +			return -EINVAL;
> +		if (vfd->v4l2_dev)
> +			strlcpy(p->name, vfd->v4l2_dev->name, sizeof(p->name));
> +		else if (vfd->parent)
> +			strlcpy(p->name, vfd->parent->driver->name, sizeof(p->name));
> +		else
> +			strlcpy(p->name, "bridge", sizeof(p->name));
> +		return 0;
> +
> +	case V4L2_CHIP_MATCH_SUBDEV_IDX:
> +	case V4L2_CHIP_MATCH_SUBDEV_NAME:
> +		if (vfd->v4l2_dev == NULL)
> +			break;
> +		v4l2_device_for_each_subdev(sd, vfd->v4l2_dev) {
> +			if (v4l_dbg_found_match(&p->match, sd, idx++)) {
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +				if (sd->ops->core && sd->ops->core->s_register)
> +					p->flags |= V4L2_CHIP_FL_WRITABLE;
> +				if (sd->ops->core && sd->ops->core->g_register)
> +					p->flags |= V4L2_CHIP_FL_READABLE;
> +#endif
> +				strlcpy(p->name, sd->name, sizeof(p->name));
> +				return 0;
> +			}
> +		}
> +		break;
> +	}
> +	return -EINVAL;
> +}

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 2/6] v4l2: add new VIDIOC_DBG_G_CHIP_NAME ioctl.

[Hans Verkuil]

On Wed March 27 2013 02:11:23 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Monday 18 March 2013 17:38:16 Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > Simplify the debugging ioctls by creating the VIDIOC_DBG_G_CHIP_NAME ioctl.
> > This will eventually replace VIDIOC_DBG_G_CHIP_IDENT. Chip matching is done
> > by the name or index of subdevices or an index to a bridge chip. Most of
> > this can all be done automatically, so most drivers just need to provide
> > get/set register ops.
> > 
> > In particular, it is now possible to get/set subdev registers without
> > requiring assistance of the bridge driver.
> 
> My biggest question is why don't we use the media controller API to get the 
> information provided by this new ioctl ?

Because the media controller is implemented by only a handful of drivers,
and this debug API is used by many more drivers. So I don't really see how
this would be feasible today.

> 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-common.c |    5 +-
> >  drivers/media/v4l2-core/v4l2-dev.c    |    5 +-
> >  drivers/media/v4l2-core/v4l2-ioctl.c  |  115 ++++++++++++++++++++++++++++--
> >  include/media/v4l2-ioctl.h            |    3 +
> >  include/uapi/linux/videodev2.h        |   34 +++++++---
> >  5 files changed, 146 insertions(+), 16 deletions(-)
> 
> [snip]
> 
> > diff --git a/drivers/media/v4l2-core/v4l2-dev.c
> > b/drivers/media/v4l2-core/v4l2-dev.c index de1e9ab..c0c651d 100644
> > --- a/drivers/media/v4l2-core/v4l2-dev.c
> > +++ b/drivers/media/v4l2-core/v4l2-dev.c
> > @@ -591,9 +591,10 @@ static void determine_valid_ioctls(struct video_device
> 
> [snip]
> 
> > +static int v4l_dbg_g_chip_name(const struct v4l2_ioctl_ops *ops,
> > +				struct file *file, void *fh, void *arg)
> 
> As this is a debug ioctl that should never be used in application, I would 
> like to guard the whole implementation with #ifdef CONFIG_VIDEO_ADV_DEBUG. 
> This will make sure that no applications will abuse it, as it won't be 
> available in distro kernels.

Agreed. I'll make that change. Actually, it's not so much userspace abuse I
am worried about, but kernel space abuse. I've found several drivers where
the bridge driver calls g_chip_ident to get information about subdevice
drivers. That was never intended and it complicates my work of removing
g_chip_ident. By putting chip_name under ADV_DEBUG we can avoid similar abuse
in the future.

Regards,

	Hans

> 
> > +{
> > +	struct video_device *vfd = video_devdata(file);
> > +	struct v4l2_dbg_chip_name *p = arg;
> > +	struct v4l2_subdev *sd;
> > +	int idx = 0;
> > +
> > +	switch (p->match.type) {
> > +	case V4L2_CHIP_MATCH_BRIDGE:
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > +		if (ops->vidioc_s_register)
> > +			p->flags |= V4L2_CHIP_FL_WRITABLE;
> > +		if (ops->vidioc_g_register)
> > +			p->flags |= V4L2_CHIP_FL_READABLE;
> > +#endif
> > +		if (ops->vidioc_g_chip_name)
> > +			return ops->vidioc_g_chip_name(file, fh, arg);
> > +		if (p->match.addr)
> > +			return -EINVAL;
> > +		if (vfd->v4l2_dev)
> > +			strlcpy(p->name, vfd->v4l2_dev->name, sizeof(p->name));
> > +		else if (vfd->parent)
> > +			strlcpy(p->name, vfd->parent->driver->name, sizeof(p->name));
> > +		else
> > +			strlcpy(p->name, "bridge", sizeof(p->name));
> > +		return 0;
> > +
> > +	case V4L2_CHIP_MATCH_SUBDEV_IDX:
> > +	case V4L2_CHIP_MATCH_SUBDEV_NAME:
> > +		if (vfd->v4l2_dev == NULL)
> > +			break;
> > +		v4l2_device_for_each_subdev(sd, vfd->v4l2_dev) {
> > +			if (v4l_dbg_found_match(&p->match, sd, idx++)) {
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > +				if (sd->ops->core && sd->ops->core->s_register)
> > +					p->flags |= V4L2_CHIP_FL_WRITABLE;
> > +				if (sd->ops->core && sd->ops->core->g_register)
> > +					p->flags |= V4L2_CHIP_FL_READABLE;
> > +#endif
> > +				strlcpy(p->name, sd->name, sizeof(p->name));
> > +				return 0;
> > +			}
> > +		}
> > +		break;
> > +	}
> > +	return -EINVAL;
> > +}
> 
> 

Re: [RFC PATCH 2/6] v4l2: add new VIDIOC_DBG_G_CHIP_NAME ioctl.

[Laurent Pinchart]

Hi Hans,

On Wednesday 27 March 2013 09:41:33 Hans Verkuil wrote:
> On Wed March 27 2013 02:11:23 Laurent Pinchart wrote:
> > On Monday 18 March 2013 17:38:16 Hans Verkuil wrote:
> > > From: Hans Verkuil <hans.verkuil@cisco.com>
> > > 
> > > Simplify the debugging ioctls by creating the VIDIOC_DBG_G_CHIP_NAME
> > > ioctl. This will eventually replace VIDIOC_DBG_G_CHIP_IDENT. Chip
> > > matching is done by the name or index of subdevices or an index to a
> > > bridge chip. Most of this can all be done automatically, so most drivers
> > > just need to provide get/set register ops.
> > > 
> > > In particular, it is now possible to get/set subdev registers without
> > > requiring assistance of the bridge driver.
> > 
> > My biggest question is why don't we use the media controller API to get
> > the information provided by this new ioctl ?
> 
> Because the media controller is implemented by only a handful of drivers,
> and this debug API is used by many more drivers.

Shouldn't we then fix that instead of adding a new ioctl ?

> So I don't really see how this would be feasible today.
> 
> > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > > ---
> > > 
> > >  drivers/media/v4l2-core/v4l2-common.c |    5 +-
> > >  drivers/media/v4l2-core/v4l2-dev.c    |    5 +-
> > >  drivers/media/v4l2-core/v4l2-ioctl.c  |  115 ++++++++++++++++++++++++--
> > >  include/media/v4l2-ioctl.h            |    3 +
> > >  include/uapi/linux/videodev2.h        |   34 +++++++---
> > >  5 files changed, 146 insertions(+), 16 deletions(-)
> > 
> > [snip]
> > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-dev.c
> > > b/drivers/media/v4l2-core/v4l2-dev.c index de1e9ab..c0c651d 100644
> > > --- a/drivers/media/v4l2-core/v4l2-dev.c
> > > +++ b/drivers/media/v4l2-core/v4l2-dev.c
> > > @@ -591,9 +591,10 @@ static void determine_valid_ioctls(struct
> > > video_device
> > 
> > [snip]
> > 
> > > +static int v4l_dbg_g_chip_name(const struct v4l2_ioctl_ops *ops,
> > > +				struct file *file, void *fh, void *arg)
> > 
> > As this is a debug ioctl that should never be used in application, I would
> > like to guard the whole implementation with #ifdef CONFIG_VIDEO_ADV_DEBUG.
> > This will make sure that no applications will abuse it, as it won't be
> > available in distro kernels.
> 
> Agreed. I'll make that change. Actually, it's not so much userspace abuse I
> am worried about, but kernel space abuse. I've found several drivers where
> the bridge driver calls g_chip_ident to get information about subdevice
> drivers. That was never intended and it complicates my work of removing
> g_chip_ident. By putting chip_name under ADV_DEBUG we can avoid similar
> abuse in the future.

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 2/6] v4l2: add new VIDIOC_DBG_G_CHIP_NAME ioctl.

[Hans Verkuil]

On Wed March 27 2013 11:11:53 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Wednesday 27 March 2013 09:41:33 Hans Verkuil wrote:
> > On Wed March 27 2013 02:11:23 Laurent Pinchart wrote:
> > > On Monday 18 March 2013 17:38:16 Hans Verkuil wrote:
> > > > From: Hans Verkuil <hans.verkuil@cisco.com>
> > > > 
> > > > Simplify the debugging ioctls by creating the VIDIOC_DBG_G_CHIP_NAME
> > > > ioctl. This will eventually replace VIDIOC_DBG_G_CHIP_IDENT. Chip
> > > > matching is done by the name or index of subdevices or an index to a
> > > > bridge chip. Most of this can all be done automatically, so most drivers
> > > > just need to provide get/set register ops.
> > > > 
> > > > In particular, it is now possible to get/set subdev registers without
> > > > requiring assistance of the bridge driver.
> > > 
> > > My biggest question is why don't we use the media controller API to get
> > > the information provided by this new ioctl ?
> > 
> > Because the media controller is implemented by only a handful of drivers,
> > and this debug API is used by many more drivers.
> 
> Shouldn't we then fix that instead of adding a new ioctl ?

Mauro was opposed to making the MC available for all drivers. So besides the
technical issues which would take a lot of time (which I don't have), there is
also a whole discussion about whether or not the MC should be there at all for
'simple' drivers.

My main goal at the moment is to make this API more powerful and simplify the
drivers. It is also my intention to get rid of G_CHIP_IDENT as soon as possible.

Should we get the MC available for all drivers in the future, then it should be
quite easy to adapt the code to the MC once G_CHIP_IDENT has been removed.
Consider this a first step into the right direction.

Regards,

	Hans

[RFC PATCH 3/6] stk1160: remove V4L2_CHIP_MATCH_AC97 placeholder.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

It was just a placeholder and we want to get rid of the AC97 matching
define.

Also replace MATCH_HOST with MATCH_BRIDGE since we are here anyway.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/stk1160/stk1160-v4l.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c
index 5307a63..ef0ca2d 100644
--- a/drivers/media/usb/stk1160/stk1160-v4l.c
+++ b/drivers/media/usb/stk1160/stk1160-v4l.c
@@ -458,7 +458,7 @@ static int vidioc_g_chip_ident(struct file *file, void *priv,
 	       struct v4l2_dbg_chip_ident *chip)
 {
 	switch (chip->match.type) {
-	case V4L2_CHIP_MATCH_HOST:
+	case V4L2_CHIP_MATCH_BRIDGE:
 		chip->ident = V4L2_IDENT_NONE;
 		chip->revision = 0;
 		return 0;
@@ -476,9 +476,6 @@ static int vidioc_g_register(struct file *file, void *priv,
 	u8 val;
 
 	switch (reg->match.type) {
-	case V4L2_CHIP_MATCH_AC97:
-		/* TODO: Support me please :-( */
-		return -EINVAL;
 	case V4L2_CHIP_MATCH_I2C_DRIVER:
 		v4l2_device_call_all(&dev->v4l2_dev, 0, core, g_register, reg);
 		return 0;
@@ -505,8 +502,6 @@ static int vidioc_s_register(struct file *file, void *priv,
 	struct stk1160 *dev = video_drvdata(file);
 
 	switch (reg->match.type) {
-	case V4L2_CHIP_MATCH_AC97:
-		return -EINVAL;
 	case V4L2_CHIP_MATCH_I2C_DRIVER:
 		v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_register, reg);
 		return 0;
-- 
1.7.10.4

[RFC PATCH 4/6] em28xx: add support for g_chip_name.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Add support for vidioc_g_chip_name, allowing AC97 to be implemented as a
second chip on the bridge with the name "ac97". v4l2-dbg can just match the
name with that string in order to detect a ac97-compliant set of registers.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |   41 +++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 93fc620..99fcd50 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1242,9 +1242,9 @@ static int vidioc_g_chip_ident(struct file *file, void *priv,
 
 	chip->ident = V4L2_IDENT_NONE;
 	chip->revision = 0;
-	if (chip->match.type == V4L2_CHIP_MATCH_HOST) {
-		if (v4l2_chip_match_host(&chip->match))
-			chip->ident = V4L2_IDENT_NONE;
+	if (chip->match.type == V4L2_CHIP_MATCH_BRIDGE) {
+		if (chip->match.addr > 1)
+			return -EINVAL;
 		return 0;
 	}
 	if (chip->match.type != V4L2_CHIP_MATCH_I2C_DRIVER &&
@@ -1256,6 +1256,21 @@ static int vidioc_g_chip_ident(struct file *file, void *priv,
 	return 0;
 }
 
+static int vidioc_g_chip_name(struct file *file, void *priv,
+	       struct v4l2_dbg_chip_name *chip)
+{
+	struct em28xx_fh      *fh  = priv;
+	struct em28xx         *dev = fh->dev;
+
+	if (chip->match.addr > 1)
+		return -EINVAL;
+	if (chip->match.addr == 1)
+		strlcpy(chip->name, "ac97", sizeof(chip->name));
+	else
+		strlcpy(chip->name, dev->v4l2_dev.name, sizeof(chip->name));
+	return 0;
+}
+
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 static int em28xx_reg_len(int reg)
 {
@@ -1277,6 +1292,12 @@ static int vidioc_g_register(struct file *file, void *priv,
 	int ret;
 
 	switch (reg->match.type) {
+	case V4L2_CHIP_MATCH_BRIDGE:
+		if (reg->match.addr > 1)
+			return -EINVAL;
+		if (!reg->match.addr)
+			break;
+		/* fall-through */
 	case V4L2_CHIP_MATCH_AC97:
 		ret = em28xx_read_ac97(dev, reg->reg);
 		if (ret < 0)
@@ -1293,8 +1314,7 @@ static int vidioc_g_register(struct file *file, void *priv,
 		v4l2_device_call_all(&dev->v4l2_dev, 0, core, g_register, reg);
 		return 0;
 	default:
-		if (!v4l2_chip_match_host(&reg->match))
-			return -EINVAL;
+		return -EINVAL;
 	}
 
 	/* Match host */
@@ -1327,6 +1347,12 @@ static int vidioc_s_register(struct file *file, void *priv,
 	__le16 buf;
 
 	switch (reg->match.type) {
+	case V4L2_CHIP_MATCH_BRIDGE:
+		if (reg->match.addr > 1)
+			return -EINVAL;
+		if (!reg->match.addr)
+			break;
+		/* fall-through */
 	case V4L2_CHIP_MATCH_AC97:
 		return em28xx_write_ac97(dev, reg->reg, reg->val);
 	case V4L2_CHIP_MATCH_I2C_DRIVER:
@@ -1337,8 +1363,7 @@ static int vidioc_s_register(struct file *file, void *priv,
 		v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_register, reg);
 		return 0;
 	default:
-		if (!v4l2_chip_match_host(&reg->match))
-			return -EINVAL;
+		return -EINVAL;
 	}
 
 	/* Match host */
@@ -1696,6 +1721,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 	.vidioc_g_chip_ident        = vidioc_g_chip_ident,
+	.vidioc_g_chip_name         = vidioc_g_chip_name,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.vidioc_g_register          = vidioc_g_register,
 	.vidioc_s_register          = vidioc_s_register,
@@ -1726,6 +1752,7 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
 	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 	.vidioc_g_chip_ident  = vidioc_g_chip_ident,
+	.vidioc_g_chip_name   = vidioc_g_chip_name,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.vidioc_g_register    = vidioc_g_register,
 	.vidioc_s_register    = vidioc_s_register,
-- 
1.7.10.4

[RFC PATCH 5/6] DocBook media: fix syntax problems in dvbproperty.xml

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Caught by xmllint.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/dvb/dvbproperty.xml |   46 +++++++++++------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/Documentation/DocBook/media/dvb/dvbproperty.xml b/Documentation/DocBook/media/dvb/dvbproperty.xml
index 4a5eaee..341b3f0 100644
--- a/Documentation/DocBook/media/dvb/dvbproperty.xml
+++ b/Documentation/DocBook/media/dvb/dvbproperty.xml
@@ -903,14 +903,12 @@ enum fe_interleaving {
 			<constant>svalue</constant> is for signed values of the measure (dB measures)
 			and <constant>uvalue</constant> is for unsigned values (counters, relative scale)</para></listitem>
 		<listitem><para><constant>scale</constant> - Scale for the value. It can be:</para>
-			<section id = "fecap-scale-params">
-			<itemizedlist mark='bullet'>
+			<itemizedlist mark='bullet' id="fecap-scale-params">
 				<listitem><para><constant>FE_SCALE_NOT_AVAILABLE</constant> - The parameter is supported by the frontend, but it was not possible to collect it (could be a transitory or permanent condition)</para></listitem>
 				<listitem><para><constant>FE_SCALE_DECIBEL</constant> - parameter is a signed value, measured in 1/1000 dB</para></listitem>
 				<listitem><para><constant>FE_SCALE_RELATIVE</constant> - parameter is a unsigned value, where 0 means 0% and 65535 means 100%.</para></listitem>
 				<listitem><para><constant>FE_SCALE_COUNTER</constant> - parameter is a unsigned value that counts the occurrence of an event, like bit error, block error, or lapsed time.</para></listitem>
 			</itemizedlist>
-			</section>
 		</listitem>
 	</itemizedlist>
 	<section id="DTV-STAT-SIGNAL-STRENGTH">
@@ -918,9 +916,9 @@ enum fe_interleaving {
 		<para>Indicates the signal strength level at the analog part of the tuner or of the demod.</para>
 		<para>Possible scales for this metric are:</para>
 		<itemizedlist mark='bullet'>
-			<listitem><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</listitem>
-			<listitem><constant>FE_SCALE_DECIBEL</constant> - signal strength is in 0.0001 dBm units, power measured in miliwatts. This value is generally negative.</listitem>
-			<listitem><constant>FE_SCALE_RELATIVE</constant> - The frontend provides a 0% to 100% measurement for power (actually, 0 to 65535).</listitem>
+			<listitem><para><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</para></listitem>
+			<listitem><para><constant>FE_SCALE_DECIBEL</constant> - signal strength is in 0.0001 dBm units, power measured in miliwatts. This value is generally negative.</para></listitem>
+			<listitem><para><constant>FE_SCALE_RELATIVE</constant> - The frontend provides a 0% to 100% measurement for power (actually, 0 to 65535).</para></listitem>
 		</itemizedlist>
 	</section>
 	<section id="DTV-STAT-CNR">
@@ -928,9 +926,9 @@ enum fe_interleaving {
 		<para>Indicates the Signal to Noise ratio for the main carrier.</para>
 		<para>Possible scales for this metric are:</para>
 		<itemizedlist mark='bullet'>
-			<listitem><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</listitem>
-			<listitem><constant>FE_SCALE_DECIBEL</constant> - Signal/Noise ratio is in 0.0001 dB units.</listitem>
-			<listitem><constant>FE_SCALE_RELATIVE</constant> - The frontend provides a 0% to 100% measurement for Signal/Noise (actually, 0 to 65535).</listitem>
+			<listitem><para><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</para></listitem>
+			<listitem><para><constant>FE_SCALE_DECIBEL</constant> - Signal/Noise ratio is in 0.0001 dB units.</para></listitem>
+			<listitem><para><constant>FE_SCALE_RELATIVE</constant> - The frontend provides a 0% to 100% measurement for Signal/Noise (actually, 0 to 65535).</para></listitem>
 		</itemizedlist>
 	</section>
 	<section id="DTV-STAT-PRE-ERROR-BIT-COUNT">
@@ -943,8 +941,8 @@ enum fe_interleaving {
 		      The frontend may reset it when a channel/transponder is tuned.</para>
 		<para>Possible scales for this metric are:</para>
 		<itemizedlist mark='bullet'>
-			<listitem><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</listitem>
-			<listitem><constant>FE_SCALE_COUNTER</constant> - Number of error bits counted before the inner coding.</listitem>
+			<listitem><para><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</para></listitem>
+			<listitem><para><constant>FE_SCALE_COUNTER</constant> - Number of error bits counted before the inner coding.</para></listitem>
 		</itemizedlist>
 	</section>
 	<section id="DTV-STAT-PRE-TOTAL-BIT-COUNT">
@@ -957,9 +955,9 @@ enum fe_interleaving {
 		      The frontend may reset it when a channel/transponder is tuned.</para>
 		<para>Possible scales for this metric are:</para>
 		<itemizedlist mark='bullet'>
-			<listitem><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</listitem>
-			<listitem><constant>FE_SCALE_COUNTER</constant> - Number of bits counted while measuring
-				 <link linkend="DTV-STAT-PRE-ERROR-BIT-COUNT"><constant>DTV_STAT_PRE_ERROR_BIT_COUNT</constant></link>.</listitem>
+			<listitem><para><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</para></listitem>
+			<listitem><para><constant>FE_SCALE_COUNTER</constant> - Number of bits counted while measuring
+				 <link linkend="DTV-STAT-PRE-ERROR-BIT-COUNT"><constant>DTV_STAT_PRE_ERROR_BIT_COUNT</constant></link>.</para></listitem>
 		</itemizedlist>
 	</section>
 	<section id="DTV-STAT-POST-ERROR-BIT-COUNT">
@@ -972,8 +970,8 @@ enum fe_interleaving {
 		      The frontend may reset it when a channel/transponder is tuned.</para>
 		<para>Possible scales for this metric are:</para>
 		<itemizedlist mark='bullet'>
-			<listitem><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</listitem>
-			<listitem><constant>FE_SCALE_COUNTER</constant> - Number of error bits counted after the inner coding.</listitem>
+			<listitem><para><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</para></listitem>
+			<listitem><para><constant>FE_SCALE_COUNTER</constant> - Number of error bits counted after the inner coding.</para></listitem>
 		</itemizedlist>
 	</section>
 	<section id="DTV-STAT-POST-TOTAL-BIT-COUNT">
@@ -986,9 +984,9 @@ enum fe_interleaving {
 		      The frontend may reset it when a channel/transponder is tuned.</para>
 		<para>Possible scales for this metric are:</para>
 		<itemizedlist mark='bullet'>
-			<listitem><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</listitem>
-			<listitem><constant>FE_SCALE_COUNTER</constant> - Number of bits counted while measuring
-				 <link linkend="DTV-STAT-POST-ERROR-BIT-COUNT"><constant>DTV_STAT_POST_ERROR_BIT_COUNT</constant></link>.</listitem>
+			<listitem><para><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</para></listitem>
+			<listitem><para><constant>FE_SCALE_COUNTER</constant> - Number of bits counted while measuring
+				 <link linkend="DTV-STAT-POST-ERROR-BIT-COUNT"><constant>DTV_STAT_POST_ERROR_BIT_COUNT</constant></link>.</para></listitem>
 		</itemizedlist>
 	</section>
 	<section id="DTV-STAT-ERROR-BLOCK-COUNT">
@@ -998,8 +996,8 @@ enum fe_interleaving {
 		      The frontend may reset it when a channel/transponder is tuned.</para>
 		<para>Possible scales for this metric are:</para>
 		<itemizedlist mark='bullet'>
-			<listitem><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</listitem>
-			<listitem><constant>FE_SCALE_COUNTER</constant> - Number of error blocks counted after the outer coding.</listitem>
+			<listitem><para><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</para></listitem>
+			<listitem><para><constant>FE_SCALE_COUNTER</constant> - Number of error blocks counted after the outer coding.</para></listitem>
 		</itemizedlist>
 	</section>
 	<section id="DTV-STAT-TOTAL-BLOCK-COUNT">
@@ -1011,9 +1009,9 @@ enum fe_interleaving {
 		by <link linkend="DTV-STAT-TOTAL-BLOCK-COUNT"><constant>DTV-STAT-TOTAL-BLOCK-COUNT</constant></link>.</para>
 		<para>Possible scales for this metric are:</para>
 		<itemizedlist mark='bullet'>
-			<listitem><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</listitem>
-			<listitem><constant>FE_SCALE_COUNTER</constant> - Number of blocks counted while measuring
-			<link linkend="DTV-STAT-ERROR-BLOCK-COUNT"><constant>DTV_STAT_ERROR_BLOCK_COUNT</constant></link>.</listitem>
+			<listitem><para><constant>FE_SCALE_NOT_AVAILABLE</constant> - it failed to measure it, or the measurement was not complete yet.</para></listitem>
+			<listitem><para><constant>FE_SCALE_COUNTER</constant> - Number of blocks counted while measuring
+			<link linkend="DTV-STAT-ERROR-BLOCK-COUNT"><constant>DTV_STAT_ERROR_BLOCK_COUNT</constant></link>.</para></listitem>
 		</itemizedlist>
 	</section>
 	</section>
-- 
1.7.10.4

[RFC PATCH 6/6] DocBook media: add VIDIOC_DBG_G_CHIP_NAME documentation

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

And update the other debug ioctls accordingly.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/v4l2.xml           |    4 +-
 .../DocBook/media/v4l/vidioc-dbg-g-chip-ident.xml  |   14 +-
 .../DocBook/media/v4l/vidioc-dbg-g-chip-name.xml   |  234 ++++++++++++++++++++
 .../DocBook/media/v4l/vidioc-dbg-g-register.xml    |   42 +++-
 4 files changed, 280 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-dbg-g-chip-name.xml

diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index a3cce18..a3c7adb7 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -144,8 +144,7 @@ applications. -->
 	<date>2012-12-03</date>
 	<authorinitials>sa, sn</authorinitials>
 	<revremark>Added timestamp types to v4l2_buffer.
-	Added <constant>V4L2_EVENT_CTRL_CH_RANGE</constant> control
-	event changes flag, see <xref linkend="changes-flags"/>.
+	Added V4L2_EVENT_CTRL_CH_RANGE control event changes flag.
 	</revremark>
       </revision>
 
@@ -537,6 +536,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-create-bufs;
     &sub-cropcap;
     &sub-dbg-g-chip-ident;
+    &sub-dbg-g-chip-name;
     &sub-dbg-g-register;
     &sub-decoder-cmd;
     &sub-dqevent;
diff --git a/Documentation/DocBook/media/v4l/vidioc-dbg-g-chip-ident.xml b/Documentation/DocBook/media/v4l/vidioc-dbg-g-chip-ident.xml
index 4ecd966..82e43c6 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dbg-g-chip-ident.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dbg-g-chip-ident.xml
@@ -200,10 +200,10 @@ the values from <xref linkend="chip-ids" />.</entry>
 	&cs-def;
 	<tbody valign="top">
 	  <row>
-	    <entry><constant>V4L2_CHIP_MATCH_HOST</constant></entry>
+	    <entry><constant>V4L2_CHIP_MATCH_BRIDGE</constant></entry>
 	    <entry>0</entry>
 	    <entry>Match the nth chip on the card, zero for the
-	    host chip. Does not match &i2c; chips.</entry>
+	    bridge chip. Does not match sub-devices.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_CHIP_MATCH_I2C_DRIVER</constant></entry>
@@ -220,6 +220,16 @@ the values from <xref linkend="chip-ids" />.</entry>
 	    <entry>3</entry>
 	    <entry>Match the nth anciliary AC97 chip.</entry>
 	  </row>
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_SUBDEV_NAME</constant></entry>
+	    <entry>4</entry>
+	    <entry>Match the sub-device by name. Can't be used with this ioctl.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_SUBDEV_IDX</constant></entry>
+	    <entry>5</entry>
+	    <entry>Match the nth sub-device. Can't be used with this ioctl.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/Documentation/DocBook/media/v4l/vidioc-dbg-g-chip-name.xml b/Documentation/DocBook/media/v4l/vidioc-dbg-g-chip-name.xml
new file mode 100644
index 0000000..4921346
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/vidioc-dbg-g-chip-name.xml
@@ -0,0 +1,234 @@
+<refentry id="vidioc-dbg-g-chip-name">
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_DBG_G_CHIP_NAME</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_DBG_G_CHIP_NAME</refname>
+    <refpurpose>Identify the chips on a TV card</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_dbg_chip_name
+*<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_DBG_G_CHIP_NAME</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <note>
+      <title>Experimental</title>
+
+      <para>This is an <link
+linkend="experimental">experimental</link> interface and may change in
+the future.</para>
+    </note>
+
+    <para>For driver debugging purposes this ioctl allows test
+applications to query the driver about the chips present on the TV
+card. Regular applications must not use it. When you found a chip
+specific bug, please contact the linux-media mailing list (&v4l-ml;)
+so it can be fixed.</para>
+
+    <para>To query the driver applications must initialize the
+<structfield>match.type</structfield> and
+<structfield>match.addr</structfield> or <structfield>match.name</structfield>
+fields of a &v4l2-dbg-chip-name;
+and call <constant>VIDIOC_DBG_G_CHIP_NAME</constant> with a pointer to
+this structure. On success the driver stores information about the
+selected chip in the <structfield>name</structfield> and
+<structfield>flags</structfield> fields. On failure the structure
+remains unchanged.</para>
+
+    <para>When <structfield>match.type</structfield> is
+<constant>V4L2_CHIP_MATCH_BRIDGE</constant>,
+<structfield>match.addr</structfield> selects the nth bridge 'chip'
+on the TV card. You can enumerate all chips by starting at zero and
+incrementing <structfield>match.addr</structfield> by one until
+<constant>VIDIOC_DBG_G_CHIP_NAME</constant> fails with an &EINVAL;.
+The number zero always selects the bridge chip itself, &eg; the chip
+connected to the PCI or USB bus. Non-zero numbers identify specific
+parts of the bridge chip such as an AC97 register block.</para>
+
+    <para>When <structfield>match.type</structfield> is
+<constant>V4L2_CHIP_MATCH_SUBDEV_NAME</constant>,
+<structfield>match.name</structfield> contains the name of a sub-device.
+For instance
+<constant>"saa7127 6-0044"</constant> will match the saa7127 sub-device
+at the given i2c bus. This match type is not very useful for this ioctl
+and is here only for consistency.
+</para>
+
+    <para>When <structfield>match.type</structfield> is
+<constant>V4L2_CHIP_MATCH_SUBDEV_IDX</constant>,
+<structfield>match.addr</structfield> selects the nth sub-device. This
+allows you to enumerate over all sub-devices.</para>
+
+    <para>On success, the <structfield>name</structfield> field will
+contain a chip name and the <structfield>flags</structfield> field will
+contain <constant>V4L2_CHIP_FL_READABLE</constant> if the driver supports
+reading registers from the device or <constant>V4L2_CHIP_FL_WRITABLE</constant>
+if the driver supports writing registers to the device.</para>
+
+    <para>We recommended the <application>v4l2-dbg</application>
+utility over calling this ioctl directly. It is available from the
+LinuxTV v4l-dvb repository; see <ulink
+url="http://linuxtv.org/repo/">http://linuxtv.org/repo/</ulink> for
+access instructions.</para>
+
+    <!-- Note for convenience vidioc-dbg-g-register.sgml
+	 contains a duplicate of this table. -->
+    <table pgwide="1" frame="none" id="name-v4l2-dbg-match">
+      <title>struct <structname>v4l2_dbg_match</structname></title>
+      <tgroup cols="4">
+	&cs-ustr;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>type</structfield></entry>
+	    <entry>See <xref linkend="name-chip-match-types" /> for a list of
+possible types.</entry>
+	  </row>
+	  <row>
+	    <entry>union</entry>
+	    <entry>(anonymous)</entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+	    <entry>__u32</entry>
+	    <entry><structfield>addr</structfield></entry>
+	    <entry>Match a chip by this number, interpreted according
+to the <structfield>type</structfield> field.</entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+	    <entry>char</entry>
+	    <entry><structfield>name[32]</structfield></entry>
+	    <entry>Match a chip by this name, interpreted according
+to the <structfield>type</structfield> field.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <table pgwide="1" frame="none" id="v4l2-dbg-chip-name">
+      <title>struct <structname>v4l2_dbg_chip_name</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>struct v4l2_dbg_match</entry>
+	    <entry><structfield>match</structfield></entry>
+	    <entry>How to match the chip, see <xref linkend="name-v4l2-dbg-match" />.</entry>
+	  </row>
+	  <row>
+	    <entry>char</entry>
+	    <entry><structfield>name[32]</structfield></entry>
+	    <entry>The name of the chip.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>flags</structfield></entry>
+	    <entry>Set by the driver. If <constant>V4L2_CHIP_FL_READABLE</constant>
+is set, then the driver supports reading registers from the device. If
+<constant>V4L2_CHIP_FL_WRITABLE</constant> is set, then it supports writing registers.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved[8]</structfield></entry>
+	    <entry>Reserved fields, both application and driver must set these to 0.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <!-- Note for convenience vidioc-dbg-g-register.sgml
+	 contains a duplicate of this table. -->
+    <table pgwide="1" frame="none" id="name-chip-match-types">
+      <title>Chip Match Types</title>
+      <tgroup cols="3">
+	&cs-def;
+	<tbody valign="top">
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_BRIDGE</constant></entry>
+	    <entry>0</entry>
+	    <entry>Match the nth chip on the card, zero for the
+	    bridge chip. Does not match sub-devices.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_I2C_DRIVER</constant></entry>
+	    <entry>1</entry>
+	    <entry>Match an &i2c; chip by its driver name. Can't be used with this ioctl.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_I2C_ADDR</constant></entry>
+	    <entry>2</entry>
+	    <entry>Match a chip by its 7 bit &i2c; bus address. Can't be used with this ioctl.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_AC97</constant></entry>
+	    <entry>3</entry>
+	    <entry>Match the nth anciliary AC97 chip. Can't be used with this ioctl.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_SUBDEV_NAME</constant></entry>
+	    <entry>4</entry>
+	    <entry>Match the sub-device by name.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_SUBDEV_IDX</constant></entry>
+	    <entry>5</entry>
+	    <entry>Match the nth sub-device.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+  </refsect1>
+
+  <refsect1>
+    &return-value;
+
+    <variablelist>
+      <varlistentry>
+	<term><errorcode>EINVAL</errorcode></term>
+	<listitem>
+	  <para>The <structfield>match_type</structfield> is invalid or
+no device could be matched.</para>
+	</listitem>
+      </varlistentry>
+     </variablelist>
+  </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/media/v4l/vidioc-dbg-g-register.xml b/Documentation/DocBook/media/v4l/vidioc-dbg-g-register.xml
index a44aebc..3082b41 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dbg-g-register.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dbg-g-register.xml
@@ -87,7 +87,7 @@ written into the register.</para>
 
     <para>To read a register applications must initialize the
 <structfield>match.type</structfield>,
-<structfield>match.chip</structfield> or <structfield>match.name</structfield> and
+<structfield>match.addr</structfield> or <structfield>match.name</structfield> and
 <structfield>reg</structfield> fields, and call
 <constant>VIDIOC_DBG_G_REGISTER</constant> with a pointer to this
 structure. On success the driver stores the register value in the
@@ -95,11 +95,11 @@ structure. On success the driver stores the register value in the
 unchanged.</para>
 
     <para>When <structfield>match.type</structfield> is
-<constant>V4L2_CHIP_MATCH_HOST</constant>,
-<structfield>match.addr</structfield> selects the nth non-&i2c; chip
+<constant>V4L2_CHIP_MATCH_BRIDGE</constant>,
+<structfield>match.addr</structfield> selects the nth non-sub-device chip
 on the TV card.  The number zero always selects the host chip, &eg; the
 chip connected to the PCI or USB bus. You can find out which chips are
-present with the &VIDIOC-DBG-G-CHIP-IDENT; ioctl.</para>
+present with the &VIDIOC-DBG-G-CHIP-NAME; ioctl.</para>
 
     <para>When <structfield>match.type</structfield> is
 <constant>V4L2_CHIP_MATCH_I2C_DRIVER</constant>,
@@ -109,7 +109,7 @@ For instance
 supported by the saa7127 driver, regardless of its &i2c; bus address.
 When multiple chips supported by the same driver are present, the
 effect of these ioctls is undefined. Again with the
-&VIDIOC-DBG-G-CHIP-IDENT; ioctl you can find out which &i2c; chips are
+&VIDIOC-DBG-G-CHIP-NAME; ioctl you can find out which &i2c; chips are
 present.</para>
 
     <para>When <structfield>match.type</structfield> is
@@ -122,19 +122,31 @@ bus address.</para>
 <structfield>match.addr</structfield> selects the nth AC97 chip
 on the TV card.</para>
 
+    <para>When <structfield>match.type</structfield> is
+<constant>V4L2_CHIP_MATCH_SUBDEV_NAME</constant>,
+<structfield>match.name</structfield> contains the sub-device name.
+For instance
+<constant>"saa7127 6-0044"</constant> will match this specific saa7127
+sub-device. Again with the &VIDIOC-DBG-G-CHIP-NAME; ioctl you can find
+out which sub-devices are present.</para>
+
+    <para>When <structfield>match.type</structfield> is
+<constant>V4L2_CHIP_MATCH_SUBDEV_IDX</constant>,
+<structfield>match.addr</structfield> selects the nth sub-device.</para>
+
     <note>
       <title>Success not guaranteed</title>
 
       <para>Due to a flaw in the Linux &i2c; bus driver these ioctls may
 return successfully without actually reading or writing a register. To
-catch the most likely failure we recommend a &VIDIOC-DBG-G-CHIP-IDENT;
+catch the most likely failure we recommend a &VIDIOC-DBG-G-CHIP-NAME;
 call confirming the presence of the selected &i2c; chip.</para>
     </note>
 
     <para>These ioctls are optional, not all drivers may support them.
 However when a driver supports these ioctls it must also support
-&VIDIOC-DBG-G-CHIP-IDENT;. Conversely it may support
-<constant>VIDIOC_DBG_G_CHIP_IDENT</constant> but not these ioctls.</para>
+&VIDIOC-DBG-G-CHIP-NAME;. Conversely it may support
+<constant>VIDIOC_DBG_G_CHIP_NAME</constant> but not these ioctls.</para>
 
     <para><constant>VIDIOC_DBG_G_REGISTER</constant> and
 <constant>VIDIOC_DBG_S_REGISTER</constant> were introduced in Linux
@@ -217,10 +229,10 @@ register.</entry>
 	&cs-def;
 	<tbody valign="top">
 	  <row>
-	    <entry><constant>V4L2_CHIP_MATCH_HOST</constant></entry>
+	    <entry><constant>V4L2_CHIP_MATCH_BRIDGE</constant></entry>
 	    <entry>0</entry>
 	    <entry>Match the nth chip on the card, zero for the
-	    host chip. Does not match &i2c; chips.</entry>
+	    bridge chip. Does not match sub-devices.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_CHIP_MATCH_I2C_DRIVER</constant></entry>
@@ -237,6 +249,16 @@ register.</entry>
 	    <entry>3</entry>
 	    <entry>Match the nth anciliary AC97 chip.</entry>
 	  </row>
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_SUBDEV_NAME</constant></entry>
+	    <entry>4</entry>
+	    <entry>Match the sub-device by name.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_CHIP_MATCH_SUBDEV_IDX</constant></entry>
+	    <entry>5</entry>
+	    <entry>Match the nth sub-device.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
-- 
1.7.10.4