On Tue, 5 Sep 2017, Liam Breck wrote:

Hi Julia, any luck on this?

I was trying to improve the parser, but it seems that my improvements
don't have a sufficient effect so I will try your suggestion of just
seding out the offending code.

julia

On Thu, Aug 31, 2017 at 4:43 AM, Liam Breck [off-list ref] wrote:

quoted

On Thu, Aug 31, 2017 at 4:33 AM, Julia Lawall [off-list ref] wrote:

quoted

On Thu, 31 Aug 2017, Liam Breck wrote:

quoted

On Thu, Aug 31, 2017 at 2:54 AM, Julia Lawall [off-list ref] wrote:

quoted

On Tue, 29 Aug 2017, Liam Breck wrote:

quoted

Coccinelle folks,

On Tue, Aug 29, 2017 at 5:29 PM, Sebastian Reichel
[off-list ref] wrote:

quoted

Hi,

On Tue, Aug 29, 2017 at 04:07:12PM -0700, Liam Breck wrote:

quoted

I don't see a Julia in CC list...

<_< let's try that again.

quoted

On Tue, Aug 29, 2017 at 2:22 PM, Sebastian Reichel
[off-list ref] wrote:

quoted

[adding Julia to Cc for Coccinelle question]

Hi,

On Tue, Aug 29, 2017 at 10:31:57AM -0500, Andrew F. Davis wrote:

quoted

On 08/29/2017 05:54 AM, Sebastian Reichel wrote:

quoted

On Wed, Aug 23, 2017 at 08:36:12PM -0700, Liam Breck wrote:

quoted

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

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/

The data structures being checked start here:
https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/tree/drivers/power/supply/bq27xxx_battery.c?h=for-next#n138

And are aggregated here:
https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/tree/drivers/power/supply/bq27xxx_battery.c?h=for-next#n743

I worked a bit on this, but unfortunately there is a major parsing
problem, and most of the definitions are ignored.  Specifically, the
definitions of the regs variables are interspersed with eg:

#define bq27510g1_regs bq27500_regs

You can transform the macros with sed to either of:

static u8 *bq27510g1_regs = 0 // skip comparison if x == 0

static u8 bq27510g1_regs[BQ27XXX_REG_MAX] = { 0xFF } // skip
comparison if x[0] == 0xff

Does that help?

Not quite, because it's really a list of variable declarations, like

int a, b, c, d;

The following could be fine:

*bq27510g1_regs = 0,

i forgot about the static u8 at the top of the array set. But yes,
just drop static u8 from either of the alternatives I gave.