[PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[Robert Nelson]

Fixes: http://bugs.elinux.org/issues/143

Entering RTC-only sleep is only properly supported on early prototypes series
(pre-A6) of the BeagleBone Black.  Since rev (A6A), which include all production
versions, it is not support at due to.

(rev A6) enable of the 3v3b regulator moved from LDO2 to LDO4 (3v3a)
side-effect: 3v3b rail remains on in sleep-mode (also in off-mode when battery-powered)

(rev A6A) am335x vdds supply moved from LDO3 to LDO1
side-effect: vdds remains supplied in sleep-mode

Reported-by: Matthijs van Duin <matthijsvanduin@gmail.com>
Tested-by: Matthijs van Duin <matthijsvanduin@gmail.com>
Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Johan Hovold <johan@kernel.org>
---
 arch/arm/boot/dts/am335x-boneblack.dts | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index 5c42d25..901739f 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -80,7 +80,3 @@
 		status = "okay";
 	};
 };
-
-&rtc {
-	system-power-controller;
-};
-- 
2.1.4

Re: [PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[Tony Lindgren]

* Robert Nelson <robertcnelson@gmail.com> [150520 08:53]:
> Fixes: http://bugs.elinux.org/issues/143
> 
> Entering RTC-only sleep is only properly supported on early prototypes series
> (pre-A6) of the BeagleBone Black.  Since rev (A6A), which include all production
> versions, it is not support at due to.
> 
> (rev A6) enable of the 3v3b regulator moved from LDO2 to LDO4 (3v3a)
> side-effect: 3v3b rail remains on in sleep-mode (also in off-mode when battery-powered)
> 
> (rev A6A) am335x vdds supply moved from LDO3 to LDO1
> side-effect: vdds remains supplied in sleep-mode
> 
> Reported-by: Matthijs van Duin <matthijsvanduin@gmail.com>
> Tested-by: Matthijs van Duin <matthijsvanduin@gmail.com>
> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Johan Hovold <johan@kernel.org>
> ---
>  arch/arm/boot/dts/am335x-boneblack.dts | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
> index 5c42d25..901739f 100644
> --- a/arch/arm/boot/dts/am335x-boneblack.dts
> +++ b/arch/arm/boot/dts/am335x-boneblack.dts
> @@ -80,7 +80,3 @@
>  		status = "okay";
>  	};
>  };
> -
> -&rtc {
> -	system-power-controller;
> -};

Applying into omap-for-v4.1/fixes thanks.

Regards,

Tony

Re: [PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[Matthijs van Duin]

Sorry for the late response, I only just noticed this since I wasn't CC'd.

This "fix" was not ever Reported-By or Tested-By me as it claims. It
is in fact wrong: &rtc { system-power-controller; } needs to be
present for every variety of beaglebone (more generally every design
with a TPS65217 whose PWR_EN is connected to the PMIC_POWER_EN pin of
an AM335x).

Without a system-power-controller, the kernel has no way of turning
off the system and shutdown will leave the system halted instead.

The previously posted fix, which added  &tps {
ti,pmic-shutdown-controller; }  was correct, and ensures that a
poweroff makes the pmic enter "OFF-state" rather than "SLEEP-state"
(aka RTC-only sleep).

Matthijs

Re: [PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[Robert Nelson]

On Sun, May 31, 2015 at 7:20 PM, Matthijs van Duin
<matthijsvanduin@gmail.com> wrote:
> Sorry for the late response, I only just noticed this since I wasn't CC'd.
>
> This "fix" was not ever Reported-By or Tested-By me as it claims. It
> is in fact wrong: &rtc { system-power-controller; } needs to be
> present for every variety of beaglebone (more generally every design
> with a TPS65217 whose PWR_EN is connected to the PMIC_POWER_EN pin of
> an AM335x).
>
> Without a system-power-controller, the kernel has no way of turning
> off the system and shutdown will leave the system halted instead.
>
> The previously posted fix, which added  &tps {
> ti,pmic-shutdown-controller; }  was correct, and ensures that a
> poweroff makes the pmic enter "OFF-state" rather than "SLEEP-state"
> (aka RTC-only sleep).

Sorry Matthijs, (and everyone else)

That's my fault for completely screwing up this patch..

Revert:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3d76be5b933e2a66d85a2f7444e68e99e8a48ad4

Then use:
http://www.spinics.net/lists/linux-omap/msg118512.html

With the update commit message:

Fixes: http://bugs.elinux.org/issues/143

Entering RTC-only sleep is only properly supported on early prototypes
series (pre-A6) of the BeagleBone Black. Since rev (A6A), which
include all production versions, it is not support at due to.

(rev A6) enable of the 3v3b regulator moved from LDO2 to LDO4 (3v3a)
side-effect: 3v3b rail remains on in sleep-mode (also in off-mode when
battery-powered)

(rev A6A) am335x vdds supply moved from LDO3 to LDO1 side-effect: vdds
remains supplied in sleep-mode

Regards,

-- 
Robert Nelson
https://rcn-ee.com/

Re: [PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[Matthijs van Duin]

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

Here's a version that attempts to address Tony's earlier request to
include some comments in the patch itself. Since I saw no way to
concisely summarize the issues, I included links to relevant messages
(and enough info to locate the thread again in case of link rot).

Since the patch itself contains more explanation, I made the commit
message more concise.

Apologies for the attachment (inline text would get fucked up by gmail)

[-- Attachment #2: 0001-ARM-dts-am335x-bone-enable-pmic-shutdown-controller.patch --]
[-- Type: text/x-patch, Size: 2105 bytes --]

From 4278ecc32e886d2e83bc486e6409d8f6df82a0d1 Mon Sep 17 00:00:00 2001
From: Matthijs van Duin <matthijsvanduin@gmail.com>
Date: Mon, 1 Jun 2015 06:56:24 +0200
Subject: [PATCH] ARM: dts: am335x-bone* enable pmic-shutdown-controller

Fixes: http://bugs.elinux.org/issues/143

Avoid entering "RTC-only mode" at poweroff. It is unsupported by most
versions of BeagleBone, and risks hardware damage.

Reported-by: Matthijs van Duin <matthijsvanduin@gmail.com>
Tested-by: Matthijs van Duin <matthijsvanduin@gmail.com>
Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Johan Hovold <johan@kernel.org>
[Matthijs van Duin: added explanatory comments]
Signed-off-by: Matthijs van Duin <matthijsvanduin@gmail.com>
---
 arch/arm/boot/dts/am335x-bone-common.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
index c3255e0..90fbbb1 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -223,6 +223,24 @@
 /include/ "tps65217.dtsi"
 
 &tps {
+	// Configure pmic to enter OFF-state instead of SLEEP-state ("RTC-only
+	// mode") at poweroff.  Most BeagleBone versions do not support RTC-only
+	// mode and risk hardware damage if this mode is entered.
+	//
+	// For details, see linux-omap mailing list May 2015 thread
+	//	[PATCH] ARM: dts: am335x-bone* enable pmic-shutdown-controller
+	// In particular, messages:
+	//	http://www.spinics.net/lists/linux-omap/msg118585.html
+	//	http://www.spinics.net/lists/linux-omap/msg118615.html
+	//
+	ti,pmic-shutdown-controller;
+	//
+	// You can override this later with
+	//	&tps {  /delete-property/ ti,pmic-shutdown-controller;  }
+	// if you want to use RTC-only mode and made sure you are not affected
+	// by the hardware problems. (Tip: double-check by performing a current
+	// measurement after shutdown: it should be less than 1 mA.)
+
 	regulators {
 		dcdc1_reg: regulator@0 {
 			regulator-name = "vdds_dpr";
-- 
2.1.4

Re: [PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[Tony Lindgren]

* Matthijs van Duin <matthijsvanduin@gmail.com> [150531 22:16]:
> Here's a version that attempts to address Tony's earlier request to
> include some comments in the patch itself. Since I saw no way to
> concisely summarize the issues, I included links to relevant messages
> (and enough info to locate the thread again in case of link rot).

Ah finally you got around doing a proper patch :)
 
> Since the patch itself contains more explanation, I made the commit
> message more concise.
> 
> Apologies for the attachment (inline text would get fucked up by gmail)

Hmm no ideas about that, I guess people are using gmail to send email
with smtps though?

> From 4278ecc32e886d2e83bc486e6409d8f6df82a0d1 Mon Sep 17 00:00:00 2001
> From: Matthijs van Duin <matthijsvanduin@gmail.com>
> Date: Mon, 1 Jun 2015 06:56:24 +0200
> Subject: [PATCH] ARM: dts: am335x-bone* enable pmic-shutdown-controller
> 
> Fixes: http://bugs.elinux.org/issues/143
> 
> Avoid entering "RTC-only mode" at poweroff. It is unsupported by most
> versions of BeagleBone, and risks hardware damage.
> 
> Reported-by: Matthijs van Duin <matthijsvanduin@gmail.com>
> Tested-by: Matthijs van Duin <matthijsvanduin@gmail.com>
> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Johan Hovold <johan@kernel.org>
> [Matthijs van Duin: added explanatory comments]
> Signed-off-by: Matthijs van Duin <matthijsvanduin@gmail.com>
> ---
>  arch/arm/boot/dts/am335x-bone-common.dtsi | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
> index c3255e0..90fbbb1 100644
> --- a/arch/arm/boot/dts/am335x-bone-common.dtsi
> +++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
> @@ -223,6 +223,24 @@
>  /include/ "tps65217.dtsi"
>  
>  &tps {
> +	// Configure pmic to enter OFF-state instead of SLEEP-state ("RTC-only
> +	// mode") at poweroff.  Most BeagleBone versions do not support RTC-only
> +	// mode and risk hardware damage if this mode is entered.
> +	//
> +	// For details, see linux-omap mailing list May 2015 thread
> +	//	[PATCH] ARM: dts: am335x-bone* enable pmic-shutdown-controller
> +	// In particular, messages:
> +	//	http://www.spinics.net/lists/linux-omap/msg118585.html
> +	//	http://www.spinics.net/lists/linux-omap/msg118615.html
> +	//
> +	ti,pmic-shutdown-controller;
> +	//
> +	// You can override this later with
> +	//	&tps {  /delete-property/ ti,pmic-shutdown-controller;  }
> +	// if you want to use RTC-only mode and made sure you are not affected
> +	// by the hardware problems. (Tip: double-check by performing a current
> +	// measurement after shutdown: it should be less than 1 mA.)
> +
>  	regulators {
>  		dcdc1_reg: regulator@0 {
>  			regulator-name = "vdds_dpr";

Linux usually does not use the // style comments.. Can you please change
that? And how about have just one section of comments then the entry
for ti,pmic-shutdown-controller? Or maybe something like this:

	/*
	 * Configure pmic to enter OFF-state instead of SLEEP-state ("RTC-only
	 * mode") at poweroff.  Most BeagleBone versions do not support RTC-only
	 * ....
	 */
	&tps {
		ti,pmic-shutdown-controller;

		regulators {
			...
		};
		...
	};

Also, if this is needed as a fix for the mainline kernel to avoid hardware
damage, please let me know too ASAP.

Regards,

Tony

Re: [PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[Matthijs van Duin]

On 1 June 2015 at 19:23, Tony Lindgren <tony@atomide.com> wrote:
> Ah finally you got around doing a proper patch :)

My First Linux Patch(tm) ;-)

> Also, if this is needed as a fix for the mainline kernel to avoid hardware
> damage, please let me know too ASAP.

The damaging configuration is having &rtc { system-power-controller; }
without &tps { ti,pmic-shutdown-controller; }

So 3d76be5b9 also avoids damage, but with the unfortunate side-effect
that the system doesn't power off at all anymore (it just halts). I do
recommend applying this patch (restyled as needed) before reverting
3d76be5b9.

> Hmm no ideas about that, I guess people are using gmail to send email
> with smtps though?

Hmm, setting up mutt turned out less hassle than I remembered...

(Let's hope all goes well)

> Linux usually does not use the // style comments.. Can you please change
> that? And how about have just one section of comments then the entry
> for ti,pmic-shutdown-controller?

Fresh try...

--- 8< -----------------------------------------------------
Fixes: http://bugs.elinux.org/issues/143

Avoid entering "RTC-only mode" at poweroff. It is unsupported by most
versions of BeagleBone, and risks hardware damage.

Reported-by: Matthijs van Duin <matthijsvanduin@gmail.com>
Tested-by: Matthijs van Duin <matthijsvanduin@gmail.com>
Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Johan Hovold <johan@kernel.org>
[Matthijs van Duin: added explanatory comments]
Signed-off-by: Matthijs van Duin <matthijsvanduin@gmail.com>
---
 arch/arm/boot/dts/am335x-bone-common.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
index c3255e0..c0b0c02 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -223,6 +223,24 @@
 /include/ "tps65217.dtsi"
 
 &tps {
+	/* Configure pmic to enter OFF-state instead of SLEEP-state ("RTC-only
+	 * mode") at poweroff.  Most BeagleBone versions do not support RTC-only
+	 * mode and risk hardware damage if this mode is entered.
+	 *
+	 * For details, see linux-omap mailing list May 2015 thread
+	 *	[PATCH] ARM: dts: am335x-bone* enable pmic-shutdown-controller
+	 * In particular, messages:
+	 *	http://www.spinics.net/lists/linux-omap/msg118585.html
+	 *	http://www.spinics.net/lists/linux-omap/msg118615.html
+	 *
+	 * You can override this later with
+	 *	&tps {  /delete-property/ ti,pmic-shutdown-controller;  }
+	 * if you want to use RTC-only mode and made sure you are not affected
+	 * by the hardware problems. (Tip: double-check by performing a current
+	 * measurement after shutdown: it should be less than 1 mA.)
+	 */
+	ti,pmic-shutdown-controller;
+
 	regulators {
 		dcdc1_reg: regulator@0 {
 			regulator-name = "vdds_dpr";
-- 
2.1.4

Re: [PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[Tony Lindgren]

* Matthijs van Duin <matthijsvanduin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150601 12:35]:
> On 1 June 2015 at 19:23, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> > Ah finally you got around doing a proper patch :)
> 
> My First Linux Patch(tm) ;-)

Probably easier than trying to get other people to do the right
thing by writing emails ;)
 
> > Also, if this is needed as a fix for the mainline kernel to avoid hardware
> > damage, please let me know too ASAP.
> 
> The damaging configuration is having &rtc { system-power-controller; }
> without &tps { ti,pmic-shutdown-controller; }

OK thanks updating the description with that.
 
> So 3d76be5b9 also avoids damage, but with the unfortunate side-effect
> that the system doesn't power off at all anymore (it just halts). I do
> recommend applying this patch (restyled as needed) before reverting
> 3d76be5b9.
> 
> > Hmm no ideas about that, I guess people are using gmail to send email
> > with smtps though?
> 
> Hmm, setting up mutt turned out less hassle than I remembered...
> 
> (Let's hope all goes well)
> 
> > Linux usually does not use the // style comments.. Can you please change
> > that? And how about have just one section of comments then the entry
> > for ti,pmic-shutdown-controller?
> 
> Fresh try...

Thanks applying into omap-for-v4.1/fixes with Cc stable.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Revert "ARM: dts: am335x-boneblack: disable RTC-only sleep"

[Johan Hovold]

This reverts commit 3d76be5b933e2a66d85a2f7444e68e99e8a48ad4.

The latest revision of Beaglebone Black does not support RTC-only mode.

To avoid potential hardware damage, RTC-only mode was disabled by
default by commit 7a6cb0abe1aa ("ARM: dts: am335x-boneblack: disable
RTC-only sleep to avoid hardware damage").

Unfortunately, an incorrect fix had already been applied, which instead
of just disabling RTC-only mode, prevents the Beaglebone from powering
down at all.

Revert this patch to fix the power-off regression.

Signed-off-by: Johan Hovold <johan@kernel.org>
---

The offending patch was incorrectly applied after Matthijs initial fix
was posted and before the final fix (which only added some comments) was
applied.

Note that the final version of the fix 7a6cb0abe1aa ("ARM: dts:
am335x-boneblack: disable RTC-only sleep to avoid hardware damage")
indicates that it should be backported to 3.12 even though RTC-only mode
was first enabled in 3.19.

Johan


 arch/arm/boot/dts/am335x-boneblack.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index 901739fcb85a..5c42d259fa68 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -80,3 +80,7 @@
 		status = "okay";
 	};
 };
+
+&rtc {
+	system-power-controller;
+};
-- 
2.3.6

Re: [PATCH] Revert "ARM: dts: am335x-boneblack: disable RTC-only sleep"

[Tony Lindgren]

* Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [150611 07:20]:
> This reverts commit 3d76be5b933e2a66d85a2f7444e68e99e8a48ad4.
> 
> The latest revision of Beaglebone Black does not support RTC-only mode.
> 
> To avoid potential hardware damage, RTC-only mode was disabled by
> default by commit 7a6cb0abe1aa ("ARM: dts: am335x-boneblack: disable
> RTC-only sleep to avoid hardware damage").
> 
> Unfortunately, an incorrect fix had already been applied, which instead
> of just disabling RTC-only mode, prevents the Beaglebone from powering
> down at all.
> 
> Revert this patch to fix the power-off regression.
> 
> Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> 
> The offending patch was incorrectly applied after Matthijs initial fix
> was posted and before the final fix (which only added some comments) was
> applied.
> 
> Note that the final version of the fix 7a6cb0abe1aa ("ARM: dts:
> am335x-boneblack: disable RTC-only sleep to avoid hardware damage")
> indicates that it should be backported to 3.12 even though RTC-only mode
> was first enabled in 3.19.

Applying this now into omap-for-v4.2/fixes.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: am335x-boneblack: disable RTC-only sleep

[Tony Lindgren]

* Robert Nelson <robertcnelson@gmail.com> [150531 19:20]:
> On Sun, May 31, 2015 at 7:20 PM, Matthijs van Duin
> <matthijsvanduin@gmail.com> wrote:
> > Sorry for the late response, I only just noticed this since I wasn't CC'd.
> >
> > This "fix" was not ever Reported-By or Tested-By me as it claims. It
> > is in fact wrong: &rtc { system-power-controller; } needs to be
> > present for every variety of beaglebone (more generally every design
> > with a TPS65217 whose PWR_EN is connected to the PMIC_POWER_EN pin of
> > an AM335x).
> >
> > Without a system-power-controller, the kernel has no way of turning
> > off the system and shutdown will leave the system halted instead.
> >
> > The previously posted fix, which added  &tps {
> > ti,pmic-shutdown-controller; }  was correct, and ensures that a
> > poweroff makes the pmic enter "OFF-state" rather than "SLEEP-state"
> > (aka RTC-only sleep).
> 
> Sorry Matthijs, (and everyone else)
> 
> That's my fault for completely screwing up this patch..

Hmm OK. So do we now have a situation in the mainline that can
potentially cause hardware damage?

It seems we don't need to revert anything, just apply Matthijs'
new patch?

Regards,

Tony