Hi Andrey,
Am 08.07.2016 um 08:43 schrieb H. Nikolaus Schaller hns@goldelico.com:
Hi Andrey,
Am 07.07.2016 um 23:47 schrieb Andrey Utkin andrey_utkin@fastmail.com:
On Thu, Jul 07, 2016 at 08:47:34PM +0300, Andrey Utkin wrote:
On Thu, Jul 07, 2016 at 02:39:49PM +0200, H. Nikolaus Schaller wrote:
Hi Andrey, I think this is a good point where you can jump in and help to edit the driver patch.
Would that be ok?
I have prepared a -v3 branch (I have a GUI tool where I just needed one click to do that):
http://git.goldelico.com/?p=gta04-kernel.git;a=shortlog;h=refs/heads/work/hn...
Ok, jumping in. Thanks for turning to me for this. Expect some results in few hours.
A draft for the Changes note for [PATCH 0/n]:
Changes V3:
- coding style improvements
Done (something).
- added a mutex to properly serialize multiple calls to set_backlight
Done.
- use regmap defaults
TODO.
- improve DT parsing
In progress.
I think we can just pile up fixes and finally merge them into the first commit.
I hope I've fixed all points of review.
Not only those, but 200% more :)
I will look through them and comment (if necessary).
Please check my changes in work/andrey/leds/is31fl319x branch. http://git.goldelico.com/?p=gta04-kernel.git;a=shortlog;h=refs/heads/work/an...
Interesting that your checkpatch did report more than mine...
The last one, "led: is31fl319x: Replace all tabs in the middle of lines with spaces", may be controversal, but I believe it is right thing to do (i.e. to have tabs in the middle of lines is wrong thing to do). Honestly, I have an idea that indentation style currently enforced by checkpatch.pl is defective regarding abuse of tabs, and discouragement of spaces in exact places where they fit better than tabs. But that's unrelated; I think I'll raise this discussion in LKML some day :)
Yes, even tools like checkpatch are not perfect. For example the warning about missing Kconfig description. I had simply ignored that warning...
What is important is that checkpatch.pl likes how the driver looks after all of my changes.
I will merge & compile & test asap and if ok resubmit.
Seems to work with no noticeable differences. Maybe because we are not using microcampere settings at all. So I can't test bugs here...
Generally I like the approach to pass the struct is31fl319x_led into the parse_dt().
One thing I don't understand exactly is why you did replace the formula by a switch in is31fl319x_microamp_to_cs. The formula is designed to accept any value and never report "invalid microamperes" but simply round down to the next available value.
Since we don't use it for the Pyra (and maybe never will need it), I don't really care how this functionality will evolve in the future...
Another minor thing is that I think we should also add IS31FL319X_CTRL1 and IS31FL319X_CTRL2 to the reg_default setup.
I will add that myself, squeeze everything into the original patches, add a description of your changes to the patch 0/n message and resubmit.
BR and thanks!
Nikolaus
Thanks and BR, Nikolaus
Kernel mailing list Kernel@pyra-handheld.com http://pyra-handheld.com/cgi-bin/mailman/listinfo/kernel