[PATCH V2 0/3] scsi: ufs: qcom: Align programming sequence as per HW spec

[PATCH V2 0/3] scsi: ufs: qcom: Align programming sequence as per HW spec

[Nitin Rawat]

This patch series adds programming support for Qualcomm UFS
to align with Hardware Specification.

In this patch series below changes are taken care.

1. Enable QUnipro Internal Clock Gating
2. Update esi_vec_mask for HW major version >= 6

Changes from v1:
1. Moved ufshcd_dme_rmw to ufshcd.h as per avri's comment.

Bao D. Nguyen (1):
  scsi: ufs: ufs-qcom: Update esi_vec_mask for HW major version >= 6

Nitin Rawat (2):
  scsi: ufs: core: Add ufshcd_dme_rmw to modify DME attributes
  scsi: ufs: qcom: Enable QUnipro Internal Clock Gating

 drivers/ufs/host/ufs-qcom.c | 24 ++++++++++++++++++++++--
 drivers/ufs/host/ufs-qcom.h |  9 +++++++++
 include/ufs/ufshcd.h        | 26 ++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 2 deletions(-)

--
2.48.1

[PATCH V2 1/3] scsi: ufs: ufs-qcom: Update esi_vec_mask for HW major version >= 6

[Nitin Rawat]

From: "Bao D. Nguyen" <quic_nguyenb@quicinc.com>

The MCQ feature and ESI are supported by all Qualcomm UFS controller
versions 6 and above.

Therefore, update the ESI vector mask in the UFS_MEM_CFG3 register
for platforms with major version number of 6 or higher.

Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 318dca7fe3d7..dfdc52333a96 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -2113,8 +2113,7 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
 
 	retain_and_null_ptr(qi);
 
-	if (host->hw_ver.major == 6 && host->hw_ver.minor == 0 &&
-	    host->hw_ver.step == 0) {
+	if (host->hw_ver.major >= 6) {
 		ufshcd_rmwl(hba, ESI_VEC_MASK, FIELD_PREP(ESI_VEC_MASK, MAX_ESI_VEC - 1),
 			    REG_UFS_CFG3);
 	}
-- 
2.48.1

Re: [PATCH V2 1/3] scsi: ufs: ufs-qcom: Update esi_vec_mask for HW major version >= 6

[Manivannan Sadhasivam]

On Tue, Jul 08, 2025 at 02:32:58AM GMT, Nitin Rawat wrote:
> From: "Bao D. Nguyen" <quic_nguyenb@quicinc.com>
> 
> The MCQ feature and ESI are supported by all Qualcomm UFS controller
> versions 6 and above.
> 
> Therefore, update the ESI vector mask in the UFS_MEM_CFG3 register
> for platforms with major version number of 6 or higher.
> 
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
>  drivers/ufs/host/ufs-qcom.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 318dca7fe3d7..dfdc52333a96 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -2113,8 +2113,7 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
>  
>  	retain_and_null_ptr(qi);
>  
> -	if (host->hw_ver.major == 6 && host->hw_ver.minor == 0 &&
> -	    host->hw_ver.step == 0) {
> +	if (host->hw_ver.major >= 6) {
>  		ufshcd_rmwl(hba, ESI_VEC_MASK, FIELD_PREP(ESI_VEC_MASK, MAX_ESI_VEC - 1),
>  			    REG_UFS_CFG3);
>  	}
> -- 
> 2.48.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V2 1/3] scsi: ufs: ufs-qcom: Update esi_vec_mask for HW major version >= 6

[Manivannan Sadhasivam]

On Tue, Jul 08, 2025 at 02:32:58AM GMT, Nitin Rawat wrote:
> From: "Bao D. Nguyen" <quic_nguyenb@quicinc.com>

Nit: Please use consistent subject prefix:

scsi: ufs: qcom:

Maybe we should get rid of 'scsi' prefix since the ufs code is now moved
outside of drivers/scsi/. Bart?

- Mani

> 
> The MCQ feature and ESI are supported by all Qualcomm UFS controller
> versions 6 and above.
> 
> Therefore, update the ESI vector mask in the UFS_MEM_CFG3 register
> for platforms with major version number of 6 or higher.
> 
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 318dca7fe3d7..dfdc52333a96 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -2113,8 +2113,7 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
>  
>  	retain_and_null_ptr(qi);
>  
> -	if (host->hw_ver.major == 6 && host->hw_ver.minor == 0 &&
> -	    host->hw_ver.step == 0) {
> +	if (host->hw_ver.major >= 6) {
>  		ufshcd_rmwl(hba, ESI_VEC_MASK, FIELD_PREP(ESI_VEC_MASK, MAX_ESI_VEC - 1),
>  			    REG_UFS_CFG3);
>  	}
> -- 
> 2.48.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V2 1/3] scsi: ufs: ufs-qcom: Update esi_vec_mask for HW major version >= 6

[Bart Van Assche]

On 7/8/25 12:53 AM, Manivannan Sadhasivam wrote:
> On Tue, Jul 08, 2025 at 02:32:58AM GMT, Nitin Rawat wrote:
>> From: "Bao D. Nguyen" <quic_nguyenb@quicinc.com>
> 
> Nit: Please use consistent subject prefix:
> 
> scsi: ufs: qcom:
> 
> Maybe we should get rid of 'scsi' prefix since the ufs code is now moved
> outside of drivers/scsi/. Bart?
Dropping the "scsi:" prefix sounds good to me because this prefix makes
patch subject lines a bit long.

Thanks,

Bart.

Re: [PATCH V2 1/3] scsi: ufs: ufs-qcom: Update esi_vec_mask for HW major version >= 6

[Martin K. Petersen]

Bart,

>> Maybe we should get rid of 'scsi' prefix since the ufs code is now
>> moved outside of drivers/scsi/. Bart?

> Dropping the "scsi:" prefix sounds good to me because this prefix makes
> patch subject lines a bit long.

I have attempting separating SCSI and UFS a couple of times in the past.
However, there always seemed to be at least one SCSI core change
dependency per cycle which prevented that from happening.

I don't think we have had any for a while so I'll try to do two separate
trees for 6.18.

-- 
Martin K. Petersen

Re: [PATCH V2 1/3] scsi: ufs: ufs-qcom: Update esi_vec_mask for HW major version >= 6

[Bart Van Assche]

On 7/14/25 10:25 AM, Martin K. Petersen wrote:
>>> Maybe we should get rid of 'scsi' prefix since the ufs code is now
>>> moved outside of drivers/scsi/. Bart?
> 
>> Dropping the "scsi:" prefix sounds good to me because this prefix makes
>> patch subject lines a bit long.
> 
> I have attempting separating SCSI and UFS a couple of times in the past.
> However, there always seemed to be at least one SCSI core change
> dependency per cycle which prevented that from happening.
> 
> I don't think we have had any for a while so I'll try to do two separate
> trees for 6.18.

Hi Martin,

Thanks for the feedback, but please keep in mind that I plan to repost
this patch series soon, a series that includes SCSI core and UFS driver
changes: "[PATCH 00/24] Optimize the hot path in the UFS driver"
(https://lore.kernel.org/linux-scsi/20250403211937.2225615-1-bvanassche@acm.org/). 
For the future, I expect such patch changes to be
the exception rather than the norm.

Thanks,

Bart.

Re: [PATCH V2 1/3] scsi: ufs: ufs-qcom: Update esi_vec_mask for HW major version >= 6

[Martin K. Petersen]

Bart,

> Thanks for the feedback, but please keep in mind that I plan to repost
> this patch series soon, a series that includes SCSI core and UFS
> driver changes: "[PATCH 00/24] Optimize the hot path in the UFS
> driver"
> (https://lore.kernel.org/linux-scsi/20250403211937.2225615-1-bvanassche@acm.org/).
> For the future, I expect such patch changes to be the exception rather
> than the norm.

OK, that's fine.

-- 
Martin K. Petersen

[PATCH V2 2/3] scsi: ufs: core: Add ufshcd_dme_rmw to modify DME attributes

[Nitin Rawat]

Introduce `ufshcd_dme_rmw` API to read, modify, and write DME
attributes in UFS host controllers using a mask and value.

Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 include/ufs/ufshcd.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 9b3515cee711..fe4bb248484c 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1498,6 +1498,32 @@ static inline int ufshcd_vops_phy_initialization(struct ufs_hba *hba)
 	return 0;
 }

+/**
+ * ufshcd_dme_rmw - get modify set a dme attribute
+ * @hba - per adapter instance
+ * @mask - mask to apply on read value
+ * @val - actual value to write
+ * @attr - dme attribute
+ */
+static inline int ufshcd_dme_rmw(struct ufs_hba *hba, u32 mask,
+				 u32 val, u32 attr)
+{
+	u32 cfg = 0;
+	int err = 0;
+
+	err = ufshcd_dme_get(hba, UIC_ARG_MIB(attr), &cfg);
+	if (err)
+		goto out;
+
+	cfg &= ~mask;
+	cfg |= (val & mask);
+
+	err = ufshcd_dme_set(hba, UIC_ARG_MIB(attr), cfg);
+
+out:
+	return err;
+}
+
 extern const struct ufs_pm_lvl_states ufs_pm_lvl_states[];

 int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
--
2.48.1

Re: [PATCH V2 2/3] scsi: ufs: core: Add ufshcd_dme_rmw to modify DME attributes

[Bart Van Assche]

On 7/7/25 2:02 PM, Nitin Rawat wrote:
> +/**
> + * ufshcd_dme_rmw - get modify set a dme attribute
> + * @hba - per adapter instance
> + * @mask - mask to apply on read value
> + * @val - actual value to write
> + * @attr - dme attribute
> + */
> +static inline int ufshcd_dme_rmw(struct ufs_hba *hba, u32 mask,
> +				 u32 val, u32 attr)
> +{
> +	u32 cfg = 0;
> +	int err = 0;

I don't think that it is necessary to zero-initialize 'err' because the
next statement overwrites the value of 'err'.

> +
> +	err = ufshcd_dme_get(hba, UIC_ARG_MIB(attr), &cfg);
> +	if (err)
> +		goto out;
> +
> +	cfg &= ~mask;
> +	cfg |= (val & mask);
> +
> +	err = ufshcd_dme_set(hba, UIC_ARG_MIB(attr), cfg);
> +
> +out:
> +	return err;
> +}
Since this code is not performance-critical, please move the function
definition into source file ufshcd.c.

Thanks,

Bart.

Re: [PATCH V2 2/3] scsi: ufs: core: Add ufshcd_dme_rmw to modify DME attributes

[Manivannan Sadhasivam]

On Tue, Jul 08, 2025 at 02:32:59AM GMT, Nitin Rawat wrote:
> Introduce `ufshcd_dme_rmw` API to read, modify, and write DME
> attributes in UFS host controllers using a mask and value.
> 
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> ---
>  include/ufs/ufshcd.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 9b3515cee711..fe4bb248484c 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -1498,6 +1498,32 @@ static inline int ufshcd_vops_phy_initialization(struct ufs_hba *hba)
>  	return 0;
>  }
> 
> +/**
> + * ufshcd_dme_rmw - get modify set a dme attribute

s/dme/DME

Maybe name the function as, 'ufshcd_dme_update()'?

> + * @hba - per adapter instance
> + * @mask - mask to apply on read value
> + * @val - actual value to write
> + * @attr - dme attribute
> + */
> +static inline int ufshcd_dme_rmw(struct ufs_hba *hba, u32 mask,
> +				 u32 val, u32 attr)

Please move the definition to ufshcd.c

> +{
> +	u32 cfg = 0;
> +	int err = 0;

No need to initialize these.

> +
> +	err = ufshcd_dme_get(hba, UIC_ARG_MIB(attr), &cfg);
> +	if (err)
> +		goto out;

Just do, 'return err'.

> +
> +	cfg &= ~mask;
> +	cfg |= (val & mask);
> +
> +	err = ufshcd_dme_set(hba, UIC_ARG_MIB(attr), cfg);

	return ufshcd_dme_set();

- Mani

-- 
மணிவண்ணன் சதாசிவம்

[PATCH V2 3/3] scsi: ufs: qcom: Enable QUnipro Internal Clock Gating

[Nitin Rawat]

Enable internal clock gating for QUnipro by setting the following
attributes to 1 during host controller initialization:
- DL_VS_CLK_CFG
- PA_VS_CLK_CFG_REG
- DME_VS_CORE_CLK_CTRL.DME_HW_CGC_EN

This change is necessary to support the internal clock gating mechanism
in Qualcomm UFS host controller.

Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 21 +++++++++++++++++++++
 drivers/ufs/host/ufs-qcom.h |  9 +++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index dfdc52333a96..25b5f83b049c 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -558,11 +558,32 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
  */
 static void ufs_qcom_enable_hw_clk_gating(struct ufs_hba *hba)
 {
+	int err = 0;
+
+	/* Enable UTP internal clock gating */
 	ufshcd_rmwl(hba, REG_UFS_CFG2_CGC_EN_ALL, REG_UFS_CFG2_CGC_EN_ALL,
 		    REG_UFS_CFG2);

 	/* Ensure that HW clock gating is enabled before next operations */
 	ufshcd_readl(hba, REG_UFS_CFG2);
+
+	/* Enable Unipro internal clock gating */
+	err = ufshcd_dme_rmw(hba, DL_VS_CLK_CFG_MASK,
+			     DL_VS_CLK_CFG_MASK, DL_VS_CLK_CFG);
+	if (err)
+		goto out;
+
+	err = ufshcd_dme_rmw(hba, PA_VS_CLK_CFG_REG_MASK,
+			     PA_VS_CLK_CFG_REG_MASK, PA_VS_CLK_CFG_REG);
+	if (err)
+		goto out;
+
+	err = ufshcd_dme_rmw(hba, DME_VS_CORE_CLK_CTRL_DME_HW_CGC_EN,
+			     DME_VS_CORE_CLK_CTRL_DME_HW_CGC_EN,
+			     DME_VS_CORE_CLK_CTRL);
+out:
+	if (err)
+		dev_err(hba->dev, "hw clk gating enabled failed\n");
 }

 static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba,
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index 0a5cfc2dd4f7..e0e129af7c16 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -24,6 +24,15 @@

 #define UFS_QCOM_LIMIT_HS_RATE		PA_HS_MODE_B

+/* bit and mask definitions for PA_VS_CLK_CFG_REG attribute */
+#define PA_VS_CLK_CFG_REG      0x9004
+#define PA_VS_CLK_CFG_REG_MASK GENMASK(8, 0)
+
+/* bit and mask definitions for DL_VS_CLK_CFG attribute */
+#define DL_VS_CLK_CFG          0xA00B
+#define DL_VS_CLK_CFG_MASK GENMASK(9, 0)
+#define DME_VS_CORE_CLK_CTRL_DME_HW_CGC_EN             BIT(9)
+
 /* QCOM UFS host controller vendor specific registers */
 enum {
 	REG_UFS_SYS1CLK_1US                 = 0xC0,
--
2.48.1

Re: [PATCH V2 3/3] scsi: ufs: qcom: Enable QUnipro Internal Clock Gating

[Manivannan Sadhasivam]

On Tue, Jul 08, 2025 at 02:33:00AM GMT, Nitin Rawat wrote:
> Enable internal clock gating for QUnipro by setting the following
> attributes to 1 during host controller initialization:
> - DL_VS_CLK_CFG
> - PA_VS_CLK_CFG_REG
> - DME_VS_CORE_CLK_CTRL.DME_HW_CGC_EN
> 
> This change is necessary to support the internal clock gating mechanism
> in Qualcomm UFS host controller.
> 
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 21 +++++++++++++++++++++
>  drivers/ufs/host/ufs-qcom.h |  9 +++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index dfdc52333a96..25b5f83b049c 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -558,11 +558,32 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
>   */
>  static void ufs_qcom_enable_hw_clk_gating(struct ufs_hba *hba)
>  {
> +	int err = 0;
> +

No need to init err.

> +	/* Enable UTP internal clock gating */
>  	ufshcd_rmwl(hba, REG_UFS_CFG2_CGC_EN_ALL, REG_UFS_CFG2_CGC_EN_ALL,
>  		    REG_UFS_CFG2);
> 
>  	/* Ensure that HW clock gating is enabled before next operations */
>  	ufshcd_readl(hba, REG_UFS_CFG2);
> +
> +	/* Enable Unipro internal clock gating */
> +	err = ufshcd_dme_rmw(hba, DL_VS_CLK_CFG_MASK,
> +			     DL_VS_CLK_CFG_MASK, DL_VS_CLK_CFG);
> +	if (err)
> +		goto out;
> +
> +	err = ufshcd_dme_rmw(hba, PA_VS_CLK_CFG_REG_MASK,
> +			     PA_VS_CLK_CFG_REG_MASK, PA_VS_CLK_CFG_REG);
> +	if (err)
> +		goto out;
> +
> +	err = ufshcd_dme_rmw(hba, DME_VS_CORE_CLK_CTRL_DME_HW_CGC_EN,
> +			     DME_VS_CORE_CLK_CTRL_DME_HW_CGC_EN,
> +			     DME_VS_CORE_CLK_CTRL);
> +out:
> +	if (err)
> +		dev_err(hba->dev, "hw clk gating enabled failed\n");

So the error is not a hard fault and you want the driver to continue? If so, it
should be justified in commit message.

- Mani

-- 
மணிவண்ணன் சதாசிவம்