diff mbox

[1/2] net: Fix lan9118 TX "CMD A" handling

Message ID 1387563997-1845-2-git-send-email-roy.franz@linaro.org
State New
Headers show

Commit Message

Roy Franz Dec. 20, 2013, 6:26 p.m. UTC
The 9118 ethernet controller supports transmission of multi-buffer packets
with arbitrary byte alignment of the start and end bytes.  All writes to
the packet fifo are 32 bits, so the controller discards bytes at the beginning
and end of each buffer based on the 'Data start offset' and 'Buffer size'
of the TX command 'A' format.

This patch changes the buffer size and offset internal state variables to be
updated on every "TX command A" write.  Previously they were only updated for
the first segment, which resulted incorrect behavior for packets with more
than one segment. Each segment of the packet has its own CMD A command, with
its own buffer size and start offset.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 hw/net/lan9118.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Peter Crosthwaite Dec. 28, 2013, 1:25 a.m. UTC | #1
On Sat, Dec 21, 2013 at 4:26 AM, Roy Franz <roy.franz@linaro.org> wrote:
> The 9118 ethernet controller supports transmission of multi-buffer packets
> with arbitrary byte alignment of the start and end bytes.  All writes to
> the packet fifo are 32 bits, so the controller discards bytes at the beginning
> and end of each buffer based on the 'Data start offset' and 'Buffer size'
> of the TX command 'A' format.
>
> This patch changes the buffer size and offset internal state variables to be
> updated on every "TX command A" write.  Previously they were only updated for
> the first segment, which resulted incorrect behavior for packets with more
> than one segment. Each segment of the packet has its own CMD A command, with
> its own buffer size and start offset.
>
> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> ---
>  hw/net/lan9118.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
> index 2315f99..c5d6f14 100644
> --- a/hw/net/lan9118.c
> +++ b/hw/net/lan9118.c
> @@ -727,14 +727,14 @@ static void tx_fifo_push(lan9118_state *s, uint32_t val)
>          s->txp->cmd_a = val & 0x831f37ff;
>          s->txp->fifo_used++;
>          s->txp->state = TX_B;
> +        s->txp->buffer_size = s->txp->cmd_a & 0x7ff;
> +        s->txp->offset = (s->txp->cmd_a >> 16) & 0x1f;

While you are touching this, you could change it to use the preferable
extract32 macro rather than the & >> logic.

Regards,
Peter

>          break;
>      case TX_B:
>          if (s->txp->cmd_a & 0x2000) {
>              /* First segment */
>              s->txp->cmd_b = val;
>              s->txp->fifo_used++;
> -            s->txp->buffer_size = s->txp->cmd_a & 0x7ff;
> -            s->txp->offset = (s->txp->cmd_a >> 16) & 0x1f;
>              /* End alignment does not include command words.  */
>              n = (s->txp->buffer_size + s->txp->offset + 3) >> 2;
>              switch ((n >> 24) & 3) {
> --
> 1.7.10.4
>
>
diff mbox

Patch

diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index 2315f99..c5d6f14 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -727,14 +727,14 @@  static void tx_fifo_push(lan9118_state *s, uint32_t val)
         s->txp->cmd_a = val & 0x831f37ff;
         s->txp->fifo_used++;
         s->txp->state = TX_B;
+        s->txp->buffer_size = s->txp->cmd_a & 0x7ff;
+        s->txp->offset = (s->txp->cmd_a >> 16) & 0x1f;
         break;
     case TX_B:
         if (s->txp->cmd_a & 0x2000) {
             /* First segment */
             s->txp->cmd_b = val;
             s->txp->fifo_used++;
-            s->txp->buffer_size = s->txp->cmd_a & 0x7ff;
-            s->txp->offset = (s->txp->cmd_a >> 16) & 0x1f;
             /* End alignment does not include command words.  */
             n = (s->txp->buffer_size + s->txp->offset + 3) >> 2;
             switch ((n >> 24) & 3) {