Message ID | 1446753510-16783-1-git-send-email-gregory.clement@free-electrons.com |
---|---|
State | Awaiting Upstream |
Delegated to: | Andreas Bießmann |
Headers | show |
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
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; }