Anton Vorontsov [off-list ref] wrote on 26/03/2009 19:03:54:

On Thu, Mar 26, 2009 at 06:44:05PM +0100, Joakim Tjernlund wrote:

quoted

The line:
 if ((bd == ugeth->txBd[txQ]) && (netif_queue_stopped(dev) == 0))
       break;
in ucc_geth_tx() didn not make sense to me. Rework & cleanup
this logic to something understandable.
---

Reworked the patch according to Antons comments.

 drivers/net/ucc_geth.c |   66 

+++++++++++++++++++++++++++--------------------

quoted

 1 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 7fc91aa..465de3a 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c

@@ -161,6 +161,17 @@ static struct ucc_geth_info ugeth_primary_info = 

{

quoted

 static struct ucc_geth_info ugeth_info[8];

+
+static inline u32 __iomem *bd2buf(u8 __iomem *bd)
+{
+   return (u32 __iomem *)(bd+4);

Spaces around "+"...

Bugger, I forgot when I moved it.

Also, now it's obvious that we're just reading status or buf.

So instead of these inlines we could use struct qe_bd as described
in asm/qe.h.

I.e.

struct qe_bd *bd = ...;

in_be32(&bd->buf);
in_be16(&bd->status);

Oh, wait. We can't, ucc_geth assumes status is u32, which includes
the length field, i.e. ucc_fast.h defines:

#define T_W     0x20000000

:-(

The cleanup work surely desires a separate patch, so bd2buf and
bd2status are OK, for now.

Exactly, I did look at using struct qe_bd *bd, but that will
affect lots of stuff. Reading the status and len in one go is
also quite handy so I not sure it is a good idea to convert.

I'll test the patch as soon as I'll get some QE board back on
my table (actually I have one QE board handy, but it's a 1GHz
one, while I'd like to test the patch on a slow machine, where
we'll actually see performance regressions).

Good, I have more coming but it touches the same code so I really want
to sort out this one first.

 Jocke