[U-Boot] mv_egiga: effect on making struct members volatile(was: [PATCH V2 3/6] mv_egiga: bugfix: DMA issue fixed using volatile)

[Albert ARIBAUD]

Le 09/07/2010 11:54, Albert ARIBAUD a écrit :

>> I think we should debug on this to fix the bug correctly.
>>
>> Regards..
>> Prafulla . .
>
> I have debugged the issue when it happened and found out that the it is
> not really about doing volatile accesses, but about doing accesses
> out-of-order: the compiler intermixes writes from adjacent source code
> lines, which causes the write to the DMA start bit to occur before
> writes to the descriptors are done. Apparently qualifying the struct
> members volatile has an effect or write order that readl/writel have not.
>
> I'll do a detailed comparison of disassembled code with and without the
> volatile qualifiers.

I've done builds with and without the volatile patch and an objdump -S 
of each, concentrating on function mv_egiga_send around lines 540-545 
(filling of TX descriptor) and 549-549 (actual launch of DMA) of mv_egiga.c.

These confirm that the issue is of write ordering, as making the members 
volatile changes this ordering and solves the issue.

Most notably, there is one tx desc field, p_txdesc->byte_cnt, which is 
filled in after start of DMA in the 'bad' case, and before start of DMA 
in the 'good' case.

After reading arch/arm/include/asm/io.h correctly, it seems like in*() 
and out*() macros do include sequence points to make sure their relative 
order is preserved, but writel/readl don't. I'll try wihout volatile but 
with a sequence point forced at the TQC register write.

Amicalement,
-- 
Albert.