All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/2] UFS driver general fixes bundle 2
@ 2020-03-26  9:25 Can Guo
  2020-03-26  9:25 ` [PATCH v6 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path Can Guo
  2020-03-26  9:25   ` Can Guo
  0 siblings, 2 replies; 12+ messages in thread
From: Can Guo @ 2020-03-26  9:25 UTC (permalink / raw)
  To: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn, cang

This bundle includes 2 general fixes for UFS driver.

Changes since v5:
- Fixed a compilation error

Changes since v4:
- Incoperated comments from Avri.
- Added trace back to func ufshcd_scale_clks()
- Removed scale_up_gear goto
- Added "Fixes" tag to commit messages

Changes since v3:
- Removed trivial spaces in comments

Changes since v2:
- Rebased on 5.7/scsi-queue and fixed minor conflicts

Changes since v1:
- Fixed minor typo


Can Guo (1):
  scsi: ufs: Do not rely on prefetched data

Subhash Jadavani (1):
  scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out
    path

 drivers/scsi/ufs/ufshcd.c | 91 ++++++++++++++++++++++++++++++-----------------
 drivers/scsi/ufs/ufshcd.h | 11 ------
 2 files changed, 59 insertions(+), 43 deletions(-)

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v6 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path
  2020-03-26  9:25 [PATCH v6 0/2] UFS driver general fixes bundle 2 Can Guo
@ 2020-03-26  9:25 ` Can Guo
  2020-03-26 10:00   ` [EXT] " Bean Huo (beanhuo)
  2020-03-27 17:04   ` Martin K. Petersen
  2020-03-26  9:25   ` Can Guo
  1 sibling, 2 replies; 12+ messages in thread
From: Can Guo @ 2020-03-26  9:25 UTC (permalink / raw)
  To: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn, cang
  Cc: Subhash Jadavani, Alim Akhtar, Avri Altman, James E.J. Bottomley,
	Martin K. Petersen, Stanley Chu, Bean Huo, Bart Van Assche,
	Venkat Gopalakrishnan, Tomas Winkler, open list

From: Subhash Jadavani <subhashj@codeaurora.org>

This change introduces a func ufshcd_set_clk_freq() to explicitly
set clock frequency so that it can be used in reset_and_resotre path and
in ufshcd_scale_clks(). Meanwhile, this change cleans up the clock scaling
error out path.

Fixes: a3cd5ec55f6c ("scsi: ufs: add load based scaling of UFS gear")
Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 65 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 44 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2a2a63b..148e73a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -855,28 +855,29 @@ static bool ufshcd_is_unipro_pa_params_tuning_req(struct ufs_hba *hba)
 		return false;
 }
 
-static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
+/**
+ * ufshcd_set_clk_freq - set UFS controller clock frequencies
+ * @hba: per adapter instance
+ * @scale_up: If True, set max possible frequency othewise set low frequency
+ *
+ * Returns 0 if successful
+ * Returns < 0 for any other errors
+ */
+static int ufshcd_set_clk_freq(struct ufs_hba *hba, bool scale_up)
 {
 	int ret = 0;
 	struct ufs_clk_info *clki;
 	struct list_head *head = &hba->clk_list_head;
-	ktime_t start = ktime_get();
-	bool clk_state_changed = false;
 
 	if (list_empty(head))
 		goto out;
 
-	ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE);
-	if (ret)
-		return ret;
-
 	list_for_each_entry(clki, head, list) {
 		if (!IS_ERR_OR_NULL(clki->clk)) {
 			if (scale_up && clki->max_freq) {
 				if (clki->curr_freq == clki->max_freq)
 					continue;
 
-				clk_state_changed = true;
 				ret = clk_set_rate(clki->clk, clki->max_freq);
 				if (ret) {
 					dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, %d\n",
@@ -895,7 +896,6 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
 				if (clki->curr_freq == clki->min_freq)
 					continue;
 
-				clk_state_changed = true;
 				ret = clk_set_rate(clki->clk, clki->min_freq);
 				if (ret) {
 					dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, %d\n",
@@ -914,11 +914,37 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
 				clki->name, clk_get_rate(clki->clk));
 	}
 
+out:
+	return ret;
+}
+
+/**
+ * ufshcd_scale_clks - scale up or scale down UFS controller clocks
+ * @hba: per adapter instance
+ * @scale_up: True if scaling up and false if scaling down
+ *
+ * Returns 0 if successful
+ * Returns < 0 for any other errors
+ */
+static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
+{
+	int ret = 0;
+	ktime_t start = ktime_get();
+
+	ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE);
+	if (ret)
+		goto out;
+
+	ret = ufshcd_set_clk_freq(hba, scale_up);
+	if (ret)
+		goto out;
+
 	ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE);
+	if (ret)
+		ufshcd_set_clk_freq(hba, !scale_up);
 
 out:
-	if (clk_state_changed)
-		trace_ufshcd_profile_clk_scaling(dev_name(hba->dev),
+	trace_ufshcd_profile_clk_scaling(dev_name(hba->dev),
 			(scale_up ? "up" : "down"),
 			ktime_to_us(ktime_sub(ktime_get(), start)), ret);
 	return ret;
@@ -1106,35 +1132,32 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool scale_up)
 
 	ret = ufshcd_clock_scaling_prepare(hba);
 	if (ret)
-		return ret;
+		goto out;
 
 	/* scale down the gear before scaling down clocks */
 	if (!scale_up) {
 		ret = ufshcd_scale_gear(hba, false);
 		if (ret)
-			goto out;
+			goto out_unprepare;
 	}
 
 	ret = ufshcd_scale_clks(hba, scale_up);
 	if (ret) {
 		if (!scale_up)
 			ufshcd_scale_gear(hba, true);
-		goto out;
+		goto out_unprepare;
 	}
 
 	/* scale up the gear after scaling up clocks */
 	if (scale_up) {
 		ret = ufshcd_scale_gear(hba, true);
-		if (ret) {
+		if (ret)
 			ufshcd_scale_clks(hba, false);
-			goto out;
-		}
 	}
 
-	ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE);
-
-out:
+out_unprepare:
 	ufshcd_clock_scaling_unprepare(hba);
+out:
 	ufshcd_release(hba);
 	return ret;
 }
@@ -6251,7 +6274,7 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba)
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 
 	/* scale up clocks to max frequency before full reinitialization */
-	ufshcd_scale_clks(hba, true);
+	ufshcd_set_clk_freq(hba, true);
 
 	err = ufshcd_hba_enable(hba);
 	if (err)
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 2/2] scsi: ufs: Do not rely on prefetched data
  2020-03-26  9:25 [PATCH v6 0/2] UFS driver general fixes bundle 2 Can Guo
@ 2020-03-26  9:25   ` Can Guo
  2020-03-26  9:25   ` Can Guo
  1 sibling, 0 replies; 12+ messages in thread
From: Can Guo @ 2020-03-26  9:25 UTC (permalink / raw)
  To: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn, cang
  Cc: Bart Van Assche, Martin K. Petersen, Venkat Gopalakrishnan,
	James E.J. Bottomley, open list, Bjorn Andersson, Avri Altman,
	moderated list:ARM/Mediatek SoC support, Alim Akhtar,
	Matthias Brugger, Tomas Winkler, Stanley Chu,
	moderated list:ARM/Mediatek SoC support, Bean Huo

We were setting bActiveICCLevel attribute for UFS device only once but
type of this attribute has changed from persistent to volatile since UFS
device specification v2.1. This attribute is set to the default value after
power cycle or hardware reset event. It isn't safe to rely on prefetched
data (only used for bActiveICCLevel attribute now). Hence this change
removes the code related to data prefetching and set this parameter on
every attempt to probe the UFS device.

Signed-off-by: Can Guo <cang@codeaurora.org>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
Tested-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/scsi/ufs/ufshcd.c | 26 +++++++++++++++-----------
 drivers/scsi/ufs/ufshcd.h | 11 -----------
 2 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 148e73a..f19a11e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6457,11 +6457,12 @@ static u32 ufshcd_find_max_sup_active_icc_level(struct ufs_hba *hba,
 	return icc_level;
 }
 
-static void ufshcd_init_icc_levels(struct ufs_hba *hba)
+static void ufshcd_set_active_icc_lvl(struct ufs_hba *hba)
 {
 	int ret;
 	int buff_len = hba->desc_size.pwr_desc;
 	u8 *desc_buf;
+	u32 icc_level;
 
 	desc_buf = kmalloc(buff_len, GFP_KERNEL);
 	if (!desc_buf)
@@ -6476,20 +6477,17 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba)
 		goto out;
 	}
 
-	hba->init_prefetch_data.icc_level =
-			ufshcd_find_max_sup_active_icc_level(hba,
-			desc_buf, buff_len);
-	dev_dbg(hba->dev, "%s: setting icc_level 0x%x",
-			__func__, hba->init_prefetch_data.icc_level);
+	icc_level = ufshcd_find_max_sup_active_icc_level(hba, desc_buf,
+							 buff_len);
+	dev_dbg(hba->dev, "%s: setting icc_level 0x%x", __func__, icc_level);
 
 	ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
-		QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0,
-		&hba->init_prefetch_data.icc_level);
+		QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, &icc_level);
 
 	if (ret)
 		dev_err(hba->dev,
 			"%s: Failed configuring bActiveICCLevel = %d ret = %d",
-			__func__, hba->init_prefetch_data.icc_level , ret);
+			__func__, icc_level, ret);
 
 out:
 	kfree(desc_buf);
@@ -6995,8 +6993,6 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
 {
 	int ret;
 
-	ufshcd_init_icc_levels(hba);
-
 	/* Add required well known logical units to scsi mid layer */
 	ret = ufshcd_scsi_add_wlus(hba);
 	if (ret)
@@ -7094,6 +7090,14 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
 		}
 	}
 
+	/*
+	 * bActiveICCLevel is volatile for UFS device (as per latest v2.1 spec)
+	 * and for removable UFS card as well, hence always set the parameter.
+	 * Note: Error handler may issue the device reset hence resetting
+	 * bActiveICCLevel as well so it is always safe to set this here.
+	 */
+	ufshcd_set_active_icc_lvl(hba);
+
 	/* set the state as operational after switching to desired gear */
 	hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index d45a044..5652d39 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -404,15 +404,6 @@ struct ufs_clk_scaling {
 	bool is_suspended;
 };
 
-/**
- * struct ufs_init_prefetch - contains data that is pre-fetched once during
- * initialization
- * @icc_level: icc level which was read during initialization
- */
-struct ufs_init_prefetch {
-	u32 icc_level;
-};
-
 #define UFS_ERR_REG_HIST_LENGTH 8
 /**
  * struct ufs_err_reg_hist - keeps history of errors
@@ -544,7 +535,6 @@ enum ufshcd_quirks {
  * @intr_mask: Interrupt Mask Bits
  * @ee_ctrl_mask: Exception event control mask
  * @is_powered: flag to check if HBA is powered
- * @init_prefetch_data: data pre-fetched during initialization
  * @eh_work: Worker to handle UFS errors that require s/w attention
  * @eeh_work: Worker to handle exception events
  * @errors: HBA errors
@@ -632,7 +622,6 @@ struct ufs_hba {
 	u32 intr_mask;
 	u16 ee_ctrl_mask;
 	bool is_powered;
-	struct ufs_init_prefetch init_prefetch_data;
 
 	/* Work Queues */
 	struct work_struct eh_work;
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 2/2] scsi: ufs: Do not rely on prefetched data
@ 2020-03-26  9:25   ` Can Guo
  0 siblings, 0 replies; 12+ messages in thread
From: Can Guo @ 2020-03-26  9:25 UTC (permalink / raw)
  To: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn, cang
  Cc: Bart Van Assche, Martin K. Petersen, Venkat Gopalakrishnan,
	James E.J. Bottomley, open list, Bjorn Andersson, Avri Altman,
	moderated list:ARM/Mediatek SoC support, Alim Akhtar,
	Matthias Brugger, Tomas Winkler, Stanley Chu,
	moderated list:ARM/Mediatek SoC support, Bean Huo

We were setting bActiveICCLevel attribute for UFS device only once but
type of this attribute has changed from persistent to volatile since UFS
device specification v2.1. This attribute is set to the default value after
power cycle or hardware reset event. It isn't safe to rely on prefetched
data (only used for bActiveICCLevel attribute now). Hence this change
removes the code related to data prefetching and set this parameter on
every attempt to probe the UFS device.

Signed-off-by: Can Guo <cang@codeaurora.org>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
Tested-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/scsi/ufs/ufshcd.c | 26 +++++++++++++++-----------
 drivers/scsi/ufs/ufshcd.h | 11 -----------
 2 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 148e73a..f19a11e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6457,11 +6457,12 @@ static u32 ufshcd_find_max_sup_active_icc_level(struct ufs_hba *hba,
 	return icc_level;
 }
 
-static void ufshcd_init_icc_levels(struct ufs_hba *hba)
+static void ufshcd_set_active_icc_lvl(struct ufs_hba *hba)
 {
 	int ret;
 	int buff_len = hba->desc_size.pwr_desc;
 	u8 *desc_buf;
+	u32 icc_level;
 
 	desc_buf = kmalloc(buff_len, GFP_KERNEL);
 	if (!desc_buf)
@@ -6476,20 +6477,17 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba)
 		goto out;
 	}
 
-	hba->init_prefetch_data.icc_level =
-			ufshcd_find_max_sup_active_icc_level(hba,
-			desc_buf, buff_len);
-	dev_dbg(hba->dev, "%s: setting icc_level 0x%x",
-			__func__, hba->init_prefetch_data.icc_level);
+	icc_level = ufshcd_find_max_sup_active_icc_level(hba, desc_buf,
+							 buff_len);
+	dev_dbg(hba->dev, "%s: setting icc_level 0x%x", __func__, icc_level);
 
 	ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
-		QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0,
-		&hba->init_prefetch_data.icc_level);
+		QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, &icc_level);
 
 	if (ret)
 		dev_err(hba->dev,
 			"%s: Failed configuring bActiveICCLevel = %d ret = %d",
-			__func__, hba->init_prefetch_data.icc_level , ret);
+			__func__, icc_level, ret);
 
 out:
 	kfree(desc_buf);
@@ -6995,8 +6993,6 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
 {
 	int ret;
 
-	ufshcd_init_icc_levels(hba);
-
 	/* Add required well known logical units to scsi mid layer */
 	ret = ufshcd_scsi_add_wlus(hba);
 	if (ret)
@@ -7094,6 +7090,14 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
 		}
 	}
 
+	/*
+	 * bActiveICCLevel is volatile for UFS device (as per latest v2.1 spec)
+	 * and for removable UFS card as well, hence always set the parameter.
+	 * Note: Error handler may issue the device reset hence resetting
+	 * bActiveICCLevel as well so it is always safe to set this here.
+	 */
+	ufshcd_set_active_icc_lvl(hba);
+
 	/* set the state as operational after switching to desired gear */
 	hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index d45a044..5652d39 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -404,15 +404,6 @@ struct ufs_clk_scaling {
 	bool is_suspended;
 };
 
-/**
- * struct ufs_init_prefetch - contains data that is pre-fetched once during
- * initialization
- * @icc_level: icc level which was read during initialization
- */
-struct ufs_init_prefetch {
-	u32 icc_level;
-};
-
 #define UFS_ERR_REG_HIST_LENGTH 8
 /**
  * struct ufs_err_reg_hist - keeps history of errors
@@ -544,7 +535,6 @@ enum ufshcd_quirks {
  * @intr_mask: Interrupt Mask Bits
  * @ee_ctrl_mask: Exception event control mask
  * @is_powered: flag to check if HBA is powered
- * @init_prefetch_data: data pre-fetched during initialization
  * @eh_work: Worker to handle UFS errors that require s/w attention
  * @eeh_work: Worker to handle exception events
  * @errors: HBA errors
@@ -632,7 +622,6 @@ struct ufs_hba {
 	u32 intr_mask;
 	u16 ee_ctrl_mask;
 	bool is_powered;
-	struct ufs_init_prefetch init_prefetch_data;
 
 	/* Work Queues */
 	struct work_struct eh_work;
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* RE: [EXT] [PATCH v6 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path
  2020-03-26  9:25 ` [PATCH v6 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path Can Guo
@ 2020-03-26 10:00   ` Bean Huo (beanhuo)
  2020-03-27 17:04   ` Martin K. Petersen
  1 sibling, 0 replies; 12+ messages in thread
From: Bean Huo (beanhuo) @ 2020-03-26 10:00 UTC (permalink / raw)
  To: Can Guo, asutoshd, nguyenb, hongwus, rnayak, linux-scsi,
	kernel-team, saravanak, salyzyn
  Cc: Subhash Jadavani, Alim Akhtar, Avri Altman, James E.J. Bottomley,
	Martin K. Petersen, Stanley Chu, Bart Van Assche,
	Venkat Gopalakrishnan, Tomas Winkler, open list

> Fixes: a3cd5ec55f6c ("scsi: ufs: add load based scaling of UFS gear")
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 2/2] scsi: ufs: Do not rely on prefetched data
  2020-03-26  9:25   ` Can Guo
  (?)
@ 2020-03-27  2:58     ` Martin K. Petersen
  -1 siblings, 0 replies; 12+ messages in thread
From: Martin K. Petersen @ 2020-03-27  2:58 UTC (permalink / raw)
  To: Can Guo
  Cc: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn, Alim Akhtar, Avri Altman,
	James E.J. Bottomley, Martin K. Petersen, Matthias Brugger,
	Stanley Chu, Bean Huo, Bart Van Assche, Venkat Gopalakrishnan,
	Tomas Winkler, Bjorn Andersson, open list,
	moderated list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support


Can,

> We were setting bActiveICCLevel attribute for UFS device only once but
> type of this attribute has changed from persistent to volatile since
> UFS device specification v2.1. This attribute is set to the default
> value after power cycle or hardware reset event. It isn't safe to rely
> on prefetched data (only used for bActiveICCLevel attribute
> now). Hence this change removes the code related to data prefetching
> and set this parameter on every attempt to probe the UFS device.

Applied patch #2 to 5.7/scsi-queue. Awaiting Avri's feedback on patch
#1. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 2/2] scsi: ufs: Do not rely on prefetched data
@ 2020-03-27  2:58     ` Martin K. Petersen
  0 siblings, 0 replies; 12+ messages in thread
From: Martin K. Petersen @ 2020-03-27  2:58 UTC (permalink / raw)
  To: Can Guo
  Cc: nguyenb, Alim Akhtar, Bean Huo, Stanley Chu, Bart Van Assche,
	saravanak, Tomas Winkler, kernel-team, salyzyn,
	James E.J. Bottomley, Avri Altman,
	moderated list:ARM/Mediatek SoC support, Matthias Brugger,
	Bjorn Andersson, moderated list:ARM/Mediatek SoC support, rnayak,
	linux-scsi, Martin K. Petersen, Venkat Gopalakrishnan, open list,
	hongwus, asutoshd


Can,

> We were setting bActiveICCLevel attribute for UFS device only once but
> type of this attribute has changed from persistent to volatile since
> UFS device specification v2.1. This attribute is set to the default
> value after power cycle or hardware reset event. It isn't safe to rely
> on prefetched data (only used for bActiveICCLevel attribute
> now). Hence this change removes the code related to data prefetching
> and set this parameter on every attempt to probe the UFS device.

Applied patch #2 to 5.7/scsi-queue. Awaiting Avri's feedback on patch
#1. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 2/2] scsi: ufs: Do not rely on prefetched data
@ 2020-03-27  2:58     ` Martin K. Petersen
  0 siblings, 0 replies; 12+ messages in thread
From: Martin K. Petersen @ 2020-03-27  2:58 UTC (permalink / raw)
  To: Can Guo
  Cc: nguyenb, Alim Akhtar, Bean Huo, Stanley Chu, Bart Van Assche,
	saravanak, Tomas Winkler, kernel-team, salyzyn,
	James E.J. Bottomley, Avri Altman,
	moderated list:ARM/Mediatek SoC support, Matthias Brugger,
	Bjorn Andersson, moderated list:ARM/Mediatek SoC support, rnayak,
	linux-scsi, Martin K. Petersen, Venkat Gopalakrishnan, open list,
	hongwus, asutoshd


Can,

> We were setting bActiveICCLevel attribute for UFS device only once but
> type of this attribute has changed from persistent to volatile since
> UFS device specification v2.1. This attribute is set to the default
> value after power cycle or hardware reset event. It isn't safe to rely
> on prefetched data (only used for bActiveICCLevel attribute
> now). Hence this change removes the code related to data prefetching
> and set this parameter on every attempt to probe the UFS device.

Applied patch #2 to 5.7/scsi-queue. Awaiting Avri's feedback on patch
#1. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v6 2/2] scsi: ufs: Do not rely on prefetched data
  2020-03-27  2:58     ` Martin K. Petersen
  (?)
@ 2020-03-27  7:49       ` Avri Altman
  -1 siblings, 0 replies; 12+ messages in thread
From: Avri Altman @ 2020-03-27  7:49 UTC (permalink / raw)
  To: Martin K. Petersen, Can Guo
  Cc: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn, Alim Akhtar, James E.J. Bottomley,
	Matthias Brugger, Stanley Chu, Bean Huo, Bart Van Assche,
	Venkat Gopalakrishnan, Tomas Winkler, Bjorn Andersson, open list,
	moderated list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support



> 
> Can,
> 
> > We were setting bActiveICCLevel attribute for UFS device only once but
> > type of this attribute has changed from persistent to volatile since
> > UFS device specification v2.1. This attribute is set to the default
> > value after power cycle or hardware reset event. It isn't safe to rely
> > on prefetched data (only used for bActiveICCLevel attribute
> > now). Hence this change removes the code related to data prefetching
> > and set this parameter on every attempt to probe the UFS device.
> 
> Applied patch #2 to 5.7/scsi-queue. Awaiting Avri's feedback on patch
> #1. Thanks!
It looks fine.
Thanks,
Avri

> 
> --
> Martin K. Petersen      Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v6 2/2] scsi: ufs: Do not rely on prefetched data
@ 2020-03-27  7:49       ` Avri Altman
  0 siblings, 0 replies; 12+ messages in thread
From: Avri Altman @ 2020-03-27  7:49 UTC (permalink / raw)
  To: Martin K. Petersen, Can Guo
  Cc: moderated list:ARM/Mediatek SoC support, Bean Huo, rnayak,
	saravanak, linux-scsi, Venkat Gopalakrishnan,
	James E.J. Bottomley, open list, nguyenb, Bjorn Andersson,
	moderated list:ARM/Mediatek SoC support, salyzyn, Alim Akhtar,
	Matthias Brugger, Tomas Winkler, Stanley Chu, kernel-team,
	Bart Van Assche, hongwus, asutoshd



> 
> Can,
> 
> > We were setting bActiveICCLevel attribute for UFS device only once but
> > type of this attribute has changed from persistent to volatile since
> > UFS device specification v2.1. This attribute is set to the default
> > value after power cycle or hardware reset event. It isn't safe to rely
> > on prefetched data (only used for bActiveICCLevel attribute
> > now). Hence this change removes the code related to data prefetching
> > and set this parameter on every attempt to probe the UFS device.
> 
> Applied patch #2 to 5.7/scsi-queue. Awaiting Avri's feedback on patch
> #1. Thanks!
It looks fine.
Thanks,
Avri

> 
> --
> Martin K. Petersen      Oracle Linux Engineering

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v6 2/2] scsi: ufs: Do not rely on prefetched data
@ 2020-03-27  7:49       ` Avri Altman
  0 siblings, 0 replies; 12+ messages in thread
From: Avri Altman @ 2020-03-27  7:49 UTC (permalink / raw)
  To: Martin K. Petersen, Can Guo
  Cc: moderated list:ARM/Mediatek SoC support, Bean Huo, rnayak,
	saravanak, linux-scsi, Venkat Gopalakrishnan,
	James E.J. Bottomley, open list, nguyenb, Bjorn Andersson,
	moderated list:ARM/Mediatek SoC support, salyzyn, Alim Akhtar,
	Matthias Brugger, Tomas Winkler, Stanley Chu, kernel-team,
	Bart Van Assche, hongwus, asutoshd



> 
> Can,
> 
> > We were setting bActiveICCLevel attribute for UFS device only once but
> > type of this attribute has changed from persistent to volatile since
> > UFS device specification v2.1. This attribute is set to the default
> > value after power cycle or hardware reset event. It isn't safe to rely
> > on prefetched data (only used for bActiveICCLevel attribute
> > now). Hence this change removes the code related to data prefetching
> > and set this parameter on every attempt to probe the UFS device.
> 
> Applied patch #2 to 5.7/scsi-queue. Awaiting Avri's feedback on patch
> #1. Thanks!
It looks fine.
Thanks,
Avri

> 
> --
> Martin K. Petersen      Oracle Linux Engineering

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path
  2020-03-26  9:25 ` [PATCH v6 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path Can Guo
  2020-03-26 10:00   ` [EXT] " Bean Huo (beanhuo)
@ 2020-03-27 17:04   ` Martin K. Petersen
  1 sibling, 0 replies; 12+ messages in thread
From: Martin K. Petersen @ 2020-03-27 17:04 UTC (permalink / raw)
  To: Can Guo
  Cc: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn, Subhash Jadavani, Alim Akhtar, Avri Altman,
	James E.J. Bottomley, Martin K. Petersen, Stanley Chu, Bean Huo,
	Bart Van Assche, Venkat Gopalakrishnan, Tomas Winkler, open list


Can,

> This change introduces a func ufshcd_set_clk_freq() to explicitly set
> clock frequency so that it can be used in reset_and_resotre path and
> in ufshcd_scale_clks(). Meanwhile, this change cleans up the clock
> scaling error out path.

Applied to 5.7/scsi-queue, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-03-27 17:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26  9:25 [PATCH v6 0/2] UFS driver general fixes bundle 2 Can Guo
2020-03-26  9:25 ` [PATCH v6 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path Can Guo
2020-03-26 10:00   ` [EXT] " Bean Huo (beanhuo)
2020-03-27 17:04   ` Martin K. Petersen
2020-03-26  9:25 ` [PATCH v6 2/2] scsi: ufs: Do not rely on prefetched data Can Guo
2020-03-26  9:25   ` Can Guo
2020-03-27  2:58   ` Martin K. Petersen
2020-03-27  2:58     ` Martin K. Petersen
2020-03-27  2:58     ` Martin K. Petersen
2020-03-27  7:49     ` Avri Altman
2020-03-27  7:49       ` Avri Altman
2020-03-27  7:49       ` 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.