Thread (40 messages) 40 messages, 9 authors, 2013-12-20

RE: [PATCH v2 1/9] drivers: net: Remove unused function igb_get_eee_status_i354() in e1000_82575.c

From: Wyborny, Carolyn <hidden>
Date: 2013-12-16 18:51:49
Also in: lkml

-----Original Message-----
From: Josh Triplett [mailto:josh@joshtriplett.org]
Sent: Monday, December 16, 2013 10:22 AM
To: Wyborny, Carolyn
Cc: Rashika Kheria; linux-kernel@vger.kernel.org; Kirsher, Jeffrey T; Brandeburg,
Jesse; Allan, Bruce W; Skidmore, Donald C; Rose, Gregory V; Waskiewicz Jr,
Peter P; Duyck, Alexander H; Ronciak, John; Dave, Tushar N; Abodunrin, Akeem
G; Vick, Matthew; e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org
Subject: Re: [PATCH v2 1/9] drivers: net: Remove unused function
igb_get_eee_status_i354() in e1000_82575.c

On Mon, Dec 16, 2013 at 06:17:40PM +0000, Wyborny, Carolyn wrote:
quoted
quoted
-----Original Message-----
From: Josh Triplett [mailto:josh@joshtriplett.org]
Sent: Monday, December 16, 2013 9:43 AM
To: Wyborny, Carolyn
Cc: Rashika Kheria; linux-kernel@vger.kernel.org; Kirsher, Jeffrey
T; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C; Rose,
Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; Ronciak,
John; Dave, Tushar N; Abodunrin, Akeem G; Vick, Matthew;
e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org
Subject: Re: [PATCH v2 1/9] drivers: net: Remove unused function
igb_get_eee_status_i354() in e1000_82575.c

On Mon, Dec 16, 2013 at 05:11:25PM +0000, Wyborny, Carolyn wrote:
quoted
quoted
-----Original Message-----
From: Rashika Kheria [mailto:rashika.kheria@gmail.com]
Sent: Saturday, December 14, 2013 4:15 AM
To: linux-kernel@vger.kernel.org
Cc: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W;
Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V;
Waskiewicz Jr, Peter P; Duyck, Alexander H; Ronciak, John; Dave,
Tushar N; Abodunrin, Akeem G; Vick, Matthew; e1000-
devel@lists.sourceforge.net; netdev@vger.kernel.org;
josh@joshtriplett.org
Subject: [PATCH v2 1/9] drivers: net: Remove unused function
igb_get_eee_status_i354() in e1000_82575.c

This patch removes the function igb_get_eee_status_i354() in
e1000_82575.c because it is unused.

It thus eliminates the following warning in
ethernet/intel/igb/e1000_82575.c:
drivers/net/ethernet/intel/igb/e1000_82575.c:2591:5: warning: no
previous prototype for ‘igb_get_eee_status_i354’
[-Wmissing-prototypes]

Signed-off-by: Rashika Kheria <redacted>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
---
 drivers/net/ethernet/intel/igb/e1000_82575.c |   32 ------------------------
--
quoted
quoted
quoted
quoted
 1 file changed, 32 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c
b/drivers/net/ethernet/intel/igb/e1000_82575.c
index 47c2d10..18e5200 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -2580,38 +2580,6 @@ out:
 	return ret_val;
 }

-/**
- *  igb_get_eee_status_i354 - Get EEE status
- *  @hw: pointer to the HW structure
- *  @status: EEE status
- *
- *  Get EEE status by guessing based on whether Tx or Rx LPI
indications have
- *  been received.
- **/
-s32 igb_get_eee_status_i354(struct e1000_hw *hw, bool *status) -{
-	struct e1000_phy_info *phy = &hw->phy;
-	s32 ret_val = 0;
-	u16 phy_data;
-
-	/* Check if EEE is supported on this device. */
-	if ((hw->phy.media_type != e1000_media_type_copper) ||
-	    (phy->id != M88E1543_E_PHY_ID))
-		goto out;
-
-	ret_val = igb_read_xmdio_reg(hw, E1000_PCS_STATUS_ADDR_I354,
-				     E1000_PCS_STATUS_DEV_I354,
-				     &phy_data);
-	if (ret_val)
-		goto out;
-
-	*status = phy_data & (E1000_PCS_STATUS_TX_LPI_RCVD |
-			      E1000_PCS_STATUS_RX_LPI_RCVD) ? true : false;
-
-out:
-	return ret_val;
-}
-
 static const u8 e1000_emc_temp_data[4] = {
 	E1000_EMC_INTERNAL_DATA,
 	E1000_EMC_DIODE1_DATA,
--
1.7.9.5
NACK.

Thanks for the patch Rashika, but this is the incorrect fix for
this warning  The function is called in the igb_probe function, so
you cannot remove it and I see the prototype in the e1000_82575.h file.
Can you double check your source pull?
From top-of-tree linux.git:

~/src/linux$ git grep igb_get_eee_status_i354
drivers/net/ethernet/intel/igb/e1000_82575.c: *
igb_get_eee_status_i354 - Get EEE status
drivers/net/ethernet/intel/igb/e1000_82575.c:s32jjj
igb_get_eee_status_i354(struct e1000_hw *hw, bool *status)

A comment and the function itself; no other references.  In what
tree are you seeing a reference to igb_get_eee_status_i354?

- Josh Triplett
Thanks Josh,

My mistake.  I was looking at the "get" not "set" function.  However, this is still
not the right fix for the driver as the actual problem is the missing code that is
supposed to be calling that function in igb_get_eee in igb_ethtool.c.  The
function needs to stay.  I will submit the correct fix ASAP.

Ah, that makes sense.  Can you please Cc Rashika and myself on that patch, with
a Reported-by for Rashika?

- Josh Triplett
Of course.  I'll add a Reported-by for you as well.

Thanks,

Carolyn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help