This is the final push to replace arm_pm_restart with the kernel restart handler. Finally drop arm_pm_restart after it is no longer used. The following changes since commit 541d8f4d59d79f5d37c8c726f723d42ff307db57: ---------------------------------------------------------------- Guenter Roeck (6): ARM: prima2: Register with kernel restart handler ARM: xen: Register with kernel restart handler ARM: PSCI: Register with kernel restart handler ARM: Register with kernel restart handler ARM64: Remove arm_pm_restart ARM: Remove arm_pm_restart arch/arm/include/asm/system_misc.h | 1 - arch/arm/kernel/reboot.c | 6 +----- arch/arm/kernel/setup.c | 20 ++++++++++++++++++-- arch/arm/mach-prima2/rstc.c | 11 +++++++++-- arch/arm/xen/enlighten.c | 13 +++++++++++-- arch/arm64/include/asm/system_misc.h | 2 -- arch/arm64/kernel/process.c | 7 +------ drivers/firmware/psci.c | 11 +++++++++-- 8 files changed, 49 insertions(+), 22 deletions(-)
This is the final push to replace arm_pm_restart with the kernel restart handler. Finally drop arm_pm_restart after it is no longer used. The following changes since commit 541d8f4d59d79f5d37c8c726f723d42ff307db57: ---------------------------------------------------------------- Guenter Roeck (6): ARM: prima2: Register with kernel restart handler ARM: xen: Register with kernel restart handler ARM: PSCI: Register with kernel restart handler ARM: Register with kernel restart handler ARM64: Remove arm_pm_restart ARM: Remove arm_pm_restart arch/arm/include/asm/system_misc.h | 1 - arch/arm/kernel/reboot.c | 6 +----- arch/arm/kernel/setup.c | 20 ++++++++++++++++++-- arch/arm/mach-prima2/rstc.c | 11 +++++++++-- arch/arm/xen/enlighten.c | 13 +++++++++++-- arch/arm64/include/asm/system_misc.h | 2 -- arch/arm64/kernel/process.c | 7 +------ drivers/firmware/psci.c | 11 +++++++++-- 8 files changed, 49 insertions(+), 22 deletions(-)
Register with kernel restart handler instead of setting arm_pm_restart directly. By doing this, the prima2 reset handler can be prioritized among other restart methods available on a particular board. Select a high priority of 192 since the original code overwrites the default arm restart handler. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm/mach-prima2/rstc.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-prima2/rstc.c b/arch/arm/mach-prima2/rstc.c index 7c251eb11d01..1639997c5b49 100644 --- a/arch/arm/mach-prima2/rstc.c +++ b/arch/arm/mach-prima2/rstc.c @@ -65,11 +65,18 @@ static struct reset_controller_dev sirfsoc_reset_controller = { #define SIRFSOC_SYS_RST_BIT BIT(31) -static void sirfsoc_restart(enum reboot_mode mode, const char *cmd) +static int sirfsoc_restart(struct notifier_block *nb, unsigned long action, + void *data) { writel(SIRFSOC_SYS_RST_BIT, sirfsoc_rstc_base); + return NOTIFY_DONE; } +static struct notifier_block sirfsoc_restart_nb = { + .notifier_call = sirfsoc_restart, + .priority = 192, +}; + static int sirfsoc_rstc_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; @@ -80,7 +87,7 @@ static int sirfsoc_rstc_probe(struct platform_device *pdev) } sirfsoc_reset_controller.of_node = np; - arm_pm_restart = sirfsoc_restart; + register_restart_handler(&sirfsoc_restart_nb); if (IS_ENABLED(CONFIG_RESET_CONTROLLER)) reset_controller_register(&sirfsoc_reset_controller); -- 2.5.0
Register with kernel restart handler instead of setting arm_pm_restart directly. By doing this, the prima2 reset handler can be prioritized among other restart methods available on a particular board. Select a high priority of 192 since the original code overwrites the default arm restart handler. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm/mach-prima2/rstc.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-prima2/rstc.c b/arch/arm/mach-prima2/rstc.c index 7c251eb11d01..1639997c5b49 100644 --- a/arch/arm/mach-prima2/rstc.c +++ b/arch/arm/mach-prima2/rstc.c @@ -65,11 +65,18 @@ static struct reset_controller_dev sirfsoc_reset_controller = { #define SIRFSOC_SYS_RST_BIT BIT(31) -static void sirfsoc_restart(enum reboot_mode mode, const char *cmd) +static int sirfsoc_restart(struct notifier_block *nb, unsigned long action, + void *data) { writel(SIRFSOC_SYS_RST_BIT, sirfsoc_rstc_base); + return NOTIFY_DONE; } +static struct notifier_block sirfsoc_restart_nb = { + .notifier_call = sirfsoc_restart, + .priority = 192, +}; + static int sirfsoc_rstc_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; @@ -80,7 +87,7 @@ static int sirfsoc_rstc_probe(struct platform_device *pdev) } sirfsoc_reset_controller.of_node = np; - arm_pm_restart = sirfsoc_restart; + register_restart_handler(&sirfsoc_restart_nb); if (IS_ENABLED(CONFIG_RESET_CONTROLLER)) reset_controller_register(&sirfsoc_reset_controller); -- 2.5.0
Register with kernel restart handler instead of setting arm_pm_restart directly. Select a high priority of 192 to ensure that default restart handlers are replaced if Xen is running. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm/xen/enlighten.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 75cd7345c654..76a1d12fd27e 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -27,6 +27,7 @@ #include <linux/cpu.h> #include <linux/console.h> #include <linux/pvclock_gtod.h> +#include <linux/reboot.h> #include <linux/time64.h> #include <linux/timekeeping.h> #include <linux/timekeeper_internal.h> @@ -193,14 +194,22 @@ after_register_vcpu_info: put_cpu(); } -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) +static int xen_restart(struct notifier_block *nb, unsigned long action, + void *data) { struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; int rc; rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); BUG_ON(rc); + + return NOTIFY_DONE; } +static struct notifier_block xen_restart_nb = { + .notifier_call = xen_restart, + .priority = 192, +}; + static void xen_power_off(void) { struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) return -ENODEV; pm_power_off = xen_power_off; - arm_pm_restart = xen_restart; + register_restart_handler(&xen_restart_nb); if (!xen_initial_domain()) { struct timespec64 ts; xen_read_wallclock(&ts); -- 2.5.0
Register with kernel restart handler instead of setting arm_pm_restart directly. Select a high priority of 192 to ensure that default restart handlers are replaced if Xen is running. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm/xen/enlighten.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 75cd7345c654..76a1d12fd27e 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -27,6 +27,7 @@ #include <linux/cpu.h> #include <linux/console.h> #include <linux/pvclock_gtod.h> +#include <linux/reboot.h> #include <linux/time64.h> #include <linux/timekeeping.h> #include <linux/timekeeper_internal.h> @@ -193,14 +194,22 @@ after_register_vcpu_info: put_cpu(); } -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) +static int xen_restart(struct notifier_block *nb, unsigned long action, + void *data) { struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; int rc; rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); BUG_ON(rc); + + return NOTIFY_DONE; } +static struct notifier_block xen_restart_nb = { + .notifier_call = xen_restart, + .priority = 192, +}; + static void xen_power_off(void) { struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) return -ENODEV; pm_power_off = xen_power_off; - arm_pm_restart = xen_restart; + register_restart_handler(&xen_restart_nb); if (!xen_initial_domain()) { struct timespec64 ts; xen_read_wallclock(&ts); -- 2.5.0
Register with kernel restart handler instead of setting arm_pm_restart directly. Select a high priority of 192 to ensure that default restart handlers are replaced if Xen is running. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm/xen/enlighten.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 75cd7345c654..76a1d12fd27e 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -27,6 +27,7 @@ #include <linux/cpu.h> #include <linux/console.h> #include <linux/pvclock_gtod.h> +#include <linux/reboot.h> #include <linux/time64.h> #include <linux/timekeeping.h> #include <linux/timekeeper_internal.h> @@ -193,14 +194,22 @@ after_register_vcpu_info: put_cpu(); } -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) +static int xen_restart(struct notifier_block *nb, unsigned long action, + void *data) { struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; int rc; rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); BUG_ON(rc); + + return NOTIFY_DONE; } +static struct notifier_block xen_restart_nb = { + .notifier_call = xen_restart, + .priority = 192, +}; + static void xen_power_off(void) { struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) return -ENODEV; pm_power_off = xen_power_off; - arm_pm_restart = xen_restart; + register_restart_handler(&xen_restart_nb); if (!xen_initial_domain()) { struct timespec64 ts; xen_read_wallclock(&ts); -- 2.5.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Register with kernel restart handler instead of setting arm_pm_restart directly. This enables support for replacing the PSCI restart handler with a different handler if necessary for a specific board. Select a priority of 129 to indicate a higher than default priority, but keep it as low as possible since PSCI reset is known to fail on some boards. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- It might make sense to introduce a restart-priority property for devicetree based configurations, but I am not sure if this would be acceptable. drivers/firmware/psci.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index 11bfee8b79a9..99fab3ac3fd5 100644 --- a/drivers/firmware/psci.c +++ b/drivers/firmware/psci.c @@ -231,11 +231,18 @@ static int get_set_conduit_method(struct device_node *np) return 0; } -static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd) +static int psci_sys_reset(struct notifier_block *np, unsigned long action, + void *data) { invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0); + return NOTIFY_DONE; } +static struct notifier_block psci_sys_reset_nb = { + .notifier_call = psci_sys_reset, + .priority = 129, +}; + static void psci_sys_poweroff(void) { invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); @@ -461,7 +468,7 @@ static void __init psci_0_2_set_functions(void) psci_ops.migrate_info_type = psci_migrate_info_type; - arm_pm_restart = psci_sys_reset; + register_restart_handler(&psci_sys_reset_nb); pm_power_off = psci_sys_poweroff; } -- 2.5.0
Register with kernel restart handler instead of setting arm_pm_restart directly. This enables support for replacing the PSCI restart handler with a different handler if necessary for a specific board. Select a priority of 129 to indicate a higher than default priority, but keep it as low as possible since PSCI reset is known to fail on some boards. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- It might make sense to introduce a restart-priority property for devicetree based configurations, but I am not sure if this would be acceptable. drivers/firmware/psci.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index 11bfee8b79a9..99fab3ac3fd5 100644 --- a/drivers/firmware/psci.c +++ b/drivers/firmware/psci.c @@ -231,11 +231,18 @@ static int get_set_conduit_method(struct device_node *np) return 0; } -static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd) +static int psci_sys_reset(struct notifier_block *np, unsigned long action, + void *data) { invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0); + return NOTIFY_DONE; } +static struct notifier_block psci_sys_reset_nb = { + .notifier_call = psci_sys_reset, + .priority = 129, +}; + static void psci_sys_poweroff(void) { invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); @@ -461,7 +468,7 @@ static void __init psci_0_2_set_functions(void) psci_ops.migrate_info_type = psci_migrate_info_type; - arm_pm_restart = psci_sys_reset; + register_restart_handler(&psci_sys_reset_nb); pm_power_off = psci_sys_poweroff; } -- 2.5.0
By making use of the kernel restart handler, board specific restart handlers can be prioritized amongst available mechanisms for a particular board or system. Select the default priority of 128 to indicate that the restart callback in the machine description is the default restart mechanism. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm/kernel/setup.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index 139791ed473d..232dba199702 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -1002,6 +1002,20 @@ void __init hyp_mode_check(void) #endif } +static void (*__arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); + +static int arm_restart(struct notifier_block *nb, unsigned long action, + void *data) +{ + __arm_pm_restart(action, data); + return NOTIFY_DONE; +} + +static struct notifier_block arm_restart_nb = { + .notifier_call = arm_restart, + .priority = 128, +}; + void __init setup_arch(char **cmdline_p) { const struct machine_desc *mdesc; @@ -1044,8 +1058,10 @@ void __init setup_arch(char **cmdline_p) paging_init(mdesc); request_standard_resources(mdesc); - if (mdesc->restart) - arm_pm_restart = mdesc->restart; + if (mdesc->restart) { + __arm_pm_restart = mdesc->restart; + register_restart_handler(&arm_restart_nb); + } unflatten_device_tree(); -- 2.5.0
By making use of the kernel restart handler, board specific restart handlers can be prioritized amongst available mechanisms for a particular board or system. Select the default priority of 128 to indicate that the restart callback in the machine description is the default restart mechanism. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm/kernel/setup.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index 139791ed473d..232dba199702 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -1002,6 +1002,20 @@ void __init hyp_mode_check(void) #endif } +static void (*__arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); + +static int arm_restart(struct notifier_block *nb, unsigned long action, + void *data) +{ + __arm_pm_restart(action, data); + return NOTIFY_DONE; +} + +static struct notifier_block arm_restart_nb = { + .notifier_call = arm_restart, + .priority = 128, +}; + void __init setup_arch(char **cmdline_p) { const struct machine_desc *mdesc; @@ -1044,8 +1058,10 @@ void __init setup_arch(char **cmdline_p) paging_init(mdesc); request_standard_resources(mdesc); - if (mdesc->restart) - arm_pm_restart = mdesc->restart; + if (mdesc->restart) { + __arm_pm_restart = mdesc->restart; + register_restart_handler(&arm_restart_nb); + } unflatten_device_tree(); -- 2.5.0
All users of arm_pm_restart have been converted to use the kernel restart handler. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm64/include/asm/system_misc.h | 2 -- arch/arm64/kernel/process.c | 7 +------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h index 57f110bea6a8..f1d865b7d38d 100644 --- a/arch/arm64/include/asm/system_misc.h +++ b/arch/arm64/include/asm/system_misc.h @@ -43,8 +43,6 @@ struct mm_struct; extern void show_pte(struct mm_struct *mm, unsigned long addr); extern void __show_regs(struct pt_regs *); -extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); - #define show_unhandled_signals_ratelimited() \ ({ \ static DEFINE_RATELIMIT_STATE(_rs, \ diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 80624829db61..29c29984eca0 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -66,8 +66,6 @@ EXPORT_SYMBOL(__stack_chk_guard); void (*pm_power_off)(void); EXPORT_SYMBOL_GPL(pm_power_off); -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); - /* * This is our default idle handler. */ @@ -153,10 +151,7 @@ void machine_restart(char *cmd) efi_reboot(reboot_mode, NULL); /* Now call the architecture specific reboot code. */ - if (arm_pm_restart) - arm_pm_restart(reboot_mode, cmd); - else - do_kernel_restart(cmd); + do_kernel_restart(cmd); /* * Whoops - the architecture was unable to reboot. -- 2.5.0
All users of arm_pm_restart have been converted to use the kernel restart handler. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm64/include/asm/system_misc.h | 2 -- arch/arm64/kernel/process.c | 7 +------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h index 57f110bea6a8..f1d865b7d38d 100644 --- a/arch/arm64/include/asm/system_misc.h +++ b/arch/arm64/include/asm/system_misc.h @@ -43,8 +43,6 @@ struct mm_struct; extern void show_pte(struct mm_struct *mm, unsigned long addr); extern void __show_regs(struct pt_regs *); -extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); - #define show_unhandled_signals_ratelimited() \ ({ \ static DEFINE_RATELIMIT_STATE(_rs, \ diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 80624829db61..29c29984eca0 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -66,8 +66,6 @@ EXPORT_SYMBOL(__stack_chk_guard); void (*pm_power_off)(void); EXPORT_SYMBOL_GPL(pm_power_off); -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); - /* * This is our default idle handler. */ @@ -153,10 +151,7 @@ void machine_restart(char *cmd) efi_reboot(reboot_mode, NULL); /* Now call the architecture specific reboot code. */ - if (arm_pm_restart) - arm_pm_restart(reboot_mode, cmd); - else - do_kernel_restart(cmd); + do_kernel_restart(cmd); /* * Whoops - the architecture was unable to reboot. -- 2.5.0
All users of arm_pm_restart have been converted to use the kernel restart handler. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm/include/asm/system_misc.h | 1 - arch/arm/kernel/reboot.c | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h index a3d61ad984af..6c952538f1e8 100644 --- a/arch/arm/include/asm/system_misc.h +++ b/arch/arm/include/asm/system_misc.h @@ -11,7 +11,6 @@ extern void cpu_init(void); void soft_restart(unsigned long); -extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); extern void (*arm_pm_idle)(void); #define UDBG_UNDEFINED (1 << 0) diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c index 71a2ff9ec490..4785c39ee3eb 100644 --- a/arch/arm/kernel/reboot.c +++ b/arch/arm/kernel/reboot.c @@ -20,7 +20,6 @@ typedef void (*phys_reset_t)(unsigned long); /* * Function pointers to optional machine specific functions */ -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); void (*pm_power_off)(void); EXPORT_SYMBOL(pm_power_off); @@ -140,10 +139,7 @@ void machine_restart(char *cmd) local_irq_disable(); smp_send_stop(); - if (arm_pm_restart) - arm_pm_restart(reboot_mode, cmd); - else - do_kernel_restart(cmd); + do_kernel_restart(cmd); /* Give a grace period for failure to restart of 1s */ mdelay(1000); -- 2.5.0
All users of arm_pm_restart have been converted to use the kernel restart handler. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- arch/arm/include/asm/system_misc.h | 1 - arch/arm/kernel/reboot.c | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h index a3d61ad984af..6c952538f1e8 100644 --- a/arch/arm/include/asm/system_misc.h +++ b/arch/arm/include/asm/system_misc.h @@ -11,7 +11,6 @@ extern void cpu_init(void); void soft_restart(unsigned long); -extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); extern void (*arm_pm_idle)(void); #define UDBG_UNDEFINED (1 << 0) diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c index 71a2ff9ec490..4785c39ee3eb 100644 --- a/arch/arm/kernel/reboot.c +++ b/arch/arm/kernel/reboot.c @@ -20,7 +20,6 @@ typedef void (*phys_reset_t)(unsigned long); /* * Function pointers to optional machine specific functions */ -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); void (*pm_power_off)(void); EXPORT_SYMBOL(pm_power_off); @@ -140,10 +139,7 @@ void machine_restart(char *cmd) local_irq_disable(); smp_send_stop(); - if (arm_pm_restart) - arm_pm_restart(reboot_mode, cmd); - else - do_kernel_restart(cmd); + do_kernel_restart(cmd); /* Give a grace period for failure to restart of 1s */ mdelay(1000); -- 2.5.0
On Fri, Apr 08, 2016 at 05:53:55AM -0700, Guenter Roeck wrote: > Register with kernel restart handler instead of setting arm_pm_restart > directly. > > Select a high priority of 192 to ensure that default restart handlers Is there some macro for that magic value? > are replaced if Xen is running. > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > arch/arm/xen/enlighten.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 75cd7345c654..76a1d12fd27e 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -27,6 +27,7 @@ > #include <linux/cpu.h> > #include <linux/console.h> > #include <linux/pvclock_gtod.h> > +#include <linux/reboot.h> > #include <linux/time64.h> > #include <linux/timekeeping.h> > #include <linux/timekeeper_internal.h> > @@ -193,14 +194,22 @@ after_register_vcpu_info: > put_cpu(); > } > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > +static int xen_restart(struct notifier_block *nb, unsigned long action, > + void *data) > { > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > int rc; > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > BUG_ON(rc); > + > + return NOTIFY_DONE; > } > > +static struct notifier_block xen_restart_nb = { > + .notifier_call = xen_restart, > + .priority = 192, > +}; > + > static void xen_power_off(void) > { > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > return -ENODEV; > > pm_power_off = xen_power_off; > - arm_pm_restart = xen_restart; > + register_restart_handler(&xen_restart_nb); > if (!xen_initial_domain()) { > struct timespec64 ts; > xen_read_wallclock(&ts); > -- > 2.5.0 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[-- Attachment #1: Type: text/plain, Size: 288 bytes --] On Fri, Apr 08, 2016 at 05:53:53AM -0700, Guenter Roeck wrote: > This is the final push to replace arm_pm_restart with the kernel restart > handler. Finally drop arm_pm_restart after it is no longer used. Nice! Will check it next week and add the reset_handler to my watchdog. Thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --]
On Fri, Apr 08, 2016 at 05:53:53AM -0700, Guenter Roeck wrote: > This is the final push to replace arm_pm_restart with the kernel restart > handler. Finally drop arm_pm_restart after it is no longer used. Nice! Will check it next week and add the reset_handler to my watchdog. Thanks! -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160408/92dd979c/attachment.sig>
On Fri, Apr 08, 2016 at 11:22:57AM -0400, Konrad Rzeszutek Wilk wrote: > On Fri, Apr 08, 2016 at 05:53:55AM -0700, Guenter Roeck wrote: > > Register with kernel restart handler instead of setting arm_pm_restart > > directly. > > > > Select a high priority of 192 to ensure that default restart handlers > > Is there some macro for that magic value? > No, only guidelines in kernel/reboot.c. Guenter > > are replaced if Xen is running. > > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > --- > > arch/arm/xen/enlighten.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index 75cd7345c654..76a1d12fd27e 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -27,6 +27,7 @@ > > #include <linux/cpu.h> > > #include <linux/console.h> > > #include <linux/pvclock_gtod.h> > > +#include <linux/reboot.h> > > #include <linux/time64.h> > > #include <linux/timekeeping.h> > > #include <linux/timekeeper_internal.h> > > @@ -193,14 +194,22 @@ after_register_vcpu_info: > > put_cpu(); > > } > > > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > > +static int xen_restart(struct notifier_block *nb, unsigned long action, > > + void *data) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > > int rc; > > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > > BUG_ON(rc); > > + > > + return NOTIFY_DONE; > > } > > > > +static struct notifier_block xen_restart_nb = { > > + .notifier_call = xen_restart, > > + .priority = 192, > > +}; > > + > > static void xen_power_off(void) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > > return -ENODEV; > > > > pm_power_off = xen_power_off; > > - arm_pm_restart = xen_restart; > > + register_restart_handler(&xen_restart_nb); > > if (!xen_initial_domain()) { > > struct timespec64 ts; > > xen_read_wallclock(&ts); > > -- > > 2.5.0 > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel
On Fri, Apr 08, 2016 at 11:22:57AM -0400, Konrad Rzeszutek Wilk wrote: > On Fri, Apr 08, 2016 at 05:53:55AM -0700, Guenter Roeck wrote: > > Register with kernel restart handler instead of setting arm_pm_restart > > directly. > > > > Select a high priority of 192 to ensure that default restart handlers > > Is there some macro for that magic value? > No, only guidelines in kernel/reboot.c. Guenter > > are replaced if Xen is running. > > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > --- > > arch/arm/xen/enlighten.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index 75cd7345c654..76a1d12fd27e 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -27,6 +27,7 @@ > > #include <linux/cpu.h> > > #include <linux/console.h> > > #include <linux/pvclock_gtod.h> > > +#include <linux/reboot.h> > > #include <linux/time64.h> > > #include <linux/timekeeping.h> > > #include <linux/timekeeper_internal.h> > > @@ -193,14 +194,22 @@ after_register_vcpu_info: > > put_cpu(); > > } > > > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > > +static int xen_restart(struct notifier_block *nb, unsigned long action, > > + void *data) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > > int rc; > > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > > BUG_ON(rc); > > + > > + return NOTIFY_DONE; > > } > > > > +static struct notifier_block xen_restart_nb = { > > + .notifier_call = xen_restart, > > + .priority = 192, > > +}; > > + > > static void xen_power_off(void) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > > return -ENODEV; > > > > pm_power_off = xen_power_off; > > - arm_pm_restart = xen_restart; > > + register_restart_handler(&xen_restart_nb); > > if (!xen_initial_domain()) { > > struct timespec64 ts; > > xen_read_wallclock(&ts); > > -- > > 2.5.0 > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel at lists.xen.org > > http://lists.xen.org/xen-devel
On Fri, Apr 08, 2016 at 11:22:57AM -0400, Konrad Rzeszutek Wilk wrote: > On Fri, Apr 08, 2016 at 05:53:55AM -0700, Guenter Roeck wrote: > > Register with kernel restart handler instead of setting arm_pm_restart > > directly. > > > > Select a high priority of 192 to ensure that default restart handlers > > Is there some macro for that magic value? > No, only guidelines in kernel/reboot.c. Guenter > > are replaced if Xen is running. > > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > --- > > arch/arm/xen/enlighten.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index 75cd7345c654..76a1d12fd27e 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -27,6 +27,7 @@ > > #include <linux/cpu.h> > > #include <linux/console.h> > > #include <linux/pvclock_gtod.h> > > +#include <linux/reboot.h> > > #include <linux/time64.h> > > #include <linux/timekeeping.h> > > #include <linux/timekeeper_internal.h> > > @@ -193,14 +194,22 @@ after_register_vcpu_info: > > put_cpu(); > > } > > > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > > +static int xen_restart(struct notifier_block *nb, unsigned long action, > > + void *data) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > > int rc; > > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > > BUG_ON(rc); > > + > > + return NOTIFY_DONE; > > } > > > > +static struct notifier_block xen_restart_nb = { > > + .notifier_call = xen_restart, > > + .priority = 192, > > +}; > > + > > static void xen_power_off(void) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > > return -ENODEV; > > > > pm_power_off = xen_power_off; > > - arm_pm_restart = xen_restart; > > + register_restart_handler(&xen_restart_nb); > > if (!xen_initial_domain()) { > > struct timespec64 ts; > > xen_read_wallclock(&ts); > > -- > > 2.5.0 > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Friday 08 April 2016, Guenter Roeck wrote:
> This is the final push to replace arm_pm_restart with the kernel restart
> handler. Finally drop arm_pm_restart after it is no longer used.
> The following changes since commit 541d8f4d59d79f5d37c8c726f723d42ff307db57:
Looks good to me,
Acked-by: Arnd Bergmann <arnd@arndb.de>
On Friday 08 April 2016, Guenter Roeck wrote:
> This is the final push to replace arm_pm_restart with the kernel restart
> handler. Finally drop arm_pm_restart after it is no longer used.
> The following changes since commit 541d8f4d59d79f5d37c8c726f723d42ff307db57:
Looks good to me,
Acked-by: Arnd Bergmann <arnd@arndb.de>
On Fri, 8 Apr 2016, Guenter Roeck wrote: > Register with kernel restart handler instead of setting arm_pm_restart > directly. > > Select a high priority of 192 to ensure that default restart handlers > are replaced if Xen is running. > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > arch/arm/xen/enlighten.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 75cd7345c654..76a1d12fd27e 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -27,6 +27,7 @@ > #include <linux/cpu.h> > #include <linux/console.h> > #include <linux/pvclock_gtod.h> > +#include <linux/reboot.h> > #include <linux/time64.h> > #include <linux/timekeeping.h> > #include <linux/timekeeper_internal.h> > @@ -193,14 +194,22 @@ after_register_vcpu_info: > put_cpu(); > } > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > +static int xen_restart(struct notifier_block *nb, unsigned long action, > + void *data) > { > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > int rc; > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > BUG_ON(rc); > + > + return NOTIFY_DONE; > } > > +static struct notifier_block xen_restart_nb = { > + .notifier_call = xen_restart, > + .priority = 192, > +}; > + > static void xen_power_off(void) > { > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > return -ENODEV; > > pm_power_off = xen_power_off; > - arm_pm_restart = xen_restart; > + register_restart_handler(&xen_restart_nb); > if (!xen_initial_domain()) { > struct timespec64 ts; > xen_read_wallclock(&ts); > -- > 2.5.0 >
On Fri, 8 Apr 2016, Guenter Roeck wrote: > Register with kernel restart handler instead of setting arm_pm_restart > directly. > > Select a high priority of 192 to ensure that default restart handlers > are replaced if Xen is running. > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > arch/arm/xen/enlighten.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 75cd7345c654..76a1d12fd27e 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -27,6 +27,7 @@ > #include <linux/cpu.h> > #include <linux/console.h> > #include <linux/pvclock_gtod.h> > +#include <linux/reboot.h> > #include <linux/time64.h> > #include <linux/timekeeping.h> > #include <linux/timekeeper_internal.h> > @@ -193,14 +194,22 @@ after_register_vcpu_info: > put_cpu(); > } > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > +static int xen_restart(struct notifier_block *nb, unsigned long action, > + void *data) > { > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > int rc; > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > BUG_ON(rc); > + > + return NOTIFY_DONE; > } > > +static struct notifier_block xen_restart_nb = { > + .notifier_call = xen_restart, > + .priority = 192, > +}; > + > static void xen_power_off(void) > { > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > return -ENODEV; > > pm_power_off = xen_power_off; > - arm_pm_restart = xen_restart; > + register_restart_handler(&xen_restart_nb); > if (!xen_initial_domain()) { > struct timespec64 ts; > xen_read_wallclock(&ts); > -- > 2.5.0 >
On Fri, 8 Apr 2016, Guenter Roeck wrote: > Register with kernel restart handler instead of setting arm_pm_restart > directly. > > Select a high priority of 192 to ensure that default restart handlers > are replaced if Xen is running. > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > arch/arm/xen/enlighten.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 75cd7345c654..76a1d12fd27e 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -27,6 +27,7 @@ > #include <linux/cpu.h> > #include <linux/console.h> > #include <linux/pvclock_gtod.h> > +#include <linux/reboot.h> > #include <linux/time64.h> > #include <linux/timekeeping.h> > #include <linux/timekeeper_internal.h> > @@ -193,14 +194,22 @@ after_register_vcpu_info: > put_cpu(); > } > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > +static int xen_restart(struct notifier_block *nb, unsigned long action, > + void *data) > { > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > int rc; > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > BUG_ON(rc); > + > + return NOTIFY_DONE; > } > > +static struct notifier_block xen_restart_nb = { > + .notifier_call = xen_restart, > + .priority = 192, > +}; > + > static void xen_power_off(void) > { > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > return -ENODEV; > > pm_power_off = xen_power_off; > - arm_pm_restart = xen_restart; > + register_restart_handler(&xen_restart_nb); > if (!xen_initial_domain()) { > struct timespec64 ts; > xen_read_wallclock(&ts); > -- > 2.5.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Sat, 9 Apr 2016, Stefano Stabellini wrote: > On Fri, 8 Apr 2016, Guenter Roeck wrote: > > Register with kernel restart handler instead of setting arm_pm_restart > > directly. > > > > Select a high priority of 192 to ensure that default restart handlers > > are replaced if Xen is running. > > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> and queued for 4.7 > > > arch/arm/xen/enlighten.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index 75cd7345c654..76a1d12fd27e 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -27,6 +27,7 @@ > > #include <linux/cpu.h> > > #include <linux/console.h> > > #include <linux/pvclock_gtod.h> > > +#include <linux/reboot.h> > > #include <linux/time64.h> > > #include <linux/timekeeping.h> > > #include <linux/timekeeper_internal.h> > > @@ -193,14 +194,22 @@ after_register_vcpu_info: > > put_cpu(); > > } > > > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > > +static int xen_restart(struct notifier_block *nb, unsigned long action, > > + void *data) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > > int rc; > > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > > BUG_ON(rc); > > + > > + return NOTIFY_DONE; > > } > > > > +static struct notifier_block xen_restart_nb = { > > + .notifier_call = xen_restart, > > + .priority = 192, > > +}; > > + > > static void xen_power_off(void) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > > return -ENODEV; > > > > pm_power_off = xen_power_off; > > - arm_pm_restart = xen_restart; > > + register_restart_handler(&xen_restart_nb); > > if (!xen_initial_domain()) { > > struct timespec64 ts; > > xen_read_wallclock(&ts); > > -- > > 2.5.0 > > >
On Sat, 9 Apr 2016, Stefano Stabellini wrote: > On Fri, 8 Apr 2016, Guenter Roeck wrote: > > Register with kernel restart handler instead of setting arm_pm_restart > > directly. > > > > Select a high priority of 192 to ensure that default restart handlers > > are replaced if Xen is running. > > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> and queued for 4.7 > > > arch/arm/xen/enlighten.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index 75cd7345c654..76a1d12fd27e 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -27,6 +27,7 @@ > > #include <linux/cpu.h> > > #include <linux/console.h> > > #include <linux/pvclock_gtod.h> > > +#include <linux/reboot.h> > > #include <linux/time64.h> > > #include <linux/timekeeping.h> > > #include <linux/timekeeper_internal.h> > > @@ -193,14 +194,22 @@ after_register_vcpu_info: > > put_cpu(); > > } > > > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > > +static int xen_restart(struct notifier_block *nb, unsigned long action, > > + void *data) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > > int rc; > > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > > BUG_ON(rc); > > + > > + return NOTIFY_DONE; > > } > > > > +static struct notifier_block xen_restart_nb = { > > + .notifier_call = xen_restart, > > + .priority = 192, > > +}; > > + > > static void xen_power_off(void) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > > return -ENODEV; > > > > pm_power_off = xen_power_off; > > - arm_pm_restart = xen_restart; > > + register_restart_handler(&xen_restart_nb); > > if (!xen_initial_domain()) { > > struct timespec64 ts; > > xen_read_wallclock(&ts); > > -- > > 2.5.0 > > >
On Sat, 9 Apr 2016, Stefano Stabellini wrote: > On Fri, 8 Apr 2016, Guenter Roeck wrote: > > Register with kernel restart handler instead of setting arm_pm_restart > > directly. > > > > Select a high priority of 192 to ensure that default restart handlers > > are replaced if Xen is running. > > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> and queued for 4.7 > > > arch/arm/xen/enlighten.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index 75cd7345c654..76a1d12fd27e 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -27,6 +27,7 @@ > > #include <linux/cpu.h> > > #include <linux/console.h> > > #include <linux/pvclock_gtod.h> > > +#include <linux/reboot.h> > > #include <linux/time64.h> > > #include <linux/timekeeping.h> > > #include <linux/timekeeper_internal.h> > > @@ -193,14 +194,22 @@ after_register_vcpu_info: > > put_cpu(); > > } > > > > -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd) > > +static int xen_restart(struct notifier_block *nb, unsigned long action, > > + void *data) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_reboot }; > > int rc; > > rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r); > > BUG_ON(rc); > > + > > + return NOTIFY_DONE; > > } > > > > +static struct notifier_block xen_restart_nb = { > > + .notifier_call = xen_restart, > > + .priority = 192, > > +}; > > + > > static void xen_power_off(void) > > { > > struct sched_shutdown r = { .reason = SHUTDOWN_poweroff }; > > @@ -370,7 +379,7 @@ static int __init xen_pm_init(void) > > return -ENODEV; > > > > pm_power_off = xen_power_off; > > - arm_pm_restart = xen_restart; > > + register_restart_handler(&xen_restart_nb); > > if (!xen_initial_domain()) { > > struct timespec64 ts; > > xen_read_wallclock(&ts); > > -- > > 2.5.0 > > > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Fri, Apr 08, 2016 at 05:53:58AM -0700, Guenter Roeck wrote:
> All users of arm_pm_restart have been converted to use the kernel restart
> handler.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> arch/arm64/include/asm/system_misc.h | 2 --
> arch/arm64/kernel/process.c | 7 +------
> 2 files changed, 1 insertion(+), 8 deletions(-)
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
On Fri, Apr 08, 2016 at 05:53:58AM -0700, Guenter Roeck wrote:
> All users of arm_pm_restart have been converted to use the kernel restart
> handler.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> arch/arm64/include/asm/system_misc.h | 2 --
> arch/arm64/kernel/process.c | 7 +------
> 2 files changed, 1 insertion(+), 8 deletions(-)
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
[-- Attachment #1: Type: text/plain, Size: 122 bytes --] > +static int psci_sys_reset(struct notifier_block *np, unsigned long action, Minor: notifier_block *nb instead of *np [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --]
> +static int psci_sys_reset(struct notifier_block *np, unsigned long action, Minor: notifier_block *nb instead of *np -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160412/0167d42a/attachment.sig>
[-- Attachment #1: Type: text/plain, Size: 1171 bytes --] On Fri, Apr 08, 2016 at 05:53:53AM -0700, Guenter Roeck wrote: > This is the final push to replace arm_pm_restart with the kernel restart > handler. Finally drop arm_pm_restart after it is no longer used. > The following changes since commit 541d8f4d59d79f5d37c8c726f723d42ff307db57: > > ---------------------------------------------------------------- > Guenter Roeck (6): > ARM: prima2: Register with kernel restart handler > ARM: xen: Register with kernel restart handler > ARM: PSCI: Register with kernel restart handler > ARM: Register with kernel restart handler > ARM64: Remove arm_pm_restart > ARM: Remove arm_pm_restart I double checked that all arm_pm_restart were converted, did a visual review of the patches (one nit found), and came to the same conclusions about the priority settings. Thus: Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> I also added a watchdog restart handler with higher priority than the PSCI handler. That now works nicely \o/ So, for patches 3+5 you could also add: Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Thanks, Wolfram [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --]
On Fri, Apr 08, 2016 at 05:53:53AM -0700, Guenter Roeck wrote: > This is the final push to replace arm_pm_restart with the kernel restart > handler. Finally drop arm_pm_restart after it is no longer used. > The following changes since commit 541d8f4d59d79f5d37c8c726f723d42ff307db57: > > ---------------------------------------------------------------- > Guenter Roeck (6): > ARM: prima2: Register with kernel restart handler > ARM: xen: Register with kernel restart handler > ARM: PSCI: Register with kernel restart handler > ARM: Register with kernel restart handler > ARM64: Remove arm_pm_restart > ARM: Remove arm_pm_restart I double checked that all arm_pm_restart were converted, did a visual review of the patches (one nit found), and came to the same conclusions about the priority settings. Thus: Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> I also added a watchdog restart handler with higher priority than the PSCI handler. That now works nicely \o/ So, for patches 3+5 you could also add: Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Thanks, Wolfram -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160412/c733d531/attachment.sig>
On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote: > Register with kernel restart handler instead of setting arm_pm_restart > directly. This enables support for replacing the PSCI restart handler > with a different handler if necessary for a specific board. > > Select a priority of 129 to indicate a higher than default priority, but > keep it as low as possible since PSCI reset is known to fail on some > boards. For reference, which boards? It's unfortunate that that a PSCI 0.2+ implementation would be lacking a working SYSTEM_RESET implementation, and it's certainly a mistake to discourage. > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > It might make sense to introduce a restart-priority property for devicetree > based configurations, but I am not sure if this would be acceptable. >From the DT side, I'm not keen on properties for priorities. They're incredibly fragile and don't really encode a HW property. A better option would be to have a property to describe how the PSCI implementation is broken (e.g. broken-system-reset), and not register the handler at all in that case. Thanks, Mark. > drivers/firmware/psci.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c > index 11bfee8b79a9..99fab3ac3fd5 100644 > --- a/drivers/firmware/psci.c > +++ b/drivers/firmware/psci.c > @@ -231,11 +231,18 @@ static int get_set_conduit_method(struct device_node *np) > return 0; > } > > -static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd) > +static int psci_sys_reset(struct notifier_block *np, unsigned long action, > + void *data) > { > invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0); > + return NOTIFY_DONE; > } > > +static struct notifier_block psci_sys_reset_nb = { > + .notifier_call = psci_sys_reset, > + .priority = 129, > +}; > + > static void psci_sys_poweroff(void) > { > invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); > @@ -461,7 +468,7 @@ static void __init psci_0_2_set_functions(void) > > psci_ops.migrate_info_type = psci_migrate_info_type; > > - arm_pm_restart = psci_sys_reset; > + register_restart_handler(&psci_sys_reset_nb); > > pm_power_off = psci_sys_poweroff; > } > -- > 2.5.0 >
On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote: > Register with kernel restart handler instead of setting arm_pm_restart > directly. This enables support for replacing the PSCI restart handler > with a different handler if necessary for a specific board. > > Select a priority of 129 to indicate a higher than default priority, but > keep it as low as possible since PSCI reset is known to fail on some > boards. For reference, which boards? It's unfortunate that that a PSCI 0.2+ implementation would be lacking a working SYSTEM_RESET implementation, and it's certainly a mistake to discourage. > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > It might make sense to introduce a restart-priority property for devicetree > based configurations, but I am not sure if this would be acceptable. >From the DT side, I'm not keen on properties for priorities. They're incredibly fragile and don't really encode a HW property. A better option would be to have a property to describe how the PSCI implementation is broken (e.g. broken-system-reset), and not register the handler at all in that case. Thanks, Mark. > drivers/firmware/psci.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c > index 11bfee8b79a9..99fab3ac3fd5 100644 > --- a/drivers/firmware/psci.c > +++ b/drivers/firmware/psci.c > @@ -231,11 +231,18 @@ static int get_set_conduit_method(struct device_node *np) > return 0; > } > > -static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd) > +static int psci_sys_reset(struct notifier_block *np, unsigned long action, > + void *data) > { > invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0); > + return NOTIFY_DONE; > } > > +static struct notifier_block psci_sys_reset_nb = { > + .notifier_call = psci_sys_reset, > + .priority = 129, > +}; > + > static void psci_sys_poweroff(void) > { > invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); > @@ -461,7 +468,7 @@ static void __init psci_0_2_set_functions(void) > > psci_ops.migrate_info_type = psci_migrate_info_type; > > - arm_pm_restart = psci_sys_reset; > + register_restart_handler(&psci_sys_reset_nb); > > pm_power_off = psci_sys_poweroff; > } > -- > 2.5.0 >
Dear Mark, On Wed, 13 Apr 2016 12:05:19 +0100 Mark Rutland wrote: > On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote: > > Register with kernel restart handler instead of setting arm_pm_restart > > directly. This enables support for replacing the PSCI restart handler > > with a different handler if necessary for a specific board. > > > > Select a priority of 129 to indicate a higher than default priority, but > > keep it as low as possible since PSCI reset is known to fail on some > > boards. > > For reference, which boards? > > It's unfortunate that that a PSCI 0.2+ implementation would be lacking a > working SYSTEM_RESET implementation, and it's certainly a mistake to > discourage. I may understand the case: on some platforms, the only reset way is to trigger the wdt, for various reason the underly firmware isn't convenient to touch the wdt. But I'd like 127 or lower for the default priority for the above case, because various wdt reset_handler priority is 128. Thanks, Jisheng > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > --- > > It might make sense to introduce a restart-priority property for devicetree > > based configurations, but I am not sure if this would be acceptable. > > From the DT side, I'm not keen on properties for priorities. They're > incredibly fragile and don't really encode a HW property. > > A better option would be to have a property to describe how the PSCI > implementation is broken (e.g. broken-system-reset), and not register > the handler at all in that case. > > Thanks, > Mark. > > > drivers/firmware/psci.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c > > index 11bfee8b79a9..99fab3ac3fd5 100644 > > --- a/drivers/firmware/psci.c > > +++ b/drivers/firmware/psci.c > > @@ -231,11 +231,18 @@ static int get_set_conduit_method(struct device_node *np) > > return 0; > > } > > > > -static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd) > > +static int psci_sys_reset(struct notifier_block *np, unsigned long action, > > + void *data) > > { > > invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0); > > + return NOTIFY_DONE; > > } > > > > +static struct notifier_block psci_sys_reset_nb = { > > + .notifier_call = psci_sys_reset, > > + .priority = 129, > > +}; > > + > > static void psci_sys_poweroff(void) > > { > > invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); > > @@ -461,7 +468,7 @@ static void __init psci_0_2_set_functions(void) > > > > psci_ops.migrate_info_type = psci_migrate_info_type; > > > > - arm_pm_restart = psci_sys_reset; > > + register_restart_handler(&psci_sys_reset_nb); > > > > pm_power_off = psci_sys_poweroff; > > } > > -- > > 2.5.0 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Dear Mark, On Wed, 13 Apr 2016 12:05:19 +0100 Mark Rutland wrote: > On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote: > > Register with kernel restart handler instead of setting arm_pm_restart > > directly. This enables support for replacing the PSCI restart handler > > with a different handler if necessary for a specific board. > > > > Select a priority of 129 to indicate a higher than default priority, but > > keep it as low as possible since PSCI reset is known to fail on some > > boards. > > For reference, which boards? > > It's unfortunate that that a PSCI 0.2+ implementation would be lacking a > working SYSTEM_RESET implementation, and it's certainly a mistake to > discourage. I may understand the case: on some platforms, the only reset way is to trigger the wdt, for various reason the underly firmware isn't convenient to touch the wdt. But I'd like 127 or lower for the default priority for the above case, because various wdt reset_handler priority is 128. Thanks, Jisheng > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > --- > > It might make sense to introduce a restart-priority property for devicetree > > based configurations, but I am not sure if this would be acceptable. > > From the DT side, I'm not keen on properties for priorities. They're > incredibly fragile and don't really encode a HW property. > > A better option would be to have a property to describe how the PSCI > implementation is broken (e.g. broken-system-reset), and not register > the handler at all in that case. > > Thanks, > Mark. > > > drivers/firmware/psci.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c > > index 11bfee8b79a9..99fab3ac3fd5 100644 > > --- a/drivers/firmware/psci.c > > +++ b/drivers/firmware/psci.c > > @@ -231,11 +231,18 @@ static int get_set_conduit_method(struct device_node *np) > > return 0; > > } > > > > -static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd) > > +static int psci_sys_reset(struct notifier_block *np, unsigned long action, > > + void *data) > > { > > invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0); > > + return NOTIFY_DONE; > > } > > > > +static struct notifier_block psci_sys_reset_nb = { > > + .notifier_call = psci_sys_reset, > > + .priority = 129, > > +}; > > + > > static void psci_sys_poweroff(void) > > { > > invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); > > @@ -461,7 +468,7 @@ static void __init psci_0_2_set_functions(void) > > > > psci_ops.migrate_info_type = psci_migrate_info_type; > > > > - arm_pm_restart = psci_sys_reset; > > + register_restart_handler(&psci_sys_reset_nb); > > > > pm_power_off = psci_sys_poweroff; > > } > > -- > > 2.5.0 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 04/13/2016 04:05 AM, Mark Rutland wrote: > On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote: >> Register with kernel restart handler instead of setting arm_pm_restart >> directly. This enables support for replacing the PSCI restart handler >> with a different handler if necessary for a specific board. >> >> Select a priority of 129 to indicate a higher than default priority, but >> keep it as low as possible since PSCI reset is known to fail on some >> boards. > > For reference, which boards? > Salvator-X, reported by Geert Uytterhoeven. Wolfram Sang also reported that it is broken on a board he is using, but I don't recall if it is the same board. > It's unfortunate that that a PSCI 0.2+ implementation would be lacking a > working SYSTEM_RESET implementation, and it's certainly a mistake to > discourage. > >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> It might make sense to introduce a restart-priority property for devicetree >> based configurations, but I am not sure if this would be acceptable. > >>From the DT side, I'm not keen on properties for priorities. They're > incredibly fragile and don't really encode a HW property. > Depends. It is a convenient means to say "primary restart method" or "may be broken". > A better option would be to have a property to describe how the PSCI > implementation is broken (e.g. broken-system-reset), and not register > the handler at all in that case. > ... just like this. I'll look into it. Thanks, Guenter > Thanks, > Mark. > >> drivers/firmware/psci.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c >> index 11bfee8b79a9..99fab3ac3fd5 100644 >> --- a/drivers/firmware/psci.c >> +++ b/drivers/firmware/psci.c >> @@ -231,11 +231,18 @@ static int get_set_conduit_method(struct device_node *np) >> return 0; >> } >> >> -static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd) >> +static int psci_sys_reset(struct notifier_block *np, unsigned long action, >> + void *data) >> { >> invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0); >> + return NOTIFY_DONE; >> } >> >> +static struct notifier_block psci_sys_reset_nb = { >> + .notifier_call = psci_sys_reset, >> + .priority = 129, >> +}; >> + >> static void psci_sys_poweroff(void) >> { >> invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); >> @@ -461,7 +468,7 @@ static void __init psci_0_2_set_functions(void) >> >> psci_ops.migrate_info_type = psci_migrate_info_type; >> >> - arm_pm_restart = psci_sys_reset; >> + register_restart_handler(&psci_sys_reset_nb); >> >> pm_power_off = psci_sys_poweroff; >> } >> -- >> 2.5.0 >> >
On 04/13/2016 04:05 AM, Mark Rutland wrote: > On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote: >> Register with kernel restart handler instead of setting arm_pm_restart >> directly. This enables support for replacing the PSCI restart handler >> with a different handler if necessary for a specific board. >> >> Select a priority of 129 to indicate a higher than default priority, but >> keep it as low as possible since PSCI reset is known to fail on some >> boards. > > For reference, which boards? > Salvator-X, reported by Geert Uytterhoeven. Wolfram Sang also reported that it is broken on a board he is using, but I don't recall if it is the same board. > It's unfortunate that that a PSCI 0.2+ implementation would be lacking a > working SYSTEM_RESET implementation, and it's certainly a mistake to > discourage. > >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> It might make sense to introduce a restart-priority property for devicetree >> based configurations, but I am not sure if this would be acceptable. > >>From the DT side, I'm not keen on properties for priorities. They're > incredibly fragile and don't really encode a HW property. > Depends. It is a convenient means to say "primary restart method" or "may be broken". > A better option would be to have a property to describe how the PSCI > implementation is broken (e.g. broken-system-reset), and not register > the handler at all in that case. > ... just like this. I'll look into it. Thanks, Guenter > Thanks, > Mark. > >> drivers/firmware/psci.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c >> index 11bfee8b79a9..99fab3ac3fd5 100644 >> --- a/drivers/firmware/psci.c >> +++ b/drivers/firmware/psci.c >> @@ -231,11 +231,18 @@ static int get_set_conduit_method(struct device_node *np) >> return 0; >> } >> >> -static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd) >> +static int psci_sys_reset(struct notifier_block *np, unsigned long action, >> + void *data) >> { >> invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0); >> + return NOTIFY_DONE; >> } >> >> +static struct notifier_block psci_sys_reset_nb = { >> + .notifier_call = psci_sys_reset, >> + .priority = 129, >> +}; >> + >> static void psci_sys_poweroff(void) >> { >> invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); >> @@ -461,7 +468,7 @@ static void __init psci_0_2_set_functions(void) >> >> psci_ops.migrate_info_type = psci_migrate_info_type; >> >> - arm_pm_restart = psci_sys_reset; >> + register_restart_handler(&psci_sys_reset_nb); >> >> pm_power_off = psci_sys_poweroff; >> } >> -- >> 2.5.0 >> >
On Wed, Apr 13, 2016 at 3:10 PM, Guenter Roeck <linux@roeck-us.net> wrote: > On 04/13/2016 04:05 AM, Mark Rutland wrote: >> On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote: >>> >>> Register with kernel restart handler instead of setting arm_pm_restart >>> directly. This enables support for replacing the PSCI restart handler >>> with a different handler if necessary for a specific board. >>> >>> Select a priority of 129 to indicate a higher than default priority, but >>> keep it as low as possible since PSCI reset is known to fail on some >>> boards. >> >> For reference, which boards? >> > Salvator-X, reported by Geert Uytterhoeven. Wolfram Sang also reported > that it is broken on a board he is using, but I don't recall if it is > the same board. Yes it is. >> It's unfortunate that that a PSCI 0.2+ implementation would be lacking a >> working SYSTEM_RESET implementation, and it's certainly a mistake to >> discourage. >> >>> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >>> --- >>> It might make sense to introduce a restart-priority property for >>> devicetree >>> based configurations, but I am not sure if this would be acceptable. >> >>> From the DT side, I'm not keen on properties for priorities. They're >> incredibly fragile and don't really encode a HW property. >> > Depends. It is a convenient means to say "primary restart method" or > "may be broken". The issue is supposed to be fixed in a more recent firmware, which I still have to try. DT indeed isn't the right place to work around this. What we need is a blacklist of bad firmware versions... Or Perfect Firmware from Day One on (like Perfect DT from Day One ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, Apr 13, 2016 at 3:10 PM, Guenter Roeck <linux@roeck-us.net> wrote: > On 04/13/2016 04:05 AM, Mark Rutland wrote: >> On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote: >>> >>> Register with kernel restart handler instead of setting arm_pm_restart >>> directly. This enables support for replacing the PSCI restart handler >>> with a different handler if necessary for a specific board. >>> >>> Select a priority of 129 to indicate a higher than default priority, but >>> keep it as low as possible since PSCI reset is known to fail on some >>> boards. >> >> For reference, which boards? >> > Salvator-X, reported by Geert Uytterhoeven. Wolfram Sang also reported > that it is broken on a board he is using, but I don't recall if it is > the same board. Yes it is. >> It's unfortunate that that a PSCI 0.2+ implementation would be lacking a >> working SYSTEM_RESET implementation, and it's certainly a mistake to >> discourage. >> >>> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >>> --- >>> It might make sense to introduce a restart-priority property for >>> devicetree >>> based configurations, but I am not sure if this would be acceptable. >> >>> From the DT side, I'm not keen on properties for priorities. They're >> incredibly fragile and don't really encode a HW property. >> > Depends. It is a convenient means to say "primary restart method" or > "may be broken". The issue is supposed to be fixed in a more recent firmware, which I still have to try. DT indeed isn't the right place to work around this. What we need is a blacklist of bad firmware versions... Or Perfect Firmware from Day One on (like Perfect DT from Day One ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, Apr 13, 2016 at 03:22:44PM +0200, Geert Uytterhoeven wrote:
> On Wed, Apr 13, 2016 at 3:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On 04/13/2016 04:05 AM, Mark Rutland wrote:
> >> On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote:
> >>>
> >>> Register with kernel restart handler instead of setting arm_pm_restart
> >>> directly. This enables support for replacing the PSCI restart handler
> >>> with a different handler if necessary for a specific board.
> >>>
> >>> Select a priority of 129 to indicate a higher than default priority, but
> >>> keep it as low as possible since PSCI reset is known to fail on some
> >>> boards.
> >>
> >> For reference, which boards?
> >>
> > Salvator-X, reported by Geert Uytterhoeven. Wolfram Sang also reported
> > that it is broken on a board he is using, but I don't recall if it is
> > the same board.
>
> Yes it is.
>
> >> It's unfortunate that that a PSCI 0.2+ implementation would be lacking a
> >> working SYSTEM_RESET implementation, and it's certainly a mistake to
> >> discourage.
> >>
> >>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >>> ---
> >>> It might make sense to introduce a restart-priority property for
> >>> devicetree
> >>> based configurations, but I am not sure if this would be acceptable.
> >>
> >>> From the DT side, I'm not keen on properties for priorities. They're
> >> incredibly fragile and don't really encode a HW property.
> >>
> > Depends. It is a convenient means to say "primary restart method" or
> > "may be broken".
>
> The issue is supposed to be fixed in a more recent firmware, which I still have
> to try.
>
> DT indeed isn't the right place to work around this. What we need is a
> blacklist of bad firmware versions...
> Or Perfect Firmware from Day One on (like Perfect DT from Day One ;-)
>
That makes things quite tricky. Best I can think of is a series of boolean
devicetree properties, such as
broken-reset-handler
last-resort-restart-handler
secondary-restart-handler
default-restart-handler
primary-restart-handler
which ends up being quite similar to the 'restart-priority' property. I'll
do this as follow-up patch, though - I do not see the point holding up the
series for this, and it is really a separate problem. I'll send rev2 with
the various Acked-by: and Reviewed-by: tags as well as the variable rename
suggested by Wolfram.
Thanks,
Guenter
On Wed, Apr 13, 2016 at 03:22:44PM +0200, Geert Uytterhoeven wrote:
> On Wed, Apr 13, 2016 at 3:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On 04/13/2016 04:05 AM, Mark Rutland wrote:
> >> On Fri, Apr 08, 2016 at 05:53:56AM -0700, Guenter Roeck wrote:
> >>>
> >>> Register with kernel restart handler instead of setting arm_pm_restart
> >>> directly. This enables support for replacing the PSCI restart handler
> >>> with a different handler if necessary for a specific board.
> >>>
> >>> Select a priority of 129 to indicate a higher than default priority, but
> >>> keep it as low as possible since PSCI reset is known to fail on some
> >>> boards.
> >>
> >> For reference, which boards?
> >>
> > Salvator-X, reported by Geert Uytterhoeven. Wolfram Sang also reported
> > that it is broken on a board he is using, but I don't recall if it is
> > the same board.
>
> Yes it is.
>
> >> It's unfortunate that that a PSCI 0.2+ implementation would be lacking a
> >> working SYSTEM_RESET implementation, and it's certainly a mistake to
> >> discourage.
> >>
> >>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >>> ---
> >>> It might make sense to introduce a restart-priority property for
> >>> devicetree
> >>> based configurations, but I am not sure if this would be acceptable.
> >>
> >>> From the DT side, I'm not keen on properties for priorities. They're
> >> incredibly fragile and don't really encode a HW property.
> >>
> > Depends. It is a convenient means to say "primary restart method" or
> > "may be broken".
>
> The issue is supposed to be fixed in a more recent firmware, which I still have
> to try.
>
> DT indeed isn't the right place to work around this. What we need is a
> blacklist of bad firmware versions...
> Or Perfect Firmware from Day One on (like Perfect DT from Day One ;-)
>
That makes things quite tricky. Best I can think of is a series of boolean
devicetree properties, such as
broken-reset-handler
last-resort-restart-handler
secondary-restart-handler
default-restart-handler
primary-restart-handler
which ends up being quite similar to the 'restart-priority' property. I'll
do this as follow-up patch, though - I do not see the point holding up the
series for this, and it is really a separate problem. I'll send rev2 with
the various Acked-by: and Reviewed-by: tags as well as the variable rename
suggested by Wolfram.
Thanks,
Guenter
[-- Attachment #1: Type: text/plain, Size: 1129 bytes --] > That makes things quite tricky. Best I can think of is a series of boolean > devicetree properties, such as > > broken-reset-handler > last-resort-restart-handler > secondary-restart-handler > default-restart-handler > primary-restart-handler > > which ends up being quite similar to the 'restart-priority' property. I'll > do this as follow-up patch, though Please CC me on this. I wanted to tackle this problem as well today. My findings/conclusions so far: * There is one driver bringing 'priority' directly to DT already: gpio-restart * Watchdog priorities are board dependant * Having the priorities clear at boot-time is safer than configuring them at run-time * The linux scheme (0-255) shouldn't be enforced in DT So, I wondered about a "priority" binding which just states "the higher, the more important". Then any OS can decide what to do with it. In the Linux case, this could be: sort them and give them priority 256 - position_in_sorted_list. Opinions? > - I do not see the point holding up the series for this, and it is > really a separate problem. Ack. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --]
> That makes things quite tricky. Best I can think of is a series of boolean > devicetree properties, such as > > broken-reset-handler > last-resort-restart-handler > secondary-restart-handler > default-restart-handler > primary-restart-handler > > which ends up being quite similar to the 'restart-priority' property. I'll > do this as follow-up patch, though Please CC me on this. I wanted to tackle this problem as well today. My findings/conclusions so far: * There is one driver bringing 'priority' directly to DT already: gpio-restart * Watchdog priorities are board dependant * Having the priorities clear at boot-time is safer than configuring them at run-time * The linux scheme (0-255) shouldn't be enforced in DT So, I wondered about a "priority" binding which just states "the higher, the more important". Then any OS can decide what to do with it. In the Linux case, this could be: sort them and give them priority 256 - position_in_sorted_list. Opinions? > - I do not see the point holding up the series for this, and it is > really a separate problem. Ack. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160414/86dea876/attachment-0001.sig>
On 04/14/2016 01:52 AM, Wolfram Sang wrote: > >> That makes things quite tricky. Best I can think of is a series of boolean >> devicetree properties, such as >> >> broken-reset-handler >> last-resort-restart-handler >> secondary-restart-handler >> default-restart-handler >> primary-restart-handler >> >> which ends up being quite similar to the 'restart-priority' property. I'll >> do this as follow-up patch, though > > Please CC me on this. I wanted to tackle this problem as well today. My Sure. > findings/conclusions so far: > > * There is one driver bringing 'priority' directly to DT already: gpio-restart > Correct. > * Watchdog priorities are board dependant > > * Having the priorities clear at boot-time is safer than configuring them > at run-time > Correct. > * The linux scheme (0-255) shouldn't be enforced in DT > > So, I wondered about a "priority" binding which just states "the higher, > the more important". Then any OS can decide what to do with it. In the > Linux case, this could be: sort them and give them priority 256 - > position_in_sorted_list. > "the higher, the more important" makes sense to me. We don't have to enforce the linux scheme, though that happens to be the same (the priority argument in the notifier block takes an int, so it would not even be necessary to adjust it unless someone specifies 0xffffffff). > Opinions? > I am fine either way - boolean properties or numbers, with a personal preference for numbers as more flexible. Whatever is acceptable for the community is fine with me. Guenter >> - I do not see the point holding up the series for this, and it is >> really a separate problem. > > Ack. >
On 04/14/2016 01:52 AM, Wolfram Sang wrote: > >> That makes things quite tricky. Best I can think of is a series of boolean >> devicetree properties, such as >> >> broken-reset-handler >> last-resort-restart-handler >> secondary-restart-handler >> default-restart-handler >> primary-restart-handler >> >> which ends up being quite similar to the 'restart-priority' property. I'll >> do this as follow-up patch, though > > Please CC me on this. I wanted to tackle this problem as well today. My Sure. > findings/conclusions so far: > > * There is one driver bringing 'priority' directly to DT already: gpio-restart > Correct. > * Watchdog priorities are board dependant > > * Having the priorities clear at boot-time is safer than configuring them > at run-time > Correct. > * The linux scheme (0-255) shouldn't be enforced in DT > > So, I wondered about a "priority" binding which just states "the higher, > the more important". Then any OS can decide what to do with it. In the > Linux case, this could be: sort them and give them priority 256 - > position_in_sorted_list. > "the higher, the more important" makes sense to me. We don't have to enforce the linux scheme, though that happens to be the same (the priority argument in the notifier block takes an int, so it would not even be necessary to adjust it unless someone specifies 0xffffffff). > Opinions? > I am fine either way - boolean properties or numbers, with a personal preference for numbers as more flexible. Whatever is acceptable for the community is fine with me. Guenter >> - I do not see the point holding up the series for this, and it is >> really a separate problem. > > Ack. >
[-- Attachment #1: Type: text/plain, Size: 767 bytes --] > "the higher, the more important" makes sense to me. We don't have to > enforce the linux scheme, though that happens to be the same (the priority > argument in the notifier block takes an int, so it would not even be > necessary to adjust it unless someone specifies 0xffffffff). I think we should enforce the scheme internally (but not in DT, of course): a) it is documented to be in the range 0-255 b) it should be valid to prioritize the watchdogs with 1,2,3 in DT. If we don't apply the '255 - pos_in_sorted_list' value, then the priority could be below some machine default of 128, or? > I am fine either way - boolean properties or numbers, with a personal > preference for numbers as more flexible. Same here. Time to go to the DT list probably. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --]
> "the higher, the more important" makes sense to me. We don't have to > enforce the linux scheme, though that happens to be the same (the priority > argument in the notifier block takes an int, so it would not even be > necessary to adjust it unless someone specifies 0xffffffff). I think we should enforce the scheme internally (but not in DT, of course): a) it is documented to be in the range 0-255 b) it should be valid to prioritize the watchdogs with 1,2,3 in DT. If we don't apply the '255 - pos_in_sorted_list' value, then the priority could be below some machine default of 128, or? > I am fine either way - boolean properties or numbers, with a personal > preference for numbers as more flexible. Same here. Time to go to the DT list probably. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160414/bdba3a4d/attachment-0001.sig>