Messages in this thread | | | Date | Wed, 30 Jul 2014 07:10:50 -0700 | From | Guenter Roeck <> | Subject | Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain |
| |
On 07/30/2014 05:16 AM, Heiko Stübner wrote: > Hi Guenter, > > Am Dienstag, 29. Juli 2014, 18:50:47 schrieb Guenter Roeck: >> On 07/18/2014 12:34 AM, 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. Several other restart drivers for >>> arm, all directly calling arm_pm_restart, are in the process of being >>> integrated into the kernel. All those drivers would benefit from the new >>> API. >>> >>> 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 multiple 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. >>> >>> Introduce a system restart handler call chain to solve the described >>> problems. This call chain is expected to be executed 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 call chain. By using the >>> priority field in the notifier block, callers can control restart handler >>> execution sequence and thus ensure that the restart handler with the >>> optimal restart capabilities for a given system is called first. >>> >>> Since the first revision of this patchset, a number of separate patch >>> submissions have been made which either depend on it or could make use of >>> it. >>> >>> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html >>> >>> registers three notifiers. >>> >>> https://lkml.org/lkml/2014/7/8/962 >>> >>> would benefit from it. >>> >>> Patch 1 of this series implements the restart handler function. Patches 2 >>> and 3 implement calling the restart handler chain from arm and arm64 >>> restart code. >>> >>> Patch 4 modifies the restart-poweroff driver to no longer call >>> arm_pm_restart directly but machine_restart. This is done to avoid >>> calling arm_pm_restart from more than one place. The change makes the >>> driver architecture independent, so it would be possible to drop the arm >>> dependency from its Kconfig entry. >>> >>> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem >>> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure >>> that no one gets the idea to implement a restart handler as module. >>> >>> --- >>> v5: Rebased series to v3.16-rc5 >>> >>> Function renames: >>> register_restart_notifier -> register_restart_handler >>> unregister_restart_notifier -> unregister_restart_handler >>> kernel_restart_notify -> do_kernel_restart >>> >>> v4: Document restart notifier priorities >>> >>> Select 128 as default priority for newly introduced notifiers >>> Fix checkpatch warning (line too long) in moxart patch >>> >>> v3: Drop RFC. >>> >>> Add kernel_restart_notify wrapper function to execute notifier >>> Improve documentation. >>> Move restart_notifier_list into kernel/reboot.c and make it static. >>> >>> v2: Add patch 4. >>> >>> Only call blocking notifier call chain if arm_pm_restart was not set. >>> >>> -- >> >> To get more test coverage, this series plus a few add-on patches which >> depend on it are now available in the restart-staging branch of my >> repository at kernel.org [1]. The branch is currently based on 3.16-rc7. >> Extensive build test results are available at [2]; look for the column >> marked 'restart-staging' on the far right of the tables. >> >> I would encourage everyone interested in this series to send me Reviewed-by: >> or at least Acked-by: tags. Note that I removed all of the earlier tags >> since I feel that the changes made subsequently warrant updated tags. An >> Acked-by: from affected maintainers would also be very helpful. > > Thanks for adapting my Samsung restart-patches to the API changes. > > The one thing I found is, in > "clk: samsung: register restart handlers for s3c2412 and s3c2443" > you seem to have forgotten the priority in the clk-s3c2443.c part while in > clk-s3c2412.c it is present. > Yes, you are correct. I fixed that after I got the message from Fengguang's robot. Which means that my auto-builder misses to build that file. Too bad that arm:allmodconfig fails with other errors and is unusable. Do you know if there is a defconfig which builds clk-s3c2443.c ?
> And I'm not sure if there shouldn't be some sort of delay, to give the > watchdog some time to work, as Tomasz suggested in my initial submission? > In your case the reset handler is in the clock code, itn's it ? Question would be if the write to SWRST results in an immediate reset or if it needs a delay. From the context, it looks to me as if reaction would be immediate, but obviously I don't have the specification so that is just a wild guess.
> > I'll update the Rockchip specific restart-patches after the merge window, when > the core clock support has landed. > > And I still like the whole concept very much, so > Acked-by: Heiko Stuebner <heiko@sntech.de>
Thanks!
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/
| |