On Wed, Jun 10, 2020 at 5:36 PM Jaehoon Chung <jh80.chung@xxxxxxxxxxx> wrote:
>
> Hi Jagan,
>
> On 6/10/20 8:43 PM, Jagan Teki wrote:
> > SDHCI HISPD bits need to be configured based on desired mmc
> > timings mode and some HISPD quirks.
> >
> > So, handle the HISPD bit based on the mmc computed selected
> > mode(timing parameter) rather than fixed mmc card clock
> > frequency.
> >
> > Linux handle the HISPD similar like this in below commit,
> >
> > commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
> >
> > This eventually fixed the mmc write issue observed in
> > rk3399 sdhci controller.
> >
> > Bug log for refernece,
> > => gpt write mmc 0 $partitions
> > Writing GPT: mmc write failed
> > ** Can't write to device 0 **
> > ** Can't write to device 0 **
> > error!
> >
> > Cc: Robin Murphy <robin.murphy@xxxxxxx>
> > Cc: Kever Yang <kever.yang@xxxxxxxxxxxxxx>
> > Cc: Peng Fan <peng.fan@xxxxxxx>
> > Reviewed-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
> > Tested-by: Marc Zyngier <maz@xxxxxxxxxx> # nanopc-t4
> > Tested-by: Suniel Mahesh <sunil@xxxxxxxxxxxxxxxxxxxx> # roc-rk3399-pc
> > Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> > ---
> > Changes for v3:
> > - use && for quirk check.
> >
> > drivers/mmc/sdhci.c | 23 +++++++++++++++--------
> > 1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > index 92cc8434af..a7db278a0e 100644
> > --- a/drivers/mmc/sdhci.c
> > +++ b/drivers/mmc/sdhci.c
> > @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
> > ctrl &= ~SDHCI_CTRL_4BITBUS;
> > }
> >
> > - if (mmc->clock > 26000000)
> > - ctrl |= SDHCI_CTRL_HISPD;
> > - else
> > - ctrl &= ~SDHCI_CTRL_HISPD;
> > -
> > - if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> > - (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
> > - ctrl &= ~SDHCI_CTRL_HISPD;
> > + if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) &&
> > + !(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) {
>
> Sorry, it needs to check more. Because it's different with kernel code.
> Kernel code is only checking condition about SDHCI_QUIRK_NO_HISPD_BIT.
> but in u-boot, one more checking about SDHCI_QUIRK_BROKEN_HISPD_MODE.
>
> So if it doesn't set to both, it's possible to set SDHCI_CTRL_HISPD as ctrl's flags.
>
> > + if (mmc->selected_mode == MMC_HS ||
> > + mmc->selected_mode == SD_HS ||
> > + mmc->selected_mode == MMC_DDR_52 ||
> > + mmc->selected_mode == MMC_HS_200 ||
> > + mmc->selected_mode == MMC_HS_400 ||
> > + mmc->selected_mode == UHS_SDR25 ||
> > + mmc->selected_mode == UHS_SDR50 ||
> > + mmc->selected_mode == UHS_SDR104 ||
> > + mmc->selected_mode == UHS_DDR50)
> > + ctrl |= SDHCI_CTRL_HISPD;
> > + else
> > + ctrl &= ~SDHCI_CTRL_HISPD;
> > + }
> >
> I think that needs to add at here
> else
> ctrl &= ~SDHCI_CTRL_HISPD;
Okay, Can you look at this logic?
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -567,6 +567,7 @@ static int sdhci_set_ios(struct mmc *mmc)
#endif
u32 ctrl;
struct sdhci_host *host = mmc->priv;
+ bool no_hispd_bit = false;
if (host->ops && host->ops->set_control_reg)
host->ops->set_control_reg(host);
@@ -594,13 +595,16 @@ static int sdhci_set_ios(struct mmc *mmc)
ctrl &= ~SDHCI_CTRL_4BITBUS;
}
- if (mmc->clock > 26000000)
- ctrl |= SDHCI_CTRL_HISPD;
- else
- ctrl &= ~SDHCI_CTRL_HISPD;
-
if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
+ no_hispd_bit = true;
+
+ if (((mmc->selected_mode != MMC_LEGACY) ||
+ (mmc->selected_mode != MMC_HS_52) ||
+ (mmc->selected_mode != UHS_SDR12) ||
+ (mmc->selected_mode != MMC_HS_400_ES)) && !no_hispd_bit)
+ ctrl |= SDHCI_CTRL_HISPD;
+ else
ctrl &= ~SDHCI_CTRL_HISPD;
sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-rockchip
