lkml.org 
[lkml]   [2010]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] ds2782_battery: add support for ds2786 battery gas gauge
    Hi,

    On Thu, Apr 22, 2010 at 09:52:52AM +0300, Mike Rapoport wrote:
    > From: Yulia Vilensky <vilensky@compulab.co.il>
    >
    > Signed-off-by: Yulia Vilensky <vilensky@compulab.co.il>
    > Signed-off-by: Mike Rapoport <mike@compulab.co.il>
    > ---

    Thanks for the patch, looks good overall. Few comments below.

    [...]
    > +struct ds278x_battery_ops {
    > + int (*get_temp)(struct ds278x_info *info, int *temp);
    > + int (*get_current)(struct ds278x_info *info, int *current_uA);
    > + int (*get_voltage)(struct ds278x_info *info, int *voltage_uA);
    > + int (*get_capacity)(struct ds278x_info *info, int *capacity_uA);
    > +

    Unneeded empty line.

    [...]
    >
    > - info->battery.name = kasprintf(GFP_KERNEL, "ds2782-%d", num);
    > + info->battery.name = kasprintf(GFP_KERNEL, "ds278x-%d", num);

    I'm a bit worried about this, as this will change sysfs stuff.
    I tend to think that this won't cause any real issues, but just
    to be on a safe side, can we change that to
    kasprintf(GFP_KERNEL, "%s-%d", client->name, num); ?

    > if (!info->battery.name) {
    > ret = -ENOMEM;
    > goto fail_name;
    > @@ -277,7 +399,10 @@ static int ds2782_battery_probe(struct i2c_client *client,
    >
    > i2c_set_clientdata(client, info);
    > info->client = client;
    > - ds2782_power_supply_init(&info->battery);
    > + info->id = num;
    > + info->ops = &ds278x_ops[id->driver_data];
    > + info->rsns = *((int *)info->client->dev.platform_data);

    Please introduce 'struct ds278x_platform_data {int rsns;};'
    for this. Place it into include/linux/ds2782_battery.h.

    Thanks!

    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2


    \
     
     \ /
      Last update: 2010-04-22 09:17    [W:5.922 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site