RESEND v2: scsi: ufshcd: use a macro for UFS versions

All of lore.kernel.org
 help / color / mirror / Atom feed

* RESEND v2: scsi: ufshcd: use a macro for UFS versions
@ 2021-03-09 12:02 Caleb Connolly
  2021-03-09 12:02 ` [RESEND PATCH v2 1/3] scsi: ufshcd: use a function to calculate versions Caleb Connolly
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Caleb Connolly @ 2021-03-09 12:02 UTC (permalink / raw)
  To: caleb
  Cc: alim.akhtar, avri.altman, ejb, martin.petersen, stanley.chu, cang,
	beanhuo, jaegeuk, asutoshd, linux-scsi, linux-kernel

When using a device with UFS > 2.1 the error "invalid UFS version" is
misleadingly printed. There was a patch for this almost a year
ago to which this solution was suggested.

This series replaces the use of the growing UFSHCI_VERSION_xy macros with
an inline function to encode a major and minor version into the scheme
used on devices, that being:

        (major << 8) + (minor << 4)

I dealt with the different encoding used for UFS 1.x by converting it
to match the newer versions in ufshcd_get_ufs_version(). That means it's
possible to use comparisons for version checks, e.g.

        if (hba->ufs_version < ufshci_version(3, 0))
                ...

I've also dropped the "invalid UFS version" check entirely as it seems to
be more misleading than useful, and hasn't been accurate for a long time.

This has been tested on a device with UFS 3.0 and a device with UFS 2.1,
however I don't own any older devices to test with.

        Caleb
---
Changes since v1:
 * Switch from macro to static inline function
 * Address Christoph's formatting comments
 * Add Nitin's signoff on patch 3
Resend:
 * Fix patches 1/3 referencing the macro from v1
   instead of the new inline function

Caleb Connolly (3):
      scsi: ufshcd: use a function to calculate versions
      scsi: ufs: qcom: use ufshci_version function
      scsi: ufshcd: remove version check

 drivers/scsi/ufs/ufs-qcom.c |  4 +--
 drivers/scsi/ufs/ufshcd.c   | 66 ++++++++++++++++++---------------------------
 drivers/scsi/ufs/ufshci.h   | 17 +++++++-----
 3 files changed, 38 insertions(+), 49 deletions(-)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RESEND PATCH v2 1/3] scsi: ufshcd: use a function to calculate versions
  2021-03-09 12:02 RESEND v2: scsi: ufshcd: use a macro for UFS versions Caleb Connolly
@ 2021-03-09 12:02 ` Caleb Connolly
  2021-03-09 14:55   ` Christoph Hellwig
  2021-03-09 12:02 ` [RESEND PATCH v2 2/3] scsi: ufs: qcom: use ufshci_version function Caleb Connolly
  2021-03-09 12:03 ` [RESEND PATCH v2 3/3] scsi: ufshcd: remove version check Caleb Connolly
  2 siblings, 1 reply; 5+ messages in thread
From: Caleb Connolly @ 2021-03-09 12:02 UTC (permalink / raw)
  To: caleb, Alim Akhtar, Avri Altman, James E.J. Bottomley,
	Martin K. Petersen
  Cc: ejb, stanley.chu, cang, beanhuo, jaegeuk, asutoshd, linux-scsi,
	linux-kernel, Christoph Hellwig

Update the driver to use a function for referencing the UFS version.
This replaces the UFSHCI_VERSION_xy macros, and supports comparisons
where they did not.

Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Caleb Connolly <caleb@connolly.tech>
---
 drivers/scsi/ufs/ufshcd.c | 64 +++++++++++++++++----------------------
 drivers/scsi/ufs/ufshci.h | 17 ++++++-----
 2 files changed, 37 insertions(+), 44 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 77161750c9fb..f4d4b885d4df 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -669,23 +669,12 @@ int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
  */
 static inline u32 ufshcd_get_intr_mask(struct ufs_hba *hba)
 {
-	u32 intr_mask = 0;
+	if (hba->ufs_version == ufshci_version(1, 0))
+		return INTERRUPT_MASK_ALL_VER_10;
+	if (hba->ufs_version <= ufshci_version(2, 0))
+		return INTERRUPT_MASK_ALL_VER_11;
 
-	switch (hba->ufs_version) {
-	case UFSHCI_VERSION_10:
-		intr_mask = INTERRUPT_MASK_ALL_VER_10;
-		break;
-	case UFSHCI_VERSION_11:
-	case UFSHCI_VERSION_20:
-		intr_mask = INTERRUPT_MASK_ALL_VER_11;
-		break;
-	case UFSHCI_VERSION_21:
-	default:
-		intr_mask = INTERRUPT_MASK_ALL_VER_21;
-		break;
-	}
-
-	return intr_mask;
+	return INTERRUPT_MASK_ALL_VER_21;
 }
 
 /**
@@ -696,10 +685,22 @@ static inline u32 ufshcd_get_intr_mask(struct ufs_hba *hba)
  */
 static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba)
 {
+	u32 ufshci_ver;
+
 	if (hba->quirks & UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION)
-		return ufshcd_vops_get_ufs_hci_version(hba);
+		ufshci_ver = ufshcd_vops_get_ufs_hci_version(hba);
+	else
+		ufshci_ver = ufshcd_readl(hba, REG_UFS_VERSION);
 
-	return ufshcd_readl(hba, REG_UFS_VERSION);
+	/*
+	 * UFSHCI v1.x uses a different version scheme, in order
+	 * to allow the use of comparisons with the ufshci_version
+	 * function, we convert it to the same scheme as ufs 2.0+.
+	 */
+	if (ufshci_ver & 0x00010000)
+		return ufshci_version(1, ufshci_ver & 0x00000100);
+
+	return ufshci_ver;
 }
 
 /**
@@ -931,8 +932,7 @@ static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
 u32 ufshcd_get_local_unipro_ver(struct ufs_hba *hba)
 {
 	/* HCI version 1.0 and 1.1 supports UniPro 1.41 */
-	if ((hba->ufs_version == UFSHCI_VERSION_10) ||
-	    (hba->ufs_version == UFSHCI_VERSION_11))
+	if (hba->ufs_version <= ufshci_version(1, 1))
 		return UFS_UNIPRO_VER_1_41;
 	else
 		return UFS_UNIPRO_VER_1_6;
@@ -2335,7 +2335,7 @@ static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
 {
 	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
 
-	if (hba->ufs_version == UFSHCI_VERSION_10) {
+	if (hba->ufs_version == ufshci_version(1, 0)) {
 		u32 rw;
 		rw = set & INTERRUPT_MASK_RW_VER_10;
 		set = rw | ((set ^ intrs) & intrs);
@@ -2355,7 +2355,7 @@ static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
 {
 	u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
 
-	if (hba->ufs_version == UFSHCI_VERSION_10) {
+	if (hba->ufs_version == ufshci_version(1, 0)) {
 		u32 rw;
 		rw = (set & INTERRUPT_MASK_RW_VER_10) &
 			~(intrs & INTERRUPT_MASK_RW_VER_10);
@@ -2518,8 +2518,7 @@ static int ufshcd_compose_devman_upiu(struct ufs_hba *hba,
 	u8 upiu_flags;
 	int ret = 0;
 
-	if ((hba->ufs_version == UFSHCI_VERSION_10) ||
-	    (hba->ufs_version == UFSHCI_VERSION_11))
+	if (hba->ufs_version <= ufshci_version(1, 1))
 		lrbp->command_type = UTP_CMD_TYPE_DEV_MANAGE;
 	else
 		lrbp->command_type = UTP_CMD_TYPE_UFS_STORAGE;
@@ -2546,8 +2545,7 @@ static int ufshcd_comp_scsi_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 	u8 upiu_flags;
 	int ret = 0;
 
-	if ((hba->ufs_version == UFSHCI_VERSION_10) ||
-	    (hba->ufs_version == UFSHCI_VERSION_11))
+	if (hba->ufs_version <= ufshci_version(1, 1))
 		lrbp->command_type = UTP_CMD_TYPE_SCSI;
 	else
 		lrbp->command_type = UTP_CMD_TYPE_UFS_STORAGE;
@@ -6561,15 +6559,10 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 	ufshcd_prepare_lrbp_crypto(NULL, lrbp);
 	hba->dev_cmd.type = cmd_type;
 
-	switch (hba->ufs_version) {
-	case UFSHCI_VERSION_10:
-	case UFSHCI_VERSION_11:
+	if (hba->ufs_version <= ufshci_version(1, 1))
 		lrbp->command_type = UTP_CMD_TYPE_DEV_MANAGE;
-		break;
-	default:
+	else
 		lrbp->command_type = UTP_CMD_TYPE_UFS_STORAGE;
-		break;
-	}
 
 	/* update the task tag in the request upiu */
 	req_upiu->header.dword_0 |= cpu_to_be32(tag);
@@ -9298,10 +9291,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	/* Get UFS version supported by the controller */
 	hba->ufs_version = ufshcd_get_ufs_version(hba);
 
-	if ((hba->ufs_version != UFSHCI_VERSION_10) &&
-	    (hba->ufs_version != UFSHCI_VERSION_11) &&
-	    (hba->ufs_version != UFSHCI_VERSION_20) &&
-	    (hba->ufs_version != UFSHCI_VERSION_21))
+	if (hba->ufs_version < ufshci_version(1, 0))
 		dev_err(hba->dev, "invalid UFS version 0x%x\n",
 			hba->ufs_version);
 
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 6795e1f0e8f8..e1eb44bacbe4 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -74,13 +74,16 @@ enum {
 #define MINOR_VERSION_NUM_MASK		UFS_MASK(0xFFFF, 0)
 #define MAJOR_VERSION_NUM_MASK		UFS_MASK(0xFFFF, 16)
 
-/* Controller UFSHCI version */
-enum {
-	UFSHCI_VERSION_10 = 0x00010000, /* 1.0 */
-	UFSHCI_VERSION_11 = 0x00010100, /* 1.1 */
-	UFSHCI_VERSION_20 = 0x00000200, /* 2.0 */
-	UFSHCI_VERSION_21 = 0x00000210, /* 2.1 */
-};
+/*
+ * Controller UFSHCI version
+ * - 2.x and newer use the following scheme:
+ *   major << 8 + minor << 4
+ * - 1.x has been converted to match this in
+ *   ufshcd_get_ufs_version()
+ */
+static inline u32 ufshci_version(u32 major, u32 minor) {
+	return (((major) << 8) + ((minor) << 4));
+}
 
 /*
  * HCDDID - Host Controller Identification Descriptor
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RESEND PATCH v2 1/3] scsi: ufshcd: use a function to calculate versions
  2021-03-09 12:02 ` [RESEND PATCH v2 1/3] scsi: ufshcd: use a function to calculate versions Caleb Connolly
@ 2021-03-09 14:55   ` Christoph Hellwig
  0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2021-03-09 14:55 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Alim Akhtar, Avri Altman, James E.J. Bottomley,
	Martin K. Petersen, ejb, stanley.chu, cang, beanhuo, jaegeuk,
	asutoshd, linux-scsi, linux-kernel, Christoph Hellwig

On Tue, Mar 09, 2021 at 12:02:47PM +0000, Caleb Connolly wrote:
> +static inline u32 ufshci_version(u32 major, u32 minor) {
> +	return (((major) << 8) + ((minor) << 4));
> +}

Kernel style puts the opening curly brace on a separate line.  Also
for an inline function adding parentheses is not required (unlike for
macros).

Otherwise this looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RESEND PATCH v2 2/3] scsi: ufs: qcom: use ufshci_version function
  2021-03-09 12:02 RESEND v2: scsi: ufshcd: use a macro for UFS versions Caleb Connolly
  2021-03-09 12:02 ` [RESEND PATCH v2 1/3] scsi: ufshcd: use a function to calculate versions Caleb Connolly
@ 2021-03-09 12:02 ` Caleb Connolly
  2021-03-09 12:03 ` [RESEND PATCH v2 3/3] scsi: ufshcd: remove version check Caleb Connolly
  2 siblings, 0 replies; 5+ messages in thread
From: Caleb Connolly @ 2021-03-09 12:02 UTC (permalink / raw)
  To: caleb, Andy Gross, Bjorn Andersson, Alim Akhtar, Avri Altman,
	James E.J. Bottomley, Martin K. Petersen
  Cc: ejb, stanley.chu, cang, beanhuo, jaegeuk, asutoshd, linux-scsi,
	linux-kernel, linux-arm-msm

Replace the UFSHCI_VERSION_xy macros.

Signed-off-by: Caleb Connolly <caleb@connolly.tech>
---
 drivers/scsi/ufs/ufs-qcom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index f97d7b0ae3b6..2d54dce0eeda 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -809,9 +809,9 @@ static u32 ufs_qcom_get_ufs_hci_version(struct ufs_hba *hba)
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
 
 	if (host->hw_ver.major == 0x1)
-		return UFSHCI_VERSION_11;
+		return ufshci_version(1, 1);
 	else
-		return UFSHCI_VERSION_20;
+		return ufshci_version(2, 0);
 }
 
 /**
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [RESEND PATCH v2 3/3] scsi: ufshcd: remove version check
  2021-03-09 12:02 RESEND v2: scsi: ufshcd: use a macro for UFS versions Caleb Connolly
  2021-03-09 12:02 ` [RESEND PATCH v2 1/3] scsi: ufshcd: use a function to calculate versions Caleb Connolly
  2021-03-09 12:02 ` [RESEND PATCH v2 2/3] scsi: ufs: qcom: use ufshci_version function Caleb Connolly
@ 2021-03-09 12:03 ` Caleb Connolly
  2 siblings, 0 replies; 5+ messages in thread
From: Caleb Connolly @ 2021-03-09 12:03 UTC (permalink / raw)
  To: caleb, Alim Akhtar, Avri Altman, James E.J. Bottomley,
	Martin K. Petersen
  Cc: ejb, stanley.chu, cang, beanhuo, jaegeuk, asutoshd, linux-scsi,
	linux-kernel, Nitin Rawat

This check is redundant as all UFS versions are currently supported.

Signed-off-by: Nitin Rawat <nitirawa@codeaurora.org>
Signed-off-by: Caleb Connolly <caleb@connolly.tech>
---
 drivers/scsi/ufs/ufshcd.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index f4d4b885d4df..a6f317f0dc9b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -9291,10 +9291,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	/* Get UFS version supported by the controller */
 	hba->ufs_version = ufshcd_get_ufs_version(hba);
 
-	if (hba->ufs_version < ufshci_version(1, 0))
-		dev_err(hba->dev, "invalid UFS version 0x%x\n",
-			hba->ufs_version);
-
 	/* Get Interrupt bit mask per version */
 	hba->intr_mask = ufshcd_get_intr_mask(hba);
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-09 14:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-09 12:02 RESEND v2: scsi: ufshcd: use a macro for UFS versions Caleb Connolly
2021-03-09 12:02 ` [RESEND PATCH v2 1/3] scsi: ufshcd: use a function to calculate versions Caleb Connolly
2021-03-09 14:55   ` Christoph Hellwig
2021-03-09 12:02 ` [RESEND PATCH v2 2/3] scsi: ufs: qcom: use ufshci_version function Caleb Connolly
2021-03-09 12:03 ` [RESEND PATCH v2 3/3] scsi: ufshcd: remove version check Caleb Connolly

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.