| Message ID | 1460938902-3120-1-git-send-email-akinobu.mita@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
Hello Thanks! I haven't tested the patch yet but I already have a question. What about s16 in ds3231_hwmon_show_temp()? 2016-04-18 4:21 GMT+04:00, Akinobu Mita <akinobu.mita@gmail.com>: > From: Zhuang Yuyao <mlistz@gmail.com> > > while retrieving temperature from ds3231, the result may be overflow > since s16 is too small for a multiplication with 250. > > ie. if temp_buf[0] == 0x2d, the result (s16 temp) will be negative. > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > Cc: Zhuang Yuyao <mlistz@gmail.com> > Cc: Zhuang Yuyao <zhuangyy@syan.com.cn> > Cc: Michael Tatarinov <kukabu@gmail.com> > Cc: Alessandro Zummo <a.zummo@towertech.it> > Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com> > --- > drivers/rtc/rtc-ds1307.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c > index cb0ffa8..90d1e5a 100644 > --- a/drivers/rtc/rtc-ds1307.c > +++ b/drivers/rtc/rtc-ds1307.c > @@ -863,7 +863,7 @@ out: > * A user-initiated temperature conversion is not started by this > function, > * so the temperature is updated once every 64 seconds. > */ > -static int ds3231_hwmon_read_temp(struct device *dev, s16 *mC) > +static int ds3231_hwmon_read_temp(struct device *dev, s32 *mC) > { > struct ds1307 *ds1307 = dev_get_drvdata(dev); > u8 temp_buf[2]; > @@ -892,7 +892,7 @@ static ssize_t ds3231_hwmon_show_temp(struct device > *dev, > struct device_attribute *attr, char *buf) > { > int ret; > - s16 temp; > + s32 temp; > > ret = ds3231_hwmon_read_temp(dev, &temp); > if (ret) > -- > 2.5.0 > >
sorry, I realized my mistake 2016-04-18 8:33 GMT+04:00, Michael Tatarinov <kukabu@gmail.com>: > Hello > > Thanks! > I haven't tested the patch yet but I already have a question. What > about s16 in ds3231_hwmon_show_temp()? > > > 2016-04-18 4:21 GMT+04:00, Akinobu Mita <akinobu.mita@gmail.com>: >> From: Zhuang Yuyao <mlistz@gmail.com> >> >> while retrieving temperature from ds3231, the result may be overflow >> since s16 is too small for a multiplication with 250. >> >> ie. if temp_buf[0] == 0x2d, the result (s16 temp) will be negative. >> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> >> Cc: Zhuang Yuyao <mlistz@gmail.com> >> Cc: Zhuang Yuyao <zhuangyy@syan.com.cn> >> Cc: Michael Tatarinov <kukabu@gmail.com> >> Cc: Alessandro Zummo <a.zummo@towertech.it> >> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com> >> --- >> drivers/rtc/rtc-ds1307.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c >> index cb0ffa8..90d1e5a 100644 >> --- a/drivers/rtc/rtc-ds1307.c >> +++ b/drivers/rtc/rtc-ds1307.c >> @@ -863,7 +863,7 @@ out: >> * A user-initiated temperature conversion is not started by this >> function, >> * so the temperature is updated once every 64 seconds. >> */ >> -static int ds3231_hwmon_read_temp(struct device *dev, s16 *mC) >> +static int ds3231_hwmon_read_temp(struct device *dev, s32 *mC) >> { >> struct ds1307 *ds1307 = dev_get_drvdata(dev); >> u8 temp_buf[2]; >> @@ -892,7 +892,7 @@ static ssize_t ds3231_hwmon_show_temp(struct device >> *dev, >> struct device_attribute *attr, char *buf) >> { >> int ret; >> - s16 temp; >> + s32 temp; >> >> ret = ds3231_hwmon_read_temp(dev, &temp); >> if (ret) >> -- >> 2.5.0 >> >> >
Thanks. I'm testing the patch, it workins as expected. $ cat /sys/devices/platform/soc/20804000.i2c/i2c-1/1-0068/hwmon/hwmon1/temp1_input 33000 2016-04-18 4:21 GMT+04:00, Akinobu Mita <akinobu.mita@gmail.com>: > From: Zhuang Yuyao <mlistz@gmail.com> > > while retrieving temperature from ds3231, the result may be overflow > since s16 is too small for a multiplication with 250. > > ie. if temp_buf[0] == 0x2d, the result (s16 temp) will be negative.
2016-04-18 19:24 GMT+09:00 Michael Tatarinov <kukabu@gmail.com>: > Thanks. > > I'm testing the patch, it workins as expected. > > $ cat /sys/devices/platform/soc/20804000.i2c/i2c-1/1-0068/hwmon/hwmon1/temp1_input > 33000 Thanks for testing! Can I put your Tested-by tag in this patch?
2016-04-18 17:05 GMT+04:00, Akinobu Mita <akinobu.mita@gmail.com>: > 2016-04-18 19:24 GMT+09:00 Michael Tatarinov <kukabu@gmail.com>: >> Thanks. >> >> I'm testing the patch, it workins as expected. >> >> $ cat >> /sys/devices/platform/soc/20804000.i2c/i2c-1/1-0068/hwmon/hwmon1/temp1_input >> 33000 > > Thanks for testing! Thanks for original patch and fix. :) > Can I put your Tested-by tag in this patch? Yes.
On 18/04/2016 at 09:21:42 +0900, Akinobu Mita wrote : > From: Zhuang Yuyao <mlistz@gmail.com> > > while retrieving temperature from ds3231, the result may be overflow > since s16 is too small for a multiplication with 250. > > ie. if temp_buf[0] == 0x2d, the result (s16 temp) will be negative. > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > Cc: Zhuang Yuyao <mlistz@gmail.com> > Cc: Zhuang Yuyao <zhuangyy@syan.com.cn> > Cc: Michael Tatarinov <kukabu@gmail.com> > Cc: Alessandro Zummo <a.zummo@towertech.it> > Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com> > --- > drivers/rtc/rtc-ds1307.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Applied, thanks.
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c index cb0ffa8..90d1e5a 100644 --- a/drivers/rtc/rtc-ds1307.c +++ b/drivers/rtc/rtc-ds1307.c @@ -863,7 +863,7 @@ out: * A user-initiated temperature conversion is not started by this function, * so the temperature is updated once every 64 seconds. */ -static int ds3231_hwmon_read_temp(struct device *dev, s16 *mC) +static int ds3231_hwmon_read_temp(struct device *dev, s32 *mC) { struct ds1307 *ds1307 = dev_get_drvdata(dev); u8 temp_buf[2]; @@ -892,7 +892,7 @@ static ssize_t ds3231_hwmon_show_temp(struct device *dev, struct device_attribute *attr, char *buf) { int ret; - s16 temp; + s32 temp; ret = ds3231_hwmon_read_temp(dev, &temp); if (ret)