[PATCH v4] mmc: sdhci: Fix HISPD bit handling

[Peng Fan]

All:

> Subject: Re: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
> 
> On 6/22/20 6:26 PM, Jagan Teki wrote:
> > On Mon, Jun 22, 2020 at 2:54 PM Jaehoon Chung
> <jh80.chung at samsung.com> wrote:
> >>
> >> Hi Peng,
> >>
> >> On 6/22/20 10:49 AM, Peng Fan wrote:
> >>> Jaehoon,
> >>>
> >>>> Subject: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
> >>>
> >>> Are you fine with this v4?
> >>
> >> Thanks for sharing it. i didn't see patch v4.
> >>
> >>>
> >>> Thanks,
> >>> Peng.
> >>>
> >>>>
> >>>> 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 but no
> >>>> SDHCI_QUIRK_BROKEN_HISPD_MODE,
> >>>>
> >>>> 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: Kever Yang <kever.yang at rock-chips.com>
> >>>> Cc: Peng Fan <peng.fan at nxp.com>
> >>>> Tested-by: Suniel Mahesh <sunil at amarulasolutions.com> #
> >>>> roc-rk3399-pc
> >>>> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> >>>> ---
> >>>> Changes for v4:
> >>>> - update commit message
> >>>> - simplify the logic.
> >>>>
> >>>>  drivers/mmc/sdhci.c | 23 +++++++++++++++++------
> >>>>  1 file changed, 17 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
> >>>> 92cc8434af..6cb702111b 100644
> >>>> --- 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,14
> +595,24
> >>>> @@ 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;
> >>>> +            no_hispd_bit = true;
> >>
> >> No. ctrl variable is set to bit[2] of HOST_CONTROL register.
> >> But Some samsung SoCs are using its bit[2] as other purpose.
> >> So it needs to revert the value with "ctrl &= ~SDHCI_CTRL_HISPD".
> >> Because it's possible to set to 1.
> >>
> >> SDHCI_QUIRK_NO_HISPD_BIT means that it's used other purpose.
> >
> > So, what are you suggesting? could you please elaborate?
> 
> 
> if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
>     (host->quirks & SDHCI_QUIRK_NO_BROKEN_HISPD_MODE)) {
> 	ctrl &= ~SDHCI_CTRL_HISPD;
> 	no_hispd_bit = true;
> }
> 
> Just adding "ctrl &= ~SDHCI_CTRL_HISPD;" into its condition.

I pushed this patch with the upper code added to my CI
with a update in commit log:
https://travis-ci.org/github/MrVan/u-boot/builds/701486010

Please let me know if any objections.

Thanks,
Peng.

> Then it's helpful to me.
> 
> Best Regards,
> Jaehoon Chung
> 
> >
> > Jagan.
> >