[U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0

[Marek Vasut]

On 03/06/2017 03:25 AM, Dalon Westergreen wrote:
> On Sun, 2017-03-05 at 18:49 +0100, Marek Vasut wrote:
>> On 03/05/2017 06:38 PM, Dalon Westergreen wrote:
>>>
>>> On Tue, 2017-02-28 at 06:45 -0800, Dalon Westergreen wrote:
>>>>
>>>> On Mon, 2017-02-20 at 06:35 -0800, Dalon Westergreen wrote:
>>>>>
>>>>>
>>>>> On Mon, 2017-02-20 at 15:24 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 02/20/2017 03:21 PM, Dalon Westergreen wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Mon, 2017-02-20 at 15:14 +0100, Marek Vasut wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 02/20/2017 03:10 PM, Dalon Westergreen wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Mon, 2017-02-20 at 10:07 +0100, Marek Vasut wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 02/18/2017 02:34 AM, Dalon Westergreen wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> When CSEL=0x0 the socfpga bootrom does not touch the clock
>>>>>>>>>>> configuration for the device.  This can lead to a boot
>>>>>>>>>>> failure
>>>>>>>>>>> on warm resets. This patch disables warm resets when CSEL=0.
>>>>>>>>>>> This results in the clock and pll configurations being reset
>>>>>>>>>>> on any reset issued when CSEL=0.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Dalon Westergreen <dwesterg at gmail.com>
>>>>>>>>>>
>>>>>>>>>> What about my suggestion for V2 about just loading function
>>>>>>>>>> pointer
>>>>>>>>>> into
>>>>>>>>>> the reset jump address register ?
>>>>>>>>>
>>>>>>>>> Frankly, i really dont like relying on the existence of a
>>>>>>>>> snippet of
>>>>>>>>> code in
>>>>>>>>> the
>>>>>>>>> onchip ram being untouched to ensure a reboot/reset will occur
>>>>>>>>> for
>>>>>>>>> this
>>>>>>>>> csel=0
>>>>>>>>> case.  i am certain this case is rarely used, and confident that
>>>>>>>>> it
>>>>>>>>> isnt
>>>>>>>>> being
>>>>>>>>> used while trying to preserve sdram contents.
>>>>>>>>
>>>>>>>> Well, you already rely on such snippet, it's SPL. If you corrupt
>>>>>>>> SPL
>>>>>>>> and
>>>>>>>> do warm reset, your system hangs, I had that multiple times :)
>>>>>>>
>>>>>>> True.  I would argue to just use cold resets but i think arria 10
>>>>>>> has
>>>>>>> more
>>>>>>> use
>>>>>>> for the warm reset case.
>>>>>>
>>>>>> OK
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> the downside is that the scorecard is reset every boot. so the
>>>>>>>>> bootrom
>>>>>>>>> will
>>>>>>>>> retry all the spl images again resulting in possibly longer boot
>>>>>>>>> times.
>>>>>>>>
>>>>>>>> Is that significant ?
>>>>>>>
>>>>>>> The watchdog timeout is on the order of 1.5 seconds.  That would be
>>>>>>> for
>>>>>>> each
>>>>>>> failed spl.
>>>>>>
>>>>>> Hm, OK. But then your system is kinda broken, so you should expect
>>>>>> this
>>>>>> I guess.
>>>>>
>>>>> My thought exactly...  I would like to see if Chin Liang or Dinh have
>>>>> any
>>>>> comments?
>>>>
>>>> Chin Liang, Dinh, any comments?
>>>
>>> Marek, I would like to propose we move forward with this patch?
>>
>> TBH I'm still thinking if you turned the V2 into C code and passed a
>> function pointer into the warm reset jump address register, that'd be
>> the best.
>>
> I will give it a shot, i just dont like relying on the spl image still
> being present in the onchip ram.  There is nothing preventing an errant
> process or user from overwriting it and i have run into customers using
> that memory for their own purposes.
>
That's very true .

Then again, we use warm reset all over the place, so we might as well 
keep things consistent.