diff mbox

[04/11] MIPS: use the common machine reset handling

Message ID 20131031062959.169063871@linux.com (mailing list archive)
State New, archived
Headers show

Commit Message

Domenico Andreoli Oct. 31, 2013, 6:27 a.m. UTC 
From: Domenico Andreoli <domenico.andreoli@linux.com>

Proof of concept: MIPS as a consumer of the machine reset hooks.

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-arch@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>
---
 arch/mips/kernel/reset.c |    7 +++++++
 kernel/power/Kconfig     |    2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

Vineet Gupta Nov. 1, 2013, 5:11 a.m. UTC | #1 
On 10/31/2013 11:57 AM, Domenico Andreoli wrote:
> From: Domenico Andreoli <domenico.andreoli@linux.com>
> 
> Proof of concept: MIPS as a consumer of the machine reset hooks.
> 
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-arch@vger.kernel.org
> Cc: linux-mips@vger.kernel.org
> Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>
> ---
>  arch/mips/kernel/reset.c |    7 +++++++
>  kernel/power/Kconfig     |    2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> Index: b/arch/mips/kernel/reset.c
> ===================================================================
> --- a/arch/mips/kernel/reset.c
> +++ b/arch/mips/kernel/reset.c
> @@ -11,6 +11,7 @@
>  #include <linux/pm.h>
>  #include <linux/types.h>
>  #include <linux/reboot.h>
> +#include <linux/machine_reset.h>
>  
>  #include <asm/reboot.h>
>  
> @@ -29,16 +30,22 @@ void machine_restart(char *command)
>  {
>  	if (_machine_restart)
>  		_machine_restart(command);
> +	else
> +		default_restart(reboot_mode, command);
>  }
>  
>  void machine_halt(void)
>  {
>  	if (_machine_halt)
>  		_machine_halt();
> +	else
> +		default_halt();
>  }
>  
>  void machine_power_off(void)
>  {
>  	if (pm_power_off)
>  		pm_power_off();
> +	else
> +		default_power_off();
>  }
> Index: b/kernel/power/Kconfig
> ===================================================================
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -297,4 +297,4 @@ config CPU_PM
>  config MACHINE_RESET
>  	bool
>  	default n
> -	depends on ARM || ARM64
> +	depends on ARM || ARM64 || MIPS

This particular idiom is frowned upon for new code. As new arches get added this
list keeps getting bigger and then those who don't need the feature need to add
the anti dependency. Also in this particular case the dependency is trivial so you
can just "select" it in arch/*/Kconfig
Domenico Andreoli Nov. 1, 2013, 5:26 a.m. UTC | #2 
On Fri, Nov 01, 2013 at 10:41:54AM +0530, Vineet Gupta wrote:
> On 10/31/2013 11:57 AM, Domenico Andreoli wrote:
> > From: Domenico Andreoli <domenico.andreoli@linux.com>
> > 
> > Proof of concept: MIPS as a consumer of the machine reset hooks.
> > 
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: linux-arch@vger.kernel.org
> > Cc: linux-mips@vger.kernel.org
> > Signed-off-by: Domenico Andreoli <domenico.andreoli@linux.com>
> > ---
> >  arch/mips/kernel/reset.c |    7 +++++++
> >  kernel/power/Kconfig     |    2 +-
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > Index: b/arch/mips/kernel/reset.c
> > ===================================================================
> > --- a/arch/mips/kernel/reset.c
> > +++ b/arch/mips/kernel/reset.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/pm.h>
> >  #include <linux/types.h>
> >  #include <linux/reboot.h>
> > +#include <linux/machine_reset.h>
> >  
> >  #include <asm/reboot.h>
> >  
> > @@ -29,16 +30,22 @@ void machine_restart(char *command)
> >  {
> >  	if (_machine_restart)
> >  		_machine_restart(command);
> > +	else
> > +		default_restart(reboot_mode, command);
> >  }
> >  
> >  void machine_halt(void)
> >  {
> >  	if (_machine_halt)
> >  		_machine_halt();
> > +	else
> > +		default_halt();
> >  }
> >  
> >  void machine_power_off(void)
> >  {
> >  	if (pm_power_off)
> >  		pm_power_off();
> > +	else
> > +		default_power_off();
> >  }
> > Index: b/kernel/power/Kconfig
> > ===================================================================
> > --- a/kernel/power/Kconfig
> > +++ b/kernel/power/Kconfig
> > @@ -297,4 +297,4 @@ config CPU_PM
> >  config MACHINE_RESET
> >  	bool
> >  	default n
> > -	depends on ARM || ARM64
> > +	depends on ARM || ARM64 || MIPS
> 
> This particular idiom is frowned upon for new code. As new arches get added this
> list keeps getting bigger and then those who don't need the feature need to add
> the anti dependency. Also in this particular case the dependency is trivial so you
> can just "select" it in arch/*/Kconfig

this dependency guarantees only that the option is available only on the
supported arches.

anyway I've not a strong opinion, I only picked the least invasive solution
I was able to think.

thanks,
Domenico
diff mbox

Patch

Index: b/arch/mips/kernel/reset.c
===================================================================
--- a/arch/mips/kernel/reset.c
+++ b/arch/mips/kernel/reset.c
@@ -11,6 +11,7 @@ 
 #include <linux/pm.h>
 #include <linux/types.h>
 #include <linux/reboot.h>
+#include <linux/machine_reset.h>
 
 #include <asm/reboot.h>
 
@@ -29,16 +30,22 @@  void machine_restart(char *command)
 {
 	if (_machine_restart)
 		_machine_restart(command);
+	else
+		default_restart(reboot_mode, command);
 }
 
 void machine_halt(void)
 {
 	if (_machine_halt)
 		_machine_halt();
+	else
+		default_halt();
 }
 
 void machine_power_off(void)
 {
 	if (pm_power_off)
 		pm_power_off();
+	else
+		default_power_off();
 }
Index: b/kernel/power/Kconfig
===================================================================
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -297,4 +297,4 @@  config CPU_PM
 config MACHINE_RESET
 	bool
 	default n
-	depends on ARM || ARM64
+	depends on ARM || ARM64 || MIPS