[U-Boot] [v2 2/6] spi: cadence_qspi: remove sram polling from flash read

[Marek Vasut]

On Friday, August 14, 2015 at 01:18:59 AM, vikas wrote:
> Hi Marek,

Hi!

> On 08/13/2015 03:47 PM, Marek Vasut wrote:
> > On Thursday, August 13, 2015 at 11:04:59 PM, vikas wrote:
> >> Hi Marek,
> > 
> > Hi!
> > 
> >> On 08/13/2015 01:35 PM, Marek Vasut wrote:
> >>> On Thursday, August 13, 2015 at 09:49:49 PM, vikas wrote:
> >>> 
> >>> Hi!
> >>> 
> >>>>>> On 08/12/2015 07:09 PM, Marek Vasut wrote:
> >>>>>>> On Thursday, July 16, 2015 at 04:27:30 AM, Vikas Manocha wrote:
> >>>>>>>> There is no need to check for sram fill level. If sram is empty,
> >>>>>>>> cpu will go in the wait state till the time data is available
> >>>>>>>> from flash.
> >>>>>>> 
> >>>>>>> Consider the following scenario:
> >>>>>>> - CPU core reads some memory area, but there are no data available
> >>>>>>> just yet
> >>>>>>> 
> >>>>>>>   => CPU core goes into wait state
> >>>>>>> 
> >>>>>>> - The data never arrive
> >>>>>>> 
> >>>>>>> Will the CPU be stuck forever ? If we checked the fill level first
> >>>>>>> instead, we would never enter such stuck-state.
> >>>>>> 
> >>>>>> This indirect mode of reading/writing would be entered when the
> >>>>>> read/write addresses are in the programmed valid range of addresses.
> >>>>>> 
> >>>>>> Even in case of "data never arrive" scenario, a simple timeout seems
> >>>>>> better then currently implemented read sram level with timeout.
> >>>>> 
> >>>>> How do you implement a "simple timeout" if the CPU core is stuck and
> >>>>> does not execute instructions ? If you mean interrupt, then forget
> >>>>> it, U-Boot does not do interrupts ;-)
> >>>> 
> >>>> Oh yes, you are right.
> >>> 
> >>> So shall we keep the SRAM piece ?
> >> 
> >> Although in this case the better solution would be to have watermark
> >> interrupt/status check based on sram fill level, let us keep the
> >> existing piece of SRAM.
> > 
> > Good.
> > 
> >> Can we make it configurable (SRAM Level test or not) like from DT or
> >> #define ?
> > 
> > How would you call such an option ? Something like CONFIG_SYS_YOLO (to
> > indicate that life is too short to use correct, but slower code) ? :-)
> > 
> > I don't want to have two different codepaths in the codebase, one of
> > which is buggy. So no, I disagree we should add this option. I also
> > don't think it would be such a performance improvement, so I only see
> > downsides in such a code.
> 
> I expected the same answer :-) & agree also.

Heh, thanks :-)

> ok, the issue is SRAM Fill Level register is not being updated on my SOC,
> seems like design issue. Any idea how can i add this fix (to avoid sram
> level polling) to my soc in u-boot mainline.

Mask all interrupts in the controller, set watermark to N bytes (depending
on the length of your transfer) and poll the interrupt status register until
the watermark is reached ? Is this possible ?