[U-Boot] [PATCH] mmc: dw_mmc: fix timeout calculate method
There are two cases not been considered: - use uint for timeout, it will overflow when size bigger than 512KB for it *8*1000 at the beginning, but we may use size up to 32MB; - The timeout is using clock speed for data rate, but the device may not have such high speed, eg. clock is 52MHz while the device write speed may be less than 10MB/s.
Fix them in this patch, the max timeout is about 6500 when size is 32MB after fix.
Signed-off-by: Kever Yang kever.yang@rock-chips.com ---
drivers/mmc/dw_mmc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 22f6c7eefd..2cb61ba184 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -117,10 +117,10 @@ static int dwmci_fifo_ready(struct dwmci_host *host, u32 bit, u32 *len)
static unsigned int dwmci_get_timeout(struct mmc *mmc, const unsigned int size) { - unsigned int timeout; + u64 timeout;
- timeout = size * 8 * 1000; /* counting in bits and msec */ - timeout *= 2; /* wait twice as long */ + timeout = (u64)size * 8 * 1000; /* counting in bits and msec */ + timeout *= 10; /* wait 10 times as long */ timeout /= mmc->clock; timeout /= mmc->bus_width; timeout /= mmc->ddr_mode ? 2 : 1;
Subject: [PATCH] mmc: dw_mmc: fix timeout calculate method
There are two cases not been considered:
- use uint for timeout, it will overflow when size bigger than 512KB for it *8*1000 at the beginning, but we may use size up to 32MB;
- The timeout is using clock speed for data rate, but the device may not have such high speed, eg. clock is 52MHz while the device write speed may be less than 10MB/s.
Fix them in this patch, the max timeout is about 6500 when size is 32MB after fix.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
Applied to mmc/master.
Thanks, Peng.
Hi Kever,
-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Peng Fan Sent: 2019年8月27日 15:40 To: Kever Yang kever.yang@rock-chips.com; u-boot@lists.denx.de Cc: trini@konsulko.com Subject: Re: [U-Boot] [PATCH] mmc: dw_mmc: fix timeout calculate method
Subject: [PATCH] mmc: dw_mmc: fix timeout calculate method
There are two cases not been considered:
- use uint for timeout, it will overflow when size bigger than 512KB for it *8*1000 at the beginning, but we may use size up to 32MB;
- The timeout is using clock speed for data rate, but the device may not have such high speed, eg. clock is 52MHz while the device write speed
may
be less than 10MB/s.
Fix them in this patch, the max timeout is about 6500 when size is 32MB after fix.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
This patch breaks build, I need drop it. See build: https://travis-ci.org/MrVan/u-boot/jobs/577210464
Regards, Peng.
Applied to mmc/master.
Thanks, Peng. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d enx.de%2Flistinfo%2Fu-boot&data=02%7C01%7CPeng.Fan%40nxp.com %7Cec929026da5f42951b6108d72ac1c446%7C686ea1d3bc2b4c6fa92cd99c 5c301635%7C0%7C0%7C637024884027193189&sdata=SPBna8IBqgpo9 %2F6ab1YO5sbpX6NGgkCowt0oLclRHEk%3D&reserved=0
Add ARC maintainers.
On 2019/8/27 下午5:58, Peng Fan wrote:
Hi Kever,
-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Peng Fan Sent: 2019年8月27日 15:40 To: Kever Yang kever.yang@rock-chips.com; u-boot@lists.denx.de Cc: trini@konsulko.com Subject: Re: [U-Boot] [PATCH] mmc: dw_mmc: fix timeout calculate method
Subject: [PATCH] mmc: dw_mmc: fix timeout calculate method
There are two cases not been considered:
- use uint for timeout, it will overflow when size bigger than 512KB for it *8*1000 at the beginning, but we may use size up to 32MB;
- The timeout is using clock speed for data rate, but the device may not have such high speed, eg. clock is 52MHz while the device write speed
may
be less than 10MB/s.
Fix them in this patch, the max timeout is about 6500 when size is 32MB after fix.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
This patch breaks build, I need drop it. See build: https://travis-ci.org/MrVan/u-boot/jobs/577210464
The error happens only in ARC architecture, and seems like the arc compiler not support
64bit div?
Any idea on how to fix this?
Thanks, - Kever
Regards, Peng.
Applied to mmc/master.
Thanks, Peng. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d enx.de%2Flistinfo%2Fu-boot&data=02%7C01%7CPeng.Fan%40nxp.com %7Cec929026da5f42951b6108d72ac1c446%7C686ea1d3bc2b4c6fa92cd99c 5c301635%7C0%7C0%7C637024884027193189&sdata=SPBna8IBqgpo9 %2F6ab1YO5sbpX6NGgkCowt0oLclRHEk%3D&reserved=0
Hi Kever,
-----Original Message----- From: Kever Yang kever.yang@rock-chips.com Sent: Thursday, August 29, 2019 10:17 AM To: Peng Fan peng.fan@nxp.com; u-boot@lists.denx.de Cc: trini@konsulko.com; Eugeniy.Paltsev@synopsys.com; Alexey Brodkin abrodkin@synopsys.com; Simon Glass sjg@chromium.org Subject: Re: [PATCH] mmc: dw_mmc: fix timeout calculate method
Add ARC maintainers.
Thanks for adding us in the loop.
On 2019/8/27 下午5:58, Peng Fan wrote:
Hi Kever,
-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Peng Fan Sent: 2019年8月27日 15:40 To: Kever Yang kever.yang@rock-chips.com; u-boot@lists.denx.de Cc: trini@konsulko.com Subject: Re: [U-Boot] [PATCH] mmc: dw_mmc: fix timeout calculate method
Subject: [PATCH] mmc: dw_mmc: fix timeout calculate method
There are two cases not been considered:
- use uint for timeout, it will overflow when size bigger than 512KB for it *8*1000 at the beginning, but we may use size up to 32MB;
- The timeout is using clock speed for data rate, but the device may not have such high speed, eg. clock is 52MHz while the device write speed
may
be less than 10MB/s.
Fix them in this patch, the max timeout is about 6500 when size is 32MB after fix.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
This patch breaks build, I need drop it. See build: https://travis-ci.org/MrVan/u-boot/jobs/577210464
The error happens only in ARC architecture, and seems like the arc compiler not support 64bit div?
It's not a compiler problem but a subtle issue with libgcc. To be independent from toolchain type (and we have quite a few flavors like little- and big-endian, ARCv1 or ARCv2 etc) decision was made to move libgcc into U-Boot but given full libgcc is quite large only primitives required by up-to-date U-Boot were copied from GCC to U-Boot.
Any idea on how to fix this?
Add missing primitives like __udivdi3() and their dependencies, see for example: https://github.com/gcc-mirror/gcc/blob/master/libgcc/libgcc2.c#L1318.
And I'll happily do it myself but for some reason I cannot reproduce that problem locally. I use the same pre-built arc-2018.09 toolchain: ---------------------------->8----------------------- # arc-linux-gcc -v gcc version 8.2.1 20180814 (ARCv2 ISA Linux uClibc toolchain 2018.09) ---------------------------->8-----------------------
And may successfully build U-Boot for HSDK from https://github.com/MrVan/u-boot/commits/mmc-8-28-2019: ---------------------------->8----------------------- # git l 76c0bf2873 - (HEAD -> mrvan, github-mrvan/mmc-8-28-2019) ARM: dts: ast2500: Add SDHCI nodes (27 hours ago) <Eddie James> 07b000cd32 - configs: AST2500 EVB: Enable SD controller (27 hours ago) <Eddie James> 5f6a033f10 - mmc: Add Aspeed SD controller driver (27 hours ago) <Eddie James> a6503eaa66 - clk: aspeed: Add support for SD clock (27 hours ago) <Eddie James> c54e328452 - mmc: Rename timeout parameters for clarification (27 hours ago) <Sam Protsenko> be949bb19c - mmc: Fix timeout values passed to mmc_wait_dat0() (27 hours ago) <Sam Protsenko> 49842f0edf - dm: mmc: remove unused U_BOOT_DRIVER(mmc) (27 hours ago) <Andy Yan> 7b91b47f8f - Prepare v2019.10-rc3 (27 hours ago) <Tom Rini> ---------------------------->8-----------------------
That way: ---------------------------->8----------------------- ./tools/buildman/buildman hsdk boards.cfg is up to date. Nothing to do. Building current source for 1 boards (1 thread, 48 jobs per thread) arc: w+ hsdk 0 1 0 /1 hsdk ---------------------------->8-----------------------
What do I do wrong?
-Alexey
Hi Alexey,
On 2019/8/29 下午5:21, Alexey Brodkin wrote:
Hi Kever,
-----Original Message----- From: Kever Yang kever.yang@rock-chips.com Sent: Thursday, August 29, 2019 10:17 AM To: Peng Fan peng.fan@nxp.com; u-boot@lists.denx.de Cc: trini@konsulko.com; Eugeniy.Paltsev@synopsys.com; Alexey Brodkin abrodkin@synopsys.com; Simon Glass sjg@chromium.org Subject: Re: [PATCH] mmc: dw_mmc: fix timeout calculate method
Add ARC maintainers.
Thanks for adding us in the loop.
On 2019/8/27 下午5:58, Peng Fan wrote:
Hi Kever,
-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Peng Fan Sent: 2019年8月27日 15:40 To: Kever Yang kever.yang@rock-chips.com; u-boot@lists.denx.de Cc: trini@konsulko.com Subject: Re: [U-Boot] [PATCH] mmc: dw_mmc: fix timeout calculate method
Subject: [PATCH] mmc: dw_mmc: fix timeout calculate method
There are two cases not been considered:
- use uint for timeout, it will overflow when size bigger than 512KB for it *8*1000 at the beginning, but we may use size up to 32MB;
- The timeout is using clock speed for data rate, but the device may not have such high speed, eg. clock is 52MHz while the device write speed
may
be less than 10MB/s.Fix them in this patch, the max timeout is about 6500 when size is 32MB after fix.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
This patch breaks build, I need drop it. See build: https://travis-ci.org/MrVan/u-boot/jobs/577210464
The error happens only in ARC architecture, and seems like the arc compiler not support 64bit div?
It's not a compiler problem but a subtle issue with libgcc. To be independent from toolchain type (and we have quite a few flavors like little- and big-endian, ARCv1 or ARCv2 etc) decision was made to move libgcc into U-Boot but given full libgcc is quite large only primitives required by up-to-date U-Boot were copied from GCC to U-Boot.
Any idea on how to fix this?
Add missing primitives like __udivdi3() and their dependencies, see for example: https://github.com/gcc-mirror/gcc/blob/master/libgcc/libgcc2.c#L1318.
And I'll happily do it myself but for some reason I cannot reproduce that problem locally. I use the same pre-built arc-2018.09 toolchain: ---------------------------->8----------------------- # arc-linux-gcc -v gcc version 8.2.1 20180814 (ARCv2 ISA Linux uClibc toolchain 2018.09) ---------------------------->8-----------------------
And may successfully build U-Boot for HSDK from https://github.com/MrVan/u-boot/commits/mmc-8-28-2019: ---------------------------->8----------------------- # git l 76c0bf2873 - (HEAD -> mrvan, github-mrvan/mmc-8-28-2019) ARM: dts: ast2500: Add SDHCI nodes (27 hours ago) <Eddie James> 07b000cd32 - configs: AST2500 EVB: Enable SD controller (27 hours ago) <Eddie James> 5f6a033f10 - mmc: Add Aspeed SD controller driver (27 hours ago) <Eddie James> a6503eaa66 - clk: aspeed: Add support for SD clock (27 hours ago) <Eddie James> c54e328452 - mmc: Rename timeout parameters for clarification (27 hours ago) <Sam Protsenko> be949bb19c - mmc: Fix timeout values passed to mmc_wait_dat0() (27 hours ago) <Sam Protsenko> 49842f0edf - dm: mmc: remove unused U_BOOT_DRIVER(mmc) (27 hours ago) <Andy Yan> 7b91b47f8f - Prepare v2019.10-rc3 (27 hours ago) <Tom Rini> ---------------------------->8-----------------------
I think this tree does not including this patch, Peng drop it because of this issue, so you need to apply this patch in your branch to reproduce the problem. I have send out V2 patch for this fix with only using 32bit variable instead of 64bit.
Thanks, - Kever
That way: ---------------------------->8----------------------- ./tools/buildman/buildman hsdk boards.cfg is up to date. Nothing to do. Building current source for 1 boards (1 thread, 48 jobs per thread) arc: w+ hsdk 0 1 0 /1 hsdk ---------------------------->8-----------------------
What do I do wrong?
-Alexey
Hi Kever,
[snip]
I think this tree does not including this patch, Peng drop it because of this issue, so you need to apply this patch in your branch to reproduce the problem. I have send out V2 patch for this fix with only using 32bit variable
Could you please refer me to the problematic patch so I may try it?
-Alexey
Hi Alexey,
On 2019/8/30 下午9:28, Alexey Brodkin wrote:
Hi Kever,
[snip]
I think this tree does not including this patch, Peng drop it because of this issue, so you need to apply this patch in your branch to reproduce the problem. I have send out V2 patch for this fix with only using 32bit variable
Could you please refer me to the problematic patch so I may try it?
This is the patch with problem, and here is the link on patchwork: https://patchwork.ozlabs.org/patch/1146845/
Thanks, - Kever
-Alexey
Hi Kever,
-----Original Message----- From: Kever Yang kever.yang@rock-chips.com Sent: Monday, September 2, 2019 11:05 AM To: Alexey Brodkin abrodkin@synopsys.com Cc: trini@konsulko.com; Eugeniy.Paltsev@synopsys.com; Simon Glass sjg@chromium.org; Peng Fan peng.fan@nxp.com; u-boot@lists.denx.de Subject: Re: [PATCH] mmc: dw_mmc: fix timeout calculate method
Hi Alexey,
On 2019/8/30 下午9:28, Alexey Brodkin wrote:
Hi Kever,
[snip]
I think this tree does not including this patch, Peng drop it because of this issue, so you need to apply this patch in your branch to reproduce the problem. I have send out V2 patch for this fix with only using 32bit variable
Could you please refer me to the problematic patch so I may try it?
This is the patch with problem, and here is the link on patchwork: https://patchwork.ozlabs.org/patch/1146845/
Please find my fixes here: https://patchwork.ozlabs.org/patch/1156541/ https://patchwork.ozlabs.org/patch/1156617/
Tom do we want https://patchwork.ozlabs.org/patch/1146845/ and fixes for it (see 2 items above) to become a part of upcoming v2019.10 release or it will be slated for the next one?
-Alexey
On Mon, Sep 02, 2019 at 12:24:42PM +0000, Alexey Brodkin wrote:
Hi Kever,
-----Original Message----- From: Kever Yang kever.yang@rock-chips.com Sent: Monday, September 2, 2019 11:05 AM To: Alexey Brodkin abrodkin@synopsys.com Cc: trini@konsulko.com; Eugeniy.Paltsev@synopsys.com; Simon Glass sjg@chromium.org; Peng Fan peng.fan@nxp.com; u-boot@lists.denx.de Subject: Re: [PATCH] mmc: dw_mmc: fix timeout calculate method
Hi Alexey,
On 2019/8/30 下午9:28, Alexey Brodkin wrote:
Hi Kever,
[snip]
I think this tree does not including this patch, Peng drop it because of this issue, so you need to apply this patch in your branch to reproduce the problem. I have send out V2 patch for this fix with only using 32bit variable
Could you please refer me to the problematic patch so I may try it?
This is the patch with problem, and here is the link on patchwork: https://patchwork.ozlabs.org/patch/1146845/
Please find my fixes here: https://patchwork.ozlabs.org/patch/1156541/ https://patchwork.ozlabs.org/patch/1156617/
Tom do we want https://patchwork.ozlabs.org/patch/1146845/ and fixes for it (see 2 items above) to become a part of upcoming v2019.10 release or it will be slated for the next one?
I think we should aim to get all the fixes in for this release.
H Tom,
[snip]
This is the patch with problem, and here is the link on patchwork: https://patchwork.ozlabs.org/patch/1146845/
Please find my fixes here: https://patchwork.ozlabs.org/patch/1156541/ https://patchwork.ozlabs.org/patch/1156617/
Tom do we want https://patchwork.ozlabs.org/patch/1146845/ and fixes for it (see 2 items above) to become a part of upcoming v2019.10 release or it will be slated for the next one?
I think we should aim to get all the fixes in for this release.
Done, see https://lists.denx.de/pipermail/u-boot/2019-September/382628.html
-Alexey
participants (4)
-
Alexey Brodkin -
Kever Yang -
Peng Fan -
Tom Rini