[U-Boot] [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes

[Stefan Roese <sr@denx.de>]

Hi Vikas,

On 09.07.2015 03:29, Vikas MANOCHA wrote:
>> -----Original Message-----
>> From: Vikas MANOCHA
>> Sent: Wednesday, July 01, 2015 9:25 AM
>> To: 'Stefan Roese'
>> Cc: 'u-boot at lists.denx.de'; 'grmoore at opensource.altera.com';
>> 'dinguyen at opensource.altera.com'; 'jteki at openedev.com'
>> Subject: RE: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect
>> rd-writes
>>
>> Hi Stefan,
>>
>>> -----Original Message-----
>>> From: Vikas MANOCHA
>>> Sent: Monday, June 22, 2015 4:31 PM
>>> To: 'Stefan Roese'
>>> Cc: u-boot at lists.denx.de; grmoore at opensource.altera.com;
>>> dinguyen at opensource.altera.com; jteki at openedev.com
>>> Subject: RE: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix
>>> indirect rd-writes
>>>
>>> Thanks Stefan,
>>>
>>>> -----Original Message-----
>>>> From: Stefan Roese [mailto:sr at denx.de]
>>>> Sent: Monday, June 22, 2015 1:35 AM
>>>> To: Vikas MANOCHA
>>>> Cc: u-boot at lists.denx.de; grmoore at opensource.altera.com;
>>>> dinguyen at opensource.altera.com; jteki at openedev.com
>>>> Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix
>>>> indirect rd-writes
>>>>
>>>> Hi Vikas,
>>>>
>>>> On 19.06.2015 23:38, Vikas MANOCHA wrote:
>>>>>>> - git bisect or cherry-pick to find out which patch is breaking the
>>>>>>>     read functionality.
>>>>>>
>>>>>> This one is the first introducing this breakage:
>>>>>>
>>>>>> spi: cadence_qspi: fix base trigger address & transfer start
>>>>>> address
>>>>>>
>>>>>
>>>>> Ok, can you confirm applying patches upto (& including)
>>>>> "spi: cadence_qspi: fix indirect read/write start address" , read/write
>>>>>    to flash are working fine.
>>>>
>>>> Please note that with this patch the code does not compile:
>>>>
>>>> drivers/spi/cadence_qspi_apb.c: In function
>> 'qpsi_write_sram_fifo_push':
>>>> drivers/spi/cadence_qspi_apb.c:227:32: error: 'struct
>>> cadence_spi_platdata'
>>>> has no member named 'trigger_base'
>>>>     unsigned int *dest_addr = plat->trigger_base;
>>>>
>>>> I've manually fixed this trivial change (trigger_base -> ahbbase)
>>>> and tests with these patches applied show some problems with "sf"
>>>> stability
>>>> (bit-flips):
>>>
>>> Sorry for this dumb mistake which happened during rebase :-(,  I will
>>> correct in next patch series.
>>>
>>>>
>>>> => sf update 400000 100000 100000
>>>> 1048576 bytes written, 0 bytes skipped in 34.196s, speed 31395 B/s
>>>> => sf read
>>>> 500000 100000 100000
>>>> SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000
>>>> byte at 0x0040001e (0x9f) != byte at 0x0050001e (0x8f) Total of 30
>>>> byte(s) were the same
>>>>
>>>> This is new - removing all your patches seems to solve this issue again.
>>>
>>> Thanks again Stefan for these tests. It is not easy for me to figure
>>> out this instability without the board but I don 't see any logical error in the
>> patches.
>>> Off-course code is more optimized & fast with these patches. Access
>>> timing in the log provided by you also confirms it.
>>>
>>> I can suggest following checks on the hardware:
>>>
>>> - Figure out the issue is with read or write, which means comparing
>>> the flash with ram after read/write.
>>> - Put some random microsecond delays in the read/write to confirm it
>>> is timing issue.
>>
>> Any update on it ?
>
> In addition can you please check the patch causing this instability
> on socfpga. I don't like to bug you but
> to close this patchset, this info & tests mentioned above seems to
> be required.

Okay. I'll try to find some time this week to do some testing here. It 
seems that the other cadence patchset from you ([v4 00/10] spi: 
cadence_qspi: sram depth from DT & fix for FIFO width) is not pulled 
into mainline yet. To make it easier for me, could you perhaps publish a 
git repository that is based on current mainline. And has the mentioned 
above patch series included. And all the patches in the latest version 
that are currently causing these problems on SoCFPGA?

> I am ready to check it but would need the hardware platform.

Understood.

Thanks,
Stefan