Hi,
Am 14.07.2016 um 23:55 schrieb Andrey Utkin andrey_utkin@fastmail.com:
On Thu, Jul 14, 2016 at 07:15:19PM +0200, H. Nikolaus Schaller wrote:
Have tested a little. I think we have broken something. The LEDs remain dark.
Oops.
One thing we also need to fix is the bindings (still talks about private led-max-current).
I don't understand what you mean here.
I will fix that. It is like a typo and fixing is easier than explaining. I just didn't have time yesterday to fix, commit and push it.
And I think about modifying
static int is31fl319x_microamp_to_cs(struct device *dev, u32 microamp) { /* invert and round down to nearest value (range check done by caller) */ const u32 max = IS31FL319X_CURRENT_MAX + IS31FL319X_CURRENT_DEFAULT + IS31FL319X_CURRENT_STEP - 1; return ((max - microamp) / IS31FL319X_CURRENT_STEP) % 8; }
Ok. But IMO CS register values map is so irregular that it requires documentation. I would add it in comment to this function.
Yes, indeed. I am not happy with as well how the formula looks like.
Maybe it is better to start with microamp / IS31FL319X_CURRENT_STEP (which is 0..n) and then apply the inversion trick i.e. (12-value)%8. And add a comment.
The reason is if we really specify the max. LED current there should be no limit reported on it. If it is 3 mA it is 3 mA. If it is 100 mA it is 100 mA.
Again I don't understand what you want to say.
So the best effort the driver can do is to take the next lower available current limit.
This is currently done in line 195:
led->max_microamp -= led->max_microamp % IS31FL319X_CURRENT_STEP;
but calculated for each LED. Could be done for all in total.
Well, we still have a problem <5mA... And should probably turn off completely? Or in that case make probing fail.
I'd leave it off, emitting a runtime warning.
probe failing is probably better since turning off all LEDs needs another flag because the brightness setter must know that after successful probing. And it shows a design issue which should not happen in practice.
I will have to take a deeper look what is wrong, but not before tomorrow morning.
I'd like to fix it myself, but just staring at diff feels not productive. It's hard to solve this telepathically. What about SSH access for me and USB webcam connected to Pyra itself and directed towards LEDs?
Well, setting this up is more complex for me than adding one or two printk and fixing it :) And I should at least look at dmesg or /sys/class/leds. Maybe the max_brightness remains 0.
So I missed to provide a good bug report.
BR, Nikolaus