[PATCH v7 2/8] scsi: ufs: set load before setting voltage in regulators

[PATCH v7 2/8] scsi: ufs: set load before setting voltage in regulators

[Can Guo]

From: Asutosh Das <asutoshd@codeaurora.org>

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 <asutoshd@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Reviewed-by: Hongwu Su <hongwus@codeaurora.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 10dbc0c..83ae093 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -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;
-- 
The Qualcomm Innovation Center, Inc. is a member of the 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

[PATCH v7 3/8] scsi: ufs: Remove the check before call setup clock notify vops

[Can Guo]

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 <cang@codeaurora.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 83ae093..bbc2607 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -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) {
-- 
The Qualcomm Innovation Center, Inc. is a member of the 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

[PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic

[Can Guo]

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 <cang@codeaurora.org>
Reviewed-by: Hongwu Su <hongwus@codeaurora.org>
Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bbc2607..e8f7f9d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -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);
-- 
The Qualcomm Innovation Center, Inc. is a member of the 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

RE: [PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic

[Avri Altman]

Hi,

> 
> 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)
> 
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Reviewed-by: Hongwu Su <hongwus@codeaurora.org>
> Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
> Reviewed-by: Bean Huo <beanhuo@micron.com>
> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index bbc2607..e8f7f9d 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -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);
Since now the above code is shared in all cases,
Maybe find a more economical way to pack it?

Thanks,
Avri


> --
> The Qualcomm Innovation Center, Inc. is a member of the 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

Re: [PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic

[Can Guo]

On 2020-02-06 18:28, Avri Altman wrote:
> Hi,
> 
>> 
>> 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.

>> 
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> Reviewed-by: Hongwu Su <hongwus@codeaurora.org>
>> Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
>> Reviewed-by: Bean Huo <beanhuo@micron.com>
>> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
>> ---
>>  drivers/scsi/ufs/ufshcd.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index bbc2607..e8f7f9d 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -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);
> Since now the above code is shared in all cases,
> Maybe find a more economical way to pack it?
> 
> 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.

>> --
>> The Qualcomm Innovation Center, Inc. is a member of the 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

Re: [PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic

[Can Guo]

On 2020-02-10 09:28, Can Guo wrote:
> On 2020-02-06 18:28, Avri Altman wrote:
>> Hi,
>> 
>>> 
>>> 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.
> 
>>> 
>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>> Reviewed-by: Hongwu Su <hongwus@codeaurora.org>
>>> Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
>>> Reviewed-by: Bean Huo <beanhuo@micron.com>
>>> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
>>> ---
>>>  drivers/scsi/ufs/ufshcd.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>> 
>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>> index bbc2607..e8f7f9d 100644
>>> --- a/drivers/scsi/ufs/ufshcd.c
>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>> @@ -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);
>> Since now the above code is shared in all cases,
>> Maybe find a more economical way to pack it?
>> 
>> 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?
> 

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.

> Regards,
> Can Guo.
> 
>>> --
>>> The Qualcomm Innovation Center, Inc. is a member of the 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

RE: [PATCH v7 5/8] scsi: ufs: Fix ufshcd_hold() caused scheduling while atomic

[Avri Altman]

> >> 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.
fair enough.

> >
> >>>
> >>> Signed-off-by: Can Guo <cang@codeaurora.org>
> >>> Reviewed-by: Hongwu Su <hongwus@codeaurora.org>
> >>> Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
> >>> Reviewed-by: Bean Huo <beanhuo@micron.com>
> >>> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
> >>> ---
> >>>  drivers/scsi/ufs/ufshcd.c | 5 +++++
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> >>> index bbc2607..e8f7f9d 100644
> >>> --- a/drivers/scsi/ufs/ufshcd.c
> >>> +++ b/drivers/scsi/ufs/ufshcd.c
> >>> @@ -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);
> >> Since now the above code is shared in all cases,
> >> Maybe find a more economical way to pack it?
> >>
> >> 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?
> >
> 
> 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.
OK.

Thanks,
Avri

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

[PATCH v7 6/8] scsi: ufs: Add dev ref clock gating wait time support

[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 <cang@codeaurora.org>
Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufs.h    |  3 +++
 drivers/scsi/ufs/ufshcd.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index cfe3803..990cb48 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -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;
 };
 
 /**
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e8f7f9d..20fa509 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -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,
-- 
The Qualcomm Innovation Center, Inc. is a member of the 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