From: Ricky WU <ricky_wu@realtek.com>
To: Kai-Heng Feng <kai.heng.feng@canonical.com>,
"arnd@arndb.de" <arnd@arndb.de>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
Yang Li <yang.lee@linux.alibaba.com>,
Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v6 2/4] misc: rtsx: Rework runtime power management flow
Date: Tue, 25 Jan 2022 06:38:09 +0000 [thread overview]
Message-ID: <aee7446ff3234dc7adb02202d70b121a@realtek.com> (raw)
In-Reply-To: <20220125055010.1866563-2-kai.heng.feng@canonical.com>
> -----Original Message-----
> From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Sent: Tuesday, January 25, 2022 1:50 PM
> To: arnd@arndb.de; gregkh@linuxfoundation.org; ulf.hansson@linaro.org
> Cc: linux-pm@vger.kernel.org; Kai-Heng Feng <kai.heng.feng@canonical.com>;
> Ricky WU <ricky_wu@realtek.com>; Yang Li <yang.lee@linux.alibaba.com>;
> Christophe JAILLET <christophe.jaillet@wanadoo.fr>;
> linux-kernel@vger.kernel.org
> Subject: [PATCH v6 2/4] misc: rtsx: Rework runtime power management flow
>
> Commit 5b4258f6721f ("misc: rtsx: rts5249 support runtime PM") uses
> "rtd3_work" and "idle_work" to manage it's own runtime PM state machine.
>
> When its child device, rtsx_pci_sdmmc, uses runtime PM refcount correctly, all
> the additional works can be managed by generic runtime PM helpers.
>
> So consolidate "idle_work" and "rtd3_work" into generic runtime idle callback
> and runtime suspend callback, respectively.
>
> Fixes: 5b4258f6721f ("misc: rtsx: rts5249 support runtime PM")
> Cc: Ricky WU <ricky_wu@realtek.com>
Tested-by: Ricky WU <ricky_wu@realtek.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v6:
> - Change autosuspend delay to 10s as Realtek suggested.
>
> v5:
> v4:
> - No change.
>
> v3:
> - Allow runtime PM for all devices, but only schedule runtime suspend
> for devices with rtd3_en flagged.
>
> v2:
> - Remove unused idle_work and rtd3_work from rtsx_pcr.
>
> drivers/misc/cardreader/rtsx_pcr.c | 118 ++++++++++-------------------
> include/linux/rtsx_pci.h | 3 -
> 2 files changed, 39 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/misc/cardreader/rtsx_pcr.c
> b/drivers/misc/cardreader/rtsx_pcr.c
> index 6ac509c1821c9..8aba47a7d9736 100644
> --- a/drivers/misc/cardreader/rtsx_pcr.c
> +++ b/drivers/misc/cardreader/rtsx_pcr.c
> @@ -152,20 +152,12 @@ void rtsx_pci_start_run(struct rtsx_pcr *pcr)
> if (pcr->remove_pci)
> return;
>
> - if (pcr->rtd3_en)
> - if (pcr->is_runtime_suspended) {
> - pm_runtime_get(&(pcr->pci->dev));
> - pcr->is_runtime_suspended = false;
> - }
> -
> if (pcr->state != PDEV_STAT_RUN) {
> pcr->state = PDEV_STAT_RUN;
> if (pcr->ops->enable_auto_blink)
> pcr->ops->enable_auto_blink(pcr);
> rtsx_pm_full_on(pcr);
> }
> -
> - mod_delayed_work(system_wq, &pcr->idle_work, msecs_to_jiffies(200));
> }
> EXPORT_SYMBOL_GPL(rtsx_pci_start_run);
>
> @@ -1094,40 +1086,6 @@ static void rtsx_pm_power_saving(struct rtsx_pcr
> *pcr)
> rtsx_comm_pm_power_saving(pcr);
> }
>
> -static void rtsx_pci_rtd3_work(struct work_struct *work) -{
> - struct delayed_work *dwork = to_delayed_work(work);
> - struct rtsx_pcr *pcr = container_of(dwork, struct rtsx_pcr, rtd3_work);
> -
> - pcr_dbg(pcr, "--> %s\n", __func__);
> - if (!pcr->is_runtime_suspended)
> - pm_runtime_put(&(pcr->pci->dev));
> -}
> -
> -static void rtsx_pci_idle_work(struct work_struct *work) -{
> - struct delayed_work *dwork = to_delayed_work(work);
> - struct rtsx_pcr *pcr = container_of(dwork, struct rtsx_pcr, idle_work);
> -
> - pcr_dbg(pcr, "--> %s\n", __func__);
> -
> - mutex_lock(&pcr->pcr_mutex);
> -
> - pcr->state = PDEV_STAT_IDLE;
> -
> - if (pcr->ops->disable_auto_blink)
> - pcr->ops->disable_auto_blink(pcr);
> - if (pcr->ops->turn_off_led)
> - pcr->ops->turn_off_led(pcr);
> -
> - rtsx_pm_power_saving(pcr);
> -
> - mutex_unlock(&pcr->pcr_mutex);
> -
> - if (pcr->rtd3_en)
> - mod_delayed_work(system_wq, &pcr->rtd3_work,
> msecs_to_jiffies(10000));
> -}
> -
> static void rtsx_base_force_power_down(struct rtsx_pcr *pcr, u8 pm_state)
> {
> /* Set relink_time to 0 */
> @@ -1598,7 +1556,6 @@ static int rtsx_pci_probe(struct pci_dev *pcidev,
> pcr->card_inserted = 0;
> pcr->card_removed = 0;
> INIT_DELAYED_WORK(&pcr->carddet_work, rtsx_pci_card_detect);
> - INIT_DELAYED_WORK(&pcr->idle_work, rtsx_pci_idle_work);
>
> pcr->msi_en = msi_en;
> if (pcr->msi_en) {
> @@ -1623,20 +1580,14 @@ static int rtsx_pci_probe(struct pci_dev *pcidev,
> rtsx_pcr_cells[i].pdata_size = sizeof(*handle);
> }
>
> - if (pcr->rtd3_en) {
> - INIT_DELAYED_WORK(&pcr->rtd3_work, rtsx_pci_rtd3_work);
> - pm_runtime_allow(&pcidev->dev);
> - pm_runtime_enable(&pcidev->dev);
> - pcr->is_runtime_suspended = false;
> - }
> -
>
> ret = mfd_add_devices(&pcidev->dev, pcr->id, rtsx_pcr_cells,
> ARRAY_SIZE(rtsx_pcr_cells), NULL, 0, NULL);
> if (ret < 0)
> goto free_slots;
>
> - schedule_delayed_work(&pcr->idle_work, msecs_to_jiffies(200));
> + pm_runtime_allow(&pcidev->dev);
> + pm_runtime_put(&pcidev->dev);
>
> return 0;
>
> @@ -1668,11 +1619,11 @@ static void rtsx_pci_remove(struct pci_dev
> *pcidev)
> struct pcr_handle *handle = pci_get_drvdata(pcidev);
> struct rtsx_pcr *pcr = handle->pcr;
>
> - if (pcr->rtd3_en)
> - pm_runtime_get_noresume(&pcr->pci->dev);
> -
> pcr->remove_pci = true;
>
> + pm_runtime_get_sync(&pcidev->dev);
> + pm_runtime_forbid(&pcidev->dev);
> +
> /* Disable interrupts at the pcr level */
> spin_lock_irq(&pcr->lock);
> rtsx_pci_writel(pcr, RTSX_BIER, 0);
> @@ -1680,9 +1631,6 @@ static void rtsx_pci_remove(struct pci_dev *pcidev)
> spin_unlock_irq(&pcr->lock);
>
> cancel_delayed_work_sync(&pcr->carddet_work);
> - cancel_delayed_work_sync(&pcr->idle_work);
> - if (pcr->rtd3_en)
> - cancel_delayed_work_sync(&pcr->rtd3_work);
>
> mfd_remove_devices(&pcidev->dev);
>
> @@ -1700,11 +1648,6 @@ static void rtsx_pci_remove(struct pci_dev
> *pcidev)
> idr_remove(&rtsx_pci_idr, pcr->id);
> spin_unlock(&rtsx_pci_lock);
>
> - if (pcr->rtd3_en) {
> - pm_runtime_disable(&pcr->pci->dev);
> - pm_runtime_put_noidle(&pcr->pci->dev);
> - }
> -
> kfree(pcr->slots);
> kfree(pcr);
> kfree(handle);
> @@ -1726,7 +1669,6 @@ static int __maybe_unused rtsx_pci_suspend(struct
> device *dev_d)
> pcr = handle->pcr;
>
> cancel_delayed_work(&pcr->carddet_work);
> - cancel_delayed_work(&pcr->idle_work);
>
> mutex_lock(&pcr->pcr_mutex);
>
> @@ -1760,8 +1702,6 @@ static int __maybe_unused rtsx_pci_resume(struct
> device *dev_d)
> if (ret)
> goto out;
>
> - schedule_delayed_work(&pcr->idle_work, msecs_to_jiffies(200));
> -
> out:
> mutex_unlock(&pcr->pcr_mutex);
> return ret;
> @@ -1786,6 +1726,33 @@ static void rtsx_pci_shutdown(struct pci_dev
> *pcidev)
> pci_disable_msi(pcr->pci);
> }
>
> +static int rtsx_pci_runtime_idle(struct device *device) {
> + struct pci_dev *pcidev = to_pci_dev(device);
> + struct pcr_handle *handle = pci_get_drvdata(pcidev);
> + struct rtsx_pcr *pcr = handle->pcr;
> +
> + dev_dbg(device, "--> %s\n", __func__);
> +
> + mutex_lock(&pcr->pcr_mutex);
> +
> + pcr->state = PDEV_STAT_IDLE;
> +
> + if (pcr->ops->disable_auto_blink)
> + pcr->ops->disable_auto_blink(pcr);
> + if (pcr->ops->turn_off_led)
> + pcr->ops->turn_off_led(pcr);
> +
> + rtsx_pm_power_saving(pcr);
> +
> + mutex_unlock(&pcr->pcr_mutex);
> +
> + if (pcr->rtd3_en)
> + pm_schedule_suspend(device, 10000);
> +
> + return -EBUSY;
> +}
> +
> static int rtsx_pci_runtime_suspend(struct device *device) {
> struct pci_dev *pcidev = to_pci_dev(device); @@ -1794,31 +1761,26 @@
> static int rtsx_pci_runtime_suspend(struct device *device)
>
> handle = pci_get_drvdata(pcidev);
> pcr = handle->pcr;
> - dev_dbg(&(pcidev->dev), "--> %s\n", __func__);
>
> - cancel_delayed_work(&pcr->carddet_work);
> - cancel_delayed_work(&pcr->rtd3_work);
> - cancel_delayed_work(&pcr->idle_work);
> + dev_dbg(device, "--> %s\n", __func__);
> +
> + cancel_delayed_work_sync(&pcr->carddet_work);
>
> mutex_lock(&pcr->pcr_mutex);
> rtsx_pci_power_off(pcr, HOST_ENTER_S3);
>
> mutex_unlock(&pcr->pcr_mutex);
>
> - pcr->is_runtime_suspended = true;
> -
> return 0;
> }
>
> static int rtsx_pci_runtime_resume(struct device *device) {
> struct pci_dev *pcidev = to_pci_dev(device);
> - struct pcr_handle *handle;
> - struct rtsx_pcr *pcr;
> + struct pcr_handle *handle = pci_get_drvdata(pcidev);
> + struct rtsx_pcr *pcr = handle->pcr;
>
> - handle = pci_get_drvdata(pcidev);
> - pcr = handle->pcr;
> - dev_dbg(&(pcidev->dev), "--> %s\n", __func__);
> + dev_dbg(device, "--> %s\n", __func__);
>
> mutex_lock(&pcr->pcr_mutex);
>
> @@ -1834,8 +1796,6 @@ static int rtsx_pci_runtime_resume(struct device
> *device)
> pcr->slots[RTSX_SD_CARD].p_dev);
> }
>
> - schedule_delayed_work(&pcr->idle_work, msecs_to_jiffies(200));
> -
> mutex_unlock(&pcr->pcr_mutex);
> return 0;
> }
> @@ -1850,7 +1810,7 @@ static int rtsx_pci_runtime_resume(struct device
> *device)
>
> static const struct dev_pm_ops rtsx_pci_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(rtsx_pci_suspend, rtsx_pci_resume)
> - SET_RUNTIME_PM_OPS(rtsx_pci_runtime_suspend,
> rtsx_pci_runtime_resume, NULL)
> + SET_RUNTIME_PM_OPS(rtsx_pci_runtime_suspend,
> rtsx_pci_runtime_resume,
> +rtsx_pci_runtime_idle)
> };
>
> static struct pci_driver rtsx_pci_driver = { diff --git a/include/linux/rtsx_pci.h
> b/include/linux/rtsx_pci.h index 4ab7bfc675f11..89b7d34e25b63 100644
> --- a/include/linux/rtsx_pci.h
> +++ b/include/linux/rtsx_pci.h
> @@ -1201,8 +1201,6 @@ struct rtsx_pcr {
> unsigned int card_exist;
>
> struct delayed_work carddet_work;
> - struct delayed_work idle_work;
> - struct delayed_work rtd3_work;
>
> spinlock_t lock;
> struct mutex pcr_mutex;
> @@ -1212,7 +1210,6 @@ struct rtsx_pcr {
> unsigned int cur_clock;
> bool remove_pci;
> bool msi_en;
> - bool is_runtime_suspended;
>
> #define EXTRA_CAPS_SD_SDR50 (1 << 0)
> #define EXTRA_CAPS_SD_SDR104 (1 << 1)
> --
> 2.33.1
next prev parent reply other threads:[~2022-01-25 6:46 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-20 14:50 [PATCH 1/4] mmc: rtsx: Use pm_runtime_{get,put}() to handle runtime PM Kai-Heng Feng
2022-01-20 14:50 ` [PATCH 2/4] misc: rtsx: Rework runtime power management flow Kai-Heng Feng
2022-01-21 1:40 ` [PATCH v2 " Kai-Heng Feng
2022-01-21 3:57 ` Ricky WU
2022-01-21 4:08 ` Kai-Heng Feng
2022-01-21 4:15 ` Ricky WU
2022-01-21 4:17 ` Kai-Heng Feng
2022-01-21 6:31 ` [PATCH v3 " Kai-Heng Feng
2022-01-21 9:58 ` Ricky WU
2022-01-21 12:39 ` Kai-Heng Feng
2022-01-24 3:26 ` Ricky WU
2022-01-24 4:53 ` Kai-Heng Feng
2022-01-24 5:10 ` Ricky WU
2022-01-24 5:26 ` Kai-Heng Feng
2022-01-24 5:47 ` [PATCH v4 1/4] mmc: rtsx: Use pm_runtime_{get,put}() to handle runtime PM Kai-Heng Feng
2022-01-24 5:47 ` [PATCH v4 2/4] misc: rtsx: Rework runtime power management flow Kai-Heng Feng
2022-01-24 5:47 ` [PATCH v4 3/4] misc: rtsx: Cleanup power management ops Kai-Heng Feng
2022-01-24 5:47 ` [PATCH v4 4/4] misc: rtsx: Quiesce rts5249 on system suspend Kai-Heng Feng
2022-01-24 7:28 ` [PATCH v5 1/4] mmc: rtsx: Use pm_runtime_{get,put}() to handle runtime PM Kai-Heng Feng
2022-01-24 7:28 ` [PATCH v5 2/4] misc: rtsx: Rework runtime power management flow Kai-Heng Feng
2022-01-24 7:28 ` [PATCH v5 3/4] misc: rtsx: Cleanup power management ops Kai-Heng Feng
2022-01-24 7:28 ` [PATCH v5 4/4] misc: rtsx: Quiesce rts5249 on system suspend Kai-Heng Feng
2022-01-25 5:50 ` [PATCH v6 1/4] mmc: rtsx: Use pm_runtime_{get,put}() to handle runtime PM Kai-Heng Feng
2022-01-25 5:50 ` [PATCH v6 2/4] misc: rtsx: Rework runtime power management flow Kai-Heng Feng
2022-01-25 6:38 ` Ricky WU [this message]
2022-01-25 5:50 ` [PATCH v6 3/4] misc: rtsx: Cleanup power management ops Kai-Heng Feng
2022-01-25 6:38 ` Ricky WU
2022-01-25 5:50 ` [PATCH v6 4/4] misc: rtsx: Quiesce rts5249 on system suspend Kai-Heng Feng
2022-01-25 6:38 ` Ricky WU
2022-01-25 6:37 ` [PATCH v6 1/4] mmc: rtsx: Use pm_runtime_{get,put}() to handle runtime PM Ricky WU
2022-02-04 12:28 ` Ulf Hansson
2022-02-16 4:21 ` Kai-Heng Feng
2022-02-16 10:22 ` Ulf Hansson
2022-01-20 14:50 ` [PATCH 3/4] misc: rtsx: Cleanup power management ops Kai-Heng Feng
2022-01-20 14:50 ` [PATCH 4/4] misc: rtsx: Quiesce rts5249 on system suspend Kai-Heng Feng
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=aee7446ff3234dc7adb02202d70b121a@realtek.com \
--to=ricky_wu@realtek.com \
--cc=arnd@arndb.de \
--cc=christophe.jaillet@wanadoo.fr \
--cc=gregkh@linuxfoundation.org \
--cc=kai.heng.feng@canonical.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=ulf.hansson@linaro.org \
--cc=yang.lee@linux.alibaba.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.