On 6/24/20 10:36 AM, Peng Fan wrote: > 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.ch...@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.y...@rock-chips.com> >>>>>> Cc: Peng Fan <peng....@nxp.com> >>>>>> Tested-by: Suniel Mahesh <su...@amarulasolutions.com> # >>>>>> roc-rk3399-pc >>>>>> Signed-off-by: Jagan Teki <ja...@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://protect2.fireeye.com/url?k=a0ad72d4-fd674763-a0acf99b-0cc47a3003e8-b7ff57d36725a01c&q=1&u=https%3A%2F%2Ftravis-ci.org%2Fgithub%2FMrVan%2Fu-boot%2Fbuilds%2F701486010 > > Please let me know if any objections.
Thanks! > > Thanks, > Peng. > >> Then it's helpful to me. >> >> Best Regards, >> Jaehoon Chung >> >>> >>> Jagan. >>> >