RE: [PATCH v4] power_supply: add new lp8788 charger driver

From: Kim, Milo
Date: Thu Sep 20 2012 - 19:19:42 EST


> -----Original Message-----
> From: Anton Vorontsov [mailto:anton.vorontsov@xxxxxxxxxx]
> Sent: Friday, September 21, 2012 7:17 AM
> To: Kim, Milo
> Cc: David Woodhouse; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4] power_supply: add new lp8788 charger driver
>
> On Wed, Sep 05, 2012 at 10:43:21AM +0000, Kim, Milo wrote:
> > TI LP8788 PMU supports regulators, battery charger, RTC,
> > ADC, backlight driver and current sinks.
> >
> > This patch supports the charger operations including
> > the charger interrupt handling.
> [...]
> > Signed-off-by: Milo(Woogyom) Kim <milo.kim@xxxxxx>
> > ---
>
> 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.

>
>
> 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.

I appreciate your help.

Best Regards,
Milo

N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ 0¶ìh®å’i