| Message ID | BANLkTinG-CmEh9yXL9Sp+W7PyatvEdOhZQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 9 June 2011 19:25, Ohad Ben-Cohen <ohad@wizery.com> wrote: > let's update that patch. I'd send you an updated one, but I have to go > for awhile, so here's the quick change you need to do. Unfortunately it doesn't help. New patch: http://dev.laptop.org/~dsd/20110609/sd-pwr-debug6.patch During resume, this happens: [ 152.156215] mmc1: new SDIO card at address 0001 [ 152.198193] mmc_power_save_host mmc1 [ 152.212397] mmc_power_restore_host mmc1 [ 152.311567] CMD5 reset failed, err=-110 [ 152.315513] 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) (I had forgotten to include final ios output in my last test, sorry about that) Full log: http://dev.laptop.org/~dsd/20110609/sd-pwr-debug6.txt What next? Is it concerning that ios ends up running at the wrong frequency and voltage? 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
On Thu, Jun 9, 2011 at 10:55 PM, Daniel Drake <dsd@laptop.org> wrote: > On 9 June 2011 19:25, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> let's update that patch. I'd send you an updated one, but I have to go >> for awhile, so here's the quick change you need to do. > > Unfortunately it doesn't help. That's ok. Let's go back to small steps now. We have cyclic rmmod/insmod working, and that's good: it means there's no hw issue. The next step to have working now is really the ifconfig up/down/up scenario. That should come before suspend/resume. And this step-by-step approach is even more relevant to libertas_sdio, because I suspect there's some if_sdio.c refactoring needed to get it right, but let's see. The expected result you want to get is: - boot, power is off - insmod, power is still off - ifconfig up, power is on - ifconfig down or rmmod, power is back off -- 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
On 10 June 2011 00:27, Ohad Ben-Cohen <ohad@wizery.com> wrote: > The next step to have working now is really the ifconfig up/down/up > scenario. That should come before suspend/resume. And this > step-by-step approach is even more relevant to libertas_sdio, because > I suspect there's some if_sdio.c refactoring needed to get it right, > but let's see. Done. Without bringing suspend/resume into the picture, this is working fine. Power is removed when the interface goes down, and is restored when the interface is brought up. ios values look fine at every stage. Wireless card works fine after several power cycles. Patches here: http://dev.laptop.org/~dsd/20110610/ The most relevant ones for this discussion are 6 and 7. What next? 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
On Fri, Jun 10, 2011 at 7:15 PM, Daniel Drake <dsd@laptop.org> wrote: > Done. Without bringing suspend/resume into the picture, this is > working fine. Power is removed when the interface goes down, and is > restored when the interface is brought up. ios values look fine at > every stage. Wireless card works fine after several power cycles. That sounds great. > Patches here: http://dev.laptop.org/~dsd/20110610/ > The most relevant ones for this discussion are 6 and 7. > > What next? We need to debug the suspend/resume path. Now that we have the other runtime pm paths working, we can pretty much tell there's no hw issue at hand. I definitely plan to help you nail this, but I'm a little tied up with some tight schedule for a little while. -- 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
On 13 June 2011 20:52, Ohad Ben-Cohen <ohad@wizery.com> wrote: > We need to debug the suspend/resume path. Now that we have the other > runtime pm paths working, we can pretty much tell there's no hw issue > at hand. Found it. It's a timing issue. In our other tests we had not yet hit the case when power is removed then immediately restored. However, during resume, the card is powered up, powered down, powered up, and then probed by libertas. A msleep(250) is needed during power_restore. Then the reset works fine. Why is there so much power flipping going on during resume? Is this a bug? Shouldn't it power it up, realise it has a driver already loaded, and go straight into probe? But even if that gets fixed, we still need to fix the case where the network interface is brought down then up immediately (another way to trigger the issue). Would you suggest a card quirk for that? Adding another 250ms to the already-slow libertas powerup routine would be a bit painful, would you support the added complexity needed to make the 250ms delay only occur when >250ms has passed since it was powered off? 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
Seeing something similar but the details are a little different. My kernel is somewhat old but ... Seeing this with sd card and android. card is removed/inserted/removed/inserted etc very quickly during power operations. After 20-30 times a problem occurs. this can be anything from file system errors, partition not recognized or a kernel hang. My thought is this is not a quirk issue but power down needs to be delayed during the power up sequence until processing has been completed in the mmc layer. Any thoughts on how to debug this would be welcome. When the hang happens the system is not dead (as in console output showing card insert/remove still happens but console is locked etc. thanks, Philip I do not think a quirk is the answer. On Jun 16, 2011, at 10:27 AM, Daniel Drake wrote: > On 13 June 2011 20:52, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> We need to debug the suspend/resume path. Now that we have the other >> runtime pm paths working, we can pretty much tell there's no hw issue >> at hand. > > Found it. It's a timing issue. In our other tests we had not yet hit > the case when power is removed then immediately restored. However, > during resume, the card is powered up, powered down, powered up, and > then probed by libertas. > > A msleep(250) is needed during power_restore. Then the reset works fine. > > Why is there so much power flipping going on during resume? Is this a > bug? Shouldn't it power it up, realise it has a driver already loaded, > and go straight into probe? > > But even if that gets fixed, we still need to fix the case where the > network interface is brought down then up immediately (another way to > trigger the issue). Would you suggest a card quirk for that? Adding > another 250ms to the already-slow libertas powerup routine would be a > bit painful, would you support the added complexity needed to make the > 250ms delay only occur when >250ms has passed since it was powered > off? > > 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 -- 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
On Thu, Jun 16, 2011 at 8:27 PM, Daniel Drake <dsd@laptop.org> wrote: > Why is there so much power flipping going on during resume? There isn't. You see this with libertas, because it doesn't really suspend/resume cleanly. Instead, it uses a pretty awkward (in my opinion) mechanism to remove the card on suspend, and then let the system re-probe it on resume. > But even if that gets fixed, we still need to fix the case where the > network interface is brought down then up immediately (another way to > trigger the issue). Could you also trigger this with the previous power off/on mechanism you used (an rfkill driver IIRC) ? if not, what's the difference ? did you have just enough delay there which didn't trigger this problem ? > Would you suggest a card quirk for that? Adding > another 250ms to the already-slow libertas powerup routine would be a > bit painful, would you support the added complexity needed to make the > 250ms delay only occur when >250ms has passed since it was powered > off? First, I suggest to understand where does this requirement come from. Is this a rigid hardware requirement ? Does it always require waiting 250ms before powering it on again ? If yes, then why not let the relevant regulator take care of it ? this way you never have to care. you just power it off and on as you please, and things just work. -- 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
On 16 June 2011 22:22, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Thu, Jun 16, 2011 at 8:27 PM, Daniel Drake <dsd@laptop.org> wrote: >> Why is there so much power flipping going on during resume? > > There isn't. > > You see this with libertas, because it doesn't really suspend/resume cleanly. It's not related to suspend/resume. If I build libertas_sdio into the kernel I see the same thing happening during boot: http://dev.laptop.org/~dsd/20110617/dmesg.txt [ 3.023445] mmc1: new SDIO card at address 0001 [ 3.039597] mmc_power_save_host mmc1 [ 3.054897] mmc_power_restore_host mmc1 [ 3.409482] CMD5 reset failed, err=-110 [ 3.427733] libertas_sdio: probe of mmc1:0001:1 failed with error -16 Ignore the probe error - just observe the fact that the card was detected and powered on, powered off, powered on and *then* probed. That's the same thing that happens in the resume path. Couldn't the power have just been maintained from when the card was detected, given that the driver was loaded and ready to go? > Could you also trigger this with the previous power off/on mechanism > you used (an rfkill driver IIRC) ? if not, what's the difference ? did > you have just enough delay there which didn't trigger this problem ? No, it couldn't be triggered. This is because our rfkill driver used the "normal" MMC card powerup routine (which runtime PM doesn't use) which unconditionally calls sdio_reset(). Thinking more, I think I prefer sdio_reset() as a quirk/workaround instead of the sleep, because the sleep is quite long. What do you think? > First, I suggest to understand where does this requirement come from. > Is this a rigid hardware requirement ? Does it always require waiting > 250ms before powering it on again ? Any suggestions for how I go about doing that? Unfortunately we have a backlog of support requests with Marvell which have not yet been responded to. > If yes, then why not let the relevant regulator take care of it ? this > way you never have to care. you just power it off and on as you > please, and things just work. Yes, a regulator could work. But, aren't regulators linked to the platform rather than to the card? I would imagine that this quirk needs to be applied to all users of the card, therefore a card quirk could be more appropriate? Also, if you think the sdio_reset() option is preferable to the sleep, can that be done from a regulator? 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
On Fri, Jun 17, 2011 at 4:58 PM, Daniel Drake <dsd@laptop.org> wrote: > Ignore the probe error - just observe the fact that the card was > detected and powered on, powered off, powered on and *then* probed. This is the normal behavior today when drivers are probed: we power up the card before probing a driver, and if the probe fails, we power it off. To be exact, we're not even doing that deterministically. We are powering off the *SDIO function*, and not the card. I once changed this to be deterministic, but didn't pursue this eventually, because frankly it's not hugely important. > That's the same thing that happens in the resume path. You mean on *libertas'* resume path, which doesn't really do suspend/resume - it just removes the card, and let it be re-probed later. > Couldn't the > power have just been maintained from when the card was detected, given > that the driver was loaded and ready to go? Most cards wouldn't care too much: we're not loading/unloading drivers too frequently, and the advantage of the current solution is simplicity. But you are welcome to change this if you need to. I don't see how it can hurt anyone, and anyway it can also happen today too in some cases. > Thinking more, I think I prefer sdio_reset() as a quirk/workaround > instead of the sleep, because the sleep is quite long. What do you > think? I agree that a 250ms delay on the power up path is way too long. But let's first try to understand what's the real problem it is solving before picking a solution. This usually pays off, and eventually you'd be happier with the end result. > Any suggestions for how I go about doing that? Unfortunately we have a > backlog of support requests with Marvell which have not yet been > responded to. Use the mailing list :) The Marvell guys were very nice and replied my questions pretty fast. > Yes, a regulator could work. But, aren't regulators linked to the > platform rather than to the card? I would imagine that this quirk > needs to be applied to all users of the card, therefore a card quirk > could be more appropriate? Also, if you think the sdio_reset() option > is preferable to the sleep, can that be done from a regulator? I'd really prefer to understand what's wrong first. If we have a rigid hardware requirement that demands waiting before powering it up again, then we better obey it. But I somehow doubt that a 250ms delay is required. That's way too much... 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
On 17 June 2011 15:31, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Fri, Jun 17, 2011 at 4:58 PM, Daniel Drake <dsd@laptop.org> wrote: >> Ignore the probe error - just observe the fact that the card was >> detected and powered on, powered off, powered on and *then* probed. > > This is the normal behavior today when drivers are probed: we power up > the card before probing a driver, and if the probe fails, we power it > off. The power off happens before the libertas-level probe starts. Is that still in line with your expectations? [ 3.023445] mmc1: new SDIO card at address 0001 [ 3.039597] mmc_power_save_host mmc1 [ 3.054897] mmc_power_restore_host mmc1 <now libertas probe starts> > Most cards wouldn't care too much: we're not loading/unloading drivers > too frequently, and the advantage of the current solution is > simplicity. > > But you are welcome to change this if you need to. I don't see how it > can hurt anyone, and anyway it can also happen today too in some > cases. OK - if you are aware of it and expect it then I'm fine with it. Likewise I'm trying to maximise our understanding on these issues. I'll start digging on that 250ms delay thing. Today I find that 250ms is unreliable so I'm now looking at 300ms... cheers 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
Hi Ohad, On 17 June 2011 15:31, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> Yes, a regulator could work. But, aren't regulators linked to the >> platform rather than to the card? I would imagine that this quirk >> needs to be applied to all users of the card, therefore a card quirk >> could be more appropriate? Also, if you think the sdio_reset() option >> is preferable to the sleep, can that be done from a regulator? > > I'd really prefer to understand what's wrong first. > > If we have a rigid hardware requirement that demands waiting before > powering it up again, then we better obey it. But I somehow doubt that > a 250ms delay is required. That's way too much... So, as I found here: http://article.gmane.org/gmane.linux.kernel.mmc/8605 This is an OLPC-specific issue due to lack of power clamping on the motherboard's power supply, which might not be shared by other sd8686 users, and a long delay is expected. I have been testing further and I have seen a couple of times that msleep(300) is not enough, or at least the card failed to do the cmd5 reset in those cases. The sdio_reset() method seems to be 100% reliable however, I haven't seen a single failure with that yet. Where do we go from here? Any suggested implementation approach? 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
Hi Daniel, On Sun, Jun 19, 2011 at 1:33 PM, Daniel Drake <dsd@laptop.org> wrote: > So, as I found here: http://article.gmane.org/gmane.linux.kernel.mmc/8605 > This is an OLPC-specific issue due to lack of power clamping on the > motherboard's power supply, which might not be shared by other sd8686 > users, and a long delay is expected. > > I have been testing further and I have seen a couple of times that > msleep(300) is not enough, or at least the card failed to do the cmd5 > reset in those cases. The sdio_reset() method seems to be 100% > reliable however, I haven't seen a single failure with that yet. Then sdio_reset() it is. We're taking a risk of covering up bugs like the one we just found here: http://www.mail-archive.com/linux-mmc@vger.kernel.org/msg08371.html So people will be happy that everything (seem to) work, without even knowing their card might not have been powered off at all. But I still think it's generally better, given the alternatives. And since this should not have any bad effect for anyone else, I'm also OK with not wrapping the sdio_reset and the CMD5 arg=0 you're adding with a quirk. After that's in place, I plan to completely get rid of MMC_CAP_POWER_OFF_CARD, since the only evidence we had when we added it was the sd8686, and today we know it's not necessary at all. 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
Hi Ohad, On 19 June 2011 12:00, Ohad Ben-Cohen <ohad@wizery.com> wrote: > But I still think it's generally better, given the alternatives. > > And since this should not have any bad effect for anyone else, I'm > also OK with not wrapping the sdio_reset and the CMD5 arg=0 you're > adding with a quirk. > > After that's in place, I plan to completely get rid of > MMC_CAP_POWER_OFF_CARD, since the only evidence we had when we added > it was the sd8686, and today we know it's not necessary at all. Thanks - I've just submitted this patch. Now we just need you to submit the remaining parts of http://dev.laptop.org/~dsd/20110609/sd-pwr-debug6.patch - can you take care of that? I'm not sure if/how it should be split up or how to write the commit message. Then we can get rid of MMC_CAP_POWER_OFF_CARD, which would be good to do ASAP to maximize testing before Linux 3.1. 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
On Sat, Jun 25, 2011 at 9:23 PM, Daniel Drake <dsd@laptop.org> wrote: > Now we just need you to submit the remaining parts of > http://dev.laptop.org/~dsd/20110609/sd-pwr-debug6.patch - can you take > care of that? I'm not sure if/how it should be split up or how to > write the commit message. Sure, i'll post it soon. > Then we can get rid of MMC_CAP_POWER_OFF_CARD, which would be good to > do ASAP to maximize testing before Linux 3.1. I'll take care of that. 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
Hi, Ohad One question :( Under pm_runtime mechanism, how to dynamically open/close wlan. If the wlan chip really power off, the firmware should be reloaded, since the firmware is download to ram and disappear after power off. So wlan probe function should be called for re-downloading, is it be achieved by calling pm_runtime_get_sync and mmc_power_restore_host? From Daniel's test log, "echo mem > /sys/power/state" is used to restart wlan, is this standard method? Since host->bus_ops is already not NULL, mmc_rescan will return direclty. 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
Hi Zhangfei, On Tue, Jun 28, 2011 at 12:13 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote: > One question :( Np, feel free to ask ! (but it's usually better to start a new thread, rather then forking an existing one under the same subject) > Under pm_runtime mechanism, how to dynamically open/close wlan. Actually it should be the other way around: when you enable/disable WLAN (e.g. Ifconfig up/down), you then use the runtime PM API to power up/down the device. > If the wlan chip really power off, the firmware should be reloaded, > since the firmware is download to ram and disappear after power off. Right. > So wlan probe function should be called for re-downloading, is it be > achieved by calling pm_runtime_get_sync and mmc_power_restore_host? Not really; I assume you refer to libertas_sdio, which AFAICT, is built to power on the device (and configure it) on ->probe(), and then power it off on ->remove(). It then seems that this model was extended to support suspend/resume by asking the MMC core to ->remove() the card on suspend, and then relying on it to re-detect and re-add the card on resume, which will trigger libertas' ->probe() again. IMHO this model is a little awkward; as you can see, powering on/off the chip requires re-probing the driver. Take a look how mac80211 drivers (and wl12xx in particular) behave: they are powered on (and firmware is downloaded) when the user brings the wlan0 interface up, and then powered off when the wlan interface is brought down. The runtime PM API is only being used to control the power to the device, but downloading the firmware and doing driver-specific configuration is up to the driver to do. > From Daniel's test log, "echo mem > /sys/power/state" is used to > restart wlan, is this standard method? That command was only used to trigger system suspend - Daniel was testing libertas & runtime PM behavior in that scenario. 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
>> So wlan probe function should be called for re-downloading, is it be >> achieved by calling pm_runtime_get_sync and mmc_power_restore_host? > > Not really; > > I assume you refer to libertas_sdio, which AFAICT, is built to power > on the device (and configure it) on ->probe(), and then power it off > on ->remove(). > It then seems that this model was extended to support suspend/resume > by asking the MMC core to ->remove() the card on suspend, and then > relying on it to re-detect and re-add the card on resume, which will > trigger libertas' ->probe() again. > > IMHO this model is a little awkward; as you can see, powering on/off > the chip requires re-probing the driver. > > Take a look how mac80211 drivers (and wl12xx in particular) behave: > they are powered on (and firmware is downloaded) when the user brings > the wlan0 interface up, and then powered off when the wlan interface > is brought down. > > The runtime PM API is only being used to control the power to the > device, but downloading the firmware and doing driver-specific > configuration is up to the driver to do. Thanks Ohad for your kind explanation. However still not fully understand how to call ->remove to power off wlan, using suspend system looks to me is only test method, which counting on bus_ops->suspend returns -ENOSYS. What we do before is 1. From user space use rfkill unblock wifi, or echo on/off > "/sys/~" to enable/disable wlan power, which could be replaced by pm_runtime of course. 2. Explicitly call mmc_detect_change, when power off, mmc_rescan -> bus_ops->detect(host) -> mmc_select_card fail -> mmc_sdio_remove; when power on, mmc_rescan -> mmc_attach_sdio->wlan probe. However, this is not workable if using mmc_power_save_host, since bus_ops->detect not workable after power off card. 1. detect is only for !MMC_CAP_NONREMOVABLE 2. In mmc_sdio_detect, power is provided before mmc_select_card via pm_runtime_get_sync. 3. Most important, mmc_power_save_host->mmc_power_off->set clk=0 via set_ios, so controller will no response and timeout. How to enable/disable mac80211 at runtime from user space? In wl12xx/debugfs.c, gpio_power_write->wl1271_sdio_power_off? still unclear how remove is called. 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
On 28 June 2011 10:13, zhangfei gao <zhangfei.gao@gmail.com> wrote: > Hi, Ohad > > One question :( > > Under pm_runtime mechanism, how to dynamically open/close wlan. > > If the wlan chip really power off, the firmware should be reloaded, > since the firmware is download to ram and disappear after power off. > So wlan probe function should be called for re-downloading, is it be > achieved by calling pm_runtime_get_sync and mmc_power_restore_host? Here is how I'm doing it for libertas: http://dev.laptop.org/~dsd/20110610/ See patches 6 and 7. This is somewhat based off the wl1xxx example that Ohad gave. 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
On Wed, Jun 29, 2011 at 11:43 AM, zhangfei gao <zhangfei.gao@gmail.com> wrote: > However still not fully understand how to call ->remove to power off > wlan, using suspend system looks to me is only test method, which > counting on bus_ops->suspend returns -ENOSYS. Please take a look at mac80211 and wl12xx. We're not using ->remove, ->probe or -ENOSYS at all. When the user brings up the interface, we then power up the chip and download the firmware. Likewise, when the interface is brought down, we just power off the chip. > 2. Explicitly call mmc_detect_change, > when power off, mmc_rescan -> bus_ops->detect(host) -> mmc_select_card > fail -> mmc_sdio_remove; > when power on, mmc_rescan -> mmc_attach_sdio->wlan probe. This is awkward. Basically what you're saying is that your driver can only power on the chip when it is probed. IMHO you need to refactor your driver, so it will power on/off the device according to interface status changes, without requiring the system to remove and re-detect the device. -- 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
On Wed, Jun 29, 2011 at 4:57 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Wed, Jun 29, 2011 at 11:43 AM, zhangfei gao <zhangfei.gao@gmail.com> wrote: >> However still not fully understand how to call ->remove to power off >> wlan, using suspend system looks to me is only test method, which >> counting on bus_ops->suspend returns -ENOSYS. > > Please take a look at mac80211 and wl12xx. > > We're not using ->remove, ->probe or -ENOSYS at all. > > When the user brings up the interface, we then power up the chip and > download the firmware. > Likewise, when the interface is brought down, we just power off the chip. Thanks a lot Ohad & Daniel :) Enable wlan: # ifconfig up mlan0 -> power up the chip via runtime PM -> wlan_probe download the firmware Disable wlan: # ifconfig down mlan0 -> power down the chip via runtime PM -> wlan_remove ? So every time ifconfig up/down, the chip is power up/down, and firmware reloaded? Is this understand correct? Then runtime pm should also integrated into wlan driver as well. -- 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
On Wed, Jun 29, 2011 at 12:19 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote: > Enable wlan: # ifconfig up mlan0 -> power up the chip via runtime PM > -> wlan_probe download the firmware > Disable wlan: # ifconfig down mlan0 -> power down the chip via runtime > PM -> wlan_remove ? Sounds all good, besides the part where you mention the "probe" and "remove" names. I'm not sure what you mean exactly, but generally, the driver's probe and remove functions should be called only once during the lifetime of the device it controls. You may want to refactor the driver a bit, so you don't need those handlers to be called whenever you power up/down the card. > So every time ifconfig up/down, the chip is power up/down, and > firmware reloaded? Yes, this is one way to go (I know some other out-of-tree WLAN drivers behave differently, but this is the way mac80211 works, and it's quite nice). -- 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 --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 4d0c15b..e23888a 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -540,6 +540,13 @@ static void mmc_sdio_remove(struct mmc_host *host) BUG_ON(!host); BUG_ON(!host->card); + /* + * if this card is managed by runtime pm, make sure it is powered on + * before invoking its SDIO functions' ->remove() handler + */ + if (host->caps & MMC_CAP_POWER_OFF_CARD) + pm_runtime_get_sync(&host->card->dev); + for (i = 0;i < host->card->sdio_funcs;i++) { if (host->card->sdio_func[i]) { sdio_remove_func(host->card->sdio_func[i]); @@ -547,6 +554,9 @@ static void mmc_sdio_remove(struct mmc_host *host) } } + if (host->caps & MMC_CAP_POWER_OFF_CARD) + pm_runtime_put_noidle(&host->card->dev); //<============== use pm_runtime_put_sync here + mmc_remove_card(host->card); host->card = NULL;