Messages in this thread | | | Date | Thu, 20 Sep 2012 16:32:46 -0700 | From | Anton Vorontsov <> | Subject | Re: [PATCH v4] power_supply: add new lp8788 charger driver |
| |
On Thu, Sep 20, 2012 at 11:19:07PM +0000, Kim, Milo wrote: [...] > > Had to fix a bunch of checkpatch complaints: > > > > CHECK: space prohibited before semicolon > > #478: FILE: drivers/power/lp8788-charger.c:396: > > + for (i = 0 ; i < pdata->num_chg_params ; i++) { > > > > CHECK: space prohibited before semicolon > > #541: FILE: drivers/power/lp8788-charger.c:459: > > + for (i = 0 ; i < pchg->num_irqs ; i++) { > > Thanks a lot for this review. > > It seems like the result of running checkpatch script with --strict option. > Should I keep this option whenever submitting the file? > I used to run w/o strict option.
It's up to you. But be aware that checkpach sometimes reports false-positives, and --strict options makes signal/noise ratio even worse.
So, checkpatch warnings (and especially with --strict options) are just hints, sometimes very bogus. CodingStyle rules have a higer priorty than automated stuff like checkpatch. :-)
> > But other than this, it looks really good, this is now applied. > > > > Obviously, I couldn't compile-test it, but hopefully it will work > > once MFD part is also in. :-) If not, you can just send a follow-up > > fix. > > Additionally, lp8788-charger depends on lp8788-adc driver. > I've found wrong calculation on getting the battery voltage and temperature. > I would submit this patch later.
Sure thing.
Thanks, Anton.
| |