Re: [PATCH v4 6/8] power: bq27xxx_battery: add i2c bulk read/write functions

[Sebastian Reichel <sre@kernel.org>]

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

Hi,

On Sun, Jan 29, 2017 at 12:08:25PM -0800, Liam Breck wrote:
> On Sun, 2017-01-29 at 18:38:26, Sebastian Reichel wrote:
> >> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
> >> index 3f265dbf11af..581402380d6e 100644
> >> --- a/include/linux/power/bq27xxx_battery.h
> >> +++ b/include/linux/power/bq27xxx_battery.h
> >> @@ -41,6 +41,9 @@ struct bq27xxx_platform_data {
> >>  struct bq27xxx_device_info;
> >>  struct bq27xxx_access_methods {
> >>  	int (*read)(struct bq27xxx_device_info *di, u8 reg, bool single);
> >> +	int (*write)(struct bq27xxx_device_info *di, u8 reg, int value, bool single);
> >> +	int (*read_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
> >> +	int (*write_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
> >
> >So I had a look at patch 8 and I think we should start with finally
> >converting bq27xxx to regmap API and probably a second regmap for
> >the block data stuff. That way you get all the debugging info in
> >debugfs and the driver looks much cleaner.
> 										
> Could we complete this patchset using the I2C api? We haven't added to it
> extensively, and regmap is unrelated to the purpose of this patchset.

Previously we had only a read operation, which could be easily
converted to regmap, since it accesses a single register. Afterwards
we have read,write,bulk_read and bulk_write. That's 400% and 2/4
functions, which do not follow the regmap style.

So what is your definition of extensively? :)

> Also maybe the primary maintainers of BQ27xxx should be on the hook for
> that api change :-)

https://lkml.org/lkml/2015/9/23/542

The w1 stuff is not that hard to implement actually by just using
the existing functions and devm_regmap_init().

> Plus we're blocked here on devicetree support for BQ24190 waiting for
> power_supply_battery_info.

Well this patch depends on the power_supply_battery_info patch, not
the other way around. I will apply the power_supply_battery_info
patch once its ok. No need to wait for this patch. So not really a
problem for your BQ24190 work?

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]