LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: arnd@arndb.de, gregkh@linuxfoundation.org
Cc: ulf.hansson@linaro.org, stern@rowland.harvard.edu,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	Kai-Heng Feng <kai.heng.feng@canonical.com>
Subject: [PATCH 4/4 v5] memstick: rtsx_usb_ms: Support runtime power management
Date: Wed, 24 Oct 2018 16:49:58 +0800	[thread overview]
Message-ID: <20181024084958.4627-5-kai.heng.feng@canonical.com> (raw)
In-Reply-To: <20181024084958.4627-1-kai.heng.feng@canonical.com>

In order to let host's parent device, rtsx_usb, to use USB remote wake
up signaling to do card detection, it needs to be suspended. Hence it's
necessary to add runtime PM support for the memstick host.

To keep memstick host stays suspended when it's not in use, convert the
card detection function from kthread to delayed_work, which can be
scheduled when the host is resumed and can be canceled when the host is
suspended.

Put the device to suspend when there's no card and the power mode is
MEMSTICK_POWER_OFF.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/memstick/host/rtsx_usb_ms.c | 167 +++++++++++++++++-----------
 1 file changed, 102 insertions(+), 65 deletions(-)

diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c
index cd12f3d1c088..3800c24b084e 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -40,15 +40,14 @@ struct rtsx_usb_ms {
 
 	struct mutex		host_mutex;
 	struct work_struct	handle_req;
-
-	struct task_struct	*detect_ms;
-	struct completion	detect_ms_exit;
+	struct delayed_work	poll_card;
 
 	u8			ssc_depth;
 	unsigned int		clock;
 	int			power_mode;
 	unsigned char           ifmode;
 	bool			eject;
+	bool			system_suspending;
 };
 
 static inline struct device *ms_dev(struct rtsx_usb_ms *host)
@@ -545,7 +544,7 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
 						host->req->error);
 			}
 		} while (!rc);
-		pm_runtime_put(ms_dev(host));
+		pm_runtime_put_sync(ms_dev(host));
 	}
 
 }
@@ -585,14 +584,14 @@ static int rtsx_usb_ms_set_param(struct memstick_host *msh,
 			break;
 
 		if (value == MEMSTICK_POWER_ON) {
-			pm_runtime_get_sync(ms_dev(host));
+			pm_runtime_get_noresume(ms_dev(host));
 			err = ms_power_on(host);
+			if (err)
+				pm_runtime_put_noidle(ms_dev(host));
 		} else if (value == MEMSTICK_POWER_OFF) {
 			err = ms_power_off(host);
-			if (host->msh->card)
+			if (!err)
 				pm_runtime_put_noidle(ms_dev(host));
-			else
-				pm_runtime_put(ms_dev(host));
 		} else
 			err = -EINVAL;
 		if (!err)
@@ -638,25 +637,44 @@ static int rtsx_usb_ms_set_param(struct memstick_host *msh,
 	}
 out:
 	mutex_unlock(&ucr->dev_mutex);
-	pm_runtime_put(ms_dev(host));
+	pm_runtime_put_sync(ms_dev(host));
 
 	/* power-on delay */
-	if (param == MEMSTICK_POWER && value == MEMSTICK_POWER_ON)
+	if (param == MEMSTICK_POWER && value == MEMSTICK_POWER_ON) {
 		usleep_range(10000, 12000);
 
+		if (!host->eject)
+			schedule_delayed_work(&host->poll_card, 100);
+	}
+
 	dev_dbg(ms_dev(host), "%s: return = %d\n", __func__, err);
 	return err;
 }
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
 static int rtsx_usb_ms_suspend(struct device *dev)
 {
 	struct rtsx_usb_ms *host = dev_get_drvdata(dev);
 	struct memstick_host *msh = host->msh;
 
-	dev_dbg(ms_dev(host), "--> %s\n", __func__);
+	/* Since we use rtsx_usb's resume callback to runtime resume its
+	 * children to implement remote wakeup signaling, this causes
+	 * rtsx_usb_ms' runtime resume callback runs after its suspend
+	 * callback:
+	 * rtsx_usb_ms_suspend()
+	 * rtsx_usb_resume()
+	 *   -> rtsx_usb_ms_runtime_resume()
+	 *     -> memstick_detect_change()
+	 *
+	 * rtsx_usb_suspend()
+	 *
+	 * To avoid this, skip runtime resume/suspend if system suspend is
+	 * underway.
+	 */
 
+	host->system_suspending = true;
 	memstick_suspend_host(msh);
+
 	return 0;
 }
 
@@ -665,58 +683,83 @@ static int rtsx_usb_ms_resume(struct device *dev)
 	struct rtsx_usb_ms *host = dev_get_drvdata(dev);
 	struct memstick_host *msh = host->msh;
 
-	dev_dbg(ms_dev(host), "--> %s\n", __func__);
-
 	memstick_resume_host(msh);
+	host->system_suspending = false;
+
 	return 0;
 }
-#endif /* CONFIG_PM_SLEEP */
 
-/*
- * Thread function of ms card slot detection. The thread starts right after
- * successful host addition. It stops while the driver removal function sets
- * host->eject true.
- */
-static int rtsx_usb_detect_ms_card(void *__host)
+static int rtsx_usb_ms_runtime_suspend(struct device *dev)
+{
+	struct rtsx_usb_ms *host = dev_get_drvdata(dev);
+
+	if (host->system_suspending)
+		return 0;
+
+	if (host->msh->card || host->power_mode != MEMSTICK_POWER_OFF)
+		return -EAGAIN;
+
+	return 0;
+}
+
+static int rtsx_usb_ms_runtime_resume(struct device *dev)
+{
+	struct rtsx_usb_ms *host = dev_get_drvdata(dev);
+
+
+	if (host->system_suspending)
+		return 0;
+
+	memstick_detect_change(host->msh);
+
+	return 0;
+}
+
+static const struct dev_pm_ops rtsx_usb_ms_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(rtsx_usb_ms_suspend, rtsx_usb_ms_resume)
+	SET_RUNTIME_PM_OPS(rtsx_usb_ms_runtime_suspend, rtsx_usb_ms_runtime_resume, NULL)
+};
+
+#endif /* CONFIG_PM */
+
+static void rtsx_usb_ms_poll_card(struct work_struct *work)
 {
-	struct rtsx_usb_ms *host = (struct rtsx_usb_ms *)__host;
+	struct rtsx_usb_ms *host = container_of(work, struct rtsx_usb_ms,
+			poll_card.work);
 	struct rtsx_ucr *ucr = host->ucr;
-	u8 val = 0;
 	int err;
+	u8 val;
 
-	for (;;) {
-		pm_runtime_get_sync(ms_dev(host));
-		mutex_lock(&ucr->dev_mutex);
-
-		/* Check pending MS card changes */
-		err = rtsx_usb_read_register(ucr, CARD_INT_PEND, &val);
-		if (err) {
-			mutex_unlock(&ucr->dev_mutex);
-			goto poll_again;
-		}
+	if (host->eject || host->power_mode != MEMSTICK_POWER_ON)
+		return;
 
-		/* Clear the pending */
-		rtsx_usb_write_register(ucr, CARD_INT_PEND,
-				XD_INT | MS_INT | SD_INT,
-				XD_INT | MS_INT | SD_INT);
+	pm_runtime_get_sync(ms_dev(host));
+	mutex_lock(&ucr->dev_mutex);
 
+	/* Check pending MS card changes */
+	err = rtsx_usb_read_register(ucr, CARD_INT_PEND, &val);
+	if (err) {
 		mutex_unlock(&ucr->dev_mutex);
+		goto poll_again;
+	}
 
-		if (val & MS_INT) {
-			dev_dbg(ms_dev(host), "MS slot change detected\n");
-			memstick_detect_change(host->msh);
-		}
+	/* Clear the pending */
+	rtsx_usb_write_register(ucr, CARD_INT_PEND,
+			XD_INT | MS_INT | SD_INT,
+			XD_INT | MS_INT | SD_INT);
 
-poll_again:
-		pm_runtime_put(ms_dev(host));
-		if (host->eject)
-			break;
+	mutex_unlock(&ucr->dev_mutex);
 
-		schedule_timeout_idle(HZ);
+	if (val & MS_INT) {
+		dev_dbg(ms_dev(host), "MS slot change detected\n");
+		memstick_detect_change(host->msh);
 	}
 
-	complete(&host->detect_ms_exit);
-	return 0;
+poll_again:
+	pm_runtime_put_sync(ms_dev(host));
+
+	if (!host->eject && host->power_mode == MEMSTICK_POWER_ON)
+		schedule_delayed_work(&host->poll_card, 100);
 }
 
 static int rtsx_usb_ms_drv_probe(struct platform_device *pdev)
@@ -747,39 +790,35 @@ static int rtsx_usb_ms_drv_probe(struct platform_device *pdev)
 	mutex_init(&host->host_mutex);
 	INIT_WORK(&host->handle_req, rtsx_usb_ms_handle_req);
 
-	init_completion(&host->detect_ms_exit);
-	host->detect_ms = kthread_create(rtsx_usb_detect_ms_card, host,
-			"rtsx_usb_ms_%d", pdev->id);
-	if (IS_ERR(host->detect_ms)) {
-		dev_dbg(&(pdev->dev),
-				"Unable to create polling thread.\n");
-		err = PTR_ERR(host->detect_ms);
-		goto err_out;
-	}
+	INIT_DELAYED_WORK(&host->poll_card, rtsx_usb_ms_poll_card);
 
 	msh->request = rtsx_usb_ms_request;
 	msh->set_param = rtsx_usb_ms_set_param;
 	msh->caps = MEMSTICK_CAP_PAR4;
 
-	pm_runtime_enable(&pdev->dev);
+	pm_runtime_set_active(ms_dev(host));
+	pm_runtime_enable(ms_dev(host));
+
+	pm_runtime_get_sync(ms_dev(host));
 	err = memstick_add_host(msh);
 	if (err)
 		goto err_out;
 
-	wake_up_process(host->detect_ms);
+	pm_runtime_put(ms_dev(host));
+
 	return 0;
 err_out:
 	memstick_free_host(msh);
+	pm_runtime_put_noidle(ms_dev(host));
 	return err;
 }
 
 static int rtsx_usb_ms_drv_remove(struct platform_device *pdev)
 {
 	struct rtsx_usb_ms *host = platform_get_drvdata(pdev);
-	struct memstick_host *msh;
+	struct memstick_host *msh = host->msh;
 	int err;
 
-	msh = host->msh;
 	host->eject = true;
 	cancel_work_sync(&host->handle_req);
 
@@ -797,7 +836,6 @@ static int rtsx_usb_ms_drv_remove(struct platform_device *pdev)
 	}
 	mutex_unlock(&host->host_mutex);
 
-	wait_for_completion(&host->detect_ms_exit);
 	memstick_remove_host(msh);
 	memstick_free_host(msh);
 
@@ -816,9 +854,6 @@ static int rtsx_usb_ms_drv_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(rtsx_usb_ms_pm_ops,
-		rtsx_usb_ms_suspend, rtsx_usb_ms_resume);
-
 static struct platform_device_id rtsx_usb_ms_ids[] = {
 	{
 		.name = "rtsx_usb_ms",
@@ -834,7 +869,9 @@ static struct platform_driver rtsx_usb_ms_driver = {
 	.id_table       = rtsx_usb_ms_ids,
 	.driver		= {
 		.name	= "rtsx_usb_ms",
+#ifdef CONFIG_PM
 		.pm	= &rtsx_usb_ms_pm_ops,
+#endif
 	},
 };
 module_platform_driver(rtsx_usb_ms_driver);
-- 
2.17.1


  parent reply	other threads:[~2018-10-24  8:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-24  8:49 [PATCH 0/4 v5] Keep rtsx_usb suspended when there's no card Kai-Heng Feng
2018-10-24  8:49 ` [PATCH 1/4 v5] misc: rtsx_usb: Use USB remote wakeup signaling for card insertion detection Kai-Heng Feng
2018-10-24  8:49 ` [PATCH 2/4 v5] memstick: Prevent memstick host from getting runtime suspended during card detection Kai-Heng Feng
2018-10-29 12:25   ` Ulf Hansson
2018-10-29 16:31     ` Kai Heng Feng
2018-10-30 13:03       ` Ulf Hansson
2018-10-30 15:23         ` Kai Heng Feng
2018-10-30 16:04           ` Ulf Hansson
2018-10-31  6:33             ` Kai Heng Feng
2018-10-24  8:49 ` [PATCH 3/4 v5] memstick: rtsx_usb_ms: Use ms_dev() helper Kai-Heng Feng
2018-10-24  8:49 ` Kai-Heng Feng [this message]
2018-10-27 13:13 ` [PATCH 0/4 v5] Keep rtsx_usb suspended when there's no card Oleksandr Natalenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181024084958.4627-5-kai.heng.feng@canonical.com \
    --to=kai.heng.feng@canonical.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=ulf.hansson@linaro.org \
    --subject='Re: [PATCH 4/4 v5] memstick: rtsx_usb_ms: Support runtime power management' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).