[PATCH v2 1/3] hwmon: (lm90) split set&show temp as common codes
Split set&show temp codes as common functions, so we can use it directly when implement linux thermal framework. Signed-off-by: Wei Ni --- drivers/hwmon/lm90.c | 118 -- 1 file changed, 75 insertions(+), 43 deletions(-) diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c index 8eeb141..2cb7f8e 100644 --- a/drivers/hwmon/lm90.c +++ b/drivers/hwmon/lm90.c @@ -702,29 +702,37 @@ static u16 temp_to_u16_adt7461(struct lm90_data *data, long val) * Sysfs stuff */ -static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, - char *buf) +static int read_temp8(struct device *dev, int index) { - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); - struct lm90_data *data = lm90_update_device(dev); int temp; + struct lm90_data *data = lm90_update_device(dev); if (data->kind == adt7461) - temp = temp_from_u8_adt7461(data, data->temp8[attr->index]); + temp = temp_from_u8_adt7461(data, data->temp8[index]); else if (data->kind == max6646) - temp = temp_from_u8(data->temp8[attr->index]); + temp = temp_from_u8(data->temp8[index]); else - temp = temp_from_s8(data->temp8[attr->index]); + temp = temp_from_s8(data->temp8[index]); /* +16 degrees offset for temp2 for the LM99 */ - if (data->kind == lm99 && attr->index == 3) + if (data->kind == lm99 && index == 3) temp += 16000; + return temp; +} + +static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, + char *buf) +{ + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); + int temp; + + temp = read_temp8(dev, attr->index); + return sprintf(buf, "%d\n", temp); } -static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr, -const char *buf, size_t count) +static void write_temp8(struct device *dev, int index, long val) { static const u8 reg[8] = { LM90_REG_W_LOCAL_LOW, @@ -737,60 +745,76 @@ static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr, MAX6659_REG_W_REMOTE_EMERG, }; - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct i2c_client *client = to_i2c_client(dev); struct lm90_data *data = i2c_get_clientdata(client); - int nr = attr->index; - long val; - int err; - - err = kstrtol(buf, 10, &val); - if (err < 0) - return err; /* +16 degrees offset for temp2 for the LM99 */ - if (data->kind == lm99 && attr->index == 3) + if (data->kind == lm99 && index == 3) val -= 16000; mutex_lock(&data->update_lock); if (data->kind == adt7461) - data->temp8[nr] = temp_to_u8_adt7461(data, val); + data->temp8[index] = temp_to_u8_adt7461(data, val); else if (data->kind == max6646) - data->temp8[nr] = temp_to_u8(val); + data->temp8[index] = temp_to_u8(val); else - data->temp8[nr] = temp_to_s8(val); + data->temp8[index] = temp_to_s8(val); - lm90_select_remote_channel(client, data, nr >= 6); - i2c_smbus_write_byte_data(client, reg[nr], data->temp8[nr]); + lm90_select_remote_channel(client, data, index >= 6); + i2c_smbus_write_byte_data(client, reg[index], data->temp8[index]); lm90_select_remote_channel(client, data, 0); mutex_unlock(&data->update_lock); +} + +static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr, +const char *buf, size_t count) +{ + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); + int index = attr->index; + long val; + int err; + + err = kstrtol(buf, 10, &val); + if (err < 0) + return err; + + write_temp8(dev, index, val); + return count; } -static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr, - char *buf) +static int read_temp11(struct device *dev, int index) { - struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr); - struct lm90_data *data = lm90_update_device(dev); int temp; + struct lm90_data *data = lm90_update_device(dev); if (data->kind == adt7461) - temp = temp_from_u16_adt7461(data, data->temp11[attr->index]); + temp = temp_from_u16_adt7461(data, data->temp11[index]); else if (data->kind == max6646) - temp = temp_from_u16(data->temp11[attr->index]); + temp = temp_from_u16(data->temp11[index]); else - temp = temp_from_s16(data->temp11[attr->index]); +
[PATCH v2 1/3] hwmon: (lm90) split set&show temp as common codes
Split set&show temp codes as common functions, so we can use it directly when implement linux thermal framework. And handle error return value for the lm90_select_remote_channel and write_tempx, then set_temp8 and set_temp11 could return it to user-space. Change-Id: Id7178f680718fd54b9ba953118d91052b59b692e Signed-off-by: Wei Ni Signed-off-by: Jean Delvare --- drivers/hwmon/lm90.c | 164 +- 1 file changed, 109 insertions(+), 55 deletions(-) diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c index c9ff08d..cb33dcf 100644 --- a/drivers/hwmon/lm90.c +++ b/drivers/hwmon/lm90.c @@ -473,20 +473,23 @@ static int lm90_read16(struct i2c_client *client, u8 regh, u8 regl, u16 *value) * various registers have different meanings as a result of selecting a * non-default remote channel. */ -static inline void lm90_select_remote_channel(struct i2c_client *client, - struct lm90_data *data, - int channel) +static inline int lm90_select_remote_channel(struct i2c_client *client, +struct lm90_data *data, +int channel) { u8 config; + int err = 0; if (data->kind == max6696) { lm90_read_reg(client, LM90_REG_R_CONFIG1, &config); config &= ~0x08; if (channel) config |= 0x08; - i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, - config); + err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, + config); } + + return err; } /* @@ -759,29 +762,34 @@ static u16 temp_to_u16_adt7461(struct lm90_data *data, long val) * Sysfs stuff */ -static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, - char *buf) +static int read_temp8(struct device *dev, int index) { - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct lm90_data *data = lm90_update_device(dev); int temp; if (data->kind == adt7461 || data->kind == tmp451) - temp = temp_from_u8_adt7461(data, data->temp8[attr->index]); + temp = temp_from_u8_adt7461(data, data->temp8[index]); else if (data->kind == max6646) - temp = temp_from_u8(data->temp8[attr->index]); + temp = temp_from_u8(data->temp8[index]); else - temp = temp_from_s8(data->temp8[attr->index]); + temp = temp_from_s8(data->temp8[index]); /* +16 degrees offset for temp2 for the LM99 */ - if (data->kind == lm99 && attr->index == 3) + if (data->kind == lm99 && index == 3) temp += 16000; - return sprintf(buf, "%d\n", temp); + return temp; } -static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr, -const char *buf, size_t count) +static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, + char *buf) +{ + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); + + return sprintf(buf, "%d\n", read_temp8(dev, attr->index)); +} + +static int write_temp8(struct device *dev, int index, long val) { static const u8 reg[TEMP8_REG_NUM] = { LM90_REG_W_LOCAL_LOW, @@ -794,60 +802,79 @@ static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr, MAX6659_REG_W_REMOTE_EMERG, }; - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct lm90_data *data = dev_get_drvdata(dev); struct i2c_client *client = data->client; - int nr = attr->index; - long val; int err; - err = kstrtol(buf, 10, &val); - if (err < 0) - return err; - /* +16 degrees offset for temp2 for the LM99 */ - if (data->kind == lm99 && attr->index == 3) + if (data->kind == lm99 && index == 3) val -= 16000; mutex_lock(&data->update_lock); if (data->kind == adt7461 || data->kind == tmp451) - data->temp8[nr] = temp_to_u8_adt7461(data, val); + data->temp8[index] = temp_to_u8_adt7461(data, val); else if (data->kind == max6646) - data->temp8[nr] = temp_to_u8(val); + data->temp8[index] = temp_to_u8(val); else - data->temp8[nr] = temp_to_s8(val); - - lm90_select_remote_channel(client, data, nr >= 6); - i2c_smbus_write_byte_data(client, reg[nr], data->temp8[nr]); - lm90_select_remote_channel(client, data, 0); + data->temp8[index] = temp_to_s8(val); + if ((err = lm90_select_remote_channel(client, data, index >= 6)) ||
Re: [PATCH v2 1/3] hwmon: (lm90) split set&show temp as common codes
On Wed, Jul 10, 2013 at 07:25:37PM +0800, Wei Ni wrote: > Split set&show temp codes as common functions, so we can use it directly when > implement linux thermal framework. > > Signed-off-by: Wei Ni > --- > drivers/hwmon/lm90.c | 118 > -- > 1 file changed, 75 insertions(+), 43 deletions(-) > > diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c > index 8eeb141..2cb7f8e 100644 > --- a/drivers/hwmon/lm90.c > +++ b/drivers/hwmon/lm90.c > @@ -702,29 +702,37 @@ static u16 temp_to_u16_adt7461(struct lm90_data *data, > long val) > * Sysfs stuff > */ > > -static ssize_t show_temp8(struct device *dev, struct device_attribute > *devattr, > - char *buf) > +static int read_temp8(struct device *dev, int index) > { > - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > - struct lm90_data *data = lm90_update_device(dev); > int temp; > + struct lm90_data *data = lm90_update_device(dev); > Why this reordering of variables ? > if (data->kind == adt7461) > - temp = temp_from_u8_adt7461(data, data->temp8[attr->index]); > + temp = temp_from_u8_adt7461(data, data->temp8[index]); > else if (data->kind == max6646) > - temp = temp_from_u8(data->temp8[attr->index]); > + temp = temp_from_u8(data->temp8[index]); > else > - temp = temp_from_s8(data->temp8[attr->index]); > + temp = temp_from_s8(data->temp8[index]); > > /* +16 degrees offset for temp2 for the LM99 */ > - if (data->kind == lm99 && attr->index == 3) > + if (data->kind == lm99 && index == 3) > temp += 16000; > > + return temp; > +} > + > +static ssize_t show_temp8(struct device *dev, struct device_attribute > *devattr, > + char *buf) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + int temp; > + > + temp = read_temp8(dev, attr->index); > + > return sprintf(buf, "%d\n", temp); Might as well simplify with return sprintf(buf, "%d\n", read_temp8(dev, attr->index)); > } > > -static ssize_t set_temp8(struct device *dev, struct device_attribute > *devattr, > - const char *buf, size_t count) > +static void write_temp8(struct device *dev, int index, long val) > { > static const u8 reg[8] = { > LM90_REG_W_LOCAL_LOW, > @@ -737,60 +745,76 @@ static ssize_t set_temp8(struct device *dev, struct > device_attribute *devattr, > MAX6659_REG_W_REMOTE_EMERG, > }; > > - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > struct i2c_client *client = to_i2c_client(dev); > struct lm90_data *data = i2c_get_clientdata(client); > - int nr = attr->index; > - long val; > - int err; > - > - err = kstrtol(buf, 10, &val); > - if (err < 0) > - return err; > > /* +16 degrees offset for temp2 for the LM99 */ > - if (data->kind == lm99 && attr->index == 3) > + if (data->kind == lm99 && index == 3) > val -= 16000; > > mutex_lock(&data->update_lock); > if (data->kind == adt7461) > - data->temp8[nr] = temp_to_u8_adt7461(data, val); > + data->temp8[index] = temp_to_u8_adt7461(data, val); > else if (data->kind == max6646) > - data->temp8[nr] = temp_to_u8(val); > + data->temp8[index] = temp_to_u8(val); > else > - data->temp8[nr] = temp_to_s8(val); > + data->temp8[index] = temp_to_s8(val); > > - lm90_select_remote_channel(client, data, nr >= 6); > - i2c_smbus_write_byte_data(client, reg[nr], data->temp8[nr]); > + lm90_select_remote_channel(client, data, index >= 6); > + i2c_smbus_write_byte_data(client, reg[index], data->temp8[index]); > lm90_select_remote_channel(client, data, 0); > > mutex_unlock(&data->update_lock); > +} > + > +static ssize_t set_temp8(struct device *dev, struct device_attribute > *devattr, > + const char *buf, size_t count) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + int index = attr->index; > + long val; > + int err; > + > + err = kstrtol(buf, 10, &val); > + if (err < 0) > + return err; > + > + write_temp8(dev, index, val); > + > return count; > } > > -static ssize_t show_temp11(struct device *dev, struct device_attribute > *devattr, > -char *buf) > +static int read_temp11(struct device *dev, int index) > { > - struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr); > - struct lm90_data *data = lm90_update_device(dev); > int temp; > + struct lm90_data *data = lm90_update_device(dev); > Another unnecessary variable reorder. > if (data->kind == adt7461) > - temp = temp_from_u16_adt7461(d
Re: [PATCH v2 1/3] hwmon: (lm90) split set&show temp as common codes
On 07/11/2013 02:21 AM, Guenter Roeck wrote: > On Wed, Jul 10, 2013 at 07:25:37PM +0800, Wei Ni wrote: >> Split set&show temp codes as common functions, so we can use it directly when >> implement linux thermal framework. >> >> Signed-off-by: Wei Ni >> --- >> drivers/hwmon/lm90.c | 118 >> -- >> 1 file changed, 75 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c >> index 8eeb141..2cb7f8e 100644 >> --- a/drivers/hwmon/lm90.c >> +++ b/drivers/hwmon/lm90.c >> @@ -702,29 +702,37 @@ static u16 temp_to_u16_adt7461(struct lm90_data *data, >> long val) >> * Sysfs stuff >> */ >> >> -static ssize_t show_temp8(struct device *dev, struct device_attribute >> *devattr, >> - char *buf) >> +static int read_temp8(struct device *dev, int index) >> { >> - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); >> - struct lm90_data *data = lm90_update_device(dev); >> int temp; >> + struct lm90_data *data = lm90_update_device(dev); >> > Why this reordering of variables ? Oh, I didn't noticed it yet, thanks for your review, I will change it. > >> if (data->kind == adt7461) >> - temp = temp_from_u8_adt7461(data, data->temp8[attr->index]); >> + temp = temp_from_u8_adt7461(data, data->temp8[index]); >> else if (data->kind == max6646) >> - temp = temp_from_u8(data->temp8[attr->index]); >> + temp = temp_from_u8(data->temp8[index]); >> else >> - temp = temp_from_s8(data->temp8[attr->index]); >> + temp = temp_from_s8(data->temp8[index]); >> >> /* +16 degrees offset for temp2 for the LM99 */ >> - if (data->kind == lm99 && attr->index == 3) >> + if (data->kind == lm99 && index == 3) >> temp += 16000; >> >> + return temp; >> +} >> + >> +static ssize_t show_temp8(struct device *dev, struct device_attribute >> *devattr, >> + char *buf) >> +{ >> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); >> + int temp; >> + >> + temp = read_temp8(dev, attr->index); >> + >> return sprintf(buf, "%d\n", temp); > > Might as well simplify with > return sprintf(buf, "%d\n", read_temp8(dev, attr->index)); Ok, I will update it. > >> } >> >> -static ssize_t set_temp8(struct device *dev, struct device_attribute >> *devattr, >> - const char *buf, size_t count) >> +static void write_temp8(struct device *dev, int index, long val) >> { >> static const u8 reg[8] = { >> LM90_REG_W_LOCAL_LOW, >> @@ -737,60 +745,76 @@ static ssize_t set_temp8(struct device *dev, struct >> device_attribute *devattr, >> MAX6659_REG_W_REMOTE_EMERG, >> }; >> >> - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); >> struct i2c_client *client = to_i2c_client(dev); >> struct lm90_data *data = i2c_get_clientdata(client); >> - int nr = attr->index; >> - long val; >> - int err; >> - >> - err = kstrtol(buf, 10, &val); >> - if (err < 0) >> - return err; >> >> /* +16 degrees offset for temp2 for the LM99 */ >> - if (data->kind == lm99 && attr->index == 3) >> + if (data->kind == lm99 && index == 3) >> val -= 16000; >> >> mutex_lock(&data->update_lock); >> if (data->kind == adt7461) >> - data->temp8[nr] = temp_to_u8_adt7461(data, val); >> + data->temp8[index] = temp_to_u8_adt7461(data, val); >> else if (data->kind == max6646) >> - data->temp8[nr] = temp_to_u8(val); >> + data->temp8[index] = temp_to_u8(val); >> else >> - data->temp8[nr] = temp_to_s8(val); >> + data->temp8[index] = temp_to_s8(val); >> >> - lm90_select_remote_channel(client, data, nr >= 6); >> - i2c_smbus_write_byte_data(client, reg[nr], data->temp8[nr]); >> + lm90_select_remote_channel(client, data, index >= 6); >> + i2c_smbus_write_byte_data(client, reg[index], data->temp8[index]); >> lm90_select_remote_channel(client, data, 0); >> >> mutex_unlock(&data->update_lock); >> +} >> + >> +static ssize_t set_temp8(struct device *dev, struct device_attribute >> *devattr, >> + const char *buf, size_t count) >> +{ >> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); >> + int index = attr->index; >> + long val; >> + int err; >> + >> + err = kstrtol(buf, 10, &val); >> + if (err < 0) >> + return err; >> + >> + write_temp8(dev, index, val); >> + >> return count; >> } >> >> -static ssize_t show_temp11(struct device *dev, struct device_attribute >> *devattr, >> -char *buf) >> +static int read_temp11(struct device *dev, int index) >> { >> - struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devatt