Re: Usage of restart_handler in pwrseq_emmc

[Marek Szyprowski]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/