Hi Moritz,
On 02/21/2021 12:36 PM, Moritz Fischer wrote:
On Wed, Feb 17, 2021 at 10:40:18PM -0800, Lizhi Hou wrote:
Add partition isolation platform driver. partition isolation is
a hardware function discovered by walking firmware metadata.
A platform device node will be created for it. Partition isolation
function isolate the different fpga regions
Signed-off-by: Sonal Santan <sonal.santan@xxxxxxxxxx>
Signed-off-by: Max Zhen <max.zhen@xxxxxxxxxx>
Signed-off-by: Lizhi Hou <lizhih@xxxxxxxxxx>
---
drivers/fpga/xrt/include/xleaf/axigate.h | 25 ++
drivers/fpga/xrt/lib/xleaf/axigate.c | 298 +++++++++++++++++++++++
2 files changed, 323 insertions(+)
create mode 100644 drivers/fpga/xrt/include/xleaf/axigate.h
create mode 100644 drivers/fpga/xrt/lib/xleaf/axigate.c
diff --git a/drivers/fpga/xrt/include/xleaf/axigate.h b/drivers/fpga/xrt/include/xleaf/axigate.h
new file mode 100644
index 000000000000..2cef71e13b30
--- /dev/null
+++ b/drivers/fpga/xrt/include/xleaf/axigate.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for XRT Axigate Leaf Driver
+ *
+ * Copyright (C) 2020-2021 Xilinx, Inc.
+ *
+ * Authors:
+ * Lizhi Hou <Lizhi.Hou@xxxxxxxxxx>
+ */
+
+#ifndef _XRT_AXIGATE_H_
+#define _XRT_AXIGATE_H_
+
+#include "xleaf.h"
+#include "metadata.h"
+
+/*
+ * AXIGATE driver IOCTL calls.
+ */
+enum xrt_axigate_ioctl_cmd {
+ XRT_AXIGATE_FREEZE = XRT_XLEAF_CUSTOM_BASE, /* See comments in xleaf.h */
+ XRT_AXIGATE_FREE,
+};
+
+#endif /* _XRT_AXIGATE_H_ */
diff --git a/drivers/fpga/xrt/lib/xleaf/axigate.c b/drivers/fpga/xrt/lib/xleaf/axigate.c
new file mode 100644
index 000000000000..382969f9925f
--- /dev/null
+++ b/drivers/fpga/xrt/lib/xleaf/axigate.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx Alveo FPGA AXI Gate Driver
+ *
+ * Copyright (C) 2020-2021 Xilinx, Inc.
+ *
+ * Authors:
+ * Lizhi Hou<Lizhi.Hou@xxxxxxxxxx>
+ */
+
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include "metadata.h"
+#include "xleaf.h"
+#include "xleaf/axigate.h"
+
+#define XRT_AXIGATE "xrt_axigate"
+
+struct axigate_regs {
+ u32 iag_wr;
+ u32 iag_rvsd;
+ u32 iag_rd;
+} __packed;
Just make them #defines, even more so if there are only 3 of them.
We will use #define and regmap.
+
+struct xrt_axigate {
+ struct platform_device *pdev;
+ void *base;
+ struct mutex gate_lock; /* gate dev lock */
+
+ void *evt_hdl;
+ const char *ep_name;
+
+ bool gate_freezed;
+};
+
+/* the ep names are in the order of hardware layers */
+static const char * const xrt_axigate_epnames[] = {
+ XRT_MD_NODE_GATE_PLP,
+ XRT_MD_NODE_GATE_ULP,
+ NULL
+};
+
+#define reg_rd(g, r) \
+ ioread32((void *)(g)->base + offsetof(struct axigate_regs, r))
+#define reg_wr(g, v, r) \
+ iowrite32(v, (void *)(g)->base + offsetof(struct axigate_regs, r))
+
+static inline void freeze_gate(struct xrt_axigate *gate)
+{
+ reg_wr(gate, 0, iag_wr);
+ ndelay(500);
+ reg_rd(gate, iag_rd);
+}
+
+static inline void free_gate(struct xrt_axigate *gate)
+{
+ reg_wr(gate, 0x2, iag_wr);
+ ndelay(500);
Magic constants?
Will use #define for 500
+ (void)reg_rd(gate, iag_rd);
At the very least add a comment on why? Is this for PCI synchronization
reasons?
Will add comment. Some old board requires this extra read and it will
not hurt performance.
+ reg_wr(gate, 0x3, iag_wr);
+ ndelay(500);
Magic constants?
+ reg_rd(gate, iag_rd);
Does it nead a (void) or not? Be consistent, again, why do we read here
at all?
+}
+
+static int xrt_axigate_epname_idx(struct platform_device *pdev)
+{
+ int i;
+ int ret;
+ struct resource *res;
Nope. Indents:
struct resource *res;
int, i, ret;
Will change this.
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ xrt_err(pdev, "Empty Resource!");
+ return -EINVAL;
+ }
+
+ for (i = 0; xrt_axigate_epnames[i]; i++) {
+ ret = strncmp(xrt_axigate_epnames[i], res->name,
+ strlen(xrt_axigate_epnames[i]) + 1);
+ if (!ret)
+ break;
+ }
+
+ ret = (xrt_axigate_epnames[i]) ? i : -EINVAL;
Why not just:
if (xrt_axigate_epnames[i])
return i;
return -EINVAL;
Will change this.
+ return ret;
+}
+
+static void xrt_axigate_freeze(struct platform_device *pdev)
+{
+ struct xrt_axigate *gate;
+ u32 freeze = 0;
Indents. Fix everywhere.
Will fix this.
+
+ gate = platform_get_drvdata(pdev);
+
+ mutex_lock(&gate->gate_lock);
+ freeze = reg_rd(gate, iag_rd);
+ if (freeze) { /* gate is opened */
+ xleaf_broadcast_event(pdev, XRT_EVENT_PRE_GATE_CLOSE, false);
+ freeze_gate(gate);
+ }
+
+ gate->gate_freezed = true;
s/freezed/frozen
Will change terms to open / close.
+ mutex_unlock(&gate->gate_lock);
+
+ xrt_info(pdev, "freeze gate %s", gate->ep_name);
debug?
axigate is a very critical part for programming FPGA. We hope to have an
explicit printk to indicate the axigate open/close.
+}
+
+static void xrt_axigate_free(struct platform_device *pdev)
+{
+ struct xrt_axigate *gate;
+ u32 freeze;
+
+ gate = platform_get_drvdata(pdev);
+
+ mutex_lock(&gate->gate_lock);
+ freeze = reg_rd(gate, iag_rd);
+ if (!freeze) { /* gate is closed */
+ free_gate(gate);
+ xleaf_broadcast_event(pdev, XRT_EVENT_POST_GATE_OPEN, true);
+ /* xrt_axigate_free() could be called in event cb, thus
+ * we can not wait for the completes
+ */
+ }
+
+ gate->gate_freezed = false;
+ mutex_unlock(&gate->gate_lock);
+
+ xrt_info(pdev, "free gate %s", gate->ep_name);
+}
+
+static void xrt_axigate_event_cb(struct platform_device *pdev, void *arg)
+{
+ struct platform_device *leaf;
+ struct xrt_event *evt = (struct xrt_event *)arg;
+ enum xrt_events e = evt->xe_evt;
+ enum xrt_subdev_id id = evt->xe_subdev.xevt_subdev_id;
+ int instance = evt->xe_subdev.xevt_subdev_instance;
+ struct xrt_axigate *gate = platform_get_drvdata(pdev);
+ struct resource *res;
Reverse x-mas tree;
xxxxxxxxxx
xxxxxxxxx
xxxxxxxx
xxxxxx
Will fix this.
+
+ switch (e) {
+ case XRT_EVENT_POST_CREATION:
+ break;
+ default:
+ return;
+ }
+
+ if (id != XRT_SUBDEV_AXIGATE)
+ return;
+
+ leaf = xleaf_get_leaf_by_id(pdev, id, instance);
+ if (!leaf)
+ return;
+
+ res = platform_get_resource(leaf, IORESOURCE_MEM, 0);
+ if (!res || !strncmp(res->name, gate->ep_name, strlen(res->name) + 1)) {
+ (void)xleaf_put_leaf(pdev, leaf);
+ return;
+ }
+
+ /*
+ * higher level axigate instance created,
+ * make sure the gate is openned. This covers 1RP flow which
+ * has plp gate as well.
+ */
+ if (xrt_axigate_epname_idx(leaf) > xrt_axigate_epname_idx(pdev))
+ xrt_axigate_free(pdev);
+ else
+ xleaf_ioctl(leaf, XRT_AXIGATE_FREE, NULL);
+
+ (void)xleaf_put_leaf(pdev, leaf);
+}
+
+static int
+xrt_axigate_leaf_ioctl(struct platform_device *pdev, u32 cmd, void *arg)
+{
+ switch (cmd) {
+ case XRT_XLEAF_EVENT:
+ xrt_axigate_event_cb(pdev, arg);
+ break;
+ case XRT_AXIGATE_FREEZE:
+ xrt_axigate_freeze(pdev);
+ break;
+ case XRT_AXIGATE_FREE:
+ xrt_axigate_free(pdev);
+ break;
+ default:
+ xrt_err(pdev, "unsupported cmd %d", cmd);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int xrt_axigate_remove(struct platform_device *pdev)
+{
+ struct xrt_axigate *gate;
+
+ gate = platform_get_drvdata(pdev);
+
+ if (gate->base)
+ iounmap(gate->base);
+
+ platform_set_drvdata(pdev, NULL);
+ devm_kfree(&pdev->dev, gate);
No! The point of using devres is so cleanup happens on removal.
While you're at it, if you move the ioremap to a devres version, this
function can basically go away entirely.
Will fix this.
+
+ return 0;
+}
+
+static int xrt_axigate_probe(struct platform_device *pdev)
+{
+ struct xrt_axigate *gate;
+ struct resource *res;
+ int ret;
+
+ gate = devm_kzalloc(&pdev->dev, sizeof(*gate), GFP_KERNEL);
+ if (!gate)
+ return -ENOMEM;
+
+ gate->pdev = pdev;
+ platform_set_drvdata(pdev, gate);
+
+ xrt_info(pdev, "probing...");
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ xrt_err(pdev, "Empty resource 0");
+ ret = -EINVAL;
+ goto failed;
+ }
+
+ gate->base = ioremap(res->start, res->end - res->start + 1);
+ if (!gate->base) {
+ xrt_err(pdev, "map base iomem failed");
+ ret = -EFAULT;
+ goto failed;
+ }
+
+ gate->ep_name = res->name;
+
+ mutex_init(&gate->gate_lock);
+
+ return 0;
+
+failed:
+ xrt_axigate_remove(pdev);
+ return ret;
+}
+
+static struct xrt_subdev_endpoints xrt_axigate_endpoints[] = {
+ {
+ .xse_names = (struct xrt_subdev_ep_names[]) {
+ { .ep_name = "ep_pr_isolate_ulp_00" },
+ { NULL },
+ },
+ .xse_min_ep = 1,
+ },
+ {
+ .xse_names = (struct xrt_subdev_ep_names[]) {
+ { .ep_name = "ep_pr_isolate_plp_00" },
+ { NULL },
+ },
+ .xse_min_ep = 1,
+ },
+ { 0 },
+};
+
+static struct xrt_subdev_drvdata xrt_axigate_data = {
+ .xsd_dev_ops = {
+ .xsd_ioctl = xrt_axigate_leaf_ioctl,
+ },
+};
+
+static const struct platform_device_id xrt_axigate_table[] = {
+ { XRT_AXIGATE, (kernel_ulong_t)&xrt_axigate_data },
+ { },
+};
+
+static struct platform_driver xrt_axigate_driver = {
+ .driver = {
+ .name = XRT_AXIGATE,
+ },
+ .probe = xrt_axigate_probe,
+ .remove = xrt_axigate_remove,
+ .id_table = xrt_axigate_table,
+};
+
+void axigate_leaf_init_fini(bool init)
+{
+ if (init) {
+ xleaf_register_driver(XRT_SUBDEV_AXIGATE,
+ &xrt_axigate_driver, xrt_axigate_endpoints);
+ } else {
+ xleaf_unregister_driver(XRT_SUBDEV_AXIGATE);
+ }
+}
This thing is duplicated in every file, maybe a macro would be an idea.
Will define a macro.
Thanks,
Lizhi
--
2.18.4
- Moritz
