| Message ID | 20240814-ufs-bug-fix-v1-2-5eb49d5f7571@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ufs: qcom: Fix probe failure on SM8550 SoC due to broken SDBS field | expand |
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. Thanks, Bart.
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 --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 {