On 07/19/2014 10:12 AM, Jean Delvare wrote:
On Sat, 19 Jul 2014 08:27:26 -0700, Guenter Roeck wrote:
On 07/19/2014 07:57 AM, Axel Lin wrote:
2014-07-19 22:49 GMT+08:00 Guenter Roeck <linux@xxxxxxxxxxxx>:
On 07/18/2014 08:37 PM, Axel Lin wrote:
Signed-off-by: Axel Lin <axel.lin@xxxxxxxxxx>
---
drivers/hwmon/adm1026.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/adm1026.c b/drivers/hwmon/adm1026.c
index ca8430f..c632e46 100644
--- a/drivers/hwmon/adm1026.c
+++ b/drivers/hwmon/adm1026.c
@@ -196,7 +196,7 @@ static int adm1026_scaling[] = { /* .001 Volts */
3330, 4995, 2250, 12000, 13875
};
#define NEG12_OFFSET 16000
-#define SCALE(val, from, to) (((val)*(to) + ((from)/2))/(from))
+#define SCALE(val, from, to) DIV_ROUND_CLOSEST((val) * (to), (from))
#define INS_TO_REG(n, val) (clamp_val(SCALE(val, adm1026_scaling[n],
192),\
0, 255))
#define INS_FROM_REG(n, val) (SCALE(val, 192, adm1026_scaling[n]))
Hi Axel,
I don't really see the value in this series. It does not really improve
anything,
or make the code smaller.
Though there is nothing wrong in original code.
IMHO, I think use DIV_ROUND_CLOSEST is less error prone than having
the similar calculation in various drivers.
That is more applicable to the SCALE macro or function itself, though.
For example, if you look into adm9240.c, you'll notice that SCALE returns
an int but gets a long as argument. This is asking for overflows. Also,
it is called prior to calling clamp_val, which again invites overflow errors.
Sure, those are corner cases, and especially the clamp_val problem would
be difficult to address, but getting rid of SCALE entirely would at least
address the long->int overflow problem. Your patch doesn't do that.
In other cases, SCALE uses a multiplication factor of 1, which means it is
really unnecessary and could be replaced directly with DIV_ROUND_CLOSEST.
This was exactly my thought when looking at this series.
In other words, I'd be more inclined to accept patches which get rid of
the SCALE macro or function entirely.
I really have no opinion about this, sorry.
Lets focus on real problems and simplifications, then. Easy to fix overflow
problems like the long->int problem mentioned above, or multiplications
by one.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
