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

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 Tested-by: Marek Vasut marex@denx.de # on DENX MA5D4EV --- drivers/mmc/gen_atmel_mci.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c index da870c6..f090b27 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; }
@@ -323,6 +325,13 @@ 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; }

On Thursday, November 05, 2015 at 08:58:30 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 Tested-by: Marek Vasut marex@denx.de # on DENX MA5D4EV
drivers/mmc/gen_atmel_mci.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c index da870c6..f090b27 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;
}
@@ -323,6 +325,13 @@ 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 */
Is there a chance that curr_clk will be inited to zero and this would be called? I guess the easy fix is to init curr_clk to 1 in the probe() call. Sorry that I didn't mention this earlier.
Otherwise,
Acked-by: Marek Vasut marex@denx.de
- return 0;
}
Best regards, Marek Vasut

Hi Marek,
On jeu., nov. 05 2015, Marek Vasut marex@denx.de wrote:
On Thursday, November 05, 2015 at 08:58:30 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 Tested-by: Marek Vasut marex@denx.de # on DENX MA5D4EV
drivers/mmc/gen_atmel_mci.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c index da870c6..f090b27 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;
}
@@ -323,6 +325,13 @@ 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 */
Is there a chance that curr_clk will be inited to zero and this would be called?
curr_clk is set in the mci_set_mode function which is called from mci_init which is called by the mmc core function mmc_start_init before any use of a command and especially of mci_send_cmd(). So we are safe.
I guess the easy fix is to init curr_clk to 1 in the probe() call. Sorry that I didn't mention this earlier.
Otherwise,
Acked-by: Marek Vasut marex@denx.de
Thanks,
Gregory
- return 0;
}
Best regards, Marek Vasut

On Friday, November 06, 2015 at 10:28:31 AM, Gregory CLEMENT wrote:
Hi Marek,
On jeu., nov. 05 2015, Marek Vasut marex@denx.de wrote:
On Thursday, November 05, 2015 at 08:58:30 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 Tested-by: Marek Vasut marex@denx.de # on DENX MA5D4EV
drivers/mmc/gen_atmel_mci.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c index da870c6..f090b27 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;
}
@@ -323,6 +325,13 @@ 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 */
Is there a chance that curr_clk will be inited to zero and this would be called?
curr_clk is set in the mci_set_mode function which is called from mci_init which is called by the mmc core function mmc_start_init before any use of a command and especially of mci_send_cmd(). So we are safe.
Excellent, thanks for checking this :-)
I guess the easy fix is to init curr_clk to 1 in the probe() call. Sorry that I didn't mention this earlier.
Otherwise,
Acked-by: Marek Vasut marex@denx.de
Thanks,
Gregory
Thanks!
Best regards, Marek Vasut

Hi Gregory,
On 05.11.15 20:58, 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 Tested-by: Marek Vasut marex@denx.de # on DENX MA5D4EV
this patch seams to break avr32 ... I'm on it to investigate the root cause.
Andreas
drivers/mmc/gen_atmel_mci.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c index da870c6..f090b27 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;
}
@@ -323,6 +325,13 @@ 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;
}

On 23.11.15 15:35, Andreas Bießmann wrote:
Hi Gregory,
On 05.11.15 20:58, 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 Tested-by: Marek Vasut marex@denx.de # on DENX MA5D4EV
this patch seams to break avr32 ... I'm on it to investigate the root cause.
Sorry, was my fault.
Tested-by: Andreas Bießmann andreas.devel@googlemail.com # on atngw100 Acked-by: Andreas Bießmann andreas.devel@googlemail.com

Hi Pantelis,
On 05.11.15 20:58, 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 Tested-by: Marek Vasut marex@denx.de # on DENX MA5D4EV
this patch [1] is delegated to you on patchwork ... will you take it?
Andreas
[1] http://patchwork.ozlabs.org/patch/540686/
drivers/mmc/gen_atmel_mci.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c index da870c6..f090b27 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;
}
@@ -323,6 +325,13 @@ 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;
}

Dear Gregory CLEMENT,
Gregory CLEMENT gregory.clement@free-electrons.com writes:
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 Tested-by: Marek Vasut marex@denx.de # on DENX MA5D4EV Acked-by: Marek Vasut marex@denx.de Tested-by: Andreas Bießmann andreas.devel@googlemail.com # on atngw100 Acked-by: Andreas Bießmann andreas.devel@googlemail.com [fixed minor checkpatch warning] Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com
drivers/mmc/gen_atmel_mci.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
applied to u-boot-atmel/master, thanks!
Best regards, Andreas Bießmann
participants (4)
-
Andreas Bießmann
-
Andreas Bießmann
-
Gregory CLEMENT
-
Marek Vasut