From: Can Guo <cang@codeaurora.org>
To: Adrian Hunter <adrian.hunter@intel.com>
Cc: asutoshd@codeaurora.org, nguyenb@codeaurora.org,
hongwus@codeaurora.org, ziqichen@codeaurora.org,
linux-scsi@vger.kernel.org, kernel-team@android.com,
Alim Akhtar <alim.akhtar@samsung.com>,
Avri Altman <avri.altman@wdc.com>,
"James E.J. Bottomley" <jejb@linux.ibm.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
Stanley Chu <stanley.chu@mediatek.com>,
Bean Huo <beanhuo@micron.com>, Jaegeuk Kim <jaegeuk@kernel.org>,
Kiwoong Kim <kwmad.kim@samsung.com>,
Satya Tangirala <satyat@google.com>,
Bart Van Assche <bvanassche@acm.org>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 1/9] scsi: ufs: Differentiate status between hba pm ops and wl pm ops
Date: Fri, 11 Jun 2021 08:53:46 +0800 [thread overview]
Message-ID: <d53df7c79e6511a1257affefc69f4cf3@codeaurora.org> (raw)
In-Reply-To: <7d7a771f-6595-0106-8ee5-4e6407caee56@intel.com>
Hi Adrian,
On 2021-06-10 19:15, Adrian Hunter wrote:
> On 10/06/21 7:43 am, Can Guo wrote:
>> Put pm_op_in_progress and is_sys_suspend flags back to ufshcd hba pm
>> ops,
>> add two new flags, namely wl_pm_op_in_progress and
>> is_wl_sys_suspended, to
>> track the UFS device W-LU pm ops. This helps us differentiate the
>> status of
>> hba and wl pm ops when we need to do troubleshooting.
>
> Really you have 2 changes here:
> 1. Renaming to pm_op_in_progress / is_sys_suspend to
> wl_pm_op_in_progress / is_wl_sys_suspended
> 2. Introducing flags for the status of hba
>
> So it should really be 2 patches.
Sure I will make it 2 in next version.
>
> That would show up things like:
> - did you intend not to change hba->is_sys_suspended in
> ufs_qcom_resume() ?
I missed it - shall change it in next version.
Thanks,
Can Guo.
>
>>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> ---
>> drivers/scsi/ufs/ufshcd.c | 42
>> ++++++++++++++++++++++++++++--------------
>> drivers/scsi/ufs/ufshcd.h | 4 +++-
>> 2 files changed, 31 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 25fe18a..47b2a9a 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -549,7 +549,9 @@ static void ufshcd_print_host_state(struct ufs_hba
>> *hba)
>> hba->saved_err, hba->saved_uic_err);
>> dev_err(hba->dev, "Device power mode=%d, UIC link state=%d\n",
>> hba->curr_dev_pwr_mode, hba->uic_link_state);
>> - dev_err(hba->dev, "PM in progress=%d, sys. suspended=%d\n",
>> + dev_err(hba->dev, "wl_pm_op_in_progress=%d,
>> is_wl_sys_suspended=%d\n",
>> + hba->wl_pm_op_in_progress, hba->is_wl_sys_suspended);
>> + dev_err(hba->dev, "pm_op_in_progress=%d, is_sys_suspended=%d\n",
>> hba->pm_op_in_progress, hba->is_sys_suspended);
>> dev_err(hba->dev, "Auto BKOPS=%d, Host self-block=%d\n",
>> hba->auto_bkops_enabled, hba->host->host_self_blocked);
>> @@ -1999,7 +2001,7 @@ static void ufshcd_clk_scaling_start_busy(struct
>> ufs_hba *hba)
>> if (!hba->clk_scaling.active_reqs++)
>> queue_resume_work = true;
>>
>> - if (!hba->clk_scaling.is_enabled || hba->pm_op_in_progress) {
>> + if (!hba->clk_scaling.is_enabled || hba->wl_pm_op_in_progress) {
>> spin_unlock_irqrestore(hba->host->host_lock, flags);
>> return;
>> }
>> @@ -2734,7 +2736,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
>> *host, struct scsi_cmnd *cmd)
>> * err handler blocked for too long. So, just fail the scsi cmd
>> * sent from PM ops, err handler can recover PM error anyways.
>> */
>> - if (hba->pm_op_in_progress) {
>> + if (hba->wl_pm_op_in_progress) {
>> hba->force_reset = true;
>> set_host_byte(cmd, DID_BAD_TARGET);
>> cmd->scsi_done(cmd);
>> @@ -2767,7 +2769,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
>> *host, struct scsi_cmnd *cmd)
>> (hba->clk_gating.state != CLKS_ON));
>>
>> if (unlikely(test_bit(tag, &hba->outstanding_reqs))) {
>> - if (hba->pm_op_in_progress)
>> + if (hba->wl_pm_op_in_progress)
>> set_host_byte(cmd, DID_BAD_TARGET);
>> else
>> err = SCSI_MLQUEUE_HOST_BUSY;
>> @@ -5116,7 +5118,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
>> struct ufshcd_lrb *lrbp)
>> * solution could be to abort the system suspend if
>> * UFS device needs urgent BKOPs.
>> */
>> - if (!hba->pm_op_in_progress &&
>> + if (!hba->wl_pm_op_in_progress &&
>> !ufshcd_eh_in_progress(hba) &&
>> ufshcd_is_exception_event(lrbp->ucd_rsp_ptr))
>> /* Flushed in suspend */
>> @@ -5916,7 +5918,7 @@ static void ufshcd_err_handling_prepare(struct
>> ufs_hba *hba)
>> {
>> ufshcd_rpm_get_sync(hba);
>> if (pm_runtime_status_suspended(&hba->sdev_ufs_device->sdev_gendev)
>> ||
>> - hba->is_sys_suspended) {
>> + hba->is_wl_sys_suspended) {
>> enum ufs_pm_op pm_op;
>>
>> /*
>> @@ -5933,7 +5935,7 @@ static void ufshcd_err_handling_prepare(struct
>> ufs_hba *hba)
>> if (!ufshcd_is_clkgating_allowed(hba))
>> ufshcd_setup_clocks(hba, true);
>> ufshcd_release(hba);
>> - pm_op = hba->is_sys_suspended ? UFS_SYSTEM_PM : UFS_RUNTIME_PM;
>> + pm_op = hba->is_wl_sys_suspended ? UFS_SYSTEM_PM : UFS_RUNTIME_PM;
>> ufshcd_vops_resume(hba, pm_op);
>> } else {
>> ufshcd_hold(hba, false);
>> @@ -5976,7 +5978,7 @@ static void ufshcd_recover_pm_error(struct
>> ufs_hba *hba)
>> struct request_queue *q;
>> int ret;
>>
>> - hba->is_sys_suspended = false;
>> + hba->is_wl_sys_suspended = false;
>> /*
>> * Set RPM status of wlun device to RPM_ACTIVE,
>> * this also clears its runtime error.
>> @@ -8784,7 +8786,7 @@ static int __ufshcd_wl_suspend(struct ufs_hba
>> *hba, enum ufs_pm_op pm_op)
>> enum ufs_dev_pwr_mode req_dev_pwr_mode;
>> enum uic_link_state req_link_state;
>>
>> - hba->pm_op_in_progress = true;
>> + hba->wl_pm_op_in_progress = true;
>> if (pm_op != UFS_SHUTDOWN_PM) {
>> pm_lvl = pm_op == UFS_RUNTIME_PM ?
>> hba->rpm_lvl : hba->spm_lvl;
>> @@ -8919,7 +8921,7 @@ static int __ufshcd_wl_suspend(struct ufs_hba
>> *hba, enum ufs_pm_op pm_op)
>> hba->clk_gating.is_suspended = false;
>> ufshcd_release(hba);
>> }
>> - hba->pm_op_in_progress = false;
>> + hba->wl_pm_op_in_progress = false;
>> return ret;
>> }
>>
>> @@ -8928,7 +8930,7 @@ static int __ufshcd_wl_resume(struct ufs_hba
>> *hba, enum ufs_pm_op pm_op)
>> int ret;
>> enum uic_link_state old_link_state = hba->uic_link_state;
>>
>> - hba->pm_op_in_progress = true;
>> + hba->wl_pm_op_in_progress = true;
>>
>> /*
>> * Call vendor specific resume callback. As these callbacks may
>> access
>> @@ -9006,7 +9008,7 @@ static int __ufshcd_wl_resume(struct ufs_hba
>> *hba, enum ufs_pm_op pm_op)
>> ufshcd_update_evt_hist(hba, UFS_EVT_WL_RES_ERR, (u32)ret);
>> hba->clk_gating.is_suspended = false;
>> ufshcd_release(hba);
>> - hba->pm_op_in_progress = false;
>> + hba->wl_pm_op_in_progress = false;
>> return ret;
>> }
>>
>> @@ -9072,7 +9074,7 @@ static int ufshcd_wl_suspend(struct device *dev)
>>
>> out:
>> if (!ret)
>> - hba->is_sys_suspended = true;
>> + hba->is_wl_sys_suspended = true;
>> trace_ufshcd_wl_suspend(dev_name(dev), ret,
>> ktime_to_us(ktime_sub(ktime_get(), start)),
>> hba->curr_dev_pwr_mode, hba->uic_link_state);
>> @@ -9100,7 +9102,7 @@ static int ufshcd_wl_resume(struct device *dev)
>> ktime_to_us(ktime_sub(ktime_get(), start)),
>> hba->curr_dev_pwr_mode, hba->uic_link_state);
>> if (!ret)
>> - hba->is_sys_suspended = false;
>> + hba->is_wl_sys_suspended = false;
>> up(&hba->host_sem);
>> return ret;
>> }
>> @@ -9141,6 +9143,8 @@ static int ufshcd_suspend(struct ufs_hba *hba)
>>
>> if (!hba->is_powered)
>> return 0;
>> +
>> + hba->pm_op_in_progress = true;
>> /*
>> * Disable the host irq as host controller as there won't be any
>> * host controller transaction expected till resume.
>> @@ -9160,6 +9164,7 @@ static int ufshcd_suspend(struct ufs_hba *hba)
>> ufshcd_vreg_set_lpm(hba);
>> /* Put the host controller in low power mode if possible */
>> ufshcd_hba_vreg_set_lpm(hba);
>> + hba->pm_op_in_progress = false;
>> return ret;
>> }
>>
>> @@ -9179,6 +9184,7 @@ static int ufshcd_resume(struct ufs_hba *hba)
>> if (!hba->is_powered)
>> return 0;
>>
>> + hba->pm_op_in_progress = true;
>> ufshcd_hba_vreg_set_hpm(hba);
>> ret = ufshcd_vreg_set_hpm(hba);
>> if (ret)
>> @@ -9198,6 +9204,7 @@ static int ufshcd_resume(struct ufs_hba *hba)
>> out:
>> if (ret)
>> ufshcd_update_evt_hist(hba, UFS_EVT_RESUME_ERR, (u32)ret);
>> + hba->pm_op_in_progress = false;
>> return ret;
>> }
>>
>> @@ -9222,6 +9229,10 @@ int ufshcd_system_suspend(struct ufs_hba *hba)
>> trace_ufshcd_system_suspend(dev_name(hba->dev), ret,
>> ktime_to_us(ktime_sub(ktime_get(), start)),
>> hba->curr_dev_pwr_mode, hba->uic_link_state);
>> +
>> + if (!ret)
>> + hba->is_sys_suspended = true;
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL(ufshcd_system_suspend);
>> @@ -9248,6 +9259,9 @@ int ufshcd_system_resume(struct ufs_hba *hba)
>> ktime_to_us(ktime_sub(ktime_get(), start)),
>> hba->curr_dev_pwr_mode, hba->uic_link_state);
>>
>> + if (!ret)
>> + hba->is_sys_suspended = false;
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL(ufshcd_system_resume);
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index c98d540..eaebb4e 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -752,7 +752,8 @@ struct ufs_hba {
>> enum ufs_pm_level spm_lvl;
>> struct device_attribute rpm_lvl_attr;
>> struct device_attribute spm_lvl_attr;
>> - int pm_op_in_progress;
>> + bool pm_op_in_progress;
>> + bool wl_pm_op_in_progress;
>>
>> /* Auto-Hibernate Idle Timer register value */
>> u32 ahit;
>> @@ -839,6 +840,7 @@ struct ufs_hba {
>> struct devfreq *devfreq;
>> struct ufs_clk_scaling clk_scaling;
>> bool is_sys_suspended;
>> + bool is_wl_sys_suspended;
>>
>> enum bkops_status urgent_bkops_lvl;
>> bool is_urgent_bkops_lvl_checked;
>>
next prev parent reply other threads:[~2021-06-11 0:54 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 4:43 [PATCH v3 0/9] Complementary changes for error handling Can Guo
2021-06-10 4:43 ` [PATCH v3 1/9] scsi: ufs: Differentiate status between hba pm ops and wl pm ops Can Guo
2021-06-10 11:15 ` Adrian Hunter
2021-06-11 0:53 ` Can Guo [this message]
2021-06-11 20:40 ` Bart Van Assche
2021-06-12 6:20 ` Can Guo
2021-06-16 17:50 ` Bart Van Assche
2021-06-23 1:32 ` Can Guo
2021-06-10 4:43 ` [PATCH v3 2/9] scsi: ufs: Update the return value of supplier " Can Guo
2021-06-10 4:43 ` [PATCH v3 3/9] scsi: ufs: Enable IRQ after enabling clocks in error handling preparation Can Guo
2021-06-10 4:43 ` [PATCH v3 4/9] scsi: ufs: Complete the cmd before returning in queuecommand Can Guo
2021-06-11 20:52 ` Bart Van Assche
2021-06-12 7:38 ` Can Guo
2021-06-12 15:50 ` Bart Van Assche
2021-06-13 13:30 ` Can Guo
2021-06-10 4:43 ` [PATCH v3 5/9] scsi: ufs: Simplify error handling preparation Can Guo
2021-06-10 12:30 ` Adrian Hunter
2021-06-11 3:01 ` Can Guo
2021-06-11 20:58 ` Bart Van Assche
2021-06-12 6:46 ` Can Guo
2021-06-12 9:49 ` Can Guo
2021-06-10 4:43 ` [PATCH v3 6/9] scsi: ufs: Update ufshcd_recover_pm_error() Can Guo
2021-06-10 4:43 ` [PATCH v3 7/9] scsi: ufs: Let host_sem cover the entire system suspend/resume Can Guo
2021-06-10 13:32 ` Adrian Hunter
2021-06-11 3:06 ` Can Guo
2021-06-11 21:00 ` Bart Van Assche
2021-06-12 6:46 ` Can Guo
2021-06-10 4:43 ` [PATCH v3 8/9] scsi: ufs: Update the fast abort path in ufshcd_abort() for PM requests Can Guo
2021-06-11 21:02 ` Bart Van Assche
2021-06-12 7:07 ` Can Guo
2021-06-12 16:50 ` Bart Van Assche
2021-06-13 14:42 ` Can Guo
2021-06-14 18:49 ` Bart Van Assche
2021-06-15 2:36 ` Can Guo
2021-06-15 3:17 ` Can Guo
2021-06-15 18:25 ` Bart Van Assche
2021-06-16 4:00 ` Can Guo
2021-06-16 4:40 ` Bart Van Assche
2021-06-16 8:47 ` Can Guo
2021-06-16 17:55 ` Bart Van Assche
2021-06-23 1:34 ` Can Guo
2021-06-10 4:43 ` [PATCH v3 9/9] scsi: ufs: Apply more limitations to user access Can Guo
2021-06-11 21:03 ` Bart Van Assche
2021-06-12 7:13 ` Can Guo
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=d53df7c79e6511a1257affefc69f4cf3@codeaurora.org \
--to=cang@codeaurora.org \
--cc=adrian.hunter@intel.com \
--cc=alim.akhtar@samsung.com \
--cc=asutoshd@codeaurora.org \
--cc=avri.altman@wdc.com \
--cc=beanhuo@micron.com \
--cc=bvanassche@acm.org \
--cc=hongwus@codeaurora.org \
--cc=jaegeuk@kernel.org \
--cc=jejb@linux.ibm.com \
--cc=kernel-team@android.com \
--cc=kwmad.kim@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=nguyenb@codeaurora.org \
--cc=satyat@google.com \
--cc=stanley.chu@mediatek.com \
--cc=ziqichen@codeaurora.org \
/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.