net: fec: fix calculation of free packets

The FEC maintains two pointers into the transmit ring:

next  - this is the insertion point in the ring, and points at a free
        entry which can be written to.
dirty - this is the last dirty entry which we successfully cleaned, and
        is always incremented prior to cleaning an entry.

The calculation used for the number of free packets is slightly buggy,
which is:

	entries = dirty - next - 1
	if (entries < 0)
		entries += size;

Let's take some examples:

At ring initialisation, dirty is set to size-1, and next is set to 0.
This gives:

	entries = size-1 - 0 - 1 = size - 2 => size - 2

But hang on, we have no packets in the empty ring, so why "size - 2" ?
Let's also check if we back the pointers up by one position - so
dirty=size-2, next=size-1.

	entries = size-2 - size-1 - 1 = -2 - -1 - 1 = -2 => size - 2

Okay, so that's the same.  Now, what about the "ring full" criteria.
We can never completely fill the transmit ring, because a completely
full ring is indistinguishable from a completely empty ring.  We
reserve one entry to permit us to keep a distinction.

Hence, our "full" case is when both pointers are equal, so dirty=size-1,
next=size-1.

	entries = size-1 - size-1 - 1 = -1 => size - 1

This is where things break down - in this case, the function is not
returning the number of free entries in the ring, because it should
be zero!

Fix this by changing the calculation to something which reflects the
actual ring behaviour:

	entries = dirty - next;
	if (entries < 0)
		entries += size;

Plugging the above three cases into this gives:

	entries = size-1 - 0 = size - 1 => size - 1
	entries = size-2 - size-1 = -1 => size - 1
	entries = size-1 - size-1 = 0 => 0

Here, we have more correct behaviour (remembering that we have reserved
one entry as described above).

The perverse thing is that every test at this function's called site
almost took account of the off-by-one error.  Let's fix this to have
saner semantics - returning the number of permissible free entries in
the ring which can then be compared using expected tests against our
required numbers of packets.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

1 files changed, 5 insertions, 8 deletions

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d48e16da198a..85841fb1dfda 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -323,14 +323,11 @@ static int fec_enet_get_bd_index(struct bufdesc *base, struct bufdesc *bdp,
 	return ((const char *)bdp - (const char *)base) / fep->bufdesc_size;
 }
 
-static int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep)
+static unsigned int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep)
 {
-	int entries;
-
-	entries = ((const char *)fep->dirty_tx -
-			(const char *)fep->cur_tx) / fep->bufdesc_size - 1;
-
-	return entries > 0 ? entries : entries + fep->tx_ring_size;
+	int num = ((const char *)fep->dirty_tx -
+			(const char *)fep->cur_tx) / fep->bufdesc_size;
+	return num < 0 ? num + fep->tx_ring_size : num;
 }
 
 static void *swap_buffer(void *bufaddr, int len)
@@ -808,7 +805,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	if (ret)
 		return ret;
 
-	if (fec_enet_get_free_txdesc_num(fep) <= fep->tx_stop_threshold)
+	if (fec_enet_get_free_txdesc_num(fep) < fep->tx_stop_threshold)
 		netif_stop_queue(ndev);
 
 	return NETDEV_TX_OK;