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. Then it's helpful to me. Best Regards, Jaehoon Chung > > Jagan. >