diff mbox series

[v2,3/3] ufs: qcom: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP for SM8550 SoC

Message ID 20240815-ufs-bug-fix-v2-3-b373afae888f@linaro.org (mailing list archive)
State Superseded
Headers show
Series ufs: qcom: Fix probe failure on SM8550 SoC due to broken LSDBS field | expand

Commit Message

Manivannan Sadhasivam via B4 Relay Aug. 15, 2024, 5:16 a.m. UTC 
From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

SM8550 SoC supports the UFSHCI 3.0 spec, but it reports a bogus value of
1 in the reserved 'Legacy Queue & Single Doorbell Support (LSDBS)' field of
the Controller Capabilities register. This field is supposed to read 0 as
per the spec.

But starting with commit 0c60eb0cc320 ("scsi: ufs: core: Check LSDBS cap
when !mcq"), ufshcd driver is now relying on the LSDBS field to decide when
to use the legacy doorbell mode if MCQ is not supported. And this ends up
breaking UFS on SM8550:

ufshcd-qcom 1d84000.ufs: ufshcd_init: failed to initialize (legacy doorbell mode not supported)
ufshcd-qcom 1d84000.ufs: error -EINVAL: Initialization failed with error -22

So use the UFSHCD_QUIRK_BROKEN_LSDBS_CAP quirk for SM8550 SoC so that the
ufshcd driver could use legacy doorbell mode correctly.

Fixes: 0c60eb0cc320 ("scsi: ufs: core: Check LSDBS cap when !mcq")
Tested-by: Amit Pundir <amit.pundir@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/ufs/host/ufs-qcom.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Bart Van Assche Aug. 15, 2024, 6:01 p.m. UTC | #1 
On 8/14/24 10:16 PM, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> SM8550 SoC supports the UFSHCI 3.0 spec, but it reports a bogus value of
> 1 in the reserved 'Legacy Queue & Single Doorbell Support (LSDBS)' field of
> the Controller Capabilities register. This field is supposed to read 0 as
> per the spec.
> 
> But starting with commit 0c60eb0cc320 ("scsi: ufs: core: Check LSDBS cap
> when !mcq"), ufshcd driver is now relying on the LSDBS field to decide when
> to use the legacy doorbell mode if MCQ is not supported. And this ends up
> breaking UFS on SM8550:
> 
> ufshcd-qcom 1d84000.ufs: ufshcd_init: failed to initialize (legacy doorbell mode not supported)
> ufshcd-qcom 1d84000.ufs: error -EINVAL: Initialization failed with error -22
> 
> So use the UFSHCD_QUIRK_BROKEN_LSDBS_CAP quirk for SM8550 SoC so that the
> ufshcd driver could use legacy doorbell mode correctly.
> 
> Fixes: 0c60eb0cc320 ("scsi: ufs: core: Check LSDBS cap when !mcq")

Since this patch depends on the previous two patches, the previous two
patches probably need a "Cc: stable" tag. Otherwise the stable
maintainers will have a hard time figuring out which patches this patch
depends on.

Since this patch by itself looks good to me:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Manivannan Sadhasivam Aug. 16, 2024, 5:28 a.m. UTC | #2 
On Thu, Aug 15, 2024 at 11:01:47AM -0700, Bart Van Assche wrote:
> On 8/14/24 10:16 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > 
> > SM8550 SoC supports the UFSHCI 3.0 spec, but it reports a bogus value of
> > 1 in the reserved 'Legacy Queue & Single Doorbell Support (LSDBS)' field of
> > the Controller Capabilities register. This field is supposed to read 0 as
> > per the spec.
> > 
> > But starting with commit 0c60eb0cc320 ("scsi: ufs: core: Check LSDBS cap
> > when !mcq"), ufshcd driver is now relying on the LSDBS field to decide when
> > to use the legacy doorbell mode if MCQ is not supported. And this ends up
> > breaking UFS on SM8550:
> > 
> > ufshcd-qcom 1d84000.ufs: ufshcd_init: failed to initialize (legacy doorbell mode not supported)
> > ufshcd-qcom 1d84000.ufs: error -EINVAL: Initialization failed with error -22
> > 
> > So use the UFSHCD_QUIRK_BROKEN_LSDBS_CAP quirk for SM8550 SoC so that the
> > ufshcd driver could use legacy doorbell mode correctly.
> > 
> > Fixes: 0c60eb0cc320 ("scsi: ufs: core: Check LSDBS cap when !mcq")
> 
> Since this patch depends on the previous two patches, the previous two
> patches probably need a "Cc: stable" tag. Otherwise the stable
> maintainers will have a hard time figuring out which patches this patch
> depends on.
> 

Well, I have not CCed stable list for this patch intentionally as the offending
commit got merged in v6.11-rc2. So there is no need of backport. Once this
series gets merged into one of the v6.11-rcS, all will be good.

> Since this patch by itself looks good to me:
> 
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
> 

Thanks!

- Mani
diff mbox series

Patch

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 810e637047d0..c87fdc849c62 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -857,6 +857,9 @@  static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
 
 	if (host->hw_ver.major > 0x3)
 		hba->quirks |= UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
+
+	if (of_device_is_compatible(hba->dev->of_node, "qcom,sm8550-ufshc"))
+		hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
 }
 
 static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
@@ -1847,7 +1850,8 @@  static void ufs_qcom_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id ufs_qcom_of_match[] __maybe_unused = {
-	{ .compatible = "qcom,ufshc"},
+	{ .compatible = "qcom,ufshc" },
+	{ .compatible = "qcom,sm8550-ufshc" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, ufs_qcom_of_match);