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

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

Hi,


On 08/13/2015 05:44 PM, Marek Vasut wrote:
> 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 ?

I already checked & it does not exist in the hardware. Solution to that are timeout interrupts
in indirect mode or use only direct mode. In case of direct mode, the u-boot driver can't be used.

But again, can we add some fix for stv0991 arch to avoid sram level. Without that we will not be
able to use the driver at all for stv0991.

In general, again it seems not a unique problem. Any idea how to apply fixes for such peripheral design bugs.

Rgds,
Vikas

> 
> .
>