linux-kernel - Re: [PATCH v2 3/4] leds: add LM3533 LED driver

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: <20120503145108.GI3955@opensource.wolfsonmicro.com>
Date:	Thu, 3 May 2012 15:51:08 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Johan Hovold <jhovold@...il.com>
Cc:	Rob Landley <rob@...dley.net>, Richard Purdie <rpurdie@...ys.net>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Jonathan Cameron <jic23@....ac.uk>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Florian Tobias Schandinat <FlorianSchandinat@....de>,
	Arnd Bergmann <arnd@...db.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-iio@...r.kernel.org, devel@...verdev.osuosl.org,
	linux-fbdev@...r.kernel.org
Subject: Re: [PATCH v2 3/4] leds: add LM3533 LED driver

On Thu, May 03, 2012 at 01:50:59PM +0200, Johan Hovold wrote:
> On Thu, May 03, 2012 at 11:43:44AM +0100, Mark Brown wrote:

> > > +		5 - 4.194 s
> > > +		6 - 8.389 s
> > > +		7 - 16.78 s

> > Shouldn't these be controlled by led_blink_set() rather than a custom
> > ABI?

> led_blink_set controls the on/off times, but the LM3533 has the two
> additional rise and fall-time settings which determine the transition
> time between these states.

Hrm.  In that case these rise times are very large - I'd expect them to
cause issues with led_set_blink() users?  Though actually I suspect the
solution here is to pull these out into the framework later; we can
probably simulate reasonably in software with a lot of brightness
variable LEDs.

> > > +What:		/sys/class/leds/<led>/max_current

> > Shouldn't this be set by platform data, the maximum current you can push
> > through the LEDs seems like a board dependant thing which won't change
> > dynamically at runtime.  The brightness can already be varied.

> I fully agree and it is possible to set via the platform data for that
> reason. The end-customer, however, insisted that even this setting be
> available through sysfs to facilitate their integration and testing.

> I'd be willing drop this attribute if requested, as it would only be used
> during integration and could easily be added back by the end-customer if
> needed.

I'd strongly suggest removing this for mainline.  If it's present it
should at least be limited to the maximum specified in platform data
(just for safety if nothing else).

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)