Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Property mbox set_power_state fails for SDHCI, get_power_state says wrong thing too #106

Open
swarren opened this Issue Oct 21, 2012 · 8 comments

Comments

Projects
None yet
3 participants

swarren commented Oct 21, 2012

I'm working on upstreaming SDHCI support for U-Boot. I've added code to ensure that the SDHCI power is enabled. However, the property mailbox messages aren't working for this:

mbox: TX buffer
0x0000: 0x00000020
0x0004: 0x00000000
0x0008: 0x00028001
0x000c: 0x00000008
0x0010: 0x00000008
0x0014: 0x00000000
0x0018: 0x00000003
0x001c: 0x00000000
mbox: TX raw: 0x0db8ded8
mbox: RX raw: 0x0db8ded8
mbox: RX buffer
0x0000: 0x00000020
0x0004: 0x80000001 <-- overall error
0x0008: 0x00028001
0x000c: 0x00000008
0x0010: 0x80000008 <-- even though this indicates "response", ...
0x0014: 0x00000000 <-- presumably these simply aren't updated
0x0018: 0x00000003
0x001c: 0x00000000
mbox: Header response code invalid
bcm2835: Could not set SDHCI power state

After finding set_power_state didn't work, I tried get_power_state first, to see if the firmware thought the power was already on, and hence wouldn't allow an idempotent "turn it on" request:

mbox: TX buffer
0x0000: 0x00000020
0x0004: 0x00000000
0x0008: 0x00020001
0x000c: 0x00000008
0x0010: 0x00000004
0x0014: 0x00000000
0x0018: 0x00000000
0x001c: 0x00000000
time: 4111338 timeout: 4211338
mbox: TX raw: 0x0db8df08
mbox: RX raw: 0x0db8df08
mbox: RX buffer
0x0000: 0x00000020
0x0004: 0x80000000 <-- query went OK
0x0008: 0x00020001
0x000c: 0x00000008
0x0010: 0x80000008 <-- response flag set
0x0014: 0x00000000 <-- state is off, "device doesn't exist" is not set
0x0018: 0x00000000
0x001c: 0x00000000
SDHCI power state: OFF

Any ideas what's up? I noticed this with both the following two firmware commits:

c58f722 Add gpu_mem_256 and gpu_mem_512 config options to bootcode.bin

c57ea9d Remove plethora of start.elf files.

(I tried the older commit first, and when that didn't work, noticed there was a new one, so tried that - no change in behaviour).

swarren commented Oct 21, 2012

Sorry, the older commit I tried was

48f8bb0 Add memory splits for 512M board

not:

c57ea9d Remove plethora of start.elf files

swarren commented Oct 26, 2012

The set power state issue may be just that the comand is returning the wrong response status; after I have executed set power state (even with error response), get power state appears to return a state that matches what I attempted to set:

// Power off SD:
U-Boot> setenv tag 0x28001 ; setenv p1 0 ; setenv p2 0; run prop-2-2 ; run send-rec
00001000: 00000020 00000000 00028001 00000008 ...............
00001010: 00000008 00000000 00000000 00000000 ................
2000b880: 00001008 ....
00001000: 00000020 80000001 00028001 00000008 ...............
00001010: 80000008 00000000 00000000 00000000 ................

// Query SD state:
U-Boot> setenv tag 0x20001 ; setenv p1 0 ; run prop-1-2 ; run send-rec
00001000: 00000020 00000000 00020001 00000008 ...............
00001010: 00000004 00000000 00000000 00000000 ................
2000b880: 00001008 ....
00001000: 00000020 80000000 00020001 00000008 ...............
00001010: 80000008 00000000 00000000 00000000 ................
// 4th column above (data at 101c) == 0 -> off

// Power on SD, wait:
U-Boot> setenv tag 0x28001 ; setenv p1 0 ; setenv p2 3; run prop-2-2 ; run send-rec
00001000: 00000020 00000000 00028001 00000008 ...............
00001010: 00000008 00000000 00000003 00000000 ................
2000b880: 00001008 ....
00001000: 00000020 80000001 00028001 00000008 ...............
00001010: 80000008 00000000 00000003 00000000 ................

// Query SD state:
U-Boot> setenv tag 0x20001 ; setenv p1 0 ; run prop-1-2 ; run send-rec
00001000: 00000020 00000000 00020001 00000008 ...............
00001010: 00000004 00000000 00000000 00000000 ................
2000b880: 00001008 ....
00001000: 00000020 80000000 00020001 00000008 ...............
00001010: 80000008 00000000 00000001 00000000 ................
// 4th column above (data at 101c) == 1 -> on

// Power off SD:
U-Boot> setenv tag 0x28001 ; setenv p1 0 ; setenv p2 0; run prop-2-2 ; run send-rec
00001000: 00000020 00000000 00028001 00000008 ...............
00001010: 00000008 00000000 00000000 00000000 ................
2000b880: 00001008 ....
00001000: 00000020 80000001 00028001 00000008 ...............
00001010: 80000008 00000000 00000000 00000000 ................

// Query SD state:
U-Boot> setenv tag 0x20001 ; setenv p1 0 ; run prop-1-2 ; run send-rec
00001000: 00000020 00000000 00020001 00000008 ...............
00001010: 00000004 00000000 00000000 00000000 ................
2000b880: 00001008 ....
00001000: 00000020 80000000 00020001 00000008 ...............
00001010: 80000008 00000000 00000000 00000000 ................
// 4th column above (data at 101c) == 0 -> off

That said, it's still odd why get power state for SD says off before I execute any commands. I wonder if set/get power state maintain their own state variable rather than querying the HW, and set power state is updating just the variable, failing to program the HW (hence returning an error), but forgetting to update its state variable again in the error case?

I did validate that get power state's response bit 1 (device doesn't exist) does work, and kicks in when I request power state for device 9.

Contributor

popcornmix commented Oct 26, 2012

Ah, yes I see a typo in the SET_POWER_STATE message. I'll push out a fix soon.

Contributor

popcornmix commented Oct 28, 2012

Update to firmware with SET_POWER_STATE fix is pushed.

swarren commented Oct 30, 2012

OK, that does solve the error response from SET_POWER. Thanks.

However, GET_POWER still returns a value that's presumably incorrect unless SET_POWER was run first.

Ruffio commented Jun 24, 2015

@swarren can this issue be closed?

swarren commented Jul 9, 2015

AFAIK the issue[1] is still present; is there reason to believe it's been fixed?

[1] However, GET_POWER still returns a value that's presumably incorrect unless SET_POWER was run first.

Ruffio commented Mar 24, 2016

@popcornmix is this easy fixable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment