| Message ID | BANLkTikvP3hc+CHEwrUEDqN6McwswikS-Q@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 5 June 2011 00:08, Ohad Ben-Cohen <ohad@wizery.com> wrote: > We can stop checking the return value of pm_runtime_get_sync in > sdio_bus_remove, like pci_device_remove is doing, but I don't think > that's enough: I guess libertas' if_sdio_remove won't be so happy > dealing with a powered off card. > > Can you try the following please (untested): Thanks, that seems to be working. -- 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 Sun, Jun 5, 2011 at 3:31 PM, Daniel Drake <dsd@laptop.org> wrote: > On 5 June 2011 00:08, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> We can stop checking the return value of pm_runtime_get_sync in >> sdio_bus_remove, like pci_device_remove is doing, but I don't think >> that's enough: I guess libertas' if_sdio_remove won't be so happy >> dealing with a powered off card. >> >> Can you try the following please (untested): > > Thanks, that seems to be working. 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 Daniel, On Sun, Jun 5, 2011 at 3:31 PM, Daniel Drake <dsd@laptop.org> wrote: > On 5 June 2011 00:08, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> We can stop checking the return value of pm_runtime_get_sync in >> sdio_bus_remove, like pci_device_remove is doing, but I don't think >> that's enough: I guess libertas' if_sdio_remove won't be so happy >> dealing with a powered off card. >> >> Can you try the following please (untested): > > Thanks, that seems to be working. That patch had two hunks. The first one, in mmc_sdio_remove, made sure libertas' ->remove() handler will be called powered on, even if the chip was powered off beforehand. The second one, in sdio_bus_remove, directly took care of the problem you saw by ignoring rumtime PM's return value (which is a valid error in your scenario). Obviously the second hunk is necessary, but I'd like to know whether the first one really is too or not. Can you please retest this without that hunk (try to suspend/resume while the chip is powered off, and again while it is powered on, but wol isn't used) ? If the second hunk is sufficient (everything works as expected + no scary error messages coming from libertas), I would like to avoid the first one. I'm not sure at all there's any reason to power up the card if it is being removed, and a quick glance shows libertas_sdio has this 'user_rmmod' flag which should prevent scary error messages in this exact 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
On 28 June 2011 06:55, Ohad Ben-Cohen <ohad@wizery.com> wrote: > Obviously the second hunk is necessary, but I'd like to know whether > the first one really is too or not. Can you please retest this without > that hunk (try to suspend/resume while the chip is powered off, and > again while it is powered on, but wol isn't used) ? Exactly which kernel should I run this test 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
On Tue, Jun 28, 2011 at 11:59 PM, Daniel Drake <dsd@laptop.org> wrote: > On 28 June 2011 06:55, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> Obviously the second hunk is necessary, but I'd like to know whether >> the first one really is too or not. Can you please retest this without >> that hunk (try to suspend/resume while the chip is powered off, and >> again while it is powered on, but wol isn't used) ? > > Exactly which kernel should I run this test on? Latest (isn't that what you've been working with all this time ?). 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 28 June 2011 22:47, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Tue, Jun 28, 2011 at 11:59 PM, Daniel Drake <dsd@laptop.org> wrote: >> On 28 June 2011 06:55, Ohad Ben-Cohen <ohad@wizery.com> wrote: >>> Obviously the second hunk is necessary, but I'd like to know whether >>> the first one really is too or not. Can you please retest this without >>> that hunk (try to suspend/resume while the chip is powered off, and >>> again while it is powered on, but wol isn't used) ? >> >> Exactly which kernel should I run this test on? > > Latest (isn't that what you've been working with all this time ?). latest linux-mmc.git ? Or linux-next? Or linus? I'll apply the patch in this thread, and my patch titled "mmc: sdio: reset card during power_restore" - anything else? Too many patches floating around, just trying to be sure of what I'm doing! 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 --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 4d0c15b..8af3330 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); + mmc_remove_card(host->card); host->card = NULL; } diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c index d29b9c3..73dc3c2 100644 --- a/drivers/mmc/core/sdio_bus.c +++ b/drivers/mmc/core/sdio_bus.c @@ -167,11 +167,8 @@ static int sdio_bus_remove(struct device *dev) int ret = 0; /* Make sure card is powered before invoking ->remove() */ - if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD) { - ret = pm_runtime_get_sync(dev); - if (ret < 0) - goto out; - } + if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD) + pm_runtime_get_sync(dev); drv->remove(func); @@ -191,7 +188,6 @@ static int sdio_bus_remove(struct device *dev) if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD) pm_runtime_put_noidle(dev); -out: return ret; } -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in