Re: [PATCH v3 0/5] bq27xxx_battery data memory update

[Sebastian Reichel <sebastian.reichel@collabora.co.uk>]

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

[adding Julia to Cc for Coccinelle question]

Hi,

On Tue, Aug 29, 2017 at 10:31:57AM -0500, Andrew F. Davis wrote:
> On 08/29/2017 05:54 AM, Sebastian Reichel wrote:
> > On Wed, Aug 23, 2017 at 08:36:12PM -0700, Liam Breck wrote:
> >> Overview:
> >> * Reorganizes chip data definitions
> >> * Enables features landed in these patches:
> >>   dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation
> >>   power: supply: bq27xxx: Add chip data memory read/write support
> >>   power: supply: bq27xxx: Add power_supply_battery_info support
> >> * Supports the following chips (only BQ27425 is active)
> >>   BQ27500, 545, 425, 421, 441, 621
> >>
> >> Changes in v3:
> >> * BQ27425 tested; workaround minor chip bug
> >> * Dropped driver_version
> >> * Fixed dbg_dupes logic for .props & .dm_regs
> >> * Dropped two props array dupes
> >>
> >> Changes in v2:
> >> * Added di->opts flags for remaining chip features
> >> * Commented out untested bq27xxx_dm_regs parameters
> >> * Changed dbg_dupes to run only once
> >>
> >> Notes on v1:
> >> * Not fully tested (hence RFC tag)
> > 
> > Thanks, full series queued.
> > 
> > -- Sebastian
> > 
> 
> Hold up, I'm just now seeing this series, looks like Liam left me out of
> the CC despite being a reviewer in the MAINTAINERS file.. (possibly due
> to me actually reviewing these patches and making him fix shit). I've
> actively NACK'd some of these changes previously, making this all the
> sneakier -_-

oh, I did not notice, that you are no longer Cc'd.

> Anyway, I've not got the time to fight these changes anymore, but at
> very least could you drop 4/5, it's static analysis code made into a
> runtime check built into a kernel driver, if not at least add my
> nacked-by. :)

Since it's not critical at all and nobody depends on it, I dropped 4/5
for now. I agree, that checking it at runtime is not nice. On the other
hand I do think a duplication check makes sense. Doing a static
check should be possible, but I have no idea how to implement this
(without much effort). I suspect Coccinelle can do it, so I added
Julia.

For reference this is the runtime check:
https://patchwork.kernel.org/patch/9918953/

-- Sebastian

> >> Liam Breck (5):
> >>   power: supply: bq27xxx: Create single chip data table
> >>   power: supply: bq27xxx: Add chip IDs for previously shadowed chips
> >>   power: supply: bq27xxx: Enable data memory update for certain chips
> >>   power: supply: bq27xxx: Flag identical chip data when in debug mode
> >>   power: supply: bq27xxx: Remove duplicate chip data arrays
> >>
> >>  drivers/power/supply/bq27xxx_battery.c     | 514 ++++++++++++++---------------
> >>  drivers/power/supply/bq27xxx_battery_i2c.c |  16 +-
> >>  include/linux/power/bq27xxx_battery.h      |  10 +-
> >>  3 files changed, 265 insertions(+), 275 deletions(-)
> >>
> >> -- 
> >> 2.13.2
> >>

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