Re: Fwd: [PATCH] bq27x00_battery: Add new properties

[Pali RohÃr]

2011/1/31 Anton Vorontsov <cbouatmailru@xxxxxxxxx>:
> 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.

Ok, I delete GPIO and ONLINE additions from this patch.

>
> [...]
>> > +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.

It think it is not needed to rename functions. In
Documentation/power/power_supply_class.txt is written that default
units are ÂA, ÂV, ÂAh, ... and functions in my patch return values in
these correct units.

>
> [...]
>> > ÂMODULE_AUTHOR("Rodolfo Giometti <giometti@xxxxxxxx>");
>> > Â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.

Ok, I separate aliases to new patch.

>
> Thanks!
>
> --
> Anton Vorontsov
> Email: cbouatmailru@xxxxxxxxx
>



-- 
Pali RohÃr
pali.rohar@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/