Re: [RFC PATCH] mmc: dw_mmc: add status check before clock update

[Andrzej Hajda <a.hajda@samsung.com>]

On 02/11/2015 09:32 AM, Jaehoon Chung wrote:
> On 02/11/2015 04:51 PM, Andrzej Hajda wrote:
>> Hi,
>>
>> Thanks for comments.
>>
>> On 02/10/2015 03:54 PM, Alim Akhtar wrote:
>>> Hi Andrzej,
>>>
>>> On Tue, Feb 10, 2015 at 7:59 PM, Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>> According to specs for version 250A, status register should be
>>>> tested before clock update. Otherwise in case MMC card is missing
>>>> mci_send_cmd timeouts and subsequent CTYPE registry write causes system hang.
>>>> This behavior has been observed on Exynos5422/Odroid-XU3.
>>>>
>>> A similar patch was posted recently[1], did you check that?
>>> [1] http://www.spinics.net/lists/linux-doc/msg28092.html
>> No, thanks for pointing it. I will look at it closer.
>>
>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>> ---
>>>> Hi,
>>>>
>>>> This version corrects usleep to usleep_range function call.
>>>>
>>>> Regards
>>>> Andrzej
>>>> ---
>>>>  drivers/mmc/host/dw_mmc.c | 26 ++++++++++++++++++++++++--
>>>>  drivers/mmc/host/dw_mmc.h |  1 +
>>>>  2 files changed, 25 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 67c0451..6619c8a 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -878,6 +878,25 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>>>>                 cmd, arg, cmd_status);
>>>>  }
>>>>
>>>> +static bool dw_mci_wait_busy(struct dw_mci *host)
>>>> +{
>>>> +       unsigned long timeout;
>>>> +
>>>> +       if (host->verid < DW_MMC_250A)
>>>> +               return true;
>>>> +
>>> I wonder this might be true for 240A as well.
>> Odroid-U3 board with Exynos4412 and MMC 240A does not have this problem.
>> On the other side busy check does not hurt it anyway.
> Which kernel version do you use? 
> I also have the exynos5422 board, but i didn't find the below error yet.
> It doesn't relate with IP version.
> If you share your environment, i can check with exynos5422 board.

linux-next on odroid-xu3. With MMC card removed, booting from sdcard.
Please also note that broken-cd quirk is on in dts.

>
> I'm not sure but this patch could be dropped.
> Because this patch is just only checking whether card is busy or not.
>
> this patch(mmc:dw_mmc: fix bug that case 'timeout sending command') can cover your patch.

I will test it.

Regards
Andrzej
>
> Best Regards,
> Jaehoon Chung
>
>> Relevant part of dmesg for Exynos4412/mmc_240a:
>> ...
>> [    2.193967] mmc1: req done (CMD55): -110: 00000000 00000000 00000000
>> 00000000
>> [    2.194000] mmc1: clock 400000Hz busmode 1 powermode 2 cs 0 Vdd 7
>> width 0 timing 0
>> [    2.194010] mmc1: starting CMD1 arg 00000000 flags 000000e1
>> [    2.194821] mmc1: req done (CMD1): -110: 00000000 00000000 00000000
>> 00000000
>> [    2.194854] mmc1: clock 0Hz busmode 2 powermode 0 cs 0 Vdd 0 width 0
>> timing 0
>> [    3.195404] mmc1: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 7 width 0
>> timing 0
>> ...
>> and for Exynos5422/mmc_250a:
>> [    3.530672] mmc0: req done (CMD55): -5: 00000000 00000000 00000000
>> 00000000
>> [    3.530707] mmc0: clock 400000Hz busmode 1 powermode 2 cs 0 Vdd 21
>> width 0 timing 0
>> [    3.530716] mmc0: starting CMD1 arg 00000000 flags 000000e1
>> [    3.531004] mmc0: req done (CMD1): -5: 00000000 00000000 00000000
>> 00000000
>> [    3.531039] mmc0: clock 0Hz busmode 2 powermode 0 cs 0 Vdd 0 width 0
>> timing 0
>> [    4.031304] mmc_host mmc0: Busy timeout (cmd 0x202000 arg 0x0)
>> [    5.031323] mmc0: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 21 width 0
>> timing 0
>> [    5.536385] mmc_host mmc0: Busy timeout (cmd 0x202000 arg 0x0)
>>
>>>> +       timeout = jiffies + msecs_to_jiffies(500);
>>>> +       while (time_before(jiffies, timeout)) {
>>>> +               if (!(mci_readl(host, STATUS) & SDMMC_STATUS_BUSY))
>>>> +                       return true;
>>>> +
>>>> +               usleep_range(1000, 2000);
>>>> +       }
>>>> +       dev_err(host->dev, "Busy timeout\n");
>>> Probably you need a controller reset here to bring back controller is
>>> original state.
>> OK.
>>
>> Regards
>> Andrzej
>>
>>
>>>> +
>>>> +       return false;
>>>> +}
>>>> +
>>>>  static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>>>  {
>>>>         struct dw_mci *host = slot->host;
>>>> @@ -891,8 +910,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>>>                 sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
>>>>
>>>>         if (!clock) {
>>>> -               mci_writel(host, CLKENA, 0);
>>>> -               mci_send_cmd(slot, sdmmc_cmd_bits, 0);
>>>> +               if (dw_mci_wait_busy(host)) {
>>>> +                       mci_writel(host, CLKENA, 0);
>>>> +                       mci_send_cmd(slot, sdmmc_cmd_bits, 0);
>>>> +               } else
>>>> +                       return;
>>>>         } else if (clock != host->current_speed || force_clkinit) {
>>>>                 div = host->bus_hz / clock;
>>>>                 if (host->bus_hz % clock && host->bus_hz > clock)
>>>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>>>> index 0d0f7a27..ea6d4d1 100644
>>>> --- a/drivers/mmc/host/dw_mmc.h
>>>> +++ b/drivers/mmc/host/dw_mmc.h
>>>> @@ -15,6 +15,7 @@
>>>>  #define _DW_MMC_H_
>>>>
>>>>  #define DW_MMC_240A            0x240a
>>>> +#define DW_MMC_250A            0x250a
>>>>
>>>>  #define SDMMC_CTRL             0x000
>>>>  #define SDMMC_PWREN            0x004
>>>> --
>>>> 1.9.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>