[parisc-linux] IBM TR patch

[parisc-linux] IBM TR patch

[Jochen Friedrich]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 836 bytes --]

Hi there,

this patch fixes some endianess issues of the current ibmtr driver. 
Although the current uses of htons() etc look correct, they are not: the 
writew/readw API already do the byte swap on big endian machines.

Additionally, i added a switch to turn off IPv4 summing. This might be 
needed for IPv6 and is absolutely necessary on HP PARISC platform with the 
current broken ioremap implementation. The IPv4 summing is the only place 
in the driver that accesses memory directly without the readX or 
memcpy_fromio macros.

With the patch, the ibmtr driver should work on big endian machines with 
ISA/EISA bus (e.g. 68000 and HP PARISC).

On PARISC, the card gets initialized and joins the ring, but due to the 
ISA memory access bug, it will receive garbage on memory regions with 
(ISA memory & 0x300) == 0.

Cheers,
Jochen

[-- Attachment #2: Type: TEXT/PLAIN, Size: 10605 bytes --]

--- linux/drivers/net/tokenring/ibmtr.c	Fri Nov  9 23:02:24 2001
+++ linux-2.4.17/drivers/net/tokenring/ibmtr.c	Mon Jan 14 00:21:21 2002
@@ -100,6 +100,9 @@
  *
  *      Changes by Burt Silverman to allow the computer to behave nicely when
  *	a cable is pulled or not in place, or a PCMCIA card is removed hot.
+ *
+ *	Changes by Jochen Friedrich to fix endianess issues. Added flag to 
+ *	turn off IPv4 pre-checksums.
  */
 
 /* change the define of IBMTR_DEBUG_MESSAGES to a nonzero value 
@@ -125,6 +128,11 @@
 /* some 95 OS send many non UI frame; this allow removing the warning */
 #define TR_FILTERNONUI	1
 
+/* switch to 0 to disable IPv4 checksums. Needed by architectures with
+ * broken ioremap (i.e. parisc).
+ */
+#define TR_CHECKSUM 1
+
 #include <linux/sched.h>
 #include <linux/ioport.h>
 #include <linux/netdevice.h>
@@ -269,7 +277,7 @@
 		}
 		isa_writeb(0x00, ram_addr+0x1E01);
 		for(i=jiffies+TR_BUSY_INTERVAL; time_before_eq(jiffies,i););
-		intf_tbl=ntohs(isa_readw(ram_addr+ACA_OFFSET+ACA_RW+WRBR_EVEN));
+		intf_tbl=swab16(isa_readw(ram_addr+ACA_OFFSET+ACA_RW+WRBR_EVEN));
 		if (intf_tbl) {
 #if IBMTR_DEBUG_MESSAGES
 			printk("ibmtr::find_turbo_adapters, Turbo found at "
@@ -280,7 +288,7 @@
 			}
 			printk("\n");
 #endif
-			turbo_io[index]=ntohs(isa_readw(ram_addr+intf_tbl+4));
+			turbo_io[index]=swab16(isa_readw(ram_addr+intf_tbl+4));
 			turbo_irq[index]=isa_readb(ram_addr+intf_tbl+3);
 			outb(0, turbo_io[index] + ADAPTRESET);
 			for(i=jiffies+TR_RST_TIME;time_before_eq(jiffies,i););
@@ -926,15 +934,15 @@
 	for (i = 0; i < sizeof(struct dir_open_adapter); i++)
 		writeb(0, ti->init_srb + i);
 	writeb(DIR_OPEN_ADAPTER, ti->init_srb + COMMAND_OFST);
-	writew(htons(OPEN_PASS_BCON_MAC), ti->init_srb + OPEN_OPTIONS_OFST);
+	writew(swab16(OPEN_PASS_BCON_MAC), ti->init_srb + OPEN_OPTIONS_OFST);
 	if (ti->ring_speed == 16) {
-		writew(htons(ti->dhb_size16mb), ti->init_srb + DHB_LENGTH_OFST);
-		writew(htons(ti->rbuf_cnt16), ti->init_srb + NUM_RCV_BUF_OFST);
-		writew(htons(ti->rbuf_len16), ti->init_srb + RCV_BUF_LEN_OFST);
+		writew(swab16(ti->dhb_size16mb), ti->init_srb + DHB_LENGTH_OFST);
+		writew(swab16(ti->rbuf_cnt16), ti->init_srb + NUM_RCV_BUF_OFST);
+		writew(swab16(ti->rbuf_len16), ti->init_srb + RCV_BUF_LEN_OFST);
 	} else {
-		writew(htons(ti->dhb_size4mb), ti->init_srb + DHB_LENGTH_OFST);
-		writew(htons(ti->rbuf_cnt4), ti->init_srb + NUM_RCV_BUF_OFST);
-		writew(htons(ti->rbuf_len4), ti->init_srb + RCV_BUF_LEN_OFST);
+		writew(swab16(ti->dhb_size4mb), ti->init_srb + DHB_LENGTH_OFST);
+		writew(swab16(ti->rbuf_cnt4), ti->init_srb + NUM_RCV_BUF_OFST);
+		writew(swab16(ti->rbuf_len4), ti->init_srb + RCV_BUF_LEN_OFST);
 	}
 	writeb(NUM_DHB,		/* always 2 */ ti->init_srb + NUM_DHB_OFST);
 	writeb(DLC_MAX_SAP, ti->init_srb + DLC_MAX_SAP_OFST);
@@ -943,9 +951,9 @@
 	ti->srb_page = ti->init_srb_page;
 	DPRINTK("Opening adapter: Xmit bfrs: %d X %d, Rcv bfrs: %d X %d\n",
 		readb(ti->init_srb + NUM_DHB_OFST),
-		ntohs(readw(ti->init_srb + DHB_LENGTH_OFST)),
-		ntohs(readw(ti->init_srb + NUM_RCV_BUF_OFST)),
-		ntohs(readw(ti->init_srb + RCV_BUF_LEN_OFST)));
+		swab16(readw(ti->init_srb + DHB_LENGTH_OFST)),
+		swab16(readw(ti->init_srb + NUM_RCV_BUF_OFST)),
+		swab16(readw(ti->init_srb + RCV_BUF_LEN_OFST)));
 	writeb(INT_ENABLE, ti->mmio + ACA_OFFSET + ACA_SET + ISRP_EVEN);
 	writeb(CMD_IN_SRB, ti->mmio + ACA_OFFSET + ACA_SET + ISRA_ODD);
 }
@@ -967,7 +975,7 @@
 #define STATION_COUNT_OFST      18
 
 	writeb(DLC_OPEN_SAP, ti->srb + COMMAND_OFST);
-	writew(htons(MAX_I_FIELD), ti->srb + MAX_I_FIELD_OFST);
+	writew(swab16(MAX_I_FIELD), ti->srb + MAX_I_FIELD_OFST);
 	writeb(SAP_OPEN_IND_SAP | SAP_OPEN_PRIORITY, ti->srb+ SAP_OPTIONS_OFST);
 	writeb(SAP_OPEN_STATION_CNT, ti->srb + STATION_COUNT_OFST);
 	writeb(type, ti->srb + SAP_VALUE_OFST);
@@ -1083,10 +1091,10 @@
         unsigned char ret_code;
         __u16 err;
 
-        ti->srb = ntohs(readw(ti->init_srb + SRB_ADDRESS_OFST));
-        ti->ssb = ntohs(readw(ti->init_srb + SSB_ADDRESS_OFST));
-        ti->arb = ntohs(readw(ti->init_srb + ARB_ADDRESS_OFST));
-        ti->asb = ntohs(readw(ti->init_srb + ASB_ADDRESS_OFST));
+        ti->srb = swab16(readw(ti->init_srb + SRB_ADDRESS_OFST));
+        ti->ssb = swab16(readw(ti->init_srb + SSB_ADDRESS_OFST));
+        ti->arb = swab16(readw(ti->init_srb + ARB_ADDRESS_OFST));
+        ti->asb = swab16(readw(ti->init_srb + ASB_ADDRESS_OFST));
         if (ti->page_mask) {
                 ti->srb_page = (ti->srb >> 8) & ti->page_mask;
                 ti->srb &= ~(ti->page_mask << 8);
@@ -1103,7 +1111,7 @@
         ti->asb += ti->sram_virt;
         ti->current_skb = NULL;
         ret_code = readb(ti->init_srb + RETCODE_OFST);
-        err = ntohs(readw(ti->init_srb + OPEN_ERROR_CODE_OFST));
+        err = swab16(readw(ti->init_srb + OPEN_ERROR_CODE_OFST));
         if (!ret_code) {
 		ti->open_status = OPEN; /* TR adapter is now available */
                 if (ti->open_mode == AUTOMATIC) {
@@ -1206,7 +1214,7 @@
 		__u32 check_reason;
 		__u8 check_reason_page = 0;
 		check_reason =
-			ntohs(readw(ti->mmio+ ACA_OFFSET+ACA_RW + WWCR_EVEN));
+			swab16(readw(ti->mmio+ ACA_OFFSET+ACA_RW + WWCR_EVEN));
 		if (ti->page_mask) {
 			check_reason_page = (check_reason >> 8) & ti->page_mask;
 			check_reason &= ~(ti->page_mask << 8);
@@ -1396,15 +1404,15 @@
 		switch (readb(ti->arb)) {	/* ARB command check */
 		case DLC_STATUS:
 			DPRINTK("DLC_STATUS new status: %02X on station %02X\n",
-				ntohs(readw(ti->arb + STATUS_OFST)),
-				ntohs(readw(ti->arb+ STATION_ID_OFST)));
+				swab16(readw(ti->arb + STATUS_OFST)),
+				swab16(readw(ti->arb+ STATION_ID_OFST)));
 			break;
 		case REC_DATA:
 			tr_rx(dev);
 			break;
 		case RING_STAT_CHANGE:{
 			unsigned short ring_status;
-			ring_status= ntohs(readw(ti->arb + NETW_STATUS_OFST));
+			ring_status= swab16(readw(ti->arb + NETW_STATUS_OFST));
 			if (ibmtr_debug_trace & TRC_INIT)
 				DPRINTK("Ring Status Change...(0x%x)\n",
 								ring_status);
@@ -1501,7 +1509,7 @@
 #ifndef PCMCIA
         ti->sram_virt = (u32)ioremap(((__u32)ti->sram_base << 12), ti->avail_shared_ram);
 #endif
-	ti->init_srb = ntohs(readw(ti->mmio + ACA_OFFSET + WRBR_EVEN));
+	ti->init_srb = swab16(readw(ti->mmio + ACA_OFFSET + WRBR_EVEN));
 	if (ti->page_mask) {
 		ti->init_srb_page = (ti->init_srb >> 8) & ti->page_mask;
 		ti->init_srb &= ~(ti->page_mask << 8);
@@ -1531,7 +1539,7 @@
 	}
 #endif
 
-	hw_encoded_addr = readw(ti->init_srb + ENCODED_ADDRESS_OFST);
+	hw_encoded_addr = swab16(readw(ti->init_srb + ENCODED_ADDRESS_OFST));
 	encoded_addr = ntohs(hw_encoded_addr);
         init_status= /*BMS 12/2000 check for shallow mode possibility (Turbo)*/
 	readb(ti->init_srb+offsetof(struct srb_init_response,init_status));
@@ -1577,7 +1585,7 @@
 	   Here we compute the effective address where we will place data.
 	*/
 	SET_PAGE(ti->arb_page);
-	dhb=dhb_base=ntohs(readw(ti->arb + DHB_ADDRESS_OFST));
+	dhb=dhb_base=swab16(readw(ti->arb + DHB_ADDRESS_OFST));
 	if (ti->page_mask) {
 		dhb_page = (dhb_base >> 8) & ti->page_mask;
 		dhb=dhb_base & ~(ti->page_mask << 8);
@@ -1600,12 +1608,12 @@
 	xmit_command = xsrb.command;
 
 	writeb(xmit_command, ti->asb + COMMAND_OFST);
-	writew(xsrb.station_id, ti->asb + STATION_ID_OFST);
+	writew(cpu_to_le16(xsrb.station_id), ti->asb + STATION_ID_OFST);
 	writeb(llc_ssap, ti->asb + RSAP_VALUE_OFST);
 	writeb(xsrb.cmd_corr, ti->asb + CMD_CORRELATE_OFST);
 	writeb(0, ti->asb + RETCODE_OFST);
 	if ((xmit_command == XMIT_XID_CMD) || (xmit_command == XMIT_TEST_CMD)) {
-		writew(htons(0x11), ti->asb + FRAME_LENGTH_OFST);
+		writew(swab16(0x11), ti->asb + FRAME_LENGTH_OFST);
 		writeb(0x0e, ti->asb + HEADER_LENGTH_OFST);
 		SET_PAGE(dhb_page);
 		writeb(AC, dhb);
@@ -1622,7 +1630,7 @@
 	 *    buffer identified in the command data received with the interrupt.
 	 */
 	writeb(hdr_len, ti->asb + HEADER_LENGTH_OFST);
-	writew(htons(ti->current_skb->len), ti->asb + FRAME_LENGTH_OFST);
+	writew(swab16(ti->current_skb->len), ti->asb + FRAME_LENGTH_OFST);
 	src_len=ti->current_skb->len;
 	src_offset=0;
 	dhb=dhb_base;
@@ -1698,8 +1706,8 @@
 	if (readb(ti->asb + RETCODE_OFST) !=0xFF) DPRINTK("ASB not free !!!\n");
 
 	writeb(REC_DATA, ti->asb + COMMAND_OFST);
-	writew(rarb.station_id, ti->asb + STATION_ID_OFST);
-	writew(rarb.rec_buf_addr, ti->asb + RECEIVE_BUFFER_OFST);
+	writew(cpu_to_le16(rarb.station_id), ti->asb + STATION_ID_OFST);
+	writew(cpu_to_le16(rarb.rec_buf_addr), ti->asb + RECEIVE_BUFFER_OFST);
 
 	lan_hdr_len = rarb.lan_hdr_len;
 	if (lan_hdr_len > sizeof(struct trh_hdr)) {
@@ -1722,7 +1730,7 @@
 		(int) readb(llc + DSAP_OFST), (int) readb(llc + SSAP_OFST),
 		(int) readb(llc + LLC_OFST), (int) readb(llc + PROTID_OFST),
 		(int) readb(llc+PROTID_OFST+1),(int)readb(llc+PROTID_OFST + 2),
-		(int) ntohs(readw(llc + ETHERTYPE_OFST)));
+		(int) swab16(readw(llc + ETHERTYPE_OFST)));
 #endif
 	if (readb(llc + offsetof(struct trllc, llc)) != UI_CMD) {
 		SET_PAGE(ti->asb_page);
@@ -1775,7 +1783,7 @@
 	skb_put(skb, length);
 	skb->dev = dev;
 	data = skb->data;
-	rbuffer_len = ntohs(readw(rbuffer + offsetof(struct rec_buf, buf_len)));
+	rbuffer_len = swab16(readw(rbuffer + offsetof(struct rec_buf, buf_len)));
 	rbufdata = rbuffer + offsetof(struct rec_buf, data);
 
 	if (IPv4_p) {
@@ -1799,12 +1807,14 @@
 		if (ibmtr_debug_trace&TRC_INITV && length < rbuffer_len)
 			DPRINTK("CURIOUS, length=%d < rbuffer_len=%d\n",
 						length,rbuffer_len);
+#if TR_CHECKSUM
 		if (IPv4_p)
 			chksum=csum_partial_copy_nocheck((void*)rbufdata,
 			    data,length<rbuffer_len?length:rbuffer_len,chksum);
 		else
+#endif
 			memcpy_fromio(data, rbufdata, rbuffer_len);
-		rbuffer = ntohs(readw(rbuffer+BUFFER_POINTER_OFST)) ;
+		rbuffer = swab16(readw(rbuffer+BUFFER_POINTER_OFST)) ;
 		if (!rbuffer)
 			break;
 		rbuffer -= 2;
@@ -1816,7 +1826,7 @@
 		}
 		rbuffer += ti->sram_virt;
 		SET_PAGE(rbuffer_page);
-		rbuffer_len = ntohs(readw(rbuffer + BUFFER_LENGTH_OFST));
+		rbuffer_len = swab16(readw(rbuffer + BUFFER_LENGTH_OFST));
 		rbufdata = rbuffer + offsetof(struct rec_buf, data);
 	}
 
@@ -1829,10 +1839,12 @@
 	ti->tr_stats.rx_packets++;
 
 	skb->protocol = tr_type_trans(skb, dev);
+#if TR_CHECKSUM
 	if (IPv4_p) {
 		skb->csum = chksum;
 		skb->ip_summed = 1;
 	}
+#endif
 	netif_rx(skb);
 	dev->last_rx = jiffies;
 }				/*tr_rx */

Re: [parisc-linux] IBM TR patch

[Randolph Chung]

> this patch fixes some endianess issues of the current ibmtr driver. 
> Although the current uses of htons() etc look correct, they are not: the 
> writew/readw API already do the byte swap on big endian machines.

the write/read macros are defined for pci accesses, and as such take a
LE point of view with respect to memory accesses. I take it that you are
not accessing a PCI device, so that's why you are having problems with
endianess.  I suspect your patch will break the driver on PCI devices...
(assuming there are both PCI and [E]ISA versions of the device that
share the same driver).

> Additionally, i added a switch to turn off IPv4 summing. This might be 
> needed for IPv6 and is absolutely necessary on HP PARISC platform with the 
> current broken ioremap implementation. The IPv4 summing is the only place 
> in the driver that accesses memory directly without the readX or 
> memcpy_fromio macros.

How is ioremap broken? (Just curious). From what I understand ioremap is
currently a nop in the parisc-linux vm/io design.

randolph
-- 
   @..@                                         http://www.TauSq.org/
  (----)
 ( >__< )
 ^^ ~~ ^^

Re: [parisc-linux] IBM TR patch

[Matthew Wilcox]

On Tue, Jan 15, 2002 at 10:11:58AM -0800, Randolph Chung wrote:
> > Additionally, i added a switch to turn off IPv4 summing. This might be 
> > needed for IPv6 and is absolutely necessary on HP PARISC platform with the 
> > current broken ioremap implementation. The IPv4 summing is the only place 
> > in the driver that accesses memory directly without the readX or 
> > memcpy_fromio macros.
> 
> How is ioremap broken? (Just curious). From what I understand ioremap is
> currently a nop in the parisc-linux vm/io design.

it's not broken, the assumptions in ibmtr are.  after you've ioremapped
a region, you can call readb/writeb on it.  ibmtr assumes you can just
do direct memory accesses.  now, this is something we want to fix,
but it's not a bug in the PA code.

-- 
Revolutions do not require corporate support.

Re: [parisc-linux] IBM TR patch

[Jochen Friedrich]

Hi Randolph,

> I suspect your patch will break the driver on PCI devices...
> (assuming there are both PCI and [E]ISA versions of the device that
> share the same driver).

Nope, it's ISA and PCMCIA only. And LE stays LE on PCI devices, as well.

--jochen

Re: [parisc-linux] IBM TR patch

[Jochen Friedrich]

Hi Matthew,

> > How is ioremap broken? (Just curious). From what I understand ioremap is
> > currently a nop in the parisc-linux vm/io design.
> 
> it's not broken, the assumptions in ibmtr are.  after you've ioremapped
> a region, you can call readb/writeb on it.  ibmtr assumes you can just
> do direct memory accesses.  now, this is something we want to fix,
> but it's not a bug in the PA code.

Then maybe it's best to define the TR_CHECKSUM macro to 0 as default with 
an exception of architectures where the above assumption in true (i.e. 
alpha, i386 and maybe ppc, as well) to gain the extra performance boost of 
the IPv4 pre summarisation.

--jochen

Re: [parisc-linux] IBM TR patch

[Daniel Engstrom]

On 2002.01.14 20:53 Jochen Friedrich wrote:
> Hi there,
HI
> this patch fixes some endianess issues of the current ibmtr driver. 
> Although the current uses of htons() etc look correct, they are not: the 
> writew/readw API already do the byte swap on big endian machines.
>
I think the driver was correct. The i/o layer should not try to byteswap
the io requests, because it can not second-guess what the driver is doing.

And in the case of the tr driver it now does two swaps for each word it tranfers on
big endian platforms instad of zero.

The ne2000 driver had the same problem, but I removed the swaps from the parisc i/o code 
instad of touching the driver.

/Daniel
-- 

Re: [parisc-linux] IBM TR patch

[Randolph Chung]

> I think the driver was correct. The i/o layer should not try to byteswap
> the io requests, because it can not second-guess what the driver is doing.

For reference, you may wish to read
http://lists.parisc-linux.org/pipermail/parisc-linux/2001-November/014606.html

Originally the parisc {read,write}[wl] macros did not swap, but this
breaks many PCI drivers. For example, we used to have hppa specific code in the
sym53c8xx driver that did the swapping....

> And in the case of the tr driver it now does two swaps for each word it tranfers on
> big endian platforms instad of zero.


I agree this is not optimal, altho, as Alan Cox pointed out in
http://lists.parisc-linux.org/pipermail/parisc-linux/2001-November/014527.html
double swaps may be optimized out by the compiler.
> 
> The ne2000 driver had the same problem, but I removed the swaps from the parisc i/o code 
> instad of touching the driver.

for parisc we follow the ppc convention of defining __raw_{read,write}[wl]
that are host-endian. perhaps the drivers can use this instead?

randolph
-- 
   @..@                                         http://www.TauSq.org/
  (----)
 ( >__< )
 ^^ ~~ ^^

Re: [parisc-linux] IBM TR patch

[Jochen Friedrich]

Hi Daniel,

> I think the driver was correct. The i/o layer should not try to byteswap
> the io requests, because it can not second-guess what the driver is doing.

I guess the 68000 code is the reference here as it was the first BE port, 
and they do the byteswap, as well.

--jochen

Re: [parisc-linux] IBM TR patch

[Daniel Engstrom]

On 2002.01.16 21:20 Jochen Friedrich wrote:
> Hi Daniel,
> 
> > I think the driver was correct. The i/o layer should not try to byteswap
> > the io requests, because it can not second-guess what the driver is doing.
> 
> I guess the 68000 code is the reference here as it was the first BE port, 
> and they do the byteswap, as well.

Do the 68000 port support ISA and PCI buses?

/Daniel
--