[U-Boot] [PATCH] Fixing timing issue with eMMC on atmel MCI

[U-Boot] [PATCH] Fixing timing issue with eMMC on atmel MCI

[Gregory CLEMENT]

Hi,

I got similar issues that Marek had:
http://lists.denx.de/pipermail/u-boot/2015-October/231420.html.

When trying to initialize an eMMC on a SAMA5D4 based board the CPU
stuck. However, the fix sent by Marek was not enough for me and after
digging in the sources and trying many combinations I found that the
culprit was the switch command (CMD 6). A small delay was needed after
this command before the next one. The MMC spec seems to confirm that
by speaking about waiting 8 clocks after this command.

In the following patch I removed the delay introduced by Marek, and
instead I added a 8 clocks wait just after the switch command.

Marek, could you test it on your board and confirm that it is still
work for you?

Thanks,

Gregory CLEMENT (1):
  mmc: atmel: Properly fix clock configuration

 drivers/mmc/gen_atmel_mci.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

-- 
2.5.0

[U-Boot] [PATCH] mmc: atmel: Properly fix clock configuration

[Gregory CLEMENT]

Timing issue occurs on eMMC not only when modifying the frequency but
also for all the switch command(CMD6). According to the MMC spec waiting
8 clocks after a switch command would be the thing to do.

This patch allows fixing CPU hang observed when trying to changing the
bus width on a eMMC on SAMA5D4.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/mmc/gen_atmel_mci.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
index da870c6..3f7cfba 100644
--- a/drivers/mmc/gen_atmel_mci.c
+++ b/drivers/mmc/gen_atmel_mci.c
@@ -36,6 +36,7 @@ struct atmel_mci_priv {
 	struct mmc_config	cfg;
 	struct atmel_mci	*mci;
 	unsigned int		initialized:1;
+	unsigned int		curr_clk;
 };
 
 /* Read Atmel MCI IP version */
@@ -91,7 +92,10 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
 
 		}
 	}
-
+	if (version >= 0x500)
+		priv->curr_clk = bus_hz / (clkdiv * 2 + clkodd + 2);
+	else
+		priv->curr_clk = (bus_hz / (clkdiv + 1)) / 2;
 	blklen &= 0xfffc;
 
 	mr = MMCI_BF(CLKDIV, clkdiv);
@@ -118,8 +122,6 @@ static void mci_set_mode(struct mmc *mmc, u32 hz, u32 blklen)
 	if (mmc->card_caps & mmc->cfg->host_caps & MMC_MODE_HS)
 		writel(MMCI_BIT(HSMODE), &mci->cfg);
 
-	udelay(50);
-
 	priv->initialized = 1;
 }
 
@@ -284,8 +286,10 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 			{
 				u32 cnt = word_count * 4;
 				printf("Read Data:\n");
+				/*
 				print_buffer(0, data->dest + cnt * block_count,
 					     1, cnt, 0);
+				*/
 			}
 #endif
 #ifdef DEBUG
@@ -323,6 +327,12 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 		}
 	}
 
+	/* After the switch command, wait for 8 clocks before the next
+	 * command
+	 */
+	if (cmd->cmdidx == MMC_CMD_SWITCH)
+		udelay(8*1000000/ priv->curr_clk); /* 8 clk in us*/
+
 	return 0;
 }
 
-- 
2.5.0

[U-Boot] [PATCH] mmc: atmel: Properly fix clock configuration

[Marek Vasut]

On Thursday, November 05, 2015 at 03:53:16 PM, Gregory CLEMENT wrote:
> Timing issue occurs on eMMC not only when modifying the frequency but
> also for all the switch command(CMD6). According to the MMC spec waiting
> 8 clocks after a switch command would be the thing to do.
> 
> This patch allows fixing CPU hang observed when trying to changing the
> bus width on a eMMC on SAMA5D4.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

On DENX MA5D4EVK:

Tested-by: Marek Vasut <marex@denx.de>

[...]

> @@ -284,8 +286,10 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> struct mmc_data *data) {
>  				u32 cnt = word_count * 4;
>  				printf("Read Data:\n");
> +				/*
>  				print_buffer(0, data->dest + cnt * block_count,
>  					     1, cnt, 0);
> +				*/

This bit shouldn't be in the patch I guess ;-)

>  			}
>  #endif
>  #ifdef DEBUG
> @@ -323,6 +327,12 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> struct mmc_data *data) }
>  	}
> 
> +	/* After the switch command, wait for 8 clocks before the next
> +	 * command
> +	 */

The comment style should match the kernel one -- multilines go like this:
/*
 * foo
 * bar
 */

> +	if (cmd->cmdidx == MMC_CMD_SWITCH)
> +		udelay(8*1000000/ priv->curr_clk); /* 8 clk in us*/
> +
>  	return 0;
>  }

Best regards,
Marek Vasut

[U-Boot] [PATCH] mmc: atmel: Properly fix clock configuration

[Gregory CLEMENT]

Hi Marek,
 
 On jeu., nov. 05 2015, Marek Vasut <marex@denx.de> wrote:

> On Thursday, November 05, 2015 at 03:53:16 PM, Gregory CLEMENT wrote:
>> Timing issue occurs on eMMC not only when modifying the frequency but
>> also for all the switch command(CMD6). According to the MMC spec waiting
>> 8 clocks after a switch command would be the thing to do.
>> 
>> This patch allows fixing CPU hang observed when trying to changing the
>> bus width on a eMMC on SAMA5D4.
>> 
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> On DENX MA5D4EVK:
>
> Tested-by: Marek Vasut <marex@denx.de>

Thanks for having tested it so quick!

>
> [...]
>
>> @@ -284,8 +286,10 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>> struct mmc_data *data) {
>>  				u32 cnt = word_count * 4;
>>  				printf("Read Data:\n");
>> +				/*
>>  				print_buffer(0, data->dest + cnt * block_count,
>>  					     1, cnt, 0);
>> +				*/
>
> This bit shouldn't be in the patch I guess ;-)

You're right!

>
>>  			}
>>  #endif
>>  #ifdef DEBUG
>> @@ -323,6 +327,12 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>> struct mmc_data *data) }
>>  	}
>> 
>> +	/* After the switch command, wait for 8 clocks before the next
>> +	 * command
>> +	 */
>
> The comment style should match the kernel one -- multilines go like this:
> /*
>  * foo
>  * bar
>  */

Sure, I will send a v2 with tour comment taken into account anw with
your tested-by.

Thanks,

Gregory

>
>> +	if (cmd->cmdidx == MMC_CMD_SWITCH)
>> +		udelay(8*1000000/ priv->curr_clk); /* 8 clk in us*/
>> +
>>  	return 0;
>>  }
>
> Best regards,
> Marek Vasut

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com