[PATCH] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[PATCH] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Addy Ke]

Because of some uncertain factors, such as worse card or worse hardware,
DAT[3:0](the data lines) may be pulled down by card, and mmc controller
will be in busy state. This should not happend when mmc controller
send command to update card clocks. If this happends, mci_send_cmd will
be failed and we will get 'Timeout sending command', and then system will
be blocked. To avoid this, we need reset mmc controller.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
 drivers/mmc/host/dw_mmc.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4d2e3c2..b1d6dfb 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -100,6 +100,7 @@ struct idmac_desc {
 };
 #endif /* CONFIG_MMC_DW_IDMAC */
 
+static int dw_mci_card_busy(struct mmc_host *mmc);
 static bool dw_mci_reset(struct dw_mci *host);
 static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
 
@@ -888,6 +889,26 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
 		cmd, arg, cmd_status);
 }
 
+static void dw_mci_wait_busy(struct dw_mci_slot *slot)
+{
+	struct dw_mci *host = slot->host;
+	unsigned long timeout = jiffies + msecs_to_jiffies(500);
+
+	while (time_before(jiffies, timeout)) {
+		if (!dw_mci_card_busy(slot->mmc))
+			return;
+	}
+	dev_err(host->dev, "Data busy (status %#x)\n",
+		mci_readl(slot->host, STATUS));
+
+	/*
+	 * Data busy, this should not happend when mmc controller send command
+	 * to update card clocks in non-volt-switch state. If it happends, we
+	 * should reset controller to avoid getting "Timeout sending command".
+	 */
+	dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
+}
+
 static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 {
 	struct dw_mci *host = slot->host;
@@ -899,6 +920,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 	/* We must continue to set bit 28 in CMD until the change is complete */
 	if (host->state == STATE_WAITING_CMD11_DONE)
 		sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
+	else
+		dw_mci_wait_busy(slot);
 
 	if (!clock) {
 		mci_writel(host, CLKENA, 0);
-- 
1.8.3.2

[PATCH] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Ulf Hansson]

On 5 February 2015 at 12:13, Addy Ke <addy.ke@rock-chips.com> wrote:
>
> Because of some uncertain factors, such as worse card or worse hardware,
> DAT[3:0](the data lines) may be pulled down by card, and mmc controller
> will be in busy state. This should not happend when mmc controller
> send command to update card clocks. If this happends, mci_send_cmd will
> be failed and we will get 'Timeout sending command', and then system will
> be blocked. To avoid this, we need reset mmc controller.
>
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>


Hi Addy,

Should I consider $subject patch as a better option to the one below?

[PATCH] mmc: dw_mmc: rockchip: Add DW_MCI_QUIRK_RETRY_DELAY
https://lkml.org/lkml/2015/1/13/562

Kind regards
Uffe


> ---
>  drivers/mmc/host/dw_mmc.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4d2e3c2..b1d6dfb 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -100,6 +100,7 @@ struct idmac_desc {
>  };
>  #endif /* CONFIG_MMC_DW_IDMAC */
>
> +static int dw_mci_card_busy(struct mmc_host *mmc);
>  static bool dw_mci_reset(struct dw_mci *host);
>  static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>
> @@ -888,6 +889,26 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>                 cmd, arg, cmd_status);
>  }
>
> +static void dw_mci_wait_busy(struct dw_mci_slot *slot)
> +{
> +       struct dw_mci *host = slot->host;
> +       unsigned long timeout = jiffies + msecs_to_jiffies(500);
> +
> +       while (time_before(jiffies, timeout)) {
> +               if (!dw_mci_card_busy(slot->mmc))
> +                       return;
> +       }
> +       dev_err(host->dev, "Data busy (status %#x)\n",
> +               mci_readl(slot->host, STATUS));
> +
> +       /*
> +        * Data busy, this should not happend when mmc controller send command
> +        * to update card clocks in non-volt-switch state. If it happends, we
> +        * should reset controller to avoid getting "Timeout sending command".
> +        */
> +       dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
> +}
> +
>  static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>  {
>         struct dw_mci *host = slot->host;
> @@ -899,6 +920,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>         /* We must continue to set bit 28 in CMD until the change is complete */
>         if (host->state == STATE_WAITING_CMD11_DONE)
>                 sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
> +       else
> +               dw_mci_wait_busy(slot);
>
>         if (!clock) {
>                 mci_writel(host, CLKENA, 0);
> --
> 1.8.3.2
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Addy]

On 2015.02.09 12:51, Ulf Hansson wrote:
> On 5 February 2015 at 12:13, Addy Ke <addy.ke@rock-chips.com> wrote:
>> Because of some uncertain factors, such as worse card or worse hardware,
>> DAT[3:0](the data lines) may be pulled down by card, and mmc controller
>> will be in busy state. This should not happend when mmc controller
>> send command to update card clocks. If this happends, mci_send_cmd will
>> be failed and we will get 'Timeout sending command', and then system will
>> be blocked. To avoid this, we need reset mmc controller.
>>
>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
>
> Hi Addy,
>
> Should I consider $subject patch as a better option to the one below?
No:
This patch fix the bug, which can be found by script:
     cd /sys/bus/platform/drivers/dwmmc_rockchip
     for i in $(seq 1 10000); do
       echo "========================" $i
       echo ff0c0000.dwmmc > unbind
       sleep .5
       echo ff0c0000.dwmmc > bind
       sleep 2
     done

> [PATCH] mmc: dw_mmc: rockchip: Add DW_MCI_QUIRK_RETRY_DELAY
This patch is for tuning issue: we should delay until card go to idle 
state, when the previous command return error.
> https://lkml.org/lkml/2015/1/13/562
>
> Kind regards
> Uffe
>
>
>> ---
>>   drivers/mmc/host/dw_mmc.c | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 4d2e3c2..b1d6dfb 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -100,6 +100,7 @@ struct idmac_desc {
>>   };
>>   #endif /* CONFIG_MMC_DW_IDMAC */
>>
>> +static int dw_mci_card_busy(struct mmc_host *mmc);
>>   static bool dw_mci_reset(struct dw_mci *host);
>>   static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>>
>> @@ -888,6 +889,26 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>>                  cmd, arg, cmd_status);
>>   }
>>
>> +static void dw_mci_wait_busy(struct dw_mci_slot *slot)
>> +{
>> +       struct dw_mci *host = slot->host;
>> +       unsigned long timeout = jiffies + msecs_to_jiffies(500);
>> +
>> +       while (time_before(jiffies, timeout)) {
>> +               if (!dw_mci_card_busy(slot->mmc))
>> +                       return;
>> +       }
>> +       dev_err(host->dev, "Data busy (status %#x)\n",
>> +               mci_readl(slot->host, STATUS));
>> +
>> +       /*
>> +        * Data busy, this should not happend when mmc controller send command
>> +        * to update card clocks in non-volt-switch state. If it happends, we
>> +        * should reset controller to avoid getting "Timeout sending command".
>> +        */
>> +       dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
>> +}
>> +
>>   static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>   {
>>          struct dw_mci *host = slot->host;
>> @@ -899,6 +920,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>          /* We must continue to set bit 28 in CMD until the change is complete */
>>          if (host->state == STATE_WAITING_CMD11_DONE)
>>                  sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
>> +       else
>> +               dw_mci_wait_busy(slot);
>>
>>          if (!clock) {
>>                  mci_writel(host, CLKENA, 0);
>> --
>> 1.8.3.2
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

[PATCH] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Jaehoon Chung]

On 02/09/2015 03:56 PM, Addy wrote:
> 
> 
> On 2015.02.09 12:51, Ulf Hansson wrote:
>> On 5 February 2015 at 12:13, Addy Ke <addy.ke@rock-chips.com> wrote:
>>> Because of some uncertain factors, such as worse card or worse hardware,
>>> DAT[3:0](the data lines) may be pulled down by card, and mmc controller
>>> will be in busy state. This should not happend when mmc controller
>>> send command to update card clocks. If this happends, mci_send_cmd will
>>> be failed and we will get 'Timeout sending command', and then system will
>>> be blocked. To avoid this, we need reset mmc controller.

I know that it needs to check whether card is busy or not, before clock-off.
This patch seems to related with it. right?

Best Regards,
Jaehoon Chung

>>>
>>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
>>
>> Hi Addy,
>>
>> Should I consider $subject patch as a better option to the one below?
> No:
> This patch fix the bug, which can be found by script:
>     cd /sys/bus/platform/drivers/dwmmc_rockchip
>     for i in $(seq 1 10000); do
>       echo "========================" $i
>       echo ff0c0000.dwmmc > unbind
>       sleep .5
>       echo ff0c0000.dwmmc > bind
>       sleep 2
>     done
> 
>> [PATCH] mmc: dw_mmc: rockchip: Add DW_MCI_QUIRK_RETRY_DELAY
> This patch is for tuning issue: we should delay until card go to idle state, when the previous command return error.
>> https://lkml.org/lkml/2015/1/13/562
>>
>> Kind regards
>> Uffe
>>
>>
>>> ---
>>>   drivers/mmc/host/dw_mmc.c | 23 +++++++++++++++++++++++
>>>   1 file changed, 23 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 4d2e3c2..b1d6dfb 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -100,6 +100,7 @@ struct idmac_desc {
>>>   };
>>>   #endif /* CONFIG_MMC_DW_IDMAC */
>>>
>>> +static int dw_mci_card_busy(struct mmc_host *mmc);
>>>   static bool dw_mci_reset(struct dw_mci *host);
>>>   static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>>>
>>> @@ -888,6 +889,26 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>>>                  cmd, arg, cmd_status);
>>>   }
>>>
>>> +static void dw_mci_wait_busy(struct dw_mci_slot *slot)
>>> +{
>>> +       struct dw_mci *host = slot->host;
>>> +       unsigned long timeout = jiffies + msecs_to_jiffies(500);
>>> +
>>> +       while (time_before(jiffies, timeout)) {
>>> +               if (!dw_mci_card_busy(slot->mmc))
>>> +                       return;
>>> +       }
>>> +       dev_err(host->dev, "Data busy (status %#x)\n",
>>> +               mci_readl(slot->host, STATUS));
>>> +
>>> +       /*
>>> +        * Data busy, this should not happend when mmc controller send command
>>> +        * to update card clocks in non-volt-switch state. If it happends, we
>>> +        * should reset controller to avoid getting "Timeout sending command".
>>> +        */
>>> +       dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
>>> +}
>>> +
>>>   static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>>   {
>>>          struct dw_mci *host = slot->host;
>>> @@ -899,6 +920,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>>          /* We must continue to set bit 28 in CMD until the change is complete */
>>>          if (host->state == STATE_WAITING_CMD11_DONE)
>>>                  sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
>>> +       else
>>> +               dw_mci_wait_busy(slot);
>>>
>>>          if (!clock) {
>>>                  mci_writel(host, CLKENA, 0);
>>> -- 
>>> 1.8.3.2
>>>
>>>
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo at vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> 
> 
> 

[PATCH v2 0/2] about data busy

[Addy Ke]

Addy Ke (2):
  mmc: dw_mmc: fix bug that cause 'Timeout sending command'
  mmc: dw_mmc: Don't start command while data busy

 drivers/mmc/host/dw_mmc.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

-- 
Changes in v2:
- add new patch to handle data busy when start command
- add cpu_relaxed, suggested by Daniel Kurtz <djkurtz@chromium.org>
1.8.3.2

[PATCH v2 1/2] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Addy Ke]

Because of some uncertain factors, such as worse card or worse hardware,
DAT[3:0](the data lines) may be pulled down by card, and mmc controller
will be in busy state. This should not happend when mmc controller
send command to update card clocks. If this happends, mci_send_cmd will
be failed and we will get 'Timeout sending command', and then system will
be blocked. To avoid this, we need reset mmc controller.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
 drivers/mmc/host/dw_mmc.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4d2e3c2..b0b57e3 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -100,6 +100,7 @@ struct idmac_desc {
 };
 #endif /* CONFIG_MMC_DW_IDMAC */
 
+static int dw_mci_card_busy(struct mmc_host *mmc);
 static bool dw_mci_reset(struct dw_mci *host);
 static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
 
@@ -888,6 +889,31 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
 		cmd, arg, cmd_status);
 }
 
+static void dw_mci_wait_busy(struct dw_mci_slot *slot)
+{
+	struct dw_mci *host = slot->host;
+	unsigned long timeout = jiffies + msecs_to_jiffies(500);
+
+	do {
+		if (!dw_mci_card_busy(slot->mmc))
+			return;
+		cpu_relax();
+	} while (time_before(jiffies, timeout));
+
+	dev_err(host->dev, "Data busy (status %#x)\n",
+		mci_readl(slot->host, STATUS));
+
+	/*
+	 * Data busy, this should not happend when mmc controller send command
+	 * to update card clocks in non-volt-switch state. If it happends, we
+	 * should reset controller to avoid getting "Timeout sending command".
+	 */
+	dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
+
+	/* Fail to reset controller or still data busy, WARN_ON! */
+	WARN_ON(dw_mci_card_busy(slot->mmc));
+}
+
 static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 {
 	struct dw_mci *host = slot->host;
@@ -899,6 +925,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 	/* We must continue to set bit 28 in CMD until the change is complete */
 	if (host->state == STATE_WAITING_CMD11_DONE)
 		sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
+	else
+		dw_mci_wait_busy(slot);
 
 	if (!clock) {
 		mci_writel(host, CLKENA, 0);
-- 
1.8.3.2

[PATCH v2 2/2] mmc: dw_mmc: Don't start command while data busy

[Addy Ke]

We should wait for data busy here in non-volt-switch state.
This may happend when sdio sends CMD53.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
 drivers/mmc/host/dw_mmc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b0b57e3..b40080d 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1007,6 +1007,13 @@ static void __dw_mci_start_request(struct dw_mci *host,
 		mci_writel(host, BLKSIZ, data->blksz);
 	}
 
+	/*
+	 * We should wait for data busy here in non-volt-switch state.
+	 * This may happend when sdio sends CMD53.
+	 */
+	if (host->state != STATE_WAITING_CMD11_DONE)
+		dw_mci_wait_busy(slot);
+
 	cmdflags = dw_mci_prepare_command(slot->mmc, cmd);
 
 	/* this is the first command, send the initialization clock */
-- 
1.8.3.2

[PATCH] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[addy ke]

On 2015/2/9 15:04, Jaehoon Chung wrote:
> On 02/09/2015 03:56 PM, Addy wrote:
>>
>>
>> On 2015.02.09 12:51, Ulf Hansson wrote:
>>> On 5 February 2015 at 12:13, Addy Ke <addy.ke@rock-chips.com> wrote:
>>>> Because of some uncertain factors, such as worse card or worse hardware,
>>>> DAT[3:0](the data lines) may be pulled down by card, and mmc controller
>>>> will be in busy state. This should not happend when mmc controller
>>>> send command to update card clocks. If this happends, mci_send_cmd will
>>>> be failed and we will get 'Timeout sending command', and then system will
>>>> be blocked. To avoid this, we need reset mmc controller.
> 
> I know that it needs to check whether card is busy or not, before clock-off.
> This patch seems to related with it. right?

Yes, it is.

> 
> Best Regards,
> Jaehoon Chung
> 
>>>>
>>>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
>>>
>>> Hi Addy,
>>>
>>> Should I consider $subject patch as a better option to the one below?
>> No:
>> This patch fix the bug, which can be found by script:
>>     cd /sys/bus/platform/drivers/dwmmc_rockchip
>>     for i in $(seq 1 10000); do
>>       echo "========================" $i
>>       echo ff0c0000.dwmmc > unbind
>>       sleep .5
>>       echo ff0c0000.dwmmc > bind
>>       sleep 2
>>     done
>>
>>> [PATCH] mmc: dw_mmc: rockchip: Add DW_MCI_QUIRK_RETRY_DELAY
>> This patch is for tuning issue: we should delay until card go to idle state, when the previous command return error.
>>> https://lkml.org/lkml/2015/1/13/562
>>>
>>> Kind regards
>>> Uffe
>>>
>>>
>>>> ---
>>>>   drivers/mmc/host/dw_mmc.c | 23 +++++++++++++++++++++++
>>>>   1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 4d2e3c2..b1d6dfb 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -100,6 +100,7 @@ struct idmac_desc {
>>>>   };
>>>>   #endif /* CONFIG_MMC_DW_IDMAC */
>>>>
>>>> +static int dw_mci_card_busy(struct mmc_host *mmc);
>>>>   static bool dw_mci_reset(struct dw_mci *host);
>>>>   static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>>>>
>>>> @@ -888,6 +889,26 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>>>>                  cmd, arg, cmd_status);
>>>>   }
>>>>
>>>> +static void dw_mci_wait_busy(struct dw_mci_slot *slot)
>>>> +{
>>>> +       struct dw_mci *host = slot->host;
>>>> +       unsigned long timeout = jiffies + msecs_to_jiffies(500);
>>>> +
>>>> +       while (time_before(jiffies, timeout)) {
>>>> +               if (!dw_mci_card_busy(slot->mmc))
>>>> +                       return;
>>>> +       }
>>>> +       dev_err(host->dev, "Data busy (status %#x)\n",
>>>> +               mci_readl(slot->host, STATUS));
>>>> +
>>>> +       /*
>>>> +        * Data busy, this should not happend when mmc controller send command
>>>> +        * to update card clocks in non-volt-switch state. If it happends, we
>>>> +        * should reset controller to avoid getting "Timeout sending command".
>>>> +        */
>>>> +       dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
>>>> +}
>>>> +
>>>>   static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>>>   {
>>>>          struct dw_mci *host = slot->host;
>>>> @@ -899,6 +920,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>>>          /* We must continue to set bit 28 in CMD until the change is complete */
>>>>          if (host->state == STATE_WAITING_CMD11_DONE)
>>>>                  sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
>>>> +       else
>>>> +               dw_mci_wait_busy(slot);
>>>>
>>>>          if (!clock) {
>>>>                  mci_writel(host, CLKENA, 0);
>>>> -- 
>>>> 1.8.3.2
>>>>
>>>>
>>>> -- 
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to majordomo at vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>
>>
>>
> 
> 
> 
> 

[PATCH v2 1/2] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Jaehoon Chung]

Hi, Addy.

On 02/09/2015 04:25 PM, Addy Ke wrote:
> Because of some uncertain factors, such as worse card or worse hardware,
> DAT[3:0](the data lines) may be pulled down by card, and mmc controller
> will be in busy state. This should not happend when mmc controller
> send command to update card clocks. If this happends, mci_send_cmd will
> be failed and we will get 'Timeout sending command', and then system will
> be blocked. To avoid this, we need reset mmc controller.
> 
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4d2e3c2..b0b57e3 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -100,6 +100,7 @@ struct idmac_desc {
>  };
>  #endif /* CONFIG_MMC_DW_IDMAC */
>  
> +static int dw_mci_card_busy(struct mmc_host *mmc);
>  static bool dw_mci_reset(struct dw_mci *host);
>  static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>  
> @@ -888,6 +889,31 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>  		cmd, arg, cmd_status);
>  }
>  
> +static void dw_mci_wait_busy(struct dw_mci_slot *slot)
> +{
> +	struct dw_mci *host = slot->host;
> +	unsigned long timeout = jiffies + msecs_to_jiffies(500);
> +
> +	do {
> +		if (!dw_mci_card_busy(slot->mmc))
> +			return;
> +		cpu_relax();
> +	} while (time_before(jiffies, timeout));
> +
> +	dev_err(host->dev, "Data busy (status %#x)\n",
> +		mci_readl(slot->host, STATUS));
> +
> +	/*
> +	 * Data busy, this should not happend when mmc controller send command
> +	 * to update card clocks in non-volt-switch state. If it happends, we
> +	 * should reset controller to avoid getting "Timeout sending command".
> +	 */
> +	dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);

If reset is failed, then dw_mci_ctrl_reset should return "false".

	ret = dw_mci_ctrl_reset();

	WARN_ON(!ret || dw_mci_card_busy(slot->mmc));

Is it right?

In my experiment, if reset is failed or card is busy, eMMC can't work anymore..right?
I think this patch is reasonable to prevent blocking issue.

Best Regards,
Jaehoon Chung


> +
> +	/* Fail to reset controller or still data busy, WARN_ON! */
> +	WARN_ON(dw_mci_card_busy(slot->mmc));
> +}
> +
>  static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>  {
>  	struct dw_mci *host = slot->host;
> @@ -899,6 +925,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>  	/* We must continue to set bit 28 in CMD until the change is complete */
>  	if (host->state == STATE_WAITING_CMD11_DONE)
>  		sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
> +	else
> +		dw_mci_wait_busy(slot);
>  
>  	if (!clock) {
>  		mci_writel(host, CLKENA, 0);
> 

[PATCH v2 1/2] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Alim Akhtar]

Hi Addy,

On Mon, Feb 9, 2015 at 12:55 PM, Addy Ke <addy.ke@rock-chips.com> wrote:
> Because of some uncertain factors, such as worse card or worse hardware,
> DAT[3:0](the data lines) may be pulled down by card, and mmc controller
> will be in busy state. This should not happend when mmc controller
> send command to update card clocks. If this happends, mci_send_cmd will
> be failed and we will get 'Timeout sending command', and then system will
> be blocked. To avoid this, we need reset mmc controller.
>
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4d2e3c2..b0b57e3 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -100,6 +100,7 @@ struct idmac_desc {
>  };
>  #endif /* CONFIG_MMC_DW_IDMAC */
>
> +static int dw_mci_card_busy(struct mmc_host *mmc);
>  static bool dw_mci_reset(struct dw_mci *host);
>  static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>
> @@ -888,6 +889,31 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>                 cmd, arg, cmd_status);
>  }
>
> +static void dw_mci_wait_busy(struct dw_mci_slot *slot)
> +{
> +       struct dw_mci *host = slot->host;
> +       unsigned long timeout = jiffies + msecs_to_jiffies(500);
> +
Why 500 msec?

> +       do {
> +               if (!dw_mci_card_busy(slot->mmc))
> +                       return;
> +               cpu_relax();
> +       } while (time_before(jiffies, timeout));
> +
> +       dev_err(host->dev, "Data busy (status %#x)\n",
> +               mci_readl(slot->host, STATUS));
> +
> +       /*
> +        * Data busy, this should not happend when mmc controller send command
> +        * to update card clocks in non-volt-switch state. If it happends, we
> +        * should reset controller to avoid getting "Timeout sending command".
> +        */
> +       dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
> +
Why you need to reset all blocks? may be CTRL_RESET is good enough here.

> +       /* Fail to reset controller or still data busy, WARN_ON! */
> +       WARN_ON(dw_mci_card_busy(slot->mmc));
> +}
> +
>  static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>  {
>         struct dw_mci *host = slot->host;
> @@ -899,6 +925,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>         /* We must continue to set bit 28 in CMD until the change is complete */
>         if (host->state == STATE_WAITING_CMD11_DONE)
>                 sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
> +       else
> +               dw_mci_wait_busy(slot);
>
hmm...I would suggest you to call dw_mci_wait_busy() from inside
mci_send_cmd(), seems like dw_mmc hangs while sending update clock cmd
in multiple cases.see [1]

[1]: http://permalink.gmane.org/gmane.linux.kernel.mmc/31140

>         if (!clock) {
>                 mci_writel(host, CLKENA, 0);
> --
> 1.8.3.2
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Regards,
Alim

[PATCH v2 1/2] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Addy]

On 2015/02/10 23:22, Alim Akhtar wrote:
> Hi Addy,
>
> On Mon, Feb 9, 2015 at 12:55 PM, Addy Ke <addy.ke@rock-chips.com> wrote:
>> Because of some uncertain factors, such as worse card or worse hardware,
>> DAT[3:0](the data lines) may be pulled down by card, and mmc controller
>> will be in busy state. This should not happend when mmc controller
>> send command to update card clocks. If this happends, mci_send_cmd will
>> be failed and we will get 'Timeout sending command', and then system will
>> be blocked. To avoid this, we need reset mmc controller.
>>
>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
>> ---
>>   drivers/mmc/host/dw_mmc.c | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 4d2e3c2..b0b57e3 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -100,6 +100,7 @@ struct idmac_desc {
>>   };
>>   #endif /* CONFIG_MMC_DW_IDMAC */
>>
>> +static int dw_mci_card_busy(struct mmc_host *mmc);
>>   static bool dw_mci_reset(struct dw_mci *host);
>>   static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>>
>> @@ -888,6 +889,31 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>>                  cmd, arg, cmd_status);
>>   }
>>
>> +static void dw_mci_wait_busy(struct dw_mci_slot *slot)
>> +{
>> +       struct dw_mci *host = slot->host;
>> +       unsigned long timeout = jiffies + msecs_to_jiffies(500);
>> +
> Why 500 msec?
This timeout value is the same as  mci_send_cmd:
static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
{
         struct dw_mci *host = slot->host;
         unsigned long timeout = jiffies + msecs_to_jiffies(500);
         ....
}

I have not clear that which is suitable.
Do you have any suggestion on it?
>
>> +       do {
>> +               if (!dw_mci_card_busy(slot->mmc))
>> +                       return;
>> +               cpu_relax();
>> +       } while (time_before(jiffies, timeout));
>> +
>> +       dev_err(host->dev, "Data busy (status %#x)\n",
>> +               mci_readl(slot->host, STATUS));
>> +
>> +       /*
>> +        * Data busy, this should not happend when mmc controller send command
>> +        * to update card clocks in non-volt-switch state. If it happends, we
>> +        * should reset controller to avoid getting "Timeout sending command".
>> +        */
>> +       dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
>> +
> Why you need to reset all blocks? may be CTRL_RESET is good enough here.
I have tested on rk3288, if only reset ctroller, data busy bit will not 
be cleaned,and we will still get

"Timeout sending command".

>
>> +       /* Fail to reset controller or still data busy, WARN_ON! */
>> +       WARN_ON(dw_mci_card_busy(slot->mmc));
>> +}
>> +
>>   static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>   {
>>          struct dw_mci *host = slot->host;
>> @@ -899,6 +925,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>          /* We must continue to set bit 28 in CMD until the change is complete */
>>          if (host->state == STATE_WAITING_CMD11_DONE)
>>                  sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
>> +       else
>> +               dw_mci_wait_busy(slot);
>>
> hmm...I would suggest you to call dw_mci_wait_busy() from inside
> mci_send_cmd(), seems like dw_mmc hangs while sending update clock cmd
> in multiple cases.see [1]
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel.mmc/31140
I think this patch is more reasonable.
So I will resend patches based on this patch.
thank you!
>
>>          if (!clock) {
>>                  mci_writel(host, CLKENA, 0);
>> --
>> 1.8.3.2
>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>

[PATCH v2 1/2] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Addy]

On 2015/02/09 18:01, Jaehoon Chung wrote:
> Hi, Addy.
>
> On 02/09/2015 04:25 PM, Addy Ke wrote:
>> Because of some uncertain factors, such as worse card or worse hardware,
>> DAT[3:0](the data lines) may be pulled down by card, and mmc controller
>> will be in busy state. This should not happend when mmc controller
>> send command to update card clocks. If this happends, mci_send_cmd will
>> be failed and we will get 'Timeout sending command', and then system will
>> be blocked. To avoid this, we need reset mmc controller.
>>
>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
>> ---
>>   drivers/mmc/host/dw_mmc.c | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 4d2e3c2..b0b57e3 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -100,6 +100,7 @@ struct idmac_desc {
>>   };
>>   #endif /* CONFIG_MMC_DW_IDMAC */
>>   
>> +static int dw_mci_card_busy(struct mmc_host *mmc);
>>   static bool dw_mci_reset(struct dw_mci *host);
>>   static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>>   
>> @@ -888,6 +889,31 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>>   		cmd, arg, cmd_status);
>>   }
>>   
>> +static void dw_mci_wait_busy(struct dw_mci_slot *slot)
>> +{
>> +	struct dw_mci *host = slot->host;
>> +	unsigned long timeout = jiffies + msecs_to_jiffies(500);
>> +
>> +	do {
>> +		if (!dw_mci_card_busy(slot->mmc))
>> +			return;
>> +		cpu_relax();
>> +	} while (time_before(jiffies, timeout));
>> +
>> +	dev_err(host->dev, "Data busy (status %#x)\n",
>> +		mci_readl(slot->host, STATUS));
>> +
>> +	/*
>> +	 * Data busy, this should not happend when mmc controller send command
>> +	 * to update card clocks in non-volt-switch state. If it happends, we
>> +	 * should reset controller to avoid getting "Timeout sending command".
>> +	 */
>> +	dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
> If reset is failed, then dw_mci_ctrl_reset should return "false".
>
> 	ret = dw_mci_ctrl_reset();
>
> 	WARN_ON(!ret || dw_mci_card_busy(slot->mmc));
>
> Is it right?
you are right, and I will update it in my next version patch. thank you.
>
> In my experiment, if reset is failed or card is busy, eMMC can't work anymore..right?
> I think this patch is reasonable to prevent blocking issue.
>
> Best Regards,
> Jaehoon Chung
>
>
>> +
>> +	/* Fail to reset controller or still data busy, WARN_ON! */
>> +	WARN_ON(dw_mci_card_busy(slot->mmc));
>> +}
>> +
>>   static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>   {
>>   	struct dw_mci *host = slot->host;
>> @@ -899,6 +925,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>   	/* We must continue to set bit 28 in CMD until the change is complete */
>>   	if (host->state == STATE_WAITING_CMD11_DONE)
>>   		sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
>> +	else
>> +		dw_mci_wait_busy(slot);
>>   
>>   	if (!clock) {
>>   		mci_writel(host, CLKENA, 0);
>>
>
>
>

[PATCH v3 0/3] about data busy

[Addy Ke]

patch 1: This patch can fix bug that controller is still data busy after
	 reset all blocks. After this patch, I still get data busy in
	 set_ios().

patch 2: This patch fix bug 'Timeout sending command'. After patch1 and
	 patch2, there is no mmc errors after:
	 cd /sys/bus/platform/drivers/dwmmc_rockchip
	 for i in $(seq 1 10000); do
  		echo "========================" $i
  		echo ff0c0000.dwmmc > unbind
  		sleep .5
  		echo ff0c0000.dwmmc > bind
  		sleep 2
	done
 
patch3: This patch fix bug that there is data busy before sdio send CMD53.
        But This patch is necessary for sd and mmc too.

Addy Ke (3):
  mmc: dw_mmc: update clock after host reach a stable voltage
  mmc: dw_mmc: fix bug that cause 'Timeout sending command'
  mmc: dw_mmc: Don't start command while data busy

 drivers/mmc/host/dw_mmc.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

-- 
1.8.3.2

[PATCH v3 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Addy Ke]

As show in mmc_power_up(), in MMC_POWER_UP state, the voltage isn't
stable and we may get 'data busy' which can't be cleaned by resetting
all blocks. So we should not send command to update clock in this state.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
 drivers/mmc/host/dw_mmc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4d2e3c2..3472f9b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1102,7 +1102,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		drv_data->set_ios(slot->host, ios);
 
 	/* Slot specific timing and width adjustment */
-	dw_mci_setup_bus(slot, false);
+	if (ios->power_mode != MMC_POWER_UP)
+		dw_mci_setup_bus(slot, false);
 
 	if (slot->host->state == STATE_WAITING_CMD11_DONE && ios->clock != 0)
 		slot->host->state = STATE_IDLE;
-- 
1.8.3.2

[PATCH v3 2/3] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Addy Ke]

Because of some uncertain factors, such as worse card or worse hardware,
DAT[3:0](the data lines) may be pulled down by card, and mmc controller
will be in busy state. This should not happend when mmc controller
send command to update card clocks. If this happends, mci_send_cmd will
be failed and we will get 'Timeout sending command', and then system will
be blocked. To avoid this, we need reset mmc controller.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
 drivers/mmc/host/dw_mmc.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 3472f9b..f0d9da5 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -100,6 +100,7 @@ struct idmac_desc {
 };
 #endif /* CONFIG_MMC_DW_IDMAC */
 
+static int dw_mci_card_busy(struct mmc_host *mmc);
 static bool dw_mci_reset(struct dw_mci *host);
 static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
 
@@ -888,6 +889,35 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
 		cmd, arg, cmd_status);
 }
 
+static void dw_mci_wait_busy(struct dw_mci_slot *slot)
+{
+	int ret;
+	struct dw_mci *host = slot->host;
+	unsigned long timeout = jiffies + msecs_to_jiffies(500);
+
+	if (host->state == STATE_WAITING_CMD11_DONE)
+		return;
+
+	do {
+		if (!dw_mci_card_busy(slot->mmc))
+			return;
+		usleep_range(50, 100);
+	} while (time_before(jiffies, timeout));
+
+	dev_err(host->dev, "Data busy (status %#x)\n",
+		mci_readl(slot->host, STATUS));
+
+	/*
+	 * Data busy, this should not happend when mmc controller send command
+	 * to update card clocks in non-volt-switch state. If it happends, we
+	 * should reset controller to avoid getting "Timeout sending command".
+	 */
+	ret = dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
+
+	/* Fail to reset controller or still data busy, WARN_ON! */
+	WARN_ON(!ret || dw_mci_card_busy(slot->mmc));
+}
+
 static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 {
 	struct dw_mci *host = slot->host;
@@ -896,6 +926,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 	u32 clk_en_a;
 	u32 sdmmc_cmd_bits = SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT;
 
+	dw_mci_wait_busy(slot);
+
 	/* We must continue to set bit 28 in CMD until the change is complete */
 	if (host->state == STATE_WAITING_CMD11_DONE)
 		sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
-- 
1.8.3.2

[PATCH v3 3/3] mmc: dw_mmc: Don't start command while data busy

[Addy Ke]

We should wait for data busy here in non-volt-switch state.
This may happend when sdio sends CMD53.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
 drivers/mmc/host/dw_mmc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index f0d9da5..23507c9 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1076,6 +1076,12 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	WARN_ON(slot->mrq);
 
 	/*
+	 * We should wait for data busy here in non-volt-switch state.
+	 * This may happend when sdio sends CMD53.
+	 */
+	dw_mci_wait_busy(slot);
+
+	/*
 	 * The check for card presence and queueing of the request must be
 	 * atomic, otherwise the card could be removed in between and the
 	 * request wouldn't fail until another card was inserted.
-- 
1.8.3.2

[PATCH v4 0/3] about data busy

[Addy Ke]

patch 1: This patch can fix bug that controller is still data busy after
	 reset all blocks. After this patch, I still get data busy in
	 set_ios().

patch 2: This patch fix bug 'Timeout sending command'. After patch1 and
	 patch2, there is no mmc errors after:
	 cd /sys/bus/platform/drivers/dwmmc_rockchip
	 for i in $(seq 1 10000); do
  		echo "========================" $i
  		echo ff0c0000.dwmmc > unbind
  		sleep .5
  		echo ff0c0000.dwmmc > bind
  		sleep 2
	done
 
patch3: This patch fix bug that there is data busy before sdio send CMD53.
        But This patch is necessary for sd and mmc too.

Addy Ke (3):
  mmc: dw_mmc: update clock after host reach a stable voltage
  mmc: dw_mmc: fix bug that cause 'Timeout sending command'
  mmc: dw_mmc: Don't start command while data busy

 drivers/mmc/host/dw_mmc.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

-- 
1.8.3.2

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Addy Ke]

As show in mmc_power_up(), in MMC_POWER_UP state, the voltage isn't
stable and we may get 'data busy' which can't be cleaned by resetting
all blocks. So we should not send command to update clock in this state.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
 drivers/mmc/host/dw_mmc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4d2e3c2..3472f9b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1102,7 +1102,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		drv_data->set_ios(slot->host, ios);
 
 	/* Slot specific timing and width adjustment */
-	dw_mci_setup_bus(slot, false);
+	if (ios->power_mode != MMC_POWER_UP)
+		dw_mci_setup_bus(slot, false);
 
 	if (slot->host->state == STATE_WAITING_CMD11_DONE && ios->clock != 0)
 		slot->host->state = STATE_IDLE;
-- 
1.8.3.2

[PATCH v4 2/3] mmc: dw_mmc: fix bug that cause 'Timeout sending command'

[Addy Ke]

Because of some uncertain factors, such as worse card or worse hardware,
DAT[3:0](the data lines) may be pulled down by card, and mmc controller
will be in busy state. This should not happend when mmc controller
send command to update card clocks. If this happends, mci_send_cmd will
be failed and we will get 'Timeout sending command', and then system will
be blocked. To avoid this, we need reset mmc controller.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
Changes in v4:
- Retry to wait and reset all blocks until data unbusy.
  I have a sd card, which need retry 2 times to change to unbusy state.

---
 drivers/mmc/host/dw_mmc.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 3472f9b..ac21863 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -100,6 +100,7 @@ struct idmac_desc {
 };
 #endif /* CONFIG_MMC_DW_IDMAC */
 
+static int dw_mci_card_busy(struct mmc_host *mmc);
 static bool dw_mci_reset(struct dw_mci *host);
 static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
 
@@ -888,6 +889,36 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
 		cmd, arg, cmd_status);
 }
 
+static void dw_mci_wait_busy(struct dw_mci_slot *slot)
+{
+	bool ret = true;
+	struct dw_mci *host = slot->host;
+	unsigned long timeout = jiffies + msecs_to_jiffies(500);
+
+	if (host->state == STATE_WAITING_CMD11_DONE)
+		return;
+
+	do {
+		do {
+			if (!dw_mci_card_busy(slot->mmc))
+				return;
+			usleep_range(50, 100);
+		} while (time_before(jiffies, timeout));
+
+		dev_err(host->dev, "Data busy (status %#x)\n",
+			mci_readl(slot->host, STATUS));
+
+		/*
+		 * Data busy, this should not happend when mmc controller
+		 * send command to update card clocks in non-volt-switch state.
+		 * If it happends, we should reset controller to avoid getting
+		 * "Timeout sending command".
+		 */
+		ret = dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS);
+
+	} while (!ret || dw_mci_card_busy(slot->mmc));
+}
+
 static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 {
 	struct dw_mci *host = slot->host;
@@ -896,6 +927,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 	u32 clk_en_a;
 	u32 sdmmc_cmd_bits = SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT;
 
+	dw_mci_wait_busy(slot);
+
 	/* We must continue to set bit 28 in CMD until the change is complete */
 	if (host->state == STATE_WAITING_CMD11_DONE)
 		sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
-- 
1.8.3.2

[PATCH v4 3/3] mmc: dw_mmc: Don't start command while data busy

[Addy Ke]

We should wait until unbusy before the next request.
But this does't need if the command is CMD13, which can access
SD Status register regardless of data busy.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
Changes in v4:
- CMD13 doesn't need wait until unbusy.

 drivers/mmc/host/dw_mmc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ac21863..692d97a 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1076,6 +1076,10 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	WARN_ON(slot->mrq);
 
+	/* Wait until unbusy if the command isn't CMD13 */
+	if (mrq->cmd->opcode != MMC_SEND_STATUS)
+		dw_mci_wait_busy(slot);
+
 	/*
 	 * The check for card presence and queueing of the request must be
 	 * atomic, otherwise the card could be removed in between and the
-- 
1.8.3.2

[PATCH v4 0/3] about data busy

[Javier Martinez Canillas]

Hello Addy,

On Sat, Feb 14, 2015 at 7:17 AM, Addy Ke <addy.ke@rock-chips.com> wrote:
> patch 1: This patch can fix bug that controller is still data busy after
>          reset all blocks. After this patch, I still get data busy in
>          set_ios().
>
> patch 2: This patch fix bug 'Timeout sending command'. After patch1 and
>          patch2, there is no mmc errors after:
>          cd /sys/bus/platform/drivers/dwmmc_rockchip
>          for i in $(seq 1 10000); do
>                 echo "========================" $i
>                 echo ff0c0000.dwmmc > unbind
>                 sleep .5
>                 echo ff0c0000.dwmmc > bind
>                 sleep 2
>         done
>
> patch3: This patch fix bug that there is data busy before sdio send CMD53.
>         But This patch is necessary for sd and mmc too.
>

I faced the same 'Timeout sending command' error when trying to enable
support for the SDIO wifi chip attached to mmc at 12210000 (mmc1) on an
Exynos5420 Peach Pit Chromebook. On booting the kernel log shows:

mmc_host mmc1: Timeout sending command (cmd 0x202000 arg 0x0 status 0x80202000)

0x202000 == SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT so your patch
#2 dw_mci_setup_bus() avoids the mmc comand to time out. However, it
has a side effect since with your series the uSD that in mmc at 12220000
(mmc2) fails to be detected and the kernel log shows:

[    5.466432] Waiting for root device /dev/mmcblk1p4...
[  240.169436] INFO: task kworker/u16:1:50 blocked for more than 120 seconds.
[  240.174844]       Not tainted
3.19.0-next-20150211-00006-g045d4aba96ce-dirty #476
[  240.182302] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[  240.190109] kworker/u16:1   D c04c2710     0    50      2 0x00000000
[  240.196446] Workqueue: kmmcd mmc_rescan
[  240.200249] [<c04c2710>] (__schedule) from [<c04c2ac0>] (schedule+0x34/0x98)
[  240.207290] [<c04c2ac0>] (schedule) from [<c04c6568>]
(schedule_timeout+0x120/0x16c)
[  240.215009] [<c04c6568>] (schedule_timeout) from [<c04c3584>]
(wait_for_common+0xb0/0x154)
[  240.223251] [<c04c3584>] (wait_for_common) from [<c038a5ac>]
(mmc_wait_for_req+0xa0/0x140)
[  240.231492] [<c038a5ac>] (mmc_wait_for_req) from [<c038a6d4>]
(mmc_wait_for_cmd+0x88/0xa8)
[  240.239735] [<c038a6d4>] (mmc_wait_for_cmd) from [<c03905b0>]
(mmc_go_idle+0x78/0xf8)
[  240.247540] [<c03905b0>] (mmc_go_idle) from [<c038c578>]
(mmc_rescan+0x254/0x300)
[  240.255003] [<c038c578>] (mmc_rescan) from [<c00346e8>]
(process_one_work+0x120/0x324)
[  240.262897] [<c00346e8>] (process_one_work) from [<c0034a58>]
(worker_thread+0x138/0x464)
[  240.271048] [<c0034a58>] (worker_thread) from [<c0039070>]
(kthread+0xd8/0xf4)
[  240.278254] [<c0039070>] (kthread) from [<c000e680>]
(ret_from_fork+0x14/0x34)


By enabling debug I see that the card is detected in dw_mci_get_cd() though.

Alim suggested [0] that dw_mci_wait_busy() should be called in
mci_send_cmd() instead dw_mci_setup_bus() because the controller hangs
when when sending update clock cmd in different cases.

I modified [1] your patch #2 to do what Alim suggested and only with
that patch on top of linux-next I have neither the the "Timeout
sending command" error nor the uSD not getting detected errors. Linux
mounts the rootfs from the uSD and the wifi SDIO device is enumerated
and listed in /sys/bus/sdio/devices/

Does that also solve your issue?

Best regards,
Javier

[0]: https://lkml.org/lkml/2015/2/10/353
[1]: http://paste.debian.net/plain/148794

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Alim Akhtar]

Hi Addy,

On Sat, Feb 14, 2015 at 11:47 AM, Addy Ke <addy.ke@rock-chips.com> wrote:
> As show in mmc_power_up(), in MMC_POWER_UP state, the voltage isn't
> stable and we may get 'data busy' which can't be cleaned by resetting
> all blocks. So we should not send command to update clock in this state.
>
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4d2e3c2..3472f9b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1102,7 +1102,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>                 drv_data->set_ios(slot->host, ios);
>
>         /* Slot specific timing and width adjustment */
> -       dw_mci_setup_bus(slot, false);
> +       if (ios->power_mode != MMC_POWER_UP)
> +               dw_mci_setup_bus(slot, false);
>
This looks a HACK to me.
If stabilizing host voltage regulator is the problem, can you try out
below patch, and see if this resolve your issue?

===========
[PATCH] mmc: dw_mmc: Wait for host voltage regulator to be stable

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
---
 drivers/mmc/host/dw_mmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4d2e3c2..dc10fbb 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1202,6 +1202,9 @@ static int dw_mci_switch_voltage(struct mmc_host
*mmc, struct mmc_ios *ios)
  }
  mci_writel(host, UHS_REG, uhs);

+ /* wait for 5ms so that host voltage regulator is stable */
+ usleep_range(5000, 5500);
+
  return 0;
 }

===============

>         if (slot->host->state == STATE_WAITING_CMD11_DONE && ios->clock != 0)
>                 slot->host->state = STATE_IDLE;
> --
> 1.8.3.2
>
>



-- 
Regards,
Alim

[PATCH v4 0/3] about data busy

[Jaehoon Chung]

On 02/15/2015 08:41 PM, Javier Martinez Canillas wrote:
> Hello Addy,
> 
> On Sat, Feb 14, 2015 at 7:17 AM, Addy Ke <addy.ke@rock-chips.com> wrote:
>> patch 1: This patch can fix bug that controller is still data busy after
>>          reset all blocks. After this patch, I still get data busy in
>>          set_ios().
>>
>> patch 2: This patch fix bug 'Timeout sending command'. After patch1 and
>>          patch2, there is no mmc errors after:
>>          cd /sys/bus/platform/drivers/dwmmc_rockchip
>>          for i in $(seq 1 10000); do
>>                 echo "========================" $i
>>                 echo ff0c0000.dwmmc > unbind
>>                 sleep .5
>>                 echo ff0c0000.dwmmc > bind
>>                 sleep 2
>>         done
>>
>> patch3: This patch fix bug that there is data busy before sdio send CMD53.
>>         But This patch is necessary for sd and mmc too.
>>
> 
> I faced the same 'Timeout sending command' error when trying to enable
> support for the SDIO wifi chip attached to mmc at 12210000 (mmc1) on an
> Exynos5420 Peach Pit Chromebook. On booting the kernel log shows:
> 
> mmc_host mmc1: Timeout sending command (cmd 0x202000 arg 0x0 status 0x80202000)
> 
> 0x202000 == SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT so your patch
> #2 dw_mci_setup_bus() avoids the mmc comand to time out. However, it
> has a side effect since with your series the uSD that in mmc at 12220000
> (mmc2) fails to be detected and the kernel log shows:
> 
> [    5.466432] Waiting for root device /dev/mmcblk1p4...
> [  240.169436] INFO: task kworker/u16:1:50 blocked for more than 120 seconds.
> [  240.174844]       Not tainted
> 3.19.0-next-20150211-00006-g045d4aba96ce-dirty #476
> [  240.182302] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
> disables this message.
> [  240.190109] kworker/u16:1   D c04c2710     0    50      2 0x00000000
> [  240.196446] Workqueue: kmmcd mmc_rescan
> [  240.200249] [<c04c2710>] (__schedule) from [<c04c2ac0>] (schedule+0x34/0x98)
> [  240.207290] [<c04c2ac0>] (schedule) from [<c04c6568>]
> (schedule_timeout+0x120/0x16c)
> [  240.215009] [<c04c6568>] (schedule_timeout) from [<c04c3584>]
> (wait_for_common+0xb0/0x154)
> [  240.223251] [<c04c3584>] (wait_for_common) from [<c038a5ac>]
> (mmc_wait_for_req+0xa0/0x140)
> [  240.231492] [<c038a5ac>] (mmc_wait_for_req) from [<c038a6d4>]
> (mmc_wait_for_cmd+0x88/0xa8)
> [  240.239735] [<c038a6d4>] (mmc_wait_for_cmd) from [<c03905b0>]
> (mmc_go_idle+0x78/0xf8)
> [  240.247540] [<c03905b0>] (mmc_go_idle) from [<c038c578>]
> (mmc_rescan+0x254/0x300)
> [  240.255003] [<c038c578>] (mmc_rescan) from [<c00346e8>]
> (process_one_work+0x120/0x324)
> [  240.262897] [<c00346e8>] (process_one_work) from [<c0034a58>]
> (worker_thread+0x138/0x464)
> [  240.271048] [<c0034a58>] (worker_thread) from [<c0039070>]
> (kthread+0xd8/0xf4)
> [  240.278254] [<c0039070>] (kthread) from [<c000e680>]
> (ret_from_fork+0x14/0x34)
> 
> 
> By enabling debug I see that the card is detected in dw_mci_get_cd() though.
> 
> Alim suggested [0] that dw_mci_wait_busy() should be called in
> mci_send_cmd() instead dw_mci_setup_bus() because the controller hangs
> when when sending update clock cmd in different cases.
> 
> I modified [1] your patch #2 to do what Alim suggested and only with
> that patch on top of linux-next I have neither the the "Timeout
> sending command" error nor the uSD not getting detected errors. Linux
> mounts the rootfs from the uSD and the wifi SDIO device is enumerated
> and listed in /sys/bus/sdio/devices/

it needs to check when clock value only update.
As Javier and Alim are mentioned, if check whether card is busy or not in setup_bus(),
should be processed unnecessary checking.
(According to TRM, before disabling clock, check whether card is busy or not.)
if my thinking is right, chekcing is located more exactly before mci_writel(host, CLKENA, 0).

And i recommend if CLK_GATE is enabled, clkgate_delay sets to the bigger value than 3.
I'm not sure Javier's issue is same thing..I will check more this.

Best Regards,
Jaehoon Chung

> 
> Does that also solve your issue?
> 
> Best regards,
> Javier
> 
> [0]: https://lkml.org/lkml/2015/2/10/353
> [1]: http://paste.debian.net/plain/148794
> 

[PATCH v4 0/3] about data busy

[Javier Martinez Canillas]

Hello Jaehoon,

On Mon, Feb 16, 2015 at 6:48 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 02/15/2015 08:41 PM, Javier Martinez Canillas wrote:
>> I modified [1] your patch #2 to do what Alim suggested and only with
>> that patch on top of linux-next I have neither the the "Timeout
>> sending command" error nor the uSD not getting detected errors. Linux
>> mounts the rootfs from the uSD and the wifi SDIO device is enumerated
>> and listed in /sys/bus/sdio/devices/
>
> it needs to check when clock value only update.
> As Javier and Alim are mentioned, if check whether card is busy or not in setup_bus(),
> should be processed unnecessary checking.
> (According to TRM, before disabling clock, check whether card is busy or not.)
> if my thinking is right, chekcing is located more exactly before mci_writel(host, CLKENA, 0).
>
> And i recommend if CLK_GATE is enabled, clkgate_delay sets to the bigger value than 3.
> I'm not sure Javier's issue is same thing..I will check more this.
>

Thanks for checking, do you have access to a Peach Pit or Pi
Chromebook to reproduce the issue I reported? Please let me know if
you need any help from me.

> Best Regards,
> Jaehoon Chung
>

Best regards,
Javier

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[addy ke]

Hi, Alim

Sorry for late reply.

On 2015/2/16 07:28, Alim Akhtar wrote:
> Hi Addy,
> 
> On Sat, Feb 14, 2015 at 11:47 AM, Addy Ke <addy.ke@rock-chips.com> wrote:
>> As show in mmc_power_up(), in MMC_POWER_UP state, the voltage isn't
>> stable and we may get 'data busy' which can't be cleaned by resetting
>> all blocks. So we should not send command to update clock in this state.
>>
>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 4d2e3c2..3472f9b 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1102,7 +1102,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>                 drv_data->set_ios(slot->host, ios);
>>
>>         /* Slot specific timing and width adjustment */
>> -       dw_mci_setup_bus(slot, false);
>> +       if (ios->power_mode != MMC_POWER_UP)
>> +               dw_mci_setup_bus(slot, false);
>>
> This looks a HACK to me.
> If stabilizing host voltage regulator is the problem, can you try out
> below patch, and see if this resolve your issue?

I have test by:
cd /sys/bus/platform/drivers/dwmmc_rockchip
for i in $(seq 1 10000); do
  echo "========================" $i
  echo ff0c0000.dwmmc > unbind
  sleep .5
  echo ff0c0000.dwmmc > bind
  sleep 2
done

There is no error.
I think this patch can resolve my issue, thank you.

Do you send this patch upstream, or can I put it in my patch list?

> 
> ===========
> [PATCH] mmc: dw_mmc: Wait for host voltage regulator to be stable
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4d2e3c2..dc10fbb 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1202,6 +1202,9 @@ static int dw_mci_switch_voltage(struct mmc_host
> *mmc, struct mmc_ios *ios)
>   }
>   mci_writel(host, UHS_REG, uhs);
> 
> + /* wait for 5ms so that host voltage regulator is stable */
> + usleep_range(5000, 5500);
> +
>   return 0;
>  }
> 
> ===============
> 
>>         if (slot->host->state == STATE_WAITING_CMD11_DONE && ios->clock != 0)
>>                 slot->host->state = STATE_IDLE;
>> --
>> 1.8.3.2
>>
>>
> 
> 
> 

[PATCH v4 0/3] about data busy

[addy ke]

Hi, Javier and Alim

These days are Spring Festival holiday.
Sorry for late reply.

On 2015/2/15 19:41, Javier Martinez Canillas wrote:
> Hello Addy,
> 
> On Sat, Feb 14, 2015 at 7:17 AM, Addy Ke <addy.ke@rock-chips.com> wrote:
>> patch 1: This patch can fix bug that controller is still data busy after
>>          reset all blocks. After this patch, I still get data busy in
>>          set_ios().
>>
>> patch 2: This patch fix bug 'Timeout sending command'. After patch1 and
>>          patch2, there is no mmc errors after:
>>          cd /sys/bus/platform/drivers/dwmmc_rockchip
>>          for i in $(seq 1 10000); do
>>                 echo "========================" $i
>>                 echo ff0c0000.dwmmc > unbind
>>                 sleep .5
>>                 echo ff0c0000.dwmmc > bind
>>                 sleep 2
>>         done
>>
>> patch3: This patch fix bug that there is data busy before sdio send CMD53.
>>         But This patch is necessary for sd and mmc too.
>>
> 
> I faced the same 'Timeout sending command' error when trying to enable
> support for the SDIO wifi chip attached to mmc at 12210000 (mmc1) on an
> Exynos5420 Peach Pit Chromebook. On booting the kernel log shows:
> 
> mmc_host mmc1: Timeout sending command (cmd 0x202000 arg 0x0 status 0x80202000)
> 
> 0x202000 == SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT so your patch
> #2 dw_mci_setup_bus() avoids the mmc comand to time out. However, it
> has a side effect since with your series the uSD that in mmc at 12220000
> (mmc2) fails to be detected and the kernel log shows:
> 
> [    5.466432] Waiting for root device /dev/mmcblk1p4...
> [  240.169436] INFO: task kworker/u16:1:50 blocked for more than 120 seconds.
> [  240.174844]       Not tainted
> 3.19.0-next-20150211-00006-g045d4aba96ce-dirty #476
> [  240.182302] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
> disables this message.
> [  240.190109] kworker/u16:1   D c04c2710     0    50      2 0x00000000
> [  240.196446] Workqueue: kmmcd mmc_rescan
> [  240.200249] [<c04c2710>] (__schedule) from [<c04c2ac0>] (schedule+0x34/0x98)
> [  240.207290] [<c04c2ac0>] (schedule) from [<c04c6568>]
> (schedule_timeout+0x120/0x16c)
> [  240.215009] [<c04c6568>] (schedule_timeout) from [<c04c3584>]
> (wait_for_common+0xb0/0x154)
> [  240.223251] [<c04c3584>] (wait_for_common) from [<c038a5ac>]
> (mmc_wait_for_req+0xa0/0x140)
> [  240.231492] [<c038a5ac>] (mmc_wait_for_req) from [<c038a6d4>]
> (mmc_wait_for_cmd+0x88/0xa8)
> [  240.239735] [<c038a6d4>] (mmc_wait_for_cmd) from [<c03905b0>]
> (mmc_go_idle+0x78/0xf8)
> [  240.247540] [<c03905b0>] (mmc_go_idle) from [<c038c578>]
> (mmc_rescan+0x254/0x300)
> [  240.255003] [<c038c578>] (mmc_rescan) from [<c00346e8>]
> (process_one_work+0x120/0x324)
> [  240.262897] [<c00346e8>] (process_one_work) from [<c0034a58>]
> (worker_thread+0x138/0x464)
> [  240.271048] [<c0034a58>] (worker_thread) from [<c0039070>]
> (kthread+0xd8/0xf4)
> [  240.278254] [<c0039070>] (kthread) from [<c000e680>]
> (ret_from_fork+0x14/0x34)
> 
> 
> By enabling debug I see that the card is detected in dw_mci_get_cd() though.
> 
> Alim suggested [0] that dw_mci_wait_busy() should be called in
> mci_send_cmd() instead dw_mci_setup_bus() because the controller hangs
> when when sending update clock cmd in different cases.
> 
> I modified [1] your patch #2 to do what Alim suggested and only with
> that patch on top of linux-next I have neither the the "Timeout
> sending command" error nor the uSD not getting detected errors. Linux
> mounts the rootfs from the uSD and the wifi SDIO device is enumerated
> and listed in /sys/bus/sdio/devices/
> 
> Does that also solve your issue?

After merge Alim patch,and set re_try 8,
it can pass test by:
cd /sys/bus/platform/drivers/dwmmc_rockchip
for i in $(seq 1 10000); do
  echo "========================" $i
  echo ff0c0000.dwmmc > unbind
  sleep .5
  echo ff0c0000.dwmmc > bind
  sleep 2
done

My card is ADATA UHS-1 card(SDR50).
The maximum retry count is 6.

[ 1146.907596] mmc1: card 59b4 removed
[ 1147.421036] dwmmc_rockchip ff0c0000.dwmmc: Using internal DMA controller.
[ 1147.427827] dwmmc_rockchip ff0c0000.dwmmc: Version ID is 270a
[ 1147.433958] dwmmc_rockchip ff0c0000.dwmmc: DW MMC controller at irq 64, 32 bit host data width, 256 deep fifo
[ 1147.444269] dwmmc_rockchip ff0c0000.dwmmc: Got CD GPIO #221.
[ 1147.450381] dwmmc_rockchip ff0c0000.dwmmc: Got WP GPIO #226.
[ 1147.456046] ff0c0000.dwmmc supply card-external-vcc not found, using dummy regulator
[ 1148.519400] dwmmc_rockchip ff0c0000.dwmmc: Data busy (status 0x206)
[ 1149.019451] dwmmc_rockchip ff0c0000.dwmmc: Data busy (status 0x206)
[ 1149.519382] dwmmc_rockchip ff0c0000.dwmmc: Data busy (status 0x206)
[ 1150.019492] dwmmc_rockchip ff0c0000.dwmmc: Data busy (status 0x206)
[ 1150.519442] dwmmc_rockchip ff0c0000.dwmmc: Data busy (status 0x206)
>>>>>>>>>>>>>>>>>>>>>>>>> if re_try is 5, I still get "Timeout sending command".
[ 1150.525711] mmc_host mmc1: Timeout sending command (cmd 0x202000 arg 0x0 status 0x80202000)
[ 1150.534723] rockchip-iodomain io-domains.25: Setting to 3300000 done

So re_try must bigger than 6, but I don't known which value is reansonable.
Do you have any idear about it?

This is the patch Alim suggests:
+       int re_try = 8; /* just random for now, 1 re-try should be ok */

-       mci_writel(host, CMDARG, arg);
-       wmb();
-       mci_writel(host, CMD, SDMMC_CMD_START | cmd);
+       while(re_try--) {
+               mci_writel(host, CMDARG, arg);
+               wmb();
+               mci_writel(host, CMD, SDMMC_CMD_START | cmd);

-       while (time_before(jiffies, timeout)) {
-               cmd_status = mci_readl(host, CMD);
-               if (!(cmd_status & SDMMC_CMD_START))
-                       return;
+               while (time_before(jiffies, timeout)) {
+                       cmd_status = mci_readl(host, CMD);
+                       if (!(cmd_status & SDMMC_CMD_START))
+                               return;
+               }
+
+               dw_mci_wait_busy(slot);
> 
> Best regards,
> Javier
> 
> [0]: https://lkml.org/lkml/2015/2/10/353
> [1]: http://paste.debian.net/plain/148794
> 
> 
> 

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Doug Anderson]

Alim and Addy,

On Sun, Feb 15, 2015 at 3:28 PM, Alim Akhtar <alim.akhtar@gmail.com> wrote:
> Hi Addy,
>
> On Sat, Feb 14, 2015 at 11:47 AM, Addy Ke <addy.ke@rock-chips.com> wrote:
>> As show in mmc_power_up(), in MMC_POWER_UP state, the voltage isn't
>> stable and we may get 'data busy' which can't be cleaned by resetting
>> all blocks. So we should not send command to update clock in this state.
>>
>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 4d2e3c2..3472f9b 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1102,7 +1102,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>                 drv_data->set_ios(slot->host, ios);
>>
>>         /* Slot specific timing and width adjustment */
>> -       dw_mci_setup_bus(slot, false);
>> +       if (ios->power_mode != MMC_POWER_UP)
>> +               dw_mci_setup_bus(slot, false);
>>
> This looks a HACK to me.
> If stabilizing host voltage regulator is the problem, can you try out
> below patch, and see if this resolve your issue?

Actually, IMHO Alim's patch is more of a hack than Addy's.  There's
already a 10ms delay between "power up" and "power on" in the MMC core
in mmc_power_up() state.  That delay is commented as:

  /*
   * This delay should be sufficient to allow the power supply
   * to reach the minimum voltage.
   */
  mmc_delay(10);

That means that assuming that the voltage is stable in MMC_POWER_UP is
not valid anyway.


Addy's patch certainly needs more comments.  In another context Olof suggested:

/*
 * Skip bus setup while voltage is still stabilizing. Instead,
 * bus setup will be done at MMC_POWER_ON.
 */


...thinking about this more, though: really the voltage should be
stabilized when the regulator call returns (see my comments below).
In actuality the "right" fix might be to just rearrange this function
a little not to turn the clock on _before_ we even try to turn the
voltage on.

I've got that coded up but I'm still testing it...  If you want to try
it too, you can find it at
<https://chromium-review.googlesource.com/251341>.

Note that without my patch I find that I _really_ need Addy's patch to
make sure that the card isn't busy in setup_bus.  With my patch Addy's
code catches the card busy less often.  I'm still trying to see if
there's a way to totally remove the need for his setup_bus and still
trying to grok all the patches flying around...


> ===========
> [PATCH] mmc: dw_mmc: Wait for host voltage regulator to be stable
>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4d2e3c2..dc10fbb 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1202,6 +1202,9 @@ static int dw_mci_switch_voltage(struct mmc_host
> *mmc, struct mmc_ios *ios)
>   }
>   mci_writel(host, UHS_REG, uhs);
>
> + /* wait for 5ms so that host voltage regulator is stable */
> + usleep_range(5000, 5500);
> +

Alim: if you have some other instance where you actually need VQMMC to
stabilize it should probably be done in a different way.  If I
understand correctly it is the regulator core's job to make sure that
voltage is stable before returning.  If you have a gpio-regulator you
may be able to use "startup-delay-us" to specify how long the
regulator takes to come up.  You could also look at
'regulator-enable-ramp-delay'

-Doug

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Russell King - ARM Linux]

On Thu, Feb 19, 2015 at 03:49:46PM -0800, Doug Anderson wrote:
> Alim and Addy,
> 
> On Sun, Feb 15, 2015 at 3:28 PM, Alim Akhtar <alim.akhtar@gmail.com> wrote:
> > Hi Addy,
> >
> > On Sat, Feb 14, 2015 at 11:47 AM, Addy Ke <addy.ke@rock-chips.com> wrote:
> >> As show in mmc_power_up(), in MMC_POWER_UP state, the voltage isn't
> >> stable and we may get 'data busy' which can't be cleaned by resetting
> >> all blocks. So we should not send command to update clock in this state.
> >>
> >> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> >> ---
> >>  drivers/mmc/host/dw_mmc.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> >> index 4d2e3c2..3472f9b 100644
> >> --- a/drivers/mmc/host/dw_mmc.c
> >> +++ b/drivers/mmc/host/dw_mmc.c
> >> @@ -1102,7 +1102,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> >>                 drv_data->set_ios(slot->host, ios);
> >>
> >>         /* Slot specific timing and width adjustment */
> >> -       dw_mci_setup_bus(slot, false);
> >> +       if (ios->power_mode != MMC_POWER_UP)
> >> +               dw_mci_setup_bus(slot, false);
> >>
> > This looks a HACK to me.
> > If stabilizing host voltage regulator is the problem, can you try out
> > below patch, and see if this resolve your issue?
> 
> Actually, IMHO Alim's patch is more of a hack than Addy's.  There's
> already a 10ms delay between "power up" and "power on" in the MMC core
> in mmc_power_up() state.  That delay is commented as:
> 
>   /*
>    * This delay should be sufficient to allow the power supply
>    * to reach the minimum voltage.
>    */
>   mmc_delay(10);
> 
> That means that assuming that the voltage is stable in MMC_POWER_UP is
> not valid anyway.
> 
> 
> Addy's patch certainly needs more comments.  In another context Olof suggested:
> 
> /*
>  * Skip bus setup while voltage is still stabilizing. Instead,
>  * bus setup will be done at MMC_POWER_ON.
>  */
> 
> 
> ...thinking about this more, though: really the voltage should be
> stabilized when the regulator call returns (see my comments below).
> In actuality the "right" fix might be to just rearrange this function
> a little not to turn the clock on _before_ we even try to turn the
> voltage on.

This is exactly why I've been saying that we need to get away from the
POWER_UP vs POWER_ON stuff.  We instead need a _single_ call into
drivers to tell them to apply power and bring the card to a state
where it can be talked to.

That includes waiting for the power to stabilise, and sending the
initial clocks to allow the card to initialise.

In the case of extra GPIOs, host drivers will need to call back into
the MMC core at the point where they have stabilised the power.

The current situation where we split the power-up/power-on sequence
between the host drivers and the core is really a hinderance to getting
a working implementation - it's additional complexity where none is
needed.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH v4 3/3] mmc: dw_mmc: Don't start command while data busy

[Doug Anderson]

Addy,

On Fri, Feb 13, 2015 at 10:17 PM, Addy Ke <addy.ke@rock-chips.com> wrote:
> We should wait until unbusy before the next request.
> But this does't need if the command is CMD13, which can access
> SD Status register regardless of data busy.
>
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> ---
> Changes in v4:
> - CMD13 doesn't need wait until unbusy.
>
>  drivers/mmc/host/dw_mmc.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ac21863..692d97a 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1076,6 +1076,10 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>
>         WARN_ON(slot->mrq);
>
> +       /* Wait until unbusy if the command isn't CMD13 */
> +       if (mrq->cmd->opcode != MMC_SEND_STATUS)
> +               dw_mci_wait_busy(slot);
> +

I think you need to be more general.  You really should be checking
"cmd_flags & SDMMC_CMD_PRV_DAT_WAIT".  That leverages dw_mmc's
knowledge about whether it needs to wait.  Right now you'll be waiting
for CMD52 (SDIO) which I don't think is a good idea.

It also seems like this would be better in dw_mci_start_command() so
that we have the least chance of hitting the case.  The downside is
that you can't sleep there, though...  Hrm.

I updated a version of my take on this at
<https://chromium-review.googlesource.com/#/c/244850/>.  I'll put a
timeout on it soon-ish.  Any extra testing that folks can do would be
appreciated...

-Doug

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Doug Anderson]

Hi,

On Thu, Feb 19, 2015 at 3:49 PM, Doug Anderson <dianders@chromium.org> wrote:
> I've got that coded up but I'm still testing it...  If you want to try
> it too, you can find it at
> <https://chromium-review.googlesource.com/251341>.
>
> Note that without my patch I find that I _really_ need Addy's patch to
> make sure that the card isn't busy in setup_bus.  With my patch Addy's
> code catches the card busy less often.  I'm still trying to see if
> there's a way to totally remove the need for his setup_bus and still
> trying to grok all the patches flying around...

Ah, this might be the magic needed:

https://chromium-review.googlesource.com/251344


I think that together with the previous patch things are happy for me
without any of Addy's patches, though it's the end of my work day and
I haven't given this nearly as much testing as I'd like.

I'll continue testing tomorrow and then post both patches together upstream.

-Doug

[PATCH v4 0/3] about data busy

[Doug Anderson]

Hi,

On Fri, Feb 13, 2015 at 10:17 PM, Addy Ke <addy.ke@rock-chips.com> wrote:
> patch 1: This patch can fix bug that controller is still data busy after
>          reset all blocks. After this patch, I still get data busy in
>          set_ios().
>
> patch 2: This patch fix bug 'Timeout sending command'. After patch1 and
>          patch2, there is no mmc errors after:
>          cd /sys/bus/platform/drivers/dwmmc_rockchip
>          for i in $(seq 1 10000); do
>                 echo "========================" $i
>                 echo ff0c0000.dwmmc > unbind
>                 sleep .5
>                 echo ff0c0000.dwmmc > bind
>                 sleep 2
>         done
>
> patch3: This patch fix bug that there is data busy before sdio send CMD53.
>         But This patch is necessary for sd and mmc too.
>
> Addy Ke (3):
>   mmc: dw_mmc: update clock after host reach a stable voltage
>   mmc: dw_mmc: fix bug that cause 'Timeout sending command'
>   mmc: dw_mmc: Don't start command while data busy
>
>  drivers/mmc/host/dw_mmc.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)

A little hard to follow all the patches flying around (so I'll
probably reply a few different places with the same info), but I
believe that all of Addy's patches (with the exception of the one
intended to fix mmc_test which needs to be spun by him for a bugfix)
can be replaced with:

* mmc: dw_mmc: Don't start commands while busy
  https://patchwork.kernel.org/patch/5858221/

* mmc: dw_mmc: Make sure we only adjust the clock when power is on
  https://patchwork.kernel.org/patch/5858261/

* mmc: dw_mmc: Give a good reset after we give power
  https://patchwork.kernel.org/patch/5858281/


In order to avoid further spreading info out among several patches,
I'd request that you don't respond here but instead respond to my
posted patches.  Thanks!

-Doug

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Doug Anderson]

Hi,

On Thu, Feb 19, 2015 at 5:04 PM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Thu, Feb 19, 2015 at 3:49 PM, Doug Anderson <dianders@chromium.org> wrote:
>> I've got that coded up but I'm still testing it...  If you want to try
>> it too, you can find it at
>> <https://chromium-review.googlesource.com/251341>.
>>
>> Note that without my patch I find that I _really_ need Addy's patch to
>> make sure that the card isn't busy in setup_bus.  With my patch Addy's
>> code catches the card busy less often.  I'm still trying to see if
>> there's a way to totally remove the need for his setup_bus and still
>> trying to grok all the patches flying around...
>
> Ah, this might be the magic needed:
>
> https://chromium-review.googlesource.com/251344
>
>
> I think that together with the previous patch things are happy for me
> without any of Addy's patches, though it's the end of my work day and
> I haven't given this nearly as much testing as I'd like.
>
> I'll continue testing tomorrow and then post both patches together upstream.

OK, posted three patches upstream...

A little hard to follow all the patches flying around (so I'll
probably reply a few different places with the same info), but I
believe that all of Addy's patches (with the exception of the one
intended to fix mmc_test which needs to be spun by him for a bugfix)
can be replaced with:

* mmc: dw_mmc: Don't start commands while busy
  https://patchwork.kernel.org/patch/5858221/

* mmc: dw_mmc: Make sure we only adjust the clock when power is on
  https://patchwork.kernel.org/patch/5858261/

* mmc: dw_mmc: Give a good reset after we give power
  https://patchwork.kernel.org/patch/5858281/


In order to avoid further spreading info out among several patches,
I'd request that you don't respond here but instead respond to my
posted patches.  Thanks!

-Doug

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Alim Akhtar]

Hi Doug,


On Fri, Feb 20, 2015 at 5:19 AM, Doug Anderson <dianders@chromium.org> wrote:
> Alim and Addy,
>
> On Sun, Feb 15, 2015 at 3:28 PM, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>> Hi Addy,
>>
>> On Sat, Feb 14, 2015 at 11:47 AM, Addy Ke <addy.ke@rock-chips.com> wrote:
>>> As show in mmc_power_up(), in MMC_POWER_UP state, the voltage isn't
>>> stable and we may get 'data busy' which can't be cleaned by resetting
>>> all blocks. So we should not send command to update clock in this state.
>>>
>>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
>>> ---
>>>  drivers/mmc/host/dw_mmc.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 4d2e3c2..3472f9b 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -1102,7 +1102,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>>                 drv_data->set_ios(slot->host, ios);
>>>
>>>         /* Slot specific timing and width adjustment */
>>> -       dw_mci_setup_bus(slot, false);
>>> +       if (ios->power_mode != MMC_POWER_UP)
>>> +               dw_mci_setup_bus(slot, false);
>>>
>> This looks a HACK to me.
>> If stabilizing host voltage regulator is the problem, can you try out
>> below patch, and see if this resolve your issue?
>
> Actually, IMHO Alim's patch is more of a hack than Addy's.  There's
> already a 10ms delay between "power up" and "power on" in the MMC core
> in mmc_power_up() state.  That delay is commented as:
>
Well, my suggestion (adding 5ms in switch_volatge) was based on DW_MMC
databook (V2.41a) section "7.4.1.2 Voltage switch Normal Scenario"
step #7 which says:" After the 5ms timer expires, the host voltage
regulator is stable".

PS: I have limited to no access of my mails and workstation until
March 9th, so replies will be slow.

>   /*
>    * This delay should be sufficient to allow the power supply
>    * to reach the minimum voltage.
>    */
>   mmc_delay(10);
>
> That means that assuming that the voltage is stable in MMC_POWER_UP is
> not valid anyway.
>
>
> Addy's patch certainly needs more comments.  In another context Olof suggested:
>
> /*
>  * Skip bus setup while voltage is still stabilizing. Instead,
>  * bus setup will be done at MMC_POWER_ON.
>  */
>
>
> ...thinking about this more, though: really the voltage should be
> stabilized when the regulator call returns (see my comments below).
> In actuality the "right" fix might be to just rearrange this function
> a little not to turn the clock on _before_ we even try to turn the
> voltage on.
>
> I've got that coded up but I'm still testing it...  If you want to try
> it too, you can find it at
> <https://chromium-review.googlesource.com/251341>.
>
> Note that without my patch I find that I _really_ need Addy's patch to
> make sure that the card isn't busy in setup_bus.  With my patch Addy's
> code catches the card busy less often.  I'm still trying to see if
> there's a way to totally remove the need for his setup_bus and still
> trying to grok all the patches flying around...
>
>
>> ===========
>> [PATCH] mmc: dw_mmc: Wait for host voltage regulator to be stable
>>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 4d2e3c2..dc10fbb 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1202,6 +1202,9 @@ static int dw_mci_switch_voltage(struct mmc_host
>> *mmc, struct mmc_ios *ios)
>>   }
>>   mci_writel(host, UHS_REG, uhs);
>>
>> + /* wait for 5ms so that host voltage regulator is stable */
>> + usleep_range(5000, 5500);
>> +
>
> Alim: if you have some other instance where you actually need VQMMC to
> stabilize it should probably be done in a different way.  If I
> understand correctly it is the regulator core's job to make sure that
> voltage is stable before returning.  If you have a gpio-regulator you
> may be able to use "startup-delay-us" to specify how long the
> regulator takes to come up.  You could also look at
> 'regulator-enable-ramp-delay'
>
> -Doug



-- 
Regards,
Alim

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Jaehoon Chung]

Hi,

On 02/25/2015 04:52 PM, Alim Akhtar wrote:
> Hi Doug,
> 
> 
> On Fri, Feb 20, 2015 at 5:19 AM, Doug Anderson <dianders@chromium.org> wrote:
>> Alim and Addy,
>>
>> On Sun, Feb 15, 2015 at 3:28 PM, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>> Hi Addy,
>>>
>>> On Sat, Feb 14, 2015 at 11:47 AM, Addy Ke <addy.ke@rock-chips.com> wrote:
>>>> As show in mmc_power_up(), in MMC_POWER_UP state, the voltage isn't
>>>> stable and we may get 'data busy' which can't be cleaned by resetting
>>>> all blocks. So we should not send command to update clock in this state.
>>>>
>>>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
>>>> ---
>>>>  drivers/mmc/host/dw_mmc.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 4d2e3c2..3472f9b 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -1102,7 +1102,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>>>                 drv_data->set_ios(slot->host, ios);
>>>>
>>>>         /* Slot specific timing and width adjustment */
>>>> -       dw_mci_setup_bus(slot, false);
>>>> +       if (ios->power_mode != MMC_POWER_UP)
>>>> +               dw_mci_setup_bus(slot, false);
>>>>
>>> This looks a HACK to me.
>>> If stabilizing host voltage regulator is the problem, can you try out
>>> below patch, and see if this resolve your issue?
>>
>> Actually, IMHO Alim's patch is more of a hack than Addy's.  There's
>> already a 10ms delay between "power up" and "power on" in the MMC core
>> in mmc_power_up() state.  That delay is commented as:
>>
> Well, my suggestion (adding 5ms in switch_volatge) was based on DW_MMC
> databook (V2.41a) section "7.4.1.2 Voltage switch Normal Scenario"
> step #7 which says:" After the 5ms timer expires, the host voltage
> regulator is stable".

if you want to stable power, How about using SDMMC_CMD_INIT flag?

It waits for 80-clock before sending command.(To stable power)
- You can refer to CMD register description.

Best Regards,
Jaehoon Chung

> 
> PS: I have limited to no access of my mails and workstation until
> March 9th, so replies will be slow.
> 
>>   /*
>>    * This delay should be sufficient to allow the power supply
>>    * to reach the minimum voltage.
>>    */
>>   mmc_delay(10);
>>
>> That means that assuming that the voltage is stable in MMC_POWER_UP is
>> not valid anyway.
>>
>>
>> Addy's patch certainly needs more comments.  In another context Olof suggested:
>>
>> /*
>>  * Skip bus setup while voltage is still stabilizing. Instead,
>>  * bus setup will be done at MMC_POWER_ON.
>>  */
>>
>>
>> ...thinking about this more, though: really the voltage should be
>> stabilized when the regulator call returns (see my comments below).
>> In actuality the "right" fix might be to just rearrange this function
>> a little not to turn the clock on _before_ we even try to turn the
>> voltage on.
>>
>> I've got that coded up but I'm still testing it...  If you want to try
>> it too, you can find it at
>> <https://chromium-review.googlesource.com/251341>.
>>
>> Note that without my patch I find that I _really_ need Addy's patch to
>> make sure that the card isn't busy in setup_bus.  With my patch Addy's
>> code catches the card busy less often.  I'm still trying to see if
>> there's a way to totally remove the need for his setup_bus and still
>> trying to grok all the patches flying around...
>>
>>
>>> ===========
>>> [PATCH] mmc: dw_mmc: Wait for host voltage regulator to be stable
>>>
>>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>>> ---
>>>  drivers/mmc/host/dw_mmc.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 4d2e3c2..dc10fbb 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -1202,6 +1202,9 @@ static int dw_mci_switch_voltage(struct mmc_host
>>> *mmc, struct mmc_ios *ios)
>>>   }
>>>   mci_writel(host, UHS_REG, uhs);
>>>
>>> + /* wait for 5ms so that host voltage regulator is stable */
>>> + usleep_range(5000, 5500);
>>> +
>>
>> Alim: if you have some other instance where you actually need VQMMC to
>> stabilize it should probably be done in a different way.  If I
>> understand correctly it is the regulator core's job to make sure that
>> voltage is stable before returning.  If you have a gpio-regulator you
>> may be able to use "startup-delay-us" to specify how long the
>> regulator takes to come up.  You could also look at
>> 'regulator-enable-ramp-delay'
>>
>> -Doug
> 
> 
> 

[PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Doug Anderson]

Alim,

On Tue, Feb 24, 2015 at 11:52 PM, Alim Akhtar <alim.akhtar@gmail.com> wrote:
>>> This looks a HACK to me.
>>> If stabilizing host voltage regulator is the problem, can you try out
>>> below patch, and see if this resolve your issue?
>>
>> Actually, IMHO Alim's patch is more of a hack than Addy's.  There's
>> already a 10ms delay between "power up" and "power on" in the MMC core
>> in mmc_power_up() state.  That delay is commented as:
>>
> Well, my suggestion (adding 5ms in switch_volatge) was based on DW_MMC
> databook (V2.41a) section "7.4.1.2 Voltage switch Normal Scenario"
> step #7 which says:" After the 5ms timer expires, the host voltage
> regulator is stable".

So all of that should be handled by the core.  Just reading the DW_MMC
databook can be confusing because they don't differentiate between
what's in the SDMMC spec and what's DW_MMC specific.

Check out mmc_set_signal_voltage(), specifically:

* During a signal voltage level switch, the clock must be gated
* for 5 ms according to the SD spec

-Doug