Re: [PATCH V6 1/6] scsi: ufs: qcom: Align mask for core_clk_1us_cycles

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/1/2023 8:47 PM, Bjorn Andersson wrote:
On Fri, Sep 01, 2023 at 05:13:31PM +0530, Nitin Rawat wrote:
Align core_clk_1us_cycles mask for Qualcomm UFS Controller V4.0.0

"Align clk mask for ... as per hardware specification."? Are you trying
to say "The DME_VS_CORE_CLK_CTRL register has changed in v4 of the
Qualcomm UFS controller, introduce support for the new register layout"?

You're not aligning the code to match the hardware specification, you're
fixing the code because the register has changed.

onwards as per Hardware Specification.


Hi Bjorn,

Yes, will update the commit text in next patchset



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

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index f88febb23123..fe36003faaa8 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1297,22 +1297,30 @@ 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 cycles_in_1us)

This is a nice clarification, but changing the function prototype gives
a sense that you changed the parameters - and that's not the case.

So if you drop this rename, you make the purpose of the patch clearer.

  {
-	int err;
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
  	u32 core_clk_ctrl_reg;
+	int ret;

Renaming err to ret is unrelated and only unnecessary complexity to the
patch.


ok..Would take care of this comment in next patchset.



-	if (clk_cycles > DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK)
-		return -EINVAL;
-
-	err = ufshcd_dme_get(hba,
+	ret = ufshcd_dme_get(hba,
  			    UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL),
  			    &core_clk_ctrl_reg);
-	if (err)
-		return err;
+	if (ret)
+		return ret;

-	core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK;
-	core_clk_ctrl_reg |= clk_cycles;
+	/* Bit mask is different for UFS host controller V4.0.0 onwards */
+	if (host->hw_ver.major >= 4) {
+		if (!FIELD_FIT(CLK_1US_CYCLES_MASK_V4, cycles_in_1us))
+			return -ERANGE;
+		core_clk_ctrl_reg &= ~CLK_1US_CYCLES_MASK_V4;
+		core_clk_ctrl_reg |= FIELD_PREP(CLK_1US_CYCLES_MASK_V4, cycles_in_1us);
+	} else {
+		if (!FIELD_FIT(CLK_1US_CYCLES_MASK, cycles_in_1us))
+			return -ERANGE;
+		core_clk_ctrl_reg &= ~CLK_1US_CYCLES_MASK;
+		core_clk_ctrl_reg |= FIELD_PREP(CLK_1US_CYCLES_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..8a9d3dbec297 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -129,8 +129,9 @@ 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 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
+#define CLK_1US_CYCLES_MASK_V4				GENMASK(27, 16)
+#define CLK_1US_CYCLES_MASK				GENMASK(7, 0)
+#define DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT	BIT(8)

Hard to say without applying the patch, please double check that the
values here have matching indentation level.

Thank you,
Bjorn

-Yes I applied and confirmed it's proper.

Thanks,
Nitin




  static inline void
  ufs_qcom_get_controller_revision(struct ufs_hba *hba,
--
2.17.1




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux