On Thu, May 03, 2012 at 03:51:08PM +0100, Mark Brown wrote: > 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? They are. The default settings (as fast a transition as possible) will probably what most people use, and if they start fiddling with the transition times they probably know what they're doing. > 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. Ok. > > > > +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). Agreed. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
