Messages in this thread | | | Subject | Re: [PATCH V3 XRT Alveo 17/18] fpga: xrt: partition isolation platform driver | From | Lizhi Hou <> | Date | Fri, 12 Mar 2021 22:53:41 -0800 |
| |
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@xilinx.com> >> Signed-off-by: Max Zhen <max.zhen@xilinx.com> >> Signed-off-by: Lizhi Hou <lizhih@xilinx.com> >> --- >> 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@xilinx.com> >> + */ >> + >> +#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@xilinx.com> >> + */ >> + >> +#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); >> + } >> +}
| |