Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757868AbaGANpa (ORCPT ); Tue, 1 Jul 2014 09:45:30 -0400 Received: from mail.active-venture.com ([67.228.131.205]:59675 "EHLO mail.active-venture.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753256AbaGANp2 (ORCPT ); Tue, 1 Jul 2014 09:45:28 -0400 X-Originating-IP: 108.223.40.66 Message-ID: <53B2BB74.7050300@roeck-us.net> Date: Tue, 01 Jul 2014 06:45:24 -0700 From: Guenter Roeck User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Catalin Marinas CC: Russell King - ARM Linux , "linux-watchdog@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Wim Van Sebroeck , Maxime Ripard , Will Deacon , Arnd Bergmann , Heiko Stuebner , Jonas Jensen , Randy Dunlap , Andrew Morton , Steven Rostedt , Ingo Molnar , "linux-doc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Dmitry Eremin-Solenikov , David Woodhouse Subject: Re: [RFC PATCH 0/6] kernel: Add support for restart notifier call chain References: <1404155499-21177-1-git-send-email-linux@roeck-us.net> <20140630195946.GI32514@n2100.arm.linux.org.uk> <20140630202851.GA12888@roeck-us.net> <20140701112413.GQ18313@arm.com> In-Reply-To: <20140701112413.GQ18313@arm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/01/2014 04:24 AM, Catalin Marinas wrote: > On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote: >> On Mon, Jun 30, 2014 at 08:59:47PM +0100, Russell King - ARM Linux wrote: >>> On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote: >>>> Various drivers implement architecture and/or device specific means >>>> to restart (reset) the system. Various mechanisms have been implemented >>>> to support those schemes. The best known mechanism is arm_pm_restart, >>>> which is a function pointer to be set either from platform specific code >>>> or from drivers. Another mechanism is to use hardware watchdogs to issue >>>> a reset; this mechanism is used if there is no other method available >>>> to reset a board or system. Two examples are alim7101_wdt, which currently >>>> uses the reboot notifier to trigger a reset, and moxart_wdt, which registers >>>> the arm_pm_restart function. >>>> >>>> The existing mechanisms have a number of drawbacks. Typically only one scheme >>>> to restart the system is supported (at least if arm_pm_restart is used). >>>> At least in theory there can be mutliple means to restart the system, some of >>>> which may be less desirable (for example one mechanism may only reset the CPU, >>>> while another may reset the entire system). Using arm_pm_restart can also be >>>> racy if the function pointer is set from a driver, as the driver may be in >>>> the process of being unloaded when arm_pm_restart is called. >>>> Using the reboot notifier is always racy, as it is unknown if and when >>>> other functions using the reboot notifier have completed execution >>>> by the time the watchdog fires. >>>> >>>> To solve the problem, introduce a system restart notifier. This notifier >>>> is expected to be called from the architecture specific machine_restart() >>>> function. Drivers providing system restart functionality (such as the watchdog >>>> drivers mentioned above) are expected to register with this notifier. >>>> >>>> Patch 1 of this series implements the notifier function. Patches 2 and 3 >>>> implement calling the notifier chain from arm and arm64 restart code. >>>> Patch 4 and 5 convert existing restart handlers in the watchdog subsystem >>>> to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure >>>> that no one gets the idea to implement a restart handler as module. >>> >>> I think you need to restructure stuff somewhat, because I think >>> you've missed drivers/power/reset/ entirely, or at least you've >>> missed drivers/power/reset/restart-poweroff.c which calls >>> arm_pm_restart directly. I'm not quite sure how we ended up with >>> that... >> >> Yes, guess I missed (and did not really expect) that arm_pm_restart >> is called from multiple places. > > Most of the ARM-specific code in drivers/power/reset/ consists of SoC > power-off/restart back-ends (e.g. vexpress-poweroff.c). Since there is > no generic pm_restart, we continued to use arm_pm_restart (also for > arm64 since we share some of the drivers). Maybe some driver model here > would help. > >> What is restart-poweroff supposed to do in the first place, and why >> doesn't it call machine_restart() ? If it is what I think it is, ie >> a fallback for pm_power_off, it could be made generic and does not >> really have to depend on ARM. > > I think this one pretends to do a power-off via restart. It could call > machine_restart() but this only passes the default reboot_mode to > arm_pm_restart(). > Unless one sets reboot_mode first. See my proposed patch in https://lkml.org/lkml/2014/6/30/792. Guenter -- 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/