| Message ID | 1730705521-23081-2-git-send-email-shawn.lin@rock-chips.com |
|---|---|
| State | New |
| Headers | show |
| Series | Initial support for RK3576 UFS controller | expand |
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@linaro.org> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > --- > > 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 >
Hi Mani, 在 2024/11/7 23:51, Manivannan Sadhasivam 写道: > 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@linaro.org> >> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> >> --- >> >> 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. Oops, my bad. > >> + } 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. > I will check and test this approach. Thanks. > - 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 >> >
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"); + } else { + dev_err(hba->dev, "Failed to do dme_reset after HCE.\n"); + } + } + ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); 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 {
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@linaro.org> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- 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(+)