igb: reset PHY after recovering from PHY power down

PATCH Superseded Koki Sanagi 2 participants 5 messages Nov 24, 2011 – Dec 19, 2011

Subsystems (3)

Networking Network drivers Intel Ethernet

lore

Share & tools

lore

Koki Sanagi

author

2011-11-24 03:18 UTC #0 lore raw

PATCH 1/1 igb: reset PHY after recovering from PHY power down

According to 82576_Datasheet.pdf, PHY setting is lost after PHY power down. So resetting PHY is needed when recovering from PHY power down to set a default setting to PHY register.

Owing to this lack, NIC doesn't link up in some rare situation. The situation I encountered is following.

1.Both ports connect to switch.

+---------+           +--------+
|         |-----------|        |
| 82576NS |           | switch |
|         |-----------|        |
+---------+           +--------+

2.Detach both cables from switch.

+---------+           +--------+
|         |-------    |        |
| 82576NS |           | switch |
|         |-------    |        |
+---------+           +--------+

3.Detach one cable from one port.

+---------+           +--------+
|         |-------    |        |
| 82576NS |           | switch |
|         |           |        |
+---------+           +--------+

4.Attach that cable to the other port.(It means connecting directly each port)

+---------+           +--------+
|         |-------+   |        |
| 82576NS |       |   | switch |
|         |-------+   |        |
+---------+           +--------+

As a result, NIC doesn't link up sometimes.

Signed-off-by Koki Sanagi <[email protected]>

---
 drivers/net/ethernet/intel/igb/igb_main.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/net/ethernet/intel/igb/igb_main.c +1 -0

index bd9b30e..4d4f065 100644
@@ -2496,6 +2496,7 @@ static int igb_open(struct net_device *netdev)
 		goto err_setup_rx;
 
 	igb_power_up_link(adapter);
+	igb_reset_phy(hw);
 
 	/* before we allocate an interrupt, we must be ready to handle it.
 	 * Setting DEBUG_SHIRQ in the kernel makes it fire an interrupt

This seems reasonable, however it would only cover cases where igb_open is being called. It would be better to integrate the phy_reset call into igb_power_up_link, so that's the default. I can provide an alternate by end of the week.

Thanks,

Carolyn

Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation

Koki Sanagi

author

2011-12-14 05:55 UTC #2 lore raw

Quoted 70 lines · Carolyn

Quoted 70 lines
-----Original Message-----
From: Koki Sanagi [mailto:[email protected]]
Sent: Wednesday, November 23, 2011 7:18 PM
To: [email protected]; 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; e1000- [email protected]
Cc: [email protected]
Subject: [PATCH net-next] igb: reset PHY after recovering from PHY power down
According to 82576_Datasheet.pdf, PHY setting is lost after PHY power down.
So resetting PHY is needed when recovering from PHY power down to set a default
setting to PHY register.
Owing to this lack, NIC doesn't link up in some rare situation. The situation I encountered is following.
1.Both ports connect to switch.
+---------+           +--------+
|         |-----------|        |
| 82576NS |           | switch |
|         |-----------|        |
+---------+           +--------+
2.Detach both cables from switch.
+---------+           +--------+
|         |-------    |        |
| 82576NS |           | switch |
|         |-------    |        |
+---------+           +--------+
3.Detach one cable from one port.
+---------+           +--------+
|         |-------    |        |
| 82576NS |           | switch |
|         |           |        |
+---------+           +--------+
4.Attach that cable to the other port.(It means connecting directly each port)
+---------+           +--------+
|         |-------+   |        |
| 82576NS |       |   | switch |
|         |-------+   |        |
+---------+           +--------+
As a result, NIC doesn't link up sometimes.
Signed-off-by Koki Sanagi <[email protected]>
---
drivers/net/ethernet/intel/igb/igb_main.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
drivers/net/ethernet/intel/igb/igb_main.c +1 -0
index bd9b30e..4d4f065 100644
@@ -2496,6 +2496,7 @@ static int igb_open(struct net_device *netdev)
 		goto err_setup_rx;
 
 	igb_power_up_link(adapter);
+	igb_reset_phy(hw);
 

Quoted 7 lines

drivers/net/ethernet/intel/igb/igb_main.c static int igb_open(struct net_device *netdev) · line 2502
Quoted 2 lines
drivers/net/ethernet/intel/igb/igb_main.c +0 -0
 	/* before we allocate an interrupt, we must be ready to handle it.
 	 * Setting DEBUG_SHIRQ in the kernel makes it fire an interrupt
Hello,
This seems reasonable, however it would only cover cases where igb_open is being called. It would be better to integrate the phy_reset call into igb_power_up_link, so that's the default. I can provide an alternate by end of the week.

When will the alternate be provided ?

In addition, more general problem which this lack of reset causes is found.

Two machine have 82576NS and each NIC is connected directly each other.

+--------+                           +--------+
|        |                           |        |
|        +-------+           +-------+        |
|        |       |           |       |        |
|machineA|82576NS|-----------|82576NS|machineB|
|        |       |           |       |        |
|        +-------+           +-------+        |
|        |                           |        |
+--------+                           +--------+

When attaching and detaching a cable repeatedly on one side port, that side port becomes not link up.

This is also fixed by adding resetting phy.

Thanks,
Koki Sanagi

Quoted 12 lines

Thanks,
Carolyn
Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation

------------------------------------------------------------------------------
Cloud Computing - Latest Buzzword or a Glimpse of the Future?
This paper surveys cloud computing today: What are the benefits?
Why are businesses embracing it? What are its payoffs and pitfalls?
http://www.accelacomm.com/jaw/sdnl/114/51425149/
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired

Wyborny, Carolyn

2011-12-16 15:53 UTC #3 lore raw

In reply to Koki Sanagi

Quoted 65 lines

-----Original Message-----
From: Koki Sanagi [mailto:[email protected]]
Sent: Wednesday, November 23, 2011 7:18 PM
To: [email protected]; 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; e1000- [email protected]
Cc: [email protected]
Subject: [PATCH net-next] igb: reset PHY after recovering from PHY power down
According to 82576_Datasheet.pdf, PHY setting is lost after PHY power down.
So resetting PHY is needed when recovering from PHY power down to set a default
setting to PHY register.
Owing to this lack, NIC doesn't link up in some rare situation. The situation I encountered is following.
1.Both ports connect to switch.
+---------+           +--------+
|         |-----------|        |
| 82576NS |           | switch |
|         |-----------|        |
+---------+           +--------+
2.Detach both cables from switch.
+---------+           +--------+
|         |-------    |        |
| 82576NS |           | switch |
|         |-------    |        |
+---------+           +--------+
3.Detach one cable from one port.
+---------+           +--------+
|         |-------    |        |
| 82576NS |           | switch |
|         |           |        |
+---------+           +--------+
4.Attach that cable to the other port.(It means connecting directly each port)
+---------+           +--------+
|         |-------+   |        |
| 82576NS |       |   | switch |
|         |-------+   |        |
+---------+           +--------+
As a result, NIC doesn't link up sometimes.
Signed-off-by Koki Sanagi <[email protected]>
---
drivers/net/ethernet/intel/igb/igb_main.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
drivers/net/ethernet/intel/igb/igb_main.c +0 -0
index bd9b30e..4d4f065 100644

drivers/net/ethernet/intel/igb/igb_main.c static int igb_open(struct net_device *netdev) · line 2502

Quoted 7 lines

drivers/net/ethernet/intel/igb/igb_main.c +1 -0

@@ -2496,6 +2496,7 @@ static int igb_open(struct net_device *netdev)
 		goto err_setup_rx;
 
 	igb_power_up_link(adapter);
+	igb_reset_phy(hw);
 
 	/* before we allocate an interrupt, we must be ready to handle it.
 	 * Setting DEBUG_SHIRQ in the kernel makes it fire an interrupt

I apologize for the delay in getting back to you. After a review, I see that the needed change is simpler than I thought at first. Can you resubmit with the igb_reset_phy call being added to the end of the igb_power_up_link function instead of after it in igb_open. That way, whenever igb_power_link is called in the code, it will also execute the igb_reset_phy function which is the complete solution. Thank you for your work on the igb driver.

Carolyn

Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation

Koki Sanagi

author

2011-12-19 00:00 UTC #4 lore raw

Quoted 70 lines · Carolyn

Quoted 70 lines
-----Original Message-----
From: Koki Sanagi [mailto:[email protected]]
Sent: Wednesday, November 23, 2011 7:18 PM
To: [email protected]; 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; e1000- [email protected]
Cc: [email protected]
Subject: [PATCH net-next] igb: reset PHY after recovering from PHY power down
According to 82576_Datasheet.pdf, PHY setting is lost after PHY power down.
So resetting PHY is needed when recovering from PHY power down to set a default
setting to PHY register.
Owing to this lack, NIC doesn't link up in some rare situation. The situation I encountered is following.
1.Both ports connect to switch.
+---------+           +--------+
|         |-----------|        |
| 82576NS |           | switch |
|         |-----------|        |
+---------+           +--------+
2.Detach both cables from switch.
+---------+           +--------+
|         |-------    |        |
| 82576NS |           | switch |
|         |-------    |        |
+---------+           +--------+
3.Detach one cable from one port.
+---------+           +--------+
|         |-------    |        |
| 82576NS |           | switch |
|         |           |        |
+---------+           +--------+
4.Attach that cable to the other port.(It means connecting directly each port)
+---------+           +--------+
|         |-------+   |        |
| 82576NS |       |   | switch |
|         |-------+   |        |
+---------+           +--------+
As a result, NIC doesn't link up sometimes.
Signed-off-by Koki Sanagi <[email protected]>
---
drivers/net/ethernet/intel/igb/igb_main.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
drivers/net/ethernet/intel/igb/igb_main.c +1 -0
index bd9b30e..4d4f065 100644
@@ -2496,6 +2496,7 @@ static int igb_open(struct net_device *netdev)
 		goto err_setup_rx;
 
 	igb_power_up_link(adapter);
+	igb_reset_phy(hw);
 

Quoted 10 lines

drivers/net/ethernet/intel/igb/igb_main.c static int igb_open(struct net_device *netdev) · line 2502
Quoted 2 lines
drivers/net/ethernet/intel/igb/igb_main.c +0 -0
 	/* before we allocate an interrupt, we must be ready to handle it.
 	 * Setting DEBUG_SHIRQ in the kernel makes it fire an interrupt
I apologize for the delay in getting back to you. After a review, I see that the needed change is simpler than I thought at first. Can you resubmit with the igb_reset_phy call being added to the end of the igb_power_up_link function instead of after it in igb_open. That way, whenever igb_power_link is called in the code, it will also execute the igb_reset_phy function which is the complete solution. Thank you for your work on the igb driver.

Thank you for your reply.

O.K. I'll resubmit it soon.

Thanks!
Koki Sanagi

Quoted 15 lines

Carolyn
Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation
--
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