linux-kernel - Re: Fwd: [PATCH] bq27x00_battery: Add new properties

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]

Message-ID: <20110131160321.GA23546@oksana.dev.rtsoft.ru>
Date:	Mon, 31 Jan 2011 19:03:21 +0300
From:	Anton Vorontsov <cbouatmailru@...il.com>
To:	Grazvydas Ignotas <notasas@...il.com>,
	linux-kernel@...r.kernel.org,
	Pali Rohár <pali.rohar@...il.com>
Subject: Re: Fwd: [PATCH] bq27x00_battery: Add new properties

On Mon, Jan 31, 2011 at 04:43:56PM +0100, Rodolfo Giometti wrote:
> On Mon, Jan 31, 2011 at 04:51:47PM +0300, Anton Vorontsov wrote:
[...]
> Why not using:
> 
>    !!(gpio & 0x40)
> 
> or
> 
>    (gpio & 0x40) ? 1 : 0
> 
> ? I think it could be more readable... :)
> 
> However, I don't understand why you fix gpio=0... it's your machine
> depending settings? In these case you should use the platform_data
> mechanism to properly setup the GPIO.

I agree. It looks like a machine-specific configuration, and
exporting this GPIO as 'ONLINE' property may break other users.

Instead, I would suggest adding gpiolib support into this driver,
and passing 'int online_gpio' via platform data. That way you can
pass any GPIOs to this driver, not only bq's.

Pali, you can just split GPIO and 'ONLINE' property additions
into a separate patch for now, so we can apply the rest of the
changes, and hold on the GPIO stuff.

[...]
> > +static int bq27x00_battery_nac(struct bq27x00_device_info *di)
> 
> Let me suggest using bq27x00_battery_nac_uAh as function name...

Regarding these suffixes. I'm not sure. This seems a bit too
verbose. So I'd prefer to not introduce these.

Though, whatever you guys like... I'm fine either way.

[...]
> >  MODULE_AUTHOR("Rodolfo Giometti <giometti@...ux.it>");
> >  MODULE_DESCRIPTION("BQ27x00 battery monitor driver");
> > +MODULE_ALIAS("i2c:bq27000");
> > +MODULE_ALIAS("i2c:bq27200");
> > +MODULE_ALIAS("i2c:bq27500");

Btw, this is actually another bugfix. Pali, you might want to separate
it into yet another patch.

Thanks!

-- 
Anton Vorontsov
Email: cbouatmailru@...il.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/