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(+)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index cfe3803..304076e 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 spec_version;
+ u32 clk_gating_wait_us;
};
/**
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e8f7f9d..d5c547b 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,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",
+ 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;
+ }
+
+ return err;
+}
+
/**
* ufshcd_memory_alloc - allocate memory for host memory space data structures
* @hba: per adapter instance
@@ -6626,6 +6660,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->spec_version = 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 +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,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Hi, Can
>
> 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]>
This looks fine.
Reviewed-by: Bean Huo <[email protected]>
Thanks,
//Bean
Hi Can,
On Mon, 2020-02-03 at 01:17 -0800, Can Guo wrote:
> 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(+)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index cfe3803..304076e 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 spec_version;
> + u32 clk_gating_wait_us;
> };
>
> /**
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index e8f7f9d..d5c547b 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,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",
> + 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
> &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,
On 2020-02-05 10:50, Stanley Chu wrote:
> Hi Can,
>
> On Mon, 2020-02-03 at 01:17 -0800, Can Guo wrote:
>> 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(+)
>>
>> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
>> index cfe3803..304076e 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 spec_version;
>> + u32 clk_gating_wait_us;
>> };
>>
>> /**
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index e8f7f9d..d5c547b 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,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",
>> + 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
>
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.
>> &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,
On Wed, 2020-02-05 at 12:52 +0800, Can Guo wrote:
> 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
>
> Thanks,
> Can Guo.
>
> >> &dev_info->model, SD_ASCII_STD);
On 2020-02-06 08:55, Stanley Chu wrote:
> Hi Can,
>
> On Wed, 2020-02-05 at 12:52 +0800, Can Guo wrote:
>
>
>> 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
>
>>
>> Thanks,
>> Can Guo.
>>
>> >> &dev_info->model, SD_ASCII_STD);
Hi Stanley,
FYI, the default values in bRefClkGatingWaitTime from vendors are around
50 - 100.
I agree with you. I will just remove the extra delay here and let's
handle it in our own platform drivers.
Thanks,
Can Guo.