diff mbox

[2/2] net: Fix lan9118 buffer length handling

Message ID 1387563997-1845-3-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 uses the provided buffer length to limit the bytes transmitted.
Previously all the bytes of the last 32-bit word written to the TX fifo
were added to the internal transmit buffer structure resulting in more bytes
being transmitted than were submitted to the hardware in the command.  This
resulted in extra bytes being inserted into the middle of multi-buffer
packets when the non-final buffers had non-32bit aligned ending addresses.

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

Comments

Roy Franz Jan. 1, 2014, 8:09 p.m. UTC | #1
On Fri, Dec 27, 2013 at 5:25 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> 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 uses the provided buffer length to limit the bytes transmitted.
>> Previously all the bytes of the last 32-bit word written to the TX fifo
>> were added to the internal transmit buffer structure resulting in more bytes
>> being transmitted than were submitted to the hardware in the command.  This
>> resulted in extra bytes being inserted into the middle of multi-buffer
>> packets when the non-final buffers had non-32bit aligned ending addresses.
>>
>> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>> ---
>>  hw/net/lan9118.c |    3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
>> index c5d6f14..712bb41 100644
>> --- a/hw/net/lan9118.c
>> +++ b/hw/net/lan9118.c
>> @@ -773,11 +773,10 @@ static void tx_fifo_push(lan9118_state *s, uint32_t val)
>>                 in FIFO words.  Empirical results show it to be little-endian.
>>                 */
>>              /* TODO: FIFO overflow checking.  */
>> -            while (n--) {
>> +            while (n-- && s->txp->buffer_size--) {
>
> can you just get n right in the first place (line 766):
>
> n = MIN(4, s->txp_buffer_size);
>
> rather than doing min calculation on two parallel iteration variables?
>
> Regards,
> Peter
>
>>                  s->txp->data[s->txp->len] = val & 0xff;
>>                  s->txp->len++;
>>                  val >>= 8;
>> -                s->txp->buffer_size--;
>>              }
>>              s->txp->fifo_used++;
>>          }
>> --
>> 1.7.10.4
>>
>>

Hi Peter,

   I'll take a look at this (and your other comments on the extract32)
when I am back at work next week.

Thanks,
Roy
diff mbox

Patch

diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index c5d6f14..712bb41 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -773,11 +773,10 @@  static void tx_fifo_push(lan9118_state *s, uint32_t val)
                in FIFO words.  Empirical results show it to be little-endian.
                */
             /* TODO: FIFO overflow checking.  */
-            while (n--) {
+            while (n-- && s->txp->buffer_size--) {
                 s->txp->data[s->txp->len] = val & 0xff;
                 s->txp->len++;
                 val >>= 8;
-                s->txp->buffer_size--;
             }
             s->txp->fifo_used++;
         }