On Mon, Nov 04, 2024 at 03:31:55PM +0800, Shawn Lin wrote: > HCE on Rockchip SoC is different from both of ufshcd_hba_execute_hce() > and UFSHCI_QUIRK_BROKEN_HCE case. It need to do dme_reset and dme_enable > after enabling HCE. So in order not to abuse UFSHCI_QUIRK_BROKEN_HCE, add > a new quirk UFSHCI_QUIRK_DME_RESET_ENABLE_AFTER_HCE, to deal with that > limitation. > > Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> > --- > > Changes in v4: > - fix typo > > Changes in v3: None > Changes in v2: None > > drivers/ufs/core/ufshcd.c | 17 +++++++++++++++++ > include/ufs/ufshcd.h | 6 ++++++ > 2 files changed, 23 insertions(+) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 7cab1031..4084bf9 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -4819,6 +4819,7 @@ static int ufshcd_hba_execute_hce(struct ufs_hba *hba) > { > int retry_outer = 3; > int retry_inner; > + int ret; > > start: > if (ufshcd_is_hba_active(hba)) > @@ -4865,6 +4866,22 @@ static int ufshcd_hba_execute_hce(struct ufs_hba *hba) > /* enable UIC related interrupts */ > ufshcd_enable_intr(hba, UFSHCD_UIC_MASK); > > + /* > + * Do dme_reset and dme_enable if a UFS host controller needs > + * this procedure to actually finish HCE. > + */ > + if (hba->quirks & UFSHCI_QUIRK_DME_RESET_ENABLE_AFTER_HCE) { > + ret = ufshcd_dme_reset(hba); > + if (!ret) { > + ret = ufshcd_dme_enable(hba); > + if (ret) > + dev_err(hba->dev, > + "Failed to do dme_enable after HCE.\n"); Don't you need to return failure for this and below error paths? Probably you need to skip post change notification as well in the case of failure. > + } else { > + dev_err(hba->dev, "Failed to do dme_reset after HCE.\n"); > + } > + } > + > ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); Is it possible for you to carry out dme_reset() and dme_enable() in the post change notifier of the rockchip glue driver? I'm trying to see if we can avoid having the quirk which is only specific to Rockchip. - Mani > > return 0; > diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h > index a95282b..e939af8 100644 > --- a/include/ufs/ufshcd.h > +++ b/include/ufs/ufshcd.h > @@ -685,6 +685,12 @@ enum ufshcd_quirks { > * single doorbell mode. > */ > UFSHCD_QUIRK_BROKEN_LSDBS_CAP = 1 << 25, > + > + /* > + * This quirk needs to be enabled if host controller need to > + * do dme_reset and dme_enable after hce. > + */ > + UFSHCI_QUIRK_DME_RESET_ENABLE_AFTER_HCE = 1 << 26, > }; > > enum ufshcd_caps { > -- > 2.7.4 > -- மணிவண்ணன் சதாசிவம்