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.
Best Regards,
Jaehoon Chung
>> +
>> + if (!no_hispd_bit) {
>> + if (mmc->selected_mode == MMC_HS ||
>> + mmc->selected_mode == SD_HS ||
>> + mmc->selected_mode == MMC_DDR_52 ||
>> + mmc->selected_mode == MMC_HS_200 ||
>> + mmc->selected_mode == MMC_HS_400 ||
>> + mmc->selected_mode == UHS_SDR25 ||
>> + mmc->selected_mode == UHS_SDR50 ||
>> + mmc->selected_mode == UHS_SDR104 ||
>> + mmc->selected_mode == UHS_DDR50)
>> + ctrl |= SDHCI_CTRL_HISPD;
>> + else
>> + ctrl &= ~SDHCI_CTRL_HISPD;
>> + }
>>
>> sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>
>> --
>> 2.25.1
>
>
