lkml.org 
[lkml]   [2011]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] MFD: MAX77693: add MAX77693 MFD driver
On 2011년 12월 09일 18:58, Mark Brown wrote:
> On Fri, Dec 09, 2011 at 06:15:39PM +0900, Donggeun Kim wrote:
>
> Overall this looks good - a few small nits below.
>
>> +int max77693_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
>> +{
>> + struct max77693_dev *max77693 = i2c_get_clientdata(i2c);
>> + int ret;
>> +
>> + mutex_lock(&max77693->iolock);
>> + ret = i2c_smbus_read_byte_data(i2c, reg);
>> + mutex_unlock(&max77693->iolock);
>> + if (ret < 0)
>> + return ret;
>
> Might be worth considering regmap - should be a bit less code and the
> register cache is likely to make performance a bit better.
I'm not familiar with regmap. If I understand it fully, I will decide to
use it or not. Until then, I'd like to maintain this.
>
>> + if (max77693_read_reg(i2c, MAX77693_PMIC_REG_PMIC_ID2, &reg_data) < 0) {
>> + dev_err(max77693->dev,
>> + "device not found on this channel\n");
>> + ret = -ENODEV;
>> + goto err;
>> + }
>
> I'd suggest also verifying that the ID register has the expected value.
> If there's a chip reision register logging it can be helpful.
>
The all expected values for the register are not specified at datasheet.
The perpose of the read function is simply checking the existence of the
device so that the following operations are stopped for error case.
>> + max77693->muic = i2c_new_dummy(i2c->adapter, I2C_ADDR_MUIC);
>> + i2c_set_clientdata(max77693->muic, max77693);
>> +
>> + max77693->haptic = i2c_new_dummy(i2c->adapter, I2C_ADDR_HAPTIC);
>> + i2c_set_clientdata(max77693->haptic, max77693);
>
>> + pm_runtime_set_active(max77693->dev);
>
>
>> + kfree(max77693);
>
> devm_kzalloc().
>
Okay, it will be used in the next version of patch.

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

\
 
 \ /
  Last update: 2011-12-12 10:05    [from the cache]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and my Meterkast|Read the blog