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

[Jaehoon Chung]

On 6/9/20 11:38 PM, Robin Murphy wrote:
> On 2020-06-09 15:01, Jagan Teki wrote:
>> 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,
>>
>> 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>
>> Reviewed-by: Jaehoon Chung <jh80.chung at samsung.com>
>> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
>> ---
>> Changes for v2:
>> - collect Jaehoon R-b
>>
>>   drivers/mmc/sdhci.c | 23 +++++++++++++++--------
>>   1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 92cc8434af..280b8c88eb 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -594,14 +594,21 @@ 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;
>> +    if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> 
> Should that be "&&" rather than "||"? Otherwise this will always evaluate to true unless *both* quirks are set, which isn't equivalent to the check being removed above.


You're right.

> 
> Robin.
> 
>> +        !(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) {
>> +        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);
>>  
>