e1000e: fix call to do_div() to use u64 arg
Subsystems (2)
PATCH v2 1/1 e1000e: fix call to do_div() to use u64 arg
We were using s64 for lat_ns (latency nano-second value) since in our calculations a negative value could be a resultant. For negative values, we then assign lat_ns to be zero, so the value passed to do_div() was never negative, but do_div() expects the argument type to be u64, so do a cast to resolve a compile warning seen on PowerPC.
---
v2: fixed the patch to not use a cast to resolve the PowerPC compile
warning
--- drivers/net/ethernet/intel/e1000e/ich8lan.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
drivers/net/ethernet/intel/e1000e/ich8lan.c +7 -6
@@ -1015,7 +1015,7 @@ static s32 e1000_platform_pm_pch_lpt(struct e1000_hw *hw, bool link)u16 max_snoop, max_nosnoop;u16 max_ltr_enc;/* max LTR latency encoded */ s64 lat_ns;/* latency (ns) */ -s64 value; +u64 value;u32 rxa;if (!hw->adapter->max_frame_size) {
@@ -1042,12 +1042,13 @@ static s32 e1000_platform_pm_pch_lpt(struct e1000_hw *hw, bool link)*/ lat_ns = ((s64 )rxa *1024 - (2 * (s64 )hw->adapter->max_frame_size)) *8 *1000 ; -if (lat_ns <0 ) - lat_ns =0 ; -else -do_div (lat_ns, speed); +if (lat_ns <0 ) { + value =0 ; + }else { + value = lat_ns; +do_div (value, speed); + } - value = lat_ns;while (value >PCI_LTR_VALUE_MASK ) { scale++; value =DIV_ROUND_UP (value, (1 <<5 ));
PATCH v2 1/1 e1000e: fix call to do_div() to use u64 arg
Quoted 40 lines · Jeff Kirsher
We were using s64 for
lat_ns(latency nano-second value) since in our calculations a negative value could be a resultant. For negative values, we then assignlat_nsto be zero, so the value passed todo_div()was never negative, butdo_div()expects the argument type to be u64, so do a cast to resolve a compile warning seen on PowerPC.CC Yanjiang Jin <[email protected]>CC Yanir Lubetkin <[email protected]>Reported-by Yanjiang Jin <[email protected]>Signed-off-by Jeff Kirsher <[email protected]>---
v2: fixed the patch to not use a cast to resolve the PowerPC compile
warning--- drivers/net/ethernet/intel/e1000e/ich8lan.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)drivers/net/ethernet/intel/e1000e/ich8lan.c +2
-5@@ -1015,7 +1015,7 @@ static s32 e1000_platform_pm_pch_lpt(struct e1000_hw *hw, bool link)u16 max_snoop, max_nosnoop;u16 max_ltr_enc;/* max LTR latency encoded */ s64 lat_ns;/* latency (ns) */ -s64 value; +u64 value;u32 rxa;if (!hw->adapter->max_frame_size) {@@ -1042,12 +1042,13 @@ static s32 e1000_platform_pm_pch_lpt(struct e1000_hw *hw, bool link)*/ lat_ns = ((s64 )rxa *1024 - (2 * (s64 )hw->adapter->max_frame_size)) *8 *1000 ; -if (lat_ns <0 ) - lat_ns =0 ; -else -do_div (lat_ns, speed); +if (lat_ns <0 ) {
drivers/net/ethernet/intel/e1000e/ich8lan.c static s32 e1000_platform_pm_pch_lpt(struct e1000_hw *hw, bool link) · line 1054
Quoted 10 lines
drivers/net/ethernet/intel/e1000e/ich8lan.c +5
-1+ value =0 ; + }else { + value = lat_ns; +do_div (value, speed); + } - value = lat_ns;while (value >PCI_LTR_VALUE_MASK ) { scale++; value =DIV_ROUND_UP (value, (1 <<5 ));
Honestly the whole signed/unsigned thing is kind of a waste anyway. You could probably rewrite it to something like this and drop lat_ns entirely.
rxa *= 512;
value = (rxa > hw->adapter->max_frame_size) ?
(rxa - hw->adapter->max_frame_size) * (16000 / speed) : 0;
That should give you the same result without any need for the do_div or the signed values.
- Alex
Brown
PATCH v2 1/1 e1000e: fix call to do_div() to use u64 arg
Quoted 28 lines
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Thursday, April 09, 2015 3:45 PM
To: Kirsher, Jeffrey T; intel-wired-lan at lists.osuosl.org
Cc: david.laight at aculab.com; Jin, Yanjiang (Wind River)
Subject: Re: [Intel-wired-lan] [net-next v2] e1000e: fix call todo_div()to use u64 argQuoted 17 lines · Jeff Kirsher
We were using s64 for
lat_ns(latency nano-second value) since in our calculations a negative value could be a resultant. For negative values, we then assignlat_nsto be zero, so the value passed todo_div()was never negative, butdo_div()expects the argument type to be u64, so do a cast to resolve a compile warning seen on PowerPC.CC Yanjiang Jin <[email protected]>CC Yanir Lubetkin <[email protected]>Reported-by Yanjiang Jin <[email protected]>Signed-off-by Jeff Kirsher <[email protected]>---
v2: fixed the patch to not use a cast to resolve the PowerPC compile
warning--- drivers/net/ethernet/intel/e1000e/ich8lan.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
<snip>
Quoted 4 lines
Honestly the whole signed/unsigned thing is kind of a waste anyway. You could probably rewrite it to something like this and drop
lat_nsentirely.
Quoted 8 lines
rxa *= 512;
value = (rxa >hw->adapter->max_frame_size) ?(rxa - hw->adapter->max_frame_size) * (16000 / speed) : 0;That should give you the same result without any need for the
do_divor the signed values.- Alex
Anyone want to take a stab at striking out the signed / unsigned thing per Alex's suggestion?
Meanwhile I've run tons of tests with this in the queue and don't want to old things up, so...
Quoted 4 lines
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
PATCH v2 1/1 e1000e: fix call to do_div() to use u64 arg
Quoted 16 lines · Aaron F
Quoted 2 lines
Honestly the whole signed/unsigned thing is kind of a waste anyway.
You
Quoted 1 line
could probably rewrite it to something like this and drop
lat_nsentirely.
Quoted 6 lines
rxa *= 512;
value = (rxa >hw->adapter->max_frame_size) ?(rxa - hw->adapter->max_frame_size) * (16000 / speed) : 0;That should give you the same result without any need for the
do_divor
Quoted 3 lines
the signed values.
- Alex
Quoted 7 lines
Anyone want to take a stab at striking out the signed / unsigned thing per Alex's suggestion?
Meanwhile I've run tons of tests with this in the queue and don't want to old things up, so...
Tested-by Aaron Brown <[email protected]>
Yeah, I will be working on a follow on patch to implement Alex's
suggestions. For now, I wanted to go forward with this change to
resolve the compile warning. I should have a patch later this week to
implement Alex's suggestions, unless Yanir gets to it before me. :-)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20150421/4d60ed61/attachment.asc>