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

[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 ?

-- 
Best regards,
Marek Vasut