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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 28 Jul 2015 21:59:29 +0200
From:	Pali Rohár <pali.rohar@...il.com>
To:	"Andrew F. Davis" <afd@...com>
Cc:	Sebastian Reichel <sre@...nel.org>,
	"Dmitry Eremin-Solenikov" <dbaryshkov@...il.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Dan Murphy <dmurphy@...com>, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/6] power: bq27xxx_battery: Add support for additional bq27xxx family devices

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@...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@...il.com

Download attachment "signature.asc " of type "application/pgp-signature" (199 bytes)