Messages in this thread |  | | Date | Mon, 08 Jun 2015 11:36:17 +0200 | From | Marek Szyprowski <> | Subject | Re: Usage of restart_handler in pwrseq_emmc |
| |
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
|  |