Confusing sequence in libchip/network/smc91111.c

[Peter Dufault]

On Feb 9, 2014, at 04:58 , Peter Dufault <dufault at hda.com> wrote:

> I decided to look at adding DMA support for the Phytec MPC5554 for the SMC91111 to speed up network performance.  It's a twisty piece of work.  So far I've only looked at where the info in the receive FIFO is copied into the mbuf.
> 
> Here's the critical loop (around line 215).  LAN91CXX_32BIT_RX is defined, meaning "sizeof(rxdt)" and "sizeof(*data)" are 2:
> 
>    while (mlen >= sizeof(*data)) {
> #ifdef LAN91CXX_32BIT_RX
>        rxd_t val = get_data(cpd);
>        *(unsigned short *)data = (val >> 16) & 0xffff;
>        data = (rxd_t *)((unsigned short *)data + 1); 
>        *(unsigned short *)data = (val & 0xffff);
>        data = (rxd_t *)((unsigned short *)data + 1); 
> #else
>        *data++ = get_data(cpd);
> #endif
>        mlen -= sizeof(*data);
>        plen -= sizeof(*data);
>    }
> 
> Let's simplify this for the case of sizeof(rxd_t) == 4:
> 
>    while (mlen >= sizeof(rxd_t)) {
>        rxd_t val = get_data(cpd);
>        ((uint16_t *)data)[0] = val >> 16;
>        ((uint16_t *)data)[1] = val;
>        data++;
>        mlen -= sizeof(rxd_t);
>        plen -= sizeof(rxd_t);
>    }
> 
> What is this trying to do?  I think on a big endian system it's (almost, other than the extra work) a NOP and on a little endian system it's swapping the two 16 bit halves of the word, but it's not swapping the bytes. I don't understand.
> 
> Maybe this is only in use on big endian systems?  I'm using it on PowerPC and I know it's also used on the SPARC.
> 

This pretty obviously has only been tested in RTEMS at least on big endian systems.  In smc91111config.h these are defined:

# define CYG_CPU_TO_BE16(_x_) (_x_)
# define CYG_CPU_TO_BE32(_x_) (_x_)
# define CYG_BE16_TO_CPU(_x_) (_x_)
# define CYG_BE32_TO_CPU(_x_) (_x_)

I'm also very confused about the odd byte handling in the transmit.  In the data sheet for the chip it says this about the ODD bit in the control byte:

"ODD - If set, indicates an odd number of bytes, with the last byte being right before the CONTROL BYTE. If clear, the number of data bytes is even and the byte before the CONTROL BYTE is not transmitted."

But the driver is looking to see if the buffer in the struct mbuf is odd aligned, and not at the number of bytes transferred.  I don't understand.

Peter
-----------------
Peter Dufault
HD Associates, Inc.      Software and System Engineering