On 07/11/2017 03:03 PM, Tom Levens wrote:
On Sat, 8 Jul 2017, Guenter Roeck wrote:
On Mon, Jul 03, 2017 at 06:28:58AM +0200, Tom Levens wrote:
Conversion from raw values to signed integers has been refactored using
bitops.h. This also fixes a bug where negative temperatures were
converted incorrectly.
Documentation/process/submitting-patches.rst, chapter 2: "Solve only one
problem per patch.".
I can understand the urge to merge two sets of changes here. However,
that creates a problem for me: The fix should be applied to stable kernels,
but not the refactoring.
Please split the patch in two, one for the bug fix and one for the
refactoring.
In reality the refactoring *is* the fix for this bug. I am not sure how to to fix it simply without using the bitops macro. Of course, I could fix only the temperature conversion in one patch and refactor the other two in a separate one if you prefer?
If so, the subject and description are reversed. The subject should then be
something like "Fix incorrect conversion of negative temperatures",
and the description should explain that this is accomplished by using
existing API functions (then you can add "while at it, refactor voltage
conversions as well").
Thanks,
Guenter
Cheers,
Thanks,
Guenter
Signed-off-by: Tom Levens <tom.levens@xxxxxxx>
---
drivers/hwmon/ltc2990.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/hwmon/ltc2990.c b/drivers/hwmon/ltc2990.c
index 8f8fe05..e320d21 100644
--- a/drivers/hwmon/ltc2990.c
+++ b/drivers/hwmon/ltc2990.c
@@ -11,6 +11,7 @@
* the chip's internal temperature and Vcc power supply voltage.
*/
+#include <linux/bitops.h>
#include <linux/err.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
@@ -34,15 +35,6 @@
#define LTC2990_CONTROL_MODE_CURRENT 0x06
#define LTC2990_CONTROL_MODE_VOLTAGE 0x07
-/* convert raw register value to sign-extended integer in 16-bit range */
-static int ltc2990_voltage_to_int(int raw)
-{
- if (raw & BIT(14))
- return -(0x4000 - (raw & 0x3FFF)) << 2;
- else
- return (raw & 0x3FFF) << 2;
-}
-
/* Return the converted value from the given register in uV or mC */
static int ltc2990_get_value(struct i2c_client *i2c, u8 reg, int *result)
{
@@ -55,18 +47,16 @@ static int ltc2990_get_value(struct i2c_client *i2c, u8 reg, int *result)
switch (reg) {
case LTC2990_TINT_MSB:
/* internal temp, 0.0625 degrees/LSB, 13-bit */
- val = (val & 0x1FFF) << 3;
- *result = (val * 1000) >> 7;
+ *result = sign_extend32(val, 12) * 1000 / 16;
break;
case LTC2990_V1_MSB:
case LTC2990_V3_MSB:
/* Vx-Vy, 19.42uV/LSB. Depends on mode. */
- *result = ltc2990_voltage_to_int(val) * 1942 / (4 * 100);
+ *result = sign_extend32(val, 14) * 1942 / 100;
break;
case LTC2990_VCC_MSB:
/* Vcc, 305.18μV/LSB, 2.5V offset */
- *result = (ltc2990_voltage_to_int(val) * 30518 /
- (4 * 100 * 1000)) + 2500;
+ *result = sign_extend32(val, 14) * 30518 / (100 * 1000) + 2500;
break;
default:
return -EINVAL; /* won't happen, keep compiler happy */
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
