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

[Vikas MANOCHA <vikas.manocha@st.com>]

Hi Stefan,

> -----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.

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

Rgds,
Vikas

> 
> Rgds,
> Vikas
> 
> >
> > > So there seems to be something wrong with the previous patches as
> well.
> > > here the output with the patches reverted:
> > >
> > > => sf probe
> > > SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB,
> > > total
> > > 32 MiB
> > > SF: Warning - Only lower 16MiB accessible, Full access #define
> > > CONFIG_SPI_FLASH_BAR => sf update 400000 100000 100000
> > > 1048576 bytes written, 0 bytes skipped in 35.872s, speed 29929 B/s
> > > => sf read
> > > 500000 100000 100000
> > > SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000
> > > Total of 1048576 byte(s) were the same
> > >
> > > > The point is if after applying above mentioned patch (...: fix
> > > > indirect read/write start address), Read/write are working fine,
> > > > then trigger_base value of 0xFFA00_0000 should also work fine.
> > > > Can you please modify the trigger_base value from 0x0 to
> > > > 0xFFA0_0000 in Socfpga.dtsi & check.
> > > > If it works, it would mean both (socfpga & stv0991) are behaving same.
> > >
> > > No. With this change, the "sf read" command crashes / hangs on the
> > > SoCFPGA board.
> >
> > Ok, I don't know why socfpga is not working with trigger_base to be
> > 0xFFA0_0000.
> > Normally it should work, Graham also thinks the same, Let's wait for
> > his discussion with the Altera designers.
> >
> > Rgds,
> > Vikas
> >
> > >
> > > Thanks,
> > > Stefan