netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] e1000: remove unused variables
@ 2014-11-27  5:22 Sudip Mukherjee
  2014-11-27  5:59 ` Hisashi T Fujinaka
  0 siblings, 1 reply; 9+ messages in thread
From: Sudip Mukherjee @ 2014-11-27  5:22 UTC (permalink / raw)
  To: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Matthew Vick, John Ronciak,
	Mitch Williams, Linux NICS
  Cc: e1000-devel, netdev, Sudip Mukherjee, linux-kernel

these variables were only being assigned some values, but were never
used.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/net/ethernet/intel/e1000/e1000_hw.c   | 142 ++++++++++++--------------
 drivers/net/ethernet/intel/e1000/e1000_main.c |   3 -
 2 files changed, 66 insertions(+), 79 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_hw.c b/drivers/net/ethernet/intel/e1000/e1000_hw.c
index 45c8c864..7812f59 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_hw.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_hw.c
@@ -154,7 +154,6 @@ static s32 e1000_set_phy_type(struct e1000_hw *hw)
  */
 static void e1000_phy_init_script(struct e1000_hw *hw)
 {
-	u32 ret_val;
 	u16 phy_saved_data;
 
 	if (hw->phy_init_script) {
@@ -163,7 +162,7 @@ static void e1000_phy_init_script(struct e1000_hw *hw)
 		/* Save off the current value of register 0x2F5B to be restored
 		 * at the end of this routine.
 		 */
-		ret_val = e1000_read_phy_reg(hw, 0x2F5B, &phy_saved_data);
+		e1000_read_phy_reg(hw, 0x2F5B, &phy_saved_data);
 
 		/* Disabled the PHY transmitter */
 		e1000_write_phy_reg(hw, 0x2F5B, 0x0003);
@@ -402,7 +401,6 @@ s32 e1000_reset_hw(struct e1000_hw *hw)
 {
 	u32 ctrl;
 	u32 ctrl_ext;
-	u32 icr;
 	u32 manc;
 	u32 led_ctrl;
 	s32 ret_val;
@@ -527,7 +525,7 @@ s32 e1000_reset_hw(struct e1000_hw *hw)
 	ew32(IMC, 0xffffffff);
 
 	/* Clear any pending interrupt events. */
-	icr = er32(ICR);
+	er32(ICR);
 
 	/* If MWI was previously enabled, reenable it. */
 	if (hw->mac_type == e1000_82542_rev2_0) {
@@ -2396,16 +2394,13 @@ static s32 e1000_check_for_serdes_link_generic(struct e1000_hw *hw)
  */
 s32 e1000_check_for_link(struct e1000_hw *hw)
 {
-	u32 rxcw = 0;
-	u32 ctrl;
 	u32 status;
 	u32 rctl;
 	u32 icr;
-	u32 signal = 0;
 	s32 ret_val;
 	u16 phy_data;
 
-	ctrl = er32(CTRL);
+	er32(CTRL);
 	status = er32(STATUS);
 
 	/* On adapters with a MAC newer than 82544, SW Definable pin 1 will be
@@ -2414,12 +2409,9 @@ s32 e1000_check_for_link(struct e1000_hw *hw)
 	 */
 	if ((hw->media_type == e1000_media_type_fiber) ||
 	    (hw->media_type == e1000_media_type_internal_serdes)) {
-		rxcw = er32(RXCW);
+		er32(RXCW);
 
 		if (hw->media_type == e1000_media_type_fiber) {
-			signal =
-			    (hw->mac_type >
-			     e1000_82544) ? E1000_CTRL_SWDPIN1 : 0;
 			if (status & E1000_STATUS_LU)
 				hw->get_link_status = false;
 		}
@@ -4698,78 +4690,76 @@ s32 e1000_led_off(struct e1000_hw *hw)
  */
 static void e1000_clear_hw_cntrs(struct e1000_hw *hw)
 {
-	volatile u32 temp;
-
-	temp = er32(CRCERRS);
-	temp = er32(SYMERRS);
-	temp = er32(MPC);
-	temp = er32(SCC);
-	temp = er32(ECOL);
-	temp = er32(MCC);
-	temp = er32(LATECOL);
-	temp = er32(COLC);
-	temp = er32(DC);
-	temp = er32(SEC);
-	temp = er32(RLEC);
-	temp = er32(XONRXC);
-	temp = er32(XONTXC);
-	temp = er32(XOFFRXC);
-	temp = er32(XOFFTXC);
-	temp = er32(FCRUC);
-
-	temp = er32(PRC64);
-	temp = er32(PRC127);
-	temp = er32(PRC255);
-	temp = er32(PRC511);
-	temp = er32(PRC1023);
-	temp = er32(PRC1522);
-
-	temp = er32(GPRC);
-	temp = er32(BPRC);
-	temp = er32(MPRC);
-	temp = er32(GPTC);
-	temp = er32(GORCL);
-	temp = er32(GORCH);
-	temp = er32(GOTCL);
-	temp = er32(GOTCH);
-	temp = er32(RNBC);
-	temp = er32(RUC);
-	temp = er32(RFC);
-	temp = er32(ROC);
-	temp = er32(RJC);
-	temp = er32(TORL);
-	temp = er32(TORH);
-	temp = er32(TOTL);
-	temp = er32(TOTH);
-	temp = er32(TPR);
-	temp = er32(TPT);
-
-	temp = er32(PTC64);
-	temp = er32(PTC127);
-	temp = er32(PTC255);
-	temp = er32(PTC511);
-	temp = er32(PTC1023);
-	temp = er32(PTC1522);
-
-	temp = er32(MPTC);
-	temp = er32(BPTC);
+	er32(CRCERRS);
+	er32(SYMERRS);
+	er32(MPC);
+	er32(SCC);
+	er32(ECOL);
+	er32(MCC);
+	er32(LATECOL);
+	er32(COLC);
+	er32(DC);
+	er32(SEC);
+	er32(RLEC);
+	er32(XONRXC);
+	er32(XONTXC);
+	er32(XOFFRXC);
+	er32(XOFFTXC);
+	er32(FCRUC);
+
+	er32(PRC64);
+	er32(PRC127);
+	er32(PRC255);
+	er32(PRC511);
+	er32(PRC1023);
+	er32(PRC1522);
+
+	er32(GPRC);
+	er32(BPRC);
+	er32(MPRC);
+	er32(GPTC);
+	er32(GORCL);
+	er32(GORCH);
+	er32(GOTCL);
+	er32(GOTCH);
+	er32(RNBC);
+	er32(RUC);
+	er32(RFC);
+	er32(ROC);
+	er32(RJC);
+	er32(TORL);
+	er32(TORH);
+	er32(TOTL);
+	er32(TOTH);
+	er32(TPR);
+	er32(TPT);
+
+	er32(PTC64);
+	er32(PTC127);
+	er32(PTC255);
+	er32(PTC511);
+	er32(PTC1023);
+	er32(PTC1522);
+
+	er32(MPTC);
+	er32(BPTC);
 
 	if (hw->mac_type < e1000_82543)
 		return;
 
-	temp = er32(ALGNERRC);
-	temp = er32(RXERRC);
-	temp = er32(TNCRS);
-	temp = er32(CEXTERR);
-	temp = er32(TSCTC);
-	temp = er32(TSCTFC);
+	er32(ALGNERRC);
+	er32(RXERRC);
+	er32(TNCRS);
+	er32(CEXTERR);
+	er32(TSCTC);
+	er32(TSCTFC);
 
 	if (hw->mac_type <= e1000_82544)
 		return;
 
-	temp = er32(MGTPRC);
-	temp = er32(MGTPDC);
-	temp = er32(MGTPTC);
+	er32(MGTPRC);
+	er32(MGTPDC);
+	er32(MGTPTC);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 24f3986..a70ea46 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -2443,7 +2443,6 @@ static void e1000_watchdog(struct work_struct *work)
 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
 			u32 ctrl;
-			bool txb2b = true;
 			/* update snapshot of PHY registers on LSC */
 			e1000_get_speed_and_duplex(hw,
 						   &adapter->link_speed,
@@ -2465,11 +2464,9 @@ static void e1000_watchdog(struct work_struct *work)
 			adapter->tx_timeout_factor = 1;
 			switch (adapter->link_speed) {
 			case SPEED_10:
-				txb2b = false;
 				adapter->tx_timeout_factor = 16;
 				break;
 			case SPEED_100:
-				txb2b = false;
 				/* maybe add some timeout factor ? */
 				break;
 			}
-- 
1.8.1.2


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] e1000: remove unused variables
  2014-11-27  5:22 [PATCH] e1000: remove unused variables Sudip Mukherjee
@ 2014-11-27  5:59 ` Hisashi T Fujinaka
  2014-11-27 13:07   ` Sudip Mukherjee
  2014-11-30  1:45   ` Ben Hutchings
  0 siblings, 2 replies; 9+ messages in thread
From: Hisashi T Fujinaka @ 2014-11-27  5:59 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Matthew Vick, John Ronciak,
	Mitch Williams, Linux NICS, e1000-devel, netdev, linux-kernel

I'm pretty sure those double reads are there for a reason, so most of
this I'm going to have to check on Monday. We have a long holiday
weekend here in the US.

I'm not sure why you're bothering with an old driver like this, but if
you haven't actually tried this on all the hardware it pertains to, I'm
going want to NAK this.

I should do this from my todd.fujinaka@intel.com account but it's 10PM
on the first day of a long holiday weekend.

On Thu, 27 Nov 2014, Sudip Mukherjee wrote:

> these variables were only being assigned some values, but were never
> used.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> drivers/net/ethernet/intel/e1000/e1000_hw.c   | 142 ++++++++++++--------------
> drivers/net/ethernet/intel/e1000/e1000_main.c |   3 -
> 2 files changed, 66 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_hw.c b/drivers/net/ethernet/intel/e1000/e1000_hw.c
> index 45c8c864..7812f59 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_hw.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_hw.c
> @@ -154,7 +154,6 @@ static s32 e1000_set_phy_type(struct e1000_hw *hw)
>  */
> static void e1000_phy_init_script(struct e1000_hw *hw)
> {
> -	u32 ret_val;
> 	u16 phy_saved_data;
>
> 	if (hw->phy_init_script) {
> @@ -163,7 +162,7 @@ static void e1000_phy_init_script(struct e1000_hw *hw)
> 		/* Save off the current value of register 0x2F5B to be restored
> 		 * at the end of this routine.
> 		 */
> -		ret_val = e1000_read_phy_reg(hw, 0x2F5B, &phy_saved_data);
> +		e1000_read_phy_reg(hw, 0x2F5B, &phy_saved_data);
>
> 		/* Disabled the PHY transmitter */
> 		e1000_write_phy_reg(hw, 0x2F5B, 0x0003);
> @@ -402,7 +401,6 @@ s32 e1000_reset_hw(struct e1000_hw *hw)
> {
> 	u32 ctrl;
> 	u32 ctrl_ext;
> -	u32 icr;
> 	u32 manc;
> 	u32 led_ctrl;
> 	s32 ret_val;
> @@ -527,7 +525,7 @@ s32 e1000_reset_hw(struct e1000_hw *hw)
> 	ew32(IMC, 0xffffffff);
>
> 	/* Clear any pending interrupt events. */
> -	icr = er32(ICR);
> +	er32(ICR);
>
> 	/* If MWI was previously enabled, reenable it. */
> 	if (hw->mac_type == e1000_82542_rev2_0) {
> @@ -2396,16 +2394,13 @@ static s32 e1000_check_for_serdes_link_generic(struct e1000_hw *hw)
>  */
> s32 e1000_check_for_link(struct e1000_hw *hw)
> {
> -	u32 rxcw = 0;
> -	u32 ctrl;
> 	u32 status;
> 	u32 rctl;
> 	u32 icr;
> -	u32 signal = 0;
> 	s32 ret_val;
> 	u16 phy_data;
>
> -	ctrl = er32(CTRL);
> +	er32(CTRL);
> 	status = er32(STATUS);
>
> 	/* On adapters with a MAC newer than 82544, SW Definable pin 1 will be
> @@ -2414,12 +2409,9 @@ s32 e1000_check_for_link(struct e1000_hw *hw)
> 	 */
> 	if ((hw->media_type == e1000_media_type_fiber) ||
> 	    (hw->media_type == e1000_media_type_internal_serdes)) {
> -		rxcw = er32(RXCW);
> +		er32(RXCW);
>
> 		if (hw->media_type == e1000_media_type_fiber) {
> -			signal =
> -			    (hw->mac_type >
> -			     e1000_82544) ? E1000_CTRL_SWDPIN1 : 0;
> 			if (status & E1000_STATUS_LU)
> 				hw->get_link_status = false;
> 		}
> @@ -4698,78 +4690,76 @@ s32 e1000_led_off(struct e1000_hw *hw)
>  */
> static void e1000_clear_hw_cntrs(struct e1000_hw *hw)
> {
> -	volatile u32 temp;
> -
> -	temp = er32(CRCERRS);
> -	temp = er32(SYMERRS);
> -	temp = er32(MPC);
> -	temp = er32(SCC);
> -	temp = er32(ECOL);
> -	temp = er32(MCC);
> -	temp = er32(LATECOL);
> -	temp = er32(COLC);
> -	temp = er32(DC);
> -	temp = er32(SEC);
> -	temp = er32(RLEC);
> -	temp = er32(XONRXC);
> -	temp = er32(XONTXC);
> -	temp = er32(XOFFRXC);
> -	temp = er32(XOFFTXC);
> -	temp = er32(FCRUC);
> -
> -	temp = er32(PRC64);
> -	temp = er32(PRC127);
> -	temp = er32(PRC255);
> -	temp = er32(PRC511);
> -	temp = er32(PRC1023);
> -	temp = er32(PRC1522);
> -
> -	temp = er32(GPRC);
> -	temp = er32(BPRC);
> -	temp = er32(MPRC);
> -	temp = er32(GPTC);
> -	temp = er32(GORCL);
> -	temp = er32(GORCH);
> -	temp = er32(GOTCL);
> -	temp = er32(GOTCH);
> -	temp = er32(RNBC);
> -	temp = er32(RUC);
> -	temp = er32(RFC);
> -	temp = er32(ROC);
> -	temp = er32(RJC);
> -	temp = er32(TORL);
> -	temp = er32(TORH);
> -	temp = er32(TOTL);
> -	temp = er32(TOTH);
> -	temp = er32(TPR);
> -	temp = er32(TPT);
> -
> -	temp = er32(PTC64);
> -	temp = er32(PTC127);
> -	temp = er32(PTC255);
> -	temp = er32(PTC511);
> -	temp = er32(PTC1023);
> -	temp = er32(PTC1522);
> -
> -	temp = er32(MPTC);
> -	temp = er32(BPTC);
> +	er32(CRCERRS);
> +	er32(SYMERRS);
> +	er32(MPC);
> +	er32(SCC);
> +	er32(ECOL);
> +	er32(MCC);
> +	er32(LATECOL);
> +	er32(COLC);
> +	er32(DC);
> +	er32(SEC);
> +	er32(RLEC);
> +	er32(XONRXC);
> +	er32(XONTXC);
> +	er32(XOFFRXC);
> +	er32(XOFFTXC);
> +	er32(FCRUC);
> +
> +	er32(PRC64);
> +	er32(PRC127);
> +	er32(PRC255);
> +	er32(PRC511);
> +	er32(PRC1023);
> +	er32(PRC1522);
> +
> +	er32(GPRC);
> +	er32(BPRC);
> +	er32(MPRC);
> +	er32(GPTC);
> +	er32(GORCL);
> +	er32(GORCH);
> +	er32(GOTCL);
> +	er32(GOTCH);
> +	er32(RNBC);
> +	er32(RUC);
> +	er32(RFC);
> +	er32(ROC);
> +	er32(RJC);
> +	er32(TORL);
> +	er32(TORH);
> +	er32(TOTL);
> +	er32(TOTH);
> +	er32(TPR);
> +	er32(TPT);
> +
> +	er32(PTC64);
> +	er32(PTC127);
> +	er32(PTC255);
> +	er32(PTC511);
> +	er32(PTC1023);
> +	er32(PTC1522);
> +
> +	er32(MPTC);
> +	er32(BPTC);
>
> 	if (hw->mac_type < e1000_82543)
> 		return;
>
> -	temp = er32(ALGNERRC);
> -	temp = er32(RXERRC);
> -	temp = er32(TNCRS);
> -	temp = er32(CEXTERR);
> -	temp = er32(TSCTC);
> -	temp = er32(TSCTFC);
> +	er32(ALGNERRC);
> +	er32(RXERRC);
> +	er32(TNCRS);
> +	er32(CEXTERR);
> +	er32(TSCTC);
> +	er32(TSCTFC);
>
> 	if (hw->mac_type <= e1000_82544)
> 		return;
>
> -	temp = er32(MGTPRC);
> -	temp = er32(MGTPDC);
> -	temp = er32(MGTPTC);
> +	er32(MGTPRC);
> +	er32(MGTPDC);
> +	er32(MGTPTC);
> }
>
> /**
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
> index 24f3986..a70ea46 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> @@ -2443,7 +2443,6 @@ static void e1000_watchdog(struct work_struct *work)
> 	if (link) {
> 		if (!netif_carrier_ok(netdev)) {
> 			u32 ctrl;
> -			bool txb2b = true;
> 			/* update snapshot of PHY registers on LSC */
> 			e1000_get_speed_and_duplex(hw,
> 						   &adapter->link_speed,
> @@ -2465,11 +2464,9 @@ static void e1000_watchdog(struct work_struct *work)
> 			adapter->tx_timeout_factor = 1;
> 			switch (adapter->link_speed) {
> 			case SPEED_10:
> -				txb2b = false;
> 				adapter->tx_timeout_factor = 16;
> 				break;
> 			case SPEED_100:
> -				txb2b = false;
> 				/* maybe add some timeout factor ? */
> 				break;
> 			}
>

-- 
Hisashi T Fujinaka - htodd@twofifty.com
BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] e1000: remove unused variables
  2014-11-27  5:59 ` Hisashi T Fujinaka
@ 2014-11-27 13:07   ` Sudip Mukherjee
  2014-11-28 23:28     ` Florian Fainelli
  2014-11-30  1:45   ` Ben Hutchings
  1 sibling, 1 reply; 9+ messages in thread
From: Sudip Mukherjee @ 2014-11-27 13:07 UTC (permalink / raw)
  To: Hisashi T Fujinaka
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Matthew Vick, John Ronciak,
	Mitch Williams, Linux NICS, e1000-devel, netdev, linux-kernel

On Wed, Nov 26, 2014 at 09:59:28PM -0800, Hisashi T Fujinaka wrote:
> I'm pretty sure those double reads are there for a reason, so most of
> this I'm going to have to check on Monday. We have a long holiday
> weekend here in the US.

if the double reads are there for some reason, can you please let me know what that reason might be..

> 
> I'm not sure why you're bothering with an old driver like this, but if
> you haven't actually tried this on all the hardware it pertains to, I'm
> going want to NAK this.

no it has not been tested on hardware.  :(

i am still in the learning process, NAK is also part of learning.

infact there is another part of the code, which, theoretically, will never get executed. but i didnot dare to send that removal patch without testing on the hardware.

thanks
sudip

> 
> I should do this from my todd.fujinaka@intel.com account but it's 10PM
> on the first day of a long holiday weekend.
> 
> On Thu, 27 Nov 2014, Sudip Mukherjee wrote:
> 
> >these variables were only being assigned some values, but were never
> >used.
> >
> >Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> >---
> >drivers/net/ethernet/intel/e1000/e1000_hw.c   | 142 ++++++++++++--------------
<snip>
> >			case SPEED_100:
> >-				txb2b = false;
> >				/* maybe add some timeout factor ? */
> >				break;
> >			}
> >
> 
> -- 
> Hisashi T Fujinaka - htodd@twofifty.com
> BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] e1000: remove unused variables
  2014-11-27 13:07   ` Sudip Mukherjee
@ 2014-11-28 23:28     ` Florian Fainelli
  2014-11-29 11:01       ` Lino Sanfilippo
  0 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2014-11-28 23:28 UTC (permalink / raw)
  To: Sudip Mukherjee, Hisashi T Fujinaka
  Cc: Linux NICS, e1000-devel, Bruce Allan, Jesse Brandeburg,
	linux-kernel, John Ronciak, netdev

Le 27/11/2014 05:07, Sudip Mukherjee a écrit :
> On Wed, Nov 26, 2014 at 09:59:28PM -0800, Hisashi T Fujinaka wrote:
>> I'm pretty sure those double reads are there for a reason, so most of
>> this I'm going to have to check on Monday. We have a long holiday
>> weekend here in the US.
> 
> if the double reads are there for some reason, can you please let me know what that reason might be..

Could be latching, especially in the context of reading from Ethernet
PHYs, some registers are latched, so you may have to do a double read to
ensure the value you get is consistent.

Also, if you do a read that is not stored in any return value, the
compiler is now free to remove that actual read, and that may have other
side effects for registers which are e.g: read to clear, or any of the like.

> 
>>
>> I'm not sure why you're bothering with an old driver like this, but if
>> you haven't actually tried this on all the hardware it pertains to, I'm
>> going want to NAK this.
> 
> no it has not been tested on hardware.  :(
> 
> i am still in the learning process, NAK is also part of learning.
> 
> infact there is another part of the code, which, theoretically, will never get executed. but i didnot dare to send that removal patch without testing on the hardware.
> 
> thanks
> sudip
> 
>>
>> I should do this from my todd.fujinaka@intel.com account but it's 10PM
>> on the first day of a long holiday weekend.
>>
>> On Thu, 27 Nov 2014, Sudip Mukherjee wrote:
>>
>>> these variables were only being assigned some values, but were never
>>> used.
>>>
>>> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
>>> ---
>>> drivers/net/ethernet/intel/e1000/e1000_hw.c   | 142 ++++++++++++--------------
> <snip>
>>> 			case SPEED_100:
>>> -				txb2b = false;
>>> 				/* maybe add some timeout factor ? */
>>> 				break;
>>> 			}
>>>
>>
>> -- 
>> Hisashi T Fujinaka - htodd@twofifty.com
>> BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] e1000: remove unused variables
  2014-11-28 23:28     ` Florian Fainelli
@ 2014-11-29 11:01       ` Lino Sanfilippo
  0 siblings, 0 replies; 9+ messages in thread
From: Lino Sanfilippo @ 2014-11-29 11:01 UTC (permalink / raw)
  To: Florian Fainelli, Sudip Mukherjee, Hisashi T Fujinaka
  Cc: Linux NICS, e1000-devel, Bruce Allan, Jesse Brandeburg,
	linux-kernel, John Ronciak, netdev

On 29.11.2014 00:28, Florian Fainelli wrote:

> Also, if you do a read that is not stored in any return value, the
> compiler is now free to remove that actual read, 

This does not apply to reads from iomem (see "volatile" specifier in
readl()).

Regards,
Lino


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] e1000: remove unused variables
  2014-11-27  5:59 ` Hisashi T Fujinaka
  2014-11-27 13:07   ` Sudip Mukherjee
@ 2014-11-30  1:45   ` Ben Hutchings
  2014-12-01  4:54     ` Sudip Mukherjee
  1 sibling, 1 reply; 9+ messages in thread
From: Ben Hutchings @ 2014-11-30  1:45 UTC (permalink / raw)
  To: Hisashi T Fujinaka
  Cc: Linux NICS, e1000-devel, Bruce Allan, Jesse Brandeburg,
	linux-kernel, John Ronciak, netdev, Sudip Mukherjee


[-- Attachment #1.1: Type: text/plain, Size: 515 bytes --]

On Wed, 2014-11-26 at 21:59 -0800, Hisashi T Fujinaka wrote:
> I'm pretty sure those double reads are there for a reason, so most of
> this I'm going to have to check on Monday. We have a long holiday
> weekend here in the US.
[...]

If there were double register reads being replaced with single register
reads, I'd agree this was likely to introduce a regression.  But all I
see is var = er32(REG) being changed to er32(REG).

Ben.

-- 
Ben Hutchings
The world is coming to an end.	Please log off.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

[-- Attachment #2: Type: text/plain, Size: 441 bytes --]

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] e1000: remove unused variables
  2014-11-30  1:45   ` Ben Hutchings
@ 2014-12-01  4:54     ` Sudip Mukherjee
  2014-12-01 18:56       ` [linux-nics] " Fujinaka, Todd
  0 siblings, 1 reply; 9+ messages in thread
From: Sudip Mukherjee @ 2014-12-01  4:54 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Linux NICS, e1000-devel, Bruce Allan, Jesse Brandeburg,
	linux-kernel, John Ronciak, netdev

On Sun, Nov 30, 2014 at 01:45:13AM +0000, Ben Hutchings wrote:
> On Wed, 2014-11-26 at 21:59 -0800, Hisashi T Fujinaka wrote:
> > I'm pretty sure those double reads are there for a reason, so most of
> > this I'm going to have to check on Monday. We have a long holiday
> > weekend here in the US.
> [...]
> 
> If there were double register reads being replaced with single register
> reads, I'd agree this was likely to introduce a regression.  But all I
> see is var = er32(REG) being changed to er32(REG).

no, double register reads are not modified. only the unused variables are removed.

thanks
sudip

> 
> Ben.
> 
> -- 
> Ben Hutchings
> The world is coming to an end.	Please log off.



------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [linux-nics] [PATCH] e1000: remove unused variables
  2014-12-01  4:54     ` Sudip Mukherjee
@ 2014-12-01 18:56       ` Fujinaka, Todd
  2014-12-02 14:24         ` Sudip Mukherjee
  0 siblings, 1 reply; 9+ messages in thread
From: Fujinaka, Todd @ 2014-12-01 18:56 UTC (permalink / raw)
  To: Sudip Mukherjee, Ben Hutchings
  Cc: Linux NICS, e1000-devel@lists.sourceforge.net, Hisashi T Fujinaka,
	Vick, Matthew, Greg@isotope.jf.intel.com, Kirsher, Jeffrey T,
	netdev@vger.kernel.org, Wyborny, Carolyn,
	John@isotope.jf.intel.com, linux-kernel@vger.kernel.org

After discussing this locally, I'd like to NAK it because this could cause regressions to parts that are still in use but we don't have access to. Also, the assignment was necessary in the past for some versions of gcc and since this may be used in embedded systems using older compilers, we should leave it be.

Thanks.

Todd Fujinaka
Software Application Engineer
Networking Division (ND)
Intel Corporation
todd.fujinaka@intel.com
(503) 712-4565

-----Original Message-----
From: linux-nics-bounces@isotope.jf.intel.com [mailto:linux-nics-bounces@isotope.jf.intel.com] On Behalf Of Sudip Mukherjee
Sent: Sunday, November 30, 2014 8:55 PM
To: Ben Hutchings
Cc: Linux NICS; e1000-devel@lists.sourceforge.net; Hisashi T Fujinaka; Vick, Matthew; Greg@isotope.jf.intel.com; Kirsher, Jeffrey T; netdev@vger.kernel.org; Wyborny, Carolyn; John@isotope.jf.intel.com; linux-kernel@vger.kernel.org
Subject: Re: [linux-nics] [PATCH] e1000: remove unused variables

On Sun, Nov 30, 2014 at 01:45:13AM +0000, Ben Hutchings wrote:
> On Wed, 2014-11-26 at 21:59 -0800, Hisashi T Fujinaka wrote:
> > I'm pretty sure those double reads are there for a reason, so most 
> > of this I'm going to have to check on Monday. We have a long holiday 
> > weekend here in the US.
> [...]
> 
> If there were double register reads being replaced with single 
> register reads, I'd agree this was likely to introduce a regression.  
> But all I see is var = er32(REG) being changed to er32(REG).

no, double register reads are not modified. only the unused variables are removed.

thanks
sudip

> 
> Ben.
> 
> --
> Ben Hutchings
> The world is coming to an end.	Please log off.


_______________________________________________
Linux-nics mailing list
Linux-nics@intel.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [linux-nics] [PATCH] e1000: remove unused variables
  2014-12-01 18:56       ` [linux-nics] " Fujinaka, Todd
@ 2014-12-02 14:24         ` Sudip Mukherjee
  0 siblings, 0 replies; 9+ messages in thread
From: Sudip Mukherjee @ 2014-12-02 14:24 UTC (permalink / raw)
  To: Fujinaka, Todd
  Cc: Ben Hutchings, Linux NICS, e1000-devel@lists.sourceforge.net,
	Hisashi T Fujinaka, Vick, Matthew, Greg@isotope.jf.intel.com,
	Kirsher, Jeffrey T, netdev@vger.kernel.org, Wyborny, Carolyn,
	John@isotope.jf.intel.com, linux-kernel@vger.kernel.org

On Mon, Dec 01, 2014 at 06:56:46PM +0000, Fujinaka, Todd wrote:
> After discussing this locally, I'd like to NAK it because this could cause regressions to parts that are still in use but we don't have access to. Also, the assignment was necessary in the past for some versions of gcc and since this may be used in embedded systems using older compilers, we should leave it be.
> 
ok. i understand.
just a thought:
maybe you can put a comment in the file that these are there for a reason and should not be removed. else, you might receive the same type of patch again from someone else.

thanks
sudip

> Thanks.
> 
> Todd Fujinaka
> Software Application Engineer
> Networking Division (ND)
> Intel Corporation
> todd.fujinaka@intel.com
> (503) 712-4565
> 
> -----Original Message-----
> From: linux-nics-bounces@isotope.jf.intel.com [mailto:linux-nics-bounces@isotope.jf.intel.com] On Behalf Of Sudip Mukherjee
> Sent: Sunday, November 30, 2014 8:55 PM
> To: Ben Hutchings
> Cc: Linux NICS; e1000-devel@lists.sourceforge.net; Hisashi T Fujinaka; Vick, Matthew; Greg@isotope.jf.intel.com; Kirsher, Jeffrey T; netdev@vger.kernel.org; Wyborny, Carolyn; John@isotope.jf.intel.com; linux-kernel@vger.kernel.org
> Subject: Re: [linux-nics] [PATCH] e1000: remove unused variables
> 
> On Sun, Nov 30, 2014 at 01:45:13AM +0000, Ben Hutchings wrote:
> > On Wed, 2014-11-26 at 21:59 -0800, Hisashi T Fujinaka wrote:
> > > I'm pretty sure those double reads are there for a reason, so most 
> > > of this I'm going to have to check on Monday. We have a long holiday 
> > > weekend here in the US.
> > [...]
> > 
> > If there were double register reads being replaced with single 
> > register reads, I'd agree this was likely to introduce a regression.  
> > But all I see is var = er32(REG) being changed to er32(REG).
> 
> no, double register reads are not modified. only the unused variables are removed.
> 
> thanks
> sudip
> 
> > 
> > Ben.
> > 
> > --
> > Ben Hutchings
> > The world is coming to an end.	Please log off.
> 
> 
> _______________________________________________
> Linux-nics mailing list
> Linux-nics@intel.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-12-02 14:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-27  5:22 [PATCH] e1000: remove unused variables Sudip Mukherjee
2014-11-27  5:59 ` Hisashi T Fujinaka
2014-11-27 13:07   ` Sudip Mukherjee
2014-11-28 23:28     ` Florian Fainelli
2014-11-29 11:01       ` Lino Sanfilippo
2014-11-30  1:45   ` Ben Hutchings
2014-12-01  4:54     ` Sudip Mukherjee
2014-12-01 18:56       ` [linux-nics] " Fujinaka, Todd
2014-12-02 14:24         ` Sudip Mukherjee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).