* [PATCH] scsi: ufs: Fix Runtime PM
@ 2017-11-13 9:14 Michal Potomski
2017-11-21 19:45 ` Subhash Jadavani
0 siblings, 1 reply; 7+ messages in thread
From: Michal Potomski @ 2017-11-13 9:14 UTC (permalink / raw)
To: linux-scsi
Cc: vinholikatti, martin.petersen, jejb, subhashj, szymonx.mielczarek
From: Michał Potomski <michalx.potomski@intel.com>
Recent testing of Runtime PM for UFS has shown it's not
working as intended. To acheive fully working Runtime PM,
first we have to put back scsi_device autopm reference counter.
Existing implementation was prone to races and not working for
tranfsers to sleeping devices. This commit aims to fix this
to acheive fully working environment. Due to runtime PM being
previously disabled by not putting scsi_device autopm counter,
the Runtime PM is set to be forbidden as default, to not cause
problems with hosts and devices, which do not fully support
different Device and Link states.
It can be still enabled through sysFS power/control attribute.
Changes since v1:
- Fix compilation error for certain kernel configs,
- Move pm_mark_last_busy(), only to relevant context.
Signed-off-by: Michał Potomski <michalx.potomski@intel.com>
---
drivers/scsi/ufs/ufshcd-pci.c | 4 ++-
drivers/scsi/ufs/ufshcd.c | 64 +++++++++++++++++++++++++++++++++++--------
2 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 925b0ec7ec54..4fffb1123876 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -184,8 +184,10 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
}
pci_set_drvdata(pdev, hba);
+ pm_runtime_set_autosuspend_delay(&pdev->dev, 3000);
+ pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_put_noidle(&pdev->dev);
- pm_runtime_allow(&pdev->dev);
+ pm_runtime_forbid(&pdev->dev);
return 0;
}
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 011c3369082c..7ff7f51afe26 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -240,6 +240,37 @@ static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag)
return tag >= 0 && tag < hba->nutrs;
}
+#ifdef CONFIG_PM
+static void ufshcd_pm_runtime_get(struct device *dev)
+{
+ // Don't trigger PM events while in transition states
+ if (dev->power.runtime_status == RPM_RESUMING ||
+ dev->power.runtime_status == RPM_SUSPENDING)
+ pm_runtime_get_noresume(dev);
+ else
+ pm_runtime_get_sync(dev);
+}
+
+static void ufshcd_pm_runtime_put(struct device *dev)
+{
+ // Don't trigger PM events while in transition states
+ if (dev->power.runtime_status == RPM_RESUMING ||
+ dev->power.runtime_status == RPM_SUSPENDING)
+ pm_runtime_put_noidle(dev);
+ else {
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+ }
+}
+#else
+static void ufshcd_pm_runtime_get(struct device *dev)
+{
+}
+static void ufshcd_pm_runtime_put(struct device *dev)
+{
+}
+#endif
+
static inline int ufshcd_enable_irq(struct ufs_hba *hba)
{
int ret = 0;
@@ -1345,7 +1376,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
if (value == hba->clk_scaling.is_allowed)
goto out;
- pm_runtime_get_sync(hba->dev);
+ ufshcd_pm_runtime_get(hba->dev);
ufshcd_hold(hba, false);
cancel_work_sync(&hba->clk_scaling.suspend_work);
@@ -1364,7 +1395,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
}
ufshcd_release(hba);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
out:
return count;
}
@@ -1948,6 +1979,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
unsigned long flags;
ufshcd_hold(hba, false);
+ ufshcd_pm_runtime_get(hba->dev);
mutex_lock(&hba->uic_cmd_mutex);
ufshcd_add_delay_before_dme_cmd(hba);
@@ -1959,6 +1991,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
mutex_unlock(&hba->uic_cmd_mutex);
+ ufshcd_pm_runtime_put(hba->dev);
ufshcd_release(hba);
return ret;
}
@@ -2345,6 +2378,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
clear_bit_unlock(tag, &hba->lrb_in_use);
goto out;
}
+ ufshcd_pm_runtime_get(hba->dev);
WARN_ON(hba->clk_gating.state != CLKS_ON);
lrbp = &hba->lrb[tag];
@@ -3555,6 +3589,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
int ret;
bool reenable_intr = false;
+ ufshcd_pm_runtime_get(hba->dev);
mutex_lock(&hba->uic_cmd_mutex);
init_completion(&uic_async_done);
ufshcd_add_delay_before_dme_cmd(hba);
@@ -3609,6 +3644,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
spin_unlock_irqrestore(hba->host->host_lock, flags);
mutex_unlock(&hba->uic_cmd_mutex);
+ ufshcd_pm_runtime_put(hba->dev);
return ret;
}
@@ -4386,6 +4422,7 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
+ scsi_autopm_put_device(sdev);
return 0;
}
@@ -4622,6 +4659,7 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba,
/* Do not touch lrbp after scsi done */
cmd->scsi_done(cmd);
__ufshcd_release(hba);
+ ufshcd_pm_runtime_put(hba->dev);
} else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE ||
lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) {
if (hba->dev_cmd.complete) {
@@ -4951,7 +4989,7 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
u32 status = 0;
hba = container_of(work, struct ufs_hba, eeh_work);
- pm_runtime_get_sync(hba->dev);
+ ufshcd_pm_runtime_get(hba->dev);
err = ufshcd_get_ee_status(hba, &status);
if (err) {
dev_err(hba->dev, "%s: failed to get exception status %d\n",
@@ -4965,7 +5003,7 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
ufshcd_bkops_exception_event_handler(hba);
out:
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
return;
}
@@ -5065,7 +5103,7 @@ static void ufshcd_err_handler(struct work_struct *work)
hba = container_of(work, struct ufs_hba, eh_work);
- pm_runtime_get_sync(hba->dev);
+ ufshcd_pm_runtime_get(hba->dev);
ufshcd_hold(hba, false);
spin_lock_irqsave(hba->host->host_lock, flags);
@@ -5177,7 +5215,7 @@ static void ufshcd_err_handler(struct work_struct *work)
spin_unlock_irqrestore(hba->host->host_lock, flags);
scsi_unblock_requests(hba->host);
ufshcd_release(hba);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
}
static void ufshcd_update_uic_reg_hist(struct ufs_uic_err_reg_hist *reg_hist,
@@ -6425,7 +6463,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
}
scsi_scan_host(hba->host);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
}
if (!hba->is_init_prefetch)
@@ -6437,7 +6475,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
* present, turn off the power/clocks etc.
*/
if (ret && !ufshcd_eh_in_progress(hba) && !hba->pm_op_in_progress) {
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
ufshcd_hba_exit(hba);
}
@@ -7747,6 +7785,8 @@ EXPORT_SYMBOL(ufshcd_shutdown);
void ufshcd_remove(struct ufs_hba *hba)
{
ufshcd_remove_sysfs_nodes(hba);
+ // Resume if suspended for sync
+ ufshcd_pm_runtime_get(hba->dev);
scsi_remove_host(hba->host);
/* disable interrupts */
ufshcd_disable_intr(hba, hba->intr_mask);
@@ -7756,6 +7796,8 @@ void ufshcd_remove(struct ufs_hba *hba)
if (ufshcd_is_clkscaling_supported(hba))
device_remove_file(hba->dev, &hba->clk_scaling.enable_attr);
ufshcd_hba_exit(hba);
+ // Final sync finished - put it back
+ ufshcd_pm_runtime_put(hba->dev);
}
EXPORT_SYMBOL_GPL(ufshcd_remove);
@@ -7978,11 +8020,11 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
UFS_SLEEP_PWR_MODE,
UIC_LINK_HIBERN8_STATE);
hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
- UFS_SLEEP_PWR_MODE,
- UIC_LINK_HIBERN8_STATE);
+ UFS_POWERDOWN_PWR_MODE,
+ UIC_LINK_OFF_STATE);
/* Hold auto suspend until async scan completes */
- pm_runtime_get_sync(dev);
+ ufshcd_pm_runtime_get(dev);
/*
* We are assuming that device wasn't put in sleep/power-down
--
2.13.0.67.g10c78a1
--------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: ufs: Fix Runtime PM
2017-11-13 9:14 [PATCH] scsi: ufs: Fix Runtime PM Michal Potomski
@ 2017-11-21 19:45 ` Subhash Jadavani
2017-11-22 10:06 ` Potomski, MichalX
0 siblings, 1 reply; 7+ messages in thread
From: Subhash Jadavani @ 2017-11-21 19:45 UTC (permalink / raw)
To: Michal Potomski
Cc: linux-scsi, vinholikatti, martin.petersen, jejb,
szymonx.mielczarek, linux-scsi-owner
On 2017-11-13 01:14, Michal Potomski wrote:
> From: Michał Potomski <michalx.potomski@intel.com>
>
> Recent testing of Runtime PM for UFS has shown it's not
> working as intended. To acheive fully working Runtime PM,
> first we have to put back scsi_device autopm reference counter.
>
> Existing implementation was prone to races and not working for
> tranfsers to sleeping devices. This commit aims to fix this
> to acheive fully working environment. Due to runtime PM being
> previously disabled by not putting scsi_device autopm counter,
> the Runtime PM is set to be forbidden as default, to not cause
> problems with hosts and devices, which do not fully support
> different Device and Link states.
>
> It can be still enabled through sysFS power/control attribute.
>
> Changes since v1:
> - Fix compilation error for certain kernel configs,
> - Move pm_mark_last_busy(), only to relevant context.
>
> Signed-off-by: Michał Potomski <michalx.potomski@intel.com>
> ---
> drivers/scsi/ufs/ufshcd-pci.c | 4 ++-
> drivers/scsi/ufs/ufshcd.c | 64
> +++++++++++++++++++++++++++++++++++--------
> 2 files changed, 56 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd-pci.c
> b/drivers/scsi/ufs/ufshcd-pci.c
> index 925b0ec7ec54..4fffb1123876 100644
> --- a/drivers/scsi/ufs/ufshcd-pci.c
> +++ b/drivers/scsi/ufs/ufshcd-pci.c
> @@ -184,8 +184,10 @@ ufshcd_pci_probe(struct pci_dev *pdev, const
> struct pci_device_id *id)
> }
>
> pci_set_drvdata(pdev, hba);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, 3000);
> + pm_runtime_use_autosuspend(&pdev->dev);
> pm_runtime_put_noidle(&pdev->dev);
> - pm_runtime_allow(&pdev->dev);
> + pm_runtime_forbid(&pdev->dev);
>
> return 0;
> }
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 011c3369082c..7ff7f51afe26 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -240,6 +240,37 @@ static inline bool ufshcd_valid_tag(struct
> ufs_hba *hba, int tag)
> return tag >= 0 && tag < hba->nutrs;
> }
>
> +#ifdef CONFIG_PM
> +static void ufshcd_pm_runtime_get(struct device *dev)
> +{
> + // Don't trigger PM events while in transition states
> + if (dev->power.runtime_status == RPM_RESUMING ||
> + dev->power.runtime_status == RPM_SUSPENDING)
Why do we need to check this? Isn't this check race condition prone?
> + pm_runtime_get_noresume(dev);
> + else
> + pm_runtime_get_sync(dev);
> +}
> +
> +static void ufshcd_pm_runtime_put(struct device *dev)
> +{
> + // Don't trigger PM events while in transition states
> + if (dev->power.runtime_status == RPM_RESUMING ||
> + dev->power.runtime_status == RPM_SUSPENDING)
> + pm_runtime_put_noidle(dev);
> + else {
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> + }
> +}
> +#else
> +static void ufshcd_pm_runtime_get(struct device *dev)
> +{
> +}
> +static void ufshcd_pm_runtime_put(struct device *dev)
> +{
> +}
> +#endif
> +
> static inline int ufshcd_enable_irq(struct ufs_hba *hba)
> {
> int ret = 0;
> @@ -1345,7 +1376,7 @@ static ssize_t
> ufshcd_clkscale_enable_store(struct device *dev,
> if (value == hba->clk_scaling.is_allowed)
> goto out;
>
> - pm_runtime_get_sync(hba->dev);
> + ufshcd_pm_runtime_get(hba->dev);
> ufshcd_hold(hba, false);
>
> cancel_work_sync(&hba->clk_scaling.suspend_work);
> @@ -1364,7 +1395,7 @@ static ssize_t
> ufshcd_clkscale_enable_store(struct device *dev,
> }
>
> ufshcd_release(hba);
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> out:
> return count;
> }
> @@ -1948,6 +1979,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct
> uic_command *uic_cmd)
> unsigned long flags;
>
> ufshcd_hold(hba, false);
> + ufshcd_pm_runtime_get(hba->dev);
> mutex_lock(&hba->uic_cmd_mutex);
> ufshcd_add_delay_before_dme_cmd(hba);
>
> @@ -1959,6 +1991,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct
> uic_command *uic_cmd)
>
> mutex_unlock(&hba->uic_cmd_mutex);
>
> + ufshcd_pm_runtime_put(hba->dev);
> ufshcd_release(hba);
> return ret;
> }
> @@ -2345,6 +2378,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
> *host, struct scsi_cmnd *cmd)
> clear_bit_unlock(tag, &hba->lrb_in_use);
> goto out;
> }
> + ufshcd_pm_runtime_get(hba->dev);
why do we need to hold PM reference count here? I thought as ufs host is
parent of scsi_host/scsi_device, scsi would make sure that UFS host is
runtime resumed before any access to it.
> WARN_ON(hba->clk_gating.state != CLKS_ON);
>
> lrbp = &hba->lrb[tag];
> @@ -3555,6 +3589,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
> int ret;
> bool reenable_intr = false;
>
> + ufshcd_pm_runtime_get(hba->dev);
> mutex_lock(&hba->uic_cmd_mutex);
> init_completion(&uic_async_done);
> ufshcd_add_delay_before_dme_cmd(hba);
> @@ -3609,6 +3644,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
> ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> mutex_unlock(&hba->uic_cmd_mutex);
> + ufshcd_pm_runtime_put(hba->dev);
>
> return ret;
> }
> @@ -4386,6 +4422,7 @@ static int ufshcd_slave_configure(struct
> scsi_device *sdev)
>
> blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
> blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
> + scsi_autopm_put_device(sdev);
scsi_sysfs_add_sdev() calls scsi_autopm_put_device() so not sure why
this is required here?
>
> return 0;
> }
> @@ -4622,6 +4659,7 @@ static void __ufshcd_transfer_req_compl(struct
> ufs_hba *hba,
> /* Do not touch lrbp after scsi done */
> cmd->scsi_done(cmd);
> __ufshcd_release(hba);
> + ufshcd_pm_runtime_put(hba->dev);
> } else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE ||
> lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) {
> if (hba->dev_cmd.complete) {
> @@ -4951,7 +4989,7 @@ static void
> ufshcd_exception_event_handler(struct work_struct *work)
> u32 status = 0;
> hba = container_of(work, struct ufs_hba, eeh_work);
>
> - pm_runtime_get_sync(hba->dev);
> + ufshcd_pm_runtime_get(hba->dev);
> err = ufshcd_get_ee_status(hba, &status);
> if (err) {
> dev_err(hba->dev, "%s: failed to get exception status %d\n",
> @@ -4965,7 +5003,7 @@ static void
> ufshcd_exception_event_handler(struct work_struct *work)
> ufshcd_bkops_exception_event_handler(hba);
>
> out:
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> return;
> }
>
> @@ -5065,7 +5103,7 @@ static void ufshcd_err_handler(struct work_struct
> *work)
>
> hba = container_of(work, struct ufs_hba, eh_work);
>
> - pm_runtime_get_sync(hba->dev);
> + ufshcd_pm_runtime_get(hba->dev);
> ufshcd_hold(hba, false);
>
> spin_lock_irqsave(hba->host->host_lock, flags);
> @@ -5177,7 +5215,7 @@ static void ufshcd_err_handler(struct work_struct
> *work)
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> scsi_unblock_requests(hba->host);
> ufshcd_release(hba);
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> }
>
> static void ufshcd_update_uic_reg_hist(struct ufs_uic_err_reg_hist
> *reg_hist,
> @@ -6425,7 +6463,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> }
>
> scsi_scan_host(hba->host);
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> }
>
> if (!hba->is_init_prefetch)
> @@ -6437,7 +6475,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> * present, turn off the power/clocks etc.
> */
> if (ret && !ufshcd_eh_in_progress(hba) && !hba->pm_op_in_progress) {
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> ufshcd_hba_exit(hba);
> }
>
> @@ -7747,6 +7785,8 @@ EXPORT_SYMBOL(ufshcd_shutdown);
> void ufshcd_remove(struct ufs_hba *hba)
> {
> ufshcd_remove_sysfs_nodes(hba);
> + // Resume if suspended for sync
> + ufshcd_pm_runtime_get(hba->dev);
> scsi_remove_host(hba->host);
> /* disable interrupts */
> ufshcd_disable_intr(hba, hba->intr_mask);
> @@ -7756,6 +7796,8 @@ void ufshcd_remove(struct ufs_hba *hba)
> if (ufshcd_is_clkscaling_supported(hba))
> device_remove_file(hba->dev, &hba->clk_scaling.enable_attr);
> ufshcd_hba_exit(hba);
> + // Final sync finished - put it back
> + ufshcd_pm_runtime_put(hba->dev);
> }
> EXPORT_SYMBOL_GPL(ufshcd_remove);
>
> @@ -7978,11 +8020,11 @@ int ufshcd_init(struct ufs_hba *hba, void
> __iomem *mmio_base, unsigned int irq)
> UFS_SLEEP_PWR_MODE,
> UIC_LINK_HIBERN8_STATE);
> hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> - UFS_SLEEP_PWR_MODE,
> - UIC_LINK_HIBERN8_STATE);
> + UFS_POWERDOWN_PWR_MODE,
> + UIC_LINK_OFF_STATE);
This may add lot of latencies on resume so not sure we want to change
the default.
>
> /* Hold auto suspend until async scan completes */
> - pm_runtime_get_sync(dev);
> + ufshcd_pm_runtime_get(dev);
>
> /*
> * We are assuming that device wasn't put in sleep/power-down
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] scsi: ufs: Fix Runtime PM
2017-11-21 19:45 ` Subhash Jadavani
@ 2017-11-22 10:06 ` Potomski, MichalX
0 siblings, 0 replies; 7+ messages in thread
From: Potomski, MichalX @ 2017-11-22 10:06 UTC (permalink / raw)
To: Subhash Jadavani
Cc: linux-scsi, vinholikatti, martin.petersen, jejb, Mielczarek,
SzymonX, linux-scsi-owner
On 2017-11-21 20:45, Subhash Jadavani wrote:
> On 2017-11-13 01:14, Michal Potomski wrote:
> > From: Michał Potomski <michalx.potomski@intel.com>
> >
> > Recent testing of Runtime PM for UFS has shown it's not working as
> > intended. To acheive fully working Runtime PM, first we have to put
> > back scsi_device autopm reference counter.
> >
> > Existing implementation was prone to races and not working for
> > tranfsers to sleeping devices. This commit aims to fix this to acheive
> > fully working environment. Due to runtime PM being previously disabled
> > by not putting scsi_device autopm counter, the Runtime PM is set to be
> > forbidden as default, to not cause problems with hosts and devices,
> > which do not fully support different Device and Link states.
> >
> > It can be still enabled through sysFS power/control attribute.
> >
> > Changes since v1:
> > - Fix compilation error for certain kernel configs,
> > - Move pm_mark_last_busy(), only to relevant context.
> >
> > Signed-off-by: Michał Potomski <michalx.potomski@intel.com>
> > ---
> > drivers/scsi/ufs/ufshcd-pci.c | 4 ++-
> > drivers/scsi/ufs/ufshcd.c | 64
> > +++++++++++++++++++++++++++++++++++--------
> > 2 files changed, 56 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd-pci.c
> > b/drivers/scsi/ufs/ufshcd-pci.c index 925b0ec7ec54..4fffb1123876
> > 100644
> > --- a/drivers/scsi/ufs/ufshcd-pci.c
> > +++ b/drivers/scsi/ufs/ufshcd-pci.c
> > @@ -184,8 +184,10 @@ ufshcd_pci_probe(struct pci_dev *pdev, const
> > struct pci_device_id *id)
> > }
> >
> > pci_set_drvdata(pdev, hba);
> > + pm_runtime_set_autosuspend_delay(&pdev->dev, 3000);
> > + pm_runtime_use_autosuspend(&pdev->dev);
> > pm_runtime_put_noidle(&pdev->dev);
> > - pm_runtime_allow(&pdev->dev);
> > + pm_runtime_forbid(&pdev->dev);
> >
> > return 0;
> > }
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 011c3369082c..7ff7f51afe26 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -240,6 +240,37 @@ static inline bool ufshcd_valid_tag(struct
> > ufs_hba *hba, int tag)
> > return tag >= 0 && tag < hba->nutrs; }
> >
> > +#ifdef CONFIG_PM
> > +static void ufshcd_pm_runtime_get(struct device *dev) {
> > + // Don't trigger PM events while in transition states
> > + if (dev->power.runtime_status == RPM_RESUMING ||
> > + dev->power.runtime_status == RPM_SUSPENDING)
>
> Why do we need to check this? Isn't this check race condition prone?
We need to check this, because we might end up in triggering resume, while
suspending or resuming, which will cause deadlock. This is especially important
in ufshcd_queuecommand directive, through which we pass e.g. START/STOP Unit
SCSI command, when resuming or suspending.
> > + pm_runtime_get_noresume(dev);
> > + else
> > + pm_runtime_get_sync(dev);
> > +}
> > +
> > +static void ufshcd_pm_runtime_put(struct device *dev) {
> > + // Don't trigger PM events while in transition states
> > + if (dev->power.runtime_status == RPM_RESUMING ||
> > + dev->power.runtime_status == RPM_SUSPENDING)
> > + pm_runtime_put_noidle(dev);
> > + else {
> > + pm_runtime_mark_last_busy(dev);
> > + pm_runtime_put_autosuspend(dev);
> > + }
> > +}
> > +#else
> > +static void ufshcd_pm_runtime_get(struct device *dev) { } static void
> > +ufshcd_pm_runtime_put(struct device *dev) { } #endif
> > +
> > static inline int ufshcd_enable_irq(struct ufs_hba *hba) {
> > int ret = 0;
> > @@ -1345,7 +1376,7 @@ static ssize_t
> > ufshcd_clkscale_enable_store(struct device *dev,
> > if (value == hba->clk_scaling.is_allowed)
> > goto out;
> >
> > - pm_runtime_get_sync(hba->dev);
> > + ufshcd_pm_runtime_get(hba->dev);
> > ufshcd_hold(hba, false);
> >
> > cancel_work_sync(&hba->clk_scaling.suspend_work);
> > @@ -1364,7 +1395,7 @@ static ssize_t
> > ufshcd_clkscale_enable_store(struct device *dev,
> > }
> >
> > ufshcd_release(hba);
> > - pm_runtime_put_sync(hba->dev);
> > + ufshcd_pm_runtime_put(hba->dev);
> > out:
> > return count;
> > }
> > @@ -1948,6 +1979,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct
> > uic_command *uic_cmd)
> > unsigned long flags;
> >
> > ufshcd_hold(hba, false);
> > + ufshcd_pm_runtime_get(hba->dev);
> > mutex_lock(&hba->uic_cmd_mutex);
> > ufshcd_add_delay_before_dme_cmd(hba);
> >
> > @@ -1959,6 +1991,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct
> > uic_command *uic_cmd)
> >
> > mutex_unlock(&hba->uic_cmd_mutex);
> >
> > + ufshcd_pm_runtime_put(hba->dev);
> > ufshcd_release(hba);
> > return ret;
> > }
> > @@ -2345,6 +2378,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
> > *host, struct scsi_cmnd *cmd)
> > clear_bit_unlock(tag, &hba->lrb_in_use);
> > goto out;
> > }
> > + ufshcd_pm_runtime_get(hba->dev);
>
>
> why do we need to hold PM reference count here? I thought as ufs host is
> parent of scsi_host/scsi_device, scsi would make sure that UFS host is runtime
> resumed before any access to it.
Theoretically yes, but we end up suspended, before getting completion on the
command, which causes abort. Thanks to this reference count we are sure, that
actual device will be suspended after command is completed.
>
> > WARN_ON(hba->clk_gating.state != CLKS_ON);
> >
> > lrbp = &hba->lrb[tag];
> > @@ -3555,6 +3589,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> > *hba, struct uic_command *cmd)
> > int ret;
> > bool reenable_intr = false;
> >
> > + ufshcd_pm_runtime_get(hba->dev);
> > mutex_lock(&hba->uic_cmd_mutex);
> > init_completion(&uic_async_done);
> > ufshcd_add_delay_before_dme_cmd(hba);
> > @@ -3609,6 +3644,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> > *hba, struct uic_command *cmd)
> > ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
> > spin_unlock_irqrestore(hba->host->host_lock, flags);
> > mutex_unlock(&hba->uic_cmd_mutex);
> > + ufshcd_pm_runtime_put(hba->dev);
> >
> > return ret;
> > }
> > @@ -4386,6 +4422,7 @@ static int ufshcd_slave_configure(struct
> > scsi_device *sdev)
> >
> > blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
> > blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
> > + scsi_autopm_put_device(sdev);
>
>
> scsi_sysfs_add_sdev() calls scsi_autopm_put_device() so not sure why this is
> required here?
While I was testing runtime PM, I stumbled upon problem, that device was not
Going to suspend due to active kids. After some research I found a commit
bc4f2401 stating:
"If a high-level driver does
nothing then the device will not be eligible for runtime-suspend
because of the elevated usage count. If a high-level driver wants to
use runtime PM then it can call scsi_autopm_put_device()"
After using this routine only I observed, that runtime PM states has
begun to be triggered, which was not the case earlier (although still
not working properly without the other changes).
>
> >
> > return 0;
> > }
> > @@ -4622,6 +4659,7 @@ static void __ufshcd_transfer_req_compl(struct
> > ufs_hba *hba,
> > /* Do not touch lrbp after scsi done */
> > cmd->scsi_done(cmd);
> > __ufshcd_release(hba);
> > + ufshcd_pm_runtime_put(hba->dev);
> > } else if (lrbp->command_type ==
> UTP_CMD_TYPE_DEV_MANAGE ||
> > lrbp->command_type ==
> UTP_CMD_TYPE_UFS_STORAGE) {
> > if (hba->dev_cmd.complete) {
> > @@ -4951,7 +4989,7 @@ static void
> > ufshcd_exception_event_handler(struct work_struct *work)
> > u32 status = 0;
> > hba = container_of(work, struct ufs_hba, eeh_work);
> >
> > - pm_runtime_get_sync(hba->dev);
> > + ufshcd_pm_runtime_get(hba->dev);
> > err = ufshcd_get_ee_status(hba, &status);
> > if (err) {
> > dev_err(hba->dev, "%s: failed to get exception status %d\n",
> @@
> > -4965,7 +5003,7 @@ static void ufshcd_exception_event_handler(struct
> > work_struct *work)
> > ufshcd_bkops_exception_event_handler(hba);
> >
> > out:
> > - pm_runtime_put_sync(hba->dev);
> > + ufshcd_pm_runtime_put(hba->dev);
> > return;
> > }
> >
> > @@ -5065,7 +5103,7 @@ static void ufshcd_err_handler(struct
> > work_struct
> > *work)
> >
> > hba = container_of(work, struct ufs_hba, eh_work);
> >
> > - pm_runtime_get_sync(hba->dev);
> > + ufshcd_pm_runtime_get(hba->dev);
> > ufshcd_hold(hba, false);
> >
> > spin_lock_irqsave(hba->host->host_lock, flags); @@ -5177,7 +5215,7
> > @@ static void ufshcd_err_handler(struct work_struct
> > *work)
> > spin_unlock_irqrestore(hba->host->host_lock, flags);
> > scsi_unblock_requests(hba->host);
> > ufshcd_release(hba);
> > - pm_runtime_put_sync(hba->dev);
> > + ufshcd_pm_runtime_put(hba->dev);
> > }
> >
> > static void ufshcd_update_uic_reg_hist(struct ufs_uic_err_reg_hist
> > *reg_hist, @@ -6425,7 +6463,7 @@ static int ufshcd_probe_hba(struct
> > ufs_hba *hba)
> > }
> >
> > scsi_scan_host(hba->host);
> > - pm_runtime_put_sync(hba->dev);
> > + ufshcd_pm_runtime_put(hba->dev);
> > }
> >
> > if (!hba->is_init_prefetch)
> > @@ -6437,7 +6475,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> > * present, turn off the power/clocks etc.
> > */
> > if (ret && !ufshcd_eh_in_progress(hba) && !hba->pm_op_in_progress) {
> > - pm_runtime_put_sync(hba->dev);
> > + ufshcd_pm_runtime_put(hba->dev);
> > ufshcd_hba_exit(hba);
> > }
> >
> > @@ -7747,6 +7785,8 @@ EXPORT_SYMBOL(ufshcd_shutdown); void
> > ufshcd_remove(struct ufs_hba *hba) {
> > ufshcd_remove_sysfs_nodes(hba);
> > + // Resume if suspended for sync
> > + ufshcd_pm_runtime_get(hba->dev);
> > scsi_remove_host(hba->host);
> > /* disable interrupts */
> > ufshcd_disable_intr(hba, hba->intr_mask); @@ -7756,6 +7796,8 @@
> void
> > ufshcd_remove(struct ufs_hba *hba)
> > if (ufshcd_is_clkscaling_supported(hba))
> > device_remove_file(hba->dev, &hba->clk_scaling.enable_attr);
> > ufshcd_hba_exit(hba);
> > + // Final sync finished - put it back
> > + ufshcd_pm_runtime_put(hba->dev);
> > }
> > EXPORT_SYMBOL_GPL(ufshcd_remove);
> >
> > @@ -7978,11 +8020,11 @@ int ufshcd_init(struct ufs_hba *hba, void
> > __iomem *mmio_base, unsigned int irq)
> > UFS_SLEEP_PWR_MODE,
> > UIC_LINK_HIBERN8_STATE);
> > hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> > - UFS_SLEEP_PWR_MODE,
> > - UIC_LINK_HIBERN8_STATE);
> > +
> UFS_POWERDOWN_PWR_MODE,
> > + UIC_LINK_OFF_STATE);
>
> This may add lot of latencies on resume so not sure we want to change the
> default.
It will possibly add latencies, but since we are going to system suspend, we'd
most likely to save power at system suspend at the highest possible rate, while
we are more interested in moderate latency increase vs. lesser power save in
runtime suspend.
>
> >
> > /* Hold auto suspend until async scan completes */
> > - pm_runtime_get_sync(dev);
> > + ufshcd_pm_runtime_get(dev);
> >
> > /*
> > * We are assuming that device wasn't put in sleep/power-down
>
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project
--------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] scsi: ufs: Fix Runtime PM
2017-11-09 9:16 Michal Potomski
2017-11-10 12:57 ` kbuild test robot
2017-11-10 16:38 ` kbuild test robot
@ 2017-11-13 8:47 ` Avri Altman
2 siblings, 0 replies; 7+ messages in thread
From: Avri Altman @ 2017-11-13 8:47 UTC (permalink / raw)
To: Michal Potomski, linux-scsi
Cc: vinholikatti, martin.petersen, jejb, subhashj, szymonx.mielczarek
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Michal Potomski
> Sent: Thursday, November 09, 2017 11:17 AM
> To: linux-scsi@vger.kernel.org
> Cc: vinholikatti@gmail.com; martin.petersen@oracle.com;
> jejb@linux.vnet.ibm.com; subhashj@codeaurora.org;
> szymonx.mielczarek@intel.com
> Subject: [PATCH] scsi: ufs: Fix Runtime PM
>
> From: Michał Potomski <michalx.potomski@intel.com>
>
> Recent testing of Runtime PM for UFS has shown it's not working as intended.
> To acheive fully working Runtime PM, first we have to put back scsi_device
> autopm reference counter.
>
> Existing implementation was prone to races and not working for tranfsers to
> sleeping devices.
Wouldn't calling pm_runtime_mark_last_busy() just before pm_runtime_put_autosuspend() suffice?
This commit aims to fix this to acheive fully working
> environment. Due to runtime PM being previously disabled by not putting
> scsi_device autopm counter, the Runtime PM is set to be forbidden as
> default, to not cause problems with hosts and devices, which do not fully
> support different Device and Link states.
>
> It can be still enabled through sysFS power/control attribute.
>
> Signed-off-by: Michał Potomski <michalx.potomski@intel.com>
> ---
> drivers/scsi/ufs/ufshcd-pci.c | 4 +++-
> drivers/scsi/ufs/ufshcd.c | 55 ++++++++++++++++++++++++++++++++++-------
> --
> 2 files changed, 47 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c index
> 925b0ec7ec54..4fffb1123876 100644
> --- a/drivers/scsi/ufs/ufshcd-pci.c
> +++ b/drivers/scsi/ufs/ufshcd-pci.c
> @@ -184,8 +184,10 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *id)
> }
>
> pci_set_drvdata(pdev, hba);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, 3000);
> + pm_runtime_use_autosuspend(&pdev->dev);
> pm_runtime_put_noidle(&pdev->dev);
> - pm_runtime_allow(&pdev->dev);
> + pm_runtime_forbid(&pdev->dev);
>
> return 0;
> }
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index
> 011c3369082c..e7c7ed9bf8a5 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -240,6 +240,28 @@ static inline bool ufshcd_valid_tag(struct ufs_hba
> *hba, int tag)
> return tag >= 0 && tag < hba->nutrs;
> }
>
> +static void ufshcd_pm_runtime_get(struct device *dev) {
> + // Don't trigger PM events while in transition states
> + if (dev->power.runtime_status == RPM_RESUMING ||
> + dev->power.runtime_status == RPM_SUSPENDING)
> + pm_runtime_get_noresume(dev);
> + else
> + pm_runtime_get_sync(dev);
> +}
> +
> +static void ufshcd_pm_runtime_put(struct device *dev) {
> + pm_runtime_mark_last_busy(dev);
> +
> + // Don't trigger PM events while in transition states
> + if (dev->power.runtime_status == RPM_RESUMING ||
> + dev->power.runtime_status == RPM_SUSPENDING)
> + pm_runtime_put_noidle(dev);
> + else
> + pm_runtime_put_autosuspend(dev);
> +}
> +
> static inline int ufshcd_enable_irq(struct ufs_hba *hba) {
> int ret = 0;
> @@ -1345,7 +1367,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct
> device *dev,
> if (value == hba->clk_scaling.is_allowed)
> goto out;
>
> - pm_runtime_get_sync(hba->dev);
> + ufshcd_pm_runtime_get(hba->dev);
> ufshcd_hold(hba, false);
>
> cancel_work_sync(&hba->clk_scaling.suspend_work);
> @@ -1364,7 +1386,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct
> device *dev,
> }
>
> ufshcd_release(hba);
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> out:
> return count;
> }
> @@ -1948,6 +1970,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct
> uic_command *uic_cmd)
> unsigned long flags;
>
> ufshcd_hold(hba, false);
> + ufshcd_pm_runtime_get(hba->dev);
> mutex_lock(&hba->uic_cmd_mutex);
> ufshcd_add_delay_before_dme_cmd(hba);
>
> @@ -1959,6 +1982,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct
> uic_command *uic_cmd)
>
> mutex_unlock(&hba->uic_cmd_mutex);
>
> + ufshcd_pm_runtime_put(hba->dev);
> ufshcd_release(hba);
> return ret;
> }
> @@ -2345,6 +2369,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
> *host, struct scsi_cmnd *cmd)
> clear_bit_unlock(tag, &hba->lrb_in_use);
> goto out;
> }
> + ufshcd_pm_runtime_get(hba->dev);
> WARN_ON(hba->clk_gating.state != CLKS_ON);
>
> lrbp = &hba->lrb[tag];
> @@ -3555,6 +3580,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba,
> struct uic_command *cmd)
> int ret;
> bool reenable_intr = false;
>
> + ufshcd_pm_runtime_get(hba->dev);
> mutex_lock(&hba->uic_cmd_mutex);
> init_completion(&uic_async_done);
> ufshcd_add_delay_before_dme_cmd(hba);
> @@ -3609,6 +3635,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba,
> struct uic_command *cmd)
> ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> mutex_unlock(&hba->uic_cmd_mutex);
> + ufshcd_pm_runtime_put(hba->dev);
>
> return ret;
> }
> @@ -4386,6 +4413,7 @@ static int ufshcd_slave_configure(struct scsi_device
> *sdev)
>
> blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
> blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
> + scsi_autopm_put_device(sdev);
>
> return 0;
> }
> @@ -4622,6 +4650,7 @@ static void __ufshcd_transfer_req_compl(struct
> ufs_hba *hba,
> /* Do not touch lrbp after scsi done */
> cmd->scsi_done(cmd);
> __ufshcd_release(hba);
> + ufshcd_pm_runtime_put(hba->dev);
> } else if (lrbp->command_type ==
> UTP_CMD_TYPE_DEV_MANAGE ||
> lrbp->command_type ==
> UTP_CMD_TYPE_UFS_STORAGE) {
> if (hba->dev_cmd.complete) {
> @@ -4951,7 +4980,7 @@ static void ufshcd_exception_event_handler(struct
> work_struct *work)
> u32 status = 0;
> hba = container_of(work, struct ufs_hba, eeh_work);
>
> - pm_runtime_get_sync(hba->dev);
> + ufshcd_pm_runtime_get(hba->dev);
> err = ufshcd_get_ee_status(hba, &status);
> if (err) {
> dev_err(hba->dev, "%s: failed to get exception status %d\n",
> @@ -4965,7 +4994,7 @@ static void ufshcd_exception_event_handler(struct
> work_struct *work)
> ufshcd_bkops_exception_event_handler(hba);
>
> out:
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> return;
> }
>
> @@ -5065,7 +5094,7 @@ static void ufshcd_err_handler(struct work_struct
> *work)
>
> hba = container_of(work, struct ufs_hba, eh_work);
>
> - pm_runtime_get_sync(hba->dev);
> + ufshcd_pm_runtime_get(hba->dev);
> ufshcd_hold(hba, false);
>
> spin_lock_irqsave(hba->host->host_lock, flags); @@ -5177,7 +5206,7
> @@ static void ufshcd_err_handler(struct work_struct *work)
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> scsi_unblock_requests(hba->host);
> ufshcd_release(hba);
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> }
>
> static void ufshcd_update_uic_reg_hist(struct ufs_uic_err_reg_hist *reg_hist,
> @@ -6425,7 +6454,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> }
>
> scsi_scan_host(hba->host);
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> }
>
> if (!hba->is_init_prefetch)
> @@ -6437,7 +6466,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> * present, turn off the power/clocks etc.
> */
> if (ret && !ufshcd_eh_in_progress(hba) && !hba-
> >pm_op_in_progress) {
> - pm_runtime_put_sync(hba->dev);
> + ufshcd_pm_runtime_put(hba->dev);
> ufshcd_hba_exit(hba);
> }
>
> @@ -7747,6 +7776,8 @@ EXPORT_SYMBOL(ufshcd_shutdown); void
> ufshcd_remove(struct ufs_hba *hba) {
> ufshcd_remove_sysfs_nodes(hba);
> + // Resume if suspended for sync
> + ufshcd_pm_runtime_get(hba->dev);
> scsi_remove_host(hba->host);
> /* disable interrupts */
> ufshcd_disable_intr(hba, hba->intr_mask); @@ -7756,6 +7787,8 @@
> void ufshcd_remove(struct ufs_hba *hba)
> if (ufshcd_is_clkscaling_supported(hba))
> device_remove_file(hba->dev, &hba->clk_scaling.enable_attr);
> ufshcd_hba_exit(hba);
> + // Final sync finished - put it back
> + ufshcd_pm_runtime_put(hba->dev);
> }
> EXPORT_SYMBOL_GPL(ufshcd_remove);
>
> @@ -7978,11 +8011,11 @@ int ufshcd_init(struct ufs_hba *hba, void
> __iomem *mmio_base, unsigned int irq)
> UFS_SLEEP_PWR_MODE,
> UIC_LINK_HIBERN8_STATE);
> hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> - UFS_SLEEP_PWR_MODE,
> - UIC_LINK_HIBERN8_STATE);
> +
> UFS_POWERDOWN_PWR_MODE,
> + UIC_LINK_OFF_STATE);
>
> /* Hold auto suspend until async scan completes */
> - pm_runtime_get_sync(dev);
> + ufshcd_pm_runtime_get(dev);
>
> /*
> * We are assuming that device wasn't put in sleep/power-down
> --
> 2.13.0.67.g10c78a1
>
> --------------------------------------------------------------------
>
> Intel Technology Poland sp. z o.o.
> ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII
> Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-
> 07-52-316 | Kapital zakladowy 200.000 PLN.
>
> Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata
> i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej
> wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie;
> jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
> This e-mail and any attachments may contain confidential material for the
> sole use of the intended recipient(s). If you are not the intended recipient,
> please contact the sender and delete all copies; any review or distribution by
> others is strictly prohibited.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: ufs: Fix Runtime PM
2017-11-09 9:16 Michal Potomski
2017-11-10 12:57 ` kbuild test robot
@ 2017-11-10 16:38 ` kbuild test robot
2017-11-13 8:47 ` Avri Altman
2 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2017-11-10 16:38 UTC (permalink / raw)
To: Michal Potomski
Cc: kbuild-all, linux-scsi, vinholikatti, martin.petersen, jejb,
subhashj, szymonx.mielczarek
[-- Attachment #1: Type: text/plain, Size: 7433 bytes --]
Hi Michał,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.14-rc8 next-20171109]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michal-Potomski/scsi-ufs-Fix-Runtime-PM/20171110-191314
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-randconfig-v0-11102116 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/async.h:15,
from drivers/scsi/ufs/ufshcd.c:40:
drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_pm_runtime_get':
drivers/scsi/ufs/ufshcd.c:246:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
if (dev->power.runtime_status == RPM_RESUMING ||
^
include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/scsi/ufs/ufshcd.c:246:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:247:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
dev->power.runtime_status == RPM_SUSPENDING)
^
include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/scsi/ufs/ufshcd.c:246:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:246:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
if (dev->power.runtime_status == RPM_RESUMING ||
^
include/linux/compiler.h:156:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/scsi/ufs/ufshcd.c:246:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:247:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
dev->power.runtime_status == RPM_SUSPENDING)
^
include/linux/compiler.h:156:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/scsi/ufs/ufshcd.c:246:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:246:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
if (dev->power.runtime_status == RPM_RESUMING ||
^
include/linux/compiler.h:167:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
>> drivers/scsi/ufs/ufshcd.c:246:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:247:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
dev->power.runtime_status == RPM_SUSPENDING)
^
include/linux/compiler.h:167:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
>> drivers/scsi/ufs/ufshcd.c:246:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_pm_runtime_put':
drivers/scsi/ufs/ufshcd.c:258:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
if (dev->power.runtime_status == RPM_RESUMING ||
^
include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
drivers/scsi/ufs/ufshcd.c:258:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:259:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
dev->power.runtime_status == RPM_SUSPENDING)
^
include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
drivers/scsi/ufs/ufshcd.c:258:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:258:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
if (dev->power.runtime_status == RPM_RESUMING ||
^
include/linux/compiler.h:156:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
drivers/scsi/ufs/ufshcd.c:258:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:259:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
dev->power.runtime_status == RPM_SUSPENDING)
^
include/linux/compiler.h:156:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
drivers/scsi/ufs/ufshcd.c:258:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:258:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
if (dev->power.runtime_status == RPM_RESUMING ||
^
include/linux/compiler.h:167:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
drivers/scsi/ufs/ufshcd.c:258:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
drivers/scsi/ufs/ufshcd.c:259:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
dev->power.runtime_status == RPM_SUSPENDING)
^
include/linux/compiler.h:167:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
drivers/scsi/ufs/ufshcd.c:258:2: note: in expansion of macro 'if'
if (dev->power.runtime_status == RPM_RESUMING ||
^~
vim +/if +246 drivers/scsi/ufs/ufshcd.c
242
243 static void ufshcd_pm_runtime_get(struct device *dev)
244 {
245 // Don't trigger PM events while in transition states
> 246 if (dev->power.runtime_status == RPM_RESUMING ||
247 dev->power.runtime_status == RPM_SUSPENDING)
248 pm_runtime_get_noresume(dev);
249 else
250 pm_runtime_get_sync(dev);
251 }
252
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28790 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: ufs: Fix Runtime PM
2017-11-09 9:16 Michal Potomski
@ 2017-11-10 12:57 ` kbuild test robot
2017-11-10 16:38 ` kbuild test robot
2017-11-13 8:47 ` Avri Altman
2 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2017-11-10 12:57 UTC (permalink / raw)
To: Michal Potomski
Cc: kbuild-all, linux-scsi, vinholikatti, martin.petersen, jejb,
subhashj, szymonx.mielczarek
[-- Attachment #1: Type: text/plain, Size: 2217 bytes --]
Hi Michał,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on scsi/for-next]
[also build test ERROR on v4.14-rc8 next-20171109]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michal-Potomski/scsi-ufs-Fix-Runtime-PM/20171110-191314
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: m68k-allyesconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k
All errors (new ones prefixed by >>):
drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_pm_runtime_get':
>> drivers/scsi/ufs/ufshcd.c:246:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
if (dev->power.runtime_status == RPM_RESUMING ||
^
drivers/scsi/ufs/ufshcd.c:247:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
dev->power.runtime_status == RPM_SUSPENDING)
^
drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_pm_runtime_put':
drivers/scsi/ufs/ufshcd.c:258:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
if (dev->power.runtime_status == RPM_RESUMING ||
^
drivers/scsi/ufs/ufshcd.c:259:16: error: 'struct dev_pm_info' has no member named 'runtime_status'
dev->power.runtime_status == RPM_SUSPENDING)
^
vim +246 drivers/scsi/ufs/ufshcd.c
242
243 static void ufshcd_pm_runtime_get(struct device *dev)
244 {
245 // Don't trigger PM events while in transition states
> 246 if (dev->power.runtime_status == RPM_RESUMING ||
247 dev->power.runtime_status == RPM_SUSPENDING)
248 pm_runtime_get_noresume(dev);
249 else
250 pm_runtime_get_sync(dev);
251 }
252
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44359 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] scsi: ufs: Fix Runtime PM
@ 2017-11-09 9:16 Michal Potomski
2017-11-10 12:57 ` kbuild test robot
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Michal Potomski @ 2017-11-09 9:16 UTC (permalink / raw)
To: linux-scsi
Cc: vinholikatti, martin.petersen, jejb, subhashj, szymonx.mielczarek
From: Michał Potomski <michalx.potomski@intel.com>
Recent testing of Runtime PM for UFS has shown it's not
working as intended. To acheive fully working Runtime PM,
first we have to put back scsi_device autopm reference counter.
Existing implementation was prone to races and not working for
tranfsers to sleeping devices. This commit aims to fix this
to acheive fully working environment. Due to runtime PM being
previously disabled by not putting scsi_device autopm counter,
the Runtime PM is set to be forbidden as default, to not cause
problems with hosts and devices, which do not fully support
different Device and Link states.
It can be still enabled through sysFS power/control attribute.
Signed-off-by: Michał Potomski <michalx.potomski@intel.com>
---
drivers/scsi/ufs/ufshcd-pci.c | 4 +++-
drivers/scsi/ufs/ufshcd.c | 55 ++++++++++++++++++++++++++++++++++---------
2 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 925b0ec7ec54..4fffb1123876 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -184,8 +184,10 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
}
pci_set_drvdata(pdev, hba);
+ pm_runtime_set_autosuspend_delay(&pdev->dev, 3000);
+ pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_put_noidle(&pdev->dev);
- pm_runtime_allow(&pdev->dev);
+ pm_runtime_forbid(&pdev->dev);
return 0;
}
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 011c3369082c..e7c7ed9bf8a5 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -240,6 +240,28 @@ static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag)
return tag >= 0 && tag < hba->nutrs;
}
+static void ufshcd_pm_runtime_get(struct device *dev)
+{
+ // Don't trigger PM events while in transition states
+ if (dev->power.runtime_status == RPM_RESUMING ||
+ dev->power.runtime_status == RPM_SUSPENDING)
+ pm_runtime_get_noresume(dev);
+ else
+ pm_runtime_get_sync(dev);
+}
+
+static void ufshcd_pm_runtime_put(struct device *dev)
+{
+ pm_runtime_mark_last_busy(dev);
+
+ // Don't trigger PM events while in transition states
+ if (dev->power.runtime_status == RPM_RESUMING ||
+ dev->power.runtime_status == RPM_SUSPENDING)
+ pm_runtime_put_noidle(dev);
+ else
+ pm_runtime_put_autosuspend(dev);
+}
+
static inline int ufshcd_enable_irq(struct ufs_hba *hba)
{
int ret = 0;
@@ -1345,7 +1367,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
if (value == hba->clk_scaling.is_allowed)
goto out;
- pm_runtime_get_sync(hba->dev);
+ ufshcd_pm_runtime_get(hba->dev);
ufshcd_hold(hba, false);
cancel_work_sync(&hba->clk_scaling.suspend_work);
@@ -1364,7 +1386,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
}
ufshcd_release(hba);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
out:
return count;
}
@@ -1948,6 +1970,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
unsigned long flags;
ufshcd_hold(hba, false);
+ ufshcd_pm_runtime_get(hba->dev);
mutex_lock(&hba->uic_cmd_mutex);
ufshcd_add_delay_before_dme_cmd(hba);
@@ -1959,6 +1982,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
mutex_unlock(&hba->uic_cmd_mutex);
+ ufshcd_pm_runtime_put(hba->dev);
ufshcd_release(hba);
return ret;
}
@@ -2345,6 +2369,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
clear_bit_unlock(tag, &hba->lrb_in_use);
goto out;
}
+ ufshcd_pm_runtime_get(hba->dev);
WARN_ON(hba->clk_gating.state != CLKS_ON);
lrbp = &hba->lrb[tag];
@@ -3555,6 +3580,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
int ret;
bool reenable_intr = false;
+ ufshcd_pm_runtime_get(hba->dev);
mutex_lock(&hba->uic_cmd_mutex);
init_completion(&uic_async_done);
ufshcd_add_delay_before_dme_cmd(hba);
@@ -3609,6 +3635,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
spin_unlock_irqrestore(hba->host->host_lock, flags);
mutex_unlock(&hba->uic_cmd_mutex);
+ ufshcd_pm_runtime_put(hba->dev);
return ret;
}
@@ -4386,6 +4413,7 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
+ scsi_autopm_put_device(sdev);
return 0;
}
@@ -4622,6 +4650,7 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba,
/* Do not touch lrbp after scsi done */
cmd->scsi_done(cmd);
__ufshcd_release(hba);
+ ufshcd_pm_runtime_put(hba->dev);
} else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE ||
lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) {
if (hba->dev_cmd.complete) {
@@ -4951,7 +4980,7 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
u32 status = 0;
hba = container_of(work, struct ufs_hba, eeh_work);
- pm_runtime_get_sync(hba->dev);
+ ufshcd_pm_runtime_get(hba->dev);
err = ufshcd_get_ee_status(hba, &status);
if (err) {
dev_err(hba->dev, "%s: failed to get exception status %d\n",
@@ -4965,7 +4994,7 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
ufshcd_bkops_exception_event_handler(hba);
out:
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
return;
}
@@ -5065,7 +5094,7 @@ static void ufshcd_err_handler(struct work_struct *work)
hba = container_of(work, struct ufs_hba, eh_work);
- pm_runtime_get_sync(hba->dev);
+ ufshcd_pm_runtime_get(hba->dev);
ufshcd_hold(hba, false);
spin_lock_irqsave(hba->host->host_lock, flags);
@@ -5177,7 +5206,7 @@ static void ufshcd_err_handler(struct work_struct *work)
spin_unlock_irqrestore(hba->host->host_lock, flags);
scsi_unblock_requests(hba->host);
ufshcd_release(hba);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
}
static void ufshcd_update_uic_reg_hist(struct ufs_uic_err_reg_hist *reg_hist,
@@ -6425,7 +6454,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
}
scsi_scan_host(hba->host);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
}
if (!hba->is_init_prefetch)
@@ -6437,7 +6466,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
* present, turn off the power/clocks etc.
*/
if (ret && !ufshcd_eh_in_progress(hba) && !hba->pm_op_in_progress) {
- pm_runtime_put_sync(hba->dev);
+ ufshcd_pm_runtime_put(hba->dev);
ufshcd_hba_exit(hba);
}
@@ -7747,6 +7776,8 @@ EXPORT_SYMBOL(ufshcd_shutdown);
void ufshcd_remove(struct ufs_hba *hba)
{
ufshcd_remove_sysfs_nodes(hba);
+ // Resume if suspended for sync
+ ufshcd_pm_runtime_get(hba->dev);
scsi_remove_host(hba->host);
/* disable interrupts */
ufshcd_disable_intr(hba, hba->intr_mask);
@@ -7756,6 +7787,8 @@ void ufshcd_remove(struct ufs_hba *hba)
if (ufshcd_is_clkscaling_supported(hba))
device_remove_file(hba->dev, &hba->clk_scaling.enable_attr);
ufshcd_hba_exit(hba);
+ // Final sync finished - put it back
+ ufshcd_pm_runtime_put(hba->dev);
}
EXPORT_SYMBOL_GPL(ufshcd_remove);
@@ -7978,11 +8011,11 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
UFS_SLEEP_PWR_MODE,
UIC_LINK_HIBERN8_STATE);
hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
- UFS_SLEEP_PWR_MODE,
- UIC_LINK_HIBERN8_STATE);
+ UFS_POWERDOWN_PWR_MODE,
+ UIC_LINK_OFF_STATE);
/* Hold auto suspend until async scan completes */
- pm_runtime_get_sync(dev);
+ ufshcd_pm_runtime_get(dev);
/*
* We are assuming that device wasn't put in sleep/power-down
--
2.13.0.67.g10c78a1
--------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-22 10:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-13 9:14 [PATCH] scsi: ufs: Fix Runtime PM Michal Potomski
2017-11-21 19:45 ` Subhash Jadavani
2017-11-22 10:06 ` Potomski, MichalX
-- strict thread matches above, loose matches on Subject: below --
2017-11-09 9:16 Michal Potomski
2017-11-10 12:57 ` kbuild test robot
2017-11-10 16:38 ` kbuild test robot
2017-11-13 8:47 ` Avri Altman
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.