On Tue, 2011-06-28 at 10:24 -0400, ashishj3 wrote:
The DA9052 PMIC provides an Analogue to Digital Converter with 10 bits resolution and 10 channels.
This patch montiors the DA9052 PMIC's ADC channels mostly for battery parameters like battery temperature, junction temperature, battery current etc.
Signed-off-by: David Dajun Chen dchen@diasemi.com Signed-off-by: Ashish Jangam ashish.jangam@kpitcummins.com
[ ... ]
+static ssize_t da9052_read_vddout(struct device *dev,
struct device_attribute *devattr, char *buf)+{
struct da9052_hwmon *hwmon = dev_get_drvdata(dev);int ret, vdd = -1;mutex_lock(&hwmon->hwmon_lock);ret = da9052_enable_vddout_channel(hwmon->da9052);if (ret < 0)goto hwmon_err;ret = da9052_reg_read(hwmon->da9052, DA9052_VDD_RES_REG);if (ret < 0)pr_err("failed to read VDD_RES_REG\n");elsevdd = ret;ret = da9052_disable_vddout_channel(hwmon->da9052);if (ret < 0)goto hwmon_err;if (vdd >= 0) {mutex_unlock(&hwmon->hwmon_lock);return sprintf(buf, "%d\n", vdd);}+hwmon_err:
mutex_unlock(&hwmon->hwmon_lock);return ret;+}
This function still produces a bad result if the call to da9052_reg_read() fails and the call to da9052_disable_vddout_channel() doesn't.
I would suggest to replace pr_err() with "goto hwmon_err". If you do that, you don't need to initialize vdd, you don't need the else case for its assignment, and you don't need "if (vdd >= 0)" either.
Guenter