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

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

On Thu, May 03, 2012 at 12:26:38PM +0200, Johan Hovold wrote:

> +What:		/sys/class/leds/<led>/risetime
> +Date:		April 2012
> +KernelVersion:	3.5
> +Contact:	Johan Hovold <jhovold@gmail.com>
> +Description:
> +		Set the pattern generator fall and rise times (0..7), where
> +
> +		0 - 2048 us
> +		1 - 262 ms
> +		2 - 524 ms
> +		3 - 1.049 s
> +		4 - 2.097 s
> +		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?

> +What:		/sys/class/leds/<led>/id
> +Date:		April 2012
> +KernelVersion:	3.5
> +Contact:	Johan Hovold <jhovold@gmail.com>
> +Description:
> +		Get the id of this led (0..3).
> +

This should just be a generic LED subsystem thing?

> +What:		/sys/class/leds/<led>/max_current
> +Date:		April 2012
> +KernelVersion:	3.5
> +Contact:	Johan Hovold <jhovold@gmail.com>
> +Description:
> +		Set the full-scale current I_{LED_FULLSCALE} (0..31), where
> +
> +		I_{LED_FULLSCALE} = 5mA + max_current * 0.8mA
> +

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.

It'd also be nicer if the kernel did the calculation for the user.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]