ARM: dts: am335x-bone* enable pmic-shutdown-controller

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

Commit 672e2b147 which introduced proper shutdown chose to let the PMIC go into sleep-state (aka "RTC-only mode") rather than off-state. This was fine for rev A5 on which that patch was tested, but since then two changes have been made to the power supply scheme:

(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

As a result heavy leakage occurs from vdds (directly) and 3v3b (via i/o pins) to the 3v3a. The amount of current depends on external connections, but for example leaving a serial console cable connected in this state resulted in serious violation of am335x absolute maximum ratings, with potential for 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]>

Quoted 10 lines · Robert Nelson

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

Commit 672e2b147 which introduced proper shutdown chose to let the PMIC go into sleep-state (aka "RTC-only mode") rather than off-state. This was fine for rev A5 on which that patch was tested, but since then two changes have been made to the power supply scheme:

(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

Quoted 8 lines

As a result heavy leakage occurs from vdds (directly) and 3v3b (via i/o pins) to the 3v3a. The amount of current depends on external connections, but for example leaving a serial console cable connected in this state resulted in serious violation of am335x absolute maximum ratings, with potential for 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]>

This should go into stable as well:

Fixes 672e2b147413 ("ARM: dts: am335x-boneblack: enable power off and

rtc wake up")

Cc stable <[email protected]> # 3.19

Acked-by Johan Hovold <[email protected]>

By the way, perhaps you should add a comment in the tps node explaining why ti,pmic-shutdown-controller must not be disabled (on what revisions of hardware) as well?

Quoted 3 lines · Johan Hovold

By the way, perhaps you should add a comment in the tps node explaining why ti,pmic-shutdown-controller must not be disabled (on what revisions of hardware) as well?

There are actually three separate obstacles for RTC-only sleep (only one of which actually BeagleBone-specific), so the "why" is a bit of a long story. The end result can however be stated briefly:

Only BBBs prior to rev A6 properly support RTC-only mode. The rare BBW with a 2.x processor also supports it, provided nothing is connected to its expansion ports that leaks significant current from 3v3exp to processor I/Os while in reset.

Any BeagleBone with an AM335x 2.x can be patched to properly support RTC-only mode. (Though reconnecting vdds to LDO3 is really no fun, and not a patch I could have done myself.)

For reference, the details on the three problems:

1. Most BeagleBone Whites use AM335x 1.0 which freezes the RTC when vdd_core is gone or power-on reset is asserted, rendering RTC-only sleep mostly useless. The BeagleBone Black, and a few BBWs, use AM335x 2.x which fixed this.

Of all problems this is the most benign one since it merely results in loss of functionality. The other two risk hardware damage.

2. TI issued a notice that the AM335x "vdds" supply must be among the first to come up, and changed the official connection diagram for the TPS65217C/D (used for boards with DDR3/3L memory) by moving vdds from LDO3 to LDO1, and the BBB implemented this change in rev A6A.

This change is incompatible with RTC-only sleep (a fact mentioned by TI), so this rules out RTC-only sleep for any AM335x board with DDR3/3L memory unless it predates (or ignored) TI's advice or uses a different power supply scheme altogether. Boards with DDR2 memory (TPS65217A/B) such as the BBW are not affected.

Curiously, based on preliminary measurements, I have not observed any leakage to vdds when it is powered up "late" (LDO3). In contrast, during shutdown vdds begins to leak heavily (far in excess of rated max current) to other rails once they drop low enough, and moving vdds to LDO1 only makes this situation persist even longer (and indefinitely in RTC-only sleep). I've asked TI support for some clarification on this, but not yet received any response.

3. All BeagleBones have a mismanaged external regulator for the "3v3exp" (BBW) / "3v3b" (BBB) rail, which remains enabled longer than the AM335x's 3.3V supplies ("3v3a").

On the BBW, external hardware may end up sourcing current into processor I/Os, which feeds the 3v3a through protection diodes. Since the 3v3a is used as enable-signal for the regulator, if non-negligible current is leaked via this path, 3v3a remains "logic high" and the situation will persist indefinitely. If the currents remain modest this won't necessarily violate any absolute maximum ratings, but I'd still worry about the processor's long-term health.

On BBB rev A6 and later the situation is the same, but without dependency on external hardware: the current from on-board pull-up resistors already suffices (3v3a remains at ~1.4V), and since rev A6A the leakage from vdds would also get the job done. Worse still, since the buffer for the console port is powered by the 3v3b, having a serial cable attached causes a dangerous amount of current to be driven into UART0_RXD (~45 mA) and I captured an event which I fear may have been a brief latch-up.

On BBBs prior to rev A6 the regulator is enabled by LDO2 (whose only other use is the power led), which prevents it from staying on indefinitely. It is however enabled 1ms before and disabled 1ms after the processor's 3.3V supplies, so external hardware must still avoid driving much current into I/O pins during those windows, e.g. by keeping drivers disabled while reset is asserted. As mentioned above, the console port buffer violates this rule.

Technically this problem is also present when performing a full shutdown ("OFF-mode"), but then the main supply is cut at the start of the power-down sequence, which proceeds while running off rapidly draining capacitors. Heavy current draw from 3v3b will drain them even faster, thus making the issue self-limiting. (This however does not apply when running on battery power, in which case even a full shutdown is hazardous on an unpatched BBB rev A6 or later.)

Quoted 23 lines · Matthijs van Duin

Quoted 3 lines · Johan Hovold

By the way, perhaps you should add a comment in the tps node explaining why ti,pmic-shutdown-controller must not be disabled (on what revisions of hardware) as well?

There are actually three separate obstacles for RTC-only sleep (only one of which actually BeagleBone-specific), so the "why" is a bit of a long story. The end result can however be stated briefly:

Only BBBs prior to rev A6 properly support RTC-only mode. The rare BBW with a 2.x processor also supports it, provided nothing is connected to its expansion ports that leaks significant current from 3v3exp to processor I/Os while in reset.

Any BeagleBone with an AM335x 2.x can be patched to properly support RTC-only mode. (Though reconnecting vdds to LDO3 is really no fun, and not a patch I could have done myself.)

For reference, the details on the three problems:

Quoted 7 lines

1. Most BeagleBone Whites use AM335x 1.0 which freezes the RTC when vdd_core is gone or power-on reset is asserted, rendering RTC-only sleep mostly useless. The BeagleBone Black, and a few BBWs, use AM335x 2.x which fixed this.

Of all problems this is the most benign one since it merely results in loss of functionality. The other two risk hardware damage.

If some of these depend on the SoC revision and cannot be detected based on the RTC driver revision register, that information should be be passed to the RTC driver in platform data. This can be done with the rev == AM35XX_REV_ES* comparison that can be added to the mach-omap2/pdata-quirks.c for am335x to initialize platform_data for the RTC driver.

Quoted 11 lines

2. TI issued a notice that the AM335x "vdds" supply must be among the first to come up, and changed the official connection diagram for the TPS65217C/D (used for boards with DDR3/3L memory) by moving vdds from LDO3 to LDO1, and the BBB implemented this change in rev A6A.

This change is incompatible with RTC-only sleep (a fact mentioned by TI), so this rules out RTC-only sleep for any AM335x board with DDR3/3L memory unless it predates (or ignored) TI's advice or uses a different power supply scheme altogether. Boards with DDR2 memory (TPS65217A/B) such as the BBW are not affected.

Quoted 7 lines

Curiously, based on preliminary measurements, I have not observed any leakage to vdds when it is powered up "late" (LDO3). In contrast, during shutdown vdds begins to leak heavily (far in excess of rated max current) to other rails once they drop low enough, and moving vdds to LDO1 only makes this situation persist even longer (and indefinitely in RTC-only sleep). I've asked TI support for some clarification on this, but not yet received any response.

This information should be passed based on the board revision in platform_data to the RTC driver assuming we can detect the board revision during the early boot.. Otherwise we should have revision specific dts files.

Quoted 29 lines

3. All BeagleBones have a mismanaged external regulator for the "3v3exp" (BBW) / "3v3b" (BBB) rail, which remains enabled longer than the AM335x's 3.3V supplies ("3v3a").

On the BBW, external hardware may end up sourcing current into processor I/Os, which feeds the 3v3a through protection diodes. Since the 3v3a is used as enable-signal for the regulator, if non-negligible current is leaked via this path, 3v3a remains "logic high" and the situation will persist indefinitely. If the currents remain modest this won't necessarily violate any absolute maximum ratings, but I'd still worry about the processor's long-term health.

On BBB rev A6 and later the situation is the same, but without dependency on external hardware: the current from on-board pull-up resistors already suffices (3v3a remains at ~1.4V), and since rev A6A the leakage from vdds would also get the job done. Worse still, since the buffer for the console port is powered by the 3v3b, having a serial cable attached causes a dangerous amount of current to be driven into UART0_RXD (~45 mA) and I captured an event which I fear may have been a brief latch-up.

On BBBs prior to rev A6 the regulator is enabled by LDO2 (whose only other use is the power led), which prevents it from staying on indefinitely. It is however enabled 1ms before and disabled 1ms after the processor's 3.3V supplies, so external hardware must still avoid driving much current into I/O pins during those windows, e.g. by keeping drivers disabled while reset is asserted. As mentioned above, the console port buffer violates this rule.

Quoted 7 lines

Technically this problem is also present when performing a full shutdown ("OFF-mode"), but then the main supply is cut at the start of the power-down sequence, which proceeds while running off rapidly draining capacitors. Heavy current draw from 3v3b will drain them even faster, thus making the issue self-limiting. (This however does not apply when running on battery power, in which case even a full shutdown is hazardous on an unpatched BBB rev A6 or later.)

Sounds like this should be configured based on the board revision too.

On Mon, May 18, 2015 at 10:21 AM, Tony Lindgren <[email protected]> wrote:

All the rev information is in the board's eeprom:

hexdump -e '8/1 "%c"' /sys/bus/i2c/devices/0-0050/eeprom -s 12 -n 4

Rev A5B
0A5B

Rev C
000C

Just another default qwerk to add to Pantelis' bone_capemgr. ;)

Quoted 6 lines · Robert Nelson

On Mon, May 18, 2015 at 10:21 AM, Tony Lindgren <[email protected]> wrote:

All the rev information is in the board's eeprom:

hexdump -e '8/1 "%c"' /sys/bus/i2c/devices/0-0050/eeprom -s 12 -n 4

Quoted 7 lines

Rev A5B
0A5B

Rev C
000C

Just another default qwerk to add to Pantelis' bone_capemgr. ;)

It seems we should not even instantiate some devices on BBB until the EEPROM is parsed.. So maybe something like this:

1. The problem devices are initially set with status = "disabled" in the dts

2. We set up drivers/*/bbb-eeprom.c that parses the board revision at module_init time, and then flips the selected devices to have status = "enabled" and populates the revision info based on the eeprom and SoC revision passed in pdata. Then those devices get their struct device created and probed, but at a much later time.

So rather than trying to init all that early, let's just init them much later when we have the proper I2C driver running?

Quoted 28 lines · Tony Lindgren

Quoted 6 lines · Robert Nelson

On Mon, May 18, 2015 at 10:21 AM, Tony Lindgren <[email protected]> wrote:

All the rev information is in the board's eeprom:

hexdump -e '8/1 "%c"' /sys/bus/i2c/devices/0-0050/eeprom -s 12 -n 4

Quoted 7 lines

Rev A5B
0A5B

Rev C
000C

Just another default qwerk to add to Pantelis' bone_capemgr. ;)

It seems we should not even instantiate some devices on BBB until the EEPROM is parsed.. So maybe something like this:

1. The problem devices are initially set with status = "disabled" in the dts

2. We set up drivers/*/bbb-eeprom.c that parses the board revision at module_init time, and then flips the selected devices to have status = "enabled" and populates the revision info based on the eeprom and SoC revision passed in pdata. Then those devices get their struct device created and probed, but at a much later time.

Quoted 3 lines

So rather than trying to init all that early, let's just init them much later when we have the proper I2C driver running?

I see that working just fine. We (beagleboard.org) enforce the eeprom data, as all the official images require a proper baseboard eeprom.

We just have to be very careful to limit the scope, otherwise we will end up with Pantelis' rejected capebus from the v3.2.x days...

Quoted 29 lines · Robert Nelson

Quoted 28 lines · Tony Lindgren

Quoted 6 lines · Robert Nelson

On Mon, May 18, 2015 at 10:21 AM, Tony Lindgren <[email protected]> wrote:

All the rev information is in the board's eeprom:

hexdump -e '8/1 "%c"' /sys/bus/i2c/devices/0-0050/eeprom -s 12 -n 4

Quoted 7 lines

Rev A5B
0A5B

Rev C
000C

Just another default qwerk to add to Pantelis' bone_capemgr. ;)

It seems we should not even instantiate some devices on BBB until the EEPROM is parsed.. So maybe something like this:

1. The problem devices are initially set with status = "disabled" in the dts

2. We set up drivers/*/bbb-eeprom.c that parses the board revision at module_init time, and then flips the selected devices to have status = "enabled" and populates the revision info based on the eeprom and SoC revision passed in pdata. Then those devices get their struct device created and probed, but at a much later time.

Quoted 6 lines

Quoted 3 lines

So rather than trying to init all that early, let's just init them much later when we have the proper I2C driver running?

I see that working just fine. We (beagleboard.org) enforce the eeprom data, as all the official images require a proper baseboard eeprom.

OK

Quoted 2 lines

We just have to be very careful to limit the scope, otherwise we will end up with Pantelis' rejected capebus from the v3.2.x days...

Naturally I was thinking #2 above would use Pantelis' code for CONFIG_OF_OVERLAY in mainline. But instead of the earlier patches, we can make things happen much later on to avoid the detect of EEPROM early on.

Regards,

Tony

Hi Tony,

Quoted 41 lines · Tony Lindgren

Quoted 29 lines · Robert Nelson

Quoted 28 lines · Tony Lindgren

Quoted 6 lines · Robert Nelson

On Mon, May 18, 2015 at 10:21 AM, Tony Lindgren <[email protected]> wrote:

All the rev information is in the board's eeprom:

hexdump -e '8/1 "%c"' /sys/bus/i2c/devices/0-0050/eeprom -s 12 -n 4

Quoted 7 lines

Rev A5B
0A5B

Rev C
000C

Just another default qwerk to add to Pantelis' bone_capemgr. ;)

It seems we should not even instantiate some devices on BBB until the EEPROM is parsed.. So maybe something like this:

1. The problem devices are initially set with status = "disabled" in the dts

2. We set up drivers/*/bbb-eeprom.c that parses the board revision at module_init time, and then flips the selected devices to have status = "enabled" and populates the revision info based on the eeprom and SoC revision passed in pdata. Then those devices get their struct device created and probed, but at a much later time.

Quoted 6 lines

Quoted 3 lines

So rather than trying to init all that early, let's just init them much later when we have the proper I2C driver running?

I see that working just fine. We (beagleboard.org) enforce the eeprom data, as all the official images require a proper baseboard eeprom.

OK

Quoted 7 lines

Quoted 2 lines

We just have to be very careful to limit the scope, otherwise we will end up with Pantelis' rejected capebus from the v3.2.x days...

Naturally I was thinking #2 above would use Pantelis' code for CONFIG_OF_OVERLAY in mainline. But instead of the earlier patches, we can make things happen much later on to avoid the detect of EEPROM early on.

Already been taken care of:

https://lkml.org/lkml/2015/2/18/258

The patchset works, but it still needs some review eyeballs. There might be a rename to variants or something.

You were part of that thread too :)

I think it?s best if we go this path instead of twiddling with the status properties manually. Conceptually the idea is similar to the detection of the white/black, with the added joy of revision detection.

Ain?t hardware designers a fun bunch or what?

Quoted 55 lines · Pantelis Antoniou

Hi Tony,

Quoted 41 lines · Tony Lindgren

Quoted 29 lines · Robert Nelson

Quoted 28 lines · Tony Lindgren

Quoted 6 lines · Robert Nelson

On Mon, May 18, 2015 at 10:21 AM, Tony Lindgren <[email protected]> wrote:

All the rev information is in the board's eeprom:

hexdump -e '8/1 "%c"' /sys/bus/i2c/devices/0-0050/eeprom -s 12 -n 4

Quoted 7 lines

Rev A5B
0A5B

Rev C
000C

Just another default qwerk to add to Pantelis' bone_capemgr. ;)

It seems we should not even instantiate some devices on BBB until the EEPROM is parsed.. So maybe something like this:

1. The problem devices are initially set with status = "disabled" in the dts

2. We set up drivers/*/bbb-eeprom.c that parses the board revision at module_init time, and then flips the selected devices to have status = "enabled" and populates the revision info based on the eeprom and SoC revision passed in pdata. Then those devices get their struct device created and probed, but at a much later time.

Quoted 6 lines

Quoted 3 lines

So rather than trying to init all that early, let's just init them much later when we have the proper I2C driver running?

I see that working just fine. We (beagleboard.org) enforce the eeprom data, as all the official images require a proper baseboard eeprom.

OK

Quoted 7 lines

Quoted 2 lines

We just have to be very careful to limit the scope, otherwise we will end up with Pantelis' rejected capebus from the v3.2.x days...

Naturally I was thinking #2 above would use Pantelis' code for CONFIG_OF_OVERLAY in mainline. But instead of the earlier patches, we can make things happen much later on to avoid the detect of EEPROM early on.

Already been taken care of:

https://lkml.org/lkml/2015/2/18/258

Quoted 4 lines

The patchset works, but it still needs some review eyeballs. There might be a rename to variants or something.

You were part of that thread too :)

Right, and what I mean with #2 above is that we can make this all into a regular drivers/* device driver with no need for the early hacks in your series in arch/arm/mach-omap2/am33xx-dt-quirks.c.

Quoted 4 lines

I think it?s best if we go this path instead of twiddling with the status properties manually. Conceptually the idea is similar to the detection of the white/black, with the added joy of revision detection.

If some device drivers depend on the I2C EEPROM, we should not create the struct device entry for those until the I2C EEPROM driver has parsed the EEPROM. And there's no need to do that early, we want to do everything as late as possible. That way we have real debug console available in most cases.

Quoted 1 line

Ain?t hardware designers a fun bunch or what?

We need something equal to the MS DOS boot floppy to limit their ideas :)

Regards,

Tony

Hi Tony,

Quoted 63 lines · Tony Lindgren

Quoted 59 lines · Pantelis Antoniou

Hi Tony,

Quoted 41 lines · Tony Lindgren

Quoted 29 lines · Robert Nelson

Quoted 28 lines · Tony Lindgren

Quoted 6 lines · Robert Nelson

On Mon, May 18, 2015 at 10:21 AM, Tony Lindgren <[email protected]> wrote:

All the rev information is in the board's eeprom:

hexdump -e '8/1 "%c"' /sys/bus/i2c/devices/0-0050/eeprom -s 12 -n 4

Quoted 7 lines

Rev A5B
0A5B

Rev C
000C

Just another default qwerk to add to Pantelis' bone_capemgr. ;)

It seems we should not even instantiate some devices on BBB until the EEPROM is parsed.. So maybe something like this:

1. The problem devices are initially set with status = "disabled" in the dts

2. We set up drivers/*/bbb-eeprom.c that parses the board revision at module_init time, and then flips the selected devices to have status = "enabled" and populates the revision info based on the eeprom and SoC revision passed in pdata. Then those devices get their struct device created and probed, but at a much later time.

Quoted 6 lines

Quoted 3 lines

So rather than trying to init all that early, let's just init them much later when we have the proper I2C driver running?

I see that working just fine. We (beagleboard.org) enforce the eeprom data, as all the official images require a proper baseboard eeprom.

OK

Quoted 7 lines

Quoted 2 lines

We just have to be very careful to limit the scope, otherwise we will end up with Pantelis' rejected capebus from the v3.2.x days...

Naturally I was thinking #2 above would use Pantelis' code for CONFIG_OF_OVERLAY in mainline. But instead of the earlier patches, we can make things happen much later on to avoid the detect of EEPROM early on.

Already been taken care of:

https://lkml.org/lkml/2015/2/18/258

The patchset works, but it still needs some review eyeballs. There might be a rename to variants or something.

You were part of that thread too :)

Quoted 4 lines

Right, and what I mean with #2 above is that we can make this all into a regular drivers/* device driver with no need for the early hacks in your series in arch/arm/mach-omap2/am33xx-dt-quirks.c.

It?s going to be tough. This is touching the pmic that needs to be initialized early since a whole bunch of drivers depend on it.

Quoted 11 lines

Quoted 4 lines

I think it?s best if we go this path instead of twiddling with the status properties manually. Conceptually the idea is similar to the detection of the white/black, with the added joy of revision detection.

If some device drivers depend on the I2C EEPROM, we should not create the struct device entry for those until the I2C EEPROM driver has parsed the EEPROM. And there's no need to do that early, we want to do everything as late as possible. That way we have real debug console available in most cases.

FWIW the first patch used an early platform device, but that made things even more complicated.

If some of these depend on the SoC revision and cannot be detected based on the RTC driver revision register, that information should be be passed to the RTC driver in platform data.

Quoted 3 lines · Tony Lindgren

If some of these depend on the SoC revision and cannot be detected based on the RTC driver revision register, that information should be be passed to the RTC driver in platform data.

The SoC revision is easy enough to check via the control module as usual, but is not really the biggest issue. Even on SoC rev 1.0 RTC-only sleep still has some functionality (the RTC freezes if PMIC enters sleep-state, but that may still be preferred compared to complete loss of date and time), so I'm not even sure I'd bother with this check.

The serious problems however depend on the PCB: Entering RTC-only sleep is only properly supported on some early prototype series (pre-A6) of the BeagleBone Black. Since rev A6A (which includes all production versions) it is not supported at all.

On rev A6 of the Black, as well as (afaik) all versions of the White (if anyone cares, since they normally have SoC rev 1.0), the impact of entering RTC-only sleep is heavily dependent on external connections and hardware, most of which not really detectable. It is possible to go from "hmm, annoying standby current" to a recipe for stir-fried I/O pin simply by connecting a console cable while the device is in sleep. This is not something the kernel can foresee or prevent.

I'm pretty sure the sane thing to do here is disable RTC-only sleep by default. People who care about it can still enable it after they verified the particular combination of pcb revision, external hardware, and usage scenario is safe (or patched the hardware to make it safe, or decided they're willing to take the risk.)

It may be useful to leave some references for those who seek guidance:

1. The issues with AM335x rev 1.0 SoCs are documented in the errata.

2. Unfortunately I still have no idea why TI altered the connection diagram w.r.t. "VDDS" (and as a consequence officially defeatured RTC-only sleep) when the TPS65217C/D PMIC for AM335x + DDR3/3L memory is used. Maybe I'll still get an answer from TI on E2E, but I'm also planning to do more thorough testing this week especially w.r.t. current consumption patterns during boot and shutdown.

3. The BeagleBone regulator problem is particularly notorious among people who (try to) use the PMIC's battery power support, since the problem then also surfaces during full shutdown. It has a long discussion thread here:
https://groups.google.com/d/topic/beagleboard/7sxPePT7wkM/discussion
which includes two posts by me with fairly detailed analysis along with scope pics:
https://groups.google.com/d/msg/beagleboard/7sxPePT7wkM/3vFMPydR20IJ
https://groups.google.com/d/msg/beagleboard/7sxPePT7wkM/V1Ft-xxh0agJ