Re: [RFC v1 2/6] power: supply: bq27xxx: Add chip IDs for previously shadowed chips

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

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

Hi,

On Sat, Jul 08, 2017 at 07:16:56PM -0700, Liam Breck wrote:
> For the existing feature set, these chips act like ones already listed,
> so they had been given false but functional IDs. We will be adding features
> which obsolete that shadowing, so the following IDs are added:
> BQ2752X, 531, 542, 546, 742, 425, 441, 621
> 
> So as not to disturb code that relies on the shadowing,
> bq27xxx_chip_data[n].acts_like now gives the previous ID for the above
> chips. That field will no longer be necessary when the code in question can
> test for specific options (to be provided in later patches) with:
> (BQ27XXX_O_XYZ & di->opts)

As stated in the other patch it should be (di->opts & BQ27XXX_O_XYZ)

> No functional changes to the driver.
> 
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>
> [...]
>
> +	[BQ2752X]   = BQ27XXX_DATA(52x,   BQ2751X, 0),
> +	[BQ27531]   = BQ27XXX_DATA(531,   BQ27530, 0),
> +	[BQ27542]   = BQ27XXX_DATA(542,   BQ27541, 0),
> +	[BQ27546]   = BQ27XXX_DATA(546,   BQ27541, 0),
> +	[BQ27742]   = BQ27XXX_DATA(742,   BQ27541, 0),
> +	[BQ27425]   = BQ27XXX_DATA(425,   BQ27421, 0),
> +	[BQ27441]   = BQ27XXX_DATA(441,   BQ27421, 0),
> +	[BQ27621]   = BQ27XXX_DATA(621,   BQ27421, 0),

Let's get rid of the chip id mapping directly. I deleted everything
but the mapped IDs above. This is what I get for a simple git grep:

drivers/power/supply $ git grep -nE "BQ2751X|BQ27530|BQ27541"
bq27xxx_battery.c:204:  [BQ2751X] = {
bq27xxx_battery.c:384:  [BQ27530] = {
bq27xxx_battery.c:404:  [BQ27541] = {
bq27xxx_battery.c:786:  BQ27XXX_PROP(BQ2751X, bq2751x_battery_props),
bq27xxx_battery.c:795:  BQ27XXX_PROP(BQ27530, bq27530_battery_props),
bq27xxx_battery.c:796:  BQ27XXX_PROP(BQ27541, bq27541_battery_props),
bq27xxx_battery.c:1523: case BQ2751X:
bq27xxx_battery.c:1532: case BQ27541:
bq27xxx_battery.c:1535: case BQ27530:
bq27xxx_battery.c:1548: if (di->chip == BQ27530 || di->chip == BQ27421)
bq27xxx_battery_i2c.c:232:      { "bq27510", BQ2751X },
bq27xxx_battery_i2c.c:233:      { "bq27520", BQ2751X },
bq27xxx_battery_i2c.c:242:      { "bq27530", BQ27530 },
bq27xxx_battery_i2c.c:243:      { "bq27531", BQ27530 },
bq27xxx_battery_i2c.c:244:      { "bq27541", BQ27541 },
bq27xxx_battery_i2c.c:245:      { "bq27542", BQ27541 },
bq27xxx_battery_i2c.c:246:      { "bq27546", BQ27541 },
bq27xxx_battery_i2c.c:247:      { "bq27742", BQ27541 },

Ignoring the chip data defines, that are not affected, this leaves
the following affected lines:

bq27xxx_battery.c:1523: case BQ2751X:
bq27xxx_battery.c:1532: case BQ27541:
bq27xxx_battery.c:1535: case BQ27530:
bq27xxx_battery.c:1548: if (di->chip == BQ27530 || di->chip == BQ27421)

FWIW I do not follow you, that introducing the flags for
those is more error prone, than the ID mapping. If you
don't want to add the flags please just add the new IDs
to the check in bq27xxx_battery_over/undertemp().

-- Sebastian

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