linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers
@ 2019-03-01 16:43 Christoph Muellner
  2019-03-01 16:43 ` [PATCH 2/3] dt-bindings: mmc: Add a new property disable-cqe-dcmd Christoph Muellner
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Christoph Muellner @ 2019-03-01 16:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, heiko, shawn.lin, ulf.hansson, adrian.hunter
  Cc: Christoph Muellner, Philipp Tomsich, Michal Simek,
	Douglas Anderson, Enric Balletbo i Serra, Viresh Kumar,
	Jeffy Chen, Shunqian Zheng, Ezequiel Garcia, Randy Li, Tony Xie,
	Vicente Bergas, Klaus Goger, linux-mmc, devicetree, linux-kernel,
	linux-arm-kernel, linux-rockchip

Direct commands (DCMDs) are an optional feature of eMMC 5.1's command
queue engine (CQE). The Arasan eMMC 5.1 controller uses the CQHCI,
which exposes a control register bit to enable the feature.
The current implementation sets this bit unconditionally.

This patch allows to surpress the feature activation,
by specifying the property disable-cqe-dcmd.

Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index c9e3e050ccc8..88dc3f00a5be 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -832,7 +832,10 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 		host->mmc_host_ops.start_signal_voltage_switch =
 					sdhci_arasan_voltage_switch;
 		sdhci_arasan->has_cqe = true;
-		host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
+		host->mmc->caps2 |= MMC_CAP2_CQE;
+
+		if (!of_property_read_bool(np, "disable-cqe-dcmd"))
+			host->mmc->caps2 |= MMC_CAP2_CQE_DCMD;
 	}
 
 	ret = sdhci_arasan_add_host(sdhci_arasan);
-- 
2.11.0


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

* [PATCH 2/3] dt-bindings: mmc: Add a new property disable-cqe-dcmd.
  2019-03-01 16:43 [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers Christoph Muellner
@ 2019-03-01 16:43 ` Christoph Muellner
  2019-03-01 16:45   ` Philipp Tomsich
  2019-03-01 16:43 ` [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller Christoph Muellner
  2019-03-01 18:17 ` [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers Heiko Stuebner
  2 siblings, 1 reply; 8+ messages in thread
From: Christoph Muellner @ 2019-03-01 16:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, heiko, shawn.lin, ulf.hansson, adrian.hunter
  Cc: Christoph Muellner, Philipp Tomsich, Michal Simek,
	Douglas Anderson, Viresh Kumar, Enric Balletbo i Serra,
	Tomasz Figa, Lin Huang, Randy Li, Tony Xie, Vicente Bergas,
	Ezequiel Garcia, Klaus Goger, linux-mmc, devicetree,
	linux-kernel, linux-arm-kernel, linux-rockchip

This patch documents the new proprty disable-cqe-dcmd
for the Arasan eMMC 5.1 driver.

Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 1edbb049cccb..ec699bf98b7c 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -44,6 +44,10 @@ Optional Properties:
     properly. Test mode can be used to force the controller to function.
   - xlnx,int-clock-stable-broken: when present, the controller always reports
     that the internal clock is stable even when it is not.
+  - disable-cqe-dcmd: The eMMC 5.1 standard specifies direct commands (DCMDs)
+    as part of the command queue engine (CQE). On controllers with a CQHCI,
+    such as the Arasan eMMC 5.1 host controller, the driver has to enable DCMDs.
+    This is done unless disable-cqe-dcmd is specified.
 
 Example:
 	sdhci@e0100000 {
-- 
2.11.0


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

* [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller.
  2019-03-01 16:43 [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers Christoph Muellner
  2019-03-01 16:43 ` [PATCH 2/3] dt-bindings: mmc: Add a new property disable-cqe-dcmd Christoph Muellner
@ 2019-03-01 16:43 ` Christoph Muellner
  2019-03-02  0:47   ` [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller.【请注意,邮件由linux-mmc-owner@vger.kernel.org代发】 Shawn Lin
  2019-03-01 18:17 ` [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers Heiko Stuebner
  2 siblings, 1 reply; 8+ messages in thread
From: Christoph Muellner @ 2019-03-01 16:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, heiko, shawn.lin, ulf.hansson, adrian.hunter
  Cc: Christoph Muellner, Philipp Tomsich, Michal Simek,
	Douglas Anderson, Viresh Kumar, Enric Balletbo i Serra,
	Shunqian Zheng, Klaus Goger, Randy Li, Tony Xie, Vicente Bergas,
	Ezequiel Garcia, linux-mmc, devicetree, linux-kernel,
	linux-arm-kernel, linux-rockchip

When using direct commands (DCMDs) on an RK3399, we get spurious
CQE completion interrupts for the DCMD transaction slot (#31):

[  931.196520] ------------[ cut here ]------------
[  931.201702] mmc1: cqhci: spurious TCN for tag 31
[  931.206906] WARNING: CPU: 0 PID: 1433 at
/usr/src/kernel/drivers/mmc/host/cqhci.c:725 cqhci_irq+0x2e4/0x490
[  931.206909] Modules linked in:
[  931.206918] CPU: 0 PID: 1433 Comm: irq/29-mmc1 Not tainted
4.19.8-rt6-funkadelic #1
[  931.206920] Hardware name: Theobroma Systems RK3399-Q7 SoM (DT)
[  931.206924] pstate: 40000005 (nZcv daif -PAN -UAO)
[  931.206927] pc : cqhci_irq+0x2e4/0x490
[  931.206931] lr : cqhci_irq+0x2e4/0x490
[  931.206933] sp : ffff00000e54bc80
[  931.206934] x29: ffff00000e54bc80 x28: 0000000000000000
[  931.206939] x27: 0000000000000001 x26: ffff000008f217e8
[  931.206944] x25: ffff8000f02ef030 x24: ffff0000091417b0
[  931.206948] x23: ffff0000090aa000 x22: ffff8000f008b000
[  931.206953] x21: 0000000000000002 x20: 000000000000001f
[  931.206957] x19: ffff8000f02ef018 x18: ffffffffffffffff
[  931.206961] x17: 0000000000000000 x16: 0000000000000000
[  931.206966] x15: ffff0000090aa6c8 x14: 0720072007200720
[  931.206970] x13: 0720072007200720 x12: 0720072007200720
[  931.206975] x11: 0720072007200720 x10: 0720072007200720
[  931.206980] x9 : 0720072007200720 x8 : 0720072007200720
[  931.206984] x7 : 0720073107330720 x6 : 00000000000005a0
[  931.206988] x5 : ffff00000860d4b0 x4 : 0000000000000000
[  931.206993] x3 : 0000000000000001 x2 : 0000000000000001
[  931.206997] x1 : 1bde3a91b0d4d900 x0 : 0000000000000000
[  931.207001] Call trace:
[  931.207005]  cqhci_irq+0x2e4/0x490
[  931.207009]  sdhci_arasan_cqhci_irq+0x5c/0x90
[  931.207013]  sdhci_irq+0x98/0x930
[  931.207019]  irq_forced_thread_fn+0x2c/0xa0
[  931.207023]  irq_thread+0x114/0x1c0
[  931.207027]  kthread+0x128/0x130
[  931.207032]  ret_from_fork+0x10/0x20
[  931.207035] ---[ end trace 0000000000000002 ]---

The driver shows this message only for the first spurious interrupt
by using WARN_ONCE(). Changing this to WARN() shows, that this is
happening quite frequently (up to once a second).

Since the eMMC 5.1 specification, where CQE and CQHCI are specified,
does not mention that spurious TCN interrupts for DCMDs can be simply
ignored, we must assume that using this feature is not working reliably.

The current implementation uses DCMD for REQ_OP_FLUSH only, and
I could not see any performance/power impact when disabling
this optional feature for RK3399.

Therefore this patch disables DCMDs for RK3399.

Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 6cc1c9fa4ea6..1bbf0da4e01d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -333,6 +333,7 @@
 		phys = <&emmc_phy>;
 		phy-names = "phy_arasan";
 		power-domains = <&power RK3399_PD_EMMC>;
+		disable-cqe-dcmd;
 		status = "disabled";
 	};
 
-- 
2.11.0


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

* Re: [PATCH 2/3] dt-bindings: mmc: Add a new property disable-cqe-dcmd.
  2019-03-01 16:43 ` [PATCH 2/3] dt-bindings: mmc: Add a new property disable-cqe-dcmd Christoph Muellner
@ 2019-03-01 16:45   ` Philipp Tomsich
  0 siblings, 0 replies; 8+ messages in thread
From: Philipp Tomsich @ 2019-03-01 16:45 UTC (permalink / raw)
  To: Christoph Muellner
  Cc: robh+dt, mark.rutland, heiko, shawn.lin, ulf.hansson,
	adrian.hunter, Michal Simek, Douglas Anderson, Viresh Kumar,
	Enric Balletbo i Serra, Tomasz Figa, Lin Huang, Randy Li,
	Tony Xie, Vicente Bergas, Ezequiel Garcia, Klaus Goger,
	linux-mmc, devicetree, linux-kernel, linux-arm-kernel,
	linux-rockchip



> On 01.03.2019, at 17:43, Christoph Muellner <christoph.muellner@theobroma-systems.com> wrote:
> 
> This patch documents the new proprty disable-cqe-dcmd

typo: proprty -> property

> for the Arasan eMMC 5.1 driver.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 ++++
> 1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 1edbb049cccb..ec699bf98b7c 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -44,6 +44,10 @@ Optional Properties:
>     properly. Test mode can be used to force the controller to function.
>   - xlnx,int-clock-stable-broken: when present, the controller always reports
>     that the internal clock is stable even when it is not.
> +  - disable-cqe-dcmd: The eMMC 5.1 standard specifies direct commands (DCMDs)
> +    as part of the command queue engine (CQE). On controllers with a CQHCI,
> +    such as the Arasan eMMC 5.1 host controller, the driver has to enable DCMDs.
> +    This is done unless disable-cqe-dcmd is specified.
> 
> Example:
> 	sdhci@e0100000 {
> -- 
> 2.11.0
> 


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

* Re: [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers
  2019-03-01 16:43 [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers Christoph Muellner
  2019-03-01 16:43 ` [PATCH 2/3] dt-bindings: mmc: Add a new property disable-cqe-dcmd Christoph Muellner
  2019-03-01 16:43 ` [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller Christoph Muellner
@ 2019-03-01 18:17 ` Heiko Stuebner
  2019-03-01 18:19   ` Christoph Müllner
  2 siblings, 1 reply; 8+ messages in thread
From: Heiko Stuebner @ 2019-03-01 18:17 UTC (permalink / raw)
  To: Christoph Muellner
  Cc: robh+dt, mark.rutland, shawn.lin, ulf.hansson, adrian.hunter,
	Philipp Tomsich, Michal Simek, Douglas Anderson,
	Enric Balletbo i Serra, Viresh Kumar, Jeffy Chen, Shunqian Zheng,
	Ezequiel Garcia, Randy Li, Tony Xie, Vicente Bergas, Klaus Goger,
	linux-mmc, devicetree, linux-kernel, linux-arm-kernel,
	linux-rockchip

Am Freitag, 1. März 2019, 17:43:45 CET schrieb Christoph Muellner:
> Direct commands (DCMDs) are an optional feature of eMMC 5.1's command
> queue engine (CQE). The Arasan eMMC 5.1 controller uses the CQHCI,
> which exposes a control register bit to enable the feature.
> The current implementation sets this bit unconditionally.
> 
> This patch allows to surpress the feature activation,
> by specifying the property disable-cqe-dcmd.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Patch subject is needs improvement :-) .
I did scratch my head a lot regarding "where is the code change"
until I actually looked into the patch itself, so you probably want

	mmc: sdhci-of-arasan: Add DTS property to disable DCMDs

or similar instead of "dt-bindings: ..." here to match the subsystem.


Heiko

> ---
>  drivers/mmc/host/sdhci-of-arasan.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index c9e3e050ccc8..88dc3f00a5be 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -832,7 +832,10 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>  		host->mmc_host_ops.start_signal_voltage_switch =
>  					sdhci_arasan_voltage_switch;
>  		sdhci_arasan->has_cqe = true;
> -		host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
> +		host->mmc->caps2 |= MMC_CAP2_CQE;
> +
> +		if (!of_property_read_bool(np, "disable-cqe-dcmd"))
> +			host->mmc->caps2 |= MMC_CAP2_CQE_DCMD;
>  	}
>  
>  	ret = sdhci_arasan_add_host(sdhci_arasan);
> 





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

* Re: [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers
  2019-03-01 18:17 ` [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers Heiko Stuebner
@ 2019-03-01 18:19   ` Christoph Müllner
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Müllner @ 2019-03-01 18:19 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: robh+dt, mark.rutland, shawn.lin, ulf.hansson, adrian.hunter,
	Philipp Tomsich, Michal Simek, Douglas Anderson,
	Enric Balletbo i Serra, Viresh Kumar, Jeffy Chen, Shunqian Zheng,
	Ezequiel Garcia, Randy Li, Tony Xie, Vicente Bergas, Klaus Goger,
	linux-mmc, devicetree, linux-kernel, linux-arm-kernel,
	linux-rockchip


> On 01.03.2019, at 19:17, Heiko Stuebner <heiko@sntech.de> wrote:
> 
> Am Freitag, 1. März 2019, 17:43:45 CET schrieb Christoph Muellner:
>> Direct commands (DCMDs) are an optional feature of eMMC 5.1's command
>> queue engine (CQE). The Arasan eMMC 5.1 controller uses the CQHCI,
>> which exposes a control register bit to enable the feature.
>> The current implementation sets this bit unconditionally.
>> 
>> This patch allows to surpress the feature activation,
>> by specifying the property disable-cqe-dcmd.
>> 
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> 
> Patch subject is needs improvement :-) .
> I did scratch my head a lot regarding "where is the code change"
> until I actually looked into the patch itself, so you probably want
> 
> 	mmc: sdhci-of-arasan: Add DTS property to disable DCMDs
> 
> or similar instead of "dt-bindings: ..." here to match the subsystem.

Yes, I've spotted this as well after sending it.
I've already renamed to "mmc: sdhci-of-arasan: Add DTS property to disable DCMDs." for v2.

Thanks,
Christoph

> 
> 
> Heiko
> 
>> ---
>> drivers/mmc/host/sdhci-of-arasan.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
>> index c9e3e050ccc8..88dc3f00a5be 100644
>> --- a/drivers/mmc/host/sdhci-of-arasan.c
>> +++ b/drivers/mmc/host/sdhci-of-arasan.c
>> @@ -832,7 +832,10 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>> 		host->mmc_host_ops.start_signal_voltage_switch =
>> 					sdhci_arasan_voltage_switch;
>> 		sdhci_arasan->has_cqe = true;
>> -		host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
>> +		host->mmc->caps2 |= MMC_CAP2_CQE;
>> +
>> +		if (!of_property_read_bool(np, "disable-cqe-dcmd"))
>> +			host->mmc->caps2 |= MMC_CAP2_CQE_DCMD;
>> 	}
>> 
>> 	ret = sdhci_arasan_add_host(sdhci_arasan);
>> 
> 
> 
> 
> 


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

* Re: [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller.【请注意,邮件由linux-mmc-owner@vger.kernel.org代发】
  2019-03-01 16:43 ` [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller Christoph Muellner
@ 2019-03-02  0:47   ` Shawn Lin
  2019-03-02  8:26     ` Christoph Müllner
  0 siblings, 1 reply; 8+ messages in thread
From: Shawn Lin @ 2019-03-02  0:47 UTC (permalink / raw)
  To: Christoph Muellner, robh+dt, mark.rutland, heiko, ulf.hansson,
	adrian.hunter
  Cc: shawn.lin, Philipp Tomsich, Michal Simek, Douglas Anderson,
	Viresh Kumar, Enric Balletbo i Serra, Shunqian Zheng,
	Klaus Goger, Randy Li, Tony Xie, Vicente Bergas, Ezequiel Garcia,
	linux-mmc, devicetree, linux-kernel, linux-arm-kernel,
	linux-rockchip

On 2019/3/2 0:43, Christoph Muellner wrote:
> When using direct commands (DCMDs) on an RK3399, we get spurious
> CQE completion interrupts for the DCMD transaction slot (#31):

I didn't see it. Do you try any newer code, for instance, linux-next?

> 
> [  931.196520] ------------[ cut here ]------------
> [  931.201702] mmc1: cqhci: spurious TCN for tag 31
> [  931.206906] WARNING: CPU: 0 PID: 1433 at
> /usr/src/kernel/drivers/mmc/host/cqhci.c:725 cqhci_irq+0x2e4/0x490
> [  931.206909] Modules linked in:
> [  931.206918] CPU: 0 PID: 1433 Comm: irq/29-mmc1 Not tainted
> 4.19.8-rt6-funkadelic #1
> [  931.206920] Hardware name: Theobroma Systems RK3399-Q7 SoM (DT)
> [  931.206924] pstate: 40000005 (nZcv daif -PAN -UAO)
> [  931.206927] pc : cqhci_irq+0x2e4/0x490
> [  931.206931] lr : cqhci_irq+0x2e4/0x490
> [  931.206933] sp : ffff00000e54bc80
> [  931.206934] x29: ffff00000e54bc80 x28: 0000000000000000
> [  931.206939] x27: 0000000000000001 x26: ffff000008f217e8
> [  931.206944] x25: ffff8000f02ef030 x24: ffff0000091417b0
> [  931.206948] x23: ffff0000090aa000 x22: ffff8000f008b000
> [  931.206953] x21: 0000000000000002 x20: 000000000000001f
> [  931.206957] x19: ffff8000f02ef018 x18: ffffffffffffffff
> [  931.206961] x17: 0000000000000000 x16: 0000000000000000
> [  931.206966] x15: ffff0000090aa6c8 x14: 0720072007200720
> [  931.206970] x13: 0720072007200720 x12: 0720072007200720
> [  931.206975] x11: 0720072007200720 x10: 0720072007200720
> [  931.206980] x9 : 0720072007200720 x8 : 0720072007200720
> [  931.206984] x7 : 0720073107330720 x6 : 00000000000005a0
> [  931.206988] x5 : ffff00000860d4b0 x4 : 0000000000000000
> [  931.206993] x3 : 0000000000000001 x2 : 0000000000000001
> [  931.206997] x1 : 1bde3a91b0d4d900 x0 : 0000000000000000
> [  931.207001] Call trace:
> [  931.207005]  cqhci_irq+0x2e4/0x490
> [  931.207009]  sdhci_arasan_cqhci_irq+0x5c/0x90
> [  931.207013]  sdhci_irq+0x98/0x930
> [  931.207019]  irq_forced_thread_fn+0x2c/0xa0
> [  931.207023]  irq_thread+0x114/0x1c0
> [  931.207027]  kthread+0x128/0x130
> [  931.207032]  ret_from_fork+0x10/0x20
> [  931.207035] ---[ end trace 0000000000000002 ]---
> 
> The driver shows this message only for the first spurious interrupt
> by using WARN_ONCE(). Changing this to WARN() shows, that this is
> happening quite frequently (up to once a second).
> 
> Since the eMMC 5.1 specification, where CQE and CQHCI are specified,
> does not mention that spurious TCN interrupts for DCMDs can be simply
> ignored, we must assume that using this feature is not working reliably.
> 
> The current implementation uses DCMD for REQ_OP_FLUSH only, and
> I could not see any performance/power impact when disabling
> this optional feature for RK3399.
> 
> Therefore this patch disables DCMDs for RK3399.

We need to sort out the problem, and see if it could be solved, or
we just simply remove MMC_CAP2_CQE_DCMD it from sdhci-of-arasan

> 
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 6cc1c9fa4ea6..1bbf0da4e01d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -333,6 +333,7 @@
>   		phys = <&emmc_phy>;
>   		phy-names = "phy_arasan";
>   		power-domains = <&power RK3399_PD_EMMC>;
> +		disable-cqe-dcmd;
>   		status = "disabled";
>   	};
>   
> 



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

* Re: [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller.【请注意,邮件由linux-mmc-owner@vger.kernel.org代发】
  2019-03-02  0:47   ` [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller.【请注意,邮件由linux-mmc-owner@vger.kernel.org代发】 Shawn Lin
@ 2019-03-02  8:26     ` Christoph Müllner
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Müllner @ 2019-03-02  8:26 UTC (permalink / raw)
  To: Shawn Lin, robh+dt, mark.rutland, heiko, ulf.hansson, adrian.hunter
  Cc: Philipp Tomsich, Michal Simek, Douglas Anderson, Viresh Kumar,
	Enric Balletbo i Serra, Shunqian Zheng, Klaus Goger, Randy Li,
	Tony Xie, Vicente Bergas, Ezequiel Garcia, linux-mmc, devicetree,
	linux-kernel, linux-arm-kernel, linux-rockchip

Hi Shawn,

On 3/2/19 1:47 AM, Shawn Lin wrote:
> On 2019/3/2 0:43, Christoph Muellner wrote:
>> When using direct commands (DCMDs) on an RK3399, we get spurious
>> CQE completion interrupts for the DCMD transaction slot (#31):
> 
> I didn't see it. Do you try any newer code, for instance, linux-next?

I can reproduce this with all kernel versions from 4.16 up to
linus/master. So all kernels with the cqhci driver (has been merged for
4.15) are affected.

All I need to do to reproduce the issue is to boot the system with a
root file system on the eMMC. I use a Debian stable based rootfs.

> 
>>
>> [  931.196520] ------------[ cut here ]------------
>> [  931.201702] mmc1: cqhci: spurious TCN for tag 31
>> [  931.206906] WARNING: CPU: 0 PID: 1433 at
>> /usr/src/kernel/drivers/mmc/host/cqhci.c:725 cqhci_irq+0x2e4/0x490
>> [  931.206909] Modules linked in:
>> [  931.206918] CPU: 0 PID: 1433 Comm: irq/29-mmc1 Not tainted
>> 4.19.8-rt6-funkadelic #1
>> [  931.206920] Hardware name: Theobroma Systems RK3399-Q7 SoM (DT)
>> [  931.206924] pstate: 40000005 (nZcv daif -PAN -UAO)
>> [  931.206927] pc : cqhci_irq+0x2e4/0x490
>> [  931.206931] lr : cqhci_irq+0x2e4/0x490
>> [  931.206933] sp : ffff00000e54bc80
>> [  931.206934] x29: ffff00000e54bc80 x28: 0000000000000000
>> [  931.206939] x27: 0000000000000001 x26: ffff000008f217e8
>> [  931.206944] x25: ffff8000f02ef030 x24: ffff0000091417b0
>> [  931.206948] x23: ffff0000090aa000 x22: ffff8000f008b000
>> [  931.206953] x21: 0000000000000002 x20: 000000000000001f
>> [  931.206957] x19: ffff8000f02ef018 x18: ffffffffffffffff
>> [  931.206961] x17: 0000000000000000 x16: 0000000000000000
>> [  931.206966] x15: ffff0000090aa6c8 x14: 0720072007200720
>> [  931.206970] x13: 0720072007200720 x12: 0720072007200720
>> [  931.206975] x11: 0720072007200720 x10: 0720072007200720
>> [  931.206980] x9 : 0720072007200720 x8 : 0720072007200720
>> [  931.206984] x7 : 0720073107330720 x6 : 00000000000005a0
>> [  931.206988] x5 : ffff00000860d4b0 x4 : 0000000000000000
>> [  931.206993] x3 : 0000000000000001 x2 : 0000000000000001
>> [  931.206997] x1 : 1bde3a91b0d4d900 x0 : 0000000000000000
>> [  931.207001] Call trace:
>> [  931.207005]  cqhci_irq+0x2e4/0x490
>> [  931.207009]  sdhci_arasan_cqhci_irq+0x5c/0x90
>> [  931.207013]  sdhci_irq+0x98/0x930
>> [  931.207019]  irq_forced_thread_fn+0x2c/0xa0
>> [  931.207023]  irq_thread+0x114/0x1c0
>> [  931.207027]  kthread+0x128/0x130
>> [  931.207032]  ret_from_fork+0x10/0x20
>> [  931.207035] ---[ end trace 0000000000000002 ]---
>>
>> The driver shows this message only for the first spurious interrupt
>> by using WARN_ONCE(). Changing this to WARN() shows, that this is
>> happening quite frequently (up to once a second).
>>
>> Since the eMMC 5.1 specification, where CQE and CQHCI are specified,
>> does not mention that spurious TCN interrupts for DCMDs can be simply
>> ignored, we must assume that using this feature is not working reliably.
>>
>> The current implementation uses DCMD for REQ_OP_FLUSH only, and
>> I could not see any performance/power impact when disabling
>> this optional feature for RK3399.
>>
>> Therefore this patch disables DCMDs for RK3399.
> 
> We need to sort out the problem, and see if it could be solved, or
> we just simply remove MMC_CAP2_CQE_DCMD it from sdhci-of-arasan

I fully agree that we should address it in the driver
if it would be buggy.

Therefore I debugged the issue and used an event-log
based on atomic_t variables to observe what is going on.
And it is indeed the case that we get a second spurious
interrupt (an interrupt for a slot, which has the doorbell
bit not set previously) from the controller every now and then.
Only slot #31 is affected (so only DCMDs).
And only if DCMD support is enabled.

I disagree, that we should disable it for sdhci-of-arasan (i.e. for all
Arasan eMMC 5.1 based controllers), because, I cannot say that all
Arasan eMMC 5.1 based implementations are affected.
I only know that the one in the RK3399 is affected (mainly because I
don't have access to more devices with this IP core). Therefore the
series disables it for RK3399.

Thanks,
Christoph


> 
>>
>> Signed-off-by: Christoph Muellner
>> <christoph.muellner@theobroma-systems.com>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>>   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index 6cc1c9fa4ea6..1bbf0da4e01d 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -333,6 +333,7 @@
>>           phys = <&emmc_phy>;
>>           phy-names = "phy_arasan";
>>           power-domains = <&power RK3399_PD_EMMC>;
>> +        disable-cqe-dcmd;
>>           status = "disabled";
>>       };
>>  
> 
> 

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

end of thread, other threads:[~2019-03-02  8:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-01 16:43 [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers Christoph Muellner
2019-03-01 16:43 ` [PATCH 2/3] dt-bindings: mmc: Add a new property disable-cqe-dcmd Christoph Muellner
2019-03-01 16:45   ` Philipp Tomsich
2019-03-01 16:43 ` [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller Christoph Muellner
2019-03-02  0:47   ` [PATCH 3/3] arm64: dts: rockchip: Disable DCMDs on RK3399's eMMC controller.【请注意,邮件由linux-mmc-owner@vger.kernel.org代发】 Shawn Lin
2019-03-02  8:26     ` Christoph Müllner
2019-03-01 18:17 ` [PATCH 1/3] dt-bindings: mmc: Add DTS property to disable DCMDs on Arasan controllers Heiko Stuebner
2019-03-01 18:19   ` Christoph Müllner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).