[U-Boot] [PATCH V2 3/6] mv_egiga: bugfix: DMA issue fixed using volatile

[Albert ARIBAUD <albert.aribaud@free.fr>]

Le 09/07/2010 11:16, Prafulla Wadaskar a ?crit :
>
>
>> -----Original Message-----
>> From: u-boot-bounces at lists.denx.de
>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
>> Sent: Friday, July 09, 2010 12:04 PM
>> To: u-boot at lists.denx.de
>> Subject: [U-Boot] [PATCH V2 3/6] mv_egiga: bugfix: DMA issue
>> fixed using volatile
>
> For each patch post, there must be some informative explanation about a patch that helps everybody to understand the objective of the patch, this is mostly applicable for all your patches
>
> Change log is something different, that is for developers and reviewers, that will not reflect on the repository after acceptance.
>
>>
>>
>> Signed-off-by: Albert Aribaud<albert.aribaud@free.fr>
>> ---
>>   drivers/net/mv_egiga.c |    2 +-
>>   drivers/net/mv_egiga.h |  206
>> ++++++++++++++++++++++++------------------------
>>   2 files changed, 104 insertions(+), 104 deletions(-)
>>
>> diff --git a/drivers/net/mv_egiga.c b/drivers/net/mv_egiga.c
>> index 0da4c4b..96e6a5a 100644
>> --- a/drivers/net/mv_egiga.c
>> +++ b/drivers/net/mv_egiga.c
>> @@ -177,7 +177,7 @@ static int smi_reg_write(char *devname,
>> u8 phy_adr, u8 reg_ofs, u16 data)
>>   }
>>
>>   /* Stop and checks all queues */
>> -static void stop_queue(u32 * qreg)
>> +static void stop_queue(volatile u32 * qreg)
>>   {
>>   	u32 reg_data;
>>
>> diff --git a/drivers/net/mv_egiga.h b/drivers/net/mv_egiga.h
>> index 61c3157..c49bf40 100644
>> --- a/drivers/net/mv_egiga.h
>> +++ b/drivers/net/mv_egiga.h
>> @@ -342,108 +342,108 @@
>>
>>   /* structures represents Controller registers */
>>   struct mv_egiga_barsz {
>> -	u32 bar;
>> -	u32 size;
>> +	volatile u32 bar;
>> +	volatile u32 size;
>
> May be, making all volatile solves problem at your end, that does not mean it is solution.
> Mostly all registers are accessed as volatile (readl/writel), register pointers are also volatile.
>
> I agree with you and suspect some code related to DMA pipes initialization has problem that's breaking it's functionality on Orion5X.
>
> 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.

Amicalement,
-- 
Albert.