Hi Nikolaus,
On 07/13/2016 08:09 AM, H. Nikolaus Schaller wrote:
[...]
+ /*
+ * Kernel conventions require per-LED led-max-microamp property.
+ * But the chip does not allow to limit individual LEDs.
+ * So we take minimum from all subnodes.
Why minimum? Choose maximum and reduce max_brightness properties
of the sub-LEDs with lesser led-max-microamp.
Hm. Is this really the correct way to handle it?
Assume you connect an LED which is specified with 10mA peak current.
And another with 20mA peak current.
So you define led-max-microamp in the DT as 10 mA and 20 mA.
Firstly a user can set the brightness only to 50% of LED_FULL since it is limited
by a reduced max_brightness. And heshe finds that not all LEDs have the same
max_brightness. The first LED will have 127 and the second one 255 for reasons
not directly understandable.
You have to know that led-max-microamp property was introduced
to standarize Device Tree definition of maximum brightness allowed for
a sub-LED.
Earlier people defined max-brightness property in DT, but at
some point we realized that brightness level is not a proper unit for
describing a hardware property.
It was not global max-microamp setting that appears in recent LED
controllers that drove the introduction of led-max-microamp property.
If you question the idea of having different maximum brightness per
sub-LEDs controlled by the same device, then it means that you have
objections to the entire idea of LED subsystem max_brightness property,
whereas it has been broadly accepted and successfully used for years.
This entangles "brightness" with "max-current" - which are IMHO two independent
things.
The fact that recent LED controllers use the same notion for one of its
settings shouldn't mislead us.
Next, this will set the hardware limit to 20 mA. So there will be current peaks
of 20 mA for an LED where the DT developer thinks to have specified a led-max-microamp
of 10 mA. So you run the LED outside of its specs although the DT seems to
tell that it is inside and user-space thinks it is ok. This will reduce lifetime of LEDs.
In what circumstances would such current peaks occur? On reset?
Shouldn't such a device be considered as broken by design?
What if all LEDs would have low amperage? Would there be some way
to protect them from being damaged by current peaks?
So either "led-max-microamp" is the wrong name for this property (could better
be "led-max-average-microamp") or the whole logic is broken.
This is why we hesitate to hide (or even disable because you can't set the limit
to 10mA by DT) the current limiting chip feature by such a difficult to understand
automatic feature.
Using the minimum of all led-max-microamp keeps everything on the safe
side, running some LEDs with less current than specified. But never outside
of the spec. And all LEDs have the same max_brightness which is IMHO
more intuitive for the user.
Our original proposal was to define led-max-microamp for the whole chip
instead of individual LEDs, which is IMHO much easier to understand,
because it corresponds one-to-one with the data sheet.
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
