UFS driver general fixes bundle 4
Subsystems (3)
v7 cover UFS driver general fixes bundle 4
This bundle includes 8 general fixes for UFS driver.
Changes since v6:
- Incoperated Avri's comment.
Changes since v5:
- Incoperated Stanley's suggestions to make the extra delay added up to bRefClkGatingWaitTime as vendor specific
Changes since v4:
- Rebased this series
Changes since v3:
- Fixed patch #8
Changes since v2:
- Move the ref clk gating wait delay to ufs-qcom.c
- Added one more change to select INITIAL adapt for HS G4
Changes since v1:
- Fixed minor typo
Asutosh Das (1):
scsi: ufs: set load before setting voltage in regulators
Can Guo (6):
scsi: ufs: Remove the check before call setup clock notify vops
scsi: ufs-qcom: Adjust bus bandwidth voting and unvoting
scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic
scsi: ufs: Add dev ref clock gating wait time support
scsi: ufs-qcom: Delay specific time before gate ref clk
scsi: ufs: Select INITIAL adapt for HS Gear4
Sayali Lokhande (1):
scsi: ufs: Flush exception event before suspend
drivers/scsi/ufs/ufs-qcom.c | 79 ++++++++++++++++++++++--------- drivers/scsi/ufs/ufs.h | 3 ++ drivers/scsi/ufs/ufshcd.c | 110 ++++++++++++++++++++++++++++++++------------ drivers/scsi/ufs/ufshci.h | 1 + drivers/scsi/ufs/unipro.h | 7 +++ 5 files changed, 150 insertions(+), 50 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v7 1/8 scsi: ufs: Flush exception event before suspend
From: Sayali Lokhande <[email protected]>
Exception event can be raised by the device when system suspend is in progress. This will result in unclocked register access in exception event handler as clocks will be turned off during suspend. This change makes sure to flush exception event handler work in suspend before disabling clocks to avoid unclocked register access issue.
--- drivers/scsi/ufs/ufshcd.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/scsi/ufs/ufshcd.c +18 -3
@@ -4730,8 +4730,15 @@ static void ufshcd_slave_destroy(struct scsi_device *sdev)* UFS device needs urgent BKOPs. */ if (!hba->pm_op_in_progress && -ufshcd_is_exception_event (lrbp->ucd_rsp_ptr)) -schedule_work (&hba->eeh_work); +ufshcd_is_exception_event (lrbp->ucd_rsp_ptr) && +schedule_work (&hba->eeh_work)) { +/* +* Prevent suspend once eeh_work is scheduled +* to avoid deadlock between ufshcd_suspend +* and exception event handler. +*/ +pm_runtime_get_noresume (hba->dev); + }break ;case UPIU_TRANSACTION_REJECT_UPIU :/* TODO: handle Reject UPIU Response */
@@ -5184,7 +5191,14 @@ static void ufshcd_exception_event_handler(struct work_struct *work) out:ufshcd_scsi_unblock_requests (hba); -pm_runtime_put_sync (hba->dev); +/* +* pm_runtime_get_noresume is called while scheduling +* eeh_work to avoid suspend racing with exception work. +* Hence decrement usage counter using pm_runtime_put_noidle +* to allow suspend on completion of exception event handler. +*/ +pm_runtime_put_noidle (hba->dev); +pm_runtime_put (hba->dev);return ; }
@@ -7924,6 +7938,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) goto enable_gating; } +flush_work (&hba->eeh_work); ret =ufshcd_link_state_transition (hba, req_link_state,1 );if (ret) goto set_dev_active;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v7 2/8 scsi: ufs: set load before setting voltage in regulators
In reply to Can GuoFrom: Asutosh Das <[email protected]>
This sequence change is required to avoid dips in voltage during boot-up.
Apparently, this dip is caused because in the original sequence, the regulators are initialized in lpm mode. And then when the load is set to high, and more current is drawn, than is allowed in lpm, the dip is seen.
--- drivers/scsi/ufs/ufshcd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/scsi/ufs/ufshcd.c +5 -5
@@ -7248,6 +7248,11 @@ static int ufshcd_config_vreg(struct device *dev, name = vreg->name;if (regulator_count_voltages (reg) >0 ) { + uA_load = on ? vreg->max_uA :0 ; + ret =ufshcd_config_vreg_load (dev, vreg, uA_load); +if (ret) + goto out; +if (vreg->min_uV && vreg->max_uV) { min_uV = on ? vreg->min_uV :0 ; ret =regulator_set_voltage (reg, min_uV, vreg->max_uV);
@@ -7258,11 +7263,6 @@ static int ufshcd_config_vreg(struct device *dev, goto out; } } - - uA_load = on ? vreg->max_uA :0 ; - ret =ufshcd_config_vreg_load (dev, vreg, uA_load); -if (ret) - goto out; } out:return ret;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v7 3/8 scsi: ufs: Remove the check before call setup clock notify vops
In reply to Can GuoThe functionality of vendor specific ops should be handled properly in platform specific driver, but should not count on the UFS driver.
--- drivers/scsi/ufs/ufshcd.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-)
drivers/scsi/ufs/ufshcd.c +6 -20
@@ -7402,16 +7402,9 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,if (list_empty (head)) goto out; -/* -* vendor specific setup_clocks ops may depend on clocks managed by -* this standard driver hence call the vendor specific setup_clocks -* before disabling the clocks managed here. -*/ -if (!on) { - ret =ufshcd_vops_setup_clocks (hba, on,PRE_CHANGE ); -if (ret) -return ret; - } + ret =ufshcd_vops_setup_clocks (hba, on,PRE_CHANGE ); +if (ret) +return ret;list_for_each_entry (clki, head, list) {if (!IS_ERR_OR_NULL (clki->clk)) {
@@ -7435,16 +7428,9 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on, } } -/* -* vendor specific setup_clocks ops may depend on clocks managed by -* this standard driver hence call the vendor specific setup_clocks -* after enabling the clocks managed here. -*/ -if (on) { - ret =ufshcd_vops_setup_clocks (hba, on,POST_CHANGE ); -if (ret) -return ret; - } + ret =ufshcd_vops_setup_clocks (hba, on,POST_CHANGE ); +if (ret) +return ret; out:if (ret) {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v7 4/8 scsi: ufs-qcom: Adjust bus bandwidth voting and unvoting
In reply to Can GuoThe bus bandwidth voting is required to be done before the bus clocks are enabled, and the unvoting is required to be done only after the bus clocks are disabled.
--- drivers/scsi/ufs/ufs-qcom.c | 57 +++++++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 18 deletions(-)
drivers/scsi/ufs/ufs-qcom.c +39 -18
@@ -38,7 +38,6 @@ enum {static struct ufs_qcom_host *ufs_qcom_hosts[MAX_UFS_QCOM_HOSTS ]; -static int ufs_qcom_set_bus_vote (struct ufs_qcom_host *host,int vote);static void ufs_qcom_get_default_testbus_cfg (struct ufs_qcom_host *host);static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div (struct ufs_hba *hba,u32 clk_cycles);
@@ -674,7 +673,7 @@ static void ufs_qcom_get_speed_mode(struct ufs_pa_layer_attr *p, char *result) } } -static int ufs_qcom_set_bus_vote (struct ufs_qcom_host *host,int vote) +static int __ufs_qcom_set_bus_vote (struct ufs_qcom_host *host,int vote) {int err =0 ;
@@ -705,7 +704,7 @@ static int ufs_qcom_update_bus_bw_vote(struct ufs_qcom_host *host) vote =ufs_qcom_get_bus_vote (host, mode);if (vote >=0 ) - err =ufs_qcom_set_bus_vote (host, vote); + err =__ufs_qcom_set_bus_vote (host, vote);else err = vote;
@@ -716,6 +715,35 @@ static int ufs_qcom_update_bus_bw_vote(struct ufs_qcom_host *host)return err; } +static int ufs_qcom_set_bus_vote (struct ufs_hba *hba,bool on) +{ +struct ufs_qcom_host *host =ufshcd_get_variant (hba); +int vote, err; + +/* +* In case ufs_qcom_init() is not yet done, simply ignore. +* This ufs_qcom_set_bus_vote() shall be called from +* ufs_qcom_init() after init is done. +*/ +if (!host) +return 0 ; + +if (on) { + vote = host->bus_vote.saved_vote; +if (vote == host->bus_vote.min_bw_vote) +ufs_qcom_update_bus_bw_vote (host); + }else { + vote = host->bus_vote.min_bw_vote; + } + + err =__ufs_qcom_set_bus_vote (host, vote); +if (err) +dev_err (hba->dev,"%s: set bus vote failed %d\n" , + __func__, err); + +return err; +} +static ssize_t show_ufs_to_mem_max_bus_bw (struct device *dev,struct device_attribute *attr,char *buf)
@@ -792,7 +820,7 @@ static int ufs_qcom_update_bus_bw_vote(struct ufs_qcom_host *host)return 0 ; } -static int ufs_qcom_set_bus_vote (struct ufs_qcom_host *host,int vote) +static int ufs_qcom_set_bus_vote (struct ufs_hba *host,bool on) {return 0 ; }
@@ -1030,8 +1058,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on, enum ufs_notify_change_status status) {struct ufs_qcom_host *host =ufshcd_get_variant (hba); -int err; -int vote =0 ; +int err =0 ;/* * In case ufs_qcom_init() is not yet done, simply ignore.
@@ -1041,28 +1068,21 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,if (!host)return 0 ; -if (on && (status ==POST_CHANGE )) { +if (on && (status ==PRE_CHANGE )) { + err =ufs_qcom_set_bus_vote (hba, true); + }else if (on && (status ==POST_CHANGE )) {/* enable the device ref clock for HS mode*/ if (ufshcd_is_hs_mode (&hba->pwr_info))ufs_qcom_dev_ref_clk_ctrl (host, true); - vote = host->bus_vote.saved_vote; -if (vote == host->bus_vote.min_bw_vote) -ufs_qcom_update_bus_bw_vote (host); - }else if (!on && (status ==PRE_CHANGE )) {if (!ufs_qcom_is_link_active (hba)) {/* disable device ref_clk */ ufs_qcom_dev_ref_clk_ctrl (host, false); } - - vote = host->bus_vote.min_bw_vote; + }else if (!on && (status ==POST_CHANGE )) { + err =ufs_qcom_set_bus_vote (hba, false); } - err =ufs_qcom_set_bus_vote (host, vote); -if (err) -dev_err (hba->dev,"%s: set bus vote failed %d\n" , - __func__, err); -return err; }
@@ -1238,6 +1258,7 @@ static int ufs_qcom_init(struct ufs_hba *hba)ufs_qcom_set_caps (hba);ufs_qcom_advertise_quirks (hba); +ufs_qcom_set_bus_vote (hba, true);ufs_qcom_setup_clocks (hba, true,POST_CHANGE );if (hba->dev->id <MAX_UFS_QCOM_HOSTS )
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v7 5/8 scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic
In reply to Can GuoThe async version of ufshcd_hold(async == true), which is only called in queuecommand path as for now, is expected to work in atomic context, thus it should not sleep or schedule out. When it runs into the condition that clocks are ON but link is still in hibern8 state, it should bail out without flushing the clock ungate work.
--- drivers/scsi/ufs/ufshcd.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/scsi/ufs/ufshcd.c +5 -0
@@ -1518,6 +1518,11 @@ int ufshcd_hold(struct ufs_hba *hba, bool async)*/ if (ufshcd_can_hibern8_during_gating (hba) &&ufshcd_is_link_hibern8 (hba)) { +if (async) { + rc = -EAGAIN ; + hba->clk_gating.active_reqs--; +break ; + }spin_unlock_irqrestore (hba->host->host_lock, flags);flush_work (&hba->clk_gating.ungate_work);spin_lock_irqsave (hba->host->host_lock, flags);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
RE: [PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic
Hi,
Quoted 6 lines
The async version of
ufshcd_hold(async == true), which is only called in queuecommand path as for now, is expected to work in atomic context, thus it should not sleep or schedule out. When it runs into the condition that clocks are ON but link is still in hibern8 state, it should bail out without flushing the clock ungate work.
Quoted 16 lines
Signed-off-by Can Guo <[email protected]>Reviewed-by Hongwu Su <[email protected]>Reviewed-by Asutosh Das <[email protected]>Reviewed-by Bean Huo <[email protected]>Reviewed-by Stanley Chu <[email protected]>--- drivers/scsi/ufs/ufshcd.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/scsi/ufs/ufshcd.c +0
-0@@ -1518,6 +1518,11 @@ int ufshcd_hold(struct ufs_hba *hba, bool async)*/
drivers/scsi/ufs/ufshcd.c int ufshcd_hold(struct ufs_hba *hba, bool async) · line 1528
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +5
-0if (ufshcd_can_hibern8_during_gating (hba) &&ufshcd_is_link_hibern8 (hba)) { +if (async) { + rc = -EAGAIN ; + hba->clk_gating.active_reqs--; +break ; + }spin_unlock_irqrestore (hba->host->host_lock, flags);flush_work (&hba->clk_gating.ungate_work);spin_lock_irqsave (hba->host->host_lock, flags);
Since now the above code is shared in all cases,
Maybe find a more economical way to pack it?
Thanks,
Avri
Quoted 4 lines
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic
Quoted 10 lines · Avri Altman
Hi,
Quoted 6 lines
The async version of
ufshcd_hold(async == true), which is only called in queuecommand path as for now, is expected to work in atomic context, thus it should not sleep or schedule out. When it runs into the condition that clocks are ON but link is still in hibern8 state, it should bail out without flushing the clock ungate work.Fixes f2a785ac2312 (scsi: ufshcd: Fix race between clk scaling and ungate work)
Sorry, missed this one, if another version is needed, I will add this line.
Quoted 32 lines
Quoted 28 lines
Signed-off-by Can Guo <[email protected]>Reviewed-by Hongwu Su <[email protected]>Reviewed-by Asutosh Das <[email protected]>Reviewed-by Bean Huo <[email protected]>Reviewed-by Stanley Chu <[email protected]>--- drivers/scsi/ufs/ufshcd.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/scsi/ufs/ufshcd.c +5
-0@@ -1518,6 +1518,11 @@ int ufshcd_hold(struct ufs_hba *hba, boolasync) */ if (ufshcd_can_hibern8_during_gating (hba) &&ufshcd_is_link_hibern8 (hba)) { +if (async) { + rc = -EAGAIN ; + hba->clk_gating.active_reqs--; +break ; + }spin_unlock_irqrestore (hba->host->host_lock, flags);flush_work(&hba->clk_gating.ungate_work); spin_lock_irqsave(hba->host->host_lock,Quoted 1 line
flags);
Since now the above code is shared in all cases,
Maybe find a more economical way to pack it?
Quoted 4 lines
Thanks,
Avri
There are only 2 of this same code pieces in ufshcd_hold() and located in different cases, meanwhile there can be fall through, I don't see a good way to pack it, can you suggest if you have any ideas?
Regards,
Can Guo.
Quoted 4 lines
Quoted 4 lines
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic
Quoted 46 lines · Can Guo
Quoted 10 lines · Avri Altman
Hi,
Quoted 6 lines
The async version of
ufshcd_hold(async == true), which is only called in queuecommand path as for now, is expected to work in atomic context, thus it should not sleep or schedule out. When it runs into the condition that clocks are ON but link is still in hibern8 state, it should bail out without flushing the clock ungate work.Fixes f2a785ac2312 (scsi: ufshcd: Fix race between clk scaling and ungate work)Sorry, missed this one, if another version is needed, I will add this line.
Quoted 32 lines
Quoted 28 lines
Signed-off-by Can Guo <[email protected]>Reviewed-by Hongwu Su <[email protected]>Reviewed-by Asutosh Das <[email protected]>Reviewed-by Bean Huo <[email protected]>Reviewed-by Stanley Chu <[email protected]>--- drivers/scsi/ufs/ufshcd.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/scsi/ufs/ufshcd.c +5
-0@@ -1518,6 +1518,11 @@ int ufshcd_hold(struct ufs_hba *hba, boolasync) */ if (ufshcd_can_hibern8_during_gating (hba) &&ufshcd_is_link_hibern8 (hba)) { +if (async) { + rc = -EAGAIN ; + hba->clk_gating.active_reqs--; +break ; + }spin_unlock_irqrestore (hba->host->host_lock, flags);flush_work(&hba->clk_gating.ungate_work); spin_lock_irqsave(hba->host->host_lock,Quoted 1 line
flags);
Since now the above code is shared in all cases,
Maybe find a more economical way to pack it?
Quoted 9 lines
Quoted 4 lines
Thanks,
AvriThere are only 2 of this same code pieces in
ufshcd_hold()and located in different cases, meanwhile there can be fall through, I don't see a good way to pack it, can you suggest if you have any ideas?
Now, with this patch, there are 2 same code snippets located in CLKS_ON and REQ_CLKS_ON. If we somehow pack them, say bring in a inline func to pack them, we would have to tear it down later if we have to fix something for only one specific case by adding lines into the snippet. And actually this is the truth, we do have some fixes for CLKS_ON's case but not yet uploaded, so let's leave it as it is for now.
Quoted 7 lines
Regards,
Can Guo.Quoted 4 lines
Quoted 4 lines
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
RE: [PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
Fixes f2a785ac2312 (scsi: ufshcd: Fix race between clk scaling andungate work)
Sorry, missed this one, if another version is needed, I will add this line.
fair enough.
Quoted 43 lines
Quoted 33 lines
Quoted 33 lines
Quoted 29 lines
Signed-off-by Can Guo <[email protected]>Reviewed-by Hongwu Su <[email protected]>Reviewed-by Asutosh Das <[email protected]>Reviewed-by Bean Huo <[email protected]>Reviewed-by Stanley Chu <[email protected]>--- drivers/scsi/ufs/ufshcd.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/scsi/ufs/ufshcd.c +5
-0@@ -1518,6 +1518,11 @@ int ufshcd_hold(struct ufs_hba *hba, boolasync) */ if (ufshcd_can_hibern8_during_gating (hba) &&ufshcd_is_link_hibern8 (hba)) { +if (async) { + rc = -EAGAIN ; + hba->clk_gating.active_reqs--; +break ; + }spin_unlock_irqrestore (hba->host->host_lock, flags);flush_work(&hba->clk_gating.ungate_work); spin_lock_irqsave(hba->host->host_lock,Quoted 1 line
flags);
Since now the above code is shared in all cases,
Maybe find a more economical way to pack it?Quoted 9 lines
Quoted 4 lines
Thanks,
AvriThere are only 2 of this same code pieces in
ufshcd_hold()and located in different cases, meanwhile there can be fall through, I don't see a good way to pack it, can you suggest if you have any ideas?
Quoted 6 lines
Now, with this patch, there are 2 same code snippets located in
CLKS_ONandREQ_CLKS_ON. If we somehow pack them, say bring in a inline func to pack them, we would have to tear it down later if we have to fix something for only one specific case by adding lines into the snippet. And actually this is the truth, we do have some fixes for CLKS_ON's case but not yet uploaded, so let's leave it as it is for now.
OK.
Thanks,
Avri
PATCH v7 6/8 scsi: ufs: Add dev ref clock gating wait time support
In reply to Can GuoIn UFS version 3.0, a newly added attribute bRefClkGatingWaitTime defines the minimum time for which the reference clock is required by device during transition to LS-MODE or HIBERN8 state. Make this change to reflect the new requirement by adding delays before turning off the clock.
--- drivers/scsi/ufs/ufs.h | 3 +++ drivers/scsi/ufs/ufshcd.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+)
drivers/scsi/ufs/ufs.h +3 -0
@@ -167,6 +167,7 @@ enum attr_idn {QUERY_ATTR_IDN_FFU_STATUS =0x14 ,QUERY_ATTR_IDN_PSA_STATE =0x15 ,QUERY_ATTR_IDN_PSA_DATA_SIZE =0x16 , + QUERY_ATTR_IDN_REF_CLK_GATING_WAIT_TIME = 0x17, };/* Descriptor idn for Query requests */
@@ -534,6 +535,8 @@ struct ufs_dev_info {u16 wmanufacturerid;/*UFS device Product Name */ u8 *model; +u16 wspecversion; +u32 clk_gating_wait_us; };/**
drivers/scsi/ufs/ufshcd.c +34 -0
@@ -91,6 +91,9 @@/* default delay of autosuspend: 2000 ms */ #define RPM_AUTOSUSPEND_DELAY_MS 2000 +/* Default value of wait time before gating device ref clock */ +#define UFSHCD_REF_CLK_GATING_WAIT_US 0xFF/* microsecs */ +#define ufshcd_toggle_vreg (_dev, _vreg, _on) \ ({ \ int _ret; \
@@ -3281,6 +3284,31 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba, param_offset, param_read_buf, param_size); } +static int ufshcd_get_ref_clk_gating_wait (struct ufs_hba *hba) +{ +int err =0 ; +u32 gating_wait =UFSHCD_REF_CLK_GATING_WAIT_US ; + +if (hba->dev_info.wspecversion >=0x300 ) { + err =ufshcd_query_attr_retry (hba,UPIU_QUERY_OPCODE_READ_ATTR , +QUERY_ATTR_IDN_REF_CLK_GATING_WAIT_TIME ,0 ,0 , + &gating_wait); +if (err) +dev_err (hba->dev,"Failed reading bRefClkGatingWait. err = %d, use default %uus\n" , + err, gating_wait); + +if (gating_wait ==0 ) { + gating_wait =UFSHCD_REF_CLK_GATING_WAIT_US ; +dev_err (hba->dev,"Undefined ref clk gating wait time, use default %uus\n" , + gating_wait); + } + } + + hba->dev_info.clk_gating_wait_us = gating_wait; + +return err; +} +/** * ufshcd_memory_alloc - allocate memory for host memory space data structures * @hba: per adapter instance
@@ -6626,6 +6654,10 @@ static int ufs_get_device_desc(struct ufs_hba *hba) dev_info->wmanufacturerid = desc_buf[DEVICE_DESC_PARAM_MANF_ID ] <<8 | desc_buf[DEVICE_DESC_PARAM_MANF_ID +1 ]; +/* getting Specification Version in big endian format */ + dev_info->wspecversion = desc_buf[DEVICE_DESC_PARAM_SPEC_VER ] <<8 | + desc_buf[DEVICE_DESC_PARAM_SPEC_VER +1 ]; + model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME ]; err =ufshcd_read_string_desc (hba, model_index, &dev_info->model,SD_ASCII_STD );
@@ -7003,6 +7035,8 @@ static int ufshcd_device_params_init(struct ufs_hba *hba) goto out; } +ufshcd_get_ref_clk_gating_wait (hba); +ufs_fixup_device_setup (hba); if (!ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG,
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v7 7/8 scsi: ufs-qcom: Delay specific time before gate ref clk
In reply to Can GuoAfter enter hibern8, as UFS JEDEC ver 3.0 requires, a specific gating wait time is required before disable the device reference clock. If it is not specified, use the old delay.
--- drivers/scsi/ufs/ufs-qcom.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
drivers/scsi/ufs/ufs-qcom.c +19 -3
@@ -833,6 +833,8 @@ static int ufs_qcom_bus_register(struct ufs_qcom_host *host)static void ufs_qcom_dev_ref_clk_ctrl (struct ufs_qcom_host *host,bool enable) { +unsigned long gating_wait; +if (host->dev_ref_clk_ctrl_mmio && (enable ^ host->is_dev_ref_clk_enabled)) {u32 temp =readl_relaxed (host->dev_ref_clk_ctrl_mmio);
@@ -845,11 +847,25 @@ static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable)/* * If we are here to disable this clock it might be immediately * after entering into hibern8 in which case we need to make -* sure that device ref_clk is active at least 1us after the +* sure that device ref_clk is active for specific time after * hibern8 enter. */ -if (!enable) -udelay (1 ); +if (!enable) { + gating_wait = host->hba->dev_info.clk_gating_wait_us; +if (!gating_wait) { +udelay (1 ); + }else { +/* +* bRefClkGatingWaitTime defines the minimum +* time for which the reference clock is +* required by device during transition from +* HS-MODE to LS-MODE or HIBERN8 state. Give it +* more time to be on the safe side. +*/ + gating_wait +=10 ; +usleep_range (gating_wait, gating_wait +10 ); + } + }writel_relaxed (temp, host->dev_ref_clk_ctrl_mmio);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v7 7/8] scsi: ufs-qcom: Delay specific time before gate ref clk
Quoted 27 lines · Can Guo
After enter hibern8, as UFS JEDEC ver 3.0 requires, a specific gating wait time is required before disable the device reference clock. If it is not specified, use the old delay.
Signed-off-by Can Guo <[email protected]>Reviewed-by Asutosh Das <[email protected]>Reviewed-by Hongwu Su <[email protected]>--- drivers/scsi/ufs/ufs-qcom.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)drivers/scsi/ufs/ufs-qcom.c +2
-0@@ -833,6 +833,8 @@ static int ufs_qcom_bus_register(struct ufs_qcom_host *host)static void ufs_qcom_dev_ref_clk_ctrl (struct ufs_qcom_host *host,bool enable) { +unsigned long gating_wait; +if (host->dev_ref_clk_ctrl_mmio && (enable ^ host->is_dev_ref_clk_enabled)) {u32 temp =readl_relaxed (host->dev_ref_clk_ctrl_mmio);@@ -845,11 +847,25 @@ static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable)/* * If we are here to disable this clock it might be immediately
drivers/scsi/ufs/ufs-qcom.c static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) · line 855
Quoted 10 lines
drivers/scsi/ufs/ufs-qcom.c +4
-3* after entering into hibern8 in which case we need to make -* sure that device ref_clk is active at least 1us after the +* sure that device ref_clk is active for specific time after * hibern8 enter. */ -if (!enable) -udelay (1 ); +if (!enable) { + gating_wait = host->hba->dev_info.clk_gating_wait_us; +if (!gating_wait) {
Afaict this can't happen, because in patch 6 you check for gating_wait being 0 and if so set it to 0xff.
drivers/scsi/ufs/ufs-qcom.c static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) · line 866
Quoted 11 lines
drivers/scsi/ufs/ufs-qcom.c +11
-0+udelay (1 ); + }else { +/* +* bRefClkGatingWaitTime defines the minimum +* time for which the reference clock is +* required by device during transition from +* HS-MODE to LS-MODE or HIBERN8 state. Give it +* more time to be on the safe side. +*/ + gating_wait +=10 ; +usleep_range (gating_wait, gating_wait +10 );
I presume there's no strong requirement on the max, so how about using a substantially larger max - say 1k, or 10k - to allow the usleep_range() to do it's job?
PS. Please include linux-arm-msm@ on all the patches in the series, not just two of them.
Regards,
Bjorn
Quoted 8 lines
drivers/scsi/ufs/ufs-qcom.c +2
-0+ } + }writel_relaxed (temp, host->dev_ref_clk_ctrl_mmio);Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v7 7/8] scsi: ufs-qcom: Delay specific time before gate ref clk
Quoted 37 lines · Bjorn Andersson
Quoted 35 lines · Can Guo
After enter hibern8, as UFS JEDEC ver 3.0 requires, a specific gating wait time is required before disable the device reference clock. If it is not specified, use the old delay.
Signed-off-by Can Guo <[email protected]>Reviewed-by Asutosh Das <[email protected]>Reviewed-by Hongwu Su <[email protected]>--- drivers/scsi/ufs/ufs-qcom.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)drivers/scsi/ufs/ufs-qcom.c +2
-0@@ -833,6 +833,8 @@ static int ufs_qcom_bus_register(struct ufs_qcom_host *host)static void ufs_qcom_dev_ref_clk_ctrl (struct ufs_qcom_host *host,bool enable) { +unsigned long gating_wait; + if (host->dev_ref_clk_ctrl_mmio &&drivers/scsi/ufs/ufs-qcom.c +1
-1(enable ^ host->is_dev_ref_clk_enabled)) {u32 temp =readl_relaxed (host->dev_ref_clk_ctrl_mmio);@@ -845,11 +847,25 @@ static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable)/* * If we are here to disable this clock it might be immediately * after entering into hibern8 in which case we need to make -* sure that device ref_clk is active at least 1us after the +* sure that device ref_clk is active for specific time after * hibern8 enter. */
Quoted 9 lines
drivers/scsi/ufs/ufs-qcom.c static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) · line 855
Quoted 5 lines
drivers/scsi/ufs/ufs-qcom.c +3
-2-if (!enable) -udelay (1 ); +if (!enable) { + gating_wait = host->hba->dev_info.clk_gating_wait_us; +if (!gating_wait) {Afaict this can't happen, because in patch 6 you check for
gating_waitbeing 0 and if so set it to0xff.
Sorry, I was intended to give clk_gating_wait_us values only if it is a UFS3.0 device. I will revise patch 6/8.
Quoted 17 lines
drivers/scsi/ufs/ufs-qcom.c static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) · line 866
Quoted 11 lines
drivers/scsi/ufs/ufs-qcom.c +11
-0+udelay (1 ); + }else { +/* +* bRefClkGatingWaitTime defines the minimum +* time for which the reference clock is +* required by device during transition from +* HS-MODE to LS-MODE or HIBERN8 state. Give it +* more time to be on the safe side. +*/ + gating_wait +=10 ; +usleep_range (gating_wait, gating_wait +10 );I presume there's no strong requirement on the max, so how about using a substantially larger max - say 1k, or 10k - to allow the
usleep_range()to do it's job?
Quoted 6 lines
PS. Please include linux-arm-msm@ on all the patches in the series, not just two of them.
Regards,
Bjorn
bRefClkGatingWaitTime, as vendor defined in their device attribute is usually around 50~100, 1k or 10k delay makes it too large. usleep_range() works well so long as the delay is within (10us - 20ms), so I added 10 to make sure it is above 10us.
SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Use usleep_range
https://www.kernel.org/doc/Documentation/timers/timers-howto.txt
Thanks,
Can Guo.
Quoted 8 lines
Quoted 8 lines
drivers/scsi/ufs/ufs-qcom.c +2
-1+ } + }writel_relaxed (temp, host->dev_ref_clk_ctrl_mmio); --The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v7 7/8] scsi: ufs-qcom: Delay specific time before gate ref clk
Quoted 50 lines · Can Guo
Quoted 39 lines · Bjorn Andersson
Quoted 37 lines · Can Guo
After enter hibern8, as UFS JEDEC ver 3.0 requires, a specific gating wait
time is required before disable the device reference clock. If it is not
specified, use the old delay.Signed-off-by Can Guo <[email protected]>Reviewed-by Asutosh Das <[email protected]>Reviewed-by Hongwu Su <[email protected]>--- drivers/scsi/ufs/ufs-qcom.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)drivers/scsi/ufs/ufs-qcom.c +2
-0@@ -833,6 +833,8 @@ static int ufs_qcom_bus_register(struct ufs_qcom_host *host)static void ufs_qcom_dev_ref_clk_ctrl (struct ufs_qcom_host *host,bool enable) { +unsigned long gating_wait; + if (host->dev_ref_clk_ctrl_mmio &&drivers/scsi/ufs/ufs-qcom.c +1
-1(enable ^ host->is_dev_ref_clk_enabled)) {u32 temp =readl_relaxed (host->dev_ref_clk_ctrl_mmio);@@ -845,11 +847,25 @@ static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable)/* * If we are here to disable this clock it might be immediately * after entering into hibern8 in which case we need to make -* sure that device ref_clk is active at least 1us after the +* sure that device ref_clk is active for specific time after * hibern8 enter. */ Quoted 9 lines
drivers/scsi/ufs/ufs-qcom.c static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) · line 855
Quoted 5 lines
drivers/scsi/ufs/ufs-qcom.c +3
-2-if (!enable) -udelay (1 ); +if (!enable) { + gating_wait = host->hba->dev_info.clk_gating_wait_us; +if (!gating_wait) {Afaict this can't happen, because in patch 6 you check for
gating_waitbeing 0 and if so set it to0xff.
Quoted 3 lines
Sorry, I was intended to give
clk_gating_wait_usvalues only if it is a UFS3.0 device. I will revise patch 6/8.
Okay, sounds good.
Quoted 24 lines
Quoted 17 lines
drivers/scsi/ufs/ufs-qcom.c static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) · line 866
Quoted 11 lines
drivers/scsi/ufs/ufs-qcom.c +11
-0+udelay (1 ); + }else { +/* +* bRefClkGatingWaitTime defines the minimum +* time for which the reference clock is +* required by device during transition from +* HS-MODE to LS-MODE or HIBERN8 state. Give it +* more time to be on the safe side. +*/ + gating_wait +=10 ; +usleep_range (gating_wait, gating_wait +10 );I presume there's no strong requirement on the max, so how about using a substantially larger max - say 1k, or 10k - to allow the
usleep_range()to do it's job?Quoted 6 lines
PS. Please include linux-arm-msm@ on all the patches in the series, not just two of them.
Regards,
Bjorn
Quoted 7 lines
bRefClkGatingWaitTime, as vendor defined in their device attribute is usually
around 50~100, 1k or 10k delay makes it too large.usleep_range()works well so long as the delay is within (10us - 20ms), so I added 10 to make sure it is
above 10us.
I meant specifically the second parameter, i.e:
usleep_range(bRefClkGatingWaitTime + 10, bRefClkGatingWaitTime + 1000);
As you're not guaranteed an upper bound of this sleep anyway you might as well give usleep_range() a window of a millisecond (or more) to give it the flexibility of matching other timer events.
The only drawback with this is that you might "waste" a millisecond.
Regards,
Bjorn
Quoted 17 lines
SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Useusleep_range
https://www.kernel.org/doc/Documentation/timers/timers-howto.txtThanks,
Can Guo.
Quoted 9 lines
Quoted 9 lines
drivers/scsi/ufs/ufs-qcom.c +2
-1+ } + }writel_relaxed (temp, host->dev_ref_clk_ctrl_mmio); --The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v7 7/8] scsi: ufs-qcom: Delay specific time before gate ref clk
Quoted 94 lines · Bjorn Andersson
Quoted 50 lines · Can Guo
Quoted 39 lines · Bjorn Andersson
Quoted 37 lines · Can Guo
After enter hibern8, as UFS JEDEC ver 3.0 requires, a specific gating wait
time is required before disable the device reference clock. If it is not
specified, use the old delay.Signed-off-by Can Guo <[email protected]>Reviewed-by Asutosh Das <[email protected]>Reviewed-by Hongwu Su <[email protected]>--- drivers/scsi/ufs/ufs-qcom.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)drivers/scsi/ufs/ufs-qcom.c +2
-0@@ -833,6 +833,8 @@ static int ufs_qcom_bus_register(struct ufs_qcom_host *host)static void ufs_qcom_dev_ref_clk_ctrl (struct ufs_qcom_host *host,bool enable) { +unsigned long gating_wait; + if (host->dev_ref_clk_ctrl_mmio &&drivers/scsi/ufs/ufs-qcom.c +1
-1(enable ^ host->is_dev_ref_clk_enabled)) {u32 temp =readl_relaxed (host->dev_ref_clk_ctrl_mmio);@@ -845,11 +847,25 @@ static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable)/* * If we are here to disable this clock it might be immediately * after entering into hibern8 in which case we need to make -* sure that device ref_clk is active at least 1us after the +* sure that device ref_clk is active for specific time after * hibern8 enter. */ Quoted 9 lines
drivers/scsi/ufs/ufs-qcom.c static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) · line 855
Quoted 5 lines
drivers/scsi/ufs/ufs-qcom.c +3
-2-if (!enable) -udelay (1 ); +if (!enable) { + gating_wait = host->hba->dev_info.clk_gating_wait_us; +if (!gating_wait) {Afaict this can't happen, because in patch 6 you check for
gating_waitbeing 0 and if so set it to0xff.Quoted 3 lines
Sorry, I was intended to give
clk_gating_wait_usvalues only if it is a UFS3.0 device. I will revise patch 6/8.Okay, sounds good.
Quoted 24 lines
Quoted 17 lines
drivers/scsi/ufs/ufs-qcom.c static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) · line 866
Quoted 11 lines
drivers/scsi/ufs/ufs-qcom.c +11
-0+udelay (1 ); + }else { +/* +* bRefClkGatingWaitTime defines the minimum +* time for which the reference clock is +* required by device during transition from +* HS-MODE to LS-MODE or HIBERN8 state. Give it +* more time to be on the safe side. +*/ + gating_wait +=10 ; +usleep_range (gating_wait, gating_wait +10 );I presume there's no strong requirement on the max, so how about using a substantially larger max - say 1k, or 10k - to allow the
usleep_range()to do it's job?Quoted 6 lines
PS. Please include linux-arm-msm@ on all the patches in the series, not just two of them.
Regards,
BjornQuoted 7 lines
bRefClkGatingWaitTime, as vendor defined in their device attribute is usually
around 50~100, 1k or 10k delay makes it too large.usleep_range()works well so long as the delay is within (10us - 20ms), so I added 10 to make sure it is
above 10us.I meant specifically the second parameter, i.e:
usleep_range(bRefClkGatingWaitTime + 10, bRefClkGatingWaitTime +
1000);
Quoted 9 lines
As you're not guaranteed an upper bound of this sleep anyway you might as well give
usleep_range()a window of a millisecond (or more) to give it the flexibility of matching other timer events.The only drawback with this is that you might "waste" a millisecond.
Regards,
Bjorn
Hi Bjorn,
Device ref clk gate/ungate can happen very frequently if clk gating is enabled. The "wasted" milliseconds might be accumulated to affect read/write latency, i.e:
If read/write requests come, clk ungate kicks start, it flushes ongoing gate work if any, if we "waste" a millisecond here, the read/write requests will be delayed a millisecond. I am not sure how much it may impact the overall performance in a long term test.
Thanks,
Can Guo.
Quoted 17 lines
Quoted 17 lines
SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Useusleep_range
https://www.kernel.org/doc/Documentation/timers/timers-howto.txtThanks,
Can Guo.
Quoted 9 lines
Quoted 9 lines
drivers/scsi/ufs/ufs-qcom.c +2
-1+ } + }writel_relaxed (temp, host->dev_ref_clk_ctrl_mmio); --The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
PATCH v7 8/8 scsi: ufs: Select INITIAL adapt for HS Gear4
In reply to Can GuoADAPT is added specifically for HS Gear4 mode only, select INITIAL adapt before do power mode change to G4 and select no adapt before switch to non-G4 modes.
--- drivers/scsi/ufs/ufshcd.c | 14 +++++++++++++- drivers/scsi/ufs/ufshci.h | 1 + drivers/scsi/ufs/unipro.h | 7 +++++++ 3 files changed, 21 insertions(+), 1 deletion(-)
drivers/scsi/ufs/ufshcd.c +13 -1
@@ -4071,6 +4071,17 @@ static int ufshcd_change_power_mode(struct ufs_hba *hba,ufshcd_dme_set (hba,UIC_ARG_MIB (PA_HSSERIES ), pwr_mode->hs_rate); +if (hba->ufs_version >=UFSHCI_VERSION_30 ) { +if (pwr_mode->gear_tx ==UFS_HS_G4 ) +/* INITIAL ADAPT */ +ufshcd_dme_set (hba,UIC_ARG_MIB (PA_TXHSADAPTTYPE ), +PA_INITIAL_ADAPT ); +else +/* NO ADAPT */ +ufshcd_dme_set (hba,UIC_ARG_MIB (PA_TXHSADAPTTYPE ), +PA_NO_ADAPT ); + } +ufshcd_dme_set (hba,UIC_ARG_MIB (PA_PWRMODEUSERDATA0 ), DL_FC0ProtectionTimeOutVal_Default); ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA1),
@@ -8449,7 +8460,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)if ((hba->ufs_version !=UFSHCI_VERSION_10 ) && (hba->ufs_version !=UFSHCI_VERSION_11 ) && (hba->ufs_version !=UFSHCI_VERSION_20 ) && - (hba->ufs_version !=UFSHCI_VERSION_21 )) + (hba->ufs_version !=UFSHCI_VERSION_21 ) && + (hba->ufs_version !=UFSHCI_VERSION_30 ))dev_err (hba->dev,"invalid UFS version 0x%x\n" , hba->ufs_version);
drivers/scsi/ufs/ufshci.h +1 -0
@@ -104,6 +104,7 @@ enum {UFSHCI_VERSION_11 =0x00010100 ,/* 1.1 */ UFSHCI_VERSION_20 =0x00000200 ,/* 2.0 */ UFSHCI_VERSION_21 =0x00000210 ,/* 2.1 */ + UFSHCI_VERSION_30 = 0x00000300, /* 3.0 */ };/*
drivers/scsi/ufs/unipro.h +7 -0
@@ -146,6 +146,12 @@#define PA_SLEEPNOCONFIGTIME 0x15A2#define PA_STALLNOCONFIGTIME 0x15A3#define PA_SAVECONFIGTIME 0x15A4 +#define PA_TXHSADAPTTYPE 0x15D4 + +/* Adpat type for PA_TXHSADAPTTYPE attribute */ +#define PA_REFRESH_ADAPT 0x00 +#define PA_INITIAL_ADAPT 0x01 +#define PA_NO_ADAPT 0x03#define PA_TACTIVATE_TIME_UNIT_US 10#define PA_HIBERN8_TIME_UNIT_US 100
@@ -203,6 +209,7 @@ enum ufs_hs_gear_tag {UFS_HS_G1 ,/* HS Gear 1 (default for reset) */ UFS_HS_G2 ,/* HS Gear 2 */ UFS_HS_G3 ,/* HS Gear 3 */ + UFS_HS_G4, /* HS Gear 4 */ }; enum ufs_unipro_ver {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
RE: [PATCH v7 8/8] scsi: ufs: Select INITIAL adapt for HS Gear4
Hi Can,
Quoted 3 lines
ADAPT is added specifically for HS Gear4 mode only, select INITIAL adapt before do power mode change to G4 and select no adapt before switch to non-G4 modes.
UFSHCI 3.0 says:
7.4.1 Adapt
The use of Adapt isn't mandatory but the specification provides some guidelines on its use. The HCI should perform an Initial Adapt in the following cases if the link is running at HS-G4 speed:
- If DME_RESET is initiated.
- If an unused line is activated for HS-G4.
- If UECDME.EC is triggered with bit 3 set to '1'.
- If a change between Rate A and Rate B in HS-G4 is performed.
If it's not mandatory - why are we setting this for all vendors on all platforms? Or am I miss-reading the spec?
Thanks,
Avri
Re: [PATCH v7 8/8] scsi: ufs: Select INITIAL adapt for HS Gear4
Quoted 19 lines · Avri Altman
Hi Can,
Quoted 3 lines
ADAPT is added specifically for HS Gear4 mode only, select INITIAL adapt before do power mode change to G4 and select no adapt before switch to non-G4 modes.
UFSHCI 3.0 says:
7.4.1 Adapt
The use of Adapt isn't mandatory but the specification provides some guidelines on its use.
The HCI should perform an Initial Adapt in the following cases if the link is running at HS-G4
speed:
- IfDME_RESETis initiated.
- If an unused line is activated for HS-G4.
- If UECDME.EC is triggered with bit 3 set to '1'.
- If a change between Rate A and Rate B in HS-G4 is performed.
Quoted 6 lines
If it's not mandatory - why are we setting this for all vendors on all platforms?
Or am I miss-reading the spec?Thanks,
Avri
Hi Avri,
Yes, it is not mandatory, but I don't know a flash vendor that refuses to use ADAPT so far, it is even recommended by flash vendors. So there is no meaning of adding a specific quirk for it, as all flash vendors need this quirk. Otherwise, we would need to add another vops to allow platform vendors to control it before send PMC.
Any other suggestions?
FYI, ADAPT sequence is used to train an M-RX equalizer. It gives both sides better signal integrity against the influence caused by temperature and voltage variations. ADAPT is also used by Quality of Service Monitoring for HS-G4.
As for the usage of it, here is just setting the type of it before PMC to HS-G4, HW will use ADAPT, only if both sides support ADAPT, when it is required as what the spec is saying.
Thanks,
Can Guo.
Re: [PATCH v7 8/8] scsi: ufs: Select INITIAL adapt for HS Gear4
Quoted 42 lines · Can Guo
Quoted 19 lines · Avri Altman
Hi Can,
Quoted 3 lines
ADAPT is added specifically for HS Gear4 mode only, select INITIAL adapt before do power mode change to G4 and select no adapt before switch to non-G4 modes.
UFSHCI 3.0 says:
7.4.1 Adapt
The use of Adapt isn't mandatory but the specification provides some guidelines on its use.
The HCI should perform an Initial Adapt in the following cases if the link is running at HS-G4
speed:
- IfDME_RESETis initiated.
- If an unused line is activated for HS-G4.
- If UECDME.EC is triggered with bit 3 set to '1'.
- If a change between Rate A and Rate B in HS-G4 is performed.Quoted 6 lines
If it's not mandatory - why are we setting this for all vendors on all platforms?
Or am I miss-reading the spec?Thanks,
AvriHi Avri,
Yes, it is not mandatory, but I don't know a flash vendor that refuses to use ADAPT so far, it is even recommended by flash vendors. So there is no meaning of adding a specific quirk for it, as all flash vendors need this quirk. Otherwise, we would need to add another vops to allow platform vendors to control it before send PMC.
Any other suggestions?
FYI, ADAPT sequence is used to train an M-RX equalizer. It gives both sides better signal integrity against the influence caused by temperature and voltage variations. ADAPT is also used by Quality of Service Monitoring for HS-G4.
Quoted 7 lines
As for the usage of it, here is just setting the type of it before PMC to HS-G4, HW will use ADAPT, only if both sides support ADAPT, when it is required as what the spec is saying.
Thanks,
Can Guo.
Hi Avri,
I will drop this one for now as it may take some time to nail down it. I will come back with a separate patch series for it later.
Thanks for the review.
Can Guo.