[U-Boot] [PATCH] imx: don't clobber reset cause

[Eric Nelson]

Hi Stefano,

On 02/11/2015 02:07 AM, Stefano Babic wrote:
> Hi Eric,
> 
> On 10/02/2015 18:19, Eric Nelson wrote:
>> Hi Stefano,
>>
>> On 02/05/2015 11:49 AM, Stefano Babic wrote:
>>> Hi Eric,
>>>
>>> On 05/02/2015 19:22, Eric Nelson wrote:
>>>
>>>>
>>>> Certainly, but it seems wrong to make a decision about where and how
>>>> this might get passed to an O/S in code.
>>>>
>>>> If we want to generalize it, I'd be inclined to add commands to
>>>> query (into a variable) and clear the reset cause.
>>>>
>>>> That would still require this patch though.
>>>
>>> I do not think there should be a command. The cause must be directly
>>> associated to the variable, and the reset cause cleared.
>>>
>>
>> I posted a couple of additional options and received no comment
>> from you.
>>
> 
> Sorry for that.
> 
No worries, and thanks for the feedback.

>> Neither of them works as-is because of the ordering of events
>> (print_cpuinfo() is called before restoring the environment),
>> so your suggestion would require an additional call at startup
>> which currently doesn't exist across i.MX boards.
> 
> Right, I know. For that reason I suggested to you to save the result
> somewhere but not into a variable, at least not at the time
> print_cpuinfo() is called.
> 
> I think we can split the issue into two parts:
> 
> - detecting the reset reason. It makes absolutely sense to check the
> reason as soon as possible to avoid to miss an event, and resetting the
> cause is also a must. This is what current code does, and it is correct
> that the reason is read by the bootloader and not by the OS. In this
> way, we do not miss events and we know the last reason.
> 

Saving the value is the easy part.

> - we need to propagate this information to the OS in a way the OS can
> understand. Anyway, this does not mean that the OS must reread from the
> srsr register.
> 
> I admit, I do not know the interface with WinCE - I cannot help a lot
> for that. But assume we can have something similar we have with Linux.
> If we want to go on with a U-Boot variable, it is not required that the
> variable is assigned at the moment the reason is read. I think there are
> some other example in U-Boot (maybe "dieid#" for TI soc ?), where the
> variable is assigned later.
> 
Thanks for the pointer. The use of the dieid# variable shows what
I'd like to avoid though. There are 17 different calls to read this
value through the dieid_num_r() routine in various board files.

I'll look again to see if there's some other common spot that
can be used to read a saved value into an environment variable
for i.MX5x and i.MX6 CPUs.

> I do not think that resetting the flags in arch_preboot_os() is a good
> idea. This is a hack, and passing parameters has nothing to do with
> turning off peripherals.
> 

Agreed.

>>
>> The primary argument against the original patch was that
>> bits **could** accumulate. In practice, I believe this to
>> be a bit of a red herring, since two bits cover essentially
>> all of the normal conditions:
>>
>> 	bit 0 	- power on
>> 	bit 4	- watchdog
>>
> 
> Let's say that my board has an issue (maybe hardware, maybe not..) and
> after a first reset, the board resets twice. It can be a problem with
> power supply, can be something different. First time bits are on, and if
> I do not clear the bits, I cannot know the reson when it happens again.
> 

The bits are cleared now, so you won't know unless you're watching
the console.

There's also data loss when converting from the register value
to string (priority discussed below).

>> The watchdog flag is set with reboot under Linux and reset
>> in U-Boot, so we could re-work the switch statement to do
>> the right thing.
> 
> I slightly disagree. You are perfectly right when everything works as it
> should work.
> 
>> In fact, it appears broken now because it
>> has 0x11 displaying "POR", when I believe that should be
>> "WDOG".
> 
> I do not know now, but of course reset reason have priorities. If "POR"
> is set, it has advantage on "WDOG". But if after a WDOG the POR bit is
> set, it is another issue, not related to this one.
> 

I think there's an errata for i.MX6 when booting to SD or eMMC which
may cause the ROM boot loader to reset via watchdog.

>>
>> Other bits could conceivably accumulate, but I don't see
>> the value of worrying about cases like "JTAG_RESET".
>>
>> The reason we're pursuing this at all is because we'd like
>> to know the difference between a restart caused by power
>> interruption and a system lockup, and we'd like to do
>> this under Linux or Windows Embedded.
> 
> Agree, but I do not think we reach the goal simply clearing the bits and
> hoping to have always the good case. Multiple reset in U-Boot (and I see
> a lot of them...) are then hidden (not the reset, but the cause, of
> course !).
> 
> I understand the goal, we have to find a suitable ways to pass the
> information to the OS, that is.
> 

Again, thanks for the feedback.

Regards,


Eric