diff mbox series

[V2] dmaengine: xilinx: dpdma: initialize registers before request_irq

Message ID 20210430064041.4058180-1-quanyang.wang@windriver.com (mailing list archive)
State Accepted
Commit 538ea65a9fd1194352a41313bff876b74b5d90c5
Headers show
Series [V2] dmaengine: xilinx: dpdma: initialize registers before request_irq | expand

Commit Message

quanyang.wang April 30, 2021, 6:40 a.m. UTC
From: Quanyang Wang <quanyang.wang@windriver.com>

In some scenarios (kdump), dpdma hardware irqs has been enabled when
calling request_irq in probe function, and then the dpdma irq handler
xilinx_dpdma_irq_handler is invoked to access xdev->chan[i]. But at
this moment xdev->chan[i] hasn't been initialized.

We should ensure the dpdma controller to be in a consistent and
clean state before further initialization. So add dpdma_hw_init()
to do this.

Furthermore, in xilinx_dpdma_disable_irq, disable all interrupts
instead of error interrupts.

This patch is to fix the kdump kernel crash as below:

[    3.696128] Unable to handle kernel NULL pointer dereference at virtual address 000000000000012c
[    3.696710] xilinx-zynqmp-dpdma fd4c0000.dma-controller: Xilinx DPDMA engine is probed
[    3.704900] Mem abort info:
[    3.704902]   ESR = 0x96000005
[    3.704905]   EC = 0x25: DABT (current EL), IL = 32 bits
[    3.704907]   SET = 0, FnV = 0
[    3.704912]   EA = 0, S1PTW = 0
[    3.713800] ahci-ceva fd0c0000.ahci: supply ahci not found, using dummy regulator
[    3.715585] Data abort info:
[    3.715587]   ISV = 0, ISS = 0x00000005
[    3.715589]   CM = 0, WnR = 0
[    3.715592] [000000000000012c] user address but active_mm is swapper
[    3.715596] Internal error: Oops: 96000005 [#1] SMP
[    3.715599] Modules linked in:
[    3.715608] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.10.0-12170-g60894882155f-dirty #77
[    3.723937] Hardware name: ZynqMP ZCU102 Rev1.0 (DT)
[    3.723942] pstate: 80000085 (Nzcv daIf -PAN -UAO -TCO BTYPE=--)
[    3.723956] pc : xilinx_dpdma_irq_handler+0x418/0x560
[    3.793049] lr : xilinx_dpdma_irq_handler+0x3d8/0x560
[    3.798089] sp : ffffffc01186bdf0
[    3.801388] x29: ffffffc01186bdf0 x28: ffffffc011836f28
[    3.806692] x27: ffffff8023e0ac80 x26: 0000000000000080
[    3.811996] x25: 0000000008000408 x24: 0000000000000003
[    3.817300] x23: ffffffc01186be70 x22: ffffffc011291740
[    3.822604] x21: 0000000000000000 x20: 0000000008000408
[    3.827908] x19: 0000000000000000 x18: 0000000000000010
[    3.833212] x17: 0000000000000000 x16: 0000000000000000
[    3.838516] x15: 0000000000000000 x14: ffffffc011291740
[    3.843820] x13: ffffffc02eb4d000 x12: 0000000034d4d91d
[    3.849124] x11: 0000000000000040 x10: ffffffc0112d2d48
[    3.854428] x9 : ffffffc0112d2d40 x8 : ffffff8021c00268
[    3.859732] x7 : 0000000000000000 x6 : ffffffc011836000
[    3.865036] x5 : 0000000000000003 x4 : 0000000000000000
[    3.870340] x3 : 0000000000000001 x2 : 0000000000000000
[    3.875644] x1 : 0000000000000000 x0 : 000000000000012c
[    3.880948] Call trace:
[    3.883382]  xilinx_dpdma_irq_handler+0x418/0x560
[    3.888079]  __handle_irq_event_percpu+0x5c/0x178
[    3.892774]  handle_irq_event_percpu+0x34/0x98
[    3.897210]  handle_irq_event+0x44/0xb8
[    3.901030]  handle_fasteoi_irq+0xd0/0x190
[    3.905117]  generic_handle_irq+0x30/0x48
[    3.909111]  __handle_domain_irq+0x64/0xc0
[    3.913192]  gic_handle_irq+0x78/0xa0
[    3.916846]  el1_irq+0xc4/0x180
[    3.919982]  cpuidle_enter_state+0x134/0x2f8
[    3.924243]  cpuidle_enter+0x38/0x50
[    3.927810]  call_cpuidle+0x1c/0x40
[    3.931290]  do_idle+0x20c/0x270
[    3.934502]  cpu_startup_entry+0x28/0x58
[    3.938410]  rest_init+0xbc/0xcc
[    3.941631]  arch_call_rest_init+0x10/0x1c
[    3.945718]  start_kernel+0x51c/0x558

Fixes: 7cbb0c63de3f ("dmaengine: xilinx: dpdma: Add the Xilinx DisplayPort DMA engine driver")
Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
---
V2:
 - As per Laurent's comments, add a new hw_init function to disable interrupts
   and clear register states.
 - Abandon all modifications in V1 since the new hw_init function can
   fix this issue

---
 drivers/dma/xilinx/xilinx_dpdma.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

Comments

quanyang.wang May 7, 2021, 5:34 a.m. UTC | #1
ping.
On Fri, Apr 30, 2021 at 02:40:41PM +0800, quanyang.wang@windriver.com wrote:
> From: Quanyang Wang <quanyang.wang@windriver.com>
> 
> In some scenarios (kdump), dpdma hardware irqs has been enabled when
> calling request_irq in probe function, and then the dpdma irq handler
> xilinx_dpdma_irq_handler is invoked to access xdev->chan[i]. But at
> this moment xdev->chan[i] hasn't been initialized.
> 
> We should ensure the dpdma controller to be in a consistent and
> clean state before further initialization. So add dpdma_hw_init()
> to do this.
> 
> Furthermore, in xilinx_dpdma_disable_irq, disable all interrupts
> instead of error interrupts.
> 
> This patch is to fix the kdump kernel crash as below:
> 
> [    3.696128] Unable to handle kernel NULL pointer dereference at virtual address 000000000000012c
> [    3.696710] xilinx-zynqmp-dpdma fd4c0000.dma-controller: Xilinx DPDMA engine is probed
> [    3.704900] Mem abort info:
> [    3.704902]   ESR = 0x96000005
> [    3.704905]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    3.704907]   SET = 0, FnV = 0
> [    3.704912]   EA = 0, S1PTW = 0
> [    3.713800] ahci-ceva fd0c0000.ahci: supply ahci not found, using dummy regulator
> [    3.715585] Data abort info:
> [    3.715587]   ISV = 0, ISS = 0x00000005
> [    3.715589]   CM = 0, WnR = 0
> [    3.715592] [000000000000012c] user address but active_mm is swapper
> [    3.715596] Internal error: Oops: 96000005 [#1] SMP
> [    3.715599] Modules linked in:
> [    3.715608] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.10.0-12170-g60894882155f-dirty #77
> [    3.723937] Hardware name: ZynqMP ZCU102 Rev1.0 (DT)
> [    3.723942] pstate: 80000085 (Nzcv daIf -PAN -UAO -TCO BTYPE=--)
> [    3.723956] pc : xilinx_dpdma_irq_handler+0x418/0x560
> [    3.793049] lr : xilinx_dpdma_irq_handler+0x3d8/0x560
> [    3.798089] sp : ffffffc01186bdf0
> [    3.801388] x29: ffffffc01186bdf0 x28: ffffffc011836f28
> [    3.806692] x27: ffffff8023e0ac80 x26: 0000000000000080
> [    3.811996] x25: 0000000008000408 x24: 0000000000000003
> [    3.817300] x23: ffffffc01186be70 x22: ffffffc011291740
> [    3.822604] x21: 0000000000000000 x20: 0000000008000408
> [    3.827908] x19: 0000000000000000 x18: 0000000000000010
> [    3.833212] x17: 0000000000000000 x16: 0000000000000000
> [    3.838516] x15: 0000000000000000 x14: ffffffc011291740
> [    3.843820] x13: ffffffc02eb4d000 x12: 0000000034d4d91d
> [    3.849124] x11: 0000000000000040 x10: ffffffc0112d2d48
> [    3.854428] x9 : ffffffc0112d2d40 x8 : ffffff8021c00268
> [    3.859732] x7 : 0000000000000000 x6 : ffffffc011836000
> [    3.865036] x5 : 0000000000000003 x4 : 0000000000000000
> [    3.870340] x3 : 0000000000000001 x2 : 0000000000000000
> [    3.875644] x1 : 0000000000000000 x0 : 000000000000012c
> [    3.880948] Call trace:
> [    3.883382]  xilinx_dpdma_irq_handler+0x418/0x560
> [    3.888079]  __handle_irq_event_percpu+0x5c/0x178
> [    3.892774]  handle_irq_event_percpu+0x34/0x98
> [    3.897210]  handle_irq_event+0x44/0xb8
> [    3.901030]  handle_fasteoi_irq+0xd0/0x190
> [    3.905117]  generic_handle_irq+0x30/0x48
> [    3.909111]  __handle_domain_irq+0x64/0xc0
> [    3.913192]  gic_handle_irq+0x78/0xa0
> [    3.916846]  el1_irq+0xc4/0x180
> [    3.919982]  cpuidle_enter_state+0x134/0x2f8
> [    3.924243]  cpuidle_enter+0x38/0x50
> [    3.927810]  call_cpuidle+0x1c/0x40
> [    3.931290]  do_idle+0x20c/0x270
> [    3.934502]  cpu_startup_entry+0x28/0x58
> [    3.938410]  rest_init+0xbc/0xcc
> [    3.941631]  arch_call_rest_init+0x10/0x1c
> [    3.945718]  start_kernel+0x51c/0x558
> 
> Fixes: 7cbb0c63de3f ("dmaengine: xilinx: dpdma: Add the Xilinx DisplayPort DMA engine driver")
> Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
> ---
> V2:
>  - As per Laurent's comments, add a new hw_init function to disable interrupts
>    and clear register states.
>  - Abandon all modifications in V1 since the new hw_init function can
>    fix this issue
> 
> ---
>  drivers/dma/xilinx/xilinx_dpdma.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
> index d8426f915c3e..60dee30a4930 100644
> --- a/drivers/dma/xilinx/xilinx_dpdma.c
> +++ b/drivers/dma/xilinx/xilinx_dpdma.c
> @@ -1554,7 +1554,7 @@ static void xilinx_dpdma_enable_irq(struct xilinx_dpdma_device *xdev)
>   */
>  static void xilinx_dpdma_disable_irq(struct xilinx_dpdma_device *xdev)
>  {
> -	dpdma_write(xdev->reg, XILINX_DPDMA_IDS, XILINX_DPDMA_INTR_ERR_ALL);
> +	dpdma_write(xdev->reg, XILINX_DPDMA_IDS, XILINX_DPDMA_INTR_ALL);
>  	dpdma_write(xdev->reg, XILINX_DPDMA_EIDS, XILINX_DPDMA_EINTR_ALL);
>  }
>  
> @@ -1691,6 +1691,26 @@ static struct dma_chan *of_dma_xilinx_xlate(struct of_phandle_args *dma_spec,
>  	return dma_get_slave_channel(&xdev->chan[chan_id]->vchan.chan);
>  }
>  
> +static void dpdma_hw_init(struct xilinx_dpdma_device *xdev)
> +{
> +	unsigned int i;
> +	void __iomem *reg;
> +
> +	/* Disable all interrupts */
> +	xilinx_dpdma_disable_irq(xdev);
> +
> +	/* Stop all channels */
> +	for (i = 0; i < ARRAY_SIZE(xdev->chan); i++) {
> +		reg = xdev->reg + XILINX_DPDMA_CH_BASE
> +				+ XILINX_DPDMA_CH_OFFSET * i;
> +		dpdma_clr(reg, XILINX_DPDMA_CH_CNTL, XILINX_DPDMA_CH_CNTL_ENABLE);
> +	}
> +
> +	/* Clear the interrupt status registers */
> +	dpdma_write(xdev->reg, XILINX_DPDMA_ISR, XILINX_DPDMA_INTR_ALL);
> +	dpdma_write(xdev->reg, XILINX_DPDMA_EISR, XILINX_DPDMA_EINTR_ALL);
> +}
> +
>  static int xilinx_dpdma_probe(struct platform_device *pdev)
>  {
>  	struct xilinx_dpdma_device *xdev;
> @@ -1717,6 +1737,8 @@ static int xilinx_dpdma_probe(struct platform_device *pdev)
>  	if (IS_ERR(xdev->reg))
>  		return PTR_ERR(xdev->reg);
>  
> +	dpdma_hw_init(xdev);
> +
>  	xdev->irq = platform_get_irq(pdev, 0);
>  	if (xdev->irq < 0) {
>  		dev_err(xdev->dev, "failed to get platform irq\n");
> -- 
> 2.25.1
>
Vinod Koul May 10, 2021, 4:10 p.m. UTC | #2
On 30-04-21, 14:40, quanyang.wang@windriver.com wrote:
> From: Quanyang Wang <quanyang.wang@windriver.com>
> 
> In some scenarios (kdump), dpdma hardware irqs has been enabled when
> calling request_irq in probe function, and then the dpdma irq handler
> xilinx_dpdma_irq_handler is invoked to access xdev->chan[i]. But at
> this moment xdev->chan[i] hasn't been initialized.
> 
> We should ensure the dpdma controller to be in a consistent and
> clean state before further initialization. So add dpdma_hw_init()
> to do this.
> 
> Furthermore, in xilinx_dpdma_disable_irq, disable all interrupts
> instead of error interrupts.

Applied, thanks
diff mbox series

Patch

diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
index d8426f915c3e..60dee30a4930 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -1554,7 +1554,7 @@  static void xilinx_dpdma_enable_irq(struct xilinx_dpdma_device *xdev)
  */
 static void xilinx_dpdma_disable_irq(struct xilinx_dpdma_device *xdev)
 {
-	dpdma_write(xdev->reg, XILINX_DPDMA_IDS, XILINX_DPDMA_INTR_ERR_ALL);
+	dpdma_write(xdev->reg, XILINX_DPDMA_IDS, XILINX_DPDMA_INTR_ALL);
 	dpdma_write(xdev->reg, XILINX_DPDMA_EIDS, XILINX_DPDMA_EINTR_ALL);
 }
 
@@ -1691,6 +1691,26 @@  static struct dma_chan *of_dma_xilinx_xlate(struct of_phandle_args *dma_spec,
 	return dma_get_slave_channel(&xdev->chan[chan_id]->vchan.chan);
 }
 
+static void dpdma_hw_init(struct xilinx_dpdma_device *xdev)
+{
+	unsigned int i;
+	void __iomem *reg;
+
+	/* Disable all interrupts */
+	xilinx_dpdma_disable_irq(xdev);
+
+	/* Stop all channels */
+	for (i = 0; i < ARRAY_SIZE(xdev->chan); i++) {
+		reg = xdev->reg + XILINX_DPDMA_CH_BASE
+				+ XILINX_DPDMA_CH_OFFSET * i;
+		dpdma_clr(reg, XILINX_DPDMA_CH_CNTL, XILINX_DPDMA_CH_CNTL_ENABLE);
+	}
+
+	/* Clear the interrupt status registers */
+	dpdma_write(xdev->reg, XILINX_DPDMA_ISR, XILINX_DPDMA_INTR_ALL);
+	dpdma_write(xdev->reg, XILINX_DPDMA_EISR, XILINX_DPDMA_EINTR_ALL);
+}
+
 static int xilinx_dpdma_probe(struct platform_device *pdev)
 {
 	struct xilinx_dpdma_device *xdev;
@@ -1717,6 +1737,8 @@  static int xilinx_dpdma_probe(struct platform_device *pdev)
 	if (IS_ERR(xdev->reg))
 		return PTR_ERR(xdev->reg);
 
+	dpdma_hw_init(xdev);
+
 	xdev->irq = platform_get_irq(pdev, 0);
 	if (xdev->irq < 0) {
 		dev_err(xdev->dev, "failed to get platform irq\n");