* [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration
2015-11-05 19:58 [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration Gregory CLEMENT
@ 2015-11-05 20:15 ` Marek Vasut
2015-11-06 9:28 ` Gregory CLEMENT
2015-11-23 14:35 ` Andreas Bießmann
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2015-11-05 20:15 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration
2015-11-05 20:15 ` Marek Vasut
@ 2015-11-06 9:28 ` Gregory CLEMENT
2015-11-06 13:39 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Gregory CLEMENT @ 2015-11-06 9:28 UTC (permalink / raw)
To: u-boot
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
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 8+ messages in thread* [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration
2015-11-06 9:28 ` Gregory CLEMENT
@ 2015-11-06 13:39 ` Marek Vasut
0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2015-11-06 13:39 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration
2015-11-05 19:58 [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration Gregory CLEMENT
2015-11-05 20:15 ` Marek Vasut
@ 2015-11-23 14:35 ` Andreas Bießmann
2015-11-23 14:49 ` Andreas Bießmann
2015-12-01 16:52 ` Andreas Bießmann
2016-01-27 12:59 ` [U-Boot] [U-Boot, " Andreas Bießmann
3 siblings, 1 reply; 8+ messages in thread
From: Andreas Bießmann @ 2015-11-23 14:35 UTC (permalink / raw)
To: u-boot
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;
> }
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration
2015-11-23 14:35 ` Andreas Bießmann
@ 2015-11-23 14:49 ` Andreas Bießmann
0 siblings, 0 replies; 8+ messages in thread
From: Andreas Bießmann @ 2015-11-23 14:49 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration
2015-11-05 19:58 [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration Gregory CLEMENT
2015-11-05 20:15 ` Marek Vasut
2015-11-23 14:35 ` Andreas Bießmann
@ 2015-12-01 16:52 ` Andreas Bießmann
2016-01-27 12:59 ` [U-Boot] [U-Boot, " Andreas Bießmann
3 siblings, 0 replies; 8+ messages in thread
From: Andreas Bießmann @ 2015-12-01 16:52 UTC (permalink / raw)
To: u-boot
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;
> }
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0xBC2469C7.asc
Type: application/pgp-keys
Size: 36384 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151201/b60c930b/attachment.key>
^ permalink raw reply [flat|nested] 8+ messages in thread* [U-Boot] [U-Boot, v2] mmc: atmel: Properly fix clock configuration
2015-11-05 19:58 [U-Boot] [PATCH v2] mmc: atmel: Properly fix clock configuration Gregory CLEMENT
` (2 preceding siblings ...)
2015-12-01 16:52 ` Andreas Bießmann
@ 2016-01-27 12:59 ` Andreas Bießmann
3 siblings, 0 replies; 8+ messages in thread
From: Andreas Bießmann @ 2016-01-27 12:59 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 8+ messages in thread