This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

CS8900A TX improvement


Hi All!

I have been experiencing problems with CS8900A driver under heavy network load.
On occasion the driver would discard the TX frame because CS8900A
won't immediatly
signal TxRDY - if there is not enough space inside hardware buffers.
After the TX frame is
discarded the driver still waits 250ms for TX done event! In my case
this can happen often
and in a row...

The right thing to do in this case is to offload the RX frames making
space for the TX frame -
TxRDY will be issued afterwards.

The attached patch does this by storing the sg_list internally if the
TxRDY is not obtained
immediately and returns the control to the upper layers. The network
stack can then read the
RX frames from CS8900A which will also trigger TxRDY signal. The
driver will now handle this
signal by coping previously stored sg_list into hardware buffers
starting the transmission.

Regards,
Savin Zlobec
diff -urw -U 5 packages-cvs-090511/devs/eth/cl/cs8900a/current/ChangeLog packages/devs/eth/cl/cs8900a/current/ChangeLog
--- packages-cvs-090511/devs/eth/cl/cs8900a/current/ChangeLog	2009-03-02 22:16:52.000000000 +0100
+++ packages/devs/eth/cl/cs8900a/current/ChangeLog	2009-05-11 12:31:59.000000000 +0200
@@ -1,5 +1,12 @@
+2009-05-11  Savin Zlobec <savinz@users.sourceforge.net>
+
+	* include/cs8900.h:
+	* src/if_cs8900a.c: Don't drop TX frames when there is not enough
+	buffer space available inside hardware, but start transmit on next
+	TxRDY event.
+
  2009-02-28  Sergei Gavrikov  <sergei.gavrikov@gmail.com>
 	
         * src/if_cs8900a.c: Fixed interrupt hooking block in cs8900a_init() to
         prevent assertion fail on cyg_interrupt_attach() when interrupt is not
         used.
diff -urw -U 5 packages-cvs-090511/devs/eth/cl/cs8900a/current/include/cs8900.h packages/devs/eth/cl/cs8900a/current/include/cs8900.h
--- packages-cvs-090511/devs/eth/cl/cs8900a/current/include/cs8900.h	2009-01-29 18:48:07.000000000 +0100
+++ packages/devs/eth/cl/cs8900a/current/include/cs8900.h	2009-05-11 11:53:30.000000000 +0200
@@ -52,10 +52,11 @@
 //==========================================================================
 
 #include <cyg/infra/cyg_type.h>
 
 #include <cyg/hal/hal_io.h>
+#include <cyg/io/eth/eth_drv.h>
 #include <pkgconf/devs_eth_cl_cs8900a.h>
 
 #define __WANT_CONFIG
 #include CYGDAT_DEVS_ETH_CL_CS8900A_INL
 #undef __WANT_CONFIG
@@ -97,20 +98,22 @@
 // Private driver structure
 struct cs8900a_priv_data;
 typedef cyg_bool (*provide_esa_t)(struct cs8900a_priv_data* cpd);
 
 typedef struct cs8900a_priv_data {
-    bool txbusy, hardwired_esa;
+    bool txbusy, txpending, hardwired_esa;
     int rxmode;
     cyg_uint8 esa[6];
     provide_esa_t provide_esa;
     cyg_vector_t interrupt;             // Interrupt vector used by controller
     int priority;						// Priority level used by controller
     cyg_handle_t  interrupt_handle;
     cyg_interrupt interrupt_object;
     cyg_addrword_t base;
     cyg_uint32 txkey;   // Used to ack when packet sent
+    struct eth_drv_sg txsg_list[MAX_ETH_DRV_SG];
+    int txsg_len;
     struct cyg_netdevtab_entry *tab;
 #ifdef CYGPKG_KERNEL
     cyg_tick_count_t txstart;
 #endif
 } cs8900a_priv_data_t;
diff -urw -U 5 packages-cvs-090511/devs/eth/cl/cs8900a/current/src/if_cs8900a.c packages/devs/eth/cl/cs8900a/current/src/if_cs8900a.c
--- packages-cvs-090511/devs/eth/cl/cs8900a/current/src/if_cs8900a.c	2009-03-02 22:16:52.000000000 +0100
+++ packages/devs/eth/cl/cs8900a/current/src/if_cs8900a.c	2009-05-11 12:24:39.000000000 +0200
@@ -344,10 +344,11 @@
     // Clear Interrupt Status Queue before enabling interrupts
     do {
         CS_IN(cpd->base, CS8900A_ISQ, stat);
     }  while (stat != 0) ;
     cpd->txbusy = false;
+    cpd->txpending = false;
 #ifndef CYGIMP_DEVS_ETH_CL_CS8900A_DATABUS_8BIT
     put_reg(base, PP_BusCtl, PP_BusCtl_EnableIRQ);
 #endif
 }
 
@@ -443,88 +444,37 @@
             // 250ms is more than enough to transmit one frame
 #if DEBUG & 1
             diag_printf("CS8900: Tx interrupt lost\n");
 #endif
             cpd->txbusy = false;
+            cpd->txpending = false;
             // Free up the buffer (with error indication)
             (sc->funs->eth_drv->tx_done)(sc, cpd->txkey, 1);
         }
     }
 #endif
     return (cpd->txbusy == false);
 }
 
-// This routine is called to send data to the hardware.
+// This routine is called to put data into the hardware tx buffer.
 static void 
-cs8900a_send(struct eth_drv_sc *sc, struct eth_drv_sg *sg_list, int sg_len, 
-            int total_len, unsigned long key)
+cs8900a_put_txdata(struct eth_drv_sc *sc, struct eth_drv_sg *sg_list, int sg_len)
 {
     cs8900a_priv_data_t *cpd = (cs8900a_priv_data_t *)sc->driver_private;
-    cyg_addrword_t base = cpd->base;
     int i;
     int len;
     cyg_uint8 *data;
     cyg_uint16 saved_data = 0, *sdata;
-    cyg_uint16 stat;
     bool force_coping_by_byte;
     bool odd_byte = false;
 
-    // Mark xmitter busy
-    cpd->txbusy = true;
-    cpd->txkey = key;
-#ifdef CYGPKG_KERNEL
-    cpd->txstart = cyg_current_time();
-#endif
-
-    // Start the xmit sequence
-#ifdef CYGIMP_DEVS_ETH_CL_CS8900A_DATABUS_BYTE_SWAPPED
-    total_len = CYG_SWAP16(total_len);
-#endif
-        
 #ifdef CYGIMP_DEVS_ETH_CL_CS8900A_DATABUS_8BIT
     force_coping_by_byte = true;
 #else
     force_coping_by_byte = false;
 #endif
 
-    // The hardware indicates that there are options as to when the actual
-    // packet transmission will start wrt moving of data into the transmit
-    // buffer.  However, impirical results seem to indicate that if the
-    // packet is large and transmission is allowed to start before the
-    // entire packet has been pushed into the buffer, the hardware gets
-    // confused and the packet is lost, along with a "lost" Tx interrupt.
-    // This may be a case of the copy loop below being interrupted, e.g.
-    // a system timer interrupt, and the hardware getting unhappy that 
-    // not all of the data was provided before the transmission should
-    // have completed (i.e. buffer underrun).
-    // For now, the solution is to not allow this overlap.
-    //CS_OUT(cpd->base, CS8900A_TxCMD, PP_TxCmd_TxStart_5)
-
-    // Start only when all data sent to chip
-    CS_OUT(cpd->base, CS8900A_TxCMD, PP_TxCmd_TxStart_Full);
-
-    CS_OUT(cpd->base, CS8900A_TxLEN, total_len);
-    // Wait for controller ready signal
-    {
-        // add timeout per cs8900a bugzilla report 1000281 */
-        int timeout = 1000;
-
-        do {
-            stat = get_reg(base, PP_BusStat);
-#if DEBUG & 1
-            if( stat & PP_BusStat_TxBid )
-                diag_printf( "cs8900a_send: Bid error!\n" );
-#endif
-        } while (!(stat & PP_BusStat_TxRDY) && --timeout);
-
-        if( !timeout ) {
-            // we might as well just return, since if we write the data it will
-            // just get thrown away
-            return;
-        }
-    }
-
     // Put data into buffer
     for (i = 0;  i < sg_len;  i++) {
         data = (cyg_uint8 *)sg_list[i].buf;
         len = sg_list[i].len;
 
@@ -586,10 +536,80 @@
     if (odd_byte) {
         CS_OUT(cpd->base, CS8900A_RTDATA, saved_data);
     }
 }
 
+// This routine is called to send data to the hardware.
+static void 
+cs8900a_send(struct eth_drv_sc *sc, struct eth_drv_sg *sg_list, int sg_len, 
+            int total_len, unsigned long key)
+{
+    cs8900a_priv_data_t *cpd = (cs8900a_priv_data_t *)sc->driver_private;
+    cyg_uint16 stat;
+
+    // Mark xmitter busy
+    cpd->txbusy = true;
+    cpd->txkey = key;
+#ifdef CYGPKG_KERNEL
+    cpd->txstart = cyg_current_time();
+#endif
+
+    // Start the xmit sequence
+#ifdef CYGIMP_DEVS_ETH_CL_CS8900A_DATABUS_BYTE_SWAPPED
+    total_len = CYG_SWAP16(total_len);
+#endif
+
+    // The hardware indicates that there are options as to when the actual
+    // packet transmission will start wrt moving of data into the transmit
+    // buffer.  However, impirical results seem to indicate that if the
+    // packet is large and transmission is allowed to start before the
+    // entire packet has been pushed into the buffer, the hardware gets
+    // confused and the packet is lost, along with a "lost" Tx interrupt.
+    // This may be a case of the copy loop below being interrupted, e.g.
+    // a system timer interrupt, and the hardware getting unhappy that 
+    // not all of the data was provided before the transmission should
+    // have completed (i.e. buffer underrun).
+    // For now, the solution is to not allow this overlap.
+    //CS_OUT(cpd->base, CS8900A_TxCMD, PP_TxCmd_TxStart_5)
+
+    // Start only when all data sent to chip
+    CS_OUT(cpd->base, CS8900A_TxCMD, PP_TxCmd_TxStart_Full);
+
+    CS_OUT(cpd->base, CS8900A_TxLEN, total_len);
+    // Wait for controller ready signal
+    {
+        // add timeout per cs8900a bugzilla report 1000281 */
+        int timeout = 100;
+
+        do {
+            stat = get_reg(cpd->base, PP_BusStat);
+#if DEBUG & 1
+            if( stat & PP_BusStat_TxBid )
+                diag_printf( "cs8900a_send: Bid error!\n" );
+#endif
+        } while (!(stat & PP_BusStat_TxRDY) && --timeout);
+
+        // If transmission can't start right away, then store the sg list and
+        // fill the TX buffer on next TxRDY signal - in cs8900a_BufEvent.
+        if( !timeout ) {
+            int i;
+            for (i = 0; i < sg_len; i++)
+                cpd->txsg_list[i] = sg_list[i];
+            cpd->txsg_len = sg_len;
+            cpd->txpending = true;
+#if DEBUG & 1
+            diag_printf("Delaying TX until TxRDY\n");
+#endif
+            return;
+        }
+        else
+            cpd->txpending = false;
+    }
+
+    cs8900a_put_txdata(sc, sg_list, sg_len);
+}
+
 // This function is called when a packet has been received.  It's job is
 // to prepare to unload the packet from the hardware.  Once the length of
 // the packet is known, the upper layer of the driver can be told.  When
 // the upper layer is ready to unload the packet, the internal function
 // 'cs8900a_recv' will be called to actually fetch it from the hardware.
@@ -693,14 +713,30 @@
 }
 
 static void
 cs8900a_BufEvent(struct eth_drv_sc *sc, int stat)
 {
+    cs8900a_priv_data_t *cpd = (cs8900a_priv_data_t *)sc->driver_private;
     if (stat & PP_BufCFG_RxMiss) {
     }
     if (stat & PP_BufCFG_TxUE) {
     }
+    if (stat & PP_BufCFG_TxRDY) {
+        if (cpd->txpending) {
+#ifdef CYGPKG_KERNEL
+            cpd->txstart = cyg_current_time();
+#endif
+            if (cpd->txsg_len > 0) {
+#if DEBUG & 1
+                diag_printf("Got TxRDY - starting TX\n");
+#endif
+                cs8900a_put_txdata(sc, cpd->txsg_list, cpd->txsg_len);
+                cpd->txsg_len = 0;
+            }
+            cpd->txpending = false;
+        }
+    }
 }
 
 static void
 cs8900a_poll(struct eth_drv_sc *sc)
 {

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]