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

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

Hi,

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

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.

Rgds,
Vikas

> .
>