Re: [PATCH V3 XRT Alveo 17/18] fpga: xrt: partition isolation platform driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tom,

On 03/06/2021 07:54 AM, Tom Rix wrote:
On 2/17/21 10:40 PM, 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,
These are substrings, could change suffix to make it harder for developer to mix up.
Will change 'freeze' to 'close', 'free' to 'open'.
+};
+
+#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;
similar to other patches, prefix of element is not needed.
Will remove this and 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,
what are plp, ulp ? it is helpful to comment or expand acronyms
plp stands for provider logic partition and ulp stands for user logic partition. Will add comment.
+     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);
The values written here and below are magic, the need to have #defines
Will add #defines
+     ndelay(500);
+     reg_rd(gate, iag_rd);
+}
+
+static inline void free_gate(struct xrt_axigate *gate)
+{
+     reg_wr(gate, 0x2, iag_wr);
+     ndelay(500);
+     (void)reg_rd(gate, iag_rd);
+     reg_wr(gate, 0x3, iag_wr);
+     ndelay(500);
+     reg_rd(gate, iag_rd);
+}
+
+static int xrt_axigate_epname_idx(struct platform_device *pdev)
+{
+     int                     i;
+     int                     ret;
int i, ret;
sure.
+     struct resource         *res;
+
+     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++) {
null guarded array is useful with the size isn't know,

in this case it is, so covert loop to using ARRAY_SIZE
Will use ARRAY_SIZE.

+             ret = strncmp(xrt_axigate_epnames[i], res->name,
+                           strlen(xrt_axigate_epnames[i]) + 1);
needs a strlen check in case res->name is just a substring
'strlen() + 1' is used, thus the comparing covers substring as well.
+             if (!ret)
+                     break;
+     }
+
+     ret = (xrt_axigate_epnames[i]) ? i : -EINVAL;
+     return ret;
+}
+
+static void xrt_axigate_freeze(struct platform_device *pdev)
+{
+     struct xrt_axigate      *gate;
+     u32                     freeze = 0;
+
+     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;
Looks like freeze could be 0, so is setting gate_freeze = true correct all the time ?
added checking for freeze_gate(). if it succeed, gate will be frozen (closed).
+     mutex_unlock(&gate->gate_lock);
+
+     xrt_info(pdev, "freeze gate %s", gate->ep_name);
+}
+
+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;
freezed is not a word, the element name should be 'gate_frozen'
Will change to gate_closed.
+     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;
+
+     switch (e) {
+     case XRT_EVENT_POST_CREATION:
+             break;
+     default:
+             return;
+     }
convert switch() to if ()
Sure.
+
+     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
is openned -> is opened
sure.

what is 1RP ?
1RP flow is one of hardware shell build flow. It is xilinx internal term I will remove this sentence.

Thanks,
Lizhi

Tom

+      * 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);
+
+     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);
+     }
+}




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux