[Prism54-devel] Re: [PATCH] for freemac

[Sebastien]

Hi,

Thanks again for the patches ! I applied most of them.

> 7.patch: dmaw was dma'ing only one 32bit word.
>         Make it consistent with dmar. Works for my hw.

This is normal, since I only want to transfer one word whose value is 
passed as parameter for now (to easily test with NET2280 registers).

On my hardware, only the three least significant bits are actually 
transferred (0x1000000b is where lies a NET2280 scratchpad register) :

FreeMAC > dmar 0x1000000b 1
rxdma complete
00000000: 00000000
dma status = 0x00000005
FreeMAC > dmaw 0x1000000b 0xffffffff
txdma complete
dma status:00000004
FreeMAC > dmar 0x1000000b 1
rxdma complete
00000000: 00000003
dma status = 0x00000005
FreeMAC > dmaw 0x1000000b 2
txdma complete
dma status:00000004
FreeMAC > dmar 0x1000000b 1
rxdma complete
00000000: 00000002
dma status = 0x00000005

That's what I precisely means by "does not work" on the webpage.

> I personally don't like if() statements with assignments
> or function calls inside:
>         if (uartpci_ringbuffer_enqueue(&uartpci_ringbuffer_rx, &c, &s) 
> 0) ...
>         if ( (var = expr1) == expr2 ) ...
> It's a bit hard to read. I prefer these splitted up:
>         var = expr1;
>         if(var == expr2) ...
> Takes one line more but allows reader to save on aspirine.
> What do you think?

Ok, let's do as you want. I don't really care.

> volatile keyword: if it is really needed, put a small comment
> why it is needed.

It is only needed obviously in isl38xx_hardware.h - I removed it from 
other places.

> Patch 7 adds handling of dma completion events. Works for me.
> Seems like these events must be handled by FIQ. When I tried
> to make them trigger IRQ, they didn't.

Also applied. I'll try to see what's wrong when using IRQs - we should 
be able to do that, I think. Doesn't the official firmware use IRQs 
there ?

Regards,
Sebastien