MIME-version: 1.0
Content-type: text/plain; charset=utf-8; format=flowed
Content-transfer-encoding: 8BIT
Message-id: <55756211.1050109@samsung.com>
Date: Mon, 08 Jun 2015 11:36:17 +0200
From: Marek Szyprowski <m.szyprowski@samsung.com>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101
 Thunderbird/31.7.0
To: Guenter Roeck <linux@roeck-us.net>,
        =?UTF-8?B?SGVpa28gU3TDvGJuZXI=?= <heiko@sntech.de>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Alexandre Courbot <acourbot@nvidia.com>
Cc: linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: Usage of restart_handler in pwrseq_emmc
References: <1789396.sexGZzDeEb@diego> <556ED06B.9030601@samsung.com>
 <556F1758.4030207@roeck-us.net>
In-reply-to: <556F1758.4030207@roeck-us.net>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3289
Lines: 94

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/