Re: RFC 2: bq2415x_charger driver

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]

On Fri, Jan 27, 2012 at 07:33:59PM +0100, Pali Rohár wrote:
> On Friday 27 January 2012 16:24:55 Mark Brown wrote:
> > On Fri, Jan 27, 2012 at 03:40:43AM +0100, Pali Roh?r wrote:

> > > +	struct i2c_client *client = to_i2c_client(bq->dev);

> > You could save a bunch of code by moving all this I2C register I/O over
> > to regmap.

> regmap is not available in 2.6.28, so I cannot use it.

You're submitting to mainline here...

In any case, it's trivial to backport - just copy the directory and
header back.

> > > +static int bq2415x_set_weak_battery_voltage(struct bq2415x_device *bq,
> > > int mV) +{
> > > +	int val = mV/100 + (mV%100 > 0 ? 1 : 0) - 34;

> > This could probably be written more clearly?

> How?

Without the ternery operator and with more spaces, probably.

> > > +		case BQ2415X_MODE_BOOST: /* Boost mode */
> > > +			dev_info(bq->dev, "mode: Boost\n");

> > Is dev_info() really appropriate for this stuff?

> How can driver write info that user (or other driver) changed chip mode?
> This should be reported in dmesg!

Why does the user care?  There's an interface for querying if they want
to know and you can generate power supply events when the mode changes.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]