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

[Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>]

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@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@gmail.com
>>>>>>        <mailto:simon.k.r.goldschmidt@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.

SPL can then e.g. reset 4-byte mode or whatever to still communicate 
with the device when Boot ROM can't.

Of course the downside is that SRAM might be overwritten meanwhile, 
which is why it's a workaround only, not the best idea how to do things...

Regards,
Simon