diff mbox series

[V4] scsi: ufs: core: Check LSDBS cap when !mcq

Message ID 20240709232520epcms2p8ebdb5c4fccc30a6221390566589bf122@epcms2p8 (mailing list archive)
State Accepted
Headers show
Series [V4] scsi: ufs: core: Check LSDBS cap when !mcq | expand

Commit Message

Kyoungrul Kim July 9, 2024, 11:25 p.m. UTC
if the user sets use_mcq_mode to 0, the host will try to activate the
lsdb mode unconditionally even when the lsdbs of device hci cap is 1. so
it makes timeout cmds and fail to device probing.

To prevent that problem. check the lsdbs cap when mcq is not supported
case.

Signed-off-by: k831.kim <k831.kim@samsung.com>
---
Changes to v1: Fix wrong bit of lsdb support.
Changes to v2: Fix extra space and wrong commit messeage.
Changes to v3: Close missing parenthesis and fix grammatical error.
---
 drivers/ufs/core/ufshcd.c | 16 ++++++++++++++++
 include/ufs/ufshcd.h      |  1 +
 include/ufs/ufshci.h      |  1 +
 3 files changed, 18 insertions(+)

Comments

Bart Van Assche July 12, 2024, 7:32 p.m. UTC | #1
On 7/9/24 4:25 PM, Kyoungrul Kim wrote:
> if the user sets use_mcq_mode to 0, the host will try to activate the
> lsdb mode unconditionally even when the lsdbs of device hci cap is 1. so
> it makes timeout cmds and fail to device probing.
> 
> To prevent that problem. check the lsdbs cap when mcq is not supported
> case.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Martin K. Petersen July 16, 2024, 2:52 a.m. UTC | #2
Kyoungrul,

> if the user sets use_mcq_mode to 0, the host will try to activate the
> lsdb mode unconditionally even when the lsdbs of device hci cap is 1.
> so it makes timeout cmds and fail to device probing.

Applied to 6.11/scsi-staging, thanks!
Martin K. Petersen July 23, 2024, 1:23 a.m. UTC | #3
On Wed, 10 Jul 2024 08:25:20 +0900, Kyoungrul Kim wrote:

> if the user sets use_mcq_mode to 0, the host will try to activate the
> lsdb mode unconditionally even when the lsdbs of device hci cap is 1. so
> it makes timeout cmds and fail to device probing.
> 
> To prevent that problem. check the lsdbs cap when mcq is not supported
> case.
> 
> [...]

Applied to 6.11/scsi-queue, thanks!

[1/1] scsi: ufs: core: Check LSDBS cap when !mcq
      https://git.kernel.org/mkp/scsi/c/0c60eb0cc320
Bjorn Andersson Aug. 16, 2024, 10:50 p.m. UTC | #4
On Wed, Jul 10, 2024 at 08:25:20AM +0900, Kyoungrul Kim wrote:
> if the user sets use_mcq_mode to 0, the host will try to activate the
> lsdb mode unconditionally even when the lsdbs of device hci cap is 1. so
> it makes timeout cmds and fail to device probing.
> 
> To prevent that problem. check the lsdbs cap when mcq is not supported
> case.
> 
> Signed-off-by: k831.kim <k831.kim@samsung.com>
> ---
> Changes to v1: Fix wrong bit of lsdb support.
> Changes to v2: Fix extra space and wrong commit messeage.
> Changes to v3: Close missing parenthesis and fix grammatical error.

This causes the probe of the UFSHCD in Qualcomm SM8550 MTP to fail with
-EINVAL.

[    6.132937] ufshcd-qcom 1d84000.ufs: Adding to iommu group 4
[    6.142509] ufshcd-qcom 1d84000.ufs: freq-table-hz property not specified
[    6.149843] ufshcd-qcom 1d84000.ufs: ufshcd_populate_vreg: Unable to find vccq2-supply regulator, assuming enabled
[    6.209794] ufshcd-qcom 1d84000.ufs: ufshcd_init: failed to initialize (legacy doorbell mode not supported)
[    6.226571] ufshcd-qcom 1d84000.ufs: error -EINVAL: Initialization failed with error -22
[    6.348770] ufshcd-qcom 1d84000.ufs: error -EINVAL: ufshcd_pltfrm_init() failed
[    6.363203] ufshcd-qcom 1d84000.ufs: probe with driver ufshcd-qcom failed with error -22

#regzbot introduced: 0c60eb0cc320
#regzbot title: scsi: ufs: Qualcomm SM8550 MTP UFSHCD probe failing

Regards,
Bjorn
Manivannan Sadhasivam Aug. 17, 2024, 5:55 a.m. UTC | #5
On Fri, Aug 16, 2024 at 03:50:20PM -0700, Bjorn Andersson wrote:
> On Wed, Jul 10, 2024 at 08:25:20AM +0900, Kyoungrul Kim wrote:
> > if the user sets use_mcq_mode to 0, the host will try to activate the
> > lsdb mode unconditionally even when the lsdbs of device hci cap is 1. so
> > it makes timeout cmds and fail to device probing.
> > 
> > To prevent that problem. check the lsdbs cap when mcq is not supported
> > case.
> > 
> > Signed-off-by: k831.kim <k831.kim@samsung.com>
> > ---
> > Changes to v1: Fix wrong bit of lsdb support.
> > Changes to v2: Fix extra space and wrong commit messeage.
> > Changes to v3: Close missing parenthesis and fix grammatical error.
> 
> This causes the probe of the UFSHCD in Qualcomm SM8550 MTP to fail with
> -EINVAL.
> 
> [    6.132937] ufshcd-qcom 1d84000.ufs: Adding to iommu group 4
> [    6.142509] ufshcd-qcom 1d84000.ufs: freq-table-hz property not specified
> [    6.149843] ufshcd-qcom 1d84000.ufs: ufshcd_populate_vreg: Unable to find vccq2-supply regulator, assuming enabled
> [    6.209794] ufshcd-qcom 1d84000.ufs: ufshcd_init: failed to initialize (legacy doorbell mode not supported)
> [    6.226571] ufshcd-qcom 1d84000.ufs: error -EINVAL: Initialization failed with error -22
> [    6.348770] ufshcd-qcom 1d84000.ufs: error -EINVAL: ufshcd_pltfrm_init() failed
> [    6.363203] ufshcd-qcom 1d84000.ufs: probe with driver ufshcd-qcom failed with error -22
> 
> #regzbot introduced: 0c60eb0cc320
> #regzbot title: scsi: ufs: Qualcomm SM8550 MTP UFSHCD probe failing
> 

Fix got merged for v6.11: https://lore.kernel.org/linux-scsi/20240816-ufs-bug-fix-v3-0-e6fe0e18e2a3@linaro.org/

- Mani
Bjorn Andersson Aug. 19, 2024, 2:55 a.m. UTC | #6
On Sat, Aug 17, 2024 at 11:25:08AM +0530, Manivannan Sadhasivam wrote:
> On Fri, Aug 16, 2024 at 03:50:20PM -0700, Bjorn Andersson wrote:
> > On Wed, Jul 10, 2024 at 08:25:20AM +0900, Kyoungrul Kim wrote:
> > > if the user sets use_mcq_mode to 0, the host will try to activate the
> > > lsdb mode unconditionally even when the lsdbs of device hci cap is 1. so
> > > it makes timeout cmds and fail to device probing.
> > > 
> > > To prevent that problem. check the lsdbs cap when mcq is not supported
> > > case.
> > > 
> > > Signed-off-by: k831.kim <k831.kim@samsung.com>
> > > ---
> > > Changes to v1: Fix wrong bit of lsdb support.
> > > Changes to v2: Fix extra space and wrong commit messeage.
> > > Changes to v3: Close missing parenthesis and fix grammatical error.
> > 
> > This causes the probe of the UFSHCD in Qualcomm SM8550 MTP to fail with
> > -EINVAL.
> > 
> > [    6.132937] ufshcd-qcom 1d84000.ufs: Adding to iommu group 4
> > [    6.142509] ufshcd-qcom 1d84000.ufs: freq-table-hz property not specified
> > [    6.149843] ufshcd-qcom 1d84000.ufs: ufshcd_populate_vreg: Unable to find vccq2-supply regulator, assuming enabled
> > [    6.209794] ufshcd-qcom 1d84000.ufs: ufshcd_init: failed to initialize (legacy doorbell mode not supported)
> > [    6.226571] ufshcd-qcom 1d84000.ufs: error -EINVAL: Initialization failed with error -22
> > [    6.348770] ufshcd-qcom 1d84000.ufs: error -EINVAL: ufshcd_pltfrm_init() failed
> > [    6.363203] ufshcd-qcom 1d84000.ufs: probe with driver ufshcd-qcom failed with error -22
> > 
> > #regzbot introduced: 0c60eb0cc320
> > #regzbot title: scsi: ufs: Qualcomm SM8550 MTP UFSHCD probe failing
> > 
> 
> Fix got merged for v6.11: https://lore.kernel.org/linux-scsi/20240816-ufs-bug-fix-v3-0-e6fe0e18e2a3@linaro.org/
> 

This seems to be included in v6.11-rc4, but I see the same issue still.
Perhaps I'm doing something wrong?

Regards,
Bjorn

> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்
Bjorn Andersson Aug. 19, 2024, 3:51 a.m. UTC | #7
On Sun, Aug 18, 2024 at 07:55:27PM -0700, Bjorn Andersson wrote:
> On Sat, Aug 17, 2024 at 11:25:08AM +0530, Manivannan Sadhasivam wrote:
> > On Fri, Aug 16, 2024 at 03:50:20PM -0700, Bjorn Andersson wrote:
> > > On Wed, Jul 10, 2024 at 08:25:20AM +0900, Kyoungrul Kim wrote:
> > > > if the user sets use_mcq_mode to 0, the host will try to activate the
> > > > lsdb mode unconditionally even when the lsdbs of device hci cap is 1. so
> > > > it makes timeout cmds and fail to device probing.
> > > > 
> > > > To prevent that problem. check the lsdbs cap when mcq is not supported
> > > > case.
> > > > 
> > > > Signed-off-by: k831.kim <k831.kim@samsung.com>
> > > > ---
> > > > Changes to v1: Fix wrong bit of lsdb support.
> > > > Changes to v2: Fix extra space and wrong commit messeage.
> > > > Changes to v3: Close missing parenthesis and fix grammatical error.
> > > 
> > > This causes the probe of the UFSHCD in Qualcomm SM8550 MTP to fail with
> > > -EINVAL.
> > > 
> > > [    6.132937] ufshcd-qcom 1d84000.ufs: Adding to iommu group 4
> > > [    6.142509] ufshcd-qcom 1d84000.ufs: freq-table-hz property not specified
> > > [    6.149843] ufshcd-qcom 1d84000.ufs: ufshcd_populate_vreg: Unable to find vccq2-supply regulator, assuming enabled
> > > [    6.209794] ufshcd-qcom 1d84000.ufs: ufshcd_init: failed to initialize (legacy doorbell mode not supported)
> > > [    6.226571] ufshcd-qcom 1d84000.ufs: error -EINVAL: Initialization failed with error -22
> > > [    6.348770] ufshcd-qcom 1d84000.ufs: error -EINVAL: ufshcd_pltfrm_init() failed
> > > [    6.363203] ufshcd-qcom 1d84000.ufs: probe with driver ufshcd-qcom failed with error -22
> > > 
> > > #regzbot introduced: 0c60eb0cc320
> > > #regzbot title: scsi: ufs: Qualcomm SM8550 MTP UFSHCD probe failing
> > > 
> > 
> > Fix got merged for v6.11: https://lore.kernel.org/linux-scsi/20240816-ufs-bug-fix-v3-0-e6fe0e18e2a3@linaro.org/
> > 
> 
> This seems to be included in v6.11-rc4, but I see the same issue still.
> Perhaps I'm doing something wrong?
> 

I'm sorry, I misread, the series hasn't made it into v6.11-rc4.
cherry-picking the fixes solves my problem.

#regzbot fixed: cd06b713a688

Thank you,
Bjorn

> Regards,
> Bjorn
> 
> > - Mani
> > 
> > -- 
> > மணிவண்ணன் சதாசிவம்
>
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 1b65e6ae4137..8587aa592d51 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2412,7 +2412,17 @@  static inline int ufshcd_hba_capabilities(struct ufs_hba *hba)
 		return err;
 	}
 
+	/*
+	 * The UFSHCI 3.0 specification does not define MCQ_SUPPORT and
+	 * LSDB_SUPPORT, but [31:29] as reserved bits with reset value 0s, which
+	 * means we can simply read values regardless to version
+	 */
 	hba->mcq_sup = FIELD_GET(MASK_MCQ_SUPPORT, hba->capabilities);
+	/*
+	 * 0h: legacy single doorbell support is available
+	 * 1h: indicate that legacy single doorbell support has been removed
+	 */
+	hba->lsdb_sup = !FIELD_GET(MASK_LSDB_SUPPORT, hba->capabilities);
 	if (!hba->mcq_sup)
 		return 0;
 
@@ -10449,6 +10459,12 @@  int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	}
 
 	if (!is_mcq_supported(hba)) {
+		if (!hba->lsdb_sup) {
+			dev_err(hba->dev, "%s: failed to initialize (legacy doorbell mode not supported)\n",
+				__func__);
+			err = -EINVAL;
+			goto out_disable;
+		}
 		err = scsi_add_host(host, hba->dev);
 		if (err) {
 			dev_err(hba->dev, "scsi_add_host failed\n");
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index bad88bd91995..fd391f6eee73 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1074,6 +1074,7 @@  struct ufs_hba {
 	bool ext_iid_sup;
 	bool scsi_host_added;
 	bool mcq_sup;
+	bool lsdb_sup;
 	bool mcq_enabled;
 	struct ufshcd_res_info res[RES_MAX];
 	void __iomem *mcq_base;
diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
index 385e1c6b8d60..22ba85e81d8c 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -75,6 +75,7 @@  enum {
 	MASK_OUT_OF_ORDER_DATA_DELIVERY_SUPPORT	= 0x02000000,
 	MASK_UIC_DME_TEST_MODE_SUPPORT		= 0x04000000,
 	MASK_CRYPTO_SUPPORT			= 0x10000000,
+	MASK_LSDB_SUPPORT			= 0x20000000,
 	MASK_MCQ_SUPPORT			= 0x40000000,
 };