All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] corenet: Add power/reset support to Cyrus
@ 2016-09-06 19:11 Andy Fleming
  2016-09-06 19:11 ` [PATCH 1/3] cyrus: Add poweroff/reset support Andy Fleming
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Andy Fleming @ 2016-09-06 19:11 UTC (permalink / raw)
  To: linuxppc-dev, scottwood; +Cc: aperez

Cyrus has gpio pins for power and reset, so we should support them

Andy Fleming (3):
  cyrus: Add poweroff/reset support
  corenet: Support gpio power/reset for corenet
  Cyrus: create a defconfig

 arch/powerpc/Makefile                         |  5 +++++
 arch/powerpc/boot/dts/fsl/cyrus_p5020.dts     | 11 +++++++++++
 arch/powerpc/configs/cyrus_basic_defconfig    |  9 +++++++++
 arch/powerpc/platforms/85xx/corenet_generic.c | 18 ++++++++++++++++++
 4 files changed, 43 insertions(+)
 create mode 100644 arch/powerpc/configs/cyrus_basic_defconfig

-- 
1.9.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] cyrus: Add poweroff/reset support
  2016-09-06 19:11 [PATCH 0/3] corenet: Add power/reset support to Cyrus Andy Fleming
@ 2016-09-06 19:11 ` Andy Fleming
  2016-09-25  6:44   ` [1/3] " Scott Wood
  2016-09-06 19:11 ` [PATCH 2/3] corenet: Support gpio power/reset for corenet Andy Fleming
  2016-09-06 19:11 ` [PATCH 3/3] Cyrus: create a defconfig Andy Fleming
  2 siblings, 1 reply; 16+ messages in thread
From: Andy Fleming @ 2016-09-06 19:11 UTC (permalink / raw)
  To: linuxppc-dev, scottwood; +Cc: aperez

Cyrus uses GPIOs to complete board shutdown/reset.
Add nodes to indicate that support to the device tree.

Signed-off-by: Andy Fleming <afleming@gmail.com>
---
 arch/powerpc/boot/dts/fsl/cyrus_p5020.dts | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
index c603390..e417aaa 100644
--- a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
+++ b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
@@ -150,6 +150,17 @@
 				  0 0x00010000>;
 		};
 	};
+
+        gpio-poweroff {
+                compatible = "gpio-poweroff";
+                gpios = <&gpio0 3 1>;
+        };
+
+        gpio-restart {
+                compatible = "gpio-restart";
+                gpios = <&gpio0 2 1>;
+        };
+
 };
 
 /include/ "p5020si-post.dtsi"
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] corenet: Support gpio power/reset for corenet
  2016-09-06 19:11 [PATCH 0/3] corenet: Add power/reset support to Cyrus Andy Fleming
  2016-09-06 19:11 ` [PATCH 1/3] cyrus: Add poweroff/reset support Andy Fleming
@ 2016-09-06 19:11 ` Andy Fleming
  2016-09-06 22:29   ` Scott Wood
  2016-09-06 19:11 ` [PATCH 3/3] Cyrus: create a defconfig Andy Fleming
  2 siblings, 1 reply; 16+ messages in thread
From: Andy Fleming @ 2016-09-06 19:11 UTC (permalink / raw)
  To: linuxppc-dev, scottwood; +Cc: aperez

Boards can implement power and reset functionality over gpio using
these drivers:
  drivers/power/reset/gpio-poweroff.c
  drivers/power/reset/gpio-restart.c

While not all corenet boards use gpio for power/reset, this
support can be added without interfering with boards that do not
use this functionality.

If a board's device tree has the related nodes, they are now probed.
Also, gpio-poweroff uses the global pm_power_off callback to implement
the shutdown. However, pm_power_off was not invoked when the kernel
halted, although that is usually the desired behavior. If the board
provides gpio power and reset support, it is reasonable to assume that
halting should also power down the system, unless it has chosen to
pass those calls on to hypervisor.

Signed-off-by: Andy Fleming <afleming@gmail.com>
---
 arch/powerpc/platforms/85xx/corenet_generic.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c
index 3a6a84f..17b8ebb 100644
--- a/arch/powerpc/platforms/85xx/corenet_generic.c
+++ b/arch/powerpc/platforms/85xx/corenet_generic.c
@@ -59,6 +59,16 @@ void __init corenet_gen_pic_init(void)
 	}
 }
 
+/* If someone has registered a poweroff callback, invoke it */
+static void __noreturn corenet_generic_halt(void)
+{
+	if (pm_power_off)
+		pm_power_off();
+
+	/* Should not return */
+	for(;;);
+}
+
 /*
  * Setup the architecture
  */
@@ -127,6 +137,12 @@ static const struct of_device_id of_device_ids[] = {
 	{
 		.name		= "handles",
 	},
+	{
+		.name		= "gpio-poweroff",
+	},
+	{
+		.name		= "gpio-restart",
+	},
 	{}
 };
 
@@ -176,6 +192,8 @@ static int __init corenet_generic_probe(void)
 	extern struct smp_ops_t smp_85xx_ops;
 #endif
 
+	ppc_md.halt = corenet_generic_halt;
+
 	if (of_device_compatible_match(of_root, boards))
 		return 1;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] Cyrus: create a defconfig
  2016-09-06 19:11 [PATCH 0/3] corenet: Add power/reset support to Cyrus Andy Fleming
  2016-09-06 19:11 ` [PATCH 1/3] cyrus: Add poweroff/reset support Andy Fleming
  2016-09-06 19:11 ` [PATCH 2/3] corenet: Support gpio power/reset for corenet Andy Fleming
@ 2016-09-06 19:11 ` Andy Fleming
  2016-09-06 20:42   ` Scott Wood
  2 siblings, 1 reply; 16+ messages in thread
From: Andy Fleming @ 2016-09-06 19:11 UTC (permalink / raw)
  To: linuxppc-dev, scottwood; +Cc: aperez

This sets up the proper config elements for Power and Reset to work
properly (using the gpio pins).

Signed-off-by: Andy Fleming <afleming@gmail.com>
---
 arch/powerpc/Makefile                      | 5 +++++
 arch/powerpc/configs/cyrus_basic_defconfig | 9 +++++++++
 2 files changed, 14 insertions(+)
 create mode 100644 arch/powerpc/configs/cyrus_basic_defconfig

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 1934707..83f257e 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -332,6 +332,11 @@ corenet64_smp_defconfig:
 	$(call merge_into_defconfig,corenet_basic_defconfig,\
 		85xx-64bit 85xx-smp altivec 85xx-hw fsl-emb-nonhw)
 
+PHONY += cyrus_defconfig
+cyrus_defconfig:
+	$(call merge_into_defconfig,cyrus_basic_defconfig,\
+		85xx-64bit 85xx-smp altivec 85xx-hw fsl-emb-nonhw)
+
 PHONY += mpc86xx_defconfig
 mpc86xx_defconfig:
 	$(call merge_into_defconfig,mpc86xx_basic_defconfig,\
diff --git a/arch/powerpc/configs/cyrus_basic_defconfig b/arch/powerpc/configs/cyrus_basic_defconfig
new file mode 100644
index 0000000..975150c
--- /dev/null
+++ b/arch/powerpc/configs/cyrus_basic_defconfig
@@ -0,0 +1,9 @@
+CONFIG_CORENET_GENERIC=y
+CONFIG_USERLIB=y
+CONFIG_GPIO_GENERIC_PLATFORM=y
+CONFIG_POWER_SUPPLY=y
+CONFIG_POWER_RESET=y
+CONFIG_POWER_RESET_GPIO=y
+CONFIG_POWER_RESET_GPIO_RESTART=y
+CONFIG_RESET_CONTROLLER=y
+CONFIG_EXT3_FS=y
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] Cyrus: create a defconfig
  2016-09-06 19:11 ` [PATCH 3/3] Cyrus: create a defconfig Andy Fleming
@ 2016-09-06 20:42   ` Scott Wood
  2016-09-10 22:12     ` Andy Fleming
  0 siblings, 1 reply; 16+ messages in thread
From: Scott Wood @ 2016-09-06 20:42 UTC (permalink / raw)
  To: Andy Fleming, linuxppc-dev, scottwood; +Cc: aperez

On 09/06/2016 02:12 PM, Andy Fleming wrote:=0A=
> This sets up the proper config elements for Power and Reset to work=0A=
> properly (using the gpio pins).=0A=
> =0A=
> Signed-off-by: Andy Fleming <afleming@gmail.com>=0A=
> ---=0A=
>  arch/powerpc/Makefile                      | 5 +++++=0A=
>  arch/powerpc/configs/cyrus_basic_defconfig | 9 +++++++++=0A=
>  2 files changed, 14 insertions(+)=0A=
>  create mode 100644 arch/powerpc/configs/cyrus_basic_defconfig=0A=
=0A=
Why does cyrus need its own defconfig?  Just enable the power/reset=0A=
stuff in 85xx-hw.config.=0A=
=0A=
> +CONFIG_EXT3_FS=3Dy=0A=
=0A=
This has nothing to do with power/reset, and is already present in=0A=
fsl-emb-nonhw.config.=0A=
=0A=
-Scott=0A=
=0A=

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] corenet: Support gpio power/reset for corenet
  2016-09-06 19:11 ` [PATCH 2/3] corenet: Support gpio power/reset for corenet Andy Fleming
@ 2016-09-06 22:29   ` Scott Wood
  2016-09-10 22:05     ` Andy Fleming
  0 siblings, 1 reply; 16+ messages in thread
From: Scott Wood @ 2016-09-06 22:29 UTC (permalink / raw)
  To: Andy Fleming, linuxppc-dev, scottwood; +Cc: aperez

On 09/06/2016 02:12 PM, Andy Fleming wrote:=0A=
> Boards can implement power and reset functionality over gpio using=0A=
> these drivers:=0A=
>   drivers/power/reset/gpio-poweroff.c=0A=
>   drivers/power/reset/gpio-restart.c=0A=
> =0A=
> While not all corenet boards use gpio for power/reset, this=0A=
> support can be added without interfering with boards that do not=0A=
> use this functionality.=0A=
> =0A=
> If a board's device tree has the related nodes, they are now probed.=0A=
> Also, gpio-poweroff uses the global pm_power_off callback to implement=0A=
> the shutdown. However, pm_power_off was not invoked when the kernel=0A=
> halted, although that is usually the desired behavior. If the board=0A=
> provides gpio power and reset support, it is reasonable to assume that=0A=
> halting should also power down the system, unless it has chosen to=0A=
> pass those calls on to hypervisor.=0A=
=0A=
Halt and poweroff are not the same thing.  If userspace requests a=0A=
poweroff, then kernel_power_off() will call machine_power_off() which=0A=
will call pm_power_off().=0A=
=0A=
Why do we need anything corenet-specific here?=0A=
=0A=
> @@ -127,6 +137,12 @@ static const struct of_device_id of_device_ids[] =3D=
 {=0A=
>  	{=0A=
>  		.name		=3D "handles",=0A=
>  	},=0A=
> +	{=0A=
> +		.name		=3D "gpio-poweroff",=0A=
> +	},=0A=
> +	{=0A=
> +		.name		=3D "gpio-restart",=0A=
> +	},=0A=
>  	{}=0A=
>  };=0A=
>  =0A=
=0A=
I don't see any other platforms doing this.  How do the nodes get probed=0A=
for them?=0A=
=0A=
-Scott=0A=
=0A=

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] corenet: Support gpio power/reset for corenet
  2016-09-06 22:29   ` Scott Wood
@ 2016-09-10 22:05     ` Andy Fleming
  2016-09-12 22:47       ` Scott Wood
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Fleming @ 2016-09-10 22:05 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, aperez

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

On Tuesday, September 6, 2016, Scott Wood <scott.wood@nxp.com> wrote:

> On 09/06/2016 02:12 PM, Andy Fleming wrote:
> > Boards can implement power and reset functionality over gpio using
> > these drivers:
> >   drivers/power/reset/gpio-poweroff.c
> >   drivers/power/reset/gpio-restart.c
> >
> > While not all corenet boards use gpio for power/reset, this
> > support can be added without interfering with boards that do not
> > use this functionality.
> >
> > If a board's device tree has the related nodes, they are now probed.
> > Also, gpio-poweroff uses the global pm_power_off callback to implement
> > the shutdown. However, pm_power_off was not invoked when the kernel
> > halted, although that is usually the desired behavior. If the board
> > provides gpio power and reset support, it is reasonable to assume that
> > halting should also power down the system, unless it has chosen to
> > pass those calls on to hypervisor.
>
> Halt and poweroff are not the same thing.  If userspace requests a
> poweroff, then kernel_power_off() will call machine_power_off() which
> will call pm_power_off().
>
> Why do we need anything corenet-specific here?


>
> We don't, but then the board will halt instead of power off when you type
> shutdown -h now. Or if you type poweroff without a high enough run level,
> apparently. I'm amenable to removing the halt code, but there are concerns
> that this will cause the systems to behave unintentionally as intended, in
> that most systems that users interact with shut down when you call shutdown
> -h now. There may be scripts that depend on that behavior (or at least
> assume it). Perhaps I could add a config option?
> CONFIG_TREAT_HALT_AS_POWEROFF? CONFIG_POWEROFF_ON_HALT?


> I'll note that there is one other board that is doing the same thing, but
> they've implemented their own gpio poweroff driver. See
> commit 5611fe48c545a22e62273428ed74c9bfae4a9406. That commit also points
> vaguely in the direction of an answer to your second question...


>
> > @@ -127,6 +137,12 @@ static const struct of_device_id of_device_ids[] = {
> >       {
> >               .name           = "handles",
> >       },
> > +     {
> > +             .name           = "gpio-poweroff",
> > +     },
> > +     {
> > +             .name           = "gpio-restart",
> > +     },
> >       {}
> >  };
> >
>
> I don't see any other platforms doing this.  How do the nodes get probed
> for them?


> The answer is I don't know, but this is a common issue with adding new
> devices to the device tree in embedded powerpc. The only other platforms
> which have gpio-poweroff nodes in their trees are in arch/arm, and none of
> those platforms call the probing function of_platform_bus_probe. I suspect
> they either probe every root node, or they somehow construct the match_id.
> As noted in the above-referenced commit, putting the nodes under the gpio
> bus does not cause them to get probed. This seemed like the best way under
> the current corenet code.
>
> -Scott
>
>

[-- Attachment #2: Type: text/html, Size: 4538 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] Cyrus: create a defconfig
  2016-09-06 20:42   ` Scott Wood
@ 2016-09-10 22:12     ` Andy Fleming
  2016-09-12 17:54       ` Scott Wood
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Fleming @ 2016-09-10 22:12 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, scottwood, aperez

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

On Tuesday, September 6, 2016, Scott Wood <scott.wood@nxp.com> wrote:

> On 09/06/2016 02:12 PM, Andy Fleming wrote:
> > This sets up the proper config elements for Power and Reset to work
> > properly (using the gpio pins).
> >
> > Signed-off-by: Andy Fleming <afleming@gmail.com <javascript:;>>
> > ---
> >  arch/powerpc/Makefile                      | 5 +++++
> >  arch/powerpc/configs/cyrus_basic_defconfig | 9 +++++++++
> >  2 files changed, 14 insertions(+)
> >  create mode 100644 arch/powerpc/configs/cyrus_basic_defconfig
>
> Why does cyrus need its own defconfig?  Just enable the power/reset
> stuff in 85xx-hw.config.


>
> Ok.
>

>
> > +CONFIG_EXT3_FS=y
>
> This has nothing to do with power/reset, and is already present in
> fsl-emb-nonhw.config.


> Ok, weird. Are you sure? I see:


> # CONFIG_EXT3_FS is not set
>
> CONFIG_EXT4_FS=y
>
>
>>
>

[-- Attachment #2: Type: text/html, Size: 2487 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] Cyrus: create a defconfig
  2016-09-10 22:12     ` Andy Fleming
@ 2016-09-12 17:54       ` Scott Wood
  2016-09-13  1:21         ` Michael Ellerman
  2016-09-15  8:08         ` Andy Fleming
  0 siblings, 2 replies; 16+ messages in thread
From: Scott Wood @ 2016-09-12 17:54 UTC (permalink / raw)
  To: Andy Fleming; +Cc: linuxppc-dev, scottwood, aperez, Andrew Donnellan

On 09/10/2016 05:12 PM, Andy Fleming wrote:=0A=
> =0A=
> =0A=
> On Tuesday, September 6, 2016, Scott Wood <scott.wood@nxp.com=0A=
> <mailto:scott.wood@nxp.com>> wrote:=0A=
> =0A=
>     On 09/06/2016 02:12 PM, Andy Fleming wrote:=0A=
>     > This sets up the proper config elements for Power and Reset to work=
=0A=
>     > properly (using the gpio pins).=0A=
>     >=0A=
>     > Signed-off-by: Andy Fleming <afleming@gmail.com <javascript:;>>=0A=
>     > ---=0A=
>     >  arch/powerpc/Makefile                      | 5 +++++=0A=
>     >  arch/powerpc/configs/cyrus_basic_defconfig | 9 +++++++++=0A=
>     >  2 files changed, 14 insertions(+)=0A=
>     >  create mode 100644 arch/powerpc/configs/cyrus_basic_defconfig=0A=
> =0A=
>     Why does cyrus need its own defconfig?  Just enable the power/reset=
=0A=
>     stuff in 85xx-hw.config.=0A=
> =0A=
> =0A=
> =0A=
>     Ok.=0A=
=0A=
Please send non-HTML mail with proper quote markers.  It's hard to read=0A=
when it looks like someone is talking to themself.=0A=
=0A=
>     > +CONFIG_EXT3_FS=3Dy=0A=
> =0A=
>     This has nothing to do with power/reset, and is already present in=0A=
>     fsl-emb-nonhw.config.=0A=
> =0A=
> =0A=
>     Ok, weird. Are you sure? I see:=0A=
> =0A=
> =0A=
>         # CONFIG_EXT3_FS is not set=0A=
> =0A=
>         CONFIG_EXT4_FS=3Dy=0A=
=0A=
Apparently it was just removed by commit=0A=
bdd910017c6a4 ("powerpc/configs: Remove old symbols from defconfigs").=0A=
=0A=
Andrew Donnellan, why was ext3 support removed?=0A=
=0A=
-Scott=0A=
=0A=

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] corenet: Support gpio power/reset for corenet
  2016-09-10 22:05     ` Andy Fleming
@ 2016-09-12 22:47       ` Scott Wood
  2016-09-15  8:03         ` Andy Fleming
  0 siblings, 1 reply; 16+ messages in thread
From: Scott Wood @ 2016-09-12 22:47 UTC (permalink / raw)
  To: Andy Fleming; +Cc: linuxppc-dev, aperez

On 09/10/2016 05:05 PM, Andy Fleming wrote:=0A=
> =0A=
> =0A=
> On Tuesday, September 6, 2016, Scott Wood <scott.wood@nxp.com=0A=
> <mailto:scott.wood@nxp.com>> wrote:=0A=
> =0A=
>     On 09/06/2016 02:12 PM, Andy Fleming wrote:=0A=
>     > Boards can implement power and reset functionality over gpio using=
=0A=
>     > these drivers:=0A=
>     >   drivers/power/reset/gpio-poweroff.c=0A=
>     >   drivers/power/reset/gpio-restart.c=0A=
>     >=0A=
>     > While not all corenet boards use gpio for power/reset, this=0A=
>     > support can be added without interfering with boards that do not=0A=
>     > use this functionality.=0A=
>     >=0A=
>     > If a board's device tree has the related nodes, they are now probed=
.=0A=
>     > Also, gpio-poweroff uses the global pm_power_off callback to implem=
ent=0A=
>     > the shutdown. However, pm_power_off was not invoked when the kernel=
=0A=
>     > halted, although that is usually the desired behavior. If the board=
=0A=
>     > provides gpio power and reset support, it is reasonable to assume t=
hat=0A=
>     > halting should also power down the system, unless it has chosen to=
=0A=
>     > pass those calls on to hypervisor.=0A=
> =0A=
>     Halt and poweroff are not the same thing.  If userspace requests a=0A=
>     poweroff, then kernel_power_off() will call machine_power_off() which=
=0A=
>     will call pm_power_off().=0A=
> =0A=
>     Why do we need anything corenet-specific here?=0A=
> =0A=
> =0A=
> =0A=
>     We don't, but then the board will halt instead of power off when you=
=0A=
>     type shutdown -h now.=0A=
=0A=
Isn't that what it's supposed to do?  If you want poweroff then ask for=0A=
poweroff.=0A=
=0A=
>     Or if you type poweroff without a high enough=0A=
>     run level, apparently.=0A=
=0A=
Hmm?=0A=
=0A=
In any case, run levels have nothing to do with the kernel.  The kernel=0A=
implements LINUX_REBOOT_CMD_HALT and LINUX_REBOOT_CMD_POWER_OFF, and=0A=
they should do what they're advertised to do.=0A=
=0A=
>     I'm amenable to removing the halt code, but=0A=
>     there are concerns that this will cause the systems to behave=0A=
>     unintentionally as intended, in that most systems that users=0A=
>     interact with shut down when you call shutdown -h now. There may be=
=0A=
>     scripts that depend on that behavior (or at least assume it).=0A=
=0A=
When did the behavior you're seeking ever exist?=0A=
=0A=
>     I don't see any other platforms doing this.  How do the nodes get pro=
bed=0A=
>     for them?=0A=
> =0A=
> =0A=
>     The answer is I don't know, but this is a common issue with adding=0A=
>     new devices to the device tree in embedded powerpc. The only other=0A=
>     platforms which have gpio-poweroff nodes in their trees are in=0A=
>     arch/arm, and none of those platforms call the probing=0A=
>     function of_platform_bus_probe. I suspect they either probe every=0A=
>     root node, or they somehow construct the match_id. As noted in the=0A=
>     above-referenced commit, putting the nodes under the gpio bus does=0A=
>     not cause them to get probed. This seemed like the best way under=0A=
>     the current corenet code.=0A=
=0A=
Well, let's figure out what it is that PPC should be doing to have=0A=
things work the way it does on ARM.=0A=
=0A=
-Scott=0A=
=0A=

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] Cyrus: create a defconfig
  2016-09-12 17:54       ` Scott Wood
@ 2016-09-13  1:21         ` Michael Ellerman
  2016-09-15  8:08         ` Andy Fleming
  1 sibling, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2016-09-13  1:21 UTC (permalink / raw)
  To: Scott Wood, Andy Fleming
  Cc: scottwood, linuxppc-dev, Andrew Donnellan, aperez

Scott Wood <scott.wood@nxp.com> writes:
> On 09/10/2016 05:12 PM, Andy Fleming wrote:
>> On Tuesday, September 6, 2016, Scott Wood <scott.wood@nxp.com
>> <mailto:scott.wood@nxp.com>> wrote:
>>     On 09/06/2016 02:12 PM, Andy Fleming wrote:
>>     > +CONFIG_EXT3_FS=y
>> 
>>     This has nothing to do with power/reset, and is already present in
>>     fsl-emb-nonhw.config.
>> 
>> 
>>     Ok, weird. Are you sure? I see:
>> 
>> 
>>         # CONFIG_EXT3_FS is not set
>> 
>>         CONFIG_EXT4_FS=y
>
> Apparently it was just removed by commit
> bdd910017c6a4 ("powerpc/configs: Remove old symbols from defconfigs").
>
> Andrew Donnellan, why was ext3 support removed?

Because ext3 was removed, ext4 handles it now.

The EXT3 Kconfig symbols still exist but they are just for backward
compat with old configs (like this one), and they just turn on EXT4.

So AFAICS Andrew's patch was correct, and you should be getting ext3
support from ext4.

cheers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] corenet: Support gpio power/reset for corenet
  2016-09-12 22:47       ` Scott Wood
@ 2016-09-15  8:03         ` Andy Fleming
  2016-09-16 21:05           ` Scott Wood
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Fleming @ 2016-09-15  8:03 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, aperez



> On Sep 12, 2016, at 23:47, Scott Wood <scott.wood@nxp.com> wrote:
>=20
>> On 09/10/2016 05:05 PM, Andy Fleming wrote:
>>=20
>>=20
>> On Tuesday, September 6, 2016, Scott Wood <scott.wood@nxp.com
>> <mailto:scott.wood@nxp.com>> wrote:
>>=20
>>>    On 09/06/2016 02:12 PM, Andy Fleming wrote:
>>> Boards can implement power and reset functionality over gpio using
>>> these drivers:
>>>  drivers/power/reset/gpio-poweroff.c
>>>  drivers/power/reset/gpio-restart.c
>>>=20
>>> While not all corenet boards use gpio for power/reset, this
>>> support can be added without interfering with boards that do not
>>> use this functionality.
>>>=20
>>> If a board's device tree has the related nodes, they are now probed.
>>> Also, gpio-poweroff uses the global pm_power_off callback to implement
>>> the shutdown. However, pm_power_off was not invoked when the kernel
>>> halted, although that is usually the desired behavior. If the board
>>> provides gpio power and reset support, it is reasonable to assume that
>>> halting should also power down the system, unless it has chosen to
>>> pass those calls on to hypervisor.
>>=20
>>    Halt and poweroff are not the same thing.  If userspace requests a
>>    poweroff, then kernel_power_off() will call machine_power_off() which
>>    will call pm_power_off().
>>=20
>>    Why do we need anything corenet-specific here?
>>=20
>>=20
>>=20
>>    We don't, but then the board will halt instead of power off when you
>>    type shutdown -h now.
>=20
> Isn't that what it's supposed to do?  If you want poweroff then ask for
> poweroff.
>=20
>>    Or if you type poweroff without a high enough
>>    run level, apparently.
>=20
> Hmm?
>=20
> In any case, run levels have nothing to do with the kernel.  The kernel
> implements LINUX_REBOOT_CMD_HALT and LINUX_REBOOT_CMD_POWER_OFF, and
> they should do what they're advertised to do.
>=20
>>    I'm amenable to removing the halt code, but
>>    there are concerns that this will cause the systems to behave
>>    unintentionally as intended, in that most systems that users
>>    interact with shut down when you call shutdown -h now. There may be
>>    scripts that depend on that behavior (or at least assume it).
>=20
> When did the behavior you're seeking ever exist?

Well, for one, on that Servergy board. I agree that halt and power off mean a=
nd have always meant different things to the kernel. The problem is that mos=
t desktop systems, having halted, pass control to the BIOS which--usually--s=
huts off the power. Am I wrong about this? I've been using shutdown -h now t=
o turn off my Linux systems for nearly 2 decades now, but I admit that I don=
't do it often, and I tend to stick with whatever works.


>=20
>>    I don't see any other platforms doing this.  How do the nodes get prob=
ed
>>    for them?
>>=20
>>=20
>>    The answer is I don't know, but this is a common issue with adding
>>    new devices to the device tree in embedded powerpc. The only other
>>    platforms which have gpio-poweroff nodes in their trees are in
>>    arch/arm, and none of those platforms call the probing
>>    function of_platform_bus_probe. I suspect they either probe every
>>    root node, or they somehow construct the match_id. As noted in the
>>    above-referenced commit, putting the nodes under the gpio bus does
>>    not cause them to get probed. This seemed like the best way under
>>    the current corenet code.
>=20
> Well, let's figure out what it is that PPC should be doing to have
> things work the way it does on ARM.

For all of the devices? Or just these two?

Andy

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] Cyrus: create a defconfig
  2016-09-12 17:54       ` Scott Wood
  2016-09-13  1:21         ` Michael Ellerman
@ 2016-09-15  8:08         ` Andy Fleming
  1 sibling, 0 replies; 16+ messages in thread
From: Andy Fleming @ 2016-09-15  8:08 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, scottwood, aperez, Andrew Donnellan



> On Sep 12, 2016, at 18:54, Scott Wood <scott.wood@nxp.com> wrote:
>=20
>> On 09/10/2016 05:12 PM, Andy Fleming wrote:
>>=20
>>=20
>> On Tuesday, September 6, 2016, Scott Wood <scott.wood@nxp.com
>> <mailto:scott.wood@nxp.com>> wrote:
>>=20
>>>    On 09/06/2016 02:12 PM, Andy Fleming wrote:
>>> This sets up the proper config elements for Power and Reset to work
>>> properly (using the gpio pins).
>>>=20
>>> Signed-off-by: Andy Fleming <afleming@gmail.com <javascript:;>>
>>> ---
>>> arch/powerpc/Makefile                      | 5 +++++
>>> arch/powerpc/configs/cyrus_basic_defconfig | 9 +++++++++
>>> 2 files changed, 14 insertions(+)
>>> create mode 100644 arch/powerpc/configs/cyrus_basic_defconfig
>>=20
>>    Why does cyrus need its own defconfig?  Just enable the power/reset
>>    stuff in 85xx-hw.config.
>>=20
>>=20
>>=20
>>    Ok.
>=20
> Please send non-HTML mail with proper quote markers.  It's hard to read
> when it looks like someone is talking to themself.

Argh, sorry. gmail iPhone client did that very subtly. I've flipped it over t=
o Mail, and hopefully that fixes it...

Andy=

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] corenet: Support gpio power/reset for corenet
  2016-09-15  8:03         ` Andy Fleming
@ 2016-09-16 21:05           ` Scott Wood
  2016-09-25  6:26             ` Scott Wood
  0 siblings, 1 reply; 16+ messages in thread
From: Scott Wood @ 2016-09-16 21:05 UTC (permalink / raw)
  To: Andy Fleming; +Cc: linuxppc-dev, aperez

On 09/15/2016 03:03 AM, Andy Fleming wrote:=0A=
> I agree that halt and power off mean and have always meant different=0A=
> things to the kernel. The problem is that most desktop systems,=0A=
> having halted, pass control to the BIOS which--usually--shuts off the=0A=
> power. Am I wrong about this? I've been using shutdown -h now to turn=0A=
> off my Linux systems for nearly 2 decades now, but I admit that I=0A=
> don't do it often, and I tend to stick with whatever works.=0A=
=0A=
>From the shutdown man page:=0A=
=0A=
     -h=0A=
           Equivalent to --poweroff, unless --halt is specified.=0A=
=0A=
Again, let's talk in terms of the kernel API rather than quirky=0A=
userspace commands.=0A=
=0A=
FWIW, I've always used "halt -p" and recall the system not powering off=0A=
on PCs when I use plain "halt", though it's been many years since I've=0A=
tried.=0A=
=0A=
>>>    I don't see any other platforms doing this.  How do the nodes get pr=
obed=0A=
>>>    for them?=0A=
>>>=0A=
>>>=0A=
>>>    The answer is I don't know, but this is a common issue with adding=
=0A=
>>>    new devices to the device tree in embedded powerpc. The only other=
=0A=
>>>    platforms which have gpio-poweroff nodes in their trees are in=0A=
>>>    arch/arm, and none of those platforms call the probing=0A=
>>>    function of_platform_bus_probe. I suspect they either probe every=0A=
>>>    root node, or they somehow construct the match_id. As noted in the=
=0A=
>>>    above-referenced commit, putting the nodes under the gpio bus does=
=0A=
>>>    not cause them to get probed. This seemed like the best way under=0A=
>>>    the current corenet code.=0A=
>>=0A=
>> Well, let's figure out what it is that PPC should be doing to have=0A=
>> things work the way it does on ARM.=0A=
> =0A=
> For all of the devices? Or just these two?=0A=
=0A=
All of them.  If ARM isn't maintaining these annoying lists why should=0A=
we have to? :-P=0A=
=0A=
-Scott=0A=
=0A=

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] corenet: Support gpio power/reset for corenet
  2016-09-16 21:05           ` Scott Wood
@ 2016-09-25  6:26             ` Scott Wood
  0 siblings, 0 replies; 16+ messages in thread
From: Scott Wood @ 2016-09-25  6:26 UTC (permalink / raw)
  To: Andy Fleming; +Cc: linuxppc-dev, aperez

On 09/16/2016 04:05 PM, Scott Wood wrote:=0A=
>>>>    I don't see any other platforms doing this.  How do the nodes get p=
robed=0A=
>>>>    for them?=0A=
>>>>=0A=
>>>>=0A=
>>>>    The answer is I don't know, but this is a common issue with adding=
=0A=
>>>>    new devices to the device tree in embedded powerpc. The only other=
=0A=
>>>>    platforms which have gpio-poweroff nodes in their trees are in=0A=
>>>>    arch/arm, and none of those platforms call the probing=0A=
>>>>    function of_platform_bus_probe. I suspect they either probe every=
=0A=
>>>>    root node, or they somehow construct the match_id. As noted in the=
=0A=
>>>>    above-referenced commit, putting the nodes under the gpio bus does=
=0A=
>>>>    not cause them to get probed. This seemed like the best way under=
=0A=
>>>>    the current corenet code.=0A=
>>>=0A=
>>> Well, let's figure out what it is that PPC should be doing to have=0A=
>>> things work the way it does on ARM.=0A=
>>=0A=
>> For all of the devices? Or just these two?=0A=
> =0A=
> All of them.  If ARM isn't maintaining these annoying lists why should=0A=
> we have to? :-P=0A=
=0A=
The answer seems to be using of_platform_populate() rather than=0A=
of_platform_bus_probe().=0A=
=0A=
-Scott=0A=
=0A=

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [1/3] cyrus: Add poweroff/reset support
  2016-09-06 19:11 ` [PATCH 1/3] cyrus: Add poweroff/reset support Andy Fleming
@ 2016-09-25  6:44   ` Scott Wood
  0 siblings, 0 replies; 16+ messages in thread
From: Scott Wood @ 2016-09-25  6:44 UTC (permalink / raw)
  To: Andy Fleming; +Cc: linuxppc-dev, scottwood, aperez

On Tue, Sep 06, 2016 at 02:11:57PM -0500, Andy Fleming wrote:
> Cyrus uses GPIOs to complete board shutdown/reset.
> Add nodes to indicate that support to the device tree.
> 
> Signed-off-by: Andy Fleming <afleming@gmail.com>
> ---
>  arch/powerpc/boot/dts/fsl/cyrus_p5020.dts | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> index c603390..e417aaa 100644
> --- a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> +++ b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> @@ -150,6 +150,17 @@
>  				  0 0x00010000>;
>  		};
>  	};
> +
> +        gpio-poweroff {
> +                compatible = "gpio-poweroff";
> +                gpios = <&gpio0 3 1>;
> +        };
> +
> +        gpio-restart {
> +                compatible = "gpio-restart";
> +                gpios = <&gpio0 2 1>;
> +        };
> +
>  };

Whitespace

-Scott

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-09-25  6:44 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-06 19:11 [PATCH 0/3] corenet: Add power/reset support to Cyrus Andy Fleming
2016-09-06 19:11 ` [PATCH 1/3] cyrus: Add poweroff/reset support Andy Fleming
2016-09-25  6:44   ` [1/3] " Scott Wood
2016-09-06 19:11 ` [PATCH 2/3] corenet: Support gpio power/reset for corenet Andy Fleming
2016-09-06 22:29   ` Scott Wood
2016-09-10 22:05     ` Andy Fleming
2016-09-12 22:47       ` Scott Wood
2016-09-15  8:03         ` Andy Fleming
2016-09-16 21:05           ` Scott Wood
2016-09-25  6:26             ` Scott Wood
2016-09-06 19:11 ` [PATCH 3/3] Cyrus: create a defconfig Andy Fleming
2016-09-06 20:42   ` Scott Wood
2016-09-10 22:12     ` Andy Fleming
2016-09-12 17:54       ` Scott Wood
2016-09-13  1:21         ` Michael Ellerman
2016-09-15  8:08         ` Andy Fleming

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.