milkymist network driver bug

[Yann Sionneau]

Hi Till,

Thank you for taking the time to look over the BSP :)

You pointed out two important problems of the driver.

The first has already been fixed a few days ago on my github repository, 
I will try to merge it ASAP with RTEMS CVS, I opened a PR about that [0].

For the "allocation of the dma buffer on the stack" bug, I haven't seen 
it :) Thanks for spotting that ! *ashamed*

I just pushed a fixed on my github for that : [1]

This driver is indeed not production ready, there is still a lot of 
cleaning (and implementing) to do. I have only a little time to give to 
this task so it will be done but when ? I don't know.

Feel free to merge these commits with the CVS, but it will not be a 
straight forward merge since my github repository has a lot of commit 
that your CVS doesn't have. We are out of sync.

But for such small commits it should be quite easy !

Thank you again for your comments :)

[0] - https://www.rtems.org/bugzilla/show_bug.cgi?id=1739
[1] - 
https://github.com/fallen/rtems-milkymist/commit/459b785b7f26d0eac52f6fb3efc0c9432d56bd69

Regards,

Yann Sionneau

Le 24/02/11 00:18, Till Straumann a écrit :
> Today I started playing with the milkymist BSP in HEAD.
>
> After some hacking I got some life out of lm32-qemu and this
> BSP but I'm not sure it has reached production quality quite yet.
>
> When sending a packet, minimac_sendpacket() copies the
> entire mbuf chain holding the packet into a driver-buffer
> (struct mm_packet) which is much smaller than the interface
> MTU and which is allocated *on*the*stack* and then handed
> (by reference) to the chip (for DMA, I guess).
> Any packet that is larger than 128 bytes overruns the stack
> and since it is very likely that the packet buffer is popped
> before the hardware is done reading it from memory it may
> very well be corrupted.
>
> Other peculiarities include not declaring memory-mapped
> registers as 'volatile' (in the network- and other drivers).
>
> -- T.
>
> PS: Here's a snippet of that code
>
> static void minimac_sendpacket(struct ifnet *ifp, struct mbuf *m)
> {
> struct mbuf *nm = m;
> struct minimac_softc *sc = &minimac_softc;
> unsigned int len = 0;
> struct mm_packet p;
> unsigned int crc;
> uint8_t i;
> for (i = 0 ; i < 7 ; i++) // Preamble
> p.preamble[i] = 0x55;
> p.preamble[7] = 0xd5;
>
> do
> {
> unsigned int mlen;
> mlen = nm->m_len;
> if (nm->m_len > 0) {
> m_copydata(nm, 0, mlen, p.raw_data + len);
> len += nm->m_len;
>
> } while ( (nm = nm->m_next) != 0 );
> for ( ; len < 60 ; len++)
> p.raw_data[len] = 0x00; // Padding
>
> crc = mm_ether_crc32((uint8_t *)p.raw_data, len); // CRC32
>
> p.raw_data[len] = crc & 0xff;
> p.raw_data[len+1] = (crc & 0xff00) >> 8;
> p.raw_data[len+2] = (crc & 0xff0000) >> 16;
> p.raw_data[len+3] = crc >> 24;
>
> len += 4; // We add 4 bytes of CRC32
>
> if (len + 8 < 64) {
> printk("[minimac] Packet is too small !\n");
> sc->txErrors++; // update stats
> return;
> }
>
> minimac_write(MM_MINIMAC_TXADR, (unsigned int)&p);
> minimac_write(MM_MINIMAC_TXREMAINING, (unsigned int)(len + 8));
> }
>
> where struct mm_packet is
>
> struct mm_packet {
> unsigned char preamble[8];
> char raw_data[MLEN]; /* MLEN == 128 bytes! */
> } __attribute__((aligned(4), packed));