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://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. > >