lkml.org 
[lkml]   [2010]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRE: [PATCH] regulators: add support max8952 regulator
    Mark Brown wrote:
    >
    Hi,

    Thanks for your review.

    > On Fri, Sep 03, 2010 at 10:15:24AM +0900, Kukjin Kim wrote:
    >
    > > + ret2 = max8649_reg_read(info->i2c, MAX8649_CHIP_ID2);
    > > + if (ret2 < 0) {
    > > + dev_err(info->dev, "Failed to detect ID of %s:%d\n",
    > > + id->name, ret2);
    > > goto out;
    > > }
    >
    > If the read fails you'll not set an error code when you jump to out so
    > the probe will report success. It might be clearer to have separate id
    > variables which you store the read values into if they're OK, just from
    > a legibility point of view.
    >
    Ok..will fix it.

    > > - dev_info(info->dev, "Detected MAX8649 (ID:%x)\n", ret);
    > > + dev_info(info->dev, "Detected %s (ID:%x %x)\n", id->name, ret,
    ret2);
    > > +
    > > + info->type = id->driver_data;
    >
    > It'd be nice to check that the type that was supplied matches the ID
    > read from the chip in case the user got things wrong, just for
    > defensiveness.

    Ok..will modify as per your suggestion.

    Thanks.

    Best regards,
    Kgene.
    --
    Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
    SW Solution Development Team, Samsung Electronics Co., Ltd.



    \
     
     \ /
      Last update: 2010-09-03 23:51    [W:3.268 / U:0.884 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site