linux-kernel - Re: [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for flash LEDs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140331092640.GB4522@valkosipuli.retiisi.org.uk>
Date:	Mon, 31 Mar 2014 12:26:40 +0300
From:	Sakari Ailus <sakari.ailus@....fi>
To:	Jacek Anaszewski <j.anaszewski@...sung.com>
Cc:	linux-media@...r.kernel.org, linux-leds@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	s.nawrocki@...sung.com, a.hajda@...sung.com,
	kyungmin.park@...sung.com, Bryan Wu <cooloney@...il.com>,
	Richard Purdie <rpurdie@...ys.net>
Subject: Re: [PATCH/RFC 1/8] leds: Add sysfs and kernel internal API for
 flash LEDs

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@...il.com>
> >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@...il.com>
> >     Signed-off-by: Sakari Ailus <sakari.ailus@....fi>
> >     Signed-off-by: Mauro Carvalho Chehab <m.chehab@...sung.com>
> 
> 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@....fi	XMPP: sailus@...iisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/