UFS driver general fixes bundle 4

Signed-off-by Sayali Lokhande <[email protected]>

Signed-off-by Asutosh Das <[email protected]>

Signed-off-by Can Guo <[email protected]>

Signed-off-by Asutosh Das <[email protected]>

Signed-off-by Can Guo <[email protected]>

Reviewed-by Hongwu Su <[email protected]>

From: 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.

Signed-off-by Asutosh Das <[email protected]>

Signed-off-by Can Guo <[email protected]>

Reviewed-by Hongwu Su <[email protected]>

Signed-off-by Can Guo <[email protected]>

The functionality of vendor specific ops should be handled properly in platform specific driver, but should not count on the UFS driver.

Signed-off-by Can Guo <[email protected]>

---

The 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.

Signed-off-by Can Guo <[email protected]>

---

  drivers/scsi/ufs/ufs-qcom.c | 57 +++++++++++++++++++++++++++++++--------------
  1 file changed, 39 insertions(+), 18 deletions(-)

drivers/scsi/ufs/ufs-qcom.c +39 -18

index c69c29a1c..85d7c17 100644
@@ -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)

Signed-off-by Can Guo <[email protected]>

Reviewed-by Hongwu Su <[email protected]>

Reviewed-by Asutosh Das <[email protected]>

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.

Signed-off-by Can Guo <[email protected]>

Reviewed-by Hongwu Su <[email protected]>

Reviewed-by Asutosh Das <[email protected]>

In 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.

Signed-off-by Can Guo <[email protected]>

Reviewed-by Asutosh Das <[email protected]>

In 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.

Signed-off-by Can Guo <[email protected]>

Reviewed-by Asutosh Das <[email protected]>

---
 drivers/scsi/ufs/ufs.h    |  3 +++
 drivers/scsi/ufs/ufshcd.c | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

drivers/scsi/ufs/ufs.h +3 -0

index cfe3803..304076e 100644
@@ -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 spec_version;
+	u32 clk_gating_wait_us;
 };
 
 /**

drivers/scsi/ufs/ufshcd.c +19 -0

index e8f7f9d..d5c547b 100644
@@ -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,37 @@ 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.spec_version >= 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",

drivers/scsi/ufs/ufshcd.c +2 -0

 				      &dev_info->model, SD_ASCII_STD);

@@ -7003,6 +7041,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,

Hi Can,

Quoted 71 lines · Can Guo

In 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.

Signed-off-by Can Guo <[email protected]>

Reviewed-by Asutosh Das <[email protected]>

---
 drivers/scsi/ufs/ufs.h    |  3 +++
 drivers/scsi/ufs/ufshcd.c | 40 
++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

drivers/scsi/ufs/ufs.h +3 -0

index cfe3803..304076e 100644
@@ -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 spec_version;
+	u32 clk_gating_wait_us;
 };
 
 /**

drivers/scsi/ufs/ufshcd.c +19 -0

index e8f7f9d..d5c547b 100644
@@ -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,37 @@ 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.spec_version >= 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",

drivers/scsi/ufs/ufshcd.c static inline int · line 3315

Quoted 10 lines

drivers/scsi/ufs/ufshcd.c +10 -0

+					 gating_wait);
+		}
+
+		/*
+		 * 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.
+		 */
+		hba->dev_info.clk_gating_wait_us = gating_wait + 50;

Not sure if the additional 50us wait time here is too large.

Is there any special reason to fix it as "50"?

Thanks,
Stanley

Quoted 11 lines

drivers/scsi/ufs/ufshcd.c +2 -0

 				      &dev_info->model, SD_ASCII_STD);

@@ -7003,6 +7041,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,

Hi Stanley,

We used to ask vendors about it, 50 is somehow agreed by them. Do you have a
better value in mind?

For me, I just wanted to give it 10, so that we can directly use usleep_range
with it, no need to decide whether to use udelay or usleep_range.

Thanks,
Can Guo.

Quoted 1 line

Quoted 1 line

&dev_info->model, SD_ASCII_STD);

Hi Can,

Quoted 9 lines · Can Guo

Hi Stanley,

We used to ask vendors about it, 50 is somehow agreed by them. Do you have a
better value in mind?

For me, I just wanted to give it 10, so that we can directly use usleep_range
with it, no need to decide whether to use udelay or usleep_range.

Actually I do not have any value in mind because I guess the 50us here is just a margin time added for safety as your comments: "Give it more time to be on the safe side".

An example case is that some vendors only specify 1us in bRefClkGatingWaitTime, so this 50us may be too long compared to device's requirement. If such device really needs this additional 50us, it shall be specified in bRefClkGatingWaitTime.

So if this additional delay does not have any special reason or not mentioned by UFS specification, would you consider move it to vendor specific implementations. By this way, it would be more flexible to be controlled by vendors or by platforms.

Thanks,
Stanley

Quoted 5 lines

Thanks,
Can Guo.

Quoted 1 line

Quoted 1 line

&dev_info->model, SD_ASCII_STD);

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]>

---
 drivers/scsi/ufs/ufs-qcom.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

drivers/scsi/ufs/ufs-qcom.c +2 -0

index 85d7c17..3b5b2d9 100644
@@ -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 -2

 	    (enable ^ host->is_dev_ref_clk_enabled)) {
 		u32 temp = readl_relaxed(host->dev_ref_clk_ctrl_mmio);

@@ -845,11 +847,16 @@ 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)

Signed-off-by Can Guo <[email protected]>

Reviewed-by Asutosh Das <[email protected]>

Hi, Can

Quoted 3 lines

Signed-off-by Can Guo <[email protected]>

Reviewed-by Asutosh Das <[email protected]>

Reviewed-by Bean Huo <[email protected]>

Thanks,

//Bean