diff mbox series

[V5,1/6] scsi: ufs: qcom: Update offset for core_clk_1us_cycles

Message ID 20230823154413.23788-2-quic_nitirawa@quicinc.com (mailing list archive)
State Superseded
Headers show
Series scsi: ufs: qcom: Align programming sequence as per HW spec | expand

Commit Message

Nitin Rawat Aug. 23, 2023, 3:44 p.m. UTC 
This Patch updates offset for core_clk_1us_cycles in DME_VS_CORE_CLK_CTRL
register. Offset for core_clk_1us_cycles is changed from Qualcomm UFS
Controller V4.0.0 onwards.

Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 19 ++++++++++++++-----
 drivers/ufs/host/ufs-qcom.h |  2 ++
 2 files changed, 16 insertions(+), 5 deletions(-)

--
2.17.1

Comments

Manivannan Sadhasivam Aug. 28, 2023, 7:38 a.m. UTC | #1 
On Wed, Aug 23, 2023 at 09:14:08PM +0530, Nitin Rawat wrote:
> This Patch updates offset for core_clk_1us_cycles in DME_VS_CORE_CLK_CTRL

Please do not use "This patch" in commit message. Just reword it in imperative
form.

> register. Offset for core_clk_1us_cycles is changed from Qualcomm UFS
> Controller V4.0.0 onwards.
> 
> Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
> Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 19 ++++++++++++++-----
>  drivers/ufs/host/ufs-qcom.h |  2 ++
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index f88febb23123..1108b0cd43b3 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -1297,12 +1297,21 @@ static void ufs_qcom_exit(struct ufs_hba *hba)
>  }
> 
>  static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
> -						       u32 clk_cycles)
> +						       u32 clk_1us_cycles)

How about "cycles_in_1us", since this value specifies "Number of clk cycles in
1us"?

>  {
> -	int err;
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +	u32 mask = DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK;
>  	u32 core_clk_ctrl_reg;
> +	u32 offset = 0;
> +	int err;
> +
> +	/* Bit mask and offset changed on UFS host controller V4.0.0 onwards */

This is not offset value, but rather shift. Still, if you use bitfield macros
as I suggested below, you could get rid of this variable.

> +	if (host->hw_ver.major >= 4) {
> +		mask = MAX_CORE_CLK_1US_CYCLES_MASK_V4;
> +		offset = MAX_CORE_CLK_1US_CYCLES_OFFSET_V4;
> +	}
> 
> -	if (clk_cycles > DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK)
> +	if (clk_1us_cycles > mask)
>  		return -EINVAL;

	if (!FIELD_FIT(mask, cycles_in_1us))
		return -ERANGE;

> 
>  	err = ufshcd_dme_get(hba,
> @@ -1311,8 +1320,8 @@ static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
>  	if (err)
>  		return err;
> 
> -	core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK;
> -	core_clk_ctrl_reg |= clk_cycles;
> +	core_clk_ctrl_reg &= ~(mask << offset);
> +	core_clk_ctrl_reg |= clk_1us_cycles << offset;
> 

	core_clk_ctrl_reg &= ~mask;
	core_clk_ctrl_reg |= FIELD_PREP(mask, cycles_in_1us);

>  	/* Clear CORE_CLK_DIV_EN */
>  	core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT;
> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
> index d6f8e74bd538..a829296e11bb 100644
> --- a/drivers/ufs/host/ufs-qcom.h
> +++ b/drivers/ufs/host/ufs-qcom.h
> @@ -129,6 +129,8 @@ enum {
>  #define PA_VS_CONFIG_REG1	0x9000
>  #define DME_VS_CORE_CLK_CTRL	0xD002
>  /* bit and mask definitions for DME_VS_CORE_CLK_CTRL attribute */

> +#define MAX_CORE_CLK_1US_CYCLES_MASK_V4		0xFFF

#define MAX_CORE_CLK_1US_CYCLES_MASK_V4				GENMASK(27, 16)
#define DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK	GENMASK(7, 0)

- Mani

> +#define MAX_CORE_CLK_1US_CYCLES_OFFSET_V4	0x10
>  #define DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT		BIT(8)
>  #define DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK	0xFF
> 
> --
> 2.17.1
>
Nitin Rawat Aug. 30, 2023, 5:37 p.m. UTC | #2 
On 8/28/2023 1:08 PM, Manivannan Sadhasivam wrote:
> On Wed, Aug 23, 2023 at 09:14:08PM +0530, Nitin Rawat wrote:
>> This Patch updates offset for core_clk_1us_cycles in DME_VS_CORE_CLK_CTRL
> 
> Please do not use "This patch" in commit message. Just reword it in imperative
> form.

Thanks Mani for the review. We Will address the commit text in next 
patchset.

-Nitin


> 
>> register. Offset for core_clk_1us_cycles is changed from Qualcomm UFS
>> Controller V4.0.0 onwards.
>>
>> Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
>> Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
>> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
>> ---
>>   drivers/ufs/host/ufs-qcom.c | 19 ++++++++++++++-----
>>   drivers/ufs/host/ufs-qcom.h |  2 ++
>>   2 files changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index f88febb23123..1108b0cd43b3 100644
>> --- a/drivers/ufs/host/ufs-qcom.c
>> +++ b/drivers/ufs/host/ufs-qcom.c
>> @@ -1297,12 +1297,21 @@ static void ufs_qcom_exit(struct ufs_hba *hba)
>>   }
>>
>>   static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
>> -						       u32 clk_cycles)
>> +						       u32 clk_1us_cycles)
> 
> How about "cycles_in_1us", since this value specifies "Number of clk cycles in
> 1us"?
I Will take care of this in next patchset

-Nitin

> 
>>   {
>> -	int err;
>> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> +	u32 mask = DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK;
>>   	u32 core_clk_ctrl_reg;
>> +	u32 offset = 0;
>> +	int err;
>> +
>> +	/* Bit mask and offset changed on UFS host controller V4.0.0 onwards */
> 
> This is not offset value, but rather shift. Still, if you use bitfield macros
> as I suggested below, you could get rid of this variable.

I Will take care of this in next patchset

-Nitin

> 
>> +	if (host->hw_ver.major >= 4) {
>> +		mask = MAX_CORE_CLK_1US_CYCLES_MASK_V4;
>> +		offset = MAX_CORE_CLK_1US_CYCLES_OFFSET_V4;
>> +	}
>>
>> -	if (clk_cycles > DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK)
>> +	if (clk_1us_cycles > mask)
>>   		return -EINVAL;
> 
> 	if (!FIELD_FIT(mask, cycles_in_1us))
> 		return -ERANGE;
> 
>>
>>   	err = ufshcd_dme_get(hba,
>> @@ -1311,8 +1320,8 @@ static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
>>   	if (err)
>>   		return err;
>>
>> -	core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK;
>> -	core_clk_ctrl_reg |= clk_cycles;
>> +	core_clk_ctrl_reg &= ~(mask << offset);
>> +	core_clk_ctrl_reg |= clk_1us_cycles << offset;
>>
> 
> 	core_clk_ctrl_reg &= ~mask;
> 	core_clk_ctrl_reg |= FIELD_PREP(mask, cycles_in_1us);
> 
>>   	/* Clear CORE_CLK_DIV_EN */
>>   	core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT;
>> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
>> index d6f8e74bd538..a829296e11bb 100644
>> --- a/drivers/ufs/host/ufs-qcom.h
>> +++ b/drivers/ufs/host/ufs-qcom.h
>> @@ -129,6 +129,8 @@ enum {
>>   #define PA_VS_CONFIG_REG1	0x9000
>>   #define DME_VS_CORE_CLK_CTRL	0xD002
>>   /* bit and mask definitions for DME_VS_CORE_CLK_CTRL attribute */
> 
>> +#define MAX_CORE_CLK_1US_CYCLES_MASK_V4		0xFFF
> 
> #define MAX_CORE_CLK_1US_CYCLES_MASK_V4				GENMASK(27, 16)
> #define DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK	GENMASK(7, 0)
> 
> - Mani


I will update it. Thanks
-Nitin

> 
>> +#define MAX_CORE_CLK_1US_CYCLES_OFFSET_V4	0x10
>>   #define DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT		BIT(8)
>>   #define DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK	0xFF
>>
>> --
>> 2.17.1
>>
>
diff mbox series

Patch

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index f88febb23123..1108b0cd43b3 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1297,12 +1297,21 @@  static void ufs_qcom_exit(struct ufs_hba *hba)
 }

 static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
-						       u32 clk_cycles)
+						       u32 clk_1us_cycles)
 {
-	int err;
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	u32 mask = DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK;
 	u32 core_clk_ctrl_reg;
+	u32 offset = 0;
+	int err;
+
+	/* Bit mask and offset changed on UFS host controller V4.0.0 onwards */
+	if (host->hw_ver.major >= 4) {
+		mask = MAX_CORE_CLK_1US_CYCLES_MASK_V4;
+		offset = MAX_CORE_CLK_1US_CYCLES_OFFSET_V4;
+	}

-	if (clk_cycles > DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK)
+	if (clk_1us_cycles > mask)
 		return -EINVAL;

 	err = ufshcd_dme_get(hba,
@@ -1311,8 +1320,8 @@  static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
 	if (err)
 		return err;

-	core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK;
-	core_clk_ctrl_reg |= clk_cycles;
+	core_clk_ctrl_reg &= ~(mask << offset);
+	core_clk_ctrl_reg |= clk_1us_cycles << offset;

 	/* Clear CORE_CLK_DIV_EN */
 	core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT;
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index d6f8e74bd538..a829296e11bb 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -129,6 +129,8 @@  enum {
 #define PA_VS_CONFIG_REG1	0x9000
 #define DME_VS_CORE_CLK_CTRL	0xD002
 /* bit and mask definitions for DME_VS_CORE_CLK_CTRL attribute */
+#define MAX_CORE_CLK_1US_CYCLES_MASK_V4		0xFFF
+#define MAX_CORE_CLK_1US_CYCLES_OFFSET_V4	0x10
 #define DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT		BIT(8)
 #define DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK	0xFF