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

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

[Przemyslaw Marczak]

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.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
---
 drivers/mmc/sdhci.c |   34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 4261991..af11fc5 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -110,6 +110,22 @@ 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
+
+/* Timeout unit - ms */
+static unsigned int cmd_timeout = CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT;
+
 int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 		       struct mmc_data *data)
 {
@@ -118,12 +134,9 @@ 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;
 
-	/* Wait max 10 ms */
-	timeout = 10;
-
 	sdhci_writel(host, SDHCI_INT_ALL_MASK, SDHCI_INT_STATUS);
 	mask = SDHCI_CMD_INHIBIT | SDHCI_DATA_INHIBIT;
 
@@ -133,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) {
+		if (time == cmd_timeout) {
 			printf("Controller never released inhibit bit(s).\n");
-			return COMM_ERR;
+			if (2 * cmd_timeout <= CONFIG_SDHCI_CMD_MAX_TIMEOUT) {
+				cmd_timeout += cmd_timeout;
+				debug("Increasing command timeout to: %u ms.\n",
+				      cmd_timeout);
+			} else {
+				error("Command timeout is set to max.\n");
+				return COMM_ERR;
+			}
 		}
-		timeout--;
+		time++;
 		udelay(1000);
 	}
 
-- 
1.7.9.5

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

[Pantelis Antoniou]

Hi there,


On Oct 1, 2013, at 3:16 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.
> 
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> ---
> drivers/mmc/sdhci.c |   34 +++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 4261991..af11fc5 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -110,6 +110,22 @@ 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
> +
> +/* Timeout unit - ms */
> +static unsigned int cmd_timeout = CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT;
> +
> int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
> 		       struct mmc_data *data)
> {
> @@ -118,12 +134,9 @@ 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;
> 
> -	/* Wait max 10 ms */
> -	timeout = 10;
> -
> 	sdhci_writel(host, SDHCI_INT_ALL_MASK, SDHCI_INT_STATUS);
> 	mask = SDHCI_CMD_INHIBIT | SDHCI_DATA_INHIBIT;
> 
> @@ -133,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) {
> +		if (time == cmd_timeout) {

time >= cmd_timeout here.

You rely on the timeout hitting exactly the same value which is not guaranteed.

> 			printf("Controller never released inhibit bit(s).\n");
> -			return COMM_ERR;
> +			if (2 * cmd_timeout <= CONFIG_SDHCI_CMD_MAX_TIMEOUT) {
> +				cmd_timeout += cmd_timeout;
> +				debug("Increasing command timeout to: %u ms.\n",
> +				      cmd_timeout);
> +			} else {
> +				error("Command timeout is set to max.\n");
> +				return COMM_ERR;
> +			}
> 		}
> -		timeout--;
> +		time++;
> 		udelay(1000);
> 	}
> 
> -- 
> 1.7.9.5
> 

Other than that the concept seems sound.

Regards

-- Pantelis

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

[Przemyslaw Marczak]

Hello Pantelis,
Thank you for reply


On 10/01/2013 05:50 PM, Pantelis Antoniou wrote:
>     while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
> -        if (timeout == 0) {
> +        if (time == cmd_timeout) {
> time >= cmd_timeout here.
>
> You rely on the timeout hitting exactly the same value which is not 
> guaranteed.
I think this condition is guaranteed here, because of "time" value that 
is incremented only inside the loop.
Also if meets (time == cmd_timeout) condition and next if timeout will 
be increased twice, then eg. if current timeout
is 100ms -> next will be 200 ms, so it needs 100 loops and no more.

Am I wrong?

Regards

-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

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

[Pantelis Antoniou]

Hi Przemyslaw,

On Oct 1, 2013, at 7:59 PM, Przemyslaw Marczak wrote:

> Hello Pantelis, 
> Thank you for reply 
> 
> 
> On 10/01/2013 05:50 PM, Pantelis Antoniou wrote: 
>>     while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) { 
>> -        if (timeout == 0) { 
>> +        if (time == cmd_timeout) { 
>> time >= cmd_timeout here. 
>> 
>> You rely on the timeout hitting exactly the same value which is not guaranteed. 
> I think this condition is guaranteed here, because of "time" value that is incremented only inside the loop. 
> Also if meets (time == cmd_timeout) condition and next if timeout will be increased twice, then eg. if current timeout 
> is 100ms -> next will be 200 ms, so it needs 100 loops and no more. 
> 
> Am I wrong? 
> 

OK, let's take things one at a time:

First of all you use the global variable cmd_timeout, and you alter it's value. Where it is reset back
in case the operation starts all over again?

Secondly the check time == cmd_timeout is very very fragile. You depend on the loop only incrementing
the time by one. 

This is not always guaranteed to be the case in the future.

Using a greater than comparison you are safe even if in sometime in the future the step changes and
there is absolutely no performance penalty.

> Regards 
> 
> -- 
> Przemyslaw Marczak 
> Samsung R&D Institute Poland 
> Samsung Electronics 
> p.marczak at samsung.com 
> 

Regards

-- Pantelis

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

[Jaehoon Chung]

Hi Przemyslaw,

On 10/01/2013 09:16 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.
> 
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> ---
>  drivers/mmc/sdhci.c |   34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 4261991..af11fc5 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -110,6 +110,22 @@ 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
How do you get "3200"? Is it too long time?

> +#endif
> +#define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT	100
> +
> +/* Timeout unit - ms */
> +static unsigned int cmd_timeout = CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT;
Global variable?

Best Regards,
Jaehoon Chung
> +
>  int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
>  		       struct mmc_data *data)
>  {
> @@ -118,12 +134,9 @@ 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;
>  
> -	/* Wait max 10 ms */
> -	timeout = 10;
> -
>  	sdhci_writel(host, SDHCI_INT_ALL_MASK, SDHCI_INT_STATUS);
>  	mask = SDHCI_CMD_INHIBIT | SDHCI_DATA_INHIBIT;
>  
> @@ -133,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) {
> +		if (time == cmd_timeout) {
>  			printf("Controller never released inhibit bit(s).\n");
> -			return COMM_ERR;
> +			if (2 * cmd_timeout <= CONFIG_SDHCI_CMD_MAX_TIMEOUT) {
> +				cmd_timeout += cmd_timeout;
> +				debug("Increasing command timeout to: %u ms.\n",
> +				      cmd_timeout);
> +			} else {
> +				error("Command timeout is set to max.\n");
> +				return COMM_ERR;
> +			}
>  		}
> -		timeout--;
> +		time++;
>  		udelay(1000);
>  	}
>  
> 

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

[Przemyslaw Marczak]

Hi Jaehoon,

On 10/04/2013 06:39 AM, Jaehoon Chung wrote:
> Hi Przemyslaw,
>
> On 10/01/2013 09:16 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.
>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> ---
>>   drivers/mmc/sdhci.c |   34 +++++++++++++++++++++++++++-------
>>   1 file changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 4261991..af11fc5 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -110,6 +110,22 @@ 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
> How do you get "3200"? Is it too long time?
>

After testing mmc read/write operations on Trats emmc card I observed 
that in most cases the timeout value was incremented to 800ms.
You can add reset timeout at every function call and than you will see 
how often and what time values are needed by commands, in most cases it 
is small time value but after transfer ends - send status command is 
send, which needs more time. I suppose that there are a lot of card 
types which need some more time - so I put there this value to be sure 
that in most cases this MAX is good enough.


>> +#endif
>> +#define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT	100
>> +
>> +/* Timeout unit - ms */
>> +static unsigned int cmd_timeout = CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT;
> Global variable?

Ok, maybe I should put this inside function...

Regards,

-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

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

[Przemyslaw Marczak]

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

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);
 	}
 
-- 
1.7.9.5

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

[Przemyslaw Marczak]

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
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

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

[Przemyslaw Marczak]

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
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

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

[Pantelis Antoniou]

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 at samsung.com

Regards

-- Pantelis

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

[Przemyslaw Marczak]

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 at samsung.com
>
> Regards
>
> -- Pantelis
>

Regards
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

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

[Pantelis Antoniou]

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 at samsung.com
>> 
>> Regards
>> 
>> -- Pantelis
>> 
> 
> Regards
> -- 
> Przemyslaw Marczak
> Samsung R&D Institute Poland
> Samsung Electronics
> p.marczak at samsung.com

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

[Mateusz Zalega]

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


-- 
Mateusz Zalega
Samsung R&D Institute Poland

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

[Pantelis Antoniou]

Hi Przemyslaw,

On Oct 1, 2013, at 3:16 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.
> 
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> ---
> drivers/mmc/sdhci.c |   34 +++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 4261991..af11fc5 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -110,6 +110,22 @@ 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
> +
> +/* Timeout unit - ms */
> +static unsigned int cmd_timeout = CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT;
> +
> int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
> 		       struct mmc_data *data)
> {
> @@ -118,12 +134,9 @@ 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;
> 
> -	/* Wait max 10 ms */
> -	timeout = 10;
> -
> 	sdhci_writel(host, SDHCI_INT_ALL_MASK, SDHCI_INT_STATUS);
> 	mask = SDHCI_CMD_INHIBIT | SDHCI_DATA_INHIBIT;
> 
> @@ -133,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) {
> +		if (time == cmd_timeout) {
> 			printf("Controller never released inhibit bit(s).\n");
> -			return COMM_ERR;
> +			if (2 * cmd_timeout <= CONFIG_SDHCI_CMD_MAX_TIMEOUT) {
> +				cmd_timeout += cmd_timeout;
> +				debug("Increasing command timeout to: %u ms.\n",
> +				      cmd_timeout);
> +			} else {
> +				error("Command timeout is set to max.\n");
> +				return COMM_ERR;
> +			}
> 		}
> -		timeout--;
> +		time++;
> 		udelay(1000);
> 	}
> 
> -- 
> 1.7.9.5
> 

Applied, thanks

Acked-by: Pantelis Antoniou <panto@antoniou-consulting.com>