Re: [PATCH V3 XRT Alveo 13/18] fpga: xrt: devctl platform driver
On 3/16/21 4:54 PM, Lizhi Hou wrote: > > > On 03/04/2021 05:39 AM, Tom Rix wrote: >> CAUTION: This message has originated from an External Source. Please use >> proper judgment and caution when opening attachments, clicking links, or >> responding to this email. >> >> >> On 2/17/21 10:40 PM, Lizhi Hou wrote: >>> Add devctl driver. devctl is a type of hardware function which only has >>> few registers to read or write. They are discovered by walking firmware >>> metadata. A platform device node will be created for them. >>> >>> Signed-off-by: Sonal Santan >>> Signed-off-by: Max Zhen >>> Signed-off-by: Lizhi Hou >>> --- >>> drivers/fpga/xrt/include/xleaf/devctl.h | 43 + >>> drivers/fpga/xrt/lib/xleaf/devctl.c | 206 >>> 2 files changed, 249 insertions(+) >>> create mode 100644 drivers/fpga/xrt/include/xleaf/devctl.h >>> create mode 100644 drivers/fpga/xrt/lib/xleaf/devctl.c >>> >>> diff --git a/drivers/fpga/xrt/include/xleaf/devctl.h >>> b/drivers/fpga/xrt/include/xleaf/devctl.h >>> new file mode 100644 >>> index ..96a40e066f83 >>> --- /dev/null >>> +++ b/drivers/fpga/xrt/include/xleaf/devctl.h >>> @@ -0,0 +1,43 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +/* >>> + * Header file for XRT DEVCTL Leaf Driver >>> + * >>> + * Copyright (C) 2020-2021 Xilinx, Inc. >>> + * >>> + * Authors: >>> + * Lizhi Hou >>> + */ >>> + >>> +#ifndef _XRT_DEVCTL_H_ >>> +#define _XRT_DEVCTL_H_ >>> + >>> +#include "xleaf.h" >>> + >>> +/* >>> + * DEVCTL driver IOCTL calls. >>> + */ >>> +enum xrt_devctl_ioctl_cmd { >>> + XRT_DEVCTL_READ = XRT_XLEAF_CUSTOM_BASE, /* See comments in xleaf.h */ >>> + XRT_DEVCTL_WRITE, >>> +}; >>> + >>> +enum xrt_devctl_id { >>> + XRT_DEVCTL_ROM_UUID, >> Assumes 0, should make this explicit and initialize to 0 > Sure. >>> + XRT_DEVCTL_DDR_CALIB, >>> + XRT_DEVCTL_GOLDEN_VER, >>> + XRT_DEVCTL_MAX >>> +}; >>> + >>> +struct xrt_devctl_ioctl_rw { >>> + u32 xgir_id; >>> + void *xgir_buf; >>> + u32 xgir_len; >>> + u32 xgir_offset; >> similar to other patches, the xgir_ prefix is not needed >>> +}; >>> + >>> +struct xrt_devctl_ioctl_intf_uuid { >>> + u32 xgir_uuid_num; >>> + uuid_t *xgir_uuids; >>> +}; >>> + >>> +#endif /* _XRT_DEVCTL_H_ */ >>> diff --git a/drivers/fpga/xrt/lib/xleaf/devctl.c >>> b/drivers/fpga/xrt/lib/xleaf/devctl.c >>> new file mode 100644 >>> index ..caf8c6569f0f >>> --- /dev/null >>> +++ b/drivers/fpga/xrt/lib/xleaf/devctl.c >>> @@ -0,0 +1,206 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * Xilinx Alveo FPGA devctl Driver >>> + * >>> + * Copyright (C) 2020-2021 Xilinx, Inc. >>> + * >>> + * Authors: >>> + * Lizhi Hou >>> + */ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include "metadata.h" >>> +#include "xleaf.h" >>> +#include "xleaf/devctl.h" >>> + >>> +#define XRT_DEVCTL "xrt_devctl" >>> + >>> +struct xrt_name_id { >>> + char *ep_name; >>> + int id; >>> +}; >>> + >>> +static struct xrt_name_id name_id[XRT_DEVCTL_MAX] = { >>> + { XRT_MD_NODE_BLP_ROM, XRT_DEVCTL_ROM_UUID }, >>> + { XRT_MD_NODE_GOLDEN_VER, XRT_DEVCTL_GOLDEN_VER }, >> DDR_CALIB is unused ? > Not sure if I understand the question correctly. ddr_calib will have more > things need to handle other than just read/write. I do not understand either, ignore this comment. If it is important, I will bring it up in a later revision's review and do a better job of explaining the issue. Tom >>> +}; >>> + >>> +struct xrt_devctl { >>> + struct platform_device *pdev; >>> + void __iomem *base_addrs[XRT_DEVCTL_MAX]; >>> + ulong sizes[XRT_DEVCTL_MAX]; >>> +}; >> similar to other patches, why not use regmap ? > Will change to regmap. >>> + >>> +static int xrt_devctl_name2id(struct xrt_devctl *devctl, const char *name) >>> +{ >>> + int i; >>> + >>> + for (i = 0; i < XRT_DEVCTL_MAX && name_id[i].ep_name; i++) { >>> + if (!strncmp(name_id[i].ep_name, name, >>> strlen(name_id[i].ep_name) + 1)) >>> + return name_id[i].id; >>> + } >>> + >>> + return -EINVAL; >>> +} >>> + >>> +static int >>> +xrt_devctl_leaf_ioctl(struct platform_device *pdev, u32 cmd, void *arg) >>> +{ >>> + struct xrt_devctl *devctl; >>> + int ret = 0; >>> + >>> + devctl = platform_get_drvdata(pdev); >>> + >>> + switch (cmd) { >>> + case XRT_XLEAF_EVENT: >>> + /* Does not handle any event. */ >>> + break; >>> + case XRT_DEVCTL_READ: { >>> + struct xrt_devctl_ioctl_rw *rw_arg = arg; >>> + u32 *p_src, *p_dst, i; >>> + >>> + if (rw_arg->xgir_len & 0x3) { >>> + xrt_err(pdev, "invalid len %d", rw_arg->xgir_len); >>> + return -EINVAL; >>> + } >>> + >>> + if (rw
Re: [PATCH V3 XRT Alveo 13/18] fpga: xrt: devctl platform driver
On 03/04/2021 05:39 AM, Tom Rix wrote: CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email. On 2/17/21 10:40 PM, Lizhi Hou wrote: Add devctl driver. devctl is a type of hardware function which only has few registers to read or write. They are discovered by walking firmware metadata. A platform device node will be created for them. Signed-off-by: Sonal Santan Signed-off-by: Max Zhen Signed-off-by: Lizhi Hou --- drivers/fpga/xrt/include/xleaf/devctl.h | 43 + drivers/fpga/xrt/lib/xleaf/devctl.c | 206 2 files changed, 249 insertions(+) create mode 100644 drivers/fpga/xrt/include/xleaf/devctl.h create mode 100644 drivers/fpga/xrt/lib/xleaf/devctl.c diff --git a/drivers/fpga/xrt/include/xleaf/devctl.h b/drivers/fpga/xrt/include/xleaf/devctl.h new file mode 100644 index ..96a40e066f83 --- /dev/null +++ b/drivers/fpga/xrt/include/xleaf/devctl.h @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Header file for XRT DEVCTL Leaf Driver + * + * Copyright (C) 2020-2021 Xilinx, Inc. + * + * Authors: + * Lizhi Hou + */ + +#ifndef _XRT_DEVCTL_H_ +#define _XRT_DEVCTL_H_ + +#include "xleaf.h" + +/* + * DEVCTL driver IOCTL calls. + */ +enum xrt_devctl_ioctl_cmd { + XRT_DEVCTL_READ = XRT_XLEAF_CUSTOM_BASE, /* See comments in xleaf.h */ + XRT_DEVCTL_WRITE, +}; + +enum xrt_devctl_id { + XRT_DEVCTL_ROM_UUID, Assumes 0, should make this explicit and initialize to 0 Sure. + XRT_DEVCTL_DDR_CALIB, + XRT_DEVCTL_GOLDEN_VER, + XRT_DEVCTL_MAX +}; + +struct xrt_devctl_ioctl_rw { + u32 xgir_id; + void*xgir_buf; + u32 xgir_len; + u32 xgir_offset; similar to other patches, the xgir_ prefix is not needed +}; + +struct xrt_devctl_ioctl_intf_uuid { + u32 xgir_uuid_num; + uuid_t *xgir_uuids; +}; + +#endif /* _XRT_DEVCTL_H_ */ diff --git a/drivers/fpga/xrt/lib/xleaf/devctl.c b/drivers/fpga/xrt/lib/xleaf/devctl.c new file mode 100644 index ..caf8c6569f0f --- /dev/null +++ b/drivers/fpga/xrt/lib/xleaf/devctl.c @@ -0,0 +1,206 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Xilinx Alveo FPGA devctl Driver + * + * Copyright (C) 2020-2021 Xilinx, Inc. + * + * Authors: + * Lizhi Hou + */ + +#include +#include +#include +#include +#include +#include "metadata.h" +#include "xleaf.h" +#include "xleaf/devctl.h" + +#define XRT_DEVCTL "xrt_devctl" + +struct xrt_name_id { + char *ep_name; + int id; +}; + +static struct xrt_name_id name_id[XRT_DEVCTL_MAX] = { + { XRT_MD_NODE_BLP_ROM, XRT_DEVCTL_ROM_UUID }, + { XRT_MD_NODE_GOLDEN_VER, XRT_DEVCTL_GOLDEN_VER }, DDR_CALIB is unused ? Not sure if I understand the question correctly. ddr_calib will have more things need to handle other than just read/write. +}; + +struct xrt_devctl { + struct platform_device *pdev; + void__iomem *base_addrs[XRT_DEVCTL_MAX]; + ulong sizes[XRT_DEVCTL_MAX]; +}; similar to other patches, why not use regmap ? Will change to regmap. + +static int xrt_devctl_name2id(struct xrt_devctl *devctl, const char *name) +{ + int i; + + for (i = 0; i < XRT_DEVCTL_MAX && name_id[i].ep_name; i++) { + if (!strncmp(name_id[i].ep_name, name, strlen(name_id[i].ep_name) + 1)) + return name_id[i].id; + } + + return -EINVAL; +} + +static int +xrt_devctl_leaf_ioctl(struct platform_device *pdev, u32 cmd, void *arg) +{ + struct xrt_devctl *devctl; + int ret = 0; + + devctl = platform_get_drvdata(pdev); + + switch (cmd) { + case XRT_XLEAF_EVENT: + /* Does not handle any event. */ + break; + case XRT_DEVCTL_READ: { + struct xrt_devctl_ioctl_rw *rw_arg = arg; + u32 *p_src, *p_dst, i; + + if (rw_arg->xgir_len & 0x3) { + xrt_err(pdev, "invalid len %d", rw_arg->xgir_len); + return -EINVAL; + } + + if (rw_arg->xgir_id >= XRT_DEVCTL_MAX) { + xrt_err(pdev, "invalid id %d", rw_arg->xgir_id); + return -EINVAL; + } needs a < 0 check ? change xgir_id to u32. + + p_src = devctl->base_addrs[rw_arg->xgir_id]; + if (!p_src) { + xrt_err(pdev, "io not found, id %d", + rw_arg->xgir_id); + return -EINVAL; + } + if (rw_arg->xgir_offset + rw_arg->xgir_len > + devctl->sizes[rw_arg->xgir_id]) { + xrt_err(pdev, "invalid argument, off %d, len %d", + rw_arg->xgir_offset, rw_arg->xgir_len); + return -EINVAL; + } + p_dst = rw_arg->xgir_buf; +
Re: [PATCH V3 XRT Alveo 13/18] fpga: xrt: devctl platform driver
On 2/17/21 10:40 PM, Lizhi Hou wrote: > Add devctl driver. devctl is a type of hardware function which only has > few registers to read or write. They are discovered by walking firmware > metadata. A platform device node will be created for them. > > Signed-off-by: Sonal Santan > Signed-off-by: Max Zhen > Signed-off-by: Lizhi Hou > --- > drivers/fpga/xrt/include/xleaf/devctl.h | 43 + > drivers/fpga/xrt/lib/xleaf/devctl.c | 206 > 2 files changed, 249 insertions(+) > create mode 100644 drivers/fpga/xrt/include/xleaf/devctl.h > create mode 100644 drivers/fpga/xrt/lib/xleaf/devctl.c > > diff --git a/drivers/fpga/xrt/include/xleaf/devctl.h > b/drivers/fpga/xrt/include/xleaf/devctl.h > new file mode 100644 > index ..96a40e066f83 > --- /dev/null > +++ b/drivers/fpga/xrt/include/xleaf/devctl.h > @@ -0,0 +1,43 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Header file for XRT DEVCTL Leaf Driver > + * > + * Copyright (C) 2020-2021 Xilinx, Inc. > + * > + * Authors: > + * Lizhi Hou > + */ > + > +#ifndef _XRT_DEVCTL_H_ > +#define _XRT_DEVCTL_H_ > + > +#include "xleaf.h" > + > +/* > + * DEVCTL driver IOCTL calls. > + */ > +enum xrt_devctl_ioctl_cmd { > + XRT_DEVCTL_READ = XRT_XLEAF_CUSTOM_BASE, /* See comments in xleaf.h */ > + XRT_DEVCTL_WRITE, > +}; > + > +enum xrt_devctl_id { > + XRT_DEVCTL_ROM_UUID, Assumes 0, should make this explicit and initialize to 0 > + XRT_DEVCTL_DDR_CALIB, > + XRT_DEVCTL_GOLDEN_VER, > + XRT_DEVCTL_MAX > +}; > + > +struct xrt_devctl_ioctl_rw { > + u32 xgir_id; > + void*xgir_buf; > + u32 xgir_len; > + u32 xgir_offset; similar to other patches, the xgir_ prefix is not needed > +}; > + > +struct xrt_devctl_ioctl_intf_uuid { > + u32 xgir_uuid_num; > + uuid_t *xgir_uuids; > +}; > + > +#endif /* _XRT_DEVCTL_H_ */ > diff --git a/drivers/fpga/xrt/lib/xleaf/devctl.c > b/drivers/fpga/xrt/lib/xleaf/devctl.c > new file mode 100644 > index ..caf8c6569f0f > --- /dev/null > +++ b/drivers/fpga/xrt/lib/xleaf/devctl.c > @@ -0,0 +1,206 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Xilinx Alveo FPGA devctl Driver > + * > + * Copyright (C) 2020-2021 Xilinx, Inc. > + * > + * Authors: > + * Lizhi Hou > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include "metadata.h" > +#include "xleaf.h" > +#include "xleaf/devctl.h" > + > +#define XRT_DEVCTL "xrt_devctl" > + > +struct xrt_name_id { > + char *ep_name; > + int id; > +}; > + > +static struct xrt_name_id name_id[XRT_DEVCTL_MAX] = { > + { XRT_MD_NODE_BLP_ROM, XRT_DEVCTL_ROM_UUID }, > + { XRT_MD_NODE_GOLDEN_VER, XRT_DEVCTL_GOLDEN_VER }, DDR_CALIB is unused ? > +}; > + > +struct xrt_devctl { > + struct platform_device *pdev; > + void__iomem *base_addrs[XRT_DEVCTL_MAX]; > + ulong sizes[XRT_DEVCTL_MAX]; > +}; similar to other patches, why not use regmap ? > + > +static int xrt_devctl_name2id(struct xrt_devctl *devctl, const char *name) > +{ > + int i; > + > + for (i = 0; i < XRT_DEVCTL_MAX && name_id[i].ep_name; i++) { > + if (!strncmp(name_id[i].ep_name, name, > strlen(name_id[i].ep_name) + 1)) > + return name_id[i].id; > + } > + > + return -EINVAL; > +} > + > +static int > +xrt_devctl_leaf_ioctl(struct platform_device *pdev, u32 cmd, void *arg) > +{ > + struct xrt_devctl *devctl; > + int ret = 0; > + > + devctl = platform_get_drvdata(pdev); > + > + switch (cmd) { > + case XRT_XLEAF_EVENT: > + /* Does not handle any event. */ > + break; > + case XRT_DEVCTL_READ: { > + struct xrt_devctl_ioctl_rw *rw_arg = arg; > + u32 *p_src, *p_dst, i; > + > + if (rw_arg->xgir_len & 0x3) { > + xrt_err(pdev, "invalid len %d", rw_arg->xgir_len); > + return -EINVAL; > + } > + > + if (rw_arg->xgir_id >= XRT_DEVCTL_MAX) { > + xrt_err(pdev, "invalid id %d", rw_arg->xgir_id); > + return -EINVAL; > + } needs a < 0 check ? > + > + p_src = devctl->base_addrs[rw_arg->xgir_id]; > + if (!p_src) { > + xrt_err(pdev, "io not found, id %d", > + rw_arg->xgir_id); > + return -EINVAL; > + } > + if (rw_arg->xgir_offset + rw_arg->xgir_len > > + devctl->sizes[rw_arg->xgir_id]) { > + xrt_err(pdev, "invalid argument, off %d, len %d", > + rw_arg->xgir_offset, rw_arg->xgir_len); > + return -EINVAL; > + } > + p_dst = rw_arg->xgir_buf; > + for (i = 0; i < rw_arg->xgir_len / sizeof(u32); i++) { > + u32
[PATCH V3 XRT Alveo 13/18] fpga: xrt: devctl platform driver
Add devctl driver. devctl is a type of hardware function which only has few registers to read or write. They are discovered by walking firmware metadata. A platform device node will be created for them. Signed-off-by: Sonal Santan Signed-off-by: Max Zhen Signed-off-by: Lizhi Hou --- drivers/fpga/xrt/include/xleaf/devctl.h | 43 + drivers/fpga/xrt/lib/xleaf/devctl.c | 206 2 files changed, 249 insertions(+) create mode 100644 drivers/fpga/xrt/include/xleaf/devctl.h create mode 100644 drivers/fpga/xrt/lib/xleaf/devctl.c diff --git a/drivers/fpga/xrt/include/xleaf/devctl.h b/drivers/fpga/xrt/include/xleaf/devctl.h new file mode 100644 index ..96a40e066f83 --- /dev/null +++ b/drivers/fpga/xrt/include/xleaf/devctl.h @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Header file for XRT DEVCTL Leaf Driver + * + * Copyright (C) 2020-2021 Xilinx, Inc. + * + * Authors: + * Lizhi Hou + */ + +#ifndef _XRT_DEVCTL_H_ +#define _XRT_DEVCTL_H_ + +#include "xleaf.h" + +/* + * DEVCTL driver IOCTL calls. + */ +enum xrt_devctl_ioctl_cmd { + XRT_DEVCTL_READ = XRT_XLEAF_CUSTOM_BASE, /* See comments in xleaf.h */ + XRT_DEVCTL_WRITE, +}; + +enum xrt_devctl_id { + XRT_DEVCTL_ROM_UUID, + XRT_DEVCTL_DDR_CALIB, + XRT_DEVCTL_GOLDEN_VER, + XRT_DEVCTL_MAX +}; + +struct xrt_devctl_ioctl_rw { + u32 xgir_id; + void*xgir_buf; + u32 xgir_len; + u32 xgir_offset; +}; + +struct xrt_devctl_ioctl_intf_uuid { + u32 xgir_uuid_num; + uuid_t *xgir_uuids; +}; + +#endif /* _XRT_DEVCTL_H_ */ diff --git a/drivers/fpga/xrt/lib/xleaf/devctl.c b/drivers/fpga/xrt/lib/xleaf/devctl.c new file mode 100644 index ..caf8c6569f0f --- /dev/null +++ b/drivers/fpga/xrt/lib/xleaf/devctl.c @@ -0,0 +1,206 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Xilinx Alveo FPGA devctl Driver + * + * Copyright (C) 2020-2021 Xilinx, Inc. + * + * Authors: + * Lizhi Hou + */ + +#include +#include +#include +#include +#include +#include "metadata.h" +#include "xleaf.h" +#include "xleaf/devctl.h" + +#define XRT_DEVCTL "xrt_devctl" + +struct xrt_name_id { + char *ep_name; + int id; +}; + +static struct xrt_name_id name_id[XRT_DEVCTL_MAX] = { + { XRT_MD_NODE_BLP_ROM, XRT_DEVCTL_ROM_UUID }, + { XRT_MD_NODE_GOLDEN_VER, XRT_DEVCTL_GOLDEN_VER }, +}; + +struct xrt_devctl { + struct platform_device *pdev; + void__iomem *base_addrs[XRT_DEVCTL_MAX]; + ulong sizes[XRT_DEVCTL_MAX]; +}; + +static int xrt_devctl_name2id(struct xrt_devctl *devctl, const char *name) +{ + int i; + + for (i = 0; i < XRT_DEVCTL_MAX && name_id[i].ep_name; i++) { + if (!strncmp(name_id[i].ep_name, name, strlen(name_id[i].ep_name) + 1)) + return name_id[i].id; + } + + return -EINVAL; +} + +static int +xrt_devctl_leaf_ioctl(struct platform_device *pdev, u32 cmd, void *arg) +{ + struct xrt_devctl *devctl; + int ret = 0; + + devctl = platform_get_drvdata(pdev); + + switch (cmd) { + case XRT_XLEAF_EVENT: + /* Does not handle any event. */ + break; + case XRT_DEVCTL_READ: { + struct xrt_devctl_ioctl_rw *rw_arg = arg; + u32 *p_src, *p_dst, i; + + if (rw_arg->xgir_len & 0x3) { + xrt_err(pdev, "invalid len %d", rw_arg->xgir_len); + return -EINVAL; + } + + if (rw_arg->xgir_id >= XRT_DEVCTL_MAX) { + xrt_err(pdev, "invalid id %d", rw_arg->xgir_id); + return -EINVAL; + } + + p_src = devctl->base_addrs[rw_arg->xgir_id]; + if (!p_src) { + xrt_err(pdev, "io not found, id %d", + rw_arg->xgir_id); + return -EINVAL; + } + if (rw_arg->xgir_offset + rw_arg->xgir_len > + devctl->sizes[rw_arg->xgir_id]) { + xrt_err(pdev, "invalid argument, off %d, len %d", + rw_arg->xgir_offset, rw_arg->xgir_len); + return -EINVAL; + } + p_dst = rw_arg->xgir_buf; + for (i = 0; i < rw_arg->xgir_len / sizeof(u32); i++) { + u32 val = ioread32(p_src + rw_arg->xgir_offset + i); + + memcpy(p_dst + i, &val, sizeof(u32)); + } + break; + } + default: + xrt_err(pdev, "unsupported cmd %d", cmd); + return -EINVAL; + } + + return ret; +} + +static int xrt_devctl_remove(struct platform_device *pdev) +{ + struct xrt_devctl *devctl; + int i; + +