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

[Marek Vasut]

On Friday, August 14, 2015 at 02:26:02 AM, vikas wrote:

Hi!

> >>>>>>>>>> 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 ?
> 
> Watermark is for sram level and might not work as well.
> In general also for such soc/platform issues for any driver, any idea how
> to apply fixes.

That's a good question . Is there any way on the ST chip to avoid such direct
blocking read which can stall the CPU core indefinitelly? Can you check with
the chip designers ?