diff mbox series

[2/3] ufs: core: Add a quirk for handling broken SDBS field in controller capabilities register

Message ID 20240814-ufs-bug-fix-v1-2-5eb49d5f7571@linaro.org
State New
Headers show
Series ufs: qcom: Fix probe failure on SM8550 SoC due to broken SDBS field | expand

Commit Message

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

'Legacy Queue & Single Doorbell Support (SDBS)' field in the controller
capabilities register is supposed to be reserved for UFSHCI 3.0 based
controllers and should read as 0. But some controllers may report bogus
value of 1 due to the hardware bug. So let's add a quirk to handle those
controllers.

If the quirk is enabled by the controller driver and MCQ is not supported,
then 'hba->sdbs_sup' field will be ignored and the SCSI device will be
added in legacy/single doorbell mode.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/ufs/core/ufshcd.c | 5 +++--
 include/ufs/ufshcd.h      | 7 +++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Manivannan Sadhasivam Aug. 15, 2024, 3:46 a.m. UTC | #1
On Wed, Aug 14, 2024 at 10:29:11AM -0700, Bart Van Assche wrote:
> On 8/14/24 10:15 AM, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > 
> > 'Legacy Queue & Single Doorbell Support (SDBS)' field in the controller
> > capabilities register is supposed to be reserved for UFSHCI 3.0 based
> > controllers and should read as 0. But some controllers may report bogus
> > value of 1 due to the hardware bug. So let's add a quirk to handle those
> > controllers.
> > 
> > If the quirk is enabled by the controller driver and MCQ is not supported,
> > then 'hba->sdbs_sup' field will be ignored and the SCSI device will be
> > added in legacy/single doorbell mode.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >   drivers/ufs/core/ufshcd.c | 5 +++--
> >   include/ufs/ufshcd.h      | 7 +++++++
> >   2 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index 168b9dbc3ada..acb6f261ecc2 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -10512,8 +10512,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
> >   	}
> >   	if (!is_mcq_supported(hba)) {
> > -		if (!hba->sdbs_sup) {
> > -			dev_err(hba->dev, "%s: failed to initialize (legacy doorbell mode not supported)\n",
> > +		if (!(hba->quirks & UFSHCD_QUIRK_BROKEN_SDBS_CAP) && !hba->sdbs_sup) {
> > +			dev_err(hba->dev,
> > +				"%s: failed to initialize (legacy doorbell mode not supported)\n",
> >   				__func__);
> >   			err = -EINVAL;
> >   			goto out_disable;
> 
> Adding a check for the UFSHCD_QUIRK_BROKEN_SDBS_CAP quirk everywhere
> hba->sdbs_sup is tested is wrong. Please move this check to the code
> that assigns a value to hba->sdbs_sup.
> 

I thought it wouldn't matter since it is used in just one place, but I agree it
is best to keep it in assignment itself.

- Mani
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 168b9dbc3ada..acb6f261ecc2 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10512,8 +10512,9 @@  int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	}
 
 	if (!is_mcq_supported(hba)) {
-		if (!hba->sdbs_sup) {
-			dev_err(hba->dev, "%s: failed to initialize (legacy doorbell mode not supported)\n",
+		if (!(hba->quirks & UFSHCD_QUIRK_BROKEN_SDBS_CAP) && !hba->sdbs_sup) {
+			dev_err(hba->dev,
+				"%s: failed to initialize (legacy doorbell mode not supported)\n",
 				__func__);
 			err = -EINVAL;
 			goto out_disable;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index d44b19cf9f82..85c6ea28d45d 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -676,6 +676,13 @@  enum ufshcd_quirks {
 	 * the standard best practice for managing keys).
 	 */
 	UFSHCD_QUIRK_KEYS_IN_PRDT			= 1 << 24,
+
+	/*
+	 * This quirk needs to be enabled if the host controller has the broken
+	 * Legacy Queue & Single Doorbell Support (SDBS) field in Controller
+	 * Capabilities register.
+	 */
+	UFSHCD_QUIRK_BROKEN_SDBS_CAP			= 1 << 25,
 };
 
 enum ufshcd_caps {