diff mbox

mmc: sdio: reset card during power_restore

Message ID BANLkTini3Oz7P3ji9rrnh75bcwf=vdyWkg@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Drake June 8, 2011, 1:36 p.m. UTC
On 8 June 2011 10:33, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Sounds like you didn't power the card off then in the driver after probe ?
>
> Can you show the diff with which you did this experiment ?

Attached. This is on top of your fix for correct driver deinit during suspend.

Test case is:
1. boot, load libertas driver
2. suspend in a way that libertas returns -ENOSYS to power down card
(this is the default)
3. resume
4. MMC layer powers up card to identify it, then powers it down, then
fails to power it up for libertas probe

If I add the reset (shown commented-out in the patch), it matches the
behaviour of other known-good codepaths and also solves the problem.

Output logs are:
bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
[   53.847863] libertas_sdio: Libertas SDIO driver
[   53.853104] libertas_sdio: Copyright Pierre Ossman
[   53.858040] mmc_power_restore_host mmc1
[   54.809019] libertas_sdio mmc1:0001:1: (unregistered net_device):
00:17:c4:a7:50:57, fw 9.70.3p36, cap 0x000003a3
[   54.825485] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter
bash-4.1# [   54.920650] udev[992]: renamed network interface wlan0 to eth0
[   54.978168] ieee80211 phy0: assoc: bss   (null) not in scan results
[   56.295225] ieee80211 phy0: assoc: bss   (null) not in scan results
[   56.337101] cfg80211: Calling CRDA for country: EU
bash-4.1# echo mem > /sys/power/state
[   65.393797] PM: Syncing filesystems ...
[   65.672372] done.
[   65.803121] Freezing user space processes ... (elapsed 0.01 seconds) done.
[   65.823048] Freezing remaining freezable tasks ... (elapsed 0.01
seconds) done.
[   65.884808] i8042 kbd 00:04: wake-up capability enabled by ACPI
[   65.891251] i8042 aux 00:03: wake-up capability disabled by ACPI
[   65.897820] ehci_hcd 0000:00:10.4: PCI INT D disabled
[   65.903294] uhci_hcd 0000:00:10.2: PCI INT C disabled
[   65.908900] uhci_hcd 0000:00:10.1: PCI INT B disabled
[   65.914431] libertas_sdio mmc1:0001:1: mmc1:0001:1: suspend: PM flags = 0x3
[   65.922077] uhci_hcd 0000:00:10.0: PCI INT A disabled
[   65.927578] viafb 0000:00:01.0: PCI INT A disabled
[   65.932646] libertas_sdio mmc1:0001:1: Suspend without wake params
-- powering down card
[   65.944166] cfg80211: Calling CRDA to update world regulatory domain
[   66.010534] mmc1: card 0001 removed
[   66.014310] sdhci-pci 0000:00:0c.0: PCI INT A disabled
[   66.030103] PM: suspend of devices complete after 186.534 msecs
[   66.110284] PM: late suspend of devices complete after 74.153 msecs
[   66.116751] ACPI: Preparing to enter system sleep state S3
[   66.122976] PM: Saving platform NVS memory
+r[   66.122976] ACPI: Low-level resume complete
[   66.122976] PM: Restoring platform NVS memory
[   66.122976] ACPI: Waking up from system sleep state S3
[   66.150047] uhci_hcd 0000:00:10.0: BAR 4: set to [io
0x8000-0x801f] (PCI address [0x8000-0x801f])
[   66.170036] uhci_hcd 0000:00:10.1: BAR 4: set to [io
0x8020-0x803f] (PCI address [0x8020-0x803f])
[   66.190035] uhci_hcd 0000:00:10.2: BAR 4: set to [io
0x8040-0x805f] (PCI address [0x8040-0x805f])
[   66.210034] ehci_hcd 0000:00:10.4: BAR 0: set to [mem
0x80003000-0x800030ff] (PCI address [0x80003000-0x800030ff])
[   66.220917] PM: early resume of devices complete after 87.797 msecs
[   66.227558] viafb 0000:00:01.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
[   66.234561] sdhci-pci 0000:00:0c.0: PCI INT A -> GSI 22 (level,
low) -> IRQ 22
[   66.241931] uhci_hcd 0000:00:10.0: PCI INT A -> GSI 20 (level, low) -> IRQ 20
[   66.249168] usb usb2: root hub lost power or was reset
[   66.260047] uhci_hcd 0000:00:10.1: PCI INT B -> GSI 22 (level, low) -> IRQ 22
[   66.260077] usb usb3: root hub lost power or was reset
[   66.260116] uhci_hcd 0000:00:10.2: PCI INT C -> GSI 21 (level, low) -> IRQ 21
[   66.260145] usb usb4: root hub lost power or was reset
[   66.260179] ehci_hcd 0000:00:10.4: PCI INT D -> GSI 23 (level, low) -> IRQ 23
[   66.260761] i8042 kbd 00:04: wake-up capability disabled by ACPI
[   66.560225] PM: resume of devices complete after 332.791 msecs
[   66.580429] Restarting tasks ... done.
[   66.650427] mmc0: mmc_rescan_try_freq: trying to init card at 400000 Hz
bash-4.1# [   66.690461] cfg80211: World regulatory domain updated:
[   66.695701] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[   66.733049] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   66.751195] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   66.759333] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   66.795426] mmc0: mmc_rescan_try_freq: trying to init card at 300000 Hz
[   66.808884] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   66.830830] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   66.865778] cfg80211: Calling CRDA to update world regulatory domain
[   66.905733] cfg80211: World regulatory domain updated:
[   66.917952] mmc0: mmc_rescan_try_freq: trying to init card at 200000 Hz
[   66.968065] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[   66.985155] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   67.015441] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   67.045234] mmc0: mmc_rescan_try_freq: trying to init card at 187500 Hz
[   67.053616] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   67.122604] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   67.151879] mmc1: mmc_rescan_try_freq: trying to init card at 400000 Hz
[   67.189830] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   67.296211] mmc1: new SDIO card at address 0001
[   67.337105] mmc_power_save_host mmc1
[   67.356088] mmc_power_restore_host mmc1
[   67.471529] CMD5 reset returned -110
[   67.477144] libertas_sdio: probe of mmc1:0001:1 failed with error -16

Comments

Ohad Ben Cohen June 8, 2011, 2:02 p.m. UTC | #1
On Wed, Jun 8, 2011 at 4:36 PM, Daniel Drake <dsd@laptop.org> wrote:
> If I add the reset (shown commented-out in the patch), it matches the
> behaviour of other known-good codepaths and also solves the problem.

Yeah, but we don't know why. And we may very well be covering a
different bug if we just take it as-is.

Don't get me wrong, I have no objection to have this if it's needed,
but we shouldn't do this just "because it works".

So I'll try to look at the logs you provided a bit more later, but the
main questions I have are:

1. How come power off+on works for you in the first time, but doesn't
work in the second time ?
2. Was the card really powered off in this 2nd-time failed scenario ?
3. what if you do a series of insmod+rmmod ? does this work ? (power
should be taken down and up every time)
4. Are you also interested in powering off the card when the wlan
interface is down (i.e. coupling the power of the card with the state
of the interface) too ? it should be easy to debug.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Drake June 8, 2011, 2:21 p.m. UTC | #2
On 8 June 2011 15:02, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> On Wed, Jun 8, 2011 at 4:36 PM, Daniel Drake <dsd@laptop.org> wrote:
>> If I add the reset (shown commented-out in the patch), it matches the
>> behaviour of other known-good codepaths and also solves the problem.
>
> Yeah, but we don't know why. And we may very well be covering a
> different bug if we just take it as-is.
>
> Don't get me wrong, I have no objection to have this if it's needed,
> but we shouldn't do this just "because it works".
>
> So I'll try to look at the logs you provided a bit more later, but the
> main questions I have are:
>
> 1. How come power off+on works for you in the first time, but doesn't
> work in the second time ?

I assume it is because a different codepath was taken in order to
power up the device the first time, before it then got powered down,
compared to the codepath that failed to power it up the 2nd time. My
efforts so far have been based around eliminating the differences in
those codepaths, and this approach seems to be successful.

> 2. Was the card really powered off in this 2nd-time failed scenario ?

How do you suggest I check that? I believe it was, since
mmc_stop_host() got called, and colleagues with proper equipment
measured the difference in power usage a while back.

> 3. what if you do a series of insmod+rmmod ? does this work ? (power
> should be taken down and up every time)

rmmod doesn't appear to take down the power.

> 4. Are you also interested in powering off the card when the wlan
> interface is down (i.e. coupling the power of the card with the state
> of the interface) too ? it should be easy to debug.

Yes, and I have a patch ready that does that. But first it depends on
getting the power on/off routines working reliably.

Thanks for your help.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ohad Ben Cohen June 8, 2011, 8:05 p.m. UTC | #3
On Wed, Jun 8, 2011 at 5:21 PM, Daniel Drake <dsd@laptop.org> wrote:
>> 1. How come power off+on works for you in the first time, but doesn't
>> work in the second time ?
>
> I assume it is because a different codepath was taken in order to
> power up the device the first time, before it then got powered down,
> compared to the codepath that failed to power it up the 2nd time. My
> efforts so far have been based around eliminating the differences in
> those codepaths, and this approach seems to be successful.

We have 3 power-on paths here: on boot, on insmod, and and resume.

I understand the first one succeeds (naturally), and the last one
failed, but now I'm confused what was the outcome of the 2nd (without
sending a reset cmd) ?

>> 3. what if you do a series of insmod+rmmod ? does this work ? (power
>> should be taken down and up every time)
>
> rmmod doesn't appear to take down the power.

Let's nail this one first. if we get it right, the rest will immediately follow.

Please reboot, and immediately after booting (without insmoding the
driver) tell me what's the output of :

mount -t debugfs none /sys/kernel/debug
cat /sys/kernel/debug/mmc1/ios

Then insmod the driver, and tell me again what's the output of
/sys/.../mmc1/ios.

Please send a contiguous terminal session of these steps (incl. the
boot messages), thanks !
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Drake June 8, 2011, 8:58 p.m. UTC | #4
On 8 June 2011 21:05, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> We have 3 power-on paths here: on boot, on insmod, and and resume.
>
> I understand the first one succeeds (naturally), and the last one
> failed, but now I'm confused what was the outcome of the 2nd (without
> sending a reset cmd) ?

Not really sure what you're asking. Unless its the test below?

>>> 3. what if you do a series of insmod+rmmod ? does this work ? (power
>>> should be taken down and up every time)
>>
>> rmmod doesn't appear to take down the power.
>
> Let's nail this one first. if we get it right, the rest will immediately follow.
>
> Please reboot, and immediately after booting (without insmoding the
> driver) tell me what's the output of :
>
> mount -t debugfs none /sys/kernel/debug
> cat /sys/kernel/debug/mmc1/ios
>
> Then insmod the driver, and tell me again what's the output of
> /sys/.../mmc1/ios.

Which base kernel setup should I run these tests on?

Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Drake June 9, 2011, 3:51 p.m. UTC | #5
On 8 June 2011 21:05, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Please reboot, and immediately after booting (without insmoding the
> driver) tell me what's the output of :
>
> mount -t debugfs none /sys/kernel/debug
> cat /sys/kernel/debug/mmc1/ios
>
> Then insmod the driver, and tell me again what's the output of
> /sys/.../mmc1/ios.

bash-4.1# mount -t debugfs none /sys/kernel/debug
bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		0 Hz
vdd:		0 (invalid)
bus mode:	1 (open drain)
chip select:	0 (don't care)
power mode:	0 (off)
bus width:	0 (1 bits)
timing spec:	0 (legacy)
bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
[   67.917748] libertas_sdio: Libertas SDIO driver
[   67.922976] libertas_sdio: Copyright Pierre Ossman
[   67.972073] libertas_sdio: probe of mmc1:0001:1 failed with error -16
bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		400000 Hz
vdd:		21 (3.3 ~ 3.4 V)
bus mode:	1 (open drain)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	0 (1 bits)
timing spec:	0 (legacy)

> Please send a contiguous terminal session of these steps (incl. the
> boot messages), thanks !

Full log: http://dev.laptop.org/~dsd/20110609/sd-pwr-debug1.txt

Thanks
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ohad Ben Cohen June 9, 2011, 3:59 p.m. UTC | #6
On Thu, Jun 9, 2011 at 6:51 PM, Daniel Drake <dsd@laptop.org> wrote:
> bash-4.1# mount -t debugfs none /sys/kernel/debug
> bash-4.1# cat /sys/kernel/debug/mmc1/ios
> clock:          0 Hz
> vdd:            0 (invalid)
> bus mode:       1 (open drain)
> chip select:    0 (don't care)
> power mode:     0 (off)
> bus width:      0 (1 bits)
> timing spec:    0 (legacy)
> bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
> [   67.917748] libertas_sdio: Libertas SDIO driver
> [   67.922976] libertas_sdio: Copyright Pierre Ossman
> [   67.972073] libertas_sdio: probe of mmc1:0001:1 failed with error -16
> bash-4.1# cat /sys/kernel/debug/mmc1/ios
> clock:          400000 Hz
> vdd:            21 (3.3 ~ 3.4 V)
> bus mode:       1 (open drain)
> chip select:    0 (don't care)
> power mode:     2 (on)
> bus width:      0 (1 bits)
> timing spec:    0 (legacy)

Ok, cool.

Now can you please repeat this, but this time add your original patch
(which only added the CMD5 arg=0 cmd, no sdio reset yet) ?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Drake June 9, 2011, 4:21 p.m. UTC | #7
On 9 June 2011 16:59, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Now can you please repeat this, but this time add your original patch
> (which only added the CMD5 arg=0 cmd, no sdio reset yet) ?

With this version of the patch:
http://dev.laptop.org/~dsd/20110609/sd-pwr-debug2.patch

bash-4.1# mount -t debugfs none /sys/kernel/debug
bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		0 Hz
vdd:		0 (invalid)
bus mode:	1 (open drain)
chip select:	0 (don't care)
power mode:	0 (off)
bus width:	0 (1 bits)
timing spec:	0 (legacy)
bash-4.1#
bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
[   53.917466] libertas_sdio: Libertas SDIO driver
[   53.922718] libertas_sdio: Copyright Pierre Ossman
[   54.839032] libertas_sdio mmc1:0001:1: (unregistered net_device):
00:17:c4:a7:50:57, fw 9.70.3p36, cap 0x000003a3
[   54.855479] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter
bash-4.1# [   54.941099] udev[985]: renamed network interface wlan0 to eth0
[   54.997656] ieee80211 phy0: assoc: bss   (null) not in scan results
[   56.310846] ieee80211 phy0: assoc: bss   (null) not in scan results
[   56.360840] cfg80211: Calling CRDA for country: EU

bash-4.1#
bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		25000000 Hz
vdd:		20 (3.2 ~ 3.3 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	0 (legacy)

Full log: http://dev.laptop.org/~dsd/20110609/sd-pwr-debug2.txt

Note that the patch includes the mmc_select_voltage() call that I was
originally unsure about, and later did decide that equivalent code was
already being called elsewhere, thats why it was removed from the most
recent revision of the patch.



With a version of the patch that just does the reset, the post-powerup
"vdd" figure does change:
http://dev.laptop.org/~dsd/20110609/sd-pwr-debug3.patch

bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		0 Hz
vdd:		0 (invalid)
bus mode:	1 (open drain)
chip select:	0 (don't care)
power mode:	0 (off)
bus width:	0 (1 bits)
timing spec:	0 (legacy)
bash-4.1#
bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
[   64.947624] libertas_sdio: Libertas SDIO driver
[   64.952866] libertas_sdio: Copyright Pierre Ossman
[   65.878171] libertas_sdio mmc1:0001:1: (unregistered net_device):
00:17:c4:a7:50:57, fw 9.70.3p36, cap 0x000003a3
[   65.896642] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter
bash-4.1# [   65.991205] udev[984]: renamed network interface wlan0 to eth0
[   66.048887] ieee80211 phy0: assoc: bss   (null) not in scan results
[   67.373572] ieee80211 phy0: assoc: bss   (null) not in scan results
[   67.423422] cfg80211: Calling CRDA for country: EU

bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		25000000 Hz
vdd:		21 (3.3 ~ 3.4 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	0 (legacy)

Full log: http://dev.laptop.org/~dsd/20110609/sd-pwr-debug3.txt



For reference, here is the equivalent test performed without runtime
PM enabled (i.e. all changes reverted)

bash-4.1# mount -t debugfs none /sys/kernel/debug
bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		25000000 Hz
vdd:		20 (3.2 ~ 3.3 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	0 (legacy)
bash-4.1#
bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
[   69.067761] libertas_sdio: Libertas SDIO driver
[   69.072995] libertas_sdio: Copyright Pierre Ossman
[   69.919033] libertas_sdio mmc1:0001:1: (unregistered net_device):
00:17:c4:a7:50:57, fw 9.70.3p36, cap 0x000003a3
[   69.935429] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter
[   70.021111] udev[991]: renamed network interface wlan0 to eth0
[   70.075870] ieee80211 phy0: assoc: bss   (null) not in scan results
[   71.460776] ieee80211 phy0: assoc: bss   (null) not in scan results
[   71.509660] cfg80211: Calling CRDA for country: EU
bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		25000000 Hz
vdd:		20 (3.2 ~ 3.3 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	0 (legacy)

Full log: http://dev.laptop.org/~dsd/20110609/sd-pwr-debug0.txt


What next?

Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ohad Ben Cohen June 9, 2011, 4:30 p.m. UTC | #8
On Thu, Jun 9, 2011 at 7:21 PM, Daniel Drake <dsd@laptop.org> wrote:
> With this version of the patch:
> http://dev.laptop.org/~dsd/20110609/sd-pwr-debug2.patch
>
> bash-4.1# mount -t debugfs none /sys/kernel/debug
> bash-4.1# cat /sys/kernel/debug/mmc1/ios
> clock:          0 Hz
> vdd:            0 (invalid)
> bus mode:       1 (open drain)
> chip select:    0 (don't care)
> power mode:     0 (off)
> bus width:      0 (1 bits)
> timing spec:    0 (legacy)
> bash-4.1#
> bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
> [   53.917466] libertas_sdio: Libertas SDIO driver
> [   53.922718] libertas_sdio: Copyright Pierre Ossman
> [   54.839032] libertas_sdio mmc1:0001:1: (unregistered net_device):
> 00:17:c4:a7:50:57, fw 9.70.3p36, cap 0x000003a3
> [   54.855479] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter
> bash-4.1# [   54.941099] udev[985]: renamed network interface wlan0 to eth0
> [   54.997656] ieee80211 phy0: assoc: bss   (null) not in scan results
> [   56.310846] ieee80211 phy0: assoc: bss   (null) not in scan results
> [   56.360840] cfg80211: Calling CRDA for country: EU

Looks good.

At this point everything works ?
(can you bring up the interface and scan/connect)

Can you now do a series of insmod-rmmod-insmod.. and see if things
always work (with no runtime pm errors) after you insmod ?

If yes, we're good.

> Note that the patch includes the mmc_select_voltage() call

Good, keep that one please.

> With a version of the patch that just does the reset, the post-powerup
> "vdd" figure does change:
> http://dev.laptop.org/~dsd/20110609/sd-pwr-debug3.patch
...
> bash-4.1# cat /sys/kernel/debug/mmc1/ios
> clock:          25000000 Hz
> vdd:            21 (3.3 ~ 3.4 V)

Without even looking exactly why it happens, it doesn't look too good.
I don't see a reason to stick to that version. Let's use the
mmc_select_voltage.

>
> For reference, here is the equivalent test performed without runtime
> PM enabled (i.e. all changes reverted)

you mean runtime PM disabled, right ?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Drake June 9, 2011, 4:44 p.m. UTC | #9
On 9 June 2011 17:30, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Looks good.
>
> At this point everything works ?
> (can you bring up the interface and scan/connect)

Yes.

> Can you now do a series of insmod-rmmod-insmod.. and see if things
> always work (with no runtime pm errors) after you insmod ?
>
> If yes, we're good.

See below.

>> Note that the patch includes the mmc_select_voltage() call
>
> Good, keep that one please.

OK

>> For reference, here is the equivalent test performed without runtime
>> PM enabled (i.e. all changes reverted)
>
> you mean runtime PM disabled, right ?

Yes.


Latest test, as suggested, based on this patch:
http://dev.laptop.org/~dsd/20110609/sd-pwr-debug4.patch
Note that I added in printk's to show when mmc_power_save_host() and
mmc_power_restore_host() get called.

bash-4.1# mount -t debugfs none /sys/kernel/debug
bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		0 Hz
vdd:		0 (invalid)
bus mode:	1 (open drain)
chip select:	0 (don't care)
power mode:	0 (off)
bus width:	0 (1 bits)
timing spec:	0 (legacy)
bash-4.1#
bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
[   82.537594] libertas_sdio: Libertas SDIO driver
[   82.542838] libertas_sdio: Copyright Pierre Ossman
[   82.547792] mmc_power_restore_host mmc1
[   83.478197] libertas_sdio mmc1:0001:1: (unregistered net_device):
00:17:c4:a7:50:57, fw 9.70.3p36, cap 0x000003a3
[   83.496623] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter
bash-4.1# [   83.581679] udev[987]: renamed network interface wlan0 to eth0
[   83.635749] ieee80211 phy0: assoc: bss   (null) not in scan results
[   85.032955] ieee80211 phy0: assoc: bss   (null) not in scan results
[   85.087132] cfg80211: Calling CRDA for country: EU

bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		25000000 Hz
vdd:		20 (3.2 ~ 3.3 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	0 (legacy)
bash-4.1#
bash-4.1# rmmod libertas_sdio
[   92.782496] cfg80211: Calling CRDA to update world regulatory domain
[   92.805661] cfg80211: World regulatory domain updated:
[   92.825864] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[   92.837401] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   92.845708] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   92.854000] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   92.865497] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   92.877042] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   92.885562] cfg80211: Calling CRDA to update world regulatory domain
[   92.905832] cfg80211: World regulatory domain updated:
[   92.916165] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[   92.928171] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   92.939651] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   92.950209] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   92.958179] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   92.971593] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
bash-4.1#
bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
[  104.187988] libertas_sdio: Libertas SDIO driver
[  104.192840] libertas_sdio: Copyright Pierre Ossman
[  104.201237] libertas_sdio mmc1:0001:1: (unregistered net_device):
00:17:c4:a7:50:57, fw 9.70.3p36, cap 0x000003a3
[  104.230342] cfg80211: Calling CRDA for country: EU
[  104.238936] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter
[  104.320479] udev[2114]: renamed network interface wlan0 to eth0
[  104.366506] ieee80211 phy1: assoc: bss   (null) not in scan results
[  105.703988] ieee80211 phy1: assoc: bss   (null) not in scan results
[  105.746816] cfg80211: Calling CRDA to update world regulatory domain
[  105.780078] cfg80211: World regulatory domain updated:
[  105.800084] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[  105.808555] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  105.850076] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  105.871870] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  105.879887] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  105.900256] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)

bash-4.1# rmmod libertas_sdio
[  109.652081] cfg80211: Calling CRDA to update world regulatory domain
[  109.673075] cfg80211: World regulatory domain updated:
[  109.678279] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[  109.692581] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  109.713624] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  109.724811] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  109.733089] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  109.741521] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)

bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
[  112.378005] libertas_sdio: Libertas SDIO driver
[  112.382826] libertas_sdio: Copyright Pierre Ossman
[  112.391190] libertas_sdio mmc1:0001:1: (unregistered net_device):
00:17:c4:a7:50:57, fw 9.70.3p36, cap 0x000003a3
[  112.420334] cfg80211: Calling CRDA for country: EU
[  112.428883] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter
bash-4.1# [  112.510471] udev[2302]: renamed network interface wlan0 to eth0
[  112.556438] ieee80211 phy2: assoc: bss   (null) not in scan results
[  113.915702] ieee80211 phy2: assoc: bss   (null) not in scan results
[  113.955631] cfg80211: Calling CRDA to update world regulatory domain
[  113.993444] cfg80211: World regulatory domain updated:
[  113.998631] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[  114.025508] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  114.052885] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  114.070131] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  114.078125] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  114.102197] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)

bash-4.1# rmmod libertas_sdio
[  118.272469] cfg80211: Calling CRDA to update world regulatory domain
[  118.296325] cfg80211: World regulatory domain updated:
[  118.315555] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[  118.324822] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  118.333074] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  118.341331] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  118.349327] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  118.360932] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)

bash-4.1# insmod /media/4E11-1D7C/libertas_sdio.ko
[  120.557985] libertas_sdio: Libertas SDIO driver
[  120.562817] libertas_sdio: Copyright Pierre Ossman
[  120.572314] libertas_sdio mmc1:0001:1: (unregistered net_device):
00:17:c4:a7:50:57, fw 9.70.3p36, cap 0x000003a3
[  120.595998] cfg80211: Calling CRDA for country: EU
[  120.605524] libertas_sdio mmc1:0001:1: wlan0: Marvell WLAN 802.11 adapter
[  120.680466] udev[2474]: renamed network interface wlan0 to eth0
[  120.750792] ieee80211 phy3: assoc: bss   (null) not in scan results
[  121.966221] ieee80211 phy3: assoc: bss   (null) not in scan results
[  122.006462] cfg80211: Calling CRDA to update world regulatory domain
[  122.042731] cfg80211: World regulatory domain updated:
[  122.047938] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[  122.081694] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  122.089714] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  122.116718] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[  122.130095] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[  122.138125] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)

bash-4.1# cat /sys/kernel/debug/mmc1/ios
clock:		25000000 Hz
vdd:		20 (3.2 ~ 3.3 V)
bus mode:	2 (push-pull)
chip select:	0 (don't care)
power mode:	2 (on)
bus width:	2 (4 bits)
timing spec:	0 (legacy)




Full log: http://dev.laptop.org/~dsd/20110609/sd-pwr-debug4.txt

Note that during the insmod/rmmod calls, runtime PM did not touch the
power state - the card remained powered ever since the first insmod.
Not sure if this is in-line with your expectations.

Thanks!
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 68091dd..92405e2 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1662,7 +1662,7 @@  void mmc_stop_host(struct mmc_host *host)
 int mmc_power_save_host(struct mmc_host *host)
 {
 	int ret = 0;
-
+printk("mmc_power_save_host %s\n", mmc_hostname(host));
 	mmc_bus_get(host);
 
 	if (!host->bus_ops || host->bus_dead || !host->bus_ops->power_restore) {
@@ -1685,6 +1685,7 @@  int mmc_power_restore_host(struct mmc_host *host)
 {
 	int ret;
 
+printk("mmc_power_restore_host %s\n", mmc_hostname(host));
 	mmc_bus_get(host);
 
 	if (!host->bus_ops || host->bus_dead || !host->bus_ops->power_restore) {
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 8af3330..61ee53a 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -706,10 +706,27 @@  static int mmc_sdio_power_restore(struct mmc_host *host)
 	BUG_ON(!host->card);
 
 	mmc_claim_host(host);
+
+	/*
+	 * Reset the card by performing the same steps that are taken by
+	 * mmc_rescan_try_freq() and mmc_attach_sdio() during a "normal" probe
+	 */
+	//sdio_reset(host);
+	//mmc_go_idle(host);
+	//mmc_send_if_cond(host, host->ocr_avail);
+
+	ret = mmc_send_io_op_cond(host, 0, NULL);
+	if (ret) {
+		printk("CMD5 reset returned %d\n", ret);
+		//goto out;
+	}
+
 	ret = mmc_sdio_init_card(host, host->ocr, host->card,
 				mmc_card_keep_power(host));
 	if (!ret && host->sdio_irqs)
 		mmc_signal_sdio_irq(host);
+
+out:
 	mmc_release_host(host);
 
 	return ret;
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 58d5436..ce3e2e2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2488,7 +2488,7 @@  int sdhci_add_host(struct sdhci_host *host)
 	} else
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
-	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
+	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23 | MMC_CAP_POWER_OFF_CARD;
 
 	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
 		host->flags |= SDHCI_AUTO_CMD12;