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

[Jaehoon Chung]

On 6/22/20 6:26 PM, Jagan Teki wrote:
> On Mon, Jun 22, 2020 at 2:54 PM Jaehoon Chung <jh80.chung at 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.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.
> 
> 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.
Then it's helpful to me.

Best Regards,
Jaehoon Chung

> 
> Jagan.
>