Re: [PATCH] sungem: PHY updates pause fixes
On 1/4/07, David Miller [EMAIL PROTECTED] wrote: I've applied that patch, thanks. David, I suppose you've applied the locking patch as well... -- Eric - 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
Re: [PATCH] sungem: PHY updates pause fixes
From: Eric Lemoine [EMAIL PROTECTED] Date: Thu, 4 Jan 2007 21:06:48 +0100 On 1/4/07, David Miller [EMAIL PROTECTED] wrote: I've applied that patch, thanks. David, I suppose you've applied the locking patch as well... No, not yet. Your locking patches are definitely 2.6.21 material, but Ben's changes fix PAUSE handling bugs so I want to put his patch into 2.6.20 I'll put your locking bits into 2.6.21, don't worry :-) - 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
Re: [PATCH] sungem: PHY updates pause fixes
On 1/4/07, David Miller [EMAIL PROTECTED] wrote: From: Eric Lemoine [EMAIL PROTECTED] Date: Thu, 4 Jan 2007 21:06:48 +0100 On 1/4/07, David Miller [EMAIL PROTECTED] wrote: I've applied that patch, thanks. David, I suppose you've applied the locking patch as well... No, not yet. Your locking patches are definitely 2.6.21 material, but Ben's changes fix PAUSE handling bugs so I want to put his patch into 2.6.20 I'll put your locking bits into 2.6.21, don't worry :-) I'm relieved now :-) I was asking because Ben said his patch applied on top of mine. -- Eric - 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
Re: [PATCH] sungem: PHY updates pause fixes
On Thu, 2007-01-04 at 21:57 +0100, Eric Lemoine wrote: On 1/4/07, David Miller [EMAIL PROTECTED] wrote: From: Eric Lemoine [EMAIL PROTECTED] Date: Thu, 4 Jan 2007 21:06:48 +0100 On 1/4/07, David Miller [EMAIL PROTECTED] wrote: I've applied that patch, thanks. David, I suppose you've applied the locking patch as well... No, not yet. Your locking patches are definitely 2.6.21 material, but Ben's changes fix PAUSE handling bugs so I want to put his patch into 2.6.20 I'll put your locking bits into 2.6.21, don't worry :-) I'm relieved now :-) I was asking because Ben said his patch applied on top of mine. Well, I did it with yours applied already but it seems like they don't really conflict so they can be applied pretty much in any order. Ben. - 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
Re: [PATCH] sungem: PHY updates pause fixes
From: Benjamin Herrenschmidt [EMAIL PROTECTED] Date: Wed, 03 Jan 2007 16:40:02 +1100 On the quad G5 which has tg3, I get Flow control is on for TX and on for RX. Let me double check if I'm on the same switch... Heh ! It's not :-) If I switch the cables, then pause is enabled on the sungem box and disabled (both Tx and Rx) on tg3, so at least the behaviour is consistent and follow the switches. Let me check the switch models... The one with only asym. support is a big Cisco Catalyst 3350 (well.. big but not that many ports :-) The one with classic pause support is a tiny Netgear GS116 It's possible that the cisco is just misconfigured though, I'll have to ask Keith when he's around tomorrow to take a peek at the mgmnt interface. Thanks for tracking this down :-) You should try to use flow control, even slower than 1000Mbit links. Sorry, not sure I parse ;-) You mean allowing pause on 10 and 100 as well ? I sure can, it's easy to fix. That's correct. On it's way. I've applied that patch, thanks. - 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
[PATCH] sungem: PHY updates pause fixes
This patch adds support for a few more PHYs used by Apple and fixes
advertising and detecting of Pause (we were missing setting the bit in
MII_ADVERTISE and weren't testing in LPA for all PHYs). I only do it for
gigabit capable PHYs for now.
Note that I currently only advertise pause, not asymetric pause. I don't
know for sure the details there, I suppose I should read a bit more
802.3 references, and I don't now what sungem is capable of, but I
noticed the PCS code (originated from you) does the same.
Unfortunately, whatever switches we have here also seem to only support
asym. pause, so while I did a quick test to verify that pause is
properly enabled on a cross-over cable to another machine, I still get
occasional RX fifo overflows due to pause support lacking on our
internal network.
So let me know if asym. pause is something we can support with sungem.
In which case, it shouldn't be very hard to add in a subsequent patch.
Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED]
---
(Applies on top of Eric's locking patch)
Index: linux-work/drivers/net/sungem_phy.c
===
--- linux-work.orig/drivers/net/sungem_phy.c2007-01-03 12:04:12.0
+1100
+++ linux-work/drivers/net/sungem_phy.c 2007-01-03 15:51:39.0 +1100
@@ -3,10 +3,9 @@
*
* This file could be shared with other drivers.
*
- * (c) 2002, Benjamin Herrenscmidt ([EMAIL PROTECTED])
+ * (c) 2002-2007, Benjamin Herrenscmidt ([EMAIL PROTECTED])
*
* TODO:
- * - Implement WOL
* - Add support for PHYs that provide an IRQ line
* - Eventually moved the entire polling state machine in
*there (out of the eth driver), so that it can easily be
@@ -15,7 +14,7 @@
*to read the link status. Figure out why and if it makes
*sense to do the same (magic aneg ?)
* - Apple has some additional power management code for some
- *Broadcom PHYs that they hide from the OpenSource version
+ *Broadcom PHYs that they hide from the OpenSource version
*of darwin, still need to reverse engineer that
*/
@@ -152,6 +151,44 @@ static int bcm5221_suspend(struct mii_ph
return 0;
}
+static int bcm5241_init(struct mii_phy* phy)
+{
+ u16 data;
+
+ data = phy_read(phy, MII_BCM5221_TEST);
+ phy_write(phy, MII_BCM5221_TEST,
+ data | MII_BCM5221_TEST_ENABLE_SHADOWS);
+
+ data = phy_read(phy, MII_BCM5221_SHDOW_AUX_STAT2);
+ phy_write(phy, MII_BCM5221_SHDOW_AUX_STAT2,
+ data | MII_BCM5221_SHDOW_AUX_STAT2_APD);
+
+ data = phy_read(phy, MII_BCM5221_SHDOW_AUX_MODE4);
+ phy_write(phy, MII_BCM5221_SHDOW_AUX_MODE4,
+ data ~MII_BCM5241_SHDOW_AUX_MODE4_STANDBYPWR);
+
+ data = phy_read(phy, MII_BCM5221_TEST);
+ phy_write(phy, MII_BCM5221_TEST,
+ data ~MII_BCM5221_TEST_ENABLE_SHADOWS);
+
+ return 0;
+}
+
+static int bcm5241_suspend(struct mii_phy* phy)
+{
+ u16 data;
+
+ data = phy_read(phy, MII_BCM5221_TEST);
+ phy_write(phy, MII_BCM5221_TEST,
+ data | MII_BCM5221_TEST_ENABLE_SHADOWS);
+
+ data = phy_read(phy, MII_BCM5221_SHDOW_AUX_MODE4);
+ phy_write(phy, MII_BCM5221_SHDOW_AUX_MODE4,
+ data | MII_BCM5241_SHDOW_AUX_MODE4_STANDBYPWR);
+
+ return 0;
+}
+
static int bcm5400_init(struct mii_phy* phy)
{
u16 data;
@@ -373,6 +410,10 @@ static int bcm54xx_setup_aneg(struct mii
adv |= ADVERTISE_100HALF;
if (advertise ADVERTISED_100baseT_Full)
adv |= ADVERTISE_100FULL;
+ if (advertise ADVERTISED_Pause)
+ adv |= ADVERTISE_PAUSE_CAP;
+ if (advertise ADVERTISED_Asym_Pause)
+ adv |= ADVERTISE_PAUSE_ASYM;
phy_write(phy, MII_ADVERTISE, adv);
/* Setup 1000BT advertise */
@@ -441,7 +482,7 @@ static int bcm54xx_read_link(struct mii_
SPEED_1000 :
(phy_BCM5400_link_table[link_mode][1] ?
SPEED_100 : SPEED_10);
val = phy_read(phy, MII_LPA);
- phy-pause = ((val LPA_PAUSE) != 0);
+ phy-pause = (phy-duplex == DUPLEX_FULL) ((val LPA_PAUSE)
!= 0);
}
/* On non-aneg, we assume what we put in BMCR is the speed,
* though magic-aneg shouldn't prevent this case from occurring
@@ -450,6 +491,28 @@ static int bcm54xx_read_link(struct mii_
return 0;
}
+static int marvell88e_init(struct mii_phy* phy)
+{
+ u16 rev;
+
+ /* magic init sequence for rev 0 */
+ rev = phy_read(phy, MII_PHYSID2) 0x000f;
+ if (rev == 0) {
+ phy_write(phy, 0x1d, 0x000a);
+ phy_write(phy, 0x1e, 0x0821);
+
+ phy_write(phy, 0x1d, 0x0006);
+ phy_write(phy, 0x1e, 0x8600);
+
+ phy_write(phy, 0x1d, 0x000b);
+ phy_write(phy, 0x1e, 0x0100);
+
+ phy_write(phy, 0x1d,
Re: [PATCH] sungem: PHY updates pause fixes
From: Benjamin Herrenschmidt [EMAIL PROTECTED] Date: Wed, 03 Jan 2007 15:58:05 +1100 This patch adds support for a few more PHYs used by Apple and fixes advertising and detecting of Pause (we were missing setting the bit in MII_ADVERTISE and weren't testing in LPA for all PHYs). I only do it for gigabit capable PHYs for now. Note that I currently only advertise pause, not asymetric pause. I don't know for sure the details there, I suppose I should read a bit more 802.3 references, and I don't now what sungem is capable of, but I noticed the PCS code (originated from you) does the same. Unfortunately, whatever switches we have here also seem to only support asym. pause, so while I did a quick test to verify that pause is properly enabled on a cross-over cable to another machine, I still get occasional RX fifo overflows due to pause support lacking on our internal network. So let me know if asym. pause is something we can support with sungem. In which case, it shouldn't be very hard to add in a subsequent patch. Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED] Thanks for finding these bugs, although that's really strange pause behavior you are seeing on your switches. By default, we advertise PAUSE but not ASYM PAUSE in the tg3 driver, and I get flow control on every switch I have here. You should try to use flow control, even slower than 1000Mbit links. That's the only problem I can see, would you mind fixing that and I'll put your change into my net-2.6 tree and perhaps play around with PAUSE on my switches here? Thanks! - 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
Re: [PATCH] sungem: PHY updates pause fixes
Thanks for finding these bugs, although that's really strange pause behavior you are seeing on your switches. By default, we advertise PAUSE but not ASYM PAUSE in the tg3 driver, and I get flow control on every switch I have here. Yeah, that's strange. I still have the debug values at hand: - I advertise 0x5e1 - I read in LPA 0xc9e1 from the switch (and my bcm PHY tells me Rx and Tx pause disabled in a separate register) Now, I cross-over with a TG3 and I get: - I advertise 0x5e1 (hopefully same value) - I read in LPA 0xc5e1 from the TG3 (and that other register tells me Rx and Tx pause can be enabled). You should try to use flow control, even slower than 1000Mbit links. Sorry, not sure I parse ;-) You mean allowing pause on 10 and 100 as well ? I sure can, it's easy to fix. That's the only problem I can see, would you mind fixing that and I'll put your change into my net-2.6 tree and perhaps play around with PAUSE on my switches here? Sure will do. Ben. - 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
Re: [PATCH] sungem: PHY updates pause fixes
From: Benjamin Herrenschmidt [EMAIL PROTECTED] Date: Wed, 03 Jan 2007 16:20:14 +1100 Now, I cross-over with a TG3 and I get: - I advertise 0x5e1 (hopefully same value) - I read in LPA 0xc5e1 from the TG3 (and that other register tells me Rx and Tx pause can be enabled). Does flow control get enabled with the TG3 on these switches? Just curious. You should try to use flow control, even slower than 1000Mbit links. Sorry, not sure I parse ;-) You mean allowing pause on 10 and 100 as well ? I sure can, it's easy to fix. That's correct. That's the only problem I can see, would you mind fixing that and I'll put your change into my net-2.6 tree and perhaps play around with PAUSE on my switches here? Sure will do. Thanks a lot. - 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
[PATCH] sungem: PHY updates pause fixes (#2)
This patch adds support for a few more PHYs used by Apple and fixes
advertising and detecting of Pause (we were missing setting the bit in
MII_ADVERTISE and weren't testing in LPA for all PHYs).
Note that I currently only advertise pause, not asymetric pause. I don't
know for sure the details there, I suppose I should read a bit more
802.3 references, and I don't now what sungem is capable of, but I
noticed the PCS code (originated from you) does the same.
Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED]
---
(Applies on top of Eric's locking patch)
This version advertise pause support for all speeds. Note that I think
pause support was never broken for PCS, only for MII.
Index: linux-work/drivers/net/sungem_phy.c
===
--- linux-work.orig/drivers/net/sungem_phy.c2007-01-03 12:04:12.0
+1100
+++ linux-work/drivers/net/sungem_phy.c 2007-01-03 16:28:55.0 +1100
@@ -3,10 +3,9 @@
*
* This file could be shared with other drivers.
*
- * (c) 2002, Benjamin Herrenscmidt ([EMAIL PROTECTED])
+ * (c) 2002-2007, Benjamin Herrenscmidt ([EMAIL PROTECTED])
*
* TODO:
- * - Implement WOL
* - Add support for PHYs that provide an IRQ line
* - Eventually moved the entire polling state machine in
*there (out of the eth driver), so that it can easily be
@@ -152,6 +151,44 @@ static int bcm5221_suspend(struct mii_ph
return 0;
}
+static int bcm5241_init(struct mii_phy* phy)
+{
+ u16 data;
+
+ data = phy_read(phy, MII_BCM5221_TEST);
+ phy_write(phy, MII_BCM5221_TEST,
+ data | MII_BCM5221_TEST_ENABLE_SHADOWS);
+
+ data = phy_read(phy, MII_BCM5221_SHDOW_AUX_STAT2);
+ phy_write(phy, MII_BCM5221_SHDOW_AUX_STAT2,
+ data | MII_BCM5221_SHDOW_AUX_STAT2_APD);
+
+ data = phy_read(phy, MII_BCM5221_SHDOW_AUX_MODE4);
+ phy_write(phy, MII_BCM5221_SHDOW_AUX_MODE4,
+ data ~MII_BCM5241_SHDOW_AUX_MODE4_STANDBYPWR);
+
+ data = phy_read(phy, MII_BCM5221_TEST);
+ phy_write(phy, MII_BCM5221_TEST,
+ data ~MII_BCM5221_TEST_ENABLE_SHADOWS);
+
+ return 0;
+}
+
+static int bcm5241_suspend(struct mii_phy* phy)
+{
+ u16 data;
+
+ data = phy_read(phy, MII_BCM5221_TEST);
+ phy_write(phy, MII_BCM5221_TEST,
+ data | MII_BCM5221_TEST_ENABLE_SHADOWS);
+
+ data = phy_read(phy, MII_BCM5221_SHDOW_AUX_MODE4);
+ phy_write(phy, MII_BCM5221_SHDOW_AUX_MODE4,
+ data | MII_BCM5241_SHDOW_AUX_MODE4_STANDBYPWR);
+
+ return 0;
+}
+
static int bcm5400_init(struct mii_phy* phy)
{
u16 data;
@@ -373,6 +410,10 @@ static int bcm54xx_setup_aneg(struct mii
adv |= ADVERTISE_100HALF;
if (advertise ADVERTISED_100baseT_Full)
adv |= ADVERTISE_100FULL;
+ if (advertise ADVERTISED_Pause)
+ adv |= ADVERTISE_PAUSE_CAP;
+ if (advertise ADVERTISED_Asym_Pause)
+ adv |= ADVERTISE_PAUSE_ASYM;
phy_write(phy, MII_ADVERTISE, adv);
/* Setup 1000BT advertise */
@@ -436,12 +477,15 @@ static int bcm54xx_read_link(struct mii_
val = phy_read(phy, MII_BCM5400_AUXSTATUS);
link_mode = ((val MII_BCM5400_AUXSTATUS_LINKMODE_MASK)
MII_BCM5400_AUXSTATUS_LINKMODE_SHIFT);
- phy-duplex = phy_BCM5400_link_table[link_mode][0] ?
DUPLEX_FULL : DUPLEX_HALF;
+ phy-duplex = phy_BCM5400_link_table[link_mode][0] ?
+ DUPLEX_FULL : DUPLEX_HALF;
phy-speed = phy_BCM5400_link_table[link_mode][2] ?
SPEED_1000 :
- (phy_BCM5400_link_table[link_mode][1] ?
SPEED_100 : SPEED_10);
+ (phy_BCM5400_link_table[link_mode][1] ?
+SPEED_100 : SPEED_10);
val = phy_read(phy, MII_LPA);
- phy-pause = ((val LPA_PAUSE) != 0);
+ phy-pause = (phy-duplex == DUPLEX_FULL)
+ ((val LPA_PAUSE) != 0);
}
/* On non-aneg, we assume what we put in BMCR is the speed,
* though magic-aneg shouldn't prevent this case from occurring
@@ -450,6 +494,28 @@ static int bcm54xx_read_link(struct mii_
return 0;
}
+static int marvell88e_init(struct mii_phy* phy)
+{
+ u16 rev;
+
+ /* magic init sequence for rev 0 */
+ rev = phy_read(phy, MII_PHYSID2) 0x000f;
+ if (rev == 0) {
+ phy_write(phy, 0x1d, 0x000a);
+ phy_write(phy, 0x1e, 0x0821);
+
+ phy_write(phy, 0x1d, 0x0006);
+ phy_write(phy, 0x1e, 0x8600);
+
+ phy_write(phy, 0x1d, 0x000b);
+ phy_write(phy, 0x1e, 0x0100);
+
+ phy_write(phy, 0x1d, 0x0004);
+ phy_write(phy, 0x1e, 0x4850);
+ }
+ return 0;
+}
+
static int
Re: [PATCH] sungem: PHY updates pause fixes
The one with only asym. support is a big Cisco Catalyst 3350 (well.. big but not that many ports :-) Ok, I got in the config of the switch with somebody who knows how to speak ciscong, and it seems that it defaults to flow control desired for send and off for receive on all ports, which means it will indeed only advertise support for asymetrical flow control. We've changed the setting for the port on which my sungem g5 is connected to desired on both send and receive, and it now advertises flow control on both, and sungem does pick it up properly. So it's indeed the default setting of those cisco switches that seem to not quite match what drivers like sungem or tg3 like. Ben. - 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
