[PATCH v4] mmc: sdhci: Fix HISPD bit handling

[Jaehoon Chung]

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.yang at rock-chips.com>
>> Cc: Peng Fan <peng.fan at nxp.com>
>> Tested-by: Suniel Mahesh <sunil at amarulasolutions.com> # roc-rk3399-pc
>> Signed-off-by: Jagan Teki <jagan at 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
> 
>