[PATCH] sky2: RX lockup fix

[PATCH] sky2: RX lockup fix

[Peter Tyser]

Hello,
I'm using a Marvell 88E8062 on a custom PPC64 blade and ran into RX
lockups while validating the sky2 driver.  The receive MAC FIFO would
become stuck during testing with high traffic.  One port of the 88E8062
would lockup, while the other port remained functional.  Re-inserting
the sky2 module would not fix the problem - only a power cycle would.

I looked over Marvell's most recent sk98lin driver and it looks like
they had a "workaround" for the Yukon XL that the sky2 doesn't have yet.
The sk98lin driver disables the RX MAC FIFO flush feature for all
revisions of the Yukon XL.

According to skgeinit.c of the sk98lin driver, "Flushing must be enabled
(needed for ASF see dev. #4.29), but the flushing mask should be
disabled (see dev. #4.115)".  Nice.   I implemented this same change in
the sky2 driver and verified that the RX lockup I was seeing was
resolved.


I didn't see the problem mentioned in the netdev list, so I've included
a patch against the
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6 tree
which disables RX flushing for the Yukon XL chips.

Please CC replies to me as I am not subscribed to the list.

Thanks,
Peter Tyser


Signed-off-by: Peter Tyser <ptyser@xes-inc.com>

--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -822,8 +822,13 @@ static void sky2_mac_init(struct sky2_hw *hw,
unsigned port)
 
 	sky2_write32(hw, SK_REG(port, RX_GMF_CTRL_T), rx_reg);
 
-	/* Flush Rx MAC FIFO on any flow control or error */
-	sky2_write16(hw, SK_REG(port, RX_GMF_FL_MSK), GMR_FS_ANY_ERR);
+	if (hw->chip_id == CHIP_ID_YUKON_XL) {
+		/* Hardware errata - clear flush mask */
+		sky2_write16(hw, SK_REG(port, RX_GMF_FL_MSK), 0);
+	} else {
+		/* Flush Rx MAC FIFO on any flow control or error */
+		sky2_write16(hw, SK_REG(port, RX_GMF_FL_MSK), GMR_FS_ANY_ERR);
+	}
 
 	/* Set threshold to 0xa (64 bytes) + 1 to workaround pause bug  */
 	reg = RX_GMF_FL_THR_DEF + 1;

Re: [PATCH] sky2: RX lockup fix

[Stephen Hemminger]

On Wed, 05 Dec 2007 12:51:03 -0600
Peter Tyser <ptyser@xes-inc.com> wrote:

> Hello,
> I'm using a Marvell 88E8062 on a custom PPC64 blade and ran into RX
> lockups while validating the sky2 driver.  The receive MAC FIFO would
> become stuck during testing with high traffic.  One port of the 88E8062
> would lockup, while the other port remained functional.  Re-inserting
> the sky2 module would not fix the problem - only a power cycle would.
> 
> I looked over Marvell's most recent sk98lin driver and it looks like
> they had a "workaround" for the Yukon XL that the sky2 doesn't have yet.
> The sk98lin driver disables the RX MAC FIFO flush feature for all
> revisions of the Yukon XL.
> 
> According to skgeinit.c of the sk98lin driver, "Flushing must be enabled
> (needed for ASF see dev. #4.29), but the flushing mask should be
> disabled (see dev. #4.115)".  Nice.   I implemented this same change in
> the sky2 driver and verified that the RX lockup I was seeing was
> resolved.
> 


Without the flush, does flow control still work? My concern is that
integrating this would cause pause packets (and over/under length packets)
to not be handled correctly.

Re: [PATCH] sky2: RX lockup fix

[Peter Tyser]

On Wed, 2007-12-05 at 16:40 -0500, Stephen Hemminger wrote:
> > I looked over Marvell's most recent sk98lin driver and it looks like
> > they had a "workaround" for the Yukon XL that the sky2 doesn't have yet.
> > The sk98lin driver disables the RX MAC FIFO flush feature for all
> > revisions of the Yukon XL.
> > 
> > According to skgeinit.c of the sk98lin driver, "Flushing must be enabled
> > (needed for ASF see dev. #4.29), but the flushing mask should be
> > disabled (see dev. #4.115)".  Nice.   I implemented this same change in
> > the sky2 driver and verified that the RX lockup I was seeing was
> > resolved.
> > 
> 
> 
> Without the flush, does flow control still work? My concern is that
> integrating this would cause pause packets (and over/under length packets)
> to not be handled correctly.

My understanding is that "bad" packets should still be filtered in
sky2_receive() when a packet's status is compared against
GMR_FS_ANY_ERR.  This comparison should prevent over/under length
packets from making their way up the stack.  This comparison also uses
the same value that was previous programmed to the RX MAC FIFO Flush
Mask, so there shouldn't be any change in the types of bad packets that
are discarded.

I don't believe that disabling RX filtering should affect the handling
of flow control packets specifically either.  The comparison in
sky2_receive() to GMR_FS_ANY_ERR does allow valid flow control packets
to be received. (I'm not intimately familiar with sky2/Linux's handling
of flow control packets, so take the above with a grain of salt)

As I understand it, the only real downside of disabling RX filtering at
the hardware level is that the CPU has to investigate every incoming
packet's status, even the ones that it is going to drop due to length,
crc, etc.  This adds some overhead, but I don't believe it should affect
the driver's operation.

Re: [PATCH] sky2: RX lockup fix

[Stephen Hemminger]

On Wed, 05 Dec 2007 18:18:46 -0600
Peter Tyser <ptyser@xes-inc.com> wrote:

> On Wed, 2007-12-05 at 16:40 -0500, Stephen Hemminger wrote:
> > > I looked over Marvell's most recent sk98lin driver and it looks like
> > > they had a "workaround" for the Yukon XL that the sky2 doesn't have yet.
> > > The sk98lin driver disables the RX MAC FIFO flush feature for all
> > > revisions of the Yukon XL.
> > > 
> > > According to skgeinit.c of the sk98lin driver, "Flushing must be enabled
> > > (needed for ASF see dev. #4.29), but the flushing mask should be
> > > disabled (see dev. #4.115)".  Nice.   I implemented this same change in
> > > the sky2 driver and verified that the RX lockup I was seeing was
> > > resolved.
> > > 
> > 
> > 
> > Without the flush, does flow control still work? My concern is that
> > integrating this would cause pause packets (and over/under length packets)
> > to not be handled correctly.
> 
> My understanding is that "bad" packets should still be filtered in
> sky2_receive() when a packet's status is compared against
> GMR_FS_ANY_ERR.  This comparison should prevent over/under length
> packets from making their way up the stack.  This comparison also uses
> the same value that was previous programmed to the RX MAC FIFO Flush
> Mask, so there shouldn't be any change in the types of bad packets that
> are discarded.
> 
> I don't believe that disabling RX filtering should affect the handling
> of flow control packets specifically either.  The comparison in
> sky2_receive() to GMR_FS_ANY_ERR does allow valid flow control packets
> to be received. (I'm not intimately familiar with sky2/Linux's handling
> of flow control packets, so take the above with a grain of salt)
> 
> As I understand it, the only real downside of disabling RX filtering at
> the hardware level is that the CPU has to investigate every incoming
> packet's status, even the ones that it is going to drop due to length,
> crc, etc.  This adds some overhead, but I don't believe it should affect
> the driver's operation.
> 

I have ways to generate errors, so I'll check

Re: [PATCH] sky2: RX lockup fix

[Peter Tyser]

> I have ways to generate errors, so I'll check

Thanks Stephen.  We didn't spend a lot of time characterizing the issue,
but our test setup had two blades, each with an 88E8062.  Our test
software pumped UDP and TCP traffic of varying packet sizes between the
blades in both directions (including  jumbo frames - we increased the
MTU of the interfaces to 9000).  The issue could generally be brought
out in about 15 minutes and almost always within an hour.

If you'd like any additional details on the test setup or would like me
to try something on my end, let me know.

[PATCH] sky2: RX lockup fix

[Stephen Hemminger]

I'm using a Marvell 88E8062 on a custom PPC64 blade and ran into RX
lockups while validating the sky2 driver.  The receive MAC FIFO would
become stuck during testing with high traffic.  One port of the 88E8062
would lockup, while the other port remained functional.  Re-inserting
the sky2 module would not fix the problem - only a power cycle would.

I looked over Marvell's most recent sk98lin driver and it looks like
they had a "workaround" for the Yukon XL that the sky2 doesn't have yet.
The sk98lin driver disables the RX MAC FIFO flush feature for all
revisions of the Yukon XL.

According to skgeinit.c of the sk98lin driver, "Flushing must be enabled
(needed for ASF see dev. #4.29), but the flushing mask should be
disabled (see dev. #4.115)".  Nice. I implemented this same change in
the sky2 driver and verified that the RX lockup I was seeing was
resolved.

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

---
Original patch reformatted to remove line wrap.

--- a/drivers/net/sky2.c	2007-12-06 09:39:12.000000000 -0800
+++ b/drivers/net/sky2.c	2007-12-06 09:54:14.000000000 -0800
@@ -821,8 +821,13 @@ static void sky2_mac_init(struct sky2_hw
 
 	sky2_write32(hw, SK_REG(port, RX_GMF_CTRL_T), rx_reg);
 
-	/* Flush Rx MAC FIFO on any flow control or error */
-	sky2_write16(hw, SK_REG(port, RX_GMF_FL_MSK), GMR_FS_ANY_ERR);
+	if (hw->chip_id == CHIP_ID_YUKON_XL) {
+		/* Hardware errata - clear flush mask */
+		sky2_write16(hw, SK_REG(port, RX_GMF_FL_MSK), 0);
+	} else {
+		/* Flush Rx MAC FIFO on any flow control or error */
+		sky2_write16(hw, SK_REG(port, RX_GMF_FL_MSK), GMR_FS_ANY_ERR);
+	}
 
 	/* Set threshold to 0xa (64 bytes) + 1 to workaround pause bug  */
 	reg = RX_GMF_FL_THR_DEF + 1;

Re: [PATCH] sky2: RX lockup fix

[Jeff Garzik]

Stephen Hemminger wrote:
> I'm using a Marvell 88E8062 on a custom PPC64 blade and ran into RX
> lockups while validating the sky2 driver.  The receive MAC FIFO would
> become stuck during testing with high traffic.  One port of the 88E8062
> would lockup, while the other port remained functional.  Re-inserting
> the sky2 module would not fix the problem - only a power cycle would.
> 
> I looked over Marvell's most recent sk98lin driver and it looks like
> they had a "workaround" for the Yukon XL that the sky2 doesn't have yet.
> The sk98lin driver disables the RX MAC FIFO flush feature for all
> revisions of the Yukon XL.
> 
> According to skgeinit.c of the sk98lin driver, "Flushing must be enabled
> (needed for ASF see dev. #4.29), but the flushing mask should be
> disabled (see dev. #4.115)".  Nice. I implemented this same change in
> the sky2 driver and verified that the RX lockup I was seeing was
> resolved.
> 
> Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> ---
> Original patch reformatted to remove line wrap.

applied #upstream-fixes