public inbox for ecos-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tarmo Kuuse <tarmo.kuuse@mail.ee>
To: ecos-patches@sources.redhat.com
Subject: Fix potential stall with Coldfire Ethernet
Date: Tue, 03 Mar 2009 11:45:00 -0000	[thread overview]
Message-ID: <goj05c$vm5$1@ger.gmane.org> (raw)

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

Hi,

While struggling with eCos on Coldfire 5208 I stumbled on a nasty bug. 
The fast ethernet controller often stalls if transmission is flagged 
immediately after copying data to buffer.

I have no idea why this happens. Maybe my compiler (from CodeSourcery, 
not eCos) does something to confuse the FEC. Maybe the 5208 hardware is 
buggy.

Hopefully the currently supported devices (5282 and 532x) are safe when 
built with eCos toolchain (I cannot test it). The patch could save a few 
neurons for someone.

Patch adds a NOP between filling the buffer and flagging the transmitter.

There is also a small change on how TX buffer descriptors are allocated 
- a loop is used instead of two individual statements. It was quite 
annoying having to debug after changing the BD count.

--
Kind regards,
Tarmo Kuuse

[-- Attachment #2: if_mcf5272_fix_stall.patch --]
[-- Type: text/x-patch, Size: 3076 bytes --]

Index: devs/eth/m68k/mcf5272/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/eth/m68k/mcf5272/current/ChangeLog,v
retrieving revision 1.3
diff -u -5 -p -r1.3 ChangeLog
--- devs/eth/m68k/mcf5272/current/ChangeLog	29 Jan 2009 17:48:09 -0000	1.3
+++ devs/eth/m68k/mcf5272/current/ChangeLog	3 Mar 2009 10:05:48 -0000
@@ -1,5 +1,10 @@
+2009-03-03  Tarmo Kuuse  <tarmo.kuuse@mail.ee>
+
+	* src/if_mcf5272.c: Fix potential stall on transmission; TX buffer
+	descriptors are initialized in loop
+
 2008-11-17  Bart Veer  <bartv@ecoscentric.com>
 
 	* cdl/mcf5272.cdl, doc/mcf5272_eth.sgml, src/if_mcf5272.c: minor
 	clean-ups.
 
Index: devs/eth/m68k/mcf5272/current/src/if_mcf5272.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/eth/m68k/mcf5272/current/src/if_mcf5272.c,v
retrieving revision 1.2
diff -u -5 -p -r1.2 if_mcf5272.c
--- devs/eth/m68k/mcf5272/current/src/if_mcf5272.c	29 Jan 2009 17:48:09 -0000	1.2
+++ devs/eth/m68k/mcf5272/current/src/if_mcf5272.c	3 Mar 2009 10:05:49 -0000
@@ -288,10 +288,16 @@ mcfxxxx_eth_ramfn(void)
 // This #elif should probably become a #else
 #elif defined(CYGPKG_HAL_M68K_MCF5282) || defined(CYGPKG_HAL_M68K_MCF532x)
 
 # define TX_NEEDS_ALIGNMENT             1
 # define TX_BUFFER_SIZE                 1520
+// It's currently safe to use only 2 TX BD-s since 
+// one BD is declared free at any given time. 
+// Freescale however recommends having at least 3 TX 
+// buffers (see MCF5208 Reference Manual, Rev. 2 or newer,
+// section 19.5.7.1 "Duplicate Frame Transmission").
+// Note: Don't use an odd number.
 # define TX_BUFFER_DESCRIPTOR_COUNT     2
 
 #else
 
 # error Current processor unsupported
@@ -520,12 +526,13 @@ mcfxxxx_eth_init(struct cyg_netdevtab_en
     // Ditto for the tx buffers, if tx involves copying into an
     // aligned buffer. Only one entry in the ring buffer will be used
     // at a time, so the two buffer descriptors can share the same
     // buffer.
 #ifdef TX_NEEDS_ALIGNMENT
-    eth->txbds[0].ethbd_buffer  = txbuffer;
-    eth->txbds[1].ethbd_buffer  = txbuffer;
+	for (i = 0; i < TX_BUFFER_DESCRIPTOR_COUNT; i++) {
+		eth->txbds[i].ethbd_buffer = txbuffer;
+	}
 #endif
     
     // rx_next_buffer tracks the next rx buffer descriptor which the
     // hardware may process.
     eth->rx_next_buffer     = 0;
@@ -737,10 +744,15 @@ mcfxxxx_eth_send(struct eth_drv_sc* sc,
     // for any bits changed by the hardware except on the xxxx which
     // does not have a data cache at all.
 #ifdef HAL_DCACHE_STORE    
     HAL_DCACHE_STORE(&(eth->txbds[0]), TX_BUFFER_DESCRIPTOR_COUNT * sizeof(hal_mcfxxxx_eth_buffer_descriptor));
 #endif    
+
+	// A NOP is needed before flagging the transmitter.
+	// Otherwise the FEC could stall.
+	asm volatile ("nop");
+
     WRITE32(TDAR(eth), HAL_MCFxxxx_ETHx_TDAR_X_DES_ACTIVE);
 }
 
 // The TX interrupt should only trigger when a whole frame has been
 // transmitted. There is no need to worry about nested tx interrupts,

             reply	other threads:[~2009-03-03 11:45 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-03 11:45 Tarmo Kuuse [this message]
2009-03-04 11:10 ` Bart Veer
2009-03-05 10:29   ` Tarmo Kuuse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='goj05c$vm5$1@ger.gmane.org' \
    --to=tarmo.kuuse@mail.ee \
    --cc=ecos-patches@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).