diff mbox

[3/5] hw/net/stellaris_enet: Rewrite tx fifo handling code

Message ID 1396371187-8567-4-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell April 1, 2014, 4:53 p.m. UTC
The datasheet is clear that the frame length written to the DATA
register is actually stored in the TX FIFO; this means we don't
need to keep both tx_frame_len and tx_fifo_len state separately.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/stellaris_enet.c | 119 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 77 insertions(+), 42 deletions(-)

Comments

Dr. David Alan Gilbert April 1, 2014, 5:26 p.m. UTC | #1
* Peter Maydell (peter.maydell@linaro.org) wrote:
> The datasheet is clear that the frame length written to the DATA
> register is actually stored in the TX FIFO; this means we don't
> need to keep both tx_frame_len and tx_fifo_len state separately.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/net/stellaris_enet.c | 119 +++++++++++++++++++++++++++++++-----------------
>  1 file changed, 77 insertions(+), 42 deletions(-)
> 
> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
> index d0da819..b99f93e 100644
> --- a/hw/net/stellaris_enet.c
> +++ b/hw/net/stellaris_enet.c
> @@ -59,7 +59,6 @@ typedef struct {
>      uint32_t mtxd;
>      uint32_t mrxd;
>      uint32_t np;
> -    int tx_frame_len;
>      int tx_fifo_len;
>      uint8_t tx_fifo[2048];
>      /* Real hardware has a 2k fifo, which works out to be at most 31 packets.
> @@ -82,6 +81,62 @@ static void stellaris_enet_update(stellaris_enet_state *s)
>      qemu_set_irq(s->irq, (s->ris & s->im) != 0);
>  }
>  
> +/* Return the data length of the packet currently being assembled
> + * in the TX fifo.
> + */
> +static inline int stellaris_txpacket_datalen(stellaris_enet_state *s)
> +{
> +    return s->tx_fifo[0] | (s->tx_fifo[1] << 8);
> +}
> +
> +/* Return true if the packet currently in the TX FIFO is complete,
> +* ie the FIFO holds enough bytes for the data length, ethernet header,
> +* payload and optionally CRC.
> +*/
> +static inline bool stellaris_txpacket_complete(stellaris_enet_state *s)
> +{
> +    int framelen = stellaris_txpacket_datalen(s);
> +    framelen += 16;

What's the magical 16? (It doesn't jump out from the data sheet).

> +    if (!(s->tctl & SE_TCTL_CRC)) {
> +        framelen += 4;
> +    }
> +    /* Cover the corner case of a 2032 byte payload with auto-CRC disabled:
> +     * this requires more bytes than will fit in the FIFO. It's not totally
> +     * clear how the h/w handles this, but if using threshold-based TX
> +     * it will definitely try to transmit something.
> +     */
> +    framelen = MIN(framelen, ARRAY_SIZE(s->tx_fifo));
> +    return s->tx_fifo_len >= framelen;
> +}
> +
> +/* Send the packet currently in the TX FIFO */
> +static void stellaris_enet_send(stellaris_enet_state *s)
> +{
> +    int framelen = stellaris_txpacket_datalen(s);
> +
> +    /* Ethernet header is in the FIFO but not in the datacount.
> +     * We don't implement explicit CRC, so just ignore any
> +     * CRC value in the FIFO.
> +     */
> +    framelen += 14;
> +    if ((s->tctl & SE_TCTL_PADEN) && framelen < 60) {
> +        memset(&s->tx_fifo[framelen + 2], 0, 60 - framelen);
> +        framelen = 60;
> +    }
> +    /* This MIN will have no effect unless the FIFO data is corrupt
> +     * (eg bad data from an incoming migration); otherwise the check
> +     * on the datalen at the start of writing the data into the FIFO
> +     * will have caught this. Silently write a corrupt half-packet,
> +     * which is what the hardware does in FIFO underrun situations.
> +     */
> +    framelen = MIN(framelen, ARRAY_SIZE(s->tx_fifo) - 2);
> +    qemu_send_packet(qemu_get_queue(s->nic), s->tx_fifo + 2, framelen);
> +    s->tx_fifo_len = 0;
> +    s->ris |= SE_INT_TXEMP;
> +    stellaris_enet_update(s);
> +    DPRINTF("Done TX\n");
> +}
> +
>  /* TODO: Implement MAC address filtering.  */
>  static ssize_t stellaris_enet_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>  {
> @@ -215,8 +270,9 @@ static void stellaris_enet_write(void *opaque, hwaddr offset,
>          DPRINTF("IRQ ack %02x/%02x\n", value, s->ris);
>          stellaris_enet_update(s);
>          /* Clearing TXER also resets the TX fifo.  */
> -        if (value & SE_INT_TXER)
> -            s->tx_frame_len = -1;
> +        if (value & SE_INT_TXER) {
> +            s->tx_fifo_len = 0;
> +        }
>          break;
>      case 0x04: /* IM */
>          DPRINTF("IRQ mask %02x/%02x\n", value, s->ris);
> @@ -235,46 +291,27 @@ static void stellaris_enet_write(void *opaque, hwaddr offset,
>          s->tctl = value;
>          break;
>      case 0x10: /* DATA */
> -        if (s->tx_frame_len == -1) {
> -            s->tx_frame_len = value & 0xffff;
> -            if (s->tx_frame_len > 2032) {
> -                DPRINTF("TX frame too long (%d)\n", s->tx_frame_len);
> -                s->tx_frame_len = 0;
> +        if (s->tx_fifo_len == 0) {
> +            /* The first word is special, it contains the data length */
> +            int framelen = value & 0xffff;
> +            if (framelen > 2032) {
> +                DPRINTF("TX frame too long (%d)\n", framelen);
>                  s->ris |= SE_INT_TXER;
>                  stellaris_enet_update(s);
> -            } else {
> -                DPRINTF("Start TX frame len=%d\n", s->tx_frame_len);
> -                /* The value written does not include the ethernet header.  */
> -                s->tx_frame_len += 14;
> -                if ((s->tctl & SE_TCTL_CRC) == 0)
> -                    s->tx_frame_len += 4;
> -                s->tx_fifo_len = 0;
> -                s->tx_fifo[s->tx_fifo_len++] = value >> 16;
> -                s->tx_fifo[s->tx_fifo_len++] = value >> 24;
> -            }
> -        } else {
> -            if (s->tx_fifo_len + 4 <= ARRAY_SIZE(s->tx_fifo)) {
> -                s->tx_fifo[s->tx_fifo_len++] = value;
> -                s->tx_fifo[s->tx_fifo_len++] = value >> 8;
> -                s->tx_fifo[s->tx_fifo_len++] = value >> 16;
> -                s->tx_fifo[s->tx_fifo_len++] = value >> 24;
> -            }
> -            if (s->tx_fifo_len >= s->tx_frame_len) {
> -                /* We don't implement explicit CRC, so just chop it off.  */
> -                if ((s->tctl & SE_TCTL_CRC) == 0)
> -                    s->tx_frame_len -= 4;
> -                if ((s->tctl & SE_TCTL_PADEN) && s->tx_frame_len < 60) {
> -                    memset(&s->tx_fifo[s->tx_frame_len], 0, 60 - s->tx_frame_len);
> -                    s->tx_frame_len = 60;
> -                }
> -                qemu_send_packet(qemu_get_queue(s->nic), s->tx_fifo,
> -                                 s->tx_frame_len);
> -                s->tx_frame_len = -1;
> -                s->ris |= SE_INT_TXEMP;
> -                stellaris_enet_update(s);
> -                DPRINTF("Done TX\n");
> +                break;
>              }
>          }
> +
> +        if (s->tx_fifo_len + 4 <= ARRAY_SIZE(s->tx_fifo)) {
> +            s->tx_fifo[s->tx_fifo_len++] = value;
> +            s->tx_fifo[s->tx_fifo_len++] = value >> 8;
> +            s->tx_fifo[s->tx_fifo_len++] = value >> 16;
> +            s->tx_fifo[s->tx_fifo_len++] = value >> 24;
> +        }
> +
> +        if (stellaris_txpacket_complete(s)) {
> +            stellaris_enet_send(s);
> +        }
>          break;
>      case 0x14: /* IA0 */
>          s->conf.macaddr.a[0] = value;
> @@ -326,7 +363,7 @@ static void stellaris_enet_reset(stellaris_enet_state *s)
>      s->im = SE_INT_PHY | SE_INT_MD | SE_INT_RXER | SE_INT_FOV | SE_INT_TXEMP
>              | SE_INT_TXER | SE_INT_RX;
>      s->thr = 0x3f;
> -    s->tx_frame_len = -1;
> +    s->tx_fifo_len = 0;
>  }
>  
>  static void stellaris_enet_save(QEMUFile *f, void *opaque)
> @@ -344,7 +381,6 @@ static void stellaris_enet_save(QEMUFile *f, void *opaque)
>      qemu_put_be32(f, s->mtxd);
>      qemu_put_be32(f, s->mrxd);
>      qemu_put_be32(f, s->np);
> -    qemu_put_be32(f, s->tx_frame_len);
>      qemu_put_be32(f, s->tx_fifo_len);
>      qemu_put_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
>      for (i = 0; i < 31; i++) {
> @@ -375,7 +411,6 @@ static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id)
>      s->mtxd = qemu_get_be32(f);
>      s->mrxd = qemu_get_be32(f);
>      s->np = qemu_get_be32(f);
> -    s->tx_frame_len = qemu_get_be32(f);
>      s->tx_fifo_len = qemu_get_be32(f);
>      qemu_get_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
>      for (i = 0; i < 31; i++) {

You should probably increment the migration state version number to 2.

Dave
> -- 
> 1.9.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Peter Maydell April 1, 2014, 5:29 p.m. UTC | #2
On 1 April 2014 18:26, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Peter Maydell (peter.maydell@linaro.org) wrote:
>> The datasheet is clear that the frame length written to the DATA
>> register is actually stored in the TX FIFO; this means we don't
>> need to keep both tx_frame_len and tx_fifo_len state separately.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/net/stellaris_enet.c | 119 +++++++++++++++++++++++++++++++-----------------
>>  1 file changed, 77 insertions(+), 42 deletions(-)
>>
>> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
>> index d0da819..b99f93e 100644
>> --- a/hw/net/stellaris_enet.c
>> +++ b/hw/net/stellaris_enet.c
>> @@ -59,7 +59,6 @@ typedef struct {
>>      uint32_t mtxd;
>>      uint32_t mrxd;
>>      uint32_t np;
>> -    int tx_frame_len;
>>      int tx_fifo_len;
>>      uint8_t tx_fifo[2048];
>>      /* Real hardware has a 2k fifo, which works out to be at most 31 packets.
>> @@ -82,6 +81,62 @@ static void stellaris_enet_update(stellaris_enet_state *s)
>>      qemu_set_irq(s->irq, (s->ris & s->im) != 0);
>>  }
>>
>> +/* Return the data length of the packet currently being assembled
>> + * in the TX fifo.
>> + */
>> +static inline int stellaris_txpacket_datalen(stellaris_enet_state *s)
>> +{
>> +    return s->tx_fifo[0] | (s->tx_fifo[1] << 8);
>> +}
>> +
>> +/* Return true if the packet currently in the TX FIFO is complete,
>> +* ie the FIFO holds enough bytes for the data length, ethernet header,
>> +* payload and optionally CRC.
>> +*/
>> +static inline bool stellaris_txpacket_complete(stellaris_enet_state *s)
>> +{
>> +    int framelen = stellaris_txpacket_datalen(s);
>> +    framelen += 16;
>
> What's the magical 16? (It doesn't jump out from the data sheet).

See table 15-3: 2 bytes of data length, 6 bytes dest addr,
6 bytes source addr, 2 bytes len/type.

> You should probably increment the migration state version number to 2.

Oops, yes.

thanks
-- PMM
Dr. David Alan Gilbert April 1, 2014, 5:35 p.m. UTC | #3
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 1 April 2014 18:26, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> >> The datasheet is clear that the frame length written to the DATA
> >> register is actually stored in the TX FIFO; this means we don't
> >> need to keep both tx_frame_len and tx_fifo_len state separately.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >>  hw/net/stellaris_enet.c | 119 +++++++++++++++++++++++++++++++-----------------
> >>  1 file changed, 77 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
> >> index d0da819..b99f93e 100644
> >> --- a/hw/net/stellaris_enet.c
> >> +++ b/hw/net/stellaris_enet.c
> >> @@ -59,7 +59,6 @@ typedef struct {
> >>      uint32_t mtxd;
> >>      uint32_t mrxd;
> >>      uint32_t np;
> >> -    int tx_frame_len;
> >>      int tx_fifo_len;
> >>      uint8_t tx_fifo[2048];
> >>      /* Real hardware has a 2k fifo, which works out to be at most 31 packets.
> >> @@ -82,6 +81,62 @@ static void stellaris_enet_update(stellaris_enet_state *s)
> >>      qemu_set_irq(s->irq, (s->ris & s->im) != 0);
> >>  }
> >>
> >> +/* Return the data length of the packet currently being assembled
> >> + * in the TX fifo.
> >> + */
> >> +static inline int stellaris_txpacket_datalen(stellaris_enet_state *s)
> >> +{
> >> +    return s->tx_fifo[0] | (s->tx_fifo[1] << 8);
> >> +}
> >> +
> >> +/* Return true if the packet currently in the TX FIFO is complete,
> >> +* ie the FIFO holds enough bytes for the data length, ethernet header,
> >> +* payload and optionally CRC.
> >> +*/
> >> +static inline bool stellaris_txpacket_complete(stellaris_enet_state *s)
> >> +{
> >> +    int framelen = stellaris_txpacket_datalen(s);
> >> +    framelen += 16;
> >
> > What's the magical 16? (It doesn't jump out from the data sheet).
> 
> See table 15-3: 2 bytes of data length, 6 bytes dest addr,
> 6 bytes source addr, 2 bytes len/type.

Ah yes or the text in 15.3.1.2 para 4 'referes to the Etehernet frame data
payload, as shown in the 5th to nth FIFO positions' (1,2,3,4 each 4 bytes).

> > You should probably increment the migration state version number to 2.
> 
> Oops, yes.

Fix if you need to reroll, but not too important since i doubt people are
migrating it.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Dr. David Alan Gilbert April 1, 2014, 5:45 p.m. UTC | #4
(resend reply - the mail gru got some of the 1st one)

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 1 April 2014 18:26, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> >> The datasheet is clear that the frame length written to the DATA
> >> register is actually stored in the TX FIFO; this means we don't
> >> need to keep both tx_frame_len and tx_fifo_len state separately.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >>  hw/net/stellaris_enet.c | 119 +++++++++++++++++++++++++++++++-----------------
> >>  1 file changed, 77 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
> >> index d0da819..b99f93e 100644
> >> --- a/hw/net/stellaris_enet.c
> >> +++ b/hw/net/stellaris_enet.c
> >> @@ -59,7 +59,6 @@ typedef struct {
> >>      uint32_t mtxd;
> >>      uint32_t mrxd;
> >>      uint32_t np;
> >> -    int tx_frame_len;
> >>      int tx_fifo_len;
> >>      uint8_t tx_fifo[2048];
> >>      /* Real hardware has a 2k fifo, which works out to be at most 31 packets.
> >> @@ -82,6 +81,62 @@ static void stellaris_enet_update(stellaris_enet_state *s)
> >>      qemu_set_irq(s->irq, (s->ris & s->im) != 0);
> >>  }
> >>
> >> +/* Return the data length of the packet currently being assembled
> >> + * in the TX fifo.
> >> + */
> >> +static inline int stellaris_txpacket_datalen(stellaris_enet_state *s)
> >> +{
> >> +    return s->tx_fifo[0] | (s->tx_fifo[1] << 8);
> >> +}
> >> +
> >> +/* Return true if the packet currently in the TX FIFO is complete,
> >> +* ie the FIFO holds enough bytes for the data length, ethernet header,
> >> +* payload and optionally CRC.
> >> +*/
> >> +static inline bool stellaris_txpacket_complete(stellaris_enet_state *s)
> >> +{
> >> +    int framelen = stellaris_txpacket_datalen(s);
> >> +    framelen += 16;
> >
> > What's the magical 16? (It doesn't jump out from the data sheet).
> 
> See table 15-3: 2 bytes of data length, 6 bytes dest addr,
> 6 bytes source addr, 2 bytes len/type.

Ah yes or the text in 15.3.1.2 para 4 'referes to the Etehernet frame data
payload, as shown in the 5th to nth FIFO positions' (1,2,3,4 each 4 bytes).

> > You should probably increment the migration state version number to 2.
>
> Oops, yes.

Fix if you need to reroll, but not too important since i doubt people are
migrating it.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Peter Maydell April 1, 2014, 5:48 p.m. UTC | #5
On 1 April 2014 18:45, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> (resend reply - the mail gru got some of the 1st one)
>
> * Peter Maydell (peter.maydell@linaro.org) wrote:
>> > You should probably increment the migration state version number to 2.
>>
>> Oops, yes.
>
> Fix if you need to reroll, but not too important since i doubt people are
> migrating it.

I'm tempted to convert it to vmstate, actually.

thanks
-- PMM
Dr. David Alan Gilbert April 1, 2014, 5:51 p.m. UTC | #6
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 1 April 2014 18:45, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > (resend reply - the mail gru got some of the 1st one)
> >
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> >> > You should probably increment the migration state version number to 2.
> >>
> >> Oops, yes.
> >
> > Fix if you need to reroll, but not too important since i doubt people are
> > migrating it.
> 
> I'm tempted to convert it to vmstate, actually.

That would be a wonderful thing to do.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox

Patch

diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index d0da819..b99f93e 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -59,7 +59,6 @@  typedef struct {
     uint32_t mtxd;
     uint32_t mrxd;
     uint32_t np;
-    int tx_frame_len;
     int tx_fifo_len;
     uint8_t tx_fifo[2048];
     /* Real hardware has a 2k fifo, which works out to be at most 31 packets.
@@ -82,6 +81,62 @@  static void stellaris_enet_update(stellaris_enet_state *s)
     qemu_set_irq(s->irq, (s->ris & s->im) != 0);
 }
 
+/* Return the data length of the packet currently being assembled
+ * in the TX fifo.
+ */
+static inline int stellaris_txpacket_datalen(stellaris_enet_state *s)
+{
+    return s->tx_fifo[0] | (s->tx_fifo[1] << 8);
+}
+
+/* Return true if the packet currently in the TX FIFO is complete,
+* ie the FIFO holds enough bytes for the data length, ethernet header,
+* payload and optionally CRC.
+*/
+static inline bool stellaris_txpacket_complete(stellaris_enet_state *s)
+{
+    int framelen = stellaris_txpacket_datalen(s);
+    framelen += 16;
+    if (!(s->tctl & SE_TCTL_CRC)) {
+        framelen += 4;
+    }
+    /* Cover the corner case of a 2032 byte payload with auto-CRC disabled:
+     * this requires more bytes than will fit in the FIFO. It's not totally
+     * clear how the h/w handles this, but if using threshold-based TX
+     * it will definitely try to transmit something.
+     */
+    framelen = MIN(framelen, ARRAY_SIZE(s->tx_fifo));
+    return s->tx_fifo_len >= framelen;
+}
+
+/* Send the packet currently in the TX FIFO */
+static void stellaris_enet_send(stellaris_enet_state *s)
+{
+    int framelen = stellaris_txpacket_datalen(s);
+
+    /* Ethernet header is in the FIFO but not in the datacount.
+     * We don't implement explicit CRC, so just ignore any
+     * CRC value in the FIFO.
+     */
+    framelen += 14;
+    if ((s->tctl & SE_TCTL_PADEN) && framelen < 60) {
+        memset(&s->tx_fifo[framelen + 2], 0, 60 - framelen);
+        framelen = 60;
+    }
+    /* This MIN will have no effect unless the FIFO data is corrupt
+     * (eg bad data from an incoming migration); otherwise the check
+     * on the datalen at the start of writing the data into the FIFO
+     * will have caught this. Silently write a corrupt half-packet,
+     * which is what the hardware does in FIFO underrun situations.
+     */
+    framelen = MIN(framelen, ARRAY_SIZE(s->tx_fifo) - 2);
+    qemu_send_packet(qemu_get_queue(s->nic), s->tx_fifo + 2, framelen);
+    s->tx_fifo_len = 0;
+    s->ris |= SE_INT_TXEMP;
+    stellaris_enet_update(s);
+    DPRINTF("Done TX\n");
+}
+
 /* TODO: Implement MAC address filtering.  */
 static ssize_t stellaris_enet_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 {
@@ -215,8 +270,9 @@  static void stellaris_enet_write(void *opaque, hwaddr offset,
         DPRINTF("IRQ ack %02x/%02x\n", value, s->ris);
         stellaris_enet_update(s);
         /* Clearing TXER also resets the TX fifo.  */
-        if (value & SE_INT_TXER)
-            s->tx_frame_len = -1;
+        if (value & SE_INT_TXER) {
+            s->tx_fifo_len = 0;
+        }
         break;
     case 0x04: /* IM */
         DPRINTF("IRQ mask %02x/%02x\n", value, s->ris);
@@ -235,46 +291,27 @@  static void stellaris_enet_write(void *opaque, hwaddr offset,
         s->tctl = value;
         break;
     case 0x10: /* DATA */
-        if (s->tx_frame_len == -1) {
-            s->tx_frame_len = value & 0xffff;
-            if (s->tx_frame_len > 2032) {
-                DPRINTF("TX frame too long (%d)\n", s->tx_frame_len);
-                s->tx_frame_len = 0;
+        if (s->tx_fifo_len == 0) {
+            /* The first word is special, it contains the data length */
+            int framelen = value & 0xffff;
+            if (framelen > 2032) {
+                DPRINTF("TX frame too long (%d)\n", framelen);
                 s->ris |= SE_INT_TXER;
                 stellaris_enet_update(s);
-            } else {
-                DPRINTF("Start TX frame len=%d\n", s->tx_frame_len);
-                /* The value written does not include the ethernet header.  */
-                s->tx_frame_len += 14;
-                if ((s->tctl & SE_TCTL_CRC) == 0)
-                    s->tx_frame_len += 4;
-                s->tx_fifo_len = 0;
-                s->tx_fifo[s->tx_fifo_len++] = value >> 16;
-                s->tx_fifo[s->tx_fifo_len++] = value >> 24;
-            }
-        } else {
-            if (s->tx_fifo_len + 4 <= ARRAY_SIZE(s->tx_fifo)) {
-                s->tx_fifo[s->tx_fifo_len++] = value;
-                s->tx_fifo[s->tx_fifo_len++] = value >> 8;
-                s->tx_fifo[s->tx_fifo_len++] = value >> 16;
-                s->tx_fifo[s->tx_fifo_len++] = value >> 24;
-            }
-            if (s->tx_fifo_len >= s->tx_frame_len) {
-                /* We don't implement explicit CRC, so just chop it off.  */
-                if ((s->tctl & SE_TCTL_CRC) == 0)
-                    s->tx_frame_len -= 4;
-                if ((s->tctl & SE_TCTL_PADEN) && s->tx_frame_len < 60) {
-                    memset(&s->tx_fifo[s->tx_frame_len], 0, 60 - s->tx_frame_len);
-                    s->tx_frame_len = 60;
-                }
-                qemu_send_packet(qemu_get_queue(s->nic), s->tx_fifo,
-                                 s->tx_frame_len);
-                s->tx_frame_len = -1;
-                s->ris |= SE_INT_TXEMP;
-                stellaris_enet_update(s);
-                DPRINTF("Done TX\n");
+                break;
             }
         }
+
+        if (s->tx_fifo_len + 4 <= ARRAY_SIZE(s->tx_fifo)) {
+            s->tx_fifo[s->tx_fifo_len++] = value;
+            s->tx_fifo[s->tx_fifo_len++] = value >> 8;
+            s->tx_fifo[s->tx_fifo_len++] = value >> 16;
+            s->tx_fifo[s->tx_fifo_len++] = value >> 24;
+        }
+
+        if (stellaris_txpacket_complete(s)) {
+            stellaris_enet_send(s);
+        }
         break;
     case 0x14: /* IA0 */
         s->conf.macaddr.a[0] = value;
@@ -326,7 +363,7 @@  static void stellaris_enet_reset(stellaris_enet_state *s)
     s->im = SE_INT_PHY | SE_INT_MD | SE_INT_RXER | SE_INT_FOV | SE_INT_TXEMP
             | SE_INT_TXER | SE_INT_RX;
     s->thr = 0x3f;
-    s->tx_frame_len = -1;
+    s->tx_fifo_len = 0;
 }
 
 static void stellaris_enet_save(QEMUFile *f, void *opaque)
@@ -344,7 +381,6 @@  static void stellaris_enet_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, s->mtxd);
     qemu_put_be32(f, s->mrxd);
     qemu_put_be32(f, s->np);
-    qemu_put_be32(f, s->tx_frame_len);
     qemu_put_be32(f, s->tx_fifo_len);
     qemu_put_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
     for (i = 0; i < 31; i++) {
@@ -375,7 +411,6 @@  static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id)
     s->mtxd = qemu_get_be32(f);
     s->mrxd = qemu_get_be32(f);
     s->np = qemu_get_be32(f);
-    s->tx_frame_len = qemu_get_be32(f);
     s->tx_fifo_len = qemu_get_be32(f);
     qemu_get_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
     for (i = 0; i < 31; i++) {