[lkml]   [2012]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [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).
    [unhandled content-type:application/pgp-signature]
     \ /
      Last update: 2012-05-03 17:21    [W:0.038 / U:17.780 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site