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

From: Mark Brown
Date: Thu May 03 2012 - 06:43:46 EST


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@xxxxxxxxx>
> +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@xxxxxxxxx>
> +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@xxxxxxxxx>
> +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: signature.asc
Description: Digital signature