[U-Boot] [PATCH] arm: socfpga: control reboot from SRAM via env callback

Wed Jul 10 18:19:35 UTC 2019

Am 06.05.2019 um 23:12 schrieb Marek Vasut:
> On 5/6/19 9:50 PM, Simon Goldschmidt wrote:
>> Am 06.05.2019 um 00:51 schrieb Marek Vasut:
>>> On 5/5/19 10:21 PM, Simon Goldschmidt wrote:
>>>> Am 05.05.2019 um 22:17 schrieb Marek Vasut:
>>>>> On 5/5/19 8:05 AM, Simon Goldschmidt wrote:
>>>>>>
>>>>>>
>>>>>> On 05.05.19 03:42, Marek Vasut wrote:
>>>>>>> On 5/4/19 9:10 PM, Simon Goldschmidt wrote:
>>>>>>>> Am 04.05.2019 um 20:43 schrieb Marek Vasut:
>>>>>>>>> On 5/3/19 10:53 PM, Simon Goldschmidt wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Marek Vasut <marex at denx.de <mailto:marex at denx.de>> schrieb am
>>>>>>>>>> Fr., 3.
>>>>>>>>>> Mai 2019, 22:42:
>>>>>>>>>>
>>>>>>>>>>          On 5/3/19 10:39 PM, Simon Goldschmidt wrote:
>>>>>>>>>>          >
>>>>>>>>>>          >
>>>>>>>>>>          > On 03.05.19 22:35, Marek Vasut wrote:
>>>>>>>>>>          >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote:
>>>>>>>>>>          >>>
>>>>>>>>>>          >>>
>>>>>>>>>>          >>> On 03.05.19 22:28, Marek Vasut wrote:
>>>>>>>>>>          >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote:
>>>>>>>>>>          >>>>> This moves the code that enables the Boot ROM to
>>>>>>>>>> just jump
>>>>>>>>>> to SRAM
>>>>>>>>>>          >>>>> instead
>>>>>>>>>>          >>>>> of loading SPL from the original boot source on warm
>>>>>>>>>> reboot.
>>>>>>>>>>          >>>>>
>>>>>>>>>>          >>>>> Instead of always enabling this, an environment
>>>>>>>>>> callback
>>>>>>>>>> for the
>>>>>>>>>>          >>>>> env var
>>>>>>>>>>          >>>>> "socfpga_reboot_from_sram" is used. This way, the
>>>>>>>>>> behaviour can be
>>>>>>>>>>          >>>>> enabled
>>>>>>>>>>          >>>>> at runtime and via saved environment.
>>>>>>>>>>          >>>>>
>>>>>>>>>>          >>>>> Signed-off-by: Simon Goldschmidt
>>>>>>>>>>          <simon.k.r.goldschmidt at gmail.com
>>>>>>>>>>          <mailto:simon.k.r.goldschmidt at gmail.com>>
>>>>>>>>>>          >>>>
>>>>>>>>>>          >>>> Would that be like a default "reset" command action ?
>>>>>>>>>>          >>>> This probably shouldn't be socfpga specific then.
>>>>>>>>>>          >>>
>>>>>>>>>>          >>> No, it's a thing that lives on and influences even the
>>>>>>>>>> soft
>>>>>>>>>>          reset issued
>>>>>>>>>>          >>> by linux "reboot" command. This is something *very*
>>>>>>>>>> socfpga
>>>>>>>>>>          specific.
>>>>>>>>>>          >>
>>>>>>>>>>          >> Hmmm, so isn't this a policy to be configured on the
>>>>>>>>>> Linux
>>>>>>>>>> end ?
>>>>>>>>>>          >
>>>>>>>>>>          > Might be, but it affects U-Boot's 'reset' command as
>>>>>>>>>> well. And
>>>>>>>>>> I guess
>>>>>>>>>>          > it's set up in U-Boot this early to ensure it always
>>>>>>>>>> works.
>>>>>>>>>>
>>>>>>>>>>          Drat, that's right. So there has to be some way to
>>>>>>>>>> agree on
>>>>>>>>>> how the
>>>>>>>>>>          reset works between the kernel and U-Boot ?
>>>>>>>>>>
>>>>>>>>>>          > If it were for me, we could drop writing this magic
>>>>>>>>>> altogether. I just
>>>>>>>>>>          > figured some boards might require it to be written
>>>>>>>>>> somewhere,
>>>>>>>>>> and came
>>>>>>>>>>          > up with a patch that might save those boards with the
>>>>>>>>>> way
>>>>>>>>>> it was
>>>>>>>>>>          before.
>>>>>>>>>>
>>>>>>>>>>          Isn't this magic actually used by bootrom ?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Right. It tells the boot rom to jump to ocram on next reboot
>>>>>>>>>> instead of
>>>>>>>>>> loading spl from qspi or mmc. But if that's required or not a good
>>>>>>>>>> idea
>>>>>>>>>> at all depends on many factors. Some of them board related, some
>>>>>>>>>> U-Boot
>>>>>>>>>> related and some Linux related (depending on the hardware and
>>>>>>>>>> drivers
>>>>>>>>>> used).
>>>>>>>>>
>>>>>>>>> Should that be runtime configurable then ?
>>>>>>>>
>>>>>>>> Since it might depend on Linux putting the qspi chip into a state
>>>>>>>> where
>>>>>>>> it's not accessible by the boot ROM. That might change without
>>>>>>>> rebuilding U-Boot.
>>>>>>>
>>>>>>> If Linux switches the chip into some weird mode the bootrom cannot
>>>>>>> cope
>>>>>>> with, it's a reset routing problem. This cannot be fixed in software.
>>>>>>
>>>>>> No, it cannot be fixed, but currently there's a workaround for those
>>>>>> boards and I thought it was worth to keep this workaround, even though
>>>>>> my own boards will be fixed and not require such a workaround in the
>>>>>> future :-)
>>>>>
>>>>> What's the workaround ?
>>>>
>>>> The workaround is what this patch is about: the Boot ROM just branches
>>>> off to SRAM where it expectes SPL to be still working.
>>>
>>> But you still cannot communicate with the SPI NOR from your SPL ?
>>
>> Well, in most "every day reboot" cases, you can. Just reset BAR or
>> 4-byte mode.
> 
> "In most" reads as "it's unreliable".
> 
>>>> SPL can then e.g. reset 4-byte mode or whatever to still communicate
>>>> with the device when Boot ROM can't.
>>>
>>> Unless, of course, the SPI NOR doesn't interpret the command as data and
>>> corrupts something in the flash itself.
>>
>> Right, in this case, you can't.
>>
>> Don't get me wrong, I'm not arguing for this to be totally right, of
>> course I'd rahter get the boards fixed.
>>
>> I'm just trying to find a way to keep this code in for people depending
>> on it. I know we have some broken boards that depend on it. I could live
>> with writing this magic in our private board code, but it's a bummer for
>> other people upgrading if we removed it...
> 
> Put it in a board file with BIG FAT WARNING that it's wrong ?

Ok, sorry for taking so long to reply. I hope you can still follow the 
discussion nevertheless.

I'll give up this patch and post v2 that just removes the "warm reboot 
from SRAM" thing. Then, users of boards that now fail to reboot write 
that magic constant in their board files. (And if enough mainline boards 
should be affected, we could still add a Kconfig option "broken reset" 
or something.)

Regards,
Simon