lkml.org 
[lkml]   [2011]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRE: [PATCH 2/3] power: Add LP8727 charger driver
    Hi, Anton Vorontsov

    Thank you for your comments.
    I send the revised patch.

    Signed-off-by: Woogyom Kim <milo.kim@ti.com>

    From 0a7658fe57ca196db6e22d1a394c3286bec19951 Mon Sep 17 00:00:00 2001
    From: Milo(Woogyom) Kim <milo.kim@ti.com>
    Date: Thu, 13 Oct 2011 14:53:27 +0900
    Subject: [PATCH] power: lp8727 coding style changes

    1. Useless braces were omitted
    2. Useless void casts were omitted
    3. module exit name changed
    lp8727_chg_exit -> lp8727_exit
    4. Pointer coding style changes
    no space between pointer('*') and pointer name
    ex) u8 * data -> u8 *data
    5. Author information change : email and additional author
    ---
    drivers/power/lp8727_charger.c | 50 ++++++++++++++++++++--------------------
    1 files changed, 25 insertions(+), 25 deletions(-)

    diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
    index 2a649e0..6d13f83 100755
    --- a/drivers/power/lp8727_charger.c
    +++ b/drivers/power/lp8727_charger.c
    @@ -91,7 +91,7 @@ struct lp8727_chg {
    enum lp8727_dev_id devid;
    };

    -static int lp8727_i2c_read(struct lp8727_chg *pchg, u8 reg, u8 * data, u8 len)
    +static int lp8727_i2c_read(struct lp8727_chg *pchg, u8 reg, u8 *data, u8 len)
    {
    s32 ret;

    @@ -102,7 +102,7 @@ static int lp8727_i2c_read(struct lp8727_chg *pchg, u8 reg, u8 * data, u8 len)
    return (ret != len) ? -EIO : 0;
    }

    -static int lp8727_i2c_write(struct lp8727_chg *pchg, u8 reg, u8 * data, u8 len)
    +static int lp8727_i2c_write(struct lp8727_chg *pchg, u8 reg, u8 *data, u8 len)
    {
    s32 ret;

    @@ -114,13 +114,13 @@ static int lp8727_i2c_write(struct lp8727_chg *pchg, u8 reg, u8 * data, u8 len)
    }

    static inline int lp8727_i2c_read_byte(struct lp8727_chg *pchg, u8 reg,
    - u8 * data)
    + u8 *data)
    {
    return lp8727_i2c_read(pchg, reg, data, 1);
    }

    static inline int lp8727_i2c_write_byte(struct lp8727_chg *pchg, u8 reg,
    - u8 * data)
    + u8 *data)
    {
    return lp8727_i2c_write(pchg, reg, data, 1);
    }
    @@ -153,21 +153,21 @@ static void lp8727_init_device(struct lp8727_chg *pchg)
    static int lp8727_is_dedicated_charger(struct lp8727_chg *pchg)
    {
    u8 val;
    - (void)lp8727_i2c_read_byte(pchg, STATUS1, &val);
    + lp8727_i2c_read_byte(pchg, STATUS1, &val);
    return (val & DCPORT);
    }

    static int lp8727_is_usb_charger(struct lp8727_chg *pchg)
    {
    u8 val;
    - (void)lp8727_i2c_read_byte(pchg, STATUS1, &val);
    + lp8727_i2c_read_byte(pchg, STATUS1, &val);
    return (val & CHPORT);
    }

    static void lp8727_ctrl_switch(struct lp8727_chg *pchg, u8 sw)
    {
    u8 val = sw;
    - (void)lp8727_i2c_write_byte(pchg, SWCTRL, &val);
    + lp8727_i2c_write_byte(pchg, SWCTRL, &val);
    }

    static void lp8727_id_detection(struct lp8727_chg *pchg, u8 id, int vbusin)
    @@ -207,9 +207,9 @@ static void lp8727_enable_chgdet(struct lp8727_chg *pchg)
    {
    u8 val;

    - (void)lp8727_i2c_read_byte(pchg, CTRL2, &val);
    + lp8727_i2c_read_byte(pchg, CTRL2, &val);
    val |= CHGDET_EN;
    - (void)lp8727_i2c_write_byte(pchg, CTRL2, &val);
    + lp8727_i2c_write_byte(pchg, CTRL2, &val);
    }

    static void lp8727_delayed_func(struct work_struct *_work)
    @@ -283,10 +283,9 @@ static int lp8727_charger_get_property(struct power_supply *psy,
    {
    struct lp8727_chg *pchg = dev_get_drvdata(psy->dev->parent);

    - if (psp == POWER_SUPPLY_PROP_ONLINE) {
    + if (psp == POWER_SUPPLY_PROP_ONLINE)
    val->intval = lp8727_is_charger_attached(psy->name,
    pchg->devid);
    - }

    return 0;
    }
    @@ -301,7 +300,7 @@ static int lp8727_battery_get_property(struct power_supply *psy,
    switch (psp) {
    case POWER_SUPPLY_PROP_STATUS:
    if (lp8727_is_charger_attached(psy->name, pchg->devid)) {
    - (void)lp8727_i2c_read_byte(pchg, STATUS1, &read);
    + lp8727_i2c_read_byte(pchg, STATUS1, &read);
    if (((read & CHGSTAT) >> 4) == EOC)
    val->intval = POWER_SUPPLY_STATUS_FULL;
    else
    @@ -311,7 +310,7 @@ static int lp8727_battery_get_property(struct power_supply *psy,
    }
    break;
    case POWER_SUPPLY_PROP_HEALTH:
    - (void)lp8727_i2c_read_byte(pchg, STATUS2, &read);
    + lp8727_i2c_read_byte(pchg, STATUS2, &read);
    read = (read & TEMP_STAT) >> 5;
    if (read >= 0x1 && read <= 0x3)
    val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
    @@ -352,7 +351,7 @@ static void lp8727_charger_changed(struct power_supply *psy)
    eoc_level = pchg->chg_parm->eoc_level;
    ichg = pchg->chg_parm->ichg;
    val = (ichg << 4) | eoc_level;
    - (void)lp8727_i2c_write_byte(pchg, CHGCTRL2, &val);
    + lp8727_i2c_write_byte(pchg, CHGCTRL2, &val);
    }
    }
    }
    @@ -412,12 +411,13 @@ static void lp8727_unregister_psy(struct lp8727_chg *pchg)
    {
    struct lp8727_psy *psy = pchg->psy;

    - if (psy) {
    - power_supply_unregister(&psy->ac);
    - power_supply_unregister(&psy->usb);
    - power_supply_unregister(&psy->batt);
    - kfree(psy);
    - }
    + if (!psy)
    + return;
    +
    + power_supply_unregister(&psy->ac);
    + power_supply_unregister(&psy->usb);
    + power_supply_unregister(&psy->batt);
    + kfree(psy);
    }

    static int lp8727_probe(struct i2c_client *cl, const struct i2c_device_id *id)
    @@ -440,10 +440,9 @@ static int lp8727_probe(struct i2c_client *cl, const struct i2c_device_id *id)
    lp8727_intr_config(pchg);

    ret = lp8727_register_psy(pchg);
    - if (ret) {
    + if (ret)
    dev_err(pchg->dev,
    "can not register power supplies. err=%d", ret);
    - }

    return 0;
    }
    @@ -478,14 +477,15 @@ static int __init lp8727_init(void)
    return i2c_add_driver(&lp8727_driver);
    }

    -static void __exit lp8727_chg_exit(void)
    +static void __exit lp8727_exit(void)
    {
    i2c_del_driver(&lp8727_driver);
    }

    module_init(lp8727_init);
    -module_exit(lp8727_chg_exit);
    +module_exit(lp8727_exit);

    MODULE_DESCRIPTION("National Semiconductor LP8727 charger driver");
    -MODULE_AUTHOR("Woogyom Kim <milo.kim@nsc.com>");
    +MODULE_AUTHOR
    + ("Woogyom Kim <milo.kim@ti.com>, Daniel Jeong <daniel.jeong@ti.com>");
    MODULE_LICENSE("GPL");
    --
    1.7.4.1
    Thanks & BR
    Milo (Woogyom) Kim
    milo.kim@ti.com

    -----Original Message-----
    From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
    Sent: Thursday, October 13, 2011 4:49 AM
    To: Kim, Milo
    Cc: linux-kernel@vger.kernel.org; dwmw2@infradead.org
    Subject: Re: [PATCH 2/3] power: Add LP8727 charger driver
    Hello Kim,

    Sorry for the delay.

    On Wed, Sep 07, 2011 at 01:55:27AM -0700, Kim, Milo wrote:
    >
    > diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
    > new file mode 100755
    > index 0000000..2a649e0

    Note that each patch should have commit message (ideally), and a
    Signed-off-by: line.

    Though, I would fixup all the issues myself, if the patch had at
    least Signed-off-by. :-)

    But while you're at it, you might also want to fix some cosmetic
    issues:

    [...]
    > +static int lp8727_is_dedicated_charger(struct lp8727_chg *pchg)
    > +{
    > + u8 val;
    > + (void)lp8727_i2c_read_byte(pchg, STATUS1, &val);

    No need for these casts, please.

    > + return (val & DCPORT);
    > +}
    [...]
    > +static int lp8727_charger_get_property(struct power_supply *psy,
    > + enum power_supply_property psp,
    > + union power_supply_propval *val)
    > +{
    > + struct lp8727_chg *pchg = dev_get_drvdata(psy->dev->parent);
    > +
    > + if (psp == POWER_SUPPLY_PROP_ONLINE) {
    > + val->intval = lp8727_is_charger_attached(psy->name,
    > + pchg->devid);
    > + }

    No need for brackets here.

    > +
    > + return 0;
    > +}
    > +
    [...]
    > +static void lp8727_unregister_psy(struct lp8727_chg *pchg)
    > +{
    > + struct lp8727_psy *psy = pchg->psy;
    > +
    > + if (psy) {

    if (!psy)
    return;
    > + power_supply_unregister(&psy->ac);
    > + power_supply_unregister(&psy->usb);
    > + power_supply_unregister(&psy->batt);
    > + kfree(psy);
    > + }
    > +}
    > +
    > +static int lp8727_probe(struct i2c_client *cl, const struct i2c_device_id *id)
    > +{
    > + struct lp8727_chg *pchg;
    > + int ret;
    > +
    > + pchg = kzalloc(sizeof(*pchg), GFP_KERNEL);
    > + if (!pchg)
    > + return -ENOMEM;
    > +
    > + pchg->client = cl;
    > + pchg->dev = &cl->dev;
    > + pchg->pdata = cl->dev.platform_data;
    > + i2c_set_clientdata(cl, pchg);
    > +
    > + mutex_init(&pchg->xfer_lock);
    > +
    > + lp8727_init_device(pchg);
    > + lp8727_intr_config(pchg);
    > +
    > + ret = lp8727_register_psy(pchg);
    > + if (ret) {
    > + dev_err(pchg->dev,
    > + "can not register power supplies. err=%d", ret);

    No need for brackets.

    > + }
    > +
    > + return 0;
    > +}
    > +
    > +static int __devexit lp8727_remove(struct i2c_client *cl)
    > +{
    > + struct lp8727_chg *pchg = i2c_get_clientdata(cl);
    > +
    > + lp8727_unregister_psy(pchg);
    > + free_irq(pchg->client->irq, pchg);
    > + flush_workqueue(pchg->irqthread);
    > + destroy_workqueue(pchg->irqthread);
    > + kfree(pchg);
    > + return 0;
    > +}
    > +
    > +static const struct i2c_device_id lp8727_ids[] = {
    > + {"lp8727", 0},
    > +};
    > +
    > +static struct i2c_driver lp8727_driver = {
    > + .driver = {
    > + .name = "lp8727",
    > + },
    > + .probe = lp8727_probe,
    > + .remove = __devexit_p(lp8727_remove),
    > + .id_table = lp8727_ids,
    > +};
    > +
    > +static int __init lp8727_init(void)
    > +{
    > + return i2c_add_driver(&lp8727_driver);
    > +}
    > +
    > +static void __exit lp8727_chg_exit(void)
    > +{
    > + i2c_del_driver(&lp8727_driver);
    > +}
    > +
    > +module_init(lp8727_init);
    > +module_exit(lp8727_chg_exit);
    > +
    > +MODULE_DESCRIPTION("National Semiconductor LP8727 charger driver");
    > +MODULE_AUTHOR("Woogyom Kim <milo.kim@nsc.com>");
    > +MODULE_LICENSE("GPL");
    >

    --
    Anton Vorontsov
    Email: cbouatmailru@gmail.com

    \
     
     \ /
      Last update: 2011-10-13 08:09    [W:0.043 / U:4.584 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site