Re: [PATCH v2 5/6] power: bq27xxx_battery: Add support for additional bq27xxx family devices

["Pali Rohár" <pali.rohar@gmail.com>]

[-- Attachment #1: Type: Text/Plain, Size: 2070 bytes --]

On Tuesday 28 July 2015 21:39:12 Andrew F. Davis wrote:
> Add support for additional devices and register equivalent family
> devices including the bq27010, bq27210, bq272500, bq27510, bq27520,
                                            ^^^^^^
                                            maybe incorrect number?
> bq27530, bq27531, bq27541, bq27542, bq27546, bq27545, bq27441,
> bq27421, and the bq27641.
> 
> To facilitate this process the register mapings have been moved to
> tables and other small cleanups have been made.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---

>  static const struct i2c_device_id bq27xxx_id[] = {
> -	{ "bq27200", BQ27000 },	/* bq27200 is same as bq27000, but with i2c
> */ +	{ "bq27200", BQ27000 },
> +	{ "bq27210", BQ27010 },
>  	{ "bq27500", BQ27500 },
> -	{ "bq27425", BQ27425 },
> -	{ "bq27742", BQ27742 },
> -	{ "bq27510", BQ27510 },
> +	{ "bq27510", BQ27500 },
> +	{ "bq27520", BQ27500 },
> +	{ "bq27530", BQ27530 },
> +	{ "bq27531", BQ27530 },
> +	{ "bq27541", BQ27541 },
> +	{ "bq27542", BQ27541 },
> +	{ "bq27546", BQ27541 },
> +	{ "bq27742", BQ27541 },
> +	{ "bq27545", BQ27545 },
> +	{ "bq27421", BQ27421 },
> +	{ "bq27425", BQ27421 },
> +	{ "bq27441", BQ27421 },
> +	{ "bq27621", BQ27421 },
>  	{},
>  };

I'm trying to understand comparator which you used for sorting these 
values... but I do not see any logic here. What about sorting list by 
first value? Now it is long list of device names and for better ability 
to read it, it would be better to have it in some order.

> -enum bq27xxx_chip { BQ27000, BQ27500, BQ27425, BQ27742, BQ27510 };
> +enum bq27xxx_chip {
> +	BQ27000, /* bq27000, bq27200 */
> +	BQ27010, /* bq27010, bq27210 */
> +	BQ27500, /* bq27500, bq27510, bq27520 */
> +	BQ27530, /* bq27530, bq27531 */
> +	BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
> +	BQ27545, /* bq27545 */
> +	BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
> +};
> 

Maybe same here? Putting BQ27421 before BQ27500?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]