* [PATCH 1/6] mmc: rtsx_usb_sdmmc: Avoid keeping the device runtime resumed when unused
2016-10-13 11:37 [PATCH 0/6] mmc/memstick: rtsx_usb: Fix runtime PM issues Ulf Hansson
@ 2016-10-13 11:37 ` Ulf Hansson
2016-10-13 11:37 ` [PATCH 2/6] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing the led Ulf Hansson
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2016-10-13 11:37 UTC (permalink / raw)
To: linux-mmc, linux-kernel, Ulf Hansson
Cc: Ritesh Raj Sarraf, Alan Stern, Oliver Neukum, USB list,
Roger Tseng, Wei WANG, stable
The rtsx_usb_sdmmc driver may bail out in its ->set_ios() callback when no
SD card is inserted. This is wrong, as it could cause the device to remain
runtime resumed when it's unused. Fix this behaviour.
Tested-by: Ritesh Raj Sarraf <rrs@researchut.com>
Cc: <stable@vger.kernel.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/host/rtsx_usb_sdmmc.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index 4106295..e0b8590 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1138,11 +1138,6 @@ static void sdmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
dev_dbg(sdmmc_dev(host), "%s\n", __func__);
mutex_lock(&ucr->dev_mutex);
- if (rtsx_usb_card_exclusive_check(ucr, RTSX_USB_SD_CARD)) {
- mutex_unlock(&ucr->dev_mutex);
- return;
- }
-
sd_set_power_mode(host, ios->power_mode);
sd_set_bus_width(host, ios->bus_width);
sd_set_timing(host, ios->timing, &host->ddr_mode);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/6] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing the led
2016-10-13 11:37 [PATCH 0/6] mmc/memstick: rtsx_usb: Fix runtime PM issues Ulf Hansson
2016-10-13 11:37 ` [PATCH 1/6] mmc: rtsx_usb_sdmmc: Avoid keeping the device runtime resumed when unused Ulf Hansson
@ 2016-10-13 11:37 ` Ulf Hansson
2016-10-13 11:37 ` [PATCH 3/6] memstick: rtsx_usb_ms: Runtime resume the device when polling for cards Ulf Hansson
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2016-10-13 11:37 UTC (permalink / raw)
To: linux-mmc, linux-kernel, Ulf Hansson
Cc: Ritesh Raj Sarraf, Alan Stern, Oliver Neukum, USB list,
Roger Tseng, Wei WANG, stable
Accesses of the rtsx sdmmc's parent device, which is the rtsx usb device,
must be done when it's runtime resumed. Currently this isn't case when
changing the led, so let's fix this by adding a pm_runtime_get_sync() and
a pm_runtime_put() around those operations.
Reported-by: Ritesh Raj Sarraf <rrs@researchut.com>
Tested-by: Ritesh Raj Sarraf <rrs@researchut.com>
Cc: <stable@vger.kernel.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/host/rtsx_usb_sdmmc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index e0b8590..6e9c0f8 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1309,6 +1309,7 @@ static void rtsx_usb_update_led(struct work_struct *work)
container_of(work, struct rtsx_usb_sdmmc, led_work);
struct rtsx_ucr *ucr = host->ucr;
+ pm_runtime_get_sync(sdmmc_dev(host));
mutex_lock(&ucr->dev_mutex);
if (host->led.brightness == LED_OFF)
@@ -1317,6 +1318,7 @@ static void rtsx_usb_update_led(struct work_struct *work)
rtsx_usb_turn_on_led(ucr);
mutex_unlock(&ucr->dev_mutex);
+ pm_runtime_put(sdmmc_dev(host));
}
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/6] memstick: rtsx_usb_ms: Runtime resume the device when polling for cards
2016-10-13 11:37 [PATCH 0/6] mmc/memstick: rtsx_usb: Fix runtime PM issues Ulf Hansson
2016-10-13 11:37 ` [PATCH 1/6] mmc: rtsx_usb_sdmmc: Avoid keeping the device runtime resumed when unused Ulf Hansson
2016-10-13 11:37 ` [PATCH 2/6] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing the led Ulf Hansson
@ 2016-10-13 11:37 ` Ulf Hansson
2016-10-13 11:37 ` [PATCH 4/6] memstick: rtsx_usb_ms: Manage runtime PM when accessing the device Ulf Hansson
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2016-10-13 11:37 UTC (permalink / raw)
To: linux-mmc, linux-kernel, Ulf Hansson
Cc: Ritesh Raj Sarraf, Alan Stern, Oliver Neukum, USB list,
Roger Tseng, Wei WANG, stable
From: Alan Stern <stern@rowland.harvard.edu>
Accesses to the rtsx usb device, which is the parent of the rtsx memstick
device, must not be done unless it's runtime resumed.
Therefore when the rtsx_usb_ms driver polls for inserted memstick cards,
let's add pm_runtime_get|put*() to make sure accesses is done when the
rtsx usb device is runtime resumed.
Reported-by: Ritesh Raj Sarraf <rrs@researchut.com>
Tested-by: Ritesh Raj Sarraf <rrs@researchut.com>
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: <stable@vger.kernel.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/memstick/host/rtsx_usb_ms.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c
index d34bc35..1b99489 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -681,6 +681,7 @@ static int rtsx_usb_detect_ms_card(void *__host)
int err;
for (;;) {
+ pm_runtime_get_sync(ms_dev(host));
mutex_lock(&ucr->dev_mutex);
/* Check pending MS card changes */
@@ -703,6 +704,7 @@ static int rtsx_usb_detect_ms_card(void *__host)
}
poll_again:
+ pm_runtime_put(ms_dev(host));
if (host->eject)
break;
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/6] memstick: rtsx_usb_ms: Manage runtime PM when accessing the device
2016-10-13 11:37 [PATCH 0/6] mmc/memstick: rtsx_usb: Fix runtime PM issues Ulf Hansson
` (2 preceding siblings ...)
2016-10-13 11:37 ` [PATCH 3/6] memstick: rtsx_usb_ms: Runtime resume the device when polling for cards Ulf Hansson
@ 2016-10-13 11:37 ` Ulf Hansson
2016-10-13 11:37 ` [PATCH 5/6] mmc: rtsx_usb_sdmmc: Enable runtime PM autosuspend Ulf Hansson
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2016-10-13 11:37 UTC (permalink / raw)
To: linux-mmc, linux-kernel, Ulf Hansson
Cc: Ritesh Raj Sarraf, Alan Stern, Oliver Neukum, USB list,
Roger Tseng, Wei WANG, stable
Accesses to the rtsx usb device, which is the parent of the rtsx memstick
device, must not be done unless it's runtime resumed. This is currently not
the case and it could trigger various errors.
Fix this by properly deal with runtime PM in this regards. This means
making sure the device is runtime resumed, when serving requests via the
->request() callback or changing settings via the ->set_param() callbacks.
Cc: <stable@vger.kernel.org>
Cc: Ritesh Raj Sarraf <rrs@researchut.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/memstick/host/rtsx_usb_ms.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c
index 1b99489..2e3cf01 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -524,6 +524,7 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
int rc;
if (!host->req) {
+ pm_runtime_get_sync(ms_dev(host));
do {
rc = memstick_next_req(msh, &host->req);
dev_dbg(ms_dev(host), "next req %d\n", rc);
@@ -544,6 +545,7 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
host->req->error);
}
} while (!rc);
+ pm_runtime_put(ms_dev(host));
}
}
@@ -570,6 +572,7 @@ static int rtsx_usb_ms_set_param(struct memstick_host *msh,
dev_dbg(ms_dev(host), "%s: param = %d, value = %d\n",
__func__, param, value);
+ pm_runtime_get_sync(ms_dev(host));
mutex_lock(&ucr->dev_mutex);
err = rtsx_usb_card_exclusive_check(ucr, RTSX_USB_MS_CARD);
@@ -635,6 +638,7 @@ static int rtsx_usb_ms_set_param(struct memstick_host *msh,
}
out:
mutex_unlock(&ucr->dev_mutex);
+ pm_runtime_put(ms_dev(host));
/* power-on delay */
if (param == MEMSTICK_POWER && value == MEMSTICK_POWER_ON)
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/6] mmc: rtsx_usb_sdmmc: Enable runtime PM autosuspend
2016-10-13 11:37 [PATCH 0/6] mmc/memstick: rtsx_usb: Fix runtime PM issues Ulf Hansson
` (3 preceding siblings ...)
2016-10-13 11:37 ` [PATCH 4/6] memstick: rtsx_usb_ms: Manage runtime PM when accessing the device Ulf Hansson
@ 2016-10-13 11:37 ` Ulf Hansson
2016-10-13 11:37 ` [PATCH 6/6] mmc: core: Don't power off the card when starting the host Ulf Hansson
[not found] ` <1476358660-8338-1-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
6 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2016-10-13 11:37 UTC (permalink / raw)
To: linux-mmc, linux-kernel, Ulf Hansson
Cc: Ritesh Raj Sarraf, Alan Stern, Oliver Neukum, USB list,
Roger Tseng, Wei WANG
Enable runtime PM autosuspend for the rtsx_usb_sdmmc driver to avoid the
device being runtime suspended and runtime resumed between each request.
Let's use a default timeout of 50ms, to be consistent with other mmc hosts.
Cc: Ritesh Raj Sarraf <rrs@researchut.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/host/rtsx_usb_sdmmc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index 6e9c0f8..dc1abd1 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1374,6 +1374,8 @@ static int rtsx_usb_sdmmc_drv_probe(struct platform_device *pdev)
mutex_init(&host->host_mutex);
rtsx_usb_init_host(host);
+ pm_runtime_use_autosuspend(&pdev->dev);
+ pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
pm_runtime_enable(&pdev->dev);
#ifdef RTSX_USB_USE_LEDS_CLASS
@@ -1428,6 +1430,7 @@ static int rtsx_usb_sdmmc_drv_remove(struct platform_device *pdev)
mmc_free_host(mmc);
pm_runtime_disable(&pdev->dev);
+ pm_runtime_dont_use_autosuspend(&pdev->dev);
platform_set_drvdata(pdev, NULL);
dev_dbg(&(pdev->dev),
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/6] mmc: core: Don't power off the card when starting the host
2016-10-13 11:37 [PATCH 0/6] mmc/memstick: rtsx_usb: Fix runtime PM issues Ulf Hansson
` (4 preceding siblings ...)
2016-10-13 11:37 ` [PATCH 5/6] mmc: rtsx_usb_sdmmc: Enable runtime PM autosuspend Ulf Hansson
@ 2016-10-13 11:37 ` Ulf Hansson
[not found] ` <1476358660-8338-1-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
6 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2016-10-13 11:37 UTC (permalink / raw)
To: linux-mmc, linux-kernel, Ulf Hansson
Cc: Ritesh Raj Sarraf, Alan Stern, Oliver Neukum, USB list,
Roger Tseng, Wei WANG
The MMC_CAP2_NO_PRESCAN_POWERUP was invented to avoid running the power up
sequence, mmc_power_up(), during ->probe() of the mmc host driver, but
instead defer this to the mmc detect work. This is especially useful for
those hosts that suffers from a long initialization time, as this time
would otherwise add up to the total boot time.
However, due to the introduction of runtime PM of mmc host devices in the
mmc core, this behaviour changed a bit. More precisely, it caused the mmc
core to runtime resume the host device during ->probe() of the host driver.
In cases like the rtsx_usb_sdmmc, runtime resuming the device may be costly
and thus affecting the total boot time.
To improve this behaviour when using MMC_CAP2_NO_PRESCAN_POWERUP, let's
postpone also calling mmc_power_off() when starting the host. This change
allows the mmc core to avoid runtime resuming the device, as it don't need
to claim the host for that execution path.
Cc: Ritesh Raj Sarraf <rrs@researchut.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/core.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 2553d90..2ad7291 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2824,12 +2824,11 @@ void mmc_start_host(struct mmc_host *host)
host->rescan_disable = 0;
host->ios.power_mode = MMC_POWER_UNDEFINED;
- mmc_claim_host(host);
- if (host->caps2 & MMC_CAP2_NO_PRESCAN_POWERUP)
- mmc_power_off(host);
- else
+ if (!(host->caps2 & MMC_CAP2_NO_PRESCAN_POWERUP)) {
+ mmc_claim_host(host);
mmc_power_up(host, host->ocr_avail);
- mmc_release_host(host);
+ mmc_release_host(host);
+ }
mmc_gpiod_request_cd_irq(host);
_mmc_detect_change(host, 0, false);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread[parent not found: <1476358660-8338-1-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 0/6] mmc/memstick: rtsx_usb: Fix runtime PM issues
[not found] ` <1476358660-8338-1-git-send-email-ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2016-10-17 13:49 ` Ulf Hansson
[not found] ` <CAPDyKFpzq2wRfh_5x119ouP-e8EAaibsSvM36msxm7oepxRoDQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2016-10-17 13:49 UTC (permalink / raw)
To: linux-mmc, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Ulf Hansson
Cc: Ritesh Raj Sarraf, Alan Stern, Oliver Neukum, USB list,
Roger Tseng, Wei WANG
On 13 October 2016 at 13:37, Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> The rtsx_usb_sdmmc (mmc/sd) and rtsx_usb_ms (memstick) devices are interfacing
> an rtsx_usb device (managed by an mfd driver) while communicating with the
> cards. The mmc/sd and memstick devices are children of the usb device.
>
> Issues has been reported [1] for rtsx, which have been investigated, discussed,
> fixed, tested, etc, particularly when using mmc/sd cards. During the
> investigation, some changes was proposed and tested successfully. In this
> series I have picked up these changes and created some proper patches with
> change-logs.
>
> It turned out that most of the problems was related to the runtime PM
> deployment in the memstick and the mmc/sd driver, which are fixed in this
> series.
>
> A couple of more issues were also discussed [2], which needs to be fixed as
> well. Although they are out of scope for this series, so we will have to look
> into those at a later point. Here's a brief summary of these leftovers:
>
> *)
> It seems reasonable to turn off autosuspend for usb devices and instead rely on
> the usb device's children to deal with autosuspend. The reason is simply that
> the children has better knowledge of when using autosuspend makes sense.
>
> **)
> Polling card detect mode is used both for mmc/sd and memstick. Because of the
> lack of synchronization between the polling attempts for these subsystems, the
> usb device may be powered on for unnecessary long intervals, thus we are
> wasting power.
>
> Besides the above changes, I folded in a change in the mmc core which improves
> the behaviour for mmc hosts using MMC_CAP2_NO_PRESCAN_POWERUP, which is the
> case for the rtsx_usb_sdmmc driver. This change should improve the boot time
> with ~100ms per rtsx_usb_sdmmc device.
>
> Finally, as I couldn't find an active maintainer for the memstick
> subsystem/driver and because the author of the drivers can be reached, I
> volunteer to take this all through my mmc tree. Please tell me if that is
> problem to any of you.
>
> [1]
> https://www.spinics.net/lists/linux-usb/msg146998.html
> [2]
> https://www.spinics.net/lists/linux-mmc/msg39235.html
>
>
> Alan Stern (1):
> memstick: rtsx_usb_ms: Runtime resume the device when polling for
> cards
>
> Ulf Hansson (5):
> mmc: rtsx_usb_sdmmc: Avoid keeping the device runtime resumed when
> unused
> mmc: rtsx_usb_sdmmc: Handle runtime PM while changing the led
> memstick: rtsx_usb_ms: Manage runtime PM when accessing the device
> mmc: rtsx_usb_sdmmc: Enable runtime PM autosuspend
> mmc: core: Don't power off the card when starting the host
>
> drivers/memstick/host/rtsx_usb_ms.c | 6 ++++++
> drivers/mmc/core/core.c | 9 ++++-----
> drivers/mmc/host/rtsx_usb_sdmmc.c | 10 +++++-----
> 3 files changed, 15 insertions(+), 10 deletions(-)
>
> --
> 1.9.1
>
Patch 1->4 applied for fixes. Holding patch 5->6 for a while, as they
are intended for next!
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread