* [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems
@ 2018-02-28 11:53 Evgeniy Didin
2018-02-28 15:47 ` Andy Shevchenko
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Evgeniy Didin @ 2018-02-28 11:53 UTC (permalink / raw)
To: linux-snps-arc
In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") and
commit 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
have been made changes which cause multiply overflow for 32-bit systems.
The broken timeout calculations leads to unexpected ETIMEDOUT errors and
causes stacktrace splat (such as below) during normal data exchange
with SD-card.
| Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
| - Info: Finished target initialization.
| mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
| 0x900, card status 0x0
DIV_ROUND_UP_ULL helps to escape usage of __udivdi3() from libgcc and so
code gets compiled on all 32-bit platforms as opposed to usage
of DIV_ROUND_UP when we may only compile stuff on a very few arches.
Lets cast this multiply to u64 type which prevents overflow.
Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
Reported-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com> # ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
Fixes: 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
CC: Alexey Brodkin <abrodkin at synopsys.com>
CC: Eugeniy Paltsev <paltsev at synopsys.com>
CC: Douglas Anderson <dianders at chromium.org>
CC: Ulf Hansson <ulf.hansson at linaro.org>
CC: Andy Shevchenko <andy.shevchenko at gmail.com>
CC: Jisheng Zhang <Jisheng.Zhang at synaptics.com>
CC: Shawn Lin <shawn.lin at rock-chips.com>
CC: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
CC: linux-kernel at vger.kernel.org
CC: linux-snps-arc at lists.infradead.org
Cc: <stable at vger.kernel.org>
---
Changes since v3:
-Switch DIV_ROUND_UP macro to DIV_ROUND_UP_ULL
-Make one patch from two patches
-Modify commit message
Changes sinve v2:
-add fix for cto_ms
Changes since v1:
-uint64_t switched to u64
drivers/mmc/host/dw_mmc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 0aa39975f33b..cba534d4c81b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -409,7 +409,9 @@ static inline void dw_mci_set_cto(struct dw_mci *host)
cto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
if (cto_div == 0)
cto_div = 1;
- cto_ms = DIV_ROUND_UP(MSEC_PER_SEC * cto_clks * cto_div, host->bus_hz);
+
+ cto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * cto_clks * cto_div,
+ host->bus_hz);
/* add a bit spare time */
cto_ms += 10;
@@ -1944,8 +1946,9 @@ static void dw_mci_set_drto(struct dw_mci *host)
drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
if (drto_div == 0)
drto_div = 1;
- drto_ms = DIV_ROUND_UP(MSEC_PER_SEC * drto_clks * drto_div,
- host->bus_hz);
+
+ drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div,
+ host->bus_hz);
/* add a bit spare time */
drto_ms += 10;
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems
2018-02-28 11:53 [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems Evgeniy Didin
@ 2018-02-28 15:47 ` Andy Shevchenko
2018-02-28 16:16 ` Doug Anderson
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-02-28 15:47 UTC (permalink / raw)
To: linux-snps-arc
On Wed, Feb 28, 2018 at 1:53 PM, Evgeniy Didin
<Evgeniy.Didin@synopsys.com> wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") and
> commit 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
> have been made changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations leads to unexpected ETIMEDOUT errors and
> causes stacktrace splat (such as below) during normal data exchange
> with SD-card.
>
> | Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
> | - Info: Finished target initialization.
> | mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
> | 0x900, card status 0x0
>
> DIV_ROUND_UP_ULL helps to escape usage of __udivdi3() from libgcc and so
> code gets compiled on all 32-bit platforms as opposed to usage
> of DIV_ROUND_UP when we may only compile stuff on a very few arches.
>
> Lets cast this multiply to u64 type which prevents overflow.
>
FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko at gmail.com>
> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> Reported-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com> # ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
> Fixes: 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
>
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
>
> CC: Alexey Brodkin <abrodkin at synopsys.com>
> CC: Eugeniy Paltsev <paltsev at synopsys.com>
> CC: Douglas Anderson <dianders at chromium.org>
> CC: Ulf Hansson <ulf.hansson at linaro.org>
> CC: Andy Shevchenko <andy.shevchenko at gmail.com>
> CC: Jisheng Zhang <Jisheng.Zhang at synaptics.com>
> CC: Shawn Lin <shawn.lin at rock-chips.com>
> CC: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> CC: linux-kernel at vger.kernel.org
> CC: linux-snps-arc at lists.infradead.org
> Cc: <stable at vger.kernel.org>
> ---
> Changes since v3:
> -Switch DIV_ROUND_UP macro to DIV_ROUND_UP_ULL
> -Make one patch from two patches
> -Modify commit message
>
> Changes sinve v2:
> -add fix for cto_ms
>
> Changes since v1:
> -uint64_t switched to u64
>
> drivers/mmc/host/dw_mmc.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..cba534d4c81b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -409,7 +409,9 @@ static inline void dw_mci_set_cto(struct dw_mci *host)
> cto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (cto_div == 0)
> cto_div = 1;
> - cto_ms = DIV_ROUND_UP(MSEC_PER_SEC * cto_clks * cto_div, host->bus_hz);
> +
> + cto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * cto_clks * cto_div,
> + host->bus_hz);
>
> /* add a bit spare time */
> cto_ms += 10;
> @@ -1944,8 +1946,9 @@ static void dw_mci_set_drto(struct dw_mci *host)
> drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (drto_div == 0)
> drto_div = 1;
> - drto_ms = DIV_ROUND_UP(MSEC_PER_SEC * drto_clks * drto_div,
> - host->bus_hz);
> +
> + drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div,
> + host->bus_hz);
>
> /* add a bit spare time */
> drto_ms += 10;
> --
> 2.11.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems
2018-02-28 11:53 [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems Evgeniy Didin
2018-02-28 15:47 ` Andy Shevchenko
@ 2018-02-28 16:16 ` Doug Anderson
2018-03-01 1:20 ` Shawn Lin
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Doug Anderson @ 2018-02-28 16:16 UTC (permalink / raw)
To: linux-snps-arc
Hi,
On Wed, Feb 28, 2018 at 3:53 AM, Evgeniy Didin
<Evgeniy.Didin@synopsys.com> wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") and
> commit 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
> have been made changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations leads to unexpected ETIMEDOUT errors and
> causes stacktrace splat (such as below) during normal data exchange
> with SD-card.
>
> | Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
> | - Info: Finished target initialization.
> | mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
> | 0x900, card status 0x0
>
> DIV_ROUND_UP_ULL helps to escape usage of __udivdi3() from libgcc and so
> code gets compiled on all 32-bit platforms as opposed to usage
> of DIV_ROUND_UP when we may only compile stuff on a very few arches.
>
> Lets cast this multiply to u64 type which prevents overflow.
>
> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> Reported-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com> # ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
> Fixes: 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
>
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
>
> CC: Alexey Brodkin <abrodkin at synopsys.com>
> CC: Eugeniy Paltsev <paltsev at synopsys.com>
> CC: Douglas Anderson <dianders at chromium.org>
> CC: Ulf Hansson <ulf.hansson at linaro.org>
> CC: Andy Shevchenko <andy.shevchenko at gmail.com>
> CC: Jisheng Zhang <Jisheng.Zhang at synaptics.com>
> CC: Shawn Lin <shawn.lin at rock-chips.com>
> CC: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> CC: linux-kernel at vger.kernel.org
> CC: linux-snps-arc at lists.infradead.org
> Cc: <stable at vger.kernel.org>
> ---
> Changes since v3:
> -Switch DIV_ROUND_UP macro to DIV_ROUND_UP_ULL
> -Make one patch from two patches
> -Modify commit message
>
> Changes sinve v2:
> -add fix for cto_ms
>
> Changes since v1:
> -uint64_t switched to u64
>
> drivers/mmc/host/dw_mmc.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
FYI that your messages keep ending up in my spam folder and I just see
other people's replies. Not exactly sure why... Note also that your
patch really doesn't have all the tags in the right places. All the
Tags including the Signed-off-by and Cc are supposed to be squashed in
one paragraph ...and I believe "CC" is canonically supposed to be
"Cc". Presumably the maintainer can fix this up when applying, but
please try to do better in the future. Speaking of which, you didn't
include the dw_mmc maintainer (Jaehoon Chung) in your post? That
seems not so great unless you know something that get_maintainers
doesn't know about who will apply this patch.
$ ./scripts/get_maintainer.pl -f drivers/mmc/host/dw_mmc.c
Jaehoon Chung <jh80.chung at samsung.com> (maintainer:SYNOPSYS DESIGNWARE
MMC/SD/SDIO DRIVER)
As far as the actual patch, at first I thought maybe we could avoid
the 64-bit math and just pre-divide bus_hz by MSEC_PER_SEC (just like
used to happen before my change), but I think there's still a chance
of overflow, right? Because, for instance:
drto_clks can be a full 24 bits
drto_div can be effectively 9 bits
...we could do further tricks (further rounding up the result), but it
seems like just doing the 64-bit math is fine too unless someone has
any strong objections...
Reviewed-by: Douglas Anderson <dianders at chromium.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems
2018-02-28 11:53 [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems Evgeniy Didin
2018-02-28 15:47 ` Andy Shevchenko
2018-02-28 16:16 ` Doug Anderson
@ 2018-03-01 1:20 ` Shawn Lin
2018-03-01 5:57 ` Jisheng Zhang
2018-03-05 8:46 ` Ulf Hansson
4 siblings, 0 replies; 7+ messages in thread
From: Shawn Lin @ 2018-03-01 1:20 UTC (permalink / raw)
To: linux-snps-arc
[+ Jaehoon]
On 2018/2/28 19:53, Evgeniy Didin wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") and
> commit 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
> have been made changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations leads to unexpected ETIMEDOUT errors and
> causes stacktrace splat (such as below) during normal data exchange
> with SD-card.
>
Reviewed-by: Shawn Lin <shawn.lin at rock-chips.com>
> | Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
> | - Info: Finished target initialization.
> | mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
> | 0x900, card status 0x0
>
> DIV_ROUND_UP_ULL helps to escape usage of __udivdi3() from libgcc and so
> code gets compiled on all 32-bit platforms as opposed to usage
> of DIV_ROUND_UP when we may only compile stuff on a very few arches.
>
> Lets cast this multiply to u64 type which prevents overflow.
>
> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> Reported-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com> # ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
> Fixes: 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
>
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
>
> CC: Alexey Brodkin <abrodkin at synopsys.com>
> CC: Eugeniy Paltsev <paltsev at synopsys.com>
> CC: Douglas Anderson <dianders at chromium.org>
> CC: Ulf Hansson <ulf.hansson at linaro.org>
> CC: Andy Shevchenko <andy.shevchenko at gmail.com>
> CC: Jisheng Zhang <Jisheng.Zhang at synaptics.com>
> CC: Shawn Lin <shawn.lin at rock-chips.com>
> CC: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> CC: linux-kernel at vger.kernel.org
> CC: linux-snps-arc at lists.infradead.org
> Cc: <stable at vger.kernel.org>
> ---
> Changes since v3:
> -Switch DIV_ROUND_UP macro to DIV_ROUND_UP_ULL
> -Make one patch from two patches
> -Modify commit message
>
> Changes sinve v2:
> -add fix for cto_ms
>
> Changes since v1:
> -uint64_t switched to u64
>
> drivers/mmc/host/dw_mmc.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..cba534d4c81b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -409,7 +409,9 @@ static inline void dw_mci_set_cto(struct dw_mci *host)
> cto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (cto_div == 0)
> cto_div = 1;
> - cto_ms = DIV_ROUND_UP(MSEC_PER_SEC * cto_clks * cto_div, host->bus_hz);
> +
> + cto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * cto_clks * cto_div,
> + host->bus_hz);
>
> /* add a bit spare time */
> cto_ms += 10;
> @@ -1944,8 +1946,9 @@ static void dw_mci_set_drto(struct dw_mci *host)
> drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (drto_div == 0)
> drto_div = 1;
> - drto_ms = DIV_ROUND_UP(MSEC_PER_SEC * drto_clks * drto_div,
> - host->bus_hz);
> +
> + drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div,
> + host->bus_hz);
>
> /* add a bit spare time */
> drto_ms += 10;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems
2018-02-28 11:53 [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems Evgeniy Didin
` (2 preceding siblings ...)
2018-03-01 1:20 ` Shawn Lin
@ 2018-03-01 5:57 ` Jisheng Zhang
2018-03-02 8:05 ` Jaehoon Chung
2018-03-05 8:46 ` Ulf Hansson
4 siblings, 1 reply; 7+ messages in thread
From: Jisheng Zhang @ 2018-03-01 5:57 UTC (permalink / raw)
To: linux-snps-arc
On Wed, 28 Feb 2018 14:53:18 +0300 Evgeniy Didin wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") and
> commit 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
> have been made changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations leads to unexpected ETIMEDOUT errors and
> causes stacktrace splat (such as below) during normal data exchange
> with SD-card.
>
> | Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
> | - Info: Finished target initialization.
> | mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
> | 0x900, card status 0x0
>
> DIV_ROUND_UP_ULL helps to escape usage of __udivdi3() from libgcc and so
> code gets compiled on all 32-bit platforms as opposed to usage
> of DIV_ROUND_UP when we may only compile stuff on a very few arches.
>
> Lets cast this multiply to u64 type which prevents overflow.
Reviewed-by: Jisheng Zhang <Jisheng.Zhang at synaptics.com>
>
> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> Reported-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com> # ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
> Fixes: 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
>
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
>
> CC: Alexey Brodkin <abrodkin at synopsys.com>
> CC: Eugeniy Paltsev <paltsev at synopsys.com>
> CC: Douglas Anderson <dianders at chromium.org>
> CC: Ulf Hansson <ulf.hansson at linaro.org>
> CC: Andy Shevchenko <andy.shevchenko at gmail.com>
> CC: Jisheng Zhang <Jisheng.Zhang at synaptics.com>
> CC: Shawn Lin <shawn.lin at rock-chips.com>
> CC: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> CC: linux-kernel at vger.kernel.org
> CC: linux-snps-arc at lists.infradead.org
> Cc: <stable at vger.kernel.org>
> ---
> Changes since v3:
> -Switch DIV_ROUND_UP macro to DIV_ROUND_UP_ULL
> -Make one patch from two patches
> -Modify commit message
>
> Changes sinve v2:
> -add fix for cto_ms
>
> Changes since v1:
> -uint64_t switched to u64
>
> drivers/mmc/host/dw_mmc.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..cba534d4c81b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -409,7 +409,9 @@ static inline void dw_mci_set_cto(struct dw_mci *host)
> cto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (cto_div == 0)
> cto_div = 1;
> - cto_ms = DIV_ROUND_UP(MSEC_PER_SEC * cto_clks * cto_div, host->bus_hz);
> +
> + cto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * cto_clks * cto_div,
> + host->bus_hz);
>
> /* add a bit spare time */
> cto_ms += 10;
> @@ -1944,8 +1946,9 @@ static void dw_mci_set_drto(struct dw_mci *host)
> drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (drto_div == 0)
> drto_div = 1;
> - drto_ms = DIV_ROUND_UP(MSEC_PER_SEC * drto_clks * drto_div,
> - host->bus_hz);
> +
> + drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div,
> + host->bus_hz);
>
> /* add a bit spare time */
> drto_ms += 10;
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems
2018-03-01 5:57 ` Jisheng Zhang
@ 2018-03-02 8:05 ` Jaehoon Chung
0 siblings, 0 replies; 7+ messages in thread
From: Jaehoon Chung @ 2018-03-02 8:05 UTC (permalink / raw)
To: linux-snps-arc
On 03/01/2018 02:57 PM, Jisheng Zhang wrote:
> On Wed, 28 Feb 2018 14:53:18 +0300 Evgeniy Didin wrote:
>
>> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") and
>> commit 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
>> have been made changes which cause multiply overflow for 32-bit systems.
>> The broken timeout calculations leads to unexpected ETIMEDOUT errors and
>> causes stacktrace splat (such as below) during normal data exchange
>> with SD-card.
>>
>> | Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
>> | - Info: Finished target initialization.
>> | mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
>> | 0x900, card status 0x0
>>
>> DIV_ROUND_UP_ULL helps to escape usage of __udivdi3() from libgcc and so
>> code gets compiled on all 32-bit platforms as opposed to usage
>> of DIV_ROUND_UP when we may only compile stuff on a very few arches.
>>
>> Lets cast this multiply to u64 type which prevents overflow.
>
> Reviewed-by: Jisheng Zhang <Jisheng.Zhang at synaptics.com>
Acked-by: Jaehoon Chung <jh80.chung at samsung.com>
Best Regards,
Jaehoon Chung
>
>>
>> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
>> Reported-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com> # ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
>> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
>> Fixes: 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
>>
>> Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
>>
>> CC: Alexey Brodkin <abrodkin at synopsys.com>
>> CC: Eugeniy Paltsev <paltsev at synopsys.com>
>> CC: Douglas Anderson <dianders at chromium.org>
>> CC: Ulf Hansson <ulf.hansson at linaro.org>
>> CC: Andy Shevchenko <andy.shevchenko at gmail.com>
>> CC: Jisheng Zhang <Jisheng.Zhang at synaptics.com>
>> CC: Shawn Lin <shawn.lin at rock-chips.com>
>> CC: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
>> CC: linux-kernel at vger.kernel.org
>> CC: linux-snps-arc at lists.infradead.org
>> Cc: <stable at vger.kernel.org>
>> ---
>> Changes since v3:
>> -Switch DIV_ROUND_UP macro to DIV_ROUND_UP_ULL
>> -Make one patch from two patches
>> -Modify commit message
>>
>> Changes sinve v2:
>> -add fix for cto_ms
>>
>> Changes since v1:
>> -uint64_t switched to u64
>>
>> drivers/mmc/host/dw_mmc.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 0aa39975f33b..cba534d4c81b 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -409,7 +409,9 @@ static inline void dw_mci_set_cto(struct dw_mci *host)
>> cto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
>> if (cto_div == 0)
>> cto_div = 1;
>> - cto_ms = DIV_ROUND_UP(MSEC_PER_SEC * cto_clks * cto_div, host->bus_hz);
>> +
>> + cto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * cto_clks * cto_div,
>> + host->bus_hz);
>>
>> /* add a bit spare time */
>> cto_ms += 10;
>> @@ -1944,8 +1946,9 @@ static void dw_mci_set_drto(struct dw_mci *host)
>> drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
>> if (drto_div == 0)
>> drto_div = 1;
>> - drto_ms = DIV_ROUND_UP(MSEC_PER_SEC * drto_clks * drto_div,
>> - host->bus_hz);
>> +
>> + drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div,
>> + host->bus_hz);
>>
>> /* add a bit spare time */
>> drto_ms += 10;
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems
2018-02-28 11:53 [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems Evgeniy Didin
` (3 preceding siblings ...)
2018-03-01 5:57 ` Jisheng Zhang
@ 2018-03-05 8:46 ` Ulf Hansson
4 siblings, 0 replies; 7+ messages in thread
From: Ulf Hansson @ 2018-03-05 8:46 UTC (permalink / raw)
To: linux-snps-arc
On 28 February 2018@12:53, Evgeniy Didin <Evgeniy.Didin@synopsys.com> wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") and
> commit 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
> have been made changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations leads to unexpected ETIMEDOUT errors and
> causes stacktrace splat (such as below) during normal data exchange
> with SD-card.
>
> | Running : 4M-check-reassembly-tcp-cmykw2-rotatew2.out -v0 -w1
> | - Info: Finished target initialization.
> | mmcblk0: error -110 transferring data, sector 320544, nr 2048, cmd response
> | 0x900, card status 0x0
>
> DIV_ROUND_UP_ULL helps to escape usage of __udivdi3() from libgcc and so
> code gets compiled on all 32-bit platforms as opposed to usage
> of DIV_ROUND_UP when we may only compile stuff on a very few arches.
>
> Lets cast this multiply to u64 type which prevents overflow.
>
> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> Reported-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com> # ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
> Fixes: 4c2357f57dd5 ("mmc: dw_mmc: Fix the CTO timeout calculation")
>
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin at synopsys.com>
>
> CC: Alexey Brodkin <abrodkin at synopsys.com>
> CC: Eugeniy Paltsev <paltsev at synopsys.com>
> CC: Douglas Anderson <dianders at chromium.org>
> CC: Ulf Hansson <ulf.hansson at linaro.org>
> CC: Andy Shevchenko <andy.shevchenko at gmail.com>
> CC: Jisheng Zhang <Jisheng.Zhang at synaptics.com>
> CC: Shawn Lin <shawn.lin at rock-chips.com>
> CC: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> CC: linux-kernel at vger.kernel.org
> CC: linux-snps-arc at lists.infradead.org
> Cc: <stable at vger.kernel.org>
Thanks, applied for fixes! I took the liberty to remove this cc-list
and did some additional minor cleanups of the change log.
Kind regards
Uffe
> ---
> Changes since v3:
> -Switch DIV_ROUND_UP macro to DIV_ROUND_UP_ULL
> -Make one patch from two patches
> -Modify commit message
>
> Changes sinve v2:
> -add fix for cto_ms
>
> Changes since v1:
> -uint64_t switched to u64
>
> drivers/mmc/host/dw_mmc.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..cba534d4c81b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -409,7 +409,9 @@ static inline void dw_mci_set_cto(struct dw_mci *host)
> cto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (cto_div == 0)
> cto_div = 1;
> - cto_ms = DIV_ROUND_UP(MSEC_PER_SEC * cto_clks * cto_div, host->bus_hz);
> +
> + cto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * cto_clks * cto_div,
> + host->bus_hz);
>
> /* add a bit spare time */
> cto_ms += 10;
> @@ -1944,8 +1946,9 @@ static void dw_mci_set_drto(struct dw_mci *host)
> drto_div = (mci_readl(host, CLKDIV) & 0xff) * 2;
> if (drto_div == 0)
> drto_div = 1;
> - drto_ms = DIV_ROUND_UP(MSEC_PER_SEC * drto_clks * drto_div,
> - host->bus_hz);
> +
> + drto_ms = DIV_ROUND_UP_ULL((u64)MSEC_PER_SEC * drto_clks * drto_div,
> + host->bus_hz);
>
> /* add a bit spare time */
> drto_ms += 10;
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-05 8:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-28 11:53 [PATCH v4] mmc: dw_mmc: Fix the DTO/CTO timeout overflow calculation for 32-bit systems Evgeniy Didin
2018-02-28 15:47 ` Andy Shevchenko
2018-02-28 16:16 ` Doug Anderson
2018-03-01 1:20 ` Shawn Lin
2018-03-01 5:57 ` Jisheng Zhang
2018-03-02 8:05 ` Jaehoon Chung
2018-03-05 8:46 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox