v4l2-compat-ioctl32.c: add missing controls to, ctrl_is_pointer

PATCH New Hans Verkuil 3 participants 18 messages Aug 4, 2017 – Aug 12, 2017 Media (V4L/DVB)

lore

Share & tools

lore

Hans Verkuil

author

2017-08-04 11:25 UTC #0 lore raw

PATCH 1/1 v4l2-compat-ioctl32.c: add missing controls to, ctrl_is_pointer

We need to find a better method for this. But for now just add the missing pointer controls to this list.

Also properly mask the id. The high flag bits shouldn't be used with these ioctls, but it certainly doesn't hurt.

Signed-off-by Hans Verkuil <[email protected]>

---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/media/v4l2-core/v4l2-compat-ioctl32.c +6 -1

index 90827073066f..afae914b8099 100644
@@ -674,9 +674,14 @@ struct v4l2_ext_control32 {
    type STRING is a pointer type. */
 static inline int ctrl_is_pointer(u32 id)
 {
-	switch (id) {
+	switch (id & V4L2_CTRL_ID_MASK) {
 	case V4L2_CID_RDS_TX_PS_NAME:
 	case V4L2_CID_RDS_TX_RADIO_TEXT:
+	case V4L2_CID_RDS_RX_PS_NAME:
+	case V4L2_CID_RDS_RX_RADIO_TEXT:
+	case V4L2_CID_DETECT_MD_REGION_GRID:
+	case V4L2_CID_DETECT_MD_THRESHOLD_GRID:
+	case V4L2_CID_RDS_TX_ALT_FREQS:
 		return 1;
 	default:
 		return 0;

Signature · 1 line

2.13.2

Mauro Carvalho Chehab

2017-08-10 23:45 UTC #1 lore raw

cover v4l2-compat-ioctl32.c: better detect pointer controls

In the past, only string controls were pointers. That changed when compounded types got added, but the compat32 code was not updated.

We could just add those controls there, but maintaining it is flaw, as we often forget about the compat code. So, instead, rely on the control type, as this is always updated when new controls are added.

As both v4l2-ctrl and compat32 code are at videodev.ko module, we can move the ctrl_is_pointer() helper function to v4l2-ctrl.c.

Mauro Carvalho Chehab (3):
media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill
media: v4l2-ctrls: prepare the function to be used by compat32 code
media: compat32: reimplement ctrl_is_pointer()

 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 18 +---------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 49 +++++++++++++++++++++++++--
 include/media/v4l2-ctrls.h                    | 28 ++++++++++-----
 3 files changed, 67 insertions(+), 28 deletions(-)

Signature · 1 line

2.13.3

Mauro Carvalho Chehab

2017-08-10 23:45 UTC #2 lore raw

PATCH 1/3 media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill

The arguments for this function are pointers. Make it clear at its documentation.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-ctrls.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

include/media/v4l2-ctrls.h +9 -9

index 2d2aed56922f..6ba30acf06aa 100644
@@ -339,18 +339,18 @@ struct v4l2_ctrl_config {
 /**
  * v4l2_ctrl_fill - Fill in the control fields based on the control ID.
  *
- * @id: ID of the control
- * @name: name of the control
- * @type: type of the control
- * @min: minimum value for the control
- * @max: maximum value for the control
- * @step: control step
- * @def: default value for the control
- * @flags: flags to be used on the control
+ * @id: pointer for storing the ID of the control
+ * @name: pointer for storing the name of the control
+ * @type: pointer for storing the type of the control
+ * @min: pointer for storing the minimum value for the control
+ * @max: pointer for storing the maximum value for the control
+ * @step: pointer for storing the control step
+ * @def: pointer for storing the default value for the control
+ * @flags: pointer for storing the flags to be used on the control
  *
  * This works for all standard V4L2 controls.
  * For non-standard controls it will only fill in the given arguments
- * and @name will be %NULL.
+ * and @name content will be filled with %NULL.
  *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on

Signature · 1 line

2.13.3

Mauro Carvalho Chehab

2017-08-10 23:45 UTC #3 lore raw

PATCH 2/3 media: v4l2-ctrls: prepare the function to be used by compat32 code

In reply to Mauro Carvalho Chehab

Right now, both v4l2_ctrl_fill() and compat32 code need to know the type of the control. As new controls are added, this cause troubles at compat32, as it won't be able to discover what functions are pointers or not.

Change v4l2_ctrl_fill() function for it to be called with just one argument: the control type. This way, the compat32 code can use it internally.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/v4l2-core/v4l2-ctrls.c | 31 +++++++++++++++++++++++++++++--
 include/media/v4l2-ctrls.h           |  2 ++
 2 files changed, 31 insertions(+), 2 deletions(-)

drivers/media/v4l2-core/v4l2-ctrls.c +29 -2

index dd1db678718c..c512b7539077 100644
@@ -939,8 +939,10 @@ EXPORT_SYMBOL(v4l2_ctrl_get_name);
 void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		    s64 *min, s64 *max, u64 *step, s64 *def, u32 *flags)
 {
-	*name = v4l2_ctrl_get_name(id);
-	*flags = 0;
+	if (name) {
+		*name = v4l2_ctrl_get_name(id);
+		*flags = 0;
+	}
 
 	switch (id) {
 	case V4L2_CID_AUDIO_MUTE:

@@ -996,11 +998,15 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_RDS_RX_TRAFFIC_PROGRAM:
 	case V4L2_CID_RDS_RX_MUSIC_SPEECH:
 		*type = V4L2_CTRL_TYPE_BOOLEAN;
+		if (!name)
+			break;
 		*min = 0;
 		*max = *step = 1;
 		break;
 	case V4L2_CID_ROTATE:
 		*type = V4L2_CTRL_TYPE_INTEGER;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
 		break;
 	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:

@@ -1015,6 +1021,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_AUTO_FOCUS_START:
 	case V4L2_CID_AUTO_FOCUS_STOP:
 		*type = V4L2_CTRL_TYPE_BUTTON;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
 			  V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
 		*min = *max = *step = *def = 0;

@@ -1099,12 +1107,16 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_RF_TUNER_CLASS:
 	case V4L2_CID_DETECT_CLASS:
 		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
+		if (!name)
+			break;
 		/* You can neither read not write these */
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
 		*min = *max = *step = *def = 0;
 		break;
 	case V4L2_CID_BG_COLOR:
 		*type = V4L2_CTRL_TYPE_INTEGER;
+		if (!name)
+			break;
 		*step = 1;
 		*min = 0;
 		/* Max is calculated as RGB888 that is 2^24 */

@@ -1123,10 +1135,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
 	case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:
 		*type = V4L2_CTRL_TYPE_INTEGER;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
 		break;
 	case V4L2_CID_MPEG_VIDEO_DEC_PTS:
 		*type = V4L2_CTRL_TYPE_INTEGER64;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;
 		*min = *def = 0;
 		*max = 0x1ffffffffLL;

@@ -1134,6 +1150,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		break;
 	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:
 		*type = V4L2_CTRL_TYPE_INTEGER64;
+		if (!name)
+			break;
+
 		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;
 		*min = *def = 0;
 		*max = 0x7fffffffffffffffLL;

@@ -1141,6 +1160,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		break;
 	case V4L2_CID_PIXEL_RATE:
 		*type = V4L2_CTRL_TYPE_INTEGER64;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
 		break;
 	case V4L2_CID_DETECT_MD_REGION_GRID:

@@ -1156,6 +1177,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
 	}
+
+	if (!name)
+		return;
+
+	/* Update flags for some controls */
+
 	switch (id) {
 	case V4L2_CID_MPEG_AUDIO_ENCODING:
 	case V4L2_CID_MPEG_AUDIO_MODE:

include/media/v4l2-ctrls.h +2 -0

index 6ba30acf06aa..e22dea218a4c 100644
@@ -352,6 +352,8 @@ struct v4l2_ctrl_config {
  * For non-standard controls it will only fill in the given arguments
  * and @name content will be filled with %NULL.
  *
+ * if @name is NULL, only the @type will be filled.
+ *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on
  * the type.

Signature · 1 line

2.13.3

Mauro Carvalho Chehab

2017-08-10 23:45 UTC #4 lore raw

PATCH 3/3 media: compat32: reimplement ctrl_is_pointer()

In reply to Mauro Carvalho Chehab

The current way that this function works is subject to problems as new controls gets added. Move it to v4l2-ctrls and use the knowledge that v4l2_ctrl_fill() has about controls, in order to detect if a given control is a pointer.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 18 +-----------------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 18 ++++++++++++++++++
 include/media/v4l2-ctrls.h                    |  8 ++++++++
 3 files changed, 27 insertions(+), 17 deletions(-)

drivers/media/v4l2-core/v4l2-compat-ioctl32.c +1 ~~-17~~

index 6f52970f8b54..1105e04dec3d 100644
@@ -19,6 +19,7 @@
 #include <linux/v4l2-subdev.h>
 #include <media/v4l2-dev.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-ctrls.h>
 
 static long native_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {

@@ -665,23 +666,6 @@ struct v4l2_ext_control32 {
 	};
 } __attribute__ ((packed));
 
-/* The following function really belong in v4l2-common, but that causes
-   a circular dependency between modules. We need to think about this, but
-   for now this will do. */
-
-/* Return non-zero if this control is a pointer type. Currently only
-   type STRING is a pointer type. */
-static inline int ctrl_is_pointer(u32 id)
-{
-	switch (id) {
-	case V4L2_CID_RDS_TX_PS_NAME:
-	case V4L2_CID_RDS_TX_RADIO_TEXT:
-		return 1;
-	default:
-		return 0;
-	}
-}
-
 static int get_v4l2_ext_controls32(struct v4l2_ext_controls *kp, struct v4l2_ext_controls32 __user *up)
 {
 	struct v4l2_ext_control32 __user *ucontrols;

drivers/media/v4l2-core/v4l2-ctrls.c +18 -0

index c512b7539077..0d5dab485723 100644
@@ -1254,6 +1254,24 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 }
 EXPORT_SYMBOL(v4l2_ctrl_fill);
 
+bool ctrl_is_pointer(u32 id)
+{
+	enum v4l2_ctrl_type type;
+
+	v4l2_ctrl_fill(id, NULL, &type, NULL, NULL, NULL, NULL, NULL);
+
+	switch (type) {
+	case V4L2_CTRL_TYPE_STRING:
+	case V4L2_CTRL_TYPE_U8:
+	case V4L2_CTRL_TYPE_U16:
+	case V4L2_CTRL_TYPE_U32:
+		return true;
+	default:
+		return false;
+	}
+}
+EXPORT_SYMBOL(ctrl_is_pointer);
+
 static void fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl, u32 changes)
 {
 	memset(ev->reserved, 0, sizeof(ev->reserved));

include/media/v4l2-ctrls.h +8 -0

index e22dea218a4c..bc6772f50956 100644
@@ -367,6 +367,14 @@ struct v4l2_ctrl_config {
 void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		    s64 *min, s64 *max, u64 *step, s64 *def, u32 *flags);
 
+/**
+ * ctrl_is_pointer - Returns non-zero if this control is a pointer type.
+ *
+ * @id: ID of the control
+ *
+ * Currently only STRING and compound types are pointers.
+ */
+bool ctrl_is_pointer(u32 id);
 
 /**
  * v4l2_ctrl_handler_init_class() - Initialize the control handler.

Signature · 1 line

2.13.3

Mauro Carvalho Chehab

2017-08-10 23:52 UTC #5 lore raw

Re: [PATCH 0/3] v4l2-compat-ioctl32.c: better detect pointer controls

In reply to Mauro Carvalho Chehab

Em Thu, 10 Aug 2017 20:45:10 -0300
Mauro Carvalho Chehab <[email protected]> escreveu:

Quoted 15 lines

In the past, only string controls were pointers. That changed when compounded types got added, but the compat32 code was not updated.
We could just add those controls there, but maintaining it is flaw, as we often forget about the compat code. So, instead, rely on the control type, as this is always updated when new controls are added.
As both v4l2-ctrl and compat32 code are at videodev.ko module, we can move the ctrl_is_pointer() helper function to v4l2-ctrl.c.
Mauro Carvalho Chehab (3):
media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill
media: v4l2-ctrls: prepare the function to be used by compat32 code
media: compat32: reimplement ctrl_is_pointer()

Quoted 5 lines

 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 18 +---------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 49 +++++++++++++++++++++++++--
 include/media/v4l2-ctrls.h                    | 28 ++++++++++-----
 3 files changed, 67 insertions(+), 28 deletions(-)

Sorry, sent this series to the wrong mailing list. Will resend to the right one.

Thanks,
Mauro

Mauro Carvalho Chehab

2017-08-11 00:16 UTC #6 lore raw

cover v4l2-compat-ioctl32.c: better detect pointer controls

In reply to Hans Verkuil

In the past, only string controls were pointers. That changed when compounded types got added, but the compat32 code was not updated.

As both v4l2-ctrl and compat32 code are at videodev.ko module, we can move the ctrl_is_pointer() helper function to v4l2-ctrl.c.

---

Re-sending this patch series, as it was c/c to the linux-doc ML by mistake.

 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 18 +---------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 49 +++++++++++++++++++++++++--
 include/media/v4l2-ctrls.h                    | 28 ++++++++++-----
 3 files changed, 67 insertions(+), 28 deletions(-)

Signature · 1 line

2.13.3

Mauro Carvalho Chehab

2017-08-11 00:16 UTC #7 lore raw

PATCH 1/3 media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill

The arguments for this function are pointers. Make it clear at its documentation.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-ctrls.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

include/media/v4l2-ctrls.h +9 -9

index 2d2aed56922f..6ba30acf06aa 100644
@@ -339,18 +339,18 @@ struct v4l2_ctrl_config {
 /**
  * v4l2_ctrl_fill - Fill in the control fields based on the control ID.
  *
- * @id: ID of the control
- * @name: name of the control
- * @type: type of the control
- * @min: minimum value for the control
- * @max: maximum value for the control
- * @step: control step
- * @def: default value for the control
- * @flags: flags to be used on the control
+ * @id: pointer for storing the ID of the control
+ * @name: pointer for storing the name of the control
+ * @type: pointer for storing the type of the control
+ * @min: pointer for storing the minimum value for the control
+ * @max: pointer for storing the maximum value for the control
+ * @step: pointer for storing the control step
+ * @def: pointer for storing the default value for the control
+ * @flags: pointer for storing the flags to be used on the control
  *
  * This works for all standard V4L2 controls.
  * For non-standard controls it will only fill in the given arguments
- * and @name will be %NULL.
+ * and @name content will be filled with %NULL.
  *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on

Signature · 1 line

2.13.3

Hans Verkuil

author

2017-08-11 06:02 UTC #8 lore raw

Re: [PATCH 1/3] media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill

Quoted 15 lines · Mauro Carvalho Chehab

The arguments for this function are pointers. Make it clear at its documentation.
Signed-off-by Mauro Carvalho Chehab <[email protected]>
---
 include/media/v4l2-ctrls.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
include/media/v4l2-ctrls.h +0 -0
index 2d2aed56922f..6ba30acf06aa 100644
@@ -339,18 +339,18 @@ struct v4l2_ctrl_config {
 /**
  * v4l2_ctrl_fill - Fill in the control fields based on the control ID.

include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 342

Quoted 10 lines

include/media/v4l2-ctrls.h +1 -8

  *
- * @id: ID of the control
- * @name: name of the control
- * @type: type of the control
- * @min: minimum value for the control
- * @max: maximum value for the control
- * @step: control step
- * @def: default value for the control
- * @flags: flags to be used on the control
+ * @id: pointer for storing the ID of the control

id isn't a pointer, all other arguments are.

include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 343

Quoted 1 line

include/media/v4l2-ctrls.h +1 -0
+ * @name: pointer for storing the name of the control

This is a pointer to a pointer.

include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 353

Quoted 11 lines

include/media/v4l2-ctrls.h +7 -1

+ * @type: pointer for storing the type of the control
+ * @min: pointer for storing the minimum value for the control
+ * @max: pointer for storing the maximum value for the control
+ * @step: pointer for storing the control step
+ * @def: pointer for storing the default value for the control
+ * @flags: pointer for storing the flags to be used on the control
  *
  * This works for all standard V4L2 controls.
  * For non-standard controls it will only fill in the given arguments
- * and @name will be %NULL.
+ * and @name content will be filled with %NULL.

I'd say: 'set to %NULL'.

include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 356

Quoted 4 lines

include/media/v4l2-ctrls.h +0 -0

  *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on

Regards,

Hans

Mauro Carvalho Chehab

2017-08-11 09:10 UTC #9 lore raw

Re: [PATCH 1/3] media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill

Em Fri, 11 Aug 2017 08:01:58 +0200
Hans Verkuil <[email protected]> escreveu:

Quoted 29 lines · Mauro Carvalho Chehab

Quoted 15 lines
The arguments for this function are pointers. Make it clear at its documentation.
Signed-off-by Mauro Carvalho Chehab <[email protected]>
---
 include/media/v4l2-ctrls.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
include/media/v4l2-ctrls.h +0 -0
index 2d2aed56922f..6ba30acf06aa 100644
@@ -339,18 +339,18 @@ struct v4l2_ctrl_config {
 /**
  * v4l2_ctrl_fill - Fill in the control fields based on the control ID.
include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 342
Quoted 10 lines
include/media/v4l2-ctrls.h +1 -8
  *
- * @id: ID of the control
- * @name: name of the control
- * @type: type of the control
- * @min: minimum value for the control
- * @max: maximum value for the control
- * @step: control step
- * @def: default value for the control
- * @flags: flags to be used on the control
+ * @id: pointer for storing the ID of the control  
id isn't a pointer, all other arguments are.

Quoted 3 lines

include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 343
Quoted 1 line
include/media/v4l2-ctrls.h +1 -0
+ * @name: pointer for storing the name of the control  
This is a pointer to a pointer.

IMO, it is better to say that it is a pointer to a string.

Quoted 15 lines

include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 353

Quoted 12 lines

include/media/v4l2-ctrls.h +7 -1

+ * @type: pointer for storing the type of the control
+ * @min: pointer for storing the minimum value for the control
+ * @max: pointer for storing the maximum value for the control
+ * @step: pointer for storing the control step
+ * @def: pointer for storing the default value for the control
+ * @flags: pointer for storing the flags to be used on the control
  *
  * This works for all standard V4L2 controls.
  * For non-standard controls it will only fill in the given arguments
- * and @name will be %NULL.
+ * and @name content will be filled with %NULL.

I'd say: 'set to %NULL'.

Quoted 8 lines

include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 356
Quoted 4 lines
include/media/v4l2-ctrls.h +0 -0
  *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on
Regards,
Hans

Thanks for reviewing. Version 2 follows.

---

[PATCH v2] media: v4l2-ctrls.h: better document the arguments for
v4l2_ctrl_fill

The arguments for this function are pointers. Make it clear at its documentation.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

include/media/v4l2-ctrls.h +10 -8

index 2d2aed56922f..044ea9bc83a8 100644
@@ -340,17 +340,19 @@ struct v4l2_ctrl_config {
  * v4l2_ctrl_fill - Fill in the control fields based on the control ID.
  *
  * @id: ID of the control
- * @name: name of the control
- * @type: type of the control
- * @min: minimum value for the control
- * @max: maximum value for the control
- * @step: control step
- * @def: default value for the control
- * @flags: flags to be used on the control
+ * @name: pointer to be filled with a string with the name of the control
+ * @type: pointer for storing the type of the control
+ * @min: pointer for storing the minimum value for the control
+ * @max: pointer for storing the maximum value for the control
+ * @step: pointer for storing the control step
+ * @def: pointer for storing the default value for the control
+ * @flags: pointer for storing the flags to be used on the control
  *
  * This works for all standard V4L2 controls.
  * For non-standard controls it will only fill in the given arguments
- * and @name will be %NULL.
+ * and @name content will be set to %NULL.
+ *
+ * if @name is NULL, only the @type will be filled.
  *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on

Thanks,
Mauro

Hans Verkuil

author

2017-08-11 09:35 UTC #10 lore raw

Re: [PATCH 1/3] media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill

Quoted 93 lines · Mauro Carvalho Chehab

Em Fri, 11 Aug 2017 08:01:58 +0200
Hans Verkuil <[email protected]> escreveu:
Quoted 29 lines · Mauro Carvalho Chehab
Quoted 15 lines
The arguments for this function are pointers. Make it clear at its documentation.
Signed-off-by Mauro Carvalho Chehab <[email protected]>
---
 include/media/v4l2-ctrls.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
include/media/v4l2-ctrls.h +0 -0
index 2d2aed56922f..6ba30acf06aa 100644
@@ -339,18 +339,18 @@ struct v4l2_ctrl_config {
 /**
  * v4l2_ctrl_fill - Fill in the control fields based on the control ID.
include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 342
Quoted 10 lines
include/media/v4l2-ctrls.h +1 -8
  *
- * @id: ID of the control
- * @name: name of the control
- * @type: type of the control
- * @min: minimum value for the control
- * @max: maximum value for the control
- * @step: control step
- * @def: default value for the control
- * @flags: flags to be used on the control
+ * @id: pointer for storing the ID of the control  
id isn't a pointer, all other arguments are.
Quoted 3 lines
include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 343
Quoted 1 line
include/media/v4l2-ctrls.h +1 -0
+ * @name: pointer for storing the name of the control  
This is a pointer to a pointer.
IMO, it is better to say that it is a pointer to a string.
Quoted 15 lines
include/media/v4l2-ctrls.h struct v4l2_ctrl_config {
Quoted 12 lines
include/media/v4l2-ctrls.h +7 -1
+ * @type: pointer for storing the type of the control
+ * @min: pointer for storing the minimum value for the control
+ * @max: pointer for storing the maximum value for the control
+ * @step: pointer for storing the control step
+ * @def: pointer for storing the default value for the control
+ * @flags: pointer for storing the flags to be used on the control
  *
  * This works for all standard V4L2 controls.
  * For non-standard controls it will only fill in the given arguments
- * and @name will be %NULL.
+ * and @name content will be filled with %NULL.  
I'd say: 'set to %NULL'.
Quoted 8 lines
include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 358
Quoted 4 lines
include/media/v4l2-ctrls.h +0 -0
  *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on
Regards,
Hans
Thanks for reviewing. Version 2 follows.
---
[PATCH v2] media: v4l2-ctrls.h: better document the arguments for
v4l2_ctrl_fill
The arguments for this function are pointers. Make it clear at its documentation.
Signed-off-by Mauro Carvalho Chehab <[email protected]>
include/media/v4l2-ctrls.h +4 -7
index 2d2aed56922f..044ea9bc83a8 100644
@@ -340,17 +340,19 @@ struct v4l2_ctrl_config {
  * v4l2_ctrl_fill - Fill in the control fields based on the control ID.
  *
  * @id: ID of the control
- * @name: name of the control
- * @type: type of the control
- * @min: minimum value for the control
- * @max: maximum value for the control
- * @step: control step
- * @def: default value for the control
- * @flags: flags to be used on the control
+ * @name: pointer to be filled with a string with the name of the control
+ * @type: pointer for storing the type of the control
+ * @min: pointer for storing the minimum value for the control
+ * @max: pointer for storing the maximum value for the control

include/media/v4l2-ctrls.h struct v4l2_ctrl_config { · line 355

Quoted 10 lines

include/media/v4l2-ctrls.h +6 -1

+ * @step: pointer for storing the control step
+ * @def: pointer for storing the default value for the control
+ * @flags: pointer for storing the flags to be used on the control
  *
  * This works for all standard V4L2 controls.
  * For non-standard controls it will only fill in the given arguments
- * and @name will be %NULL.
+ * and @name content will be set to %NULL.
+ *
+ * if @name is NULL, only the @type will be filled.

Not quite correct, I'd say:

* if @name is NULL, then @type is set to V4L2_CTRL_TYPE_INTEGER and @flags to 0.

Regards,

Hans

Quoted 8 lines

include/media/v4l2-ctrls.h +0 -0

  *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on

Thanks,
Mauro

Mauro Carvalho Chehab

2017-08-12 09:57 UTC #11 lore raw

PATCH v3 1/1 media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill

The arguments for this function are pointers. Make it clear at its documentation.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---

Hans,

Feel free to pick this patch on your tree, if you're ok with it. Your approach for using v4l_queryctl() sounds better, as it covers private controls too. So I'm not submitting the other patches that used to be in this series. Yet, I think it doesn't hurt to make the documentation clearer about the pointers.

 include/media/v4l2-ctrls.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

include/media/v4l2-ctrls.h +8 -8

index 2d2aed56922f..dacfe54057f8 100644
@@ -340,17 +340,17 @@ struct v4l2_ctrl_config {
  * v4l2_ctrl_fill - Fill in the control fields based on the control ID.
  *
  * @id: ID of the control
- * @name: name of the control
- * @type: type of the control
- * @min: minimum value for the control
- * @max: maximum value for the control
- * @step: control step
- * @def: default value for the control
- * @flags: flags to be used on the control
+ * @name: pointer to be filled with a string with the name of the control
+ * @type: pointer for storing the type of the control
+ * @min: pointer for storing the minimum value for the control
+ * @max: pointer for storing the maximum value for the control
+ * @step: pointer for storing the control step
+ * @def: pointer for storing the default value for the control
+ * @flags: pointer for storing the flags to be used on the control
  *
  * This works for all standard V4L2 controls.
  * For non-standard controls it will only fill in the given arguments
- * and @name will be %NULL.
+ * and @name content will be set to %NULL.
  *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on

Signature · 1 line

2.13.3

Mauro Carvalho Chehab

2017-08-11 00:16 UTC #12 lore raw

PATCH 2/3 media: v4l2-ctrls: prepare the function to be used by compat32 code

In reply to Mauro Carvalho Chehab

Change v4l2_ctrl_fill() function for it to be called with just one argument: the control type. This way, the compat32 code can use it internally.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/v4l2-core/v4l2-ctrls.c | 31 +++++++++++++++++++++++++++++--
 include/media/v4l2-ctrls.h           |  2 ++
 2 files changed, 31 insertions(+), 2 deletions(-)

drivers/media/v4l2-core/v4l2-ctrls.c +29 -2

index dd1db678718c..c512b7539077 100644
@@ -939,8 +939,10 @@ EXPORT_SYMBOL(v4l2_ctrl_get_name);
 void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		    s64 *min, s64 *max, u64 *step, s64 *def, u32 *flags)
 {
-	*name = v4l2_ctrl_get_name(id);
-	*flags = 0;
+	if (name) {
+		*name = v4l2_ctrl_get_name(id);
+		*flags = 0;
+	}
 
 	switch (id) {
 	case V4L2_CID_AUDIO_MUTE:

@@ -996,11 +998,15 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_RDS_RX_TRAFFIC_PROGRAM:
 	case V4L2_CID_RDS_RX_MUSIC_SPEECH:
 		*type = V4L2_CTRL_TYPE_BOOLEAN;
+		if (!name)
+			break;
 		*min = 0;
 		*max = *step = 1;
 		break;
 	case V4L2_CID_ROTATE:
 		*type = V4L2_CTRL_TYPE_INTEGER;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
 		break;
 	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:

@@ -1015,6 +1021,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_AUTO_FOCUS_START:
 	case V4L2_CID_AUTO_FOCUS_STOP:
 		*type = V4L2_CTRL_TYPE_BUTTON;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
 			  V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
 		*min = *max = *step = *def = 0;

@@ -1099,12 +1107,16 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_RF_TUNER_CLASS:
 	case V4L2_CID_DETECT_CLASS:
 		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
+		if (!name)
+			break;
 		/* You can neither read not write these */
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
 		*min = *max = *step = *def = 0;
 		break;
 	case V4L2_CID_BG_COLOR:
 		*type = V4L2_CTRL_TYPE_INTEGER;
+		if (!name)
+			break;
 		*step = 1;
 		*min = 0;
 		/* Max is calculated as RGB888 that is 2^24 */

@@ -1123,10 +1135,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
 	case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:
 		*type = V4L2_CTRL_TYPE_INTEGER;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
 		break;
 	case V4L2_CID_MPEG_VIDEO_DEC_PTS:
 		*type = V4L2_CTRL_TYPE_INTEGER64;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;
 		*min = *def = 0;
 		*max = 0x1ffffffffLL;

@@ -1134,6 +1150,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		break;
 	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:
 		*type = V4L2_CTRL_TYPE_INTEGER64;
+		if (!name)
+			break;
+
 		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;
 		*min = *def = 0;
 		*max = 0x7fffffffffffffffLL;

@@ -1141,6 +1160,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		break;
 	case V4L2_CID_PIXEL_RATE:
 		*type = V4L2_CTRL_TYPE_INTEGER64;
+		if (!name)
+			break;
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
 		break;
 	case V4L2_CID_DETECT_MD_REGION_GRID:

@@ -1156,6 +1177,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
 	}
+
+	if (!name)
+		return;
+
+	/* Update flags for some controls */
+
 	switch (id) {
 	case V4L2_CID_MPEG_AUDIO_ENCODING:
 	case V4L2_CID_MPEG_AUDIO_MODE:

include/media/v4l2-ctrls.h +2 -0

index 6ba30acf06aa..e22dea218a4c 100644
@@ -352,6 +352,8 @@ struct v4l2_ctrl_config {
  * For non-standard controls it will only fill in the given arguments
  * and @name content will be filled with %NULL.
  *
+ * if @name is NULL, only the @type will be filled.
+ *
  * This function will overwrite the contents of @name, @type and @flags.
  * The contents of @min, @max, @step and @def may be modified depending on
  * the type.

Signature · 1 line

2.13.3

Mauro Carvalho Chehab

2017-08-11 00:16 UTC #13 lore raw

PATCH 3/3 media: compat32: reimplement ctrl_is_pointer()

In reply to Mauro Carvalho Chehab

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 18 +-----------------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 18 ++++++++++++++++++
 include/media/v4l2-ctrls.h                    |  8 ++++++++
 3 files changed, 27 insertions(+), 17 deletions(-)

drivers/media/v4l2-core/v4l2-compat-ioctl32.c +1 ~~-17~~

index af8b4c5b0efa..0adcc37280c8 100644
@@ -19,6 +19,7 @@
 #include <linux/v4l2-subdev.h>
 #include <media/v4l2-dev.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-ctrls.h>
 
 static long native_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {

@@ -668,23 +669,6 @@ struct v4l2_ext_control32 {
 	};
 } __attribute__ ((packed));
 
-/* The following function really belong in v4l2-common, but that causes
-   a circular dependency between modules. We need to think about this, but
-   for now this will do. */
-
-/* Return non-zero if this control is a pointer type. Currently only
-   type STRING is a pointer type. */
-static inline int ctrl_is_pointer(u32 id)
-{
-	switch (id) {
-	case V4L2_CID_RDS_TX_PS_NAME:
-	case V4L2_CID_RDS_TX_RADIO_TEXT:
-		return 1;
-	default:
-		return 0;
-	}
-}
-
 static int get_v4l2_ext_controls32(struct v4l2_ext_controls *kp, struct v4l2_ext_controls32 __user *up)
 {
 	struct v4l2_ext_control32 __user *ucontrols;

drivers/media/v4l2-core/v4l2-ctrls.c +18 -0

index c512b7539077..0d5dab485723 100644
@@ -1254,6 +1254,24 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 }
 EXPORT_SYMBOL(v4l2_ctrl_fill);
 
+bool ctrl_is_pointer(u32 id)
+{
+	enum v4l2_ctrl_type type;
+
+	v4l2_ctrl_fill(id, NULL, &type, NULL, NULL, NULL, NULL, NULL);
+
+	switch (type) {
+	case V4L2_CTRL_TYPE_STRING:
+	case V4L2_CTRL_TYPE_U8:
+	case V4L2_CTRL_TYPE_U16:
+	case V4L2_CTRL_TYPE_U32:
+		return true;
+	default:
+		return false;
+	}
+}
+EXPORT_SYMBOL(ctrl_is_pointer);
+
 static void fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl, u32 changes)
 {
 	memset(ev->reserved, 0, sizeof(ev->reserved));

include/media/v4l2-ctrls.h +8 -0

index e22dea218a4c..bc6772f50956 100644
@@ -367,6 +367,14 @@ struct v4l2_ctrl_config {
 void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		    s64 *min, s64 *max, u64 *step, s64 *def, u32 *flags);
 
+/**
+ * ctrl_is_pointer - Returns non-zero if this control is a pointer type.
+ *
+ * @id: ID of the control
+ *
+ * Currently only STRING and compound types are pointers.
+ */
+bool ctrl_is_pointer(u32 id);
 
 /**
  * v4l2_ctrl_handler_init_class() - Initialize the control handler.

Signature · 1 line

2.13.3

Hans Verkuil

author

2017-08-11 06:05 UTC #14 lore raw

Re: [PATCH RESEND 0/3] v4l2-compat-ioctl32.c: better detect pointer controls

In reply to Mauro Carvalho Chehab

Quoted 9 lines · Mauro Carvalho Chehab

In the past, only string controls were pointers. That changed when compounded types got added, but the compat32 code was not updated.
We could just add those controls there, but maintaining it is flaw, as we often forget about the compat code. So, instead, rely on the control type, as this is always updated when new controls are added.
As both v4l2-ctrl and compat32 code are at videodev.ko module, we can move the ctrl_is_pointer() helper function to v4l2-ctrl.c.

This series doesn't really solve anything:

- it introduces a circular dependency between two modules
- it doesn't handle driver-custom controls (the old code didn't either). For example vivid has custom pointer controls.
- it replaces a list of control IDs with a list of type IDs, which also has to be kept up to date.

I thought this over and I have a better and much simpler idea. I'll post a patch for that.

Regards,

Hans

Quoted 15 lines

---
Re-sending this patch series, as it was c/c to the linux-doc ML by mistake.
Mauro Carvalho Chehab (3):
media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill
media: v4l2-ctrls: prepare the function to be used by compat32 code
media: compat32: reimplement ctrl_is_pointer()
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 18 +---------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 49 +++++++++++++++++++++++++--
 include/media/v4l2-ctrls.h                    | 28 ++++++++++-----
 3 files changed, 67 insertions(+), 28 deletions(-)

Mauro Carvalho Chehab

2017-08-11 09:00 UTC #15 lore raw

Re: [PATCH RESEND 0/3] v4l2-compat-ioctl32.c: better detect pointer controls

Em Fri, 11 Aug 2017 08:05:03 +0200
Hans Verkuil <[email protected]> escreveu:

Quoted 8 lines · Mauro Carvalho Chehab

Quoted 7 lines
In the past, only string controls were pointers. That changed when compounded types got added, but the compat32 code was not updated.
We could just add those controls there, but maintaining it is flaw, as we often forget about the compat code. So, instead, rely on the control type, as this is always updated when new controls are added.

Quoted 6 lines

Quoted 2 lines
As both v4l2-ctrl and compat32 code are at videodev.ko module, we can move the ctrl_is_pointer() helper function to v4l2-ctrl.c.
This series doesn't really solve anything:
- it introduces a circular dependency between two modules

What two modules? both v4l2-ctrl and compat32 belong to the *same* module. See the Makefile:

videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
			v4l2-async.o

ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif

Both belong to videodev module. IMHO, the best is to move whatever control check logic it might need to v4l2-ctrls.

Quoted 2 lines

- it doesn't handle driver-custom controls (the old code didn't either). For example vivid has custom pointer controls.

True.

Quoted 2 lines

- it replaces a list of control IDs with a list of type IDs, which also has to be kept up to date.

True, but at least after the patch, the ancillary function is together with the code that handles the controls. Also, we don't introduce new types too often.

Quoted 3 lines

I thought this over and I have a better and much simpler idea. I'll post a patch for that.

OK.

Quoted 21 lines

Regards,
Hans
Quoted 15 lines
---
Re-sending this patch series, as it was c/c to the linux-doc ML by mistake.
Mauro Carvalho Chehab (3):
media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill
media: v4l2-ctrls: prepare the function to be used by compat32 code
media: compat32: reimplement ctrl_is_pointer()
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 18 +---------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 49 +++++++++++++++++++++++++--
 include/media/v4l2-ctrls.h                    | 28 ++++++++++-----
 3 files changed, 67 insertions(+), 28 deletions(-)

Thanks,
Mauro

Hans Verkuil

author

2017-08-11 09:24 UTC #16 lore raw

Re: [PATCH RESEND 0/3] v4l2-compat-ioctl32.c: better detect pointer controls

Quoted 21 lines · Mauro Carvalho Chehab

Em Fri, 11 Aug 2017 08:05:03 +0200
Hans Verkuil <[email protected]> escreveu:
Quoted 8 lines · Mauro Carvalho Chehab
Quoted 7 lines
In the past, only string controls were pointers. That changed when compounded types got added, but the compat32 code was not updated.
We could just add those controls there, but maintaining it is flaw, as we often forget about the compat code. So, instead, rely on the control type, as this is always updated when new controls are added.
Quoted 6 lines
Quoted 2 lines
As both v4l2-ctrl and compat32 code are at videodev.ko module, we can move the ctrl_is_pointer() helper function to v4l2-ctrl.c.
This series doesn't really solve anything:
- it introduces a circular dependency between two modules
What two modules? both v4l2-ctrl and compat32 belong to the *same* module. See the Makefile:

Quoted 6 lines

videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
			v4l2-async.o
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif

My fault, I should have checked. The 'circular dependency' comment in the code referred to the bad old days when v4l2_ctrl_fill was in v4l2-common.c and that was a separate module. All that has long since changed.

Quoted 16 lines

Both belong to videodev module. IMHO, the best is to move whatever control check logic it might need to v4l2-ctrls.
Quoted 2 lines
- it doesn't handle driver-custom controls (the old code didn't either). For example vivid has custom pointer controls.
True.
Quoted 2 lines
- it replaces a list of control IDs with a list of type IDs, which also has to be kept up to date.
True, but at least after the patch, the ancillary function is together with the code that handles the controls. Also, we don't introduce new types too often.
Quoted 1 line

Quoted 4 lines

Quoted 2 lines
I thought this over and I have a better and much simpler idea. I'll post a patch for that.
OK.

I have the patch ready, but I need to test it first with vivid.

Regards,

Hans

Laurent Pinchart

2017-08-11 09:46 UTC #17 lore raw

Re: [PATCH RESEND 0/3] v4l2-compat-ioctl32.c: better detect pointer controls

In reply to Hans Verkuil

Hi Hans,

Quoted 13 lines · On Friday 11 Aug 2017 08:05:03 Hans Verkuil wrote:

Quoted 9 lines · Mauro Carvalho Chehab
In the past, only string controls were pointers. That changed when compounded types got added, but the compat32 code was not updated.
We could just add those controls there, but maintaining it is flaw, as we often forget about the compat code. So, instead, rely on the control type, as this is always updated when new controls are added.
As both v4l2-ctrl and compat32 code are at videodev.ko module, we can move the ctrl_is_pointer() helper function to v4l2-ctrl.c.
This series doesn't really solve anything:

Quoted 8 lines

- it introduces a circular dependency between two modules
- it doesn't handle driver-custom controls (the old code didn't either). For example vivid has custom pointer controls.
- it replaces a list of control IDs with a list of type IDs, which also has to be kept up to date.
I thought this over and I have a better and much simpler idea. I'll post a patch for that.

Wouldn't it be time to replace the large switch/case with an array of control information ? Maybe that was your better idea already :-)

Quoted 14 lines

Quoted 14 lines
---
Re-sending this patch series, as it was c/c to the linux-doc ML by mistake.
Mauro Carvalho Chehab (3):
media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill
media: v4l2-ctrls: prepare the function to be used by compat32 code
media: compat32: reimplement ctrl_is_pointer()
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 18 +---------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 49 ++++++++++++++++++++--
 include/media/v4l2-ctrls.h                    | 28 ++++++++++-----
 3 files changed, 67 insertions(+), 28 deletions(-)

Signature · 3 lines

Regards,

Laurent Pinchart