ARM: dts: am335x-boneblack: disable RTC-only sleep

PATCH New Robert Nelson 4 participants 11 messages May 20, 2015 – Jul 6, 2015 Device tree bindings

lore

Share & tools

lore

Robert Nelson

author

2015-05-20 15:38 UTC #0 lore raw

PATCH 1/1 ARM: dts: am335x-boneblack: disable RTC-only sleep

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

Tested-by Matthijs van Duin <[email protected]>

Signed-off-by Robert Nelson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Felipe Balbi <[email protected]>

Cc Johan Hovold <[email protected]>

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

arch/arm/boot/dts/am335x-boneblack.dts +0 -4

index 5c42d25..901739f 100644
@@ -80,7 +80,3 @@
 		status = "okay";
 	};
 };
-
-&rtc {
-	system-power-controller;
-};

Signature · 1 line

2.1.4

Tony Lindgren

2015-05-20 16:47 UTC #1 lore raw

PATCH 1/1 ARM: dts: am335x-boneblack: disable RTC-only sleep

Quoted 27 lines · 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 <[email protected]>
Tested-by Matthijs van Duin <[email protected]>
Signed-off-by Robert Nelson <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc Felipe Balbi <[email protected]>
Cc Johan Hovold <[email protected]>
---
 arch/arm/boot/dts/am335x-boneblack.dts | 4 ----
 1 file changed, 4 deletions(-)
arch/arm/boot/dts/am335x-boneblack.dts +0 -0
index 5c42d25..901739f 100644

arch/arm/boot/dts/am335x-boneblack.dts line 86

Quoted 7 lines

arch/arm/boot/dts/am335x-boneblack.dts +0 -4

@@ -80,7 +80,3 @@
 		status = "okay";
 	};
 };
-
-&rtc {
-	system-power-controller;
-};

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

Regards,

Tony

Matthijs van Duin

2015-06-01 00:20 UTC #2 lore raw

PATCH 1/1 ARM: dts: am335x-boneblack: disable RTC-only sleep

In reply to Robert Nelson

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

Robert Nelson

author

2015-06-01 02:18 UTC #3 lore raw

PATCH 1/1 ARM: dts: am335x-boneblack: disable RTC-only sleep

Quoted 8 lines · 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).

Quoted 7 lines

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,

Signature · 2 lines

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

Matthijs van Duin

2015-06-01 05:14 UTC #4 lore raw

PATCH 1/1 ARM: dts: am335x-boneblack: disable RTC-only sleep

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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-dts-am335x-bone-enable-pmic-shutdown-controller.patch
Type: text/x-patch
Size: 2105 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150601/4cd12600/attachment.bin>

Tony Lindgren

2015-06-01 17:23 UTC #5 lore raw

PATCH 1/1 ARM: dts: am335x-boneblack: disable RTC-only sleep

Quoted 4 lines · Matthijs van Duin

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

Quoted 4 lines

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?

Quoted 41 lines

From 4278ecc32e886d2e83bc486e6409d8f6df82a0d1 Mon Sep 17 00:00:00 2001
From: Matthijs van Duin <[email protected]>
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 <[email protected]>
Tested-by Matthijs van Duin <[email protected]>
Signed-off-by Robert Nelson <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc Felipe Balbi <[email protected]>
Cc Johan Hovold <[email protected]>
[Matthijs van Duin: added explanatory comments]
Signed-off-by Matthijs van Duin <[email protected]>
---
 arch/arm/boot/dts/am335x-bone-common.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
arch/arm/boot/dts/am335x-bone-common.dtsi +11 -0
index c3255e0..90fbbb1 100644
@@ -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;

arch/arm/boot/dts/am335x-bone-common.dtsi line 246

Quoted 10 lines

arch/arm/boot/dts/am335x-bone-common.dtsi +7 -0

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

Matthijs van Duin

2015-06-01 19:33 UTC #6 lore raw

PATCH 1/1 ARM: dts: am335x-boneblack: disable RTC-only sleep

Quoted 1 line · Tony Lindgren

Ah finally you got around doing a proper patch :)

My First Linux Patch(tm) ;-)

Quoted 2 lines

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.

Quoted 2 lines

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)

Quoted 3 lines

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

Tested-by Matthijs van Duin <[email protected]>

Signed-off-by Robert Nelson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Felipe Balbi <[email protected]>

Cc Johan Hovold <[email protected]>

[Matthijs van Duin: added explanatory comments]

Signed-off-by Matthijs van Duin <[email protected]>

---
 arch/arm/boot/dts/am335x-bone-common.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

arch/arm/boot/dts/am335x-bone-common.dtsi +18 -0

index c3255e0..c0b0c02 100644
@@ -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 at 0 {
 			regulator-name = "vdds_dpr";

Signature · 1 line

2.1.4

Tony Lindgren

2015-06-01 20:01 UTC #7 lore raw

PATCH 1/1 ARM: dts: am335x-boneblack: disable RTC-only sleep

Quoted 4 lines · Matthijs van Duin

Quoted 1 line · Tony Lindgren
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 ;)

Quoted 5 lines

Quoted 2 lines
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.

Quoted 10 lines

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.
Quoted 2 lines
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...

Quoted 7 lines

(Let's hope all goes well)
Quoted 3 lines
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

Johan Hovold

2015-06-11 14:18 UTC #8 lore raw

PATCH 1/1 Revert "ARM: dts: am335x-boneblack: disable RTC-only sleep"

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

---

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

arch/arm/boot/dts/am335x-boneblack.dts +4 -0

index 901739fcb85a..5c42d259fa68 100644
@@ -80,3 +80,7 @@
 		status = "okay";
 	};
 };
+
+&rtc {
+	system-power-controller;
+};

Signature · 1 line

2.3.6

Tony Lindgren

2015-07-06 12:16 UTC #9 lore raw

PATCH 1/1 Revert "ARM: dts: am335x-boneblack: disable RTC-only sleep"

Quoted 17 lines · 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 <[email protected]>
---

Quoted 8 lines

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

Tony Lindgren

2015-06-01 17:12 UTC #10 lore raw

PATCH 1/1 ARM: dts: am335x-boneblack: disable RTC-only sleep

In reply to Robert Nelson

Quoted 13 lines · Robert Nelson

Quoted 11 lines · 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.

Quoted 8 lines

Quoted 4 lines
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