2014-11-27 05:22:24

by Sudip Mukherjee

[permalink] [raw]
Subject: [PATCH] e1000: remove unused variables

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

Signed-off-by: Sudip Mukherjee <[email protected]>
---
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


2014-11-27 06:09:14

by Hisashi T Fujinaka

[permalink] [raw]
Subject: Re: [PATCH] e1000: remove unused variables

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 [email protected] 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 <[email protected]>
> ---
> 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 - [email protected]
BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee

2014-11-27 13:07:41

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [PATCH] e1000: remove unused variables

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 [email protected] 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 <[email protected]>
> >---
> >drivers/net/ethernet/intel/e1000/e1000_hw.c | 142 ++++++++++++--------------
<snip>
> > case SPEED_100:
> >- txb2b = false;
> > /* maybe add some timeout factor ? */
> > break;
> > }
> >
>
> --
> Hisashi T Fujinaka - [email protected]
> BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee

2014-11-28 23:29:02

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH] e1000: remove unused variables

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 [email protected] 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 <[email protected]>
>>> ---
>>> drivers/net/ethernet/intel/e1000/e1000_hw.c | 142 ++++++++++++--------------
> <snip>
>>> case SPEED_100:
>>> - txb2b = false;
>>> /* maybe add some timeout factor ? */
>>> break;
>>> }
>>>
>>
>> --
>> Hisashi T Fujinaka - [email protected]
>> BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-11-29 11:01:19

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH] e1000: remove unused variables

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

2014-11-30 01:45:41

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH] e1000: remove unused variables

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.


Attachments:
signature.asc (811.00 B)
This is a digitally signed message part

2014-12-01 04:55:00

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [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.

2014-12-01 18:56:58

by Fujinaka, Todd

[permalink] [raw]
Subject: RE: [linux-nics] [PATCH] e1000: remove unused variables

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
[email protected]
(503) 712-4565

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Sudip Mukherjee
Sent: Sunday, November 30, 2014 8:55 PM
To: Ben Hutchings
Cc: Linux NICS; [email protected]; Hisashi T Fujinaka; Vick, Matthew; [email protected]; Kirsher, Jeffrey T; [email protected]; Wyborny, Carolyn; [email protected]; [email protected]
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
[email protected]

2014-12-02 14:24:54

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [linux-nics] [PATCH] e1000: remove unused variables

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
> [email protected]
> (503) 712-4565
>
> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Sudip Mukherjee
> Sent: Sunday, November 30, 2014 8:55 PM
> To: Ben Hutchings
> Cc: Linux NICS; [email protected]; Hisashi T Fujinaka; Vick, Matthew; [email protected]; Kirsher, Jeffrey T; [email protected]; Wyborny, Carolyn; [email protected]; [email protected]
> 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
> [email protected]