add feedback LED control

PATCH In review Németh Márton 5 participants 10 messages Feb 28, 2010 – May 6, 2010 Media (V4L/DVB)

lore

Share & tools

lore

Németh Márton

author

2010-02-28 07:55 UTC #0 lore raw

PATCH 1/3 add feedback LED control

From: Márton Németh <[email protected]>

On some webcams a feedback LED can be found. This LED usually shows the state of streaming mode: this is the "Auto" mode. The LED can be programmed to constantly switched off state (e.g. for power saving reasons, preview mode) or on state (e.g. the application shows motion detection or "on-air").

Signed-off-by Márton Németh <[email protected]>

---

linux/Documentation/DocBook/v4l/controls.xml +11 -0

diff -r d8fafa7d88dc linux/Documentation/DocBook/v4l/controls.xml
@@ -311,6 +311,17 @@
 Applications depending on particular custom controls should check the
 driver name and version, see <xref linkend="querycap" />.</entry>
 	  </row>
+	  <row id="v4l2-led">
+	    <entry><constant>V4L2_CID_LED</constant></entry>
+	    <entry>enum</entry>
+	    <entry>Controls the feedback LED on the camera. In auto mode
+the LED is on when the streaming is active. The LED is off when not streaming.
+The LED can be also turned on and off independent from streaming.
+Possible values for <constant>enum v4l2_led</constant> are:
+<constant>V4L2_CID_LED_AUTO</constant> (0),
+<constant>V4L2_CID_LED_ON</constant> (1) and
+<constant>V4L2_CID_LED_OFF</constant> (2).</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>

linux/Documentation/DocBook/v4l/videodev2.h.xml +7 -1

diff -r d8fafa7d88dc linux/Documentation/DocBook/v4l/videodev2.h.xml
@@ -1024,8 +1024,14 @@
 
 #define V4L2_CID_ROTATE                         (V4L2_CID_BASE+34)
 #define V4L2_CID_BG_COLOR                       (V4L2_CID_BASE+35)
+#define V4L2_CID_LED                            (V4L2_CID_BASE+36)
+enum v4l2_led {
+        V4L2_LED_AUTO           = 0,
+        V4L2_LED_ON             = 1,
+        V4L2_LED_OFF            = 2,
+};
 /* last CID + 1 */
-#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+36)
+#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+37)
 
 /*  MPEG-class control IDs defined by V4L2 */
 #define V4L2_CID_MPEG_BASE                      (V4L2_CTRL_CLASS_MPEG | 0x900)

linux/drivers/media/video/v4l2-common.c +10 -0

diff -r d8fafa7d88dc linux/drivers/media/video/v4l2-common.c
@@ -349,6 +349,12 @@
 		"75 useconds",
 		NULL,
 	};
+	static const char *led[] = {
+		"Auto",
+		"On",
+		"Off",
+		NULL,
+	};
 
 	switch (id) {
 		case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:

@@ -389,6 +395,8 @@
 			return colorfx;
 		case V4L2_CID_TUNE_PREEMPHASIS:
 			return tune_preemphasis;
+		case V4L2_CID_LED:
+			return led;
 		default:
 			return NULL;
 	}

@@ -434,6 +442,7 @@
 	case V4L2_CID_COLORFX:			return "Color Effects";
 	case V4L2_CID_ROTATE:			return "Rotate";
 	case V4L2_CID_BG_COLOR:			return "Background color";
+	case V4L2_CID_LED:			return "Feedback LED";
 
 	/* MPEG controls */
 	case V4L2_CID_MPEG_CLASS: 		return "MPEG Encoder Controls";

@@ -575,6 +584,7 @@
 	case V4L2_CID_EXPOSURE_AUTO:
 	case V4L2_CID_COLORFX:
 	case V4L2_CID_TUNE_PREEMPHASIS:
+	case V4L2_CID_LED:
 		qctrl->type = V4L2_CTRL_TYPE_MENU;
 		step = 1;
 		break;

linux/include/linux/videodev2.h +7 -1

diff -r d8fafa7d88dc linux/include/linux/videodev2.h
@@ -1030,8 +1030,14 @@
 
 #define V4L2_CID_ROTATE				(V4L2_CID_BASE+34)
 #define V4L2_CID_BG_COLOR			(V4L2_CID_BASE+35)
+#define V4L2_CID_LED				(V4L2_CID_BASE+36)
+enum v4l2_led {
+	V4L2_LED_AUTO		= 0,
+	V4L2_LED_ON		= 1,
+	V4L2_LED_OFF		= 2,
+};
 /* last CID + 1 */
-#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+36)
+#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+37)
 
 /*  MPEG-class control IDs defined by V4L2 */
 #define V4L2_CID_MPEG_BASE 			(V4L2_CTRL_CLASS_MPEG | 0x900)

Jean-Francois Moine

2010-02-28 19:27 UTC #1 lore raw

Quoted 5 lines · On Sun, 28 Feb 2010 08:55:04 +0100 Németh Márton <[email protected]> wrote:

On some webcams a feedback LED can be found. This LED usually shows the state of streaming mode: this is the "Auto" mode. The LED can be programmed to constantly switched off state (e.g. for power saving reasons, preview mode) or on state (e.g. the application shows motion detection or "on-air").

Hi,

There may be many LEDs on the webcams. One LED may be used for the streaming state, Some other ones may be used to give more light in dark rooms. One webcam, the microscope 093a:050f, has a top and a bottom lights/illuminators; an other one, the MSI StarCam 370i 0c45:60c0, has an infra-red light.

That's why I proposed to have bit fields in the control value to switch on/off each LED.

Cheers.

Signature · 2 lines

Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Németh Márton

author

2010-02-28 19:38 UTC #2 lore raw

Quoted 11 lines · Jean-Francois Moine

Quoted 5 lines · On Sun, 28 Feb 2010 08:55:04 +0100 Németh Márton <[email protected]> wrote:
On some webcams a feedback LED can be found. This LED usually shows the state of streaming mode: this is the "Auto" mode. The LED can be programmed to constantly switched off state (e.g. for power saving reasons, preview mode) or on state (e.g. the application shows motion detection or "on-air").
Hi,

Quoted 8 lines

There may be many LEDs on the webcams. One LED may be used for the streaming state, Some other ones may be used to give more light in dark rooms. One webcam, the microscope 093a:050f, has a top and a bottom lights/illuminators; an other one, the MSI StarCam 370i 0c45:60c0, has an infra-red light.
That's why I proposed to have bit fields in the control value to switch on/off each LED.

With a bitfield on and off state can be specified. What about the "auto" mode? Should two bits grouped together to have auto, on and off state? Is there already a similar control?

Is the brightness of the background light LEDs adjustable or are they just on/off? If yes, then maybe the feedback LEDs and the background light LEDs should be treated as different kind.

Regards,

Márton Németh

Laurent Pinchart

2010-03-01 09:02 UTC #3 lore raw

Quoted 18 lines · On Sunday 28 February 2010 20:38:00 Németh Márton wrote:

Quoted 17 lines · Jean-Francois Moine
On Sun, 28 Feb 2010 08:55:04 +0100
Quoted 5 lines · Németh Márton
On some webcams a feedback LED can be found. This LED usually shows the state of streaming mode: this is the "Auto" mode. The LED can be programmed to constantly switched off state (e.g. for power saving reasons, preview mode) or on state (e.g. the application shows motion detection or "on-air").
Hi,
There may be many LEDs on the webcams. One LED may be used for the streaming state, Some other ones may be used to give more light in dark rooms. One webcam, the microscope 093a:050f, has a top and a bottom lights/illuminators; an other one, the MSI StarCam 370i 0c45:60c0, has an infra-red light.

Quoted 6 lines

Quoted 2 lines
That's why I proposed to have bit fields in the control value to switch on/off each LED.
With a bitfield on and off state can be specified. What about the "auto" mode? Should two bits grouped together to have auto, on and off state? Is there already a similar control?

I don't like the bitfield either. As stated in my previous mail, we can have more than 3 states, so using 2 bits per LED will simply not scale.

Quoted 3 lines

Is the brightness of the background light LEDs adjustable or are they just on/off? If yes, then maybe the feedback LEDs and the background light LEDs should be treated as different kind.

I think there should indeed be a different control for the background LEDs. Still, there could be more than one feedback LED.

Signature · 3 lines

Regards,

Laurent Pinchart

Jean-Francois Moine

2010-03-01 09:17 UTC #4 lore raw

In reply to Németh Márton

Quoted 7 lines · On Sun, 28 Feb 2010 20:38:00 +0100 Németh Márton <[email protected]> wrote:

With a bitfield on and off state can be specified. What about the "auto" mode? Should two bits grouped together to have auto, on and off state? Is there already a similar control?
Is the brightness of the background light LEDs adjustable or are they just on/off? If yes, then maybe the feedback LEDs and the background light LEDs should be treated as different kind.

OK. My idea about switching the LEDs by v4l2 controls was not good. So, forget about it.

Instead, some job of the led class may be done in the gspca main, especially register/unregister.

I propose to add a LED description in the gspca structure (level 'struct cam'). There would be 'nleds' for the number of LEDS and 'leds', a pointer to an array of:

	struct gspca_led {
		struct led_classdev led_cdev;
		char led_name[32];
		struct led_trigger led_trigger;
		char trigger_name[32];
	};

(this array should be in the subdriver structure - I don't show the #ifdef's)

Then, this would work as:

- on probe, in the 'config' function of the subdriver, this one initializes the led and trigger fields. The 'led_cdev.name' and 'led_trigger.name' should point to a sprintf format with one
argument: the video device number (ex: "video%d::toplight").

- then, at the end of gspca_dev_probe(), the gspca main creates the real names of the leds and triggers, and does the register job.

- all led/trigger events are treated by the subdriver, normally by a workqueue. This one must not be the system workqueue.

- on disconnection, the gspca main unregisters the leds and triggers without calling the subdriver. In the workqueue, the disconnection can be simply handled testing the flag 'present' after each subsystem call.

Cheers.

Signature · 2 lines

Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Németh Márton

author

2010-03-02 23:42 UTC #5 lore raw

RFC PATCH 1/3 gspca: add LEDs subsystem connection

From: Márton Németh <[email protected]>

On some webcams one or more LEDs can be found. One type of these LEDs are feedback LEDs: they usually shows the state of streaming mode. The LED can be programmed to constantly switched off state (e.g. for power saving reasons, preview mode) or on state (e.g. the application shows motion detection or "on-air").

The second type of LEDs are used to create enough light for the sensor for example visible or in infra-red light.

Both type of these LEDs can be handled using the LEDs subsystem. This patch add support to connect a gspca based driver to the LEDs subsystem.

Signed-off-by Márton Németh <[email protected]>

---

linux/drivers/media/video/gspca/gspca.h +29 -0

diff -r d8fafa7d88dc linux/drivers/media/video/gspca/gspca.h
@@ -7,6 +7,8 @@
 #include <linux/videodev2.h>
 #include <media/v4l2-common.h>
 #include <linux/mutex.h>
+#include <linux/leds.h>
+#include <linux/workqueue.h>
 #include "compat.h"
 
 /* compilation option */

@@ -53,14 +55,41 @@
 	int nrates;
 };
 
+#if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE)
+/**
+ * struct gspca_led - data structure for one LED on a camera
+ * @led_cdev: the class device for the LED
+ * @name: place for the LED name which will appear under /sys/class/leds/
+ * @led_trigger: the trigger structure for the same LED
+ * @trigger_name: place for the trigger name which will appear in the file
+ *                /sys/class/leds/{led name}/trigger
+ * @parent: pointer to the parent gspca_dev
+ */
+struct gspca_led {
+	struct led_classdev led_cdev;
+	char name[32];
+#ifdef CONFIG_LEDS_TRIGGERS
+	struct led_trigger led_trigger;
+	char trigger_name[32];
+#endif
+	struct gspca_dev *parent;
+};
+#endif
+
 /* device information - set at probe time */
 struct cam {
 	const struct v4l2_pix_format *cam_mode;	/* size nmodes */
 	const struct framerates *mode_framerates; /* must have size nmode,
 						   * just like cam_mode */
+#if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE)
+	struct gspca_led *leds;
+#endif
 	u32 bulk_size;		/* buffer size when image transfer by bulk */
 	u32 input_flags;	/* value for ENUM_INPUT status flags */
 	u8 nmodes;		/* size of cam_mode */
+#if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE)
+	u8 nleds;		/* number of LEDs */
+#endif
 	u8 no_urb_create;	/* don't create transfer URBs */
 	u8 bulk_nurbs;		/* number of URBs in bulk mode
 				 * - cannot be > MAX_NURBS

linux/drivers/media/video/gspca/gspca.c +75 -0

99 lines not on this page · raw message

Laurent Pinchart

2010-03-09 14:57 UTC #6 lore raw

Re: [RFC, PATCH 1/3] gspca: add LEDs subsystem connection

Hi Màrton,

Thanks for the patch.

Quoted 8 lines · On Wednesday 03 March 2010 00:42:23 Németh Márton wrote:

From: Márton Németh <[email protected]>
On some webcams one or more LEDs can be found. One type of these LEDs are feedback LEDs: they usually shows the state of streaming mode. The LED can be programmed to constantly switched off state (e.g. for power saving reasons, preview mode) or on state (e.g. the application shows motion detection or "on-air").

Quoted 5 lines

The second type of LEDs are used to create enough light for the sensor for example visible or in infra-red light.
Both type of these LEDs can be handled using the LEDs subsystem. This patch add support to connect a gspca based driver to the LEDs subsystem.

They can indeed, but I'm not sure if the LEDs subsystem was designed for that kind of use cases.

The LED framework was developed to handle LEDs found in embedded systems (usually connected to GPIOs) that needed to be connected to software triggers or controlled by drivers and/or specific userspace applications. Webcam LEDs seem a bit out of scope to me, especially the "light" LED that might be better handled by a V4L2 set of controls (we're currently missing controls for camera flashes, be they LEDs or Xenon based).

I'll let Richard speak on this.

Signature · 3 lines

Regards,

Laurent Pinchart

Richard Purdie

2010-03-09 16:35 UTC #7 lore raw

Re: [RFC, PATCH 1/3] gspca: add LEDs subsystem connection

Quoted 13 lines · Laurent Pinchart

Hi Màrton,
Thanks for the patch.
Quoted 8 lines · On Wednesday 03 March 2010 00:42:23 Németh Márton wrote:
From: Márton Németh <[email protected]>
On some webcams one or more LEDs can be found. One type of these LEDs are feedback LEDs: they usually shows the state of streaming mode. The LED can be programmed to constantly switched off state (e.g. for power saving reasons, preview mode) or on state (e.g. the application shows motion detection or "on-air").

Quoted 8 lines

Quoted 5 lines
The second type of LEDs are used to create enough light for the sensor for example visible or in infra-red light.
Both type of these LEDs can be handled using the LEDs subsystem. This patch add support to connect a gspca based driver to the LEDs subsystem.
They can indeed, but I'm not sure if the LEDs subsystem was designed for that kind of use cases.

The LED subsystem was designed to support LED lights and the above scenarios can certainly fit that. It provides a nice system where default use cases should just work (power light on a laptop) but the system has the control to override than and do other interesting things with it if it so wishes.

Quoted 3 lines

The LED framework was developed to handle LEDs found in embedded systems (usually connected to GPIOs) that needed to be connected to software triggers or controlled by drivers and/or specific userspace applications.

Its used by many laptops so its not just embedded systems.

Quoted 6 lines

Webcam LEDs
seem a bit out of scope to me, especially the "light" LED that might be better handled by a V4L2 set of controls (we're currently missing controls for camera flashes, be they LEDs or Xenon based).
I'll let Richard speak on this.

I'm not going to push one way or another and its up to individual subsystems to way up the benefits and drawbacks and make their decision. There is nothing in the design of the system which would stop it working or being used in this case though. If the susystsme needs improvements to work well, I'm happy to accept patches too.

Cheers,

Richard

Mauro Carvalho Chehab

2010-05-06 18:15 UTC #8 lore raw

In reply to Jean-Francois Moine

Quoted 44 lines · Jean-Francois Moine

Quoted 7 lines · On Sun, 28 Feb 2010 20:38:00 +0100 Németh Márton <[email protected]> wrote:
With a bitfield on and off state can be specified. What about the "auto" mode? Should two bits grouped together to have auto, on and off state? Is there already a similar control?
Is the brightness of the background light LEDs adjustable or are they just on/off? If yes, then maybe the feedback LEDs and the background light LEDs should be treated as different kind.
OK. My idea about switching the LEDs by v4l2 controls was not good. So, forget about it.
Instead, some job of the led class may be done in the gspca main, especially register/unregister.
I propose to add a LED description in the gspca structure (level 'struct cam'). There would be 'nleds' for the number of LEDS and 'leds', a pointer to an array of:
	struct gspca_led {
		struct led_classdev led_cdev;
		char led_name[32];
		struct led_trigger led_trigger;
		char trigger_name[32];
	};
(this array should be in the subdriver structure - I don't show the #ifdef's)
Then, this would work as:
- on probe, in the 'config' function of the subdriver, this one initializes the led and trigger fields. The 'led_cdev.name' and 'led_trigger.name' should point to a sprintf format with one
argument: the video device number (ex: "video%d::toplight").
- then, at the end of gspca_dev_probe(), the gspca main creates the real names of the leds and triggers, and does the register job.
- all led/trigger events are treated by the subdriver, normally by a workqueue. This one must not be the system workqueue.

Quoted 7 lines

- on disconnection, the gspca main unregisters the leds and triggers without calling the subdriver. In the workqueue, the disconnection can be simply handled testing the flag 'present' after each subsystem call.
Cheers.

The feedback LED patch series doesn't apply anymore.

patching file Documentation/DocBook/v4l/controls.xml
Hunk #1 succeeded at 324 (offset 13 lines).
patching file Documentation/DocBook/v4l/videodev2.h.xml
Hunk #1 succeeded at 1031 (offset 7 lines).
patching file drivers/media/video/v4l2-common.c
Hunk #1 succeeded at 358 (offset 9 lines).
Hunk #3 FAILED at 442.
Hunk #4 succeeded at 598 (offset 14 lines).
1 out of 4 hunks FAILED -- saving rejects to file drivers/media/video/v4l2-common.c.rej patching file include/linux/videodev2.h
Hunk #1 FAILED at 1030.
1 out of 1 hunk FAILED -- saving rejects to file include/linux/videodev2.h.rej

Quoted 1 line

Quoted 1 line
Quoted 1 line
Patch patches/lmml_82773_1_3_add_feedback_led_control.patch doesn't apply

As the original approach weren't accepted, I'm dropping those patches from my queue. Please re-submit after having some agreement about that.

Signature · 2 lines

Cheers,
Mauro

Laurent Pinchart

2010-03-01 09:00 UTC #9 lore raw

In reply to Németh Márton

Hi Márton,

Quoted 8 lines · On Sunday 28 February 2010 08:55:04 Németh Márton wrote:

From: Márton Németh <[email protected]>
On some webcams a feedback LED can be found. This LED usually shows the state of streaming mode: this is the "Auto" mode. The LED can be programmed to constantly switched off state (e.g. for power saving reasons, preview mode) or on state (e.g. the application shows motion detection or "on-air").

Quoted 4 lines

Signed-off-by Márton Németh <[email protected]>
---
diff -r d8fafa7d88dc linux/Documentation/DocBook/v4l/controls.xml
--- a/linux/Documentation/DocBook/v4l/controls.xml Thu Feb 18 19:02:51

2010

Quoted 15 lines

+0100 +++ b/linux/Documentation/DocBook/v4l/controls.xml Sun Feb 28
08:41:17 2010 +0100 @@ -311,6 +311,17 @@
Applications depending on particular custom controls should check the driver name and version, see <xref linkend="querycap" />.</entry>
linux/Documentation/DocBook/v4l/controls.xml +5 -0
 	  </row>
+	  <row id="v4l2-led">
+	    <entry><constant>V4L2_CID_LED</constant></entry>
+	    <entry>enum</entry>
+	    <entry>Controls the feedback LED on the camera. In auto mode
+the LED is on when the streaming is active. The LED is off when not
streaming. +The LED can be also turned on and off independent from streaming. +Possible values for <constant>enum v4l2_led</constant> are:
+<constant>V4L2_CID_LED_AUTO</constant> (0), +<constant>V4L2_CID_LED_ON</constant> (1) and +<constant>V4L2_CID_LED_OFF</constant> (2).</entry>

linux/Documentation/DocBook/v4l/controls.xml line 324

Quoted 1 line

linux/Documentation/DocBook/v4l/controls.xml +1 -0
+	  </row>

There's more than just auto, on and off. On Logitech webcams, LEDs can be configured to blink as well.

Quoted 67 lines

</tbody>

       </tgroup>
     </table>

diff -r d8fafa7d88dc linux/Documentation/DocBook/v4l/videodev2.h.xml
--- a/linux/Documentation/DocBook/v4l/videodev2.h.xml Thu Feb 18 19:02:51
2010 +0100 +++ b/linux/Documentation/DocBook/v4l/videodev2.h.xml Sun Feb
28 08:41:17 2010 +0100 @@ -1024,8 +1024,14 @@

#define V4L2_CID_ROTATE (V4L2_CID_BASE+34)
#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+35)

linux/Documentation/DocBook/v4l/videodev2.h.xml +7 -1

+#define V4L2_CID_LED                            (V4L2_CID_BASE+36)
+enum v4l2_led {
+        V4L2_LED_AUTO           = 0,
+        V4L2_LED_ON             = 1,
+        V4L2_LED_OFF            = 2,
+};
 /* last CID + 1 */
-#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+36)
+#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+37)
 
 /*  MPEG-class control IDs defined by V4L2 */
 #define V4L2_CID_MPEG_BASE                      (V4L2_CTRL_CLASS_MPEG |

0x900) diff -r d8fafa7d88dc linux/drivers/media/video/v4l2-common.c
--- a/linux/drivers/media/video/v4l2-common.c Thu Feb 18 19:02:51 2010
+0100 +++ b/linux/drivers/media/video/v4l2-common.c Sun Feb 28 08:41:17
2010 +0100 @@ -349,6 +349,12 @@

linux/drivers/media/video/v4l2-common.c +10 -0

 		"75 useconds",
 		NULL,
 	};
+	static const char *led[] = {
+		"Auto",
+		"On",
+		"Off",
+		NULL,
+	};
 
 	switch (id) {
 		case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:

@@ -389,6 +395,8 @@
 			return colorfx;
 		case V4L2_CID_TUNE_PREEMPHASIS:
 			return tune_preemphasis;
+		case V4L2_CID_LED:
+			return led;
 		default:
 			return NULL;
 	}

@@ -434,6 +442,7 @@
 	case V4L2_CID_COLORFX:			return "Color Effects";
 	case V4L2_CID_ROTATE:			return "Rotate";
 	case V4L2_CID_BG_COLOR:			return "Background color";
+	case V4L2_CID_LED:			return "Feedback LED";
 
 	/* MPEG controls */
 	case V4L2_CID_MPEG_CLASS: 		return "MPEG Encoder Controls";

@@ -575,6 +584,7 @@
 	case V4L2_CID_EXPOSURE_AUTO:
 	case V4L2_CID_COLORFX:
 	case V4L2_CID_TUNE_PREEMPHASIS:
+	case V4L2_CID_LED:
 		qctrl->type = V4L2_CTRL_TYPE_MENU;
 		step = 1;
 		break;

diff -r d8fafa7d88dc linux/include/linux/videodev2.h

linux/include/linux/videodev2.h line 1038

Quoted 9 lines

linux/include/linux/videodev2.h +6 -0

@@ -1030,8 +1030,14 @@
 
 #define V4L2_CID_ROTATE				(V4L2_CID_BASE+34)
 #define V4L2_CID_BG_COLOR			(V4L2_CID_BASE+35)
+#define V4L2_CID_LED				(V4L2_CID_BASE+36)
+enum v4l2_led {
+	V4L2_LED_AUTO		= 0,
+	V4L2_LED_ON		= 1,
+	V4L2_LED_OFF		= 2,
+};

enums shouldn't be used in kernelspace <-> userspace APIs, as their size is not stable across ARM ABIs. In this case it won't matter much, the enum not being part of any structure. If someone creates a new ioctl that uses enum v4l2_led as one field of its structure argument, things will break.

Quoted 6 lines

linux/include/linux/videodev2.h +1 -1

 /* last CID + 1 */
-#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+36)
+#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+37)
 
 /*  MPEG-class control IDs defined by V4L2 */
 #define V4L2_CID_MPEG_BASE 			(V4L2_CTRL_CLASS_MPEG | 0x900)

Signature · 3 lines

Regards,

Laurent Pinchart