All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 13:34 ` Evgeniy Didin
  0 siblings, 0 replies; 18+ messages in thread
From: Evgeniy Didin @ 2018-02-22 13:34 UTC (permalink / raw)
  To: linux-mmc
  Cc: Alexey Brodkin, Eugeniy Paltsev, Douglas Anderson, Ulf Hansson,
	linux-kernel, linux-snps-arc, stable, Vineet Gupta

In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
changes which cause multiply overflow for 32-bit systems.
The broken timeout calculations caused false interrupt latency warnings
and stacktrace splat (such as below) when accessing the 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
| mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
| 396825HZ div = 63)
| mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
|  25000000HZ div = 1)
| ------------[ cut here ]------------
| softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
| exited with 00000100?
| WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
| Modules linked in:
| CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
|
| Stack Trace:
|  arc_unwind_core.constprop.1+0xd0/0xf4
|  dump_stack+0x68/0x80
|  warn_slowpath_null+0x4e/0xec
|  sg_miter_next+0x28/0x20c
|  dw_mci_read_data_pio+0x44/0x190
|  dw_mmc f000a000.mmc: Unexpected interrupt latency
|   dw_mci_interrupt+0x3ee/0x530
|  __handle_irq_event_percpu+0x56/0x150
|  handle_irq_event+0x34/0x78
|  handle_level_irq+0x8e/0x120
|  generic_handle_irq+0x1c/0x2c
|  idu_cascade_isr+0x30/0x6c
|  __handle_domain_irq+0x72/0xc8
|  ret_from_exception+0x0/0x8
|---[ end trace 2a58c9af6c25fe51 ]---

Lets cast this multiply to u64 type which prevents overflow.

Tested-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files

Signed-off-by: Evgeniy Didin <Evgeniy.Didin@synopsys.com>

CC: Alexey Brodkin <abrodkin@synopsys.com>
CC: Eugeniy Paltsev <paltsev@synopsys.com>
CC: Douglas Anderson <dianders@chromium.org>
CC: Ulf Hansson <ulf.hansson@linaro.org>
CC: linux-kernel@vger.kernel.org
CC: linux-snps-arc@lists.infradead.org
Cc: <stable@vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation
---
Changes since v1:
-uint64_t switched to u64
-changed commit message
 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 0aa39975f33b..1a0b9751c67c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1944,7 +1944,7 @@ 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,
+	drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
 			       host->bus_hz);
 
 	/* add a bit spare time */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 13:34 ` Evgeniy Didin
  0 siblings, 0 replies; 18+ messages in thread
From: Evgeniy Didin @ 2018-02-22 13:34 UTC (permalink / raw)
  To: linux-mmc
  Cc: Ulf Hansson, Vineet Gupta, Alexey Brodkin, Douglas Anderson,
	stable, linux-kernel, linux-snps-arc, Eugeniy Paltsev

In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
changes which cause multiply overflow for 32-bit systems.
The broken timeout calculations caused false interrupt latency warnings
and stacktrace splat (such as below) when accessing the 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
| mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
| 396825HZ div = 63)
| mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
|  25000000HZ div = 1)
| ------------[ cut here ]------------
| softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
| exited with 00000100?
| WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
| Modules linked in:
| CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
|
| Stack Trace:
|  arc_unwind_core.constprop.1+0xd0/0xf4
|  dump_stack+0x68/0x80
|  warn_slowpath_null+0x4e/0xec
|  sg_miter_next+0x28/0x20c
|  dw_mci_read_data_pio+0x44/0x190
|  dw_mmc f000a000.mmc: Unexpected interrupt latency
|   dw_mci_interrupt+0x3ee/0x530
|  __handle_irq_event_percpu+0x56/0x150
|  handle_irq_event+0x34/0x78
|  handle_level_irq+0x8e/0x120
|  generic_handle_irq+0x1c/0x2c
|  idu_cascade_isr+0x30/0x6c
|  __handle_domain_irq+0x72/0xc8
|  ret_from_exception+0x0/0x8
|---[ end trace 2a58c9af6c25fe51 ]---

Lets cast this multiply to u64 type which prevents overflow.

Tested-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files

Signed-off-by: Evgeniy Didin <Evgeniy.Didin@synopsys.com>

CC: Alexey Brodkin <abrodkin@synopsys.com>
CC: Eugeniy Paltsev <paltsev@synopsys.com>
CC: Douglas Anderson <dianders@chromium.org>
CC: Ulf Hansson <ulf.hansson@linaro.org>
CC: linux-kernel@vger.kernel.org
CC: linux-snps-arc@lists.infradead.org
Cc: <stable@vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation
---
Changes since v1:
-uint64_t switched to u64
-changed commit message
 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 0aa39975f33b..1a0b9751c67c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1944,7 +1944,7 @@ 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,
+	drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
 			       host->bus_hz);
 
 	/* add a bit spare time */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 13:34 ` Evgeniy Didin
  0 siblings, 0 replies; 18+ messages in thread
From: Evgeniy Didin @ 2018-02-22 13:34 UTC (permalink / raw)
  To: linux-snps-arc

In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
changes which cause multiply overflow for 32-bit systems.
The broken timeout calculations caused false interrupt latency warnings
and stacktrace splat (such as below) when accessing the 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
| mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
| 396825HZ div = 63)
| mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
|  25000000HZ div = 1)
| ------------[ cut here ]------------
| softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
| exited with 00000100?
| WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
| Modules linked in:
| CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
|
| Stack Trace:
|  arc_unwind_core.constprop.1+0xd0/0xf4
|  dump_stack+0x68/0x80
|  warn_slowpath_null+0x4e/0xec
|  sg_miter_next+0x28/0x20c
|  dw_mci_read_data_pio+0x44/0x190
|  dw_mmc f000a000.mmc: Unexpected interrupt latency
|   dw_mci_interrupt+0x3ee/0x530
|  __handle_irq_event_percpu+0x56/0x150
|  handle_irq_event+0x34/0x78
|  handle_level_irq+0x8e/0x120
|  generic_handle_irq+0x1c/0x2c
|  idu_cascade_isr+0x30/0x6c
|  __handle_domain_irq+0x72/0xc8
|  ret_from_exception+0x0/0x8
|---[ end trace 2a58c9af6c25fe51 ]---

Lets cast this multiply to u64 type which prevents overflow.

Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files

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: linux-kernel at vger.kernel.org
CC: linux-snps-arc at lists.infradead.org
Cc: <stable at vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation
---
Changes since v1:
-uint64_t switched to u64
-changed commit message
 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 0aa39975f33b..1a0b9751c67c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1944,7 +1944,7 @@ 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,
+	drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
 			       host->bus_hz);
 
 	/* add a bit spare time */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
  2018-02-22 13:34 ` Evgeniy Didin
  (?)
@ 2018-02-22 15:28   ` Shawn Lin
  -1 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2018-02-22 15:28 UTC (permalink / raw)
  To: Evgeniy Didin, linux-mmc
  Cc: shawn.lin, Alexey Brodkin, Eugeniy Paltsev, Douglas Anderson,
	Ulf Hansson, linux-snps-arc, stable, Vineet Gupta, Jaehoon Chung

[+ Jaehoon]

On 2018/2/22 21:34, Evgeniy Didin wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
> changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations caused false interrupt latency warnings
> and stacktrace splat (such as below) when accessing the 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
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
> | 396825HZ div = 63)
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
> |  25000000HZ div = 1)
> | ------------[ cut here ]------------
> | softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
> | exited with 00000100?
> | WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
> | Modules linked in:
> | CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
> |
> | Stack Trace:
> |  arc_unwind_core.constprop.1+0xd0/0xf4
> |  dump_stack+0x68/0x80
> |  warn_slowpath_null+0x4e/0xec
> |  sg_miter_next+0x28/0x20c
> |  dw_mci_read_data_pio+0x44/0x190
> |  dw_mmc f000a000.mmc: Unexpected interrupt latency

I think we tested SD cards but the main reason we missed
this is that we don't use pio mode since dw_mmc decides
the transfer mode via HCON register but we don't have one
platform at hand then to do that. Given the data-transfer-over
interrupt should come after the data hit the RAM, so pio mode
could probably consume more time than IDMAC.

> |   dw_mci_interrupt+0x3ee/0x530
> |  __handle_irq_event_percpu+0x56/0x150
> |  handle_irq_event+0x34/0x78
> |  handle_level_irq+0x8e/0x120
> |  generic_handle_irq+0x1c/0x2c
> |  idu_cascade_isr+0x30/0x6c
> |  __handle_domain_irq+0x72/0xc8
> |  ret_from_exception+0x0/0x8
> |---[ end trace 2a58c9af6c25fe51 ]---
> 
> Lets cast this multiply to u64 type which prevents overflow.
> 
> Tested-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> 
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin@synopsys.com>
> 
> CC: Alexey Brodkin <abrodkin@synopsys.com>
> CC: Eugeniy Paltsev <paltsev@synopsys.com>
> CC: Douglas Anderson <dianders@chromium.org>
> CC: Ulf Hansson <ulf.hansson@linaro.org>
> CC: linux-kernel@vger.kernel.org
> CC: linux-snps-arc@lists.infradead.org
> Cc: <stable@vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation

The fix looks good to me, but the tags should be improved a bit:

Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
Cc: <stable@vger.kernel.org>

Otherwise,
Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

BTW, would you mind cooking another patch for fixing cto_ms case?


> ---
> Changes since v1:
> -uint64_t switched to u64
> -changed commit message
>   drivers/mmc/host/dw_mmc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..1a0b9751c67c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1944,7 +1944,7 @@ 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,
> +	drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
>   			       host->bus_hz);
>   
>   	/* add a bit spare time */
> 


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 15:28   ` Shawn Lin
  0 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2018-02-22 15:28 UTC (permalink / raw)
  To: Evgeniy Didin, linux-mmc
  Cc: Ulf Hansson, shawn.lin, Alexey Brodkin, Douglas Anderson, stable,
	Jaehoon Chung, linux-snps-arc, Eugeniy Paltsev, Vineet Gupta

[+ Jaehoon]

On 2018/2/22 21:34, Evgeniy Didin wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
> changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations caused false interrupt latency warnings
> and stacktrace splat (such as below) when accessing the 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
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
> | 396825HZ div = 63)
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
> |  25000000HZ div = 1)
> | ------------[ cut here ]------------
> | softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
> | exited with 00000100?
> | WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
> | Modules linked in:
> | CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
> |
> | Stack Trace:
> |  arc_unwind_core.constprop.1+0xd0/0xf4
> |  dump_stack+0x68/0x80
> |  warn_slowpath_null+0x4e/0xec
> |  sg_miter_next+0x28/0x20c
> |  dw_mci_read_data_pio+0x44/0x190
> |  dw_mmc f000a000.mmc: Unexpected interrupt latency

I think we tested SD cards but the main reason we missed
this is that we don't use pio mode since dw_mmc decides
the transfer mode via HCON register but we don't have one
platform at hand then to do that. Given the data-transfer-over
interrupt should come after the data hit the RAM, so pio mode
could probably consume more time than IDMAC.

> |   dw_mci_interrupt+0x3ee/0x530
> |  __handle_irq_event_percpu+0x56/0x150
> |  handle_irq_event+0x34/0x78
> |  handle_level_irq+0x8e/0x120
> |  generic_handle_irq+0x1c/0x2c
> |  idu_cascade_isr+0x30/0x6c
> |  __handle_domain_irq+0x72/0xc8
> |  ret_from_exception+0x0/0x8
> |---[ end trace 2a58c9af6c25fe51 ]---
> 
> Lets cast this multiply to u64 type which prevents overflow.
> 
> Tested-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> 
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin@synopsys.com>
> 
> CC: Alexey Brodkin <abrodkin@synopsys.com>
> CC: Eugeniy Paltsev <paltsev@synopsys.com>
> CC: Douglas Anderson <dianders@chromium.org>
> CC: Ulf Hansson <ulf.hansson@linaro.org>
> CC: linux-kernel@vger.kernel.org
> CC: linux-snps-arc@lists.infradead.org
> Cc: <stable@vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation

The fix looks good to me, but the tags should be improved a bit:

Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
Cc: <stable@vger.kernel.org>

Otherwise,
Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

BTW, would you mind cooking another patch for fixing cto_ms case?


> ---
> Changes since v1:
> -uint64_t switched to u64
> -changed commit message
>   drivers/mmc/host/dw_mmc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..1a0b9751c67c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1944,7 +1944,7 @@ 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,
> +	drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
>   			       host->bus_hz);
>   
>   	/* add a bit spare time */
> 


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 15:28   ` Shawn Lin
  0 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2018-02-22 15:28 UTC (permalink / raw)
  To: linux-snps-arc

[+ Jaehoon]

On 2018/2/22 21:34, Evgeniy Didin wrote:
> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
> changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations caused false interrupt latency warnings
> and stacktrace splat (such as below) when accessing the 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
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
> | 396825HZ div = 63)
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
> |  25000000HZ div = 1)
> | ------------[ cut here ]------------
> | softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
> | exited with 00000100?
> | WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
> | Modules linked in:
> | CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
> |
> | Stack Trace:
> |  arc_unwind_core.constprop.1+0xd0/0xf4
> |  dump_stack+0x68/0x80
> |  warn_slowpath_null+0x4e/0xec
> |  sg_miter_next+0x28/0x20c
> |  dw_mci_read_data_pio+0x44/0x190
> |  dw_mmc f000a000.mmc: Unexpected interrupt latency

I think we tested SD cards but the main reason we missed
this is that we don't use pio mode since dw_mmc decides
the transfer mode via HCON register but we don't have one
platform at hand then to do that. Given the data-transfer-over
interrupt should come after the data hit the RAM, so pio mode
could probably consume more time than IDMAC.

> |   dw_mci_interrupt+0x3ee/0x530
> |  __handle_irq_event_percpu+0x56/0x150
> |  handle_irq_event+0x34/0x78
> |  handle_level_irq+0x8e/0x120
> |  generic_handle_irq+0x1c/0x2c
> |  idu_cascade_isr+0x30/0x6c
> |  __handle_domain_irq+0x72/0xc8
> |  ret_from_exception+0x0/0x8
> |---[ end trace 2a58c9af6c25fe51 ]---
> 
> Lets cast this multiply to u64 type which prevents overflow.
> 
> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> 
> 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: linux-kernel at vger.kernel.org
> CC: linux-snps-arc at lists.infradead.org
> Cc: <stable at vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation

The fix looks good to me, but the tags should be improved a bit:

Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")
Cc: <stable at vger.kernel.org>

Otherwise,
Reviewed-by: Shawn Lin <shawn.lin at rock-chips.com>

BTW, would you mind cooking another patch for fixing cto_ms case?


> ---
> Changes since v1:
> -uint64_t switched to u64
> -changed commit message
>   drivers/mmc/host/dw_mmc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..1a0b9751c67c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1944,7 +1944,7 @@ 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,
> +	drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
>   			       host->bus_hz);
>   
>   	/* add a bit spare time */
> 


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
  2018-02-22 15:28   ` Shawn Lin
  (?)
@ 2018-02-22 16:03     ` Alexey Brodkin
  -1 siblings, 0 replies; 18+ messages in thread
From: Alexey Brodkin @ 2018-02-22 16:03 UTC (permalink / raw)
  To: Evgeniy.Didin
  Cc: jh80.chung, Alexey.Brodkin, dianders, linux-mmc, Vineet.Gupta1,
	Eugeniy.Paltsev, linux-snps-arc, stable, shawn.lin, ulf.hansson

Hi Shawn,

On Thu, 2018-02-22 at 23:28 +0800, Shawn Lin wrote:

[snip]

> > > Stack Trace:
> > >  arc_unwind_core.constprop.1+0xd0/0xf4
> > >  dump_stack+0x68/0x80
> > >  warn_slowpath_null+0x4e/0xec
> > >  sg_miter_next+0x28/0x20c
> > >  dw_mci_read_data_pio+0x44/0x190
> > >  dw_mmc f000a000.mmc: Unexpected interrupt latency
> 
> I think we tested SD cards but the main reason we missed
> this is that we don't use pio mode since dw_mmc decides
> the transfer mode via HCON register but we don't have one
> platform at hand then to do that. Given the data-transfer-over
> interrupt should come after the data hit the RAM, so pio mode
> could probably consume more time than IDMAC.

That's really interesting.

I was under impression that we use internal DMA controller (AKA IDMAC)
on that platform (HSDK).

This is what we typically see in bootlog (this extract is taken from
4.15-r9):
--------------------------------->8--------------------------------
dw_mmc f000a000.mmc: 'num-slots' was deprecated.
dw_mmc f000a000.mmc: IDMAC supports 32-bit address mode.
dw_mmc f000a000.mmc: Using internal DMA controller.
dw_mmc f000a000.mmc: Version ID is 290a
dw_mmc f000a000.mmc: DW MMC controller at irq 12,32 bit host data width,16 deep fifo
--------------------------------->8--------------------------------

I'm not really sure how PIO mode (which stands for non-DMA mode) got used
given we have IDMAC in the hardware.

@ Evgeniy, could you please check why IDMAC is not used?

-Alexey


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 16:03     ` Alexey Brodkin
  0 siblings, 0 replies; 18+ messages in thread
From: Alexey Brodkin @ 2018-02-22 16:03 UTC (permalink / raw)
  To: Evgeniy.Didin
  Cc: ulf.hansson, Vineet.Gupta1, Alexey.Brodkin, linux-mmc, dianders,
	stable, jh80.chung, linux-snps-arc, Eugeniy.Paltsev, shawn.lin

Hi Shawn,

On Thu, 2018-02-22 at 23:28 +0800, Shawn Lin wrote:

[snip]

> > > Stack Trace:
> > >  arc_unwind_core.constprop.1+0xd0/0xf4
> > >  dump_stack+0x68/0x80
> > >  warn_slowpath_null+0x4e/0xec
> > >  sg_miter_next+0x28/0x20c
> > >  dw_mci_read_data_pio+0x44/0x190
> > >  dw_mmc f000a000.mmc: Unexpected interrupt latency
> 
> I think we tested SD cards but the main reason we missed
> this is that we don't use pio mode since dw_mmc decides
> the transfer mode via HCON register but we don't have one
> platform at hand then to do that. Given the data-transfer-over
> interrupt should come after the data hit the RAM, so pio mode
> could probably consume more time than IDMAC.

That's really interesting.

I was under impression that we use internal DMA controller (AKA IDMAC)
on that platform (HSDK).

This is what we typically see in bootlog (this extract is taken from
4.15-r9):
--------------------------------->8--------------------------------
dw_mmc f000a000.mmc: 'num-slots' was deprecated.
dw_mmc f000a000.mmc: IDMAC supports 32-bit address mode.
dw_mmc f000a000.mmc: Using internal DMA controller.
dw_mmc f000a000.mmc: Version ID is 290a
dw_mmc f000a000.mmc: DW MMC controller at irq 12,32 bit host data width,16 deep fifo
--------------------------------->8--------------------------------

I'm not really sure how PIO mode (which stands for non-DMA mode) got used
given we have IDMAC in the hardware.

@ Evgeniy, could you please check why IDMAC is not used?

-Alexey

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 16:03     ` Alexey Brodkin
  0 siblings, 0 replies; 18+ messages in thread
From: Alexey Brodkin @ 2018-02-22 16:03 UTC (permalink / raw)
  To: linux-snps-arc

Hi Shawn,

On Thu, 2018-02-22@23:28 +0800, Shawn Lin wrote:

[snip]

> > > Stack Trace:
> > >  arc_unwind_core.constprop.1+0xd0/0xf4
> > >  dump_stack+0x68/0x80
> > >  warn_slowpath_null+0x4e/0xec
> > >  sg_miter_next+0x28/0x20c
> > >  dw_mci_read_data_pio+0x44/0x190
> > >  dw_mmc f000a000.mmc: Unexpected interrupt latency
> 
> I think we tested SD cards but the main reason we missed
> this is that we don't use pio mode since dw_mmc decides
> the transfer mode via HCON register but we don't have one
> platform at hand then to do that. Given the data-transfer-over
> interrupt should come after the data hit the RAM, so pio mode
> could probably consume more time than IDMAC.

That's really interesting.

I was under impression that we use internal DMA controller (AKA IDMAC)
on that platform (HSDK).

This is what we typically see in bootlog (this extract is taken from
4.15-r9):
--------------------------------->8--------------------------------
dw_mmc f000a000.mmc: 'num-slots' was deprecated.
dw_mmc f000a000.mmc: IDMAC supports 32-bit address mode.
dw_mmc f000a000.mmc: Using internal DMA controller.
dw_mmc f000a000.mmc: Version ID is 290a
dw_mmc f000a000.mmc: DW MMC controller at irq 12,32 bit host data width,16 deep fifo
--------------------------------->8--------------------------------

I'm not really sure how PIO mode (which stands for non-DMA mode) got used
given we have IDMAC in the hardware.

@ Evgeniy, could you please check why IDMAC is not used?

-Alexey

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
  2018-02-22 16:03     ` Alexey Brodkin
  (?)
@ 2018-02-22 16:36       ` Shawn Lin
  -1 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2018-02-22 16:36 UTC (permalink / raw)
  To: Alexey Brodkin, Evgeniy.Didin
  Cc: shawn.lin, jh80.chung, dianders, linux-mmc, Vineet.Gupta1,
	Eugeniy.Paltsev, linux-snps-arc, stable, ulf.hansson

On 2018/2/23 0:03, Alexey Brodkin wrote:
> Hi Shawn,
> 
> On Thu, 2018-02-22 at 23:28 +0800, Shawn Lin wrote:
> 
> [snip]
> 
>>>> Stack Trace:
>>>>   arc_unwind_core.constprop.1+0xd0/0xf4
>>>>   dump_stack+0x68/0x80
>>>>   warn_slowpath_null+0x4e/0xec
>>>>   sg_miter_next+0x28/0x20c
>>>>   dw_mci_read_data_pio+0x44/0x190
>>>>   dw_mmc f000a000.mmc: Unexpected interrupt latency
>>
>> I think we tested SD cards but the main reason we missed
>> this is that we don't use pio mode since dw_mmc decides
>> the transfer mode via HCON register but we don't have one
>> platform at hand then to do that. Given the data-transfer-over
>> interrupt should come after the data hit the RAM, so pio mode
>> could probably consume more time than IDMAC.
> 
> That's really interesting.
> 
> I was under impression that we use internal DMA controller (AKA IDMAC)
> on that platform (HSDK).
> 
> This is what we typically see in bootlog (this extract is taken from
> 4.15-r9):
> --------------------------------->8--------------------------------
> dw_mmc f000a000.mmc: 'num-slots' was deprecated.
> dw_mmc f000a000.mmc: IDMAC supports 32-bit address mode.
> dw_mmc f000a000.mmc: Using internal DMA controller.
> dw_mmc f000a000.mmc: Version ID is 290a
> dw_mmc f000a000.mmc: DW MMC controller at irq 12,32 bit host data width,16 deep fifo
> --------------------------------->8--------------------------------
> 
> I'm not really sure how PIO mode (which stands for non-DMA mode) got used
> given we have IDMAC in the hardware.

There is a DW_MCI_DMA_THRESHOLD to decides whether we use IDMA or pio
per request, since pio probably do fast than IDMA which need setup the
decriptor list if the request size is small. But that never happen for
this case as the V1 commit log said "copying file from mmc" which means
your minimal data block size is 512 anyway. So the only reasonable guess
is that just happend when initializing the card rather then copying the 
file..

> 
> @ Evgeniy, could you please check why IDMAC is not used?
> 
> -Alexey
> 


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 16:36       ` Shawn Lin
  0 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2018-02-22 16:36 UTC (permalink / raw)
  To: Alexey Brodkin, Evgeniy.Didin
  Cc: ulf.hansson, shawn.lin, linux-mmc, dianders, stable, jh80.chung,
	linux-snps-arc, Eugeniy.Paltsev, Vineet.Gupta1

On 2018/2/23 0:03, Alexey Brodkin wrote:
> Hi Shawn,
> 
> On Thu, 2018-02-22 at 23:28 +0800, Shawn Lin wrote:
> 
> [snip]
> 
>>>> Stack Trace:
>>>>   arc_unwind_core.constprop.1+0xd0/0xf4
>>>>   dump_stack+0x68/0x80
>>>>   warn_slowpath_null+0x4e/0xec
>>>>   sg_miter_next+0x28/0x20c
>>>>   dw_mci_read_data_pio+0x44/0x190
>>>>   dw_mmc f000a000.mmc: Unexpected interrupt latency
>>
>> I think we tested SD cards but the main reason we missed
>> this is that we don't use pio mode since dw_mmc decides
>> the transfer mode via HCON register but we don't have one
>> platform at hand then to do that. Given the data-transfer-over
>> interrupt should come after the data hit the RAM, so pio mode
>> could probably consume more time than IDMAC.
> 
> That's really interesting.
> 
> I was under impression that we use internal DMA controller (AKA IDMAC)
> on that platform (HSDK).
> 
> This is what we typically see in bootlog (this extract is taken from
> 4.15-r9):
> --------------------------------->8--------------------------------
> dw_mmc f000a000.mmc: 'num-slots' was deprecated.
> dw_mmc f000a000.mmc: IDMAC supports 32-bit address mode.
> dw_mmc f000a000.mmc: Using internal DMA controller.
> dw_mmc f000a000.mmc: Version ID is 290a
> dw_mmc f000a000.mmc: DW MMC controller at irq 12,32 bit host data width,16 deep fifo
> --------------------------------->8--------------------------------
> 
> I'm not really sure how PIO mode (which stands for non-DMA mode) got used
> given we have IDMAC in the hardware.

There is a DW_MCI_DMA_THRESHOLD to decides whether we use IDMA or pio
per request, since pio probably do fast than IDMA which need setup the
decriptor list if the request size is small. But that never happen for
this case as the V1 commit log said "copying file from mmc" which means
your minimal data block size is 512 anyway. So the only reasonable guess
is that just happend when initializing the card rather then copying the 
file..

> 
> @ Evgeniy, could you please check why IDMAC is not used?
> 
> -Alexey
> 


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 16:36       ` Shawn Lin
  0 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2018-02-22 16:36 UTC (permalink / raw)
  To: linux-snps-arc

On 2018/2/23 0:03, Alexey Brodkin wrote:
> Hi Shawn,
> 
> On Thu, 2018-02-22@23:28 +0800, Shawn Lin wrote:
> 
> [snip]
> 
>>>> Stack Trace:
>>>>   arc_unwind_core.constprop.1+0xd0/0xf4
>>>>   dump_stack+0x68/0x80
>>>>   warn_slowpath_null+0x4e/0xec
>>>>   sg_miter_next+0x28/0x20c
>>>>   dw_mci_read_data_pio+0x44/0x190
>>>>   dw_mmc f000a000.mmc: Unexpected interrupt latency
>>
>> I think we tested SD cards but the main reason we missed
>> this is that we don't use pio mode since dw_mmc decides
>> the transfer mode via HCON register but we don't have one
>> platform at hand then to do that. Given the data-transfer-over
>> interrupt should come after the data hit the RAM, so pio mode
>> could probably consume more time than IDMAC.
> 
> That's really interesting.
> 
> I was under impression that we use internal DMA controller (AKA IDMAC)
> on that platform (HSDK).
> 
> This is what we typically see in bootlog (this extract is taken from
> 4.15-r9):
> --------------------------------->8--------------------------------
> dw_mmc f000a000.mmc: 'num-slots' was deprecated.
> dw_mmc f000a000.mmc: IDMAC supports 32-bit address mode.
> dw_mmc f000a000.mmc: Using internal DMA controller.
> dw_mmc f000a000.mmc: Version ID is 290a
> dw_mmc f000a000.mmc: DW MMC controller at irq 12,32 bit host data width,16 deep fifo
> --------------------------------->8--------------------------------
> 
> I'm not really sure how PIO mode (which stands for non-DMA mode) got used
> given we have IDMAC in the hardware.

There is a DW_MCI_DMA_THRESHOLD to decides whether we use IDMA or pio
per request, since pio probably do fast than IDMA which need setup the
decriptor list if the request size is small. But that never happen for
this case as the V1 commit log said "copying file from mmc" which means
your minimal data block size is 512 anyway. So the only reasonable guess
is that just happend when initializing the card rather then copying the 
file..

> 
> @ Evgeniy, could you please check why IDMAC is not used?
> 
> -Alexey
> 


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
  2018-02-22 15:28   ` Shawn Lin
  (?)
@ 2018-02-22 17:38     ` Vineet Gupta
  -1 siblings, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2018-02-22 17:38 UTC (permalink / raw)
  To: Shawn Lin, Evgeniy Didin, linux-mmc
  Cc: Ulf Hansson, Alexey Brodkin, Douglas Anderson, stable,
	Jaehoon Chung, linux-snps-arc, Eugeniy Paltsev, Vineet Gupta

On 02/22/2018 07:28 AM, Shawn Lin wrote:
>>
>> Lets cast this multiply to u64 type which prevents overflow.
>>
>> Tested-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
>> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
>>
>> Signed-off-by: Evgeniy Didin <Evgeniy.Didin@synopsys.com>
>>
>> CC: Alexey Brodkin <abrodkin@synopsys.com>
>> CC: Eugeniy Paltsev <paltsev@synopsys.com>
>> CC: Douglas Anderson <dianders@chromium.org>
>> CC: Ulf Hansson <ulf.hansson@linaro.org>
>> CC: linux-kernel@vger.kernel.org
>> CC: linux-snps-arc@lists.infradead.org
>> Cc: <stable@vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout 
>> calculation
> 
> The fix looks good to me, but the tags should be improved a bit:
> 
> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")

Ok, then perhaps use Reported-by tag to call out when/how issue was found.

Reported-by: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files

> Cc: <stable@vger.kernel.org>
> 
> Otherwise,
> Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 17:38     ` Vineet Gupta
  0 siblings, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2018-02-22 17:38 UTC (permalink / raw)
  To: Shawn Lin, Evgeniy Didin, linux-mmc
  Cc: Ulf Hansson, Vineet Gupta, Alexey Brodkin, Douglas Anderson,
	stable, Jaehoon Chung, linux-snps-arc, Eugeniy Paltsev

On 02/22/2018 07:28 AM, Shawn Lin wrote:
>>
>> Lets cast this multiply to u64 type which prevents overflow.
>>
>> Tested-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
>> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
>>
>> Signed-off-by: Evgeniy Didin <Evgeniy.Didin@synopsys.com>
>>
>> CC: Alexey Brodkin <abrodkin@synopsys.com>
>> CC: Eugeniy Paltsev <paltsev@synopsys.com>
>> CC: Douglas Anderson <dianders@chromium.org>
>> CC: Ulf Hansson <ulf.hansson@linaro.org>
>> CC: linux-kernel@vger.kernel.org
>> CC: linux-snps-arc@lists.infradead.org
>> Cc: <stable@vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout 
>> calculation
> 
> The fix looks good to me, but the tags should be improved a bit:
> 
> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")

Ok, then perhaps use Reported-by tag to call out when/how issue was found.

Reported-by: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files

> Cc: <stable@vger.kernel.org>
> 
> Otherwise,
> Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>



_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-22 17:38     ` Vineet Gupta
  0 siblings, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2018-02-22 17:38 UTC (permalink / raw)
  To: linux-snps-arc

On 02/22/2018 07:28 AM, Shawn Lin wrote:
>>
>> Lets cast this multiply to u64 type which prevents overflow.
>>
>> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
>> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
>>
>> 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: linux-kernel at vger.kernel.org
>> CC: linux-snps-arc at lists.infradead.org
>> Cc: <stable at vger.kernel.org> #? 9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout 
>> calculation
> 
> The fix looks good to me, but the tags should be improved a bit:
> 
> Fixes: 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation")

Ok, then perhaps use Reported-by tag to call out when/how issue was found.

Reported-by: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files

> Cc: <stable at vger.kernel.org>
> 
> Otherwise,
> Reviewed-by: Shawn Lin <shawn.lin at rock-chips.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
  2018-02-22 13:34 ` Evgeniy Didin
  (?)
@ 2018-02-23  3:24   ` Jisheng Zhang
  -1 siblings, 0 replies; 18+ messages in thread
From: Jisheng Zhang @ 2018-02-23  3:24 UTC (permalink / raw)
  To: Evgeniy Didin
  Cc: linux-mmc, Alexey Brodkin, Eugeniy Paltsev, Douglas Anderson,
	Ulf Hansson, linux-kernel, linux-snps-arc, stable, Vineet Gupta

On Thu, 22 Feb 2018 16:34:18 +0300 Evgeniy Didin wrote:

> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
> changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations caused false interrupt latency warnings
> and stacktrace splat (such as below) when accessing the 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
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
> | 396825HZ div = 63)
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
> |  25000000HZ div = 1)
> | ------------[ cut here ]------------
> | softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
> | exited with 00000100?
> | WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
> | Modules linked in:
> | CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
> |
> | Stack Trace:
> |  arc_unwind_core.constprop.1+0xd0/0xf4
> |  dump_stack+0x68/0x80
> |  warn_slowpath_null+0x4e/0xec
> |  sg_miter_next+0x28/0x20c
> |  dw_mci_read_data_pio+0x44/0x190
> |  dw_mmc f000a000.mmc: Unexpected interrupt latency
> |   dw_mci_interrupt+0x3ee/0x530
> |  __handle_irq_event_percpu+0x56/0x150
> |  handle_irq_event+0x34/0x78
> |  handle_level_irq+0x8e/0x120
> |  generic_handle_irq+0x1c/0x2c
> |  idu_cascade_isr+0x30/0x6c
> |  __handle_domain_irq+0x72/0xc8
> |  ret_from_exception+0x0/0x8
> |---[ end trace 2a58c9af6c25fe51 ]---
> 
> Lets cast this multiply to u64 type which prevents overflow.
> 
> Tested-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> 
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin@synopsys.com>
> 
> CC: Alexey Brodkin <abrodkin@synopsys.com>
> CC: Eugeniy Paltsev <paltsev@synopsys.com>
> CC: Douglas Anderson <dianders@chromium.org>
> CC: Ulf Hansson <ulf.hansson@linaro.org>
> CC: linux-kernel@vger.kernel.org
> CC: linux-snps-arc@lists.infradead.org
> Cc: <stable@vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation
> ---
> Changes since v1:
> -uint64_t switched to u64
> -changed commit message
>  drivers/mmc/host/dw_mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..1a0b9751c67c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1944,7 +1944,7 @@ 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,
> +	drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
>  			       host->bus_hz);

DIV_ROUND_UP_ULL?

>  
>  	/* add a bit spare time */

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-23  3:24   ` Jisheng Zhang
  0 siblings, 0 replies; 18+ messages in thread
From: Jisheng Zhang @ 2018-02-23  3:24 UTC (permalink / raw)
  To: Evgeniy Didin
  Cc: Ulf Hansson, Vineet Gupta, Alexey Brodkin, linux-mmc,
	Douglas Anderson, stable, linux-kernel, linux-snps-arc,
	Eugeniy Paltsev

On Thu, 22 Feb 2018 16:34:18 +0300 Evgeniy Didin wrote:

> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
> changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations caused false interrupt latency warnings
> and stacktrace splat (such as below) when accessing the 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
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
> | 396825HZ div = 63)
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
> |  25000000HZ div = 1)
> | ------------[ cut here ]------------
> | softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
> | exited with 00000100?
> | WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
> | Modules linked in:
> | CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
> |
> | Stack Trace:
> |  arc_unwind_core.constprop.1+0xd0/0xf4
> |  dump_stack+0x68/0x80
> |  warn_slowpath_null+0x4e/0xec
> |  sg_miter_next+0x28/0x20c
> |  dw_mci_read_data_pio+0x44/0x190
> |  dw_mmc f000a000.mmc: Unexpected interrupt latency
> |   dw_mci_interrupt+0x3ee/0x530
> |  __handle_irq_event_percpu+0x56/0x150
> |  handle_irq_event+0x34/0x78
> |  handle_level_irq+0x8e/0x120
> |  generic_handle_irq+0x1c/0x2c
> |  idu_cascade_isr+0x30/0x6c
> |  __handle_domain_irq+0x72/0xc8
> |  ret_from_exception+0x0/0x8
> |---[ end trace 2a58c9af6c25fe51 ]---
> 
> Lets cast this multiply to u64 type which prevents overflow.
> 
> Tested-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> 
> Signed-off-by: Evgeniy Didin <Evgeniy.Didin@synopsys.com>
> 
> CC: Alexey Brodkin <abrodkin@synopsys.com>
> CC: Eugeniy Paltsev <paltsev@synopsys.com>
> CC: Douglas Anderson <dianders@chromium.org>
> CC: Ulf Hansson <ulf.hansson@linaro.org>
> CC: linux-kernel@vger.kernel.org
> CC: linux-snps-arc@lists.infradead.org
> Cc: <stable@vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation
> ---
> Changes since v1:
> -uint64_t switched to u64
> -changed commit message
>  drivers/mmc/host/dw_mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..1a0b9751c67c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1944,7 +1944,7 @@ 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,
> +	drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
>  			       host->bus_hz);

DIV_ROUND_UP_ULL?

>  
>  	/* add a bit spare time */

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems
@ 2018-02-23  3:24   ` Jisheng Zhang
  0 siblings, 0 replies; 18+ messages in thread
From: Jisheng Zhang @ 2018-02-23  3:24 UTC (permalink / raw)
  To: linux-snps-arc

On Thu, 22 Feb 2018 16:34:18 +0300 Evgeniy Didin wrote:

> In commit 9d9491a7da2a ("mmc: dw_mmc: Fix the DTO timeout calculation") have been made
> changes which cause multiply overflow for 32-bit systems.
> The broken timeout calculations caused false interrupt latency warnings
> and stacktrace splat (such as below) when accessing the 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
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual
> | 396825HZ div = 63)
> | mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 25000000Hz, actual
> |  25000000HZ div = 1)
> | ------------[ cut here ]------------
> | softirq: huh, entered softirq 6 TASKLET 6f6a9412 with preempt_count 00000101,
> | exited with 00000100?
> | WARNING: CPU: 2 PID: 0 at ../lib/scatterlist.c:652 sg_miter_next+0x28/0x20c
> | Modules linked in:
> | CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.15.0 #57
> |
> | Stack Trace:
> |  arc_unwind_core.constprop.1+0xd0/0xf4
> |  dump_stack+0x68/0x80
> |  warn_slowpath_null+0x4e/0xec
> |  sg_miter_next+0x28/0x20c
> |  dw_mci_read_data_pio+0x44/0x190
> |  dw_mmc f000a000.mmc: Unexpected interrupt latency
> |   dw_mci_interrupt+0x3ee/0x530
> |  __handle_irq_event_percpu+0x56/0x150
> |  handle_irq_event+0x34/0x78
> |  handle_level_irq+0x8e/0x120
> |  generic_handle_irq+0x1c/0x2c
> |  idu_cascade_isr+0x30/0x6c
> |  __handle_domain_irq+0x72/0xc8
> |  ret_from_exception+0x0/0x8
> |---[ end trace 2a58c9af6c25fe51 ]---
> 
> Lets cast this multiply to u64 type which prevents overflow.
> 
> Tested-by: Vineet Gupta <Vineet.Gupta1 at synopsys.com>
> Fixes: ARC STAR 9001306872 HSDK, sdio: board crashes when copying big files
> 
> 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: linux-kernel at vger.kernel.org
> CC: linux-snps-arc at lists.infradead.org
> Cc: <stable at vger.kernel.org> #  9d9491a7da2a mmc: dw_mmc: Fix the DTO timeout calculation
> ---
> Changes since v1:
> -uint64_t switched to u64
> -changed commit message
>  drivers/mmc/host/dw_mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 0aa39975f33b..1a0b9751c67c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1944,7 +1944,7 @@ 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,
> +	drto_ms = DIV_ROUND_UP((u64)MSEC_PER_SEC * drto_clks * drto_div,
>  			       host->bus_hz);

DIV_ROUND_UP_ULL?

>  
>  	/* add a bit spare time */

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-02-23  3:25 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-22 13:34 [PATCH v2] mmc: dw_mmc: Fix the DTO timeout overflow calculation for 32-bit systems Evgeniy Didin
2018-02-22 13:34 ` Evgeniy Didin
2018-02-22 13:34 ` Evgeniy Didin
2018-02-22 15:28 ` Shawn Lin
2018-02-22 15:28   ` Shawn Lin
2018-02-22 15:28   ` Shawn Lin
2018-02-22 16:03   ` Alexey Brodkin
2018-02-22 16:03     ` Alexey Brodkin
2018-02-22 16:03     ` Alexey Brodkin
2018-02-22 16:36     ` Shawn Lin
2018-02-22 16:36       ` Shawn Lin
2018-02-22 16:36       ` Shawn Lin
2018-02-22 17:38   ` Vineet Gupta
2018-02-22 17:38     ` Vineet Gupta
2018-02-22 17:38     ` Vineet Gupta
2018-02-23  3:24 ` Jisheng Zhang
2018-02-23  3:24   ` Jisheng Zhang
2018-02-23  3:24   ` Jisheng Zhang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.