Hi Jacek, On Fri, Mar 28, 2014 at 04:30:19PM +0100, Jacek Anaszewski wrote: > Hi Sakari, > > Thanks for the review. > > On 03/24/2014 12:18 AM, Sakari Ailus wrote: > >Hi Jacek, > > > >Thanks for the patchset. It's very nice in general. I have a few comments > >below. > > [...] > > >>diff --git a/include/linux/leds.h b/include/linux/leds.h > >>index 0287ab2..1bf0ab3 100644 > >>--- a/include/linux/leds.h > >>+++ b/include/linux/leds.h > >>@@ -17,6 +17,14 @@ > >> #include <linux/rwsem.h> > >> #include <linux/timer.h> > >> #include <linux/workqueue.h> > >>+#include <linux/mutex.h> > > > >mutex.h should be earlier in the list of included files. > > > >>+#include <media/v4l2-device.h> > >>+ > >>+#define LED_FAULT_OVER_VOLTAGE (1 << 0) > >>+#define LED_FAULT_TIMEOUT (1 << 1) > >>+#define LED_FAULT_OVER_TEMPERATURE (1 << 2) > >>+#define LED_FAULT_SHORT_CIRCUIT (1 << 3) > >>+#define LED_FAULT_OVER_CURRENT (1 << 4) > > > >This patch went in to the media-tree some time ago. I wonder if the relevant > >bits should be added here now as well. > > > >commit 935aa6b2e8a911e81baecec0537dd7e478dc8c91 > >Author: Daniel Jeong <gshark.jeong@xxxxxxxxx> > >Date: Mon Mar 3 06:52:08 2014 -0300 > > > > [media] v4l2-controls.h: Add addtional Flash fault bits > > > > Three Flash fault are added. V4L2_FLASH_FAULT_UNDER_VOLTAGE for the case low > > voltage below the min. limit. V4L2_FLASH_FAULT_INPUT_VOLTAGE for the case > > falling input voltage and chip adjust flash current not occur under voltage > > event. V4L2_FLASH_FAULT_LED_OVER_TEMPERATURE for the case the temperature > > exceed the maximun limit > > > > Signed-off-by: Daniel Jeong <gshark.jeong@xxxxxxxxx> > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx> > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@xxxxxxxxxxx> > > As it will not cause a build break and any runtime problems, even if > the patch is not merged, I added these bits to my implementation. > > BTW I have doubts about V4L2_FLASH_FAULT_INDICATOR and > V4L2_CID_FLASH_INDICATOR_INTENSITY control. I did not take them > into account in my implementation because it is not clear for > me how an indicator led is related to a torch led. There is > a control for setting indicator intensity but there is not > one for enabling it. Could you shed some light on this issue? The indicator isn't related to the torch mode in any way. Some flash controllers contain an additional output for indicator, also called privacy led. These faults are related to that. -- Regards, Sakari Ailus e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
