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?
Jagan.
