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

[Marek Vasut]

On 02/20/2017 03:07 PM, Dalon Westergreen wrote:
[...]
>>> Okay, after much discussion and debate with a colleague..\
>>>
>>> Warm reset is preferred as the bootrom keeps a score card to determine
>>> whether an spl image in the boot media failed or not.  If it failed,
>>> on a warm reset it will not retry the failed image.
>>
>> So what will it do ? Try a valid image from another slot at offset
>> +n*64kiB ?
> 
> Yes.  Or when the scorecard indicates 4 failures it will just stop.

I see.

>>>
>>> The other reason warm resets are preferred is for preservation of the
>>> dram contents.  On a warm reset it is possible to skip io configuration
>>> and dram calibration so that the contents can be saved.
>>
>> That's a good point.
>>
>> But here's a counterargument, what if you upgrade U-Boot ? Warm reset
>> will use the old SPL and the system will likely hang upon reboot ;-)
>>
>>>
>>>>
>>>> How do you point bootrom to run that snippet instead of whatever is in
>>>> the OCRAM ?
>>>>
>>>
>>> This code here
>>>
>>>>
>>>>  > > > > > > +		writel(ramboot_addr,
>>>>  > > > > > > +		      &sysmgr_regs-
>>>>  > > > > > > >romcodegrp_warmramgrp_execution);
>>
>> Can't you just feed a function pointer pointing into some function which
>> is part of the SPL into that register then ? I think that'd do the same
>> trick, no ?
> 
> Yes, you could, but the idea of putting the code at the end of memory is to
> allow the onchip ram to be used for other things.

Well if you corrupt SPL and do a warm-reset, your system won't boot. We
had that already (and that's another +1 for just doing cold reset, always).

>>>
>>> writes the address to jump to on warm reset.  The register value
>>> is preserved through a warm reset.
>>
>> That's neat, I didn't know that.
>>
>>>
>>>>
>>>>
>>>
>>> All that said, i frankly do not believe for the CSEL=0 case
>>> there is any merit to doing the above.  Although it "preserves"
>>> the behaviour as compared to other CSEL values, i think a much
>>> simpler method is to, for the CSEL=0 case, just issue a cold reset.
>>>
>>> As in this case we are touching the clocks, i am not sure the
>>> use cases for a warm reset even hold (sdram preservation, etc).
>>> So i agree with you, and suggest only enabling the warm reset
>>> for cases where CSEL != 0.
>>>
>>> Unless there are objections, I will do just that and resubmit a
>>> patch. (which should now be just a few lines of code)
>>
>> See above, if this actually fixes issue, let's get it in. But in a
>> civilized fashion, no random ad-hoc asm if possible please :)
> 
> In v3 i just used the simpler method of not allowing warm resets for
> csel=0. This is far cleaner, and likely more reliable.

Hm, OK, it's fine either way for me.

-- 
Best regards,
Marek Vasut