Re: Usage of restart_handler in pwrseq_emmc

Marek Szyprowski
2015-06-08T09:36:31Z

Hello,

On 2015-06-03 17:03, Guenter Roeck wrote:
> On 06/03/2015 03:01 AM, Marek Szyprowski wrote:
>> Hello,
>>
>> On 2015-06-02 17:29, Heiko Stübner wrote:
>>> I'm confused by the pwrseq-emmc registering a restart_handler for
>>> resetting an
>>> emmc in a panic-reboot case at priority 129 to "schedules it just
>>> before
>>> system reboot".
>>>
>>> >From what I remember from the restart-handler discussion the
>>> actuall usage is
>>> traversing the ordered list until one registered handler sucessfully
>>> restarts
>>> the system and not to have arbitary actions in there not related to
>>> the actual
>>> restart process?
>>>
>>> The actual documentation in kernel/reboot.c supports this assumption,
>>> describing register_restart_handler as "Register function to be
>>> called to
>>> reset the system".
>>>
>>>
>>> Additionally, 128 isn't even _the_ priority to reboot the system as
>>> described
>>> above and some drivers use higher priorities per default, see in
>>> drivers/power/reset arm-versatile-reboot.c; at91-reset.c;
>>> rmobile-reset.c and
>>> some more.
>>>
>>>
>>> So I guess this should use some other mechanism (reboot notifier)
>>> instead of
>>> restart_handlers?
>>
>> The first problem with reboot notifiers is that they are called too
>> early - before
>> device_shutdown(), what interferes with the code in mmc_bus_shutdown
>> and causes
>> lockup. The second problem is
>> that reboot notifiers are not called from emergency_restart() path. I
>> agree that
>> 129 value for priority might not be the best, maybe according to
>> documentation,
>> 255 value should be used to ensure that the handler will be called
>> first before
>> any real restart handler.
>>
>
> There is no non-real restart handler, and the documentation does not
> say anything
> about "called first before any real restart handler". Even with a
> priority of 255
> you would have no guarantee that your handler is called. Restart
> handlers are
> supposed to restart the system, nothing else. Actually, you have no
> guarantee
> that the restart handler is called in the first place - not all
> architectures
> support it (currently only arm, arm64, and mips do). Presumably mmc
> support is
> not limited to those architectures.
>
>> If you have any idea how to avoid restart handler and ensure proper
>> eMMC card
>> reboot sequence on any system reboot, I'm open for suggestions.
>>
>
> Why not execute the device-specific restart in the shutdown function ?
> You could register a reboot notifier to mark that a reboot is happening,
> and then execute the restart at the end of mmc_bus_shutdown.

Okay, this will solve one issue with reboot notifier, but there is still
a problem
with emergency_restart(). Do you think that it will be okay to add a call to
restart_notifiers (for example with some higher priority) also for
emergency case?
If so, I can rework my emmc pwr seq driver to use it and propose a patch for
emergency restart code.

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland