RE: [PATCH 6/8] power: Add support for DA9150 Charger

[Opensource [Adam Thomson]]

On June 16, 2014 14:28, Lee Jones wrote:

> > +#ifndef _DA9150_CHARGER_H
> > +#define _DA9150_CHARGER_H
> 
> Two '_'s are normally preferred.

Ok, can change it accordingly.

> 
> > +#include <linux/device.h>
> > +#include <linux/i2c.h>
> 
> What are you using this for?

Nothing. Will remove them. Thanks.

> > +#include <linux/mfd/da9150/core.h>
> > +
> > +
> 
> Extra '\n' here.

Yes, just separate includes from the rest of the file. Find it more readable.
If it's an issue though I can remove it.

> > +	struct power_supply ac;
> > +	struct power_supply usb;
> > +	struct power_supply battery;
> 
> Do you want these (or pointers to these) here?
> 
> How much space do they take up?

Yes these are required in that structure. This is the common approach for
power_supply drivers. The structures, having had a very quick look, are around
maybe 70 bytes each.
N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nlj·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~†­†Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ0¶ìå