Messages in this thread |  | | Date | Fri, 9 Dec 2011 09:58:56 +0000 | | From | Mark Brown <> | | Subject | Re: [PATCH 1/2] MFD: MAX77693: add MAX77693 MFD driver |
| |
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.
> + if (max77693_read_reg(i2c, MAX77693_PMIC_REG_PMIC_ID2, ®_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.
> + 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().
|  |