[PATCH 0/6] ARM/ARM64: Drop arm_pm_restart

[PATCH 0/6] ARM/ARM64: Drop arm_pm_restart

[Guenter Roeck]

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(-)

[PATCH 0/6] ARM/ARM64: Drop arm_pm_restart

[Guenter Roeck]

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(-)

[PATCH 1/6] ARM: prima2: Register with kernel restart handler

[Guenter Roeck]

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

[PATCH 1/6] ARM: prima2: Register with kernel restart handler

[Guenter Roeck]

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

[PATCH 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

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

[PATCH 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

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

[PATCH 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

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

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Konrad Rzeszutek Wilk]

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

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

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

Re: [Xen-devel] [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

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] [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

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

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Stefano Stabellini]

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
> 

[PATCH 2/6] ARM: xen: Register with kernel restart handler

[Stefano Stabellini]

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
> 

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Stefano Stabellini]

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
> > 
> 

[PATCH 2/6] ARM: xen: Register with kernel restart handler

[Stefano Stabellini]

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
> > 
> 

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Stefano Stabellini]

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

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Stefano Stabellini]

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

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Guenter Roeck]

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

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Guenter Roeck]

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

Re: [PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Wolfram Sang]

[-- 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 --]

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Wolfram Sang]

> +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>

Re: [PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Mark Rutland]

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
> 

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Mark Rutland]

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
> 

Re: [PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Jisheng Zhang]

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

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Jisheng Zhang]

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

Re: [PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Guenter Roeck]

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
>>
>

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Guenter Roeck]

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
>>
>

Re: [PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Geert Uytterhoeven]

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

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Geert Uytterhoeven]

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

Re: [PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Guenter Roeck]

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

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Guenter Roeck]

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

Re: [PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Wolfram Sang]

[-- 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 --]

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Wolfram Sang]

> 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>

Re: [PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Guenter Roeck]

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.
>

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Guenter Roeck]

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.
>

Re: [PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Wolfram Sang]

[-- 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 --]

[PATCH 3/6] ARM: PSCI: Register with kernel restart handler

[Wolfram Sang]

> "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>

[PATCH 4/6] ARM: Register with kernel restart handler

[Guenter Roeck]

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

[PATCH 4/6] ARM: Register with kernel restart handler

[Guenter Roeck]

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

[PATCH 5/6] ARM64: Remove arm_pm_restart

[Guenter Roeck]

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

[PATCH 5/6] ARM64: Remove arm_pm_restart

[Guenter Roeck]

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

Re: [PATCH 5/6] ARM64: Remove arm_pm_restart

[Catalin Marinas]

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>

[PATCH 5/6] ARM64: Remove arm_pm_restart

[Catalin Marinas]

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>

[PATCH 6/6] ARM: Remove arm_pm_restart

[Guenter Roeck]

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

[PATCH 6/6] ARM: Remove arm_pm_restart

[Guenter Roeck]

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

Re: [PATCH 0/6] ARM/ARM64: Drop arm_pm_restart

[Wolfram Sang]

[-- 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 --]

[PATCH 0/6] ARM/ARM64: Drop arm_pm_restart

[Wolfram Sang]

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>

Re: [PATCH 0/6] ARM/ARM64: Drop arm_pm_restart

[Arnd Bergmann]

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>

[PATCH 0/6] ARM/ARM64: Drop arm_pm_restart

[Arnd Bergmann]

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>

Re: [PATCH 0/6] ARM/ARM64: Drop arm_pm_restart

[Wolfram Sang]

[-- 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 --]

[PATCH 0/6] ARM/ARM64: Drop arm_pm_restart

[Wolfram Sang]

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>

[PATCH 0/6] ARM, arm64: Remove arm_pm_restart()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Hi everyone,

This small series is preparatory work for a series that I'm working on
which attempts to establish a formal framework for system restart and
power off.

Guenter has done a lot of good work in this area, but it never got
merged. I think this set is a valuable addition to the kernel because
it converts all odd providers to the established mechanism for restart.

Since this is stretched across both 32-bit and 64-bit ARM, as well as
PSCI, and given the SoC/board level of functionality, I think it might
make sense to take this through the ARM SoC tree in order to simplify
the interdependencies. But it should also be possible to take patches
1-4 via their respective trees this cycle and patches 5-6 through the
ARM and arm64 trees for the next cycle, if that's preferred.

Thanks,
Thierry

Guenter Roeck (6):
  ARM: prima2: Register with kernel restart handler
  ARM: xen: Register with kernel restart handler
  drivers: firmware: 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(-)

-- 
2.11.0

[PATCH 2/6] ARM: xen: Register with kernel restart handler

[Thierry Reding]

From: Guenter Roeck <linux@roeck-us.net>

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.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Thierry Reding <treding@nvidia.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 11d9f2898b16..85d678e1d826 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -29,6 +29,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>
@@ -191,14 +192,22 @@ static int xen_dying_cpu(unsigned int cpu)
 	return 0;
 }
 
-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 };
@@ -423,7 +432,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.11.0

[PATCH 2/6] ARM: xen: Register with kernel restart handler

[Thierry Reding]

From: Guenter Roeck <linux@roeck-us.net>

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.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Thierry Reding <treding@nvidia.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 11d9f2898b16..85d678e1d826 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -29,6 +29,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>
@@ -191,14 +192,22 @@ static int xen_dying_cpu(unsigned int cpu)
 	return 0;
 }
 
-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 };
@@ -423,7 +432,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.11.0

[PATCH 0/6] ARM/arm64: arm_pm_restart removal

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Hi Russell, ARM SoC maintainers,

here's the full set of patches that remove arm_pm_restart as discussed
earlier. There's some background on the series in this thread:

	https://lore.kernel.org/linux-arm-kernel/20170130110512.6943-1-thierry.reding@gmail.com/

I also have a set of patches that build on top of this and try to add
something slightly more formal by adding a power/reset framework that
driver can register with. If we can get this series merged, I'll find
some time to refresh those patches and send out for review again.

Thierry

Guenter Roeck (6):
  ARM: prima2: Register with kernel restart handler
  ARM: xen: Register with kernel restart handler
  drivers: firmware: 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             | 12 ++++++++++--
 arch/arm64/include/asm/system_misc.h |  2 --
 arch/arm64/kernel/process.c          |  7 +------
 drivers/firmware/psci/psci.c         | 12 ++++++++++--
 8 files changed, 49 insertions(+), 22 deletions(-)

-- 
2.23.0

[PATCH 2/6] ARM: xen: Register with kernel restart handler

[Thierry Reding]

From: Guenter Roeck <linux@roeck-us.net>

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.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 arch/arm/xen/enlighten.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 1e57692552d9..eb0a0edb9909 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -30,6 +30,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>
@@ -181,11 +182,18 @@ void xen_reboot(int reason)
 	BUG_ON(rc);
 }
 
-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)
 {
 	xen_reboot(SHUTDOWN_reboot);
+
+	return NOTIFY_DONE;
 }
 
+static struct notifier_block xen_restart_nb = {
+	.notifier_call = xen_restart,
+	.priority = 192,
+};
 
 static void xen_power_off(void)
 {
@@ -406,7 +414,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.23.0

[PATCH 2/6] ARM: xen: Register with kernel restart handler

[Thierry Reding]

From: Guenter Roeck <linux@roeck-us.net>

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.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 arch/arm/xen/enlighten.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 1e57692552d9..eb0a0edb9909 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -30,6 +30,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>
@@ -181,11 +182,18 @@ void xen_reboot(int reason)
 	BUG_ON(rc);
 }
 
-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)
 {
 	xen_reboot(SHUTDOWN_reboot);
+
+	return NOTIFY_DONE;
 }
 
+static struct notifier_block xen_restart_nb = {
+	.notifier_call = xen_restart,
+	.priority = 192,
+};
 
 static void xen_power_off(void)
 {
@@ -406,7 +414,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.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> wrote:
> 
> > From: Guenter Roeck <linux@roeck-us.net>
> >
> > 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.
> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  arch/arm/xen/enlighten.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> 
> This patch does appear to be useful.
> 
> Is this just being solved in downstream trees at the moment?
> 
> It would be nice if we could relinquish people of this burden and get it
> into Mainline finally.
> 

There must have been half a dozen attempts to send this patch series
upstream. I have tried, and others have tried. Each attempt failed with
someone else objecting for non-technical reasons (such as "we need more
reviews") or no reaction at all, and maintainers just don't pick it up.

So, yes, this patch series can only be found in downstream trees,
and it seems pointless to submit it yet again.

Guenter

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> wrote:
> 
> > From: Guenter Roeck <linux@roeck-us.net>
> >
> > 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.
> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  arch/arm/xen/enlighten.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> 
> This patch does appear to be useful.
> 
> Is this just being solved in downstream trees at the moment?
> 
> It would be nice if we could relinquish people of this burden and get it
> into Mainline finally.
> 

There must have been half a dozen attempts to send this patch series
upstream. I have tried, and others have tried. Each attempt failed with
someone else objecting for non-technical reasons (such as "we need more
reviews") or no reaction at all, and maintainers just don't pick it up.

So, yes, this patch series can only be found in downstream trees,
and it seems pointless to submit it yet again.

Guenter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Lee Jones]

On Thu, 03 Jun 2021, Guenter Roeck wrote:

> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > wrote:
> > 
> > > From: Guenter Roeck <linux@roeck-us.net>
> > >
> > > 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.
> > >
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > >  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > 
> > This patch does appear to be useful.
> > 
> > Is this just being solved in downstream trees at the moment?
> > 
> > It would be nice if we could relinquish people of this burden and get it
> > into Mainline finally.
> > 
> 
> There must have been half a dozen attempts to send this patch series
> upstream. I have tried, and others have tried. Each attempt failed with
> someone else objecting for non-technical reasons (such as "we need more
> reviews") or no reaction at all, and maintainers just don't pick it up.

Looking at the *-by tag list above, I think we have enough quality
reviews to take this forward.

> So, yes, this patch series can only be found in downstream trees,
> and it seems pointless to submit it yet again.

IMHO, it's unfair to burden multiple downstream trees with this purely
due to poor or nervy maintainership.  Functionality as broadly useful
as this should be merged and maintained in Mainline.

OOI, who is blocking?  As I see it, we have 2 of the key maintainers
in the *-by list.  With those on-board, it's difficult to envisage
what the problem is.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Lee Jones]

On Thu, 03 Jun 2021, Guenter Roeck wrote:

> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > wrote:
> > 
> > > From: Guenter Roeck <linux@roeck-us.net>
> > >
> > > 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.
> > >
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > >  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > 
> > This patch does appear to be useful.
> > 
> > Is this just being solved in downstream trees at the moment?
> > 
> > It would be nice if we could relinquish people of this burden and get it
> > into Mainline finally.
> > 
> 
> There must have been half a dozen attempts to send this patch series
> upstream. I have tried, and others have tried. Each attempt failed with
> someone else objecting for non-technical reasons (such as "we need more
> reviews") or no reaction at all, and maintainers just don't pick it up.

Looking at the *-by tag list above, I think we have enough quality
reviews to take this forward.

> So, yes, this patch series can only be found in downstream trees,
> and it seems pointless to submit it yet again.

IMHO, it's unfair to burden multiple downstream trees with this purely
due to poor or nervy maintainership.  Functionality as broadly useful
as this should be merged and maintained in Mainline.

OOI, who is blocking?  As I see it, we have 2 of the key maintainers
in the *-by list.  With those on-board, it's difficult to envisage
what the problem is.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Boris Ostrovsky]

On 6/3/21 9:38 AM, Lee Jones wrote:
> On Thu, 03 Jun 2021, Guenter Roeck wrote:
>
>> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
>>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
>>> wrote:
>>>
>>>> From: Guenter Roeck <linux@roeck-us.net>
>>>>
>>>> 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.
>>>>
>>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>> ---
>>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
>>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>>
>>> This patch does appear to be useful.
>>>
>>> Is this just being solved in downstream trees at the moment?
>>>
>>> It would be nice if we could relinquish people of this burden and get it
>>> into Mainline finally.
>>>
>> There must have been half a dozen attempts to send this patch series
>> upstream. I have tried, and others have tried. Each attempt failed with
>> someone else objecting for non-technical reasons (such as "we need more
>> reviews") or no reaction at all, and maintainers just don't pick it up.
> Looking at the *-by tag list above, I think we have enough quality
> reviews to take this forward.
>
>> So, yes, this patch series can only be found in downstream trees,
>> and it seems pointless to submit it yet again.
> IMHO, it's unfair to burden multiple downstream trees with this purely
> due to poor or nervy maintainership.  Functionality as broadly useful
> as this should be merged and maintained in Mainline.
>
> OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> in the *-by list.  With those on-board, it's difficult to envisage
> what the problem is.


Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).


-boris

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Boris Ostrovsky]

On 6/3/21 9:38 AM, Lee Jones wrote:
> On Thu, 03 Jun 2021, Guenter Roeck wrote:
>
>> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
>>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
>>> wrote:
>>>
>>>> From: Guenter Roeck <linux@roeck-us.net>
>>>>
>>>> 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.
>>>>
>>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>> ---
>>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
>>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>>
>>> This patch does appear to be useful.
>>>
>>> Is this just being solved in downstream trees at the moment?
>>>
>>> It would be nice if we could relinquish people of this burden and get it
>>> into Mainline finally.
>>>
>> There must have been half a dozen attempts to send this patch series
>> upstream. I have tried, and others have tried. Each attempt failed with
>> someone else objecting for non-technical reasons (such as "we need more
>> reviews") or no reaction at all, and maintainers just don't pick it up.
> Looking at the *-by tag list above, I think we have enough quality
> reviews to take this forward.
>
>> So, yes, this patch series can only be found in downstream trees,
>> and it seems pointless to submit it yet again.
> IMHO, it's unfair to burden multiple downstream trees with this purely
> due to poor or nervy maintainership.  Functionality as broadly useful
> as this should be merged and maintained in Mainline.
>
> OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> in the *-by list.  With those on-board, it's difficult to envisage
> what the problem is.


Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).


-boris


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Russell King (Oracle)]

On Thu, Jun 03, 2021 at 09:48:59AM -0400, Boris Ostrovsky wrote:
> On 6/3/21 9:38 AM, Lee Jones wrote:
> > On Thu, 03 Jun 2021, Guenter Roeck wrote:
> >
> >> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> >>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> >>> wrote:
> >>>
> >>>> From: Guenter Roeck <linux@roeck-us.net>
> >>>>
> >>>> 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.
> >>>>
> >>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
> >>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >>>> ---
> >>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
> >>>>  1 file changed, 10 insertions(+), 2 deletions(-)
> >>>>
> >>> This patch does appear to be useful.
> >>>
> >>> Is this just being solved in downstream trees at the moment?
> >>>
> >>> It would be nice if we could relinquish people of this burden and get it
> >>> into Mainline finally.
> >>>
> >> There must have been half a dozen attempts to send this patch series
> >> upstream. I have tried, and others have tried. Each attempt failed with
> >> someone else objecting for non-technical reasons (such as "we need more
> >> reviews") or no reaction at all, and maintainers just don't pick it up.
> > Looking at the *-by tag list above, I think we have enough quality
> > reviews to take this forward.
> >
> >> So, yes, this patch series can only be found in downstream trees,
> >> and it seems pointless to submit it yet again.
> > IMHO, it's unfair to burden multiple downstream trees with this purely
> > due to poor or nervy maintainership.  Functionality as broadly useful
> > as this should be merged and maintained in Mainline.
> >
> > OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> > in the *-by list.  With those on-board, it's difficult to envisage
> > what the problem is.
> 
> 
> Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).

Stefano already reviewed this patch, which is part of a larger series
that primarily touches 32-bit ARM code, but also touches 64-bit ARM
code as well.

As I said in my previous reply, I don't see that there's any problem
with getting these patches merged had the usual processes been
followed - either ending up in the patch system, or the pull request
being sent to me directly.

Sadly, the pull request was sent to the arm-soc people excluding me,
I happened to notice it and requested to see the patches that were
being asked to be pulled (since I probably couldn't find them)...
and it then took two further weeks before the patches were posted,
which I then missed amongst all the other email.

It's a process failure and unfortunate timing rather than anything
malicious.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Russell King (Oracle)]

On Thu, Jun 03, 2021 at 09:48:59AM -0400, Boris Ostrovsky wrote:
> On 6/3/21 9:38 AM, Lee Jones wrote:
> > On Thu, 03 Jun 2021, Guenter Roeck wrote:
> >
> >> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> >>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> >>> wrote:
> >>>
> >>>> From: Guenter Roeck <linux@roeck-us.net>
> >>>>
> >>>> 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.
> >>>>
> >>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
> >>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >>>> ---
> >>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
> >>>>  1 file changed, 10 insertions(+), 2 deletions(-)
> >>>>
> >>> This patch does appear to be useful.
> >>>
> >>> Is this just being solved in downstream trees at the moment?
> >>>
> >>> It would be nice if we could relinquish people of this burden and get it
> >>> into Mainline finally.
> >>>
> >> There must have been half a dozen attempts to send this patch series
> >> upstream. I have tried, and others have tried. Each attempt failed with
> >> someone else objecting for non-technical reasons (such as "we need more
> >> reviews") or no reaction at all, and maintainers just don't pick it up.
> > Looking at the *-by tag list above, I think we have enough quality
> > reviews to take this forward.
> >
> >> So, yes, this patch series can only be found in downstream trees,
> >> and it seems pointless to submit it yet again.
> > IMHO, it's unfair to burden multiple downstream trees with this purely
> > due to poor or nervy maintainership.  Functionality as broadly useful
> > as this should be merged and maintained in Mainline.
> >
> > OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> > in the *-by list.  With those on-board, it's difficult to envisage
> > what the problem is.
> 
> 
> Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).

Stefano already reviewed this patch, which is part of a larger series
that primarily touches 32-bit ARM code, but also touches 64-bit ARM
code as well.

As I said in my previous reply, I don't see that there's any problem
with getting these patches merged had the usual processes been
followed - either ending up in the patch system, or the pull request
being sent to me directly.

Sadly, the pull request was sent to the arm-soc people excluding me,
I happened to notice it and requested to see the patches that were
being asked to be pulled (since I probably couldn't find them)...
and it then took two further weeks before the patches were posted,
which I then missed amongst all the other email.

It's a process failure and unfortunate timing rather than anything
malicious.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Lee Jones]

On Thu, 03 Jun 2021, Russell King (Oracle) wrote:

> On Thu, Jun 03, 2021 at 09:48:59AM -0400, Boris Ostrovsky wrote:
> > On 6/3/21 9:38 AM, Lee Jones wrote:
> > > On Thu, 03 Jun 2021, Guenter Roeck wrote:
> > >
> > >> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > >>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > >>> wrote:
> > >>>
> > >>>> From: Guenter Roeck <linux@roeck-us.net>
> > >>>>
> > >>>> 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.
> > >>>>
> > >>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
> > >>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > >>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > >>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> > >>>> ---
> > >>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > >>>>  1 file changed, 10 insertions(+), 2 deletions(-)
> > >>>>
> > >>> This patch does appear to be useful.
> > >>>
> > >>> Is this just being solved in downstream trees at the moment?
> > >>>
> > >>> It would be nice if we could relinquish people of this burden and get it
> > >>> into Mainline finally.
> > >>>
> > >> There must have been half a dozen attempts to send this patch series
> > >> upstream. I have tried, and others have tried. Each attempt failed with
> > >> someone else objecting for non-technical reasons (such as "we need more
> > >> reviews") or no reaction at all, and maintainers just don't pick it up.
> > > Looking at the *-by tag list above, I think we have enough quality
> > > reviews to take this forward.
> > >
> > >> So, yes, this patch series can only be found in downstream trees,
> > >> and it seems pointless to submit it yet again.
> > > IMHO, it's unfair to burden multiple downstream trees with this purely
> > > due to poor or nervy maintainership.  Functionality as broadly useful
> > > as this should be merged and maintained in Mainline.
> > >
> > > OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> > > in the *-by list.  With those on-board, it's difficult to envisage
> > > what the problem is.
> > 
> > 
> > Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).
> 
> Stefano already reviewed this patch, which is part of a larger series
> that primarily touches 32-bit ARM code, but also touches 64-bit ARM
> code as well.
> 
> As I said in my previous reply, I don't see that there's any problem
> with getting these patches merged had the usual processes been
> followed - either ending up in the patch system, or the pull request
> being sent to me directly.
> 
> Sadly, the pull request was sent to the arm-soc people excluding me,
> I happened to notice it and requested to see the patches that were
> being asked to be pulled (since I probably couldn't find them)...
> and it then took two further weeks before the patches were posted,
> which I then missed amongst all the other email.
> 
> It's a process failure and unfortunate timing rather than anything
> malicious.

Understood.

Is there anything I can do to help this forward?

I can either collect and re-submit the patches to the MLs if that
makes people's lives any easier.  Or if one of the original submitters
wish to retain responsibility, I have no qualms with that either.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Lee Jones]

On Thu, 03 Jun 2021, Russell King (Oracle) wrote:

> On Thu, Jun 03, 2021 at 09:48:59AM -0400, Boris Ostrovsky wrote:
> > On 6/3/21 9:38 AM, Lee Jones wrote:
> > > On Thu, 03 Jun 2021, Guenter Roeck wrote:
> > >
> > >> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > >>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > >>> wrote:
> > >>>
> > >>>> From: Guenter Roeck <linux@roeck-us.net>
> > >>>>
> > >>>> 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.
> > >>>>
> > >>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
> > >>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > >>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > >>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> > >>>> ---
> > >>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > >>>>  1 file changed, 10 insertions(+), 2 deletions(-)
> > >>>>
> > >>> This patch does appear to be useful.
> > >>>
> > >>> Is this just being solved in downstream trees at the moment?
> > >>>
> > >>> It would be nice if we could relinquish people of this burden and get it
> > >>> into Mainline finally.
> > >>>
> > >> There must have been half a dozen attempts to send this patch series
> > >> upstream. I have tried, and others have tried. Each attempt failed with
> > >> someone else objecting for non-technical reasons (such as "we need more
> > >> reviews") or no reaction at all, and maintainers just don't pick it up.
> > > Looking at the *-by tag list above, I think we have enough quality
> > > reviews to take this forward.
> > >
> > >> So, yes, this patch series can only be found in downstream trees,
> > >> and it seems pointless to submit it yet again.
> > > IMHO, it's unfair to burden multiple downstream trees with this purely
> > > due to poor or nervy maintainership.  Functionality as broadly useful
> > > as this should be merged and maintained in Mainline.
> > >
> > > OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> > > in the *-by list.  With those on-board, it's difficult to envisage
> > > what the problem is.
> > 
> > 
> > Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).
> 
> Stefano already reviewed this patch, which is part of a larger series
> that primarily touches 32-bit ARM code, but also touches 64-bit ARM
> code as well.
> 
> As I said in my previous reply, I don't see that there's any problem
> with getting these patches merged had the usual processes been
> followed - either ending up in the patch system, or the pull request
> being sent to me directly.
> 
> Sadly, the pull request was sent to the arm-soc people excluding me,
> I happened to notice it and requested to see the patches that were
> being asked to be pulled (since I probably couldn't find them)...
> and it then took two further weeks before the patches were posted,
> which I then missed amongst all the other email.
> 
> It's a process failure and unfortunate timing rather than anything
> malicious.

Understood.

Is there anything I can do to help this forward?

I can either collect and re-submit the patches to the MLs if that
makes people's lives any easier.  Or if one of the original submitters
wish to retain responsibility, I have no qualms with that either.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Russell King (Oracle)]

On Thu, Jun 03, 2021 at 03:03:01PM +0100, Lee Jones wrote:
> On Thu, 03 Jun 2021, Russell King (Oracle) wrote:
> 
> > On Thu, Jun 03, 2021 at 09:48:59AM -0400, Boris Ostrovsky wrote:
> > > On 6/3/21 9:38 AM, Lee Jones wrote:
> > > > On Thu, 03 Jun 2021, Guenter Roeck wrote:
> > > >
> > > >> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > > >>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > > >>> wrote:
> > > >>>
> > > >>>> From: Guenter Roeck <linux@roeck-us.net>
> > > >>>>
> > > >>>> 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.
> > > >>>>
> > > >>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > >>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > >>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > >>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > >>>> ---
> > > >>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > > >>>>  1 file changed, 10 insertions(+), 2 deletions(-)
> > > >>>>
> > > >>> This patch does appear to be useful.
> > > >>>
> > > >>> Is this just being solved in downstream trees at the moment?
> > > >>>
> > > >>> It would be nice if we could relinquish people of this burden and get it
> > > >>> into Mainline finally.
> > > >>>
> > > >> There must have been half a dozen attempts to send this patch series
> > > >> upstream. I have tried, and others have tried. Each attempt failed with
> > > >> someone else objecting for non-technical reasons (such as "we need more
> > > >> reviews") or no reaction at all, and maintainers just don't pick it up.
> > > > Looking at the *-by tag list above, I think we have enough quality
> > > > reviews to take this forward.
> > > >
> > > >> So, yes, this patch series can only be found in downstream trees,
> > > >> and it seems pointless to submit it yet again.
> > > > IMHO, it's unfair to burden multiple downstream trees with this purely
> > > > due to poor or nervy maintainership.  Functionality as broadly useful
> > > > as this should be merged and maintained in Mainline.
> > > >
> > > > OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> > > > in the *-by list.  With those on-board, it's difficult to envisage
> > > > what the problem is.
> > > 
> > > 
> > > Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).
> > 
> > Stefano already reviewed this patch, which is part of a larger series
> > that primarily touches 32-bit ARM code, but also touches 64-bit ARM
> > code as well.
> > 
> > As I said in my previous reply, I don't see that there's any problem
> > with getting these patches merged had the usual processes been
> > followed - either ending up in the patch system, or the pull request
> > being sent to me directly.
> > 
> > Sadly, the pull request was sent to the arm-soc people excluding me,
> > I happened to notice it and requested to see the patches that were
> > being asked to be pulled (since I probably couldn't find them)...
> > and it then took two further weeks before the patches were posted,
> > which I then missed amongst all the other email.
> > 
> > It's a process failure and unfortunate timing rather than anything
> > malicious.
> 
> Understood.
> 
> Is there anything I can do to help this forward?
> 
> I can either collect and re-submit the patches to the MLs if that
> makes people's lives any easier.  Or if one of the original submitters
> wish to retain responsibility, I have no qualms with that either.

I think at this point the usual applies - to make sure that they still
apply to current kernels and don't cause any regressions. I would hope
there won't be anything significant to invalidate the reviews already
given. If that's the case, it should just be a matter of someone
putting them in the patch system or send me a pull request.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Russell King (Oracle)]

On Thu, Jun 03, 2021 at 03:03:01PM +0100, Lee Jones wrote:
> On Thu, 03 Jun 2021, Russell King (Oracle) wrote:
> 
> > On Thu, Jun 03, 2021 at 09:48:59AM -0400, Boris Ostrovsky wrote:
> > > On 6/3/21 9:38 AM, Lee Jones wrote:
> > > > On Thu, 03 Jun 2021, Guenter Roeck wrote:
> > > >
> > > >> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > > >>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > > >>> wrote:
> > > >>>
> > > >>>> From: Guenter Roeck <linux@roeck-us.net>
> > > >>>>
> > > >>>> 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.
> > > >>>>
> > > >>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > >>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > >>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > >>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > >>>> ---
> > > >>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > > >>>>  1 file changed, 10 insertions(+), 2 deletions(-)
> > > >>>>
> > > >>> This patch does appear to be useful.
> > > >>>
> > > >>> Is this just being solved in downstream trees at the moment?
> > > >>>
> > > >>> It would be nice if we could relinquish people of this burden and get it
> > > >>> into Mainline finally.
> > > >>>
> > > >> There must have been half a dozen attempts to send this patch series
> > > >> upstream. I have tried, and others have tried. Each attempt failed with
> > > >> someone else objecting for non-technical reasons (such as "we need more
> > > >> reviews") or no reaction at all, and maintainers just don't pick it up.
> > > > Looking at the *-by tag list above, I think we have enough quality
> > > > reviews to take this forward.
> > > >
> > > >> So, yes, this patch series can only be found in downstream trees,
> > > >> and it seems pointless to submit it yet again.
> > > > IMHO, it's unfair to burden multiple downstream trees with this purely
> > > > due to poor or nervy maintainership.  Functionality as broadly useful
> > > > as this should be merged and maintained in Mainline.
> > > >
> > > > OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> > > > in the *-by list.  With those on-board, it's difficult to envisage
> > > > what the problem is.
> > > 
> > > 
> > > Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).
> > 
> > Stefano already reviewed this patch, which is part of a larger series
> > that primarily touches 32-bit ARM code, but also touches 64-bit ARM
> > code as well.
> > 
> > As I said in my previous reply, I don't see that there's any problem
> > with getting these patches merged had the usual processes been
> > followed - either ending up in the patch system, or the pull request
> > being sent to me directly.
> > 
> > Sadly, the pull request was sent to the arm-soc people excluding me,
> > I happened to notice it and requested to see the patches that were
> > being asked to be pulled (since I probably couldn't find them)...
> > and it then took two further weeks before the patches were posted,
> > which I then missed amongst all the other email.
> > 
> > It's a process failure and unfortunate timing rather than anything
> > malicious.
> 
> Understood.
> 
> Is there anything I can do to help this forward?
> 
> I can either collect and re-submit the patches to the MLs if that
> makes people's lives any easier.  Or if one of the original submitters
> wish to retain responsibility, I have no qualms with that either.

I think at this point the usual applies - to make sure that they still
apply to current kernels and don't cause any regressions. I would hope
there won't be anything significant to invalidate the reviews already
given. If that's the case, it should just be a matter of someone
putting them in the patch system or send me a pull request.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4863 bytes --]

On Thu, Jun 03, 2021 at 03:03:01PM +0100, Lee Jones wrote:
> On Thu, 03 Jun 2021, Russell King (Oracle) wrote:
> 
> > On Thu, Jun 03, 2021 at 09:48:59AM -0400, Boris Ostrovsky wrote:
> > > On 6/3/21 9:38 AM, Lee Jones wrote:
> > > > On Thu, 03 Jun 2021, Guenter Roeck wrote:
> > > >
> > > >> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > > >>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > > >>> wrote:
> > > >>>
> > > >>>> From: Guenter Roeck <linux@roeck-us.net>
> > > >>>>
> > > >>>> 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.
> > > >>>>
> > > >>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > >>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > >>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > >>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > >>>> ---
> > > >>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > > >>>>  1 file changed, 10 insertions(+), 2 deletions(-)
> > > >>>>
> > > >>> This patch does appear to be useful.
> > > >>>
> > > >>> Is this just being solved in downstream trees at the moment?
> > > >>>
> > > >>> It would be nice if we could relinquish people of this burden and get it
> > > >>> into Mainline finally.
> > > >>>
> > > >> There must have been half a dozen attempts to send this patch series
> > > >> upstream. I have tried, and others have tried. Each attempt failed with
> > > >> someone else objecting for non-technical reasons (such as "we need more
> > > >> reviews") or no reaction at all, and maintainers just don't pick it up.
> > > > Looking at the *-by tag list above, I think we have enough quality
> > > > reviews to take this forward.
> > > >
> > > >> So, yes, this patch series can only be found in downstream trees,
> > > >> and it seems pointless to submit it yet again.
> > > > IMHO, it's unfair to burden multiple downstream trees with this purely
> > > > due to poor or nervy maintainership.  Functionality as broadly useful
> > > > as this should be merged and maintained in Mainline.
> > > >
> > > > OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> > > > in the *-by list.  With those on-board, it's difficult to envisage
> > > > what the problem is.
> > > 
> > > 
> > > Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).
> > 
> > Stefano already reviewed this patch, which is part of a larger series
> > that primarily touches 32-bit ARM code, but also touches 64-bit ARM
> > code as well.
> > 
> > As I said in my previous reply, I don't see that there's any problem
> > with getting these patches merged had the usual processes been
> > followed - either ending up in the patch system, or the pull request
> > being sent to me directly.
> > 
> > Sadly, the pull request was sent to the arm-soc people excluding me,
> > I happened to notice it and requested to see the patches that were
> > being asked to be pulled (since I probably couldn't find them)...
> > and it then took two further weeks before the patches were posted,
> > which I then missed amongst all the other email.
> > 
> > It's a process failure and unfortunate timing rather than anything
> > malicious.
> 
> Understood.
> 
> Is there anything I can do to help this forward?
> 
> I can either collect and re-submit the patches to the MLs if that
> makes people's lives any easier.  Or if one of the original submitters
> wish to retain responsibility, I have no qualms with that either.

I had stumbled across these patches from Guenter when I was looking to
solve a reboot/power-off issue on one of the boards that I was working
on. This was supposed to be preparatory work to get rid of the global
function pointers that drivers are simply overwriting, and the goal had
been to add a "system power" framework that would allow drivers to
register a chip structure to provide a bit more "formality" than
overwriting function pointers or registering notifier callbacks.

There ended up not being any interest in such a subsystem, so I wanted
to at least get this prep work in, because it is at least a bit of an
improvement.

I vaguely recall that Arnd or perhaps Olof had mentioned that they'd
pull these patches, but the timing was bad, so they asked me to remind
them after the merge window. By the time we had gotten through the merge
window, I probably had gotten sidetracked and forgot...

Feel free to give this a shot. This series itself is still useful, in my
opinion.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 4863 bytes --]

On Thu, Jun 03, 2021 at 03:03:01PM +0100, Lee Jones wrote:
> On Thu, 03 Jun 2021, Russell King (Oracle) wrote:
> 
> > On Thu, Jun 03, 2021 at 09:48:59AM -0400, Boris Ostrovsky wrote:
> > > On 6/3/21 9:38 AM, Lee Jones wrote:
> > > > On Thu, 03 Jun 2021, Guenter Roeck wrote:
> > > >
> > > >> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > > >>> On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > > >>> wrote:
> > > >>>
> > > >>>> From: Guenter Roeck <linux@roeck-us.net>
> > > >>>>
> > > >>>> 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.
> > > >>>>
> > > >>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > >>>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > >>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > >>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > >>>> ---
> > > >>>>  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > > >>>>  1 file changed, 10 insertions(+), 2 deletions(-)
> > > >>>>
> > > >>> This patch does appear to be useful.
> > > >>>
> > > >>> Is this just being solved in downstream trees at the moment?
> > > >>>
> > > >>> It would be nice if we could relinquish people of this burden and get it
> > > >>> into Mainline finally.
> > > >>>
> > > >> There must have been half a dozen attempts to send this patch series
> > > >> upstream. I have tried, and others have tried. Each attempt failed with
> > > >> someone else objecting for non-technical reasons (such as "we need more
> > > >> reviews") or no reaction at all, and maintainers just don't pick it up.
> > > > Looking at the *-by tag list above, I think we have enough quality
> > > > reviews to take this forward.
> > > >
> > > >> So, yes, this patch series can only be found in downstream trees,
> > > >> and it seems pointless to submit it yet again.
> > > > IMHO, it's unfair to burden multiple downstream trees with this purely
> > > > due to poor or nervy maintainership.  Functionality as broadly useful
> > > > as this should be merged and maintained in Mainline.
> > > >
> > > > OOI, who is blocking?  As I see it, we have 2 of the key maintainers
> > > > in the *-by list.  With those on-board, it's difficult to envisage
> > > > what the problem is.
> > > 
> > > 
> > > Stefano (who is ARM Xen maintainer) left Citrix a while ago. He is at sstabellini@kernel.org (which I added to the CC line).
> > 
> > Stefano already reviewed this patch, which is part of a larger series
> > that primarily touches 32-bit ARM code, but also touches 64-bit ARM
> > code as well.
> > 
> > As I said in my previous reply, I don't see that there's any problem
> > with getting these patches merged had the usual processes been
> > followed - either ending up in the patch system, or the pull request
> > being sent to me directly.
> > 
> > Sadly, the pull request was sent to the arm-soc people excluding me,
> > I happened to notice it and requested to see the patches that were
> > being asked to be pulled (since I probably couldn't find them)...
> > and it then took two further weeks before the patches were posted,
> > which I then missed amongst all the other email.
> > 
> > It's a process failure and unfortunate timing rather than anything
> > malicious.
> 
> Understood.
> 
> Is there anything I can do to help this forward?
> 
> I can either collect and re-submit the patches to the MLs if that
> makes people's lives any easier.  Or if one of the original submitters
> wish to retain responsibility, I have no qualms with that either.

I had stumbled across these patches from Guenter when I was looking to
solve a reboot/power-off issue on one of the boards that I was working
on. This was supposed to be preparatory work to get rid of the global
function pointers that drivers are simply overwriting, and the goal had
been to add a "system power" framework that would allow drivers to
register a chip structure to provide a bit more "formality" than
overwriting function pointers or registering notifier callbacks.

There ended up not being any interest in such a subsystem, so I wanted
to at least get this prep work in, because it is at least a bit of an
improvement.

I vaguely recall that Arnd or perhaps Olof had mentioned that they'd
pull these patches, but the timing was bad, so they asked me to remind
them after the merge window. By the time we had gotten through the merge
window, I probably had gotten sidetracked and forgot...

Feel free to give this a shot. This series itself is still useful, in my
opinion.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Russell King (Oracle)]

On Thu, Jun 03, 2021 at 06:11:24AM -0700, Guenter Roeck wrote:
> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > wrote:
> > 
> > > From: Guenter Roeck <linux@roeck-us.net>
> > >
> > > 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.
> > >
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > >  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > 
> > This patch does appear to be useful.
> > 
> > Is this just being solved in downstream trees at the moment?
> > 
> > It would be nice if we could relinquish people of this burden and get it
> > into Mainline finally.
> > 
> 
> There must have been half a dozen attempts to send this patch series
> upstream. I have tried, and others have tried. Each attempt failed with
> someone else objecting for non-technical reasons (such as "we need more
> reviews") or no reaction at all, and maintainers just don't pick it up.
> 
> So, yes, this patch series can only be found in downstream trees,
> and it seems pointless to submit it yet again.

It has plenty of reviews and acks, so that's not the problem. If you
look back at the 2019 attempt:

1) there was a pull request sent on the 2 October 2019 to the arm soc
   guys to merge a series that quite obviously is outside of their
   remit as it touches mostly ARM core code - it should have been
   sent to me but wasn't, not even as a Cc.

2) I raised that issue, and as I could find no trace of the patches,
   I asked for the to be posted - which they were, eventually, two
   weeks later. It looks like I completely missed the patches amongst
   all the other email I don't bother to read anymore though. In any
   case, the pull request by that time would have been completely
   forgotten about.

And that's where it ended - no apparent follow-ups until now.

*Shrug*.

So in summary, I was expected to notice the patches amongst all the
other email, and then remember that there was a pull request that
wasn't even addressed to me...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Russell King (Oracle)]

On Thu, Jun 03, 2021 at 06:11:24AM -0700, Guenter Roeck wrote:
> On Thu, Jun 03, 2021 at 01:43:36PM +0100, Lee Jones wrote:
> > On Tue, 15 Oct 2019 at 15:52, Thierry Reding <thierry.reding@gmail.com>
> > wrote:
> > 
> > > From: Guenter Roeck <linux@roeck-us.net>
> > >
> > > 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.
> > >
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > >  arch/arm/xen/enlighten.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > 
> > This patch does appear to be useful.
> > 
> > Is this just being solved in downstream trees at the moment?
> > 
> > It would be nice if we could relinquish people of this burden and get it
> > into Mainline finally.
> > 
> 
> There must have been half a dozen attempts to send this patch series
> upstream. I have tried, and others have tried. Each attempt failed with
> someone else objecting for non-technical reasons (such as "we need more
> reviews") or no reaction at all, and maintainers just don't pick it up.
> 
> So, yes, this patch series can only be found in downstream trees,
> and it seems pointless to submit it yet again.

It has plenty of reviews and acks, so that's not the problem. If you
look back at the 2019 attempt:

1) there was a pull request sent on the 2 October 2019 to the arm soc
   guys to merge a series that quite obviously is outside of their
   remit as it touches mostly ARM core code - it should have been
   sent to me but wasn't, not even as a Cc.

2) I raised that issue, and as I could find no trace of the patches,
   I asked for the to be posted - which they were, eventually, two
   weeks later. It looks like I completely missed the patches amongst
   all the other email I don't bother to read anymore though. In any
   case, the pull request by that time would have been completely
   forgotten about.

And that's where it ended - no apparent follow-ups until now.

*Shrug*.

So in summary, I was expected to notice the patches amongst all the
other email, and then remember that there was a pull request that
wasn't even addressed to me...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel