Change log from v1:
- remove clkgating_enable check in __ufshcd_release
- use __uhfshcd_release instead of active_req.
From: Jaegeuk Kim <[email protected]>
In order to conduct FFU or RPMB operations, UFS needs to clear UAC. This patch
clears it explicitly, so that we could get no failure given early execution.
Cc: Alim Akhtar <[email protected]>
Cc: Avri Altman <[email protected]>
Cc: Can Guo <[email protected]>
Signed-off-by: Jaegeuk Kim <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 70 +++++++++++++++++++++++++++++++++++----
drivers/scsi/ufs/ufshcd.h | 1 +
2 files changed, 65 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 7344353a9167..feb10ebf7a35 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7057,7 +7057,6 @@ static inline void ufshcd_blk_pm_runtime_init(struct scsi_device *sdev)
static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
{
int ret = 0;
- struct scsi_device *sdev_rpmb;
struct scsi_device *sdev_boot;
hba->sdev_ufs_device = __scsi_add_device(hba->host, 0, 0,
@@ -7070,14 +7069,14 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
ufshcd_blk_pm_runtime_init(hba->sdev_ufs_device);
scsi_device_put(hba->sdev_ufs_device);
- sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
+ hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_RPMB_WLUN), NULL);
- if (IS_ERR(sdev_rpmb)) {
- ret = PTR_ERR(sdev_rpmb);
+ if (IS_ERR(hba->sdev_rpmb)) {
+ ret = PTR_ERR(hba->sdev_rpmb);
goto remove_sdev_ufs_device;
}
- ufshcd_blk_pm_runtime_init(sdev_rpmb);
- scsi_device_put(sdev_rpmb);
+ ufshcd_blk_pm_runtime_init(hba->sdev_rpmb);
+ scsi_device_put(hba->sdev_rpmb);
sdev_boot = __scsi_add_device(hba->host, 0, 0,
ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_BOOT_WLUN), NULL);
@@ -7601,6 +7600,63 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
return ret;
}
+static int
+ufshcd_send_request_sense(struct ufs_hba *hba, struct scsi_device *sdp);
+
+static int ufshcd_clear_ua_wlun(struct ufs_hba *hba, u8 wlun)
+{
+ struct scsi_device *sdp;
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ if (wlun == UFS_UPIU_UFS_DEVICE_WLUN)
+ sdp = hba->sdev_ufs_device;
+ else if (wlun == UFS_UPIU_RPMB_WLUN)
+ sdp = hba->sdev_rpmb;
+ else
+ BUG_ON(1);
+ if (sdp) {
+ ret = scsi_device_get(sdp);
+ if (!ret && !scsi_device_online(sdp)) {
+ ret = -ENODEV;
+ scsi_device_put(sdp);
+ }
+ } else {
+ ret = -ENODEV;
+ }
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ if (ret)
+ goto out_err;
+
+ ret = ufshcd_send_request_sense(hba, sdp);
+ scsi_device_put(sdp);
+out_err:
+ if (ret)
+ dev_err(hba->dev, "%s: UAC clear LU=%x ret = %d\n",
+ __func__, wlun, ret);
+ return ret;
+}
+
+static int ufshcd_clear_ua_wluns(struct ufs_hba *hba)
+{
+ int ret = 0;
+
+ if (!hba->wlun_dev_clr_ua)
+ goto out;
+
+ ret = ufshcd_clear_ua_wlun(hba, UFS_UPIU_UFS_DEVICE_WLUN);
+ if (!ret)
+ ret = ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
+ if (!ret)
+ hba->wlun_dev_clr_ua = false;
+out:
+ if (ret)
+ dev_err(hba->dev, "%s: Failed to clear UAC WLUNS ret = %d\n",
+ __func__, ret);
+ return ret;
+}
+
/**
* ufshcd_probe_hba - probe hba to detect device and initialize
* @hba: per-adapter instance
@@ -7720,6 +7776,8 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
pm_runtime_put_sync(hba->dev);
ufshcd_exit_clk_scaling(hba);
ufshcd_hba_exit(hba);
+ } else {
+ ufshcd_clear_ua_wluns(hba);
}
}
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 47eb1430274c..718881d038f5 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -681,6 +681,7 @@ struct ufs_hba {
* "UFS device" W-LU.
*/
struct scsi_device *sdev_ufs_device;
+ struct scsi_device *sdev_rpmb;
enum ufs_dev_pwr_mode curr_dev_pwr_mode;
enum uic_link_state uic_link_state;
--
2.29.0.rc1.297.gfa9743e501-goog
The below call stack prevents clk_gating at every IO completion.
We can remove the condition, ufshcd_any_tag_in_use(), since clkgating_work
will check it again.
ufshcd_complete_requests(struct ufs_hba *hba)
ufshcd_transfer_req_compl()
__ufshcd_transfer_req_compl()
__ufshcd_release(hba)
if (ufshcd_any_tag_in_use() == 1)
return;
ufshcd_tmc_handler(hba);
blk_mq_tagset_busy_iter();
Cc: Alim Akhtar <[email protected]>
Cc: Avri Altman <[email protected]>
Cc: Can Guo <[email protected]>
Signed-off-by: Jaegeuk Kim <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b5ca0effe636..cecbd4ace8b4 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1746,7 +1746,7 @@ static void __ufshcd_release(struct ufs_hba *hba)
if (hba->clk_gating.active_reqs || hba->clk_gating.is_suspended ||
hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL ||
- ufshcd_any_tag_in_use(hba) || hba->outstanding_tasks ||
+ hba->outstanding_tasks ||
hba->active_uic_cmd || hba->uic_async_done)
return;
--
2.29.0.rc1.297.gfa9743e501-goog
On 2020-10-21 03:52, Jaegeuk Kim wrote:
> The below call stack prevents clk_gating at every IO completion.
> We can remove the condition, ufshcd_any_tag_in_use(), since
> clkgating_work
> will check it again.
>
I think checking ufshcd_any_tag_in_use() in either ufshcd_release() or
gate_work() can break UFS clk gating's functionality.
ufshcd_any_tag_in_use() was introduced to replace hba->lrb_in_use.
However,
they are not exactly same - ufshcd_any_tag_in_use() returns true if any
tag
assigned from block layer is still in use, but tags are released
asynchronously
(through block softirq), meaning it does not reflect the real occupation
of UFS host.
That is after UFS host finishes all tasks, ufshcd_any_tag_in_use() can
still return true.
This change only removes the check of ufshcd_any_tag_in_use() in
ufshcd_release(),
but having the check of it in gate_work() can still prevent gating from
happening.
The current change works for you maybe because the tags are release
before
hba->clk_gating.delay_ms expires, but if hba->clk_gating.delay_ms is
shorter or
somehow block softirq is retarded, gate_work() may have chance to see
ufshcd_any_tag_in_use()
returns true. What do you think?
Thanks,
Can Guo.
In __ufshcd_transfer_req_compl
Ihba->lrb_in_use is cleared immediately when UFS driver
finishes all tasks
> ufshcd_complete_requests(struct ufs_hba *hba)
> ufshcd_transfer_req_compl()
> __ufshcd_transfer_req_compl()
> __ufshcd_release(hba)
> if (ufshcd_any_tag_in_use() == 1)
> return;
> ufshcd_tmc_handler(hba);
> blk_mq_tagset_busy_iter();
>
> Cc: Alim Akhtar <[email protected]>
> Cc: Avri Altman <[email protected]>
> Cc: Can Guo <[email protected]>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> drivers/scsi/ufs/ufshcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b5ca0effe636..cecbd4ace8b4 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1746,7 +1746,7 @@ static void __ufshcd_release(struct ufs_hba *hba)
>
> if (hba->clk_gating.active_reqs || hba->clk_gating.is_suspended ||
> hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL ||
> - ufshcd_any_tag_in_use(hba) || hba->outstanding_tasks ||
> + hba->outstanding_tasks ||
> hba->active_uic_cmd || hba->uic_async_done)
> return;
On 10/21, Can Guo wrote:
> On 2020-10-21 03:52, Jaegeuk Kim wrote:
> > The below call stack prevents clk_gating at every IO completion.
> > We can remove the condition, ufshcd_any_tag_in_use(), since
> > clkgating_work
> > will check it again.
> >
>
> I think checking ufshcd_any_tag_in_use() in either ufshcd_release() or
> gate_work() can break UFS clk gating's functionality.
>
> ufshcd_any_tag_in_use() was introduced to replace hba->lrb_in_use. However,
> they are not exactly same - ufshcd_any_tag_in_use() returns true if any tag
> assigned from block layer is still in use, but tags are released
> asynchronously
> (through block softirq), meaning it does not reflect the real occupation of
> UFS host.
> That is after UFS host finishes all tasks, ufshcd_any_tag_in_use() can still
> return true.
>
> This change only removes the check of ufshcd_any_tag_in_use() in
> ufshcd_release(),
> but having the check of it in gate_work() can still prevent gating from
> happening.
> The current change works for you maybe because the tags are release before
> hba->clk_gating.delay_ms expires, but if hba->clk_gating.delay_ms is shorter
> or
> somehow block softirq is retarded, gate_work() may have chance to see
> ufshcd_any_tag_in_use()
> returns true. What do you think?
I don't think this breaks clkgating, but fix the wrong condition check which
prevented gate_work at all. As you mentioned, even if this schedules gate_work
by racy conditions, gate_work will handle it as a last resort.
>
> Thanks,
>
> Can Guo.
>
> In __ufshcd_transfer_req_compl
> Ihba->lrb_in_use is cleared immediately when UFS driver
> finishes all tasks
>
> > ufshcd_complete_requests(struct ufs_hba *hba)
> > ufshcd_transfer_req_compl()
> > __ufshcd_transfer_req_compl()
> > __ufshcd_release(hba)
> > if (ufshcd_any_tag_in_use() == 1)
> > return;
> > ufshcd_tmc_handler(hba);
> > blk_mq_tagset_busy_iter();
> >
> > Cc: Alim Akhtar <[email protected]>
> > Cc: Avri Altman <[email protected]>
> > Cc: Can Guo <[email protected]>
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > drivers/scsi/ufs/ufshcd.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index b5ca0effe636..cecbd4ace8b4 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -1746,7 +1746,7 @@ static void __ufshcd_release(struct ufs_hba *hba)
> >
> > if (hba->clk_gating.active_reqs || hba->clk_gating.is_suspended ||
> > hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL ||
> > - ufshcd_any_tag_in_use(hba) || hba->outstanding_tasks ||
> > + hba->outstanding_tasks ||
> > hba->active_uic_cmd || hba->uic_async_done)
> > return;