Re: [PATCH V4 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic interrupt support
From: Lizhi Hou
Date: Tue Sep 27 2022 - 12:30:09 EST
On 9/27/22 08:31, Martin Tůma wrote:
On 22. 09. 22 20:38, Lizhi Hou wrote:
The Xilinx DMA/Bridge Subsystem for PCIe (XDMA) provides up to 16 user
interrupt wires to user logic that generate interrupts to the host.
This patch adds APIs to register/unregister interrupt handler for a
given
interrupt wire index.
Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
Signed-off-by: Sonal Santan <sonal.santan@xxxxxxx>
Signed-off-by: Max Zhen <max.zhen@xxxxxxx>
Signed-off-by: Brian Xu <brian.xu@xxxxxxx>
---
MAINTAINERS | 1 +
drivers/dma/xilinx/xdma.c | 80 ++++++++++++++++++++++++++++++++++++
include/linux/dma/amd_xdma.h | 17 ++++++++
3 files changed, 98 insertions(+)
create mode 100644 include/linux/dma/amd_xdma.h
diff --git a/MAINTAINERS b/MAINTAINERS
index c1be0b2e378a..019d84b2b086 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21691,6 +21691,7 @@ L: dmaengine@xxxxxxxxxxxxxxx
S: Supported
F: drivers/dma/xilinx/xdma-regs.h
F: drivers/dma/xilinx/xdma.c
+F: include/linux/dma/amd_xdma.h
F: include/linux/platform_data/amd_xdma.h
XILINX ZYNQMP DPDMA DRIVER
diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index 58a57e03bef5..884942cd2a37 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -25,6 +25,7 @@
#include <linux/dmapool.h>
#include <linux/regmap.h>
#include <linux/dmaengine.h>
+#include <linux/dma/amd_xdma.h>
#include <linux/platform_device.h>
#include <linux/platform_data/amd_xdma.h>
#include <linux/dma-mapping.h>
@@ -736,6 +737,7 @@ static int xdma_set_vector_reg(struct xdma_device
*xdev, u32 vec_tbl_start,
static int xdma_irq_init(struct xdma_device *xdev)
{
u32 irq = xdev->irq_start;
+ u32 user_irq_start;
int i, j, ret;
/* return failure if there are not enough IRQs */
@@ -786,6 +788,18 @@ static int xdma_irq_init(struct xdma_device *xdev)
goto failed;
}
+ /* config user IRQ registers if needed */
+ user_irq_start = xdev->h2c_chan_num + xdev->c2h_chan_num;
+ if (xdev->irq_num > user_irq_start) {
+ ret = xdma_set_vector_reg(xdev, XDMA_IRQ_USER_VEC_NUM,
+ user_irq_start,
+ xdev->irq_num - user_irq_start);
+ if (ret) {
+ xdma_err(xdev, "failed to set user vectors: %d", ret);
+ goto failed;
+ }
+ }
+
/* enable interrupt */
ret = xdma_enable_intr(xdev);
if (ret) {
@@ -816,6 +830,72 @@ static bool xdma_filter_fn(struct dma_chan
*chan, void *param)
return true;
}
+/**
+ * xdma_free_user_irq - Free user interrupt
+ * @pdev: Pointer to the platform_device structure
+ * @user_irq_index: User IRQ index
+ * @arg: User interrupt cookie
+ */
+void xdma_free_user_irq(struct platform_device *pdev, u32
user_irq_index,
+ void *arg)
+{
+ struct xdma_device *xdev = platform_get_drvdata(pdev);
+ u32 user_irq;
+
+ user_irq = xdev->h2c_chan_num + xdev->c2h_chan_num +
user_irq_index;
+ if (user_irq >= xdev->irq_num) {
+ xdma_err(xdev, "invalid user irq index");
+ return;
+ }
+ user_irq += xdev->irq_start;
+
+ xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_USER_INT_EN_W1C,
+ (1 << user_irq_index));
+
+ free_irq(user_irq, arg);
+}
+EXPORT_SYMBOL(xdma_free_user_irq);
+
+/**
+ * xdma_request_user_irq - Register user interrupt
+ * @pdev: Pointer to the platform_device structure
+ * @user_irq_index: User IRQ index
+ * @flags: Handling flags
+ * @handler: User interrupt handler
+ * @arg: User interrupt cookie
+ */
+int xdma_request_user_irq(struct platform_device *pdev, u32
user_irq_index,
+ irq_handler_t handler, void *arg, ulong flags)
+{
+ struct xdma_device *xdev = platform_get_drvdata(pdev);
+ u32 user_irq;
+ int ret;
+
+ user_irq = xdev->h2c_chan_num + xdev->c2h_chan_num +
user_irq_index;
+ if (user_irq >= xdev->irq_num) {
+ xdma_err(xdev, "invalid user irq index");
+ return -EINVAL;
+ }
+ user_irq += xdev->irq_start;
+
+ ret = request_irq(user_irq, handler, flags, "xdma-user", arg);
+ if (ret) {
+ xdma_err(xdev, "request irq failed, %d", ret);
+ return ret;
+ }
+
+ ret = xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_USER_INT_EN_W1S,
+ (1 << user_irq_index));
+ if (ret) {
+ xdma_err(xdev, "set user irq mask failed, %d", ret);
+ free_irq(user_irq, arg);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(xdma_request_user_irq);
+
/**
* xdma_remove - Driver remove function
* @pdev: Pointer to the platform_device structure
diff --git a/include/linux/dma/amd_xdma.h b/include/linux/dma/amd_xdma.h
new file mode 100644
index 000000000000..b70486121ca6
--- /dev/null
+++ b/include/linux/dma/amd_xdma.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2022, Advanced Micro Devices, Inc.
+ */
+
+#ifndef _DMAENGINE_AMD_XDMA_H
+#define _DMAENGINE_AMD_XDMA_H
+
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+
+int xdma_request_user_irq(struct platform_device *pdev, u32
user_irq_index,
+ irq_handler_t handler, void *arg, ulong flags);
+void xdma_free_user_irq(struct platform_device *pdev, u32
user_irq_index,
+ void *arg);
+
+#endif /* _DMAENGINE_AMD_XDMA_H */
Hi,
The user IRQ logic is unusable, if you have other IP cores with linux
drivers (eg. spi or i2c) connected to that IRQs like we (and many
other users as well) have on our PCIe card. You can't use the already
existing
xiic or xilinx-spi drivers with your xdma driver!
You must split the "IRQ enable" and "IRQ request" operations for your
driver to be usable with the other platform drivers in linux. Instead of
"xdma_request_user_irq" there must be something like
"xdma_enable_user_irq" that only enables the IRQ in the IP block, but
does not "eat" the IRQ so you can pass it to the initialization of
the platform driver of xiic or xilinx-spi.
M.
Okay, I got the point. How about changing request/remove APIs to
enable/disable APIs as below
xdma_enable_user_irq(struct platform_device *pdev, u32
user_irq_index, u32 *irq)
user_irq_index: user logic interrupt wire index. (XDMA
driver determines how system IRQs are mapped to DMA channels and user
logic wires)
irq: IRQ number returned for registering interrupt handler
(request_irq()) or passing to existing platform driver.
xdma_disable_user_irq(struct platform_device *pdev, u32 user_irq_index)
Does this make sense to you?
Thanks,
Lizhi