Patchwork [U-Boot,v2] mmc: sdhci: Avoid commands errors by simple timeout adaptation.

login
register
mail settings
Submitter Przemyslaw Marczak
Date Oct. 8, 2013, 4:12 p.m.
Message ID <1381248729-21470-1-git-send-email-p.marczak@samsung.com>
Download mbox | patch
Permalink /patch/281512/
State Accepted
Delegated to: Pantelis Antoniou
Headers show

Comments

Przemyslaw Marczak - Oct. 8, 2013, 4:12 p.m.
Old command timeout value was too small and it caused I/O errors which
led to uncompleted read/write/erase operations and filesystem errors.
Timeout adaptation fixes this issue.

Changes in sdhci_send_command() function:
- change timeout variable to static
- increase default command timeout to 100 ms
- add definition of max command timeout value,
  which can be redefined in each board config file
- wait for card ready state for max defined time
  if it doesn't exceed defined maximum or return COMM_ERR

Once successfully increased timeout value will be used in next function
call. This fix was tested on Goni, Trats, Trats2 boards by testing UMS
on MMC storage.

Changes v2:
- move global variable cmd_timeout into function sdhci_send_command()
- change condition "==" to ">=" when comparing time with timeout
- print information about timeout increasing and card busy timeout

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/mmc/sdhci.c |   35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)
Przemyslaw Marczak - Oct. 15, 2013, 8:18 p.m.
Hello Pantelis,

Could you look at this patch, please?

On 10/08/2013 06:12 PM, Przemyslaw Marczak wrote:
> Old command timeout value was too small and it caused I/O errors which
> led to uncompleted read/write/erase operations and filesystem errors.
> Timeout adaptation fixes this issue.
>
> Changes in sdhci_send_command() function:
> - change timeout variable to static
> - increase default command timeout to 100 ms
> - add definition of max command timeout value,
>    which can be redefined in each board config file
> - wait for card ready state for max defined time
>    if it doesn't exceed defined maximum or return COMM_ERR
>
> Once successfully increased timeout value will be used in next function
> call. This fix was tested on Goni, Trats, Trats2 boards by testing UMS
> on MMC storage.
>
> Changes v2:
> - move global variable cmd_timeout into function sdhci_send_command()
> - change condition "==" to ">=" when comparing time with timeout
> - print information about timeout increasing and card busy timeout
>
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>

Thank you.
Przemyslaw Marczak - Oct. 29, 2013, 1:02 p.m.
Dear Pantelis,
You don't reply for a long time. Could you look on it at this time?

On 10/15/2013 10:18 PM, Przemyslaw Marczak wrote:
> Hello Pantelis,
>
> Could you look at this patch, please?
>
> On 10/08/2013 06:12 PM, Przemyslaw Marczak wrote:
>> Old command timeout value was too small and it caused I/O errors which
>> led to uncompleted read/write/erase operations and filesystem errors.
>> Timeout adaptation fixes this issue.
>>
>> Changes in sdhci_send_command() function:
>> - change timeout variable to static
>> - increase default command timeout to 100 ms
>> - add definition of max command timeout value,
>>    which can be redefined in each board config file
>> - wait for card ready state for max defined time
>>    if it doesn't exceed defined maximum or return COMM_ERR
>>
>> Once successfully increased timeout value will be used in next function
>> call. This fix was tested on Goni, Trats, Trats2 boards by testing UMS
>> on MMC storage.
>>
>> Changes v2:
>> - move global variable cmd_timeout into function sdhci_send_command()
>> - change condition "==" to ">=" when comparing time with timeout
>> - print information about timeout increasing and card busy timeout
>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>
> Thank you.
>
>

Regards
Pantelis Antoniou - Oct. 29, 2013, 1:08 p.m.
Dear Przemyslaw,

On Oct 29, 2013, at 3:02 PM, Przemyslaw Marczak wrote:

> Dear Pantelis,
> You don't reply for a long time. Could you look on it at this time?
> 

Is this a way to address people on a mailing list?

People, especially volunteers are busy with other things too.

Eventually things get merged, but not when people are being rude.

> On 10/15/2013 10:18 PM, Przemyslaw Marczak wrote:
>> Hello Pantelis,
>> 
>> Could you look at this patch, please?
>> 
>> On 10/08/2013 06:12 PM, Przemyslaw Marczak wrote:
>>> Old command timeout value was too small and it caused I/O errors which
>>> led to uncompleted read/write/erase operations and filesystem errors.
>>> Timeout adaptation fixes this issue.
>>> 
>>> Changes in sdhci_send_command() function:
>>> - change timeout variable to static
>>> - increase default command timeout to 100 ms
>>> - add definition of max command timeout value,
>>>   which can be redefined in each board config file
>>> - wait for card ready state for max defined time
>>>   if it doesn't exceed defined maximum or return COMM_ERR
>>> 
>>> Once successfully increased timeout value will be used in next function
>>> call. This fix was tested on Goni, Trats, Trats2 boards by testing UMS
>>> on MMC storage.
>>> 
>>> Changes v2:
>>> - move global variable cmd_timeout into function sdhci_send_command()
>>> - change condition "==" to ">=" when comparing time with timeout
>>> - print information about timeout increasing and card busy timeout
>>> 
>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>> 
>> Thank you.
>> 
>> 
> 
> Regards
> -- 
> Przemyslaw Marczak
> Samsung R&D Institute Poland
> Samsung Electronics
> p.marczak@samsung.com

Regards

-- Pantelis
Przemyslaw Marczak - Oct. 29, 2013, 1:24 p.m.
Dear Pantelis,

On 10/29/2013 02:08 PM, Pantelis Antoniou wrote:
> Dear Przemyslaw,
>
> On Oct 29, 2013, at 3:02 PM, Przemyslaw Marczak wrote:
>
>> Dear Pantelis,
>> You don't reply for a long time. Could you look on it at this time?
>>
>
> Is this a way to address people on a mailing list?
>
> People, especially volunteers are busy with other things too.
>
> Eventually things get merged, but not when people are being rude.

What did I wrong by contacting mmc-u-boot custodian via the mailing list?

At least for linux it is a good and common practice to "ping" 
maintainers after 2 weeks of not reply to the patch.

>
>> On 10/15/2013 10:18 PM, Przemyslaw Marczak wrote:
>>> Hello Pantelis,
>>>
>>> Could you look at this patch, please?
>>>
>>> On 10/08/2013 06:12 PM, Przemyslaw Marczak wrote:
>>>> Old command timeout value was too small and it caused I/O errors which
>>>> led to uncompleted read/write/erase operations and filesystem errors.
>>>> Timeout adaptation fixes this issue.
>>>>
>>>> Changes in sdhci_send_command() function:
>>>> - change timeout variable to static
>>>> - increase default command timeout to 100 ms
>>>> - add definition of max command timeout value,
>>>>    which can be redefined in each board config file
>>>> - wait for card ready state for max defined time
>>>>    if it doesn't exceed defined maximum or return COMM_ERR
>>>>
>>>> Once successfully increased timeout value will be used in next function
>>>> call. This fix was tested on Goni, Trats, Trats2 boards by testing UMS
>>>> on MMC storage.
>>>>
>>>> Changes v2:
>>>> - move global variable cmd_timeout into function sdhci_send_command()
>>>> - change condition "==" to ">=" when comparing time with timeout
>>>> - print information about timeout increasing and card busy timeout
>>>>
>>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>
>>> Thank you.
>>>
>>>
>>
>> Regards
>> --
>> Przemyslaw Marczak
>> Samsung R&D Institute Poland
>> Samsung Electronics
>> p.marczak@samsung.com
>
> Regards
>
> -- Pantelis
>

Regards
Pantelis Antoniou - Oct. 29, 2013, 1:28 p.m.
Dear Przemyslaw,
On Oct 29, 2013, at 3:24 PM, Przemyslaw Marczak wrote:

> Dear Pantelis,
> 
> On 10/29/2013 02:08 PM, Pantelis Antoniou wrote:
>> Dear Przemyslaw,
>> 
>> On Oct 29, 2013, at 3:02 PM, Przemyslaw Marczak wrote:
>> 
>>> Dear Pantelis,
>>> You don't reply for a long time. Could you look on it at this time?
>>> 
>> 
>> Is this a way to address people on a mailing list?
>> 
>> People, especially volunteers are busy with other things too.
>> 
>> Eventually things get merged, but not when people are being rude.
> 
> What did I wrong by contacting mmc-u-boot custodian via the mailing list?
> 
> At least for linux it is a good and common practice to "ping" maintainers after 2 weeks of not reply to the patch.
> 

This passive aggressive crap might work on your mother, but not here.

Learn how open source works, and expect delays when it is conference season.
I am in contact with the core maintainers and nothing is being dropped.

That is all.

-- Pantelis


>> 
>>> On 10/15/2013 10:18 PM, Przemyslaw Marczak wrote:
>>>> Hello Pantelis,
>>>> 
>>>> Could you look at this patch, please?
>>>> 
>>>> On 10/08/2013 06:12 PM, Przemyslaw Marczak wrote:
>>>>> Old command timeout value was too small and it caused I/O errors which
>>>>> led to uncompleted read/write/erase operations and filesystem errors.
>>>>> Timeout adaptation fixes this issue.
>>>>> 
>>>>> Changes in sdhci_send_command() function:
>>>>> - change timeout variable to static
>>>>> - increase default command timeout to 100 ms
>>>>> - add definition of max command timeout value,
>>>>>   which can be redefined in each board config file
>>>>> - wait for card ready state for max defined time
>>>>>   if it doesn't exceed defined maximum or return COMM_ERR
>>>>> 
>>>>> Once successfully increased timeout value will be used in next function
>>>>> call. This fix was tested on Goni, Trats, Trats2 boards by testing UMS
>>>>> on MMC storage.
>>>>> 
>>>>> Changes v2:
>>>>> - move global variable cmd_timeout into function sdhci_send_command()
>>>>> - change condition "==" to ">=" when comparing time with timeout
>>>>> - print information about timeout increasing and card busy timeout
>>>>> 
>>>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>> 
>>>> Thank you.
>>>> 
>>>> 
>>> 
>>> Regards
>>> --
>>> Przemyslaw Marczak
>>> Samsung R&D Institute Poland
>>> Samsung Electronics
>>> p.marczak@samsung.com
>> 
>> Regards
>> 
>> -- Pantelis
>> 
> 
> Regards
> -- 
> Przemyslaw Marczak
> Samsung R&D Institute Poland
> Samsung Electronics
> p.marczak@samsung.com
Mateusz Zalega - Oct. 29, 2013, 1:39 p.m.
>>>> Dear Pantelis,
>>>> You don't reply for a long time. Could you look on it at this time?
>>>>
>>>
>>> Is this a way to address people on a mailing list?
>>>
>>> People, especially volunteers are busy with other things too.
>>>
>>> Eventually things get merged, but not when people are being rude.
>>
>> What did I wrong by contacting mmc-u-boot custodian via the mailing list?
>>
>> At least for linux it is a good and common practice to "ping" maintainers after 2 weeks of not reply to the patch.
>>
> 
> This passive aggressive crap might work on your mother, but not here.
> 
> Learn how open source works, and expect delays when it is conference season.
> I am in contact with the core maintainers and nothing is being dropped.
> 
> That is all.
> 
> -- Pantelis

Mister, you seem to be upset.

Regards,

Patch

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index dfb2eee..46ae9cb 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -109,6 +109,19 @@  static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 	return 0;
 }
 
+/*
+ * No command will be sent by driver if card is busy, so driver must wait
+ * for card ready state.
+ * Every time when card is busy after timeout then (last) timeout value will be
+ * increased twice but only if it doesn't exceed global defined maximum.
+ * Each function call will use last timeout value. Max timeout can be redefined
+ * in board config file.
+ */
+#ifndef CONFIG_SDHCI_CMD_MAX_TIMEOUT
+#define CONFIG_SDHCI_CMD_MAX_TIMEOUT		3200
+#endif
+#define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT	100
+
 int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 		       struct mmc_data *data)
 {
@@ -117,11 +130,12 @@  int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 	int ret = 0;
 	int trans_bytes = 0, is_aligned = 1;
 	u32 mask, flags, mode;
-	unsigned int timeout, start_addr = 0;
+	unsigned int time = 0, start_addr = 0;
 	unsigned int retry = 10000;
+	int mmc_dev = mmc->block_dev.dev;
 
-	/* Wait max 10 ms */
-	timeout = 10;
+	/* Timeout unit - ms */
+	static unsigned int cmd_timeout = CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT;
 
 	sdhci_writel(host, SDHCI_INT_ALL_MASK, SDHCI_INT_STATUS);
 	mask = SDHCI_CMD_INHIBIT | SDHCI_DATA_INHIBIT;
@@ -132,11 +146,18 @@  int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 		mask &= ~SDHCI_DATA_INHIBIT;
 
 	while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
-		if (timeout == 0) {
-			printf("Controller never released inhibit bit(s).\n");
-			return COMM_ERR;
+		if (time >= cmd_timeout) {
+			printf("MMC: %d busy ", mmc_dev);
+			if (2 * cmd_timeout <= CONFIG_SDHCI_CMD_MAX_TIMEOUT) {
+				cmd_timeout += cmd_timeout;
+				printf("timeout increasing to: %u ms.\n",
+				       cmd_timeout);
+			} else {
+				puts("timeout.\n");
+				return COMM_ERR;
+			}
 		}
-		timeout--;
+		time++;
 		udelay(1000);
 	}