[PATCH 0/2] myri10ge update for 2.6.27

[PATCH 0/2] myri10ge update for 2.6.27

[Brice Goglin]

Hello Jeff,

Here are 2 small myri10ge patches for 2.6.27:
1) remove the obsolete wcfifo code
2) use ioremap_wc()

A multiqueue TX patch is underwork but I am not sure it will be ready
for 2.6.27. We'll see.

thanks,
Brice

[PATCH 1/2] myri10ge: remove wcfifo

[Brice Goglin]

Remove the wcfifo since it never gave any performance improvement.

Signed-off-by: Brice Goglin <brice@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |   54 ++--------------------------------------
 1 file changed, 3 insertions(+), 51 deletions(-)

Index: linux-2.6/drivers/net/myri10ge/myri10ge.c
===================================================================
--- linux-2.6.orig/drivers/net/myri10ge/myri10ge.c	2008-07-21 09:23:20.000000000 +0200
+++ linux-2.6/drivers/net/myri10ge/myri10ge.c	2008-07-21 09:24:21.000000000 +0200
@@ -125,7 +125,6 @@
 
 struct myri10ge_rx_buf {
 	struct mcp_kreq_ether_recv __iomem *lanai;	/* lanai ptr for recv ring */
-	u8 __iomem *wc_fifo;	/* w/c rx dma addr fifo address */
 	struct mcp_kreq_ether_recv *shadow;	/* host shadow of recv ring */
 	struct myri10ge_rx_buffer_state *info;
 	struct page *page;
@@ -140,7 +139,6 @@
 
 struct myri10ge_tx_buf {
 	struct mcp_kreq_ether_send __iomem *lanai;	/* lanai ptr for sendq */
-	u8 __iomem *wc_fifo;	/* w/c send fifo address */
 	struct mcp_kreq_ether_send *req_list;	/* host shadow of sendq */
 	char *req_bytes;
 	struct myri10ge_tx_buffer_state *info;
@@ -332,10 +330,6 @@
 
 static int myri10ge_reset_recover = 1;
 
-static int myri10ge_wcfifo = 0;
-module_param(myri10ge_wcfifo, int, S_IRUGO);
-MODULE_PARM_DESC(myri10ge_wcfifo, "Enable WC Fifo when WC is enabled");
-
 static int myri10ge_max_slices = 1;
 module_param(myri10ge_max_slices, int, S_IRUGO);
 MODULE_PARM_DESC(myri10ge_max_slices, "Max tx/rx queues");
@@ -1218,14 +1212,8 @@
 
 		/* copy 8 descriptors to the firmware at a time */
 		if ((idx & 7) == 7) {
-			if (rx->wc_fifo == NULL)
-				myri10ge_submit_8rx(&rx->lanai[idx - 7],
-						    &rx->shadow[idx - 7]);
-			else {
-				mb();
-				myri10ge_pio_copy(rx->wc_fifo,
-						  &rx->shadow[idx - 7], 64);
-			}
+			myri10ge_submit_8rx(&rx->lanai[idx - 7],
+					    &rx->shadow[idx - 7]);
 		}
 	}
 }
@@ -2229,18 +2217,6 @@
 	ss->rx_big.lanai = (struct mcp_kreq_ether_recv __iomem *)
 	    (mgp->sram + cmd.data0);
 
-	if (myri10ge_wcfifo && mgp->wc_enabled) {
-		ss->tx.wc_fifo = (u8 __iomem *)
-		    mgp->sram + MXGEFW_ETH_SEND_4 + 64 * slice;
-		ss->rx_small.wc_fifo = (u8 __iomem *)
-		    mgp->sram + MXGEFW_ETH_RECV_SMALL + 64 * slice;
-		ss->rx_big.wc_fifo = (u8 __iomem *)
-		    mgp->sram + MXGEFW_ETH_RECV_BIG + 64 * slice;
-	} else {
-		ss->tx.wc_fifo = NULL;
-		ss->rx_small.wc_fifo = NULL;
-		ss->rx_big.wc_fifo = NULL;
-	}
 	return status;
 
 }
@@ -2573,27 +2549,6 @@
 	mb();
 }
 
-static inline void
-myri10ge_submit_req_wc(struct myri10ge_tx_buf *tx,
-		       struct mcp_kreq_ether_send *src, int cnt)
-{
-	tx->req += cnt;
-	mb();
-	while (cnt >= 4) {
-		myri10ge_pio_copy(tx->wc_fifo, src, 64);
-		mb();
-		src += 4;
-		cnt -= 4;
-	}
-	if (cnt > 0) {
-		/* pad it to 64 bytes.  The src is 64 bytes bigger than it
-		 * needs to be so that we don't overrun it */
-		myri10ge_pio_copy(tx->wc_fifo + MXGEFW_ETH_SEND_OFFSET(cnt),
-				  src, 64);
-		mb();
-	}
-}
-
 /*
  * Transmit a packet.  We need to split the packet so that a single
  * segment does not cross myri10ge->tx_boundary, so this makes segment
@@ -2830,10 +2785,7 @@
 					 MXGEFW_FLAGS_FIRST)));
 	idx = ((count - 1) + tx->req) & tx->mask;
 	tx->info[idx].last = 1;
-	if (tx->wc_fifo == NULL)
-		myri10ge_submit_req(tx, tx->req_list, count);
-	else
-		myri10ge_submit_req_wc(tx, tx->req_list, count);
+	myri10ge_submit_req(tx, tx->req_list, count);
 	tx->pkt_start++;
 	if ((avail - count) < MXGEFW_MAX_SEND_DESC) {
 		tx->stop_queue++;

[PATCH 2/2] myri10ge: use ioremap_wc

[Brice Goglin]

Switch to ioremap_wc(). We keep the MTRR code since ioremap_wc()
will use UC_MINUS when falling back to uncachable, and thus let
the MTRR WC take precedence.

Also rename the error path better.

Signed-off-by: Brice Goglin <brice@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.6/drivers/net/myri10ge/myri10ge.c
===================================================================
--- linux-2.6.orig/drivers/net/myri10ge/myri10ge.c	2008-07-21 09:24:21.000000000 +0200
+++ linux-2.6/drivers/net/myri10ge/myri10ge.c	2008-07-21 09:24:33.000000000 +0200
@@ -3720,14 +3720,14 @@
 	if (mgp->sram_size > mgp->board_span) {
 		dev_err(&pdev->dev, "board span %ld bytes too small\n",
 			mgp->board_span);
-		goto abort_with_wc;
+		goto abort_with_mtrr;
 	}
-	mgp->sram = ioremap(mgp->iomem_base, mgp->board_span);
+	mgp->sram = ioremap_wc(mgp->iomem_base, mgp->board_span);
 	if (mgp->sram == NULL) {
 		dev_err(&pdev->dev, "ioremap failed for %ld bytes at 0x%lx\n",
 			mgp->board_span, mgp->iomem_base);
 		status = -ENXIO;
-		goto abort_with_wc;
+		goto abort_with_mtrr;
 	}
 	memcpy_fromio(mgp->eeprom_strings,
 		      mgp->sram + mgp->sram_size - MYRI10GE_EEPROM_STRINGS_SIZE,
@@ -3828,7 +3828,7 @@
 abort_with_ioremap:
 	iounmap(mgp->sram);
 
-abort_with_wc:
+abort_with_mtrr:
 #ifdef CONFIG_MTRR
 	if (mgp->mtrr >= 0)
 		mtrr_del(mgp->mtrr, mgp->iomem_base, mgp->board_span);

Re: [PATCH 2/2] myri10ge: use ioremap_wc

[Jeff Garzik]

Brice Goglin wrote:
> Switch to ioremap_wc(). We keep the MTRR code since ioremap_wc()
> will use UC_MINUS when falling back to uncachable, and thus let
> the MTRR WC take precedence.
> 
> Also rename the error path better.
> 
> Signed-off-by: Brice Goglin <brice@myri.com>
> ---
>  drivers/net/myri10ge/myri10ge.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

applied 1-2

Re: [PATCH 2/2] myri10ge: use ioremap_wc

[Martin Michlmayr]

* Brice Goglin <brice@myri.com> [2008-07-21 10:26]:
> Switch to ioremap_wc(). We keep the MTRR code since ioremap_wc()
> will use UC_MINUS when falling back to uncachable, and thus let
> the MTRR WC take precedence.

This change leads to a compilation failure on (at least) arm:

  CC [M]  drivers/net/myri10ge/myri10ge.o
drivers/net/myri10ge/myri10ge.c: In function ‘myri10ge_probe’:
drivers/net/myri10ge/myri10ge.c:3725: error: implicit declaration of function ‘ioremap_wc’
drivers/net/myri10ge/myri10ge.c:3725: warning: assignment makes pointer from integer without a cast
make[3]: *** [drivers/net/myri10ge/myri10ge.o] Error 1

-- 
Martin Michlmayr
http://www.cyrius.com/

Re: [PATCH 2/2] myri10ge: use ioremap_wc

[Brice Goglin]

Martin Michlmayr wrote:
> * Brice Goglin <brice@myri.com> [2008-07-21 10:26]:
>   
>> Switch to ioremap_wc(). We keep the MTRR code since ioremap_wc()
>> will use UC_MINUS when falling back to uncachable, and thus let
>> the MTRR WC take precedence.
>>     
>
> This change leads to a compilation failure on (at least) arm:
>
>   CC [M]  drivers/net/myri10ge/myri10ge.o
> drivers/net/myri10ge/myri10ge.c: In function ‘myri10ge_probe’:
> drivers/net/myri10ge/myri10ge.c:3725: error: implicit declaration of function ‘ioremap_wc’
> drivers/net/myri10ge/myri10ge.c:3725: warning: assignment makes pointer from integer without a cast
> make[3]: *** [drivers/net/myri10ge/myri10ge.o] Error 1
>   


Isn't arm's asm/io.h supposed to get
#ifndef ARCH_HAS_IOREMAP_WC
#define ioremap_wc ioremap_nocache
#endif
from asm-generic/iomap.h since ARCH_HAS_IOREMAP_WC isn't defined on arm ?

Brice

Re: [PATCH 2/2] myri10ge: use ioremap_wc

[Martin Michlmayr]

* Brice Goglin <brice@myri.com> [2008-08-10 13:59]:
> > This change leads to a compilation failure on (at least) arm:
> >
> >   CC [M]  drivers/net/myri10ge/myri10ge.o
> > drivers/net/myri10ge/myri10ge.c: In function ‘myri10ge_probe’:
> > drivers/net/myri10ge/myri10ge.c:3725: error: implicit declaration of function ‘ioremap_wc’
> > drivers/net/myri10ge/myri10ge.c:3725: warning: assignment makes pointer from integer without a cast
> > make[3]: *** [drivers/net/myri10ge/myri10ge.o] Error 1
> 
> Isn't arm's asm/io.h supposed to get
> #ifndef ARCH_HAS_IOREMAP_WC
> #define ioremap_wc ioremap_nocache
> #endif
> from asm-generic/iomap.h since ARCH_HAS_IOREMAP_WC isn't defined on arm ?

arch/arm/include/asm/io.h doesn't include asm-generic/iomap.h.  When I
add this includes, myri10ge compiles.

Russell, any comments on the change below?

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 94a95d7..eb71224 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -26,6 +26,7 @@
 #include <linux/types.h>
 #include <asm/byteorder.h>
 #include <asm/memory.h>
+#include <asm-generic/iomap.h>
 
 /*
  * ISA I/O bus memory addresses are 1:1 with the physical address.

-- 
Martin Michlmayr
http://www.cyrius.com/

Re: [PATCH 2/2] myri10ge: use ioremap_wc

[Russell King - ARM Linux]

On Sun, Aug 10, 2008 at 03:38:44PM +0300, Martin Michlmayr wrote:
> * Brice Goglin <brice@myri.com> [2008-08-10 13:59]:
> > > This change leads to a compilation failure on (at least) arm:
> > >
> > >   CC [M]  drivers/net/myri10ge/myri10ge.o
> > > drivers/net/myri10ge/myri10ge.c: In function ???myri10ge_probe???:
> > > drivers/net/myri10ge/myri10ge.c:3725: error: implicit declaration of function ???ioremap_wc???
> > > drivers/net/myri10ge/myri10ge.c:3725: warning: assignment makes pointer from integer without a cast
> > > make[3]: *** [drivers/net/myri10ge/myri10ge.o] Error 1
> > 
> > Isn't arm's asm/io.h supposed to get
> > #ifndef ARCH_HAS_IOREMAP_WC
> > #define ioremap_wc ioremap_nocache
> > #endif
> > from asm-generic/iomap.h since ARCH_HAS_IOREMAP_WC isn't defined on arm ?
> 
> arch/arm/include/asm/io.h doesn't include asm-generic/iomap.h.  When I
> add this includes, myri10ge compiles.
> 
> Russell, any comments on the change below?

Only that I'm not including asm-generic/iomap.h - we have our own
iomap implementation, and the generic stuff in there isn't applicable
to ARM.

I believe Lennert is sorting out ioremap_wc().

Re: [PATCH 2/2] myri10ge: use ioremap_wc

[Martin Michlmayr]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [2008-08-11 10:09]:
> > > > drivers/net/myri10ge/myri10ge.c:3725: error: implicit declaration of function ???ioremap_wc???
> > > 
> > arch/arm/include/asm/io.h doesn't include asm-generic/iomap.h.  When I
> > add this includes, myri10ge compiles.
> > 
> Only that I'm not including asm-generic/iomap.h - we have our own
> iomap implementation, and the generic stuff in there isn't applicable
> to ARM.
> 
> I believe Lennert is sorting out ioremap_wc().

In that case, let's add Lennert to the CC line.
-- 
Martin Michlmayr
http://www.cyrius.com/