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

[Jaehoon Chung <jh80.chung@samsung.com>]

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.

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.

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
>>
>>
> 
>