Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752693AbdDDGxG (ORCPT ); Tue, 4 Apr 2017 02:53:06 -0400 Received: from mga03.intel.com ([134.134.136.65]:37723 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751130AbdDDGxE (ORCPT ); Tue, 4 Apr 2017 02:53:04 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,274,1486454400"; d="scan'208";a="68859253" Date: Tue, 4 Apr 2017 14:48:25 +0800 From: Wu Hao To: matthew.gerlach@linux.intel.com Cc: Alan Tull , Moritz Fischer , linux-fpga@vger.kernel.org, linux-kernel , luwei.kang@intel.com, yi.z.zhang@intel.com, Tim Whisonant , Enno Luebbers , Shiva Rao , Christopher Rauer , Alan Tull , Xiao Guangrong Subject: Re: [PATCH 11/16] fpga: intel: fme: add partial reconfiguration sub feature support Message-ID: <20170404064825.GF13968@hao-dev> References: <1490875696-15145-1-git-send-email-hao.wu@intel.com> <1490875696-15145-12-git-send-email-hao.wu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 17816 Lines: 481 On Mon, Apr 03, 2017 at 03:49:41PM -0700, matthew.gerlach@linux.intel.com wrote: > > > On Mon, 3 Apr 2017, Alan Tull wrote: > > >On Thu, Mar 30, 2017 at 7:08 AM, Wu Hao wrote: > >>From: Kang Luwei > >> > >>Partial Reconfiguration (PR) is the most important function for FME. It > >>allows reconfiguration for given Port/Accelerated Function Unit (AFU). > >> > >>This patch adds support for PR sub feature. In this patch, it registers > >>a fpga_mgr and implements fpga_manager_ops, and invoke fpga_mgr_buf_load > >>for PR operation once PR request received via ioctl. > > > >The code that invokes fpga_mgr_buf_load should be a different layer. > > > >>Below user space > >>interfaces are exposed by this sub feature. > >> > >>Sysfs interface: > >>* /sys/class/fpga///interface_id > >> Read-only. Indicate the hardware interface information. Userspace > >> applications need to check this interface to select correct green > >> bitstream format before PR. > >> > >>Ioctl interface: > >>* FPGA_FME_PORT_PR > >> Do partial reconfiguration per information from userspace, including > >> target port(AFU), buffer size and address info. It returns the PR status > >> (PR error code if failed) to userspace. > >> > >>Signed-off-by: Tim Whisonant > >>Signed-off-by: Enno Luebbers > >>Signed-off-by: Shiva Rao > >>Signed-off-by: Christopher Rauer > >>Signed-off-by: Alan Tull > >>Signed-off-by: Kang Luwei > >>Signed-off-by: Xiao Guangrong > >>Signed-off-by: Wu Hao > >>--- > >> drivers/fpga/intel/Makefile | 2 +- > >> drivers/fpga/intel/feature-dev.h | 58 ++++++ > >> drivers/fpga/intel/fme-main.c | 44 ++++- > >> drivers/fpga/intel/fme-pr.c | 400 +++++++++++++++++++++++++++++++++++++++ > >> drivers/fpga/intel/fme.h | 32 ++++ > >> include/uapi/linux/intel-fpga.h | 44 +++++ > >> 6 files changed, 578 insertions(+), 2 deletions(-) > >> create mode 100644 drivers/fpga/intel/fme-pr.c > >> create mode 100644 drivers/fpga/intel/fme.h > >> > >>diff --git a/drivers/fpga/intel/Makefile b/drivers/fpga/intel/Makefile > >>index 546861d..0452cb6 100644 > >>--- a/drivers/fpga/intel/Makefile > >>+++ b/drivers/fpga/intel/Makefile > >>@@ -2,4 +2,4 @@ obj-$(CONFIG_INTEL_FPGA_PCI) += intel-fpga-pci.o > >> obj-$(CONFIG_INTEL_FPGA_FME) += intel-fpga-fme.o > >> > >> intel-fpga-pci-objs := pcie.o feature-dev.o > >>-intel-fpga-fme-objs := fme-main.o > >>+intel-fpga-fme-objs := fme-main.o fme-pr.o > >>diff --git a/drivers/fpga/intel/feature-dev.h b/drivers/fpga/intel/feature-dev.h > >>index dccc283..5a25c915 100644 > >>--- a/drivers/fpga/intel/feature-dev.h > >>+++ b/drivers/fpga/intel/feature-dev.h > >>@@ -150,8 +150,66 @@ struct feature_fme_err { > >> }; > >> > >> /* FME Partial Reconfiguration Sub Feature Register Set */ > >>+/* FME PR Control Register */ > >>+struct feature_fme_pr_ctl { > >>+ union { > >>+ u64 csr; > >>+ struct { > >>+ u8 pr_reset:1; /* Reset PR Engine */ > >>+ u8 rsvdz1:3; > >>+ u8 pr_reset_ack:1; /* Reset PR Engine Ack */ > >>+ u8 rsvdz2:3; > >>+ u8 pr_regionid:2; /* PR Region ID */ > >>+ u8 rsvdz3:2; > >>+ u8 pr_start_req:1; /* PR Start Request */ > >>+ u8 pr_push_complete:1; /* PR Data push complete */ > >>+ u8 pr_kind:1; /* Load Customer or Intel GBS */ > >>+ u32 rsvdz4:17; > >>+ u32 config_data; > >>+ }; > >>+ }; > >>+}; > >>+ > >>+/* FME PR Status Register */ > >>+struct feature_fme_pr_status { > >>+ union { > >>+ u64 csr; > >>+ struct { > >>+ u16 pr_credit:9; /* Number of PR Credits */ > >>+ u8 rsvdz1:7; > >>+ u8 pr_status:1; /* PR Operation status */ > >>+ u8 rsvdz2:3; > >>+ u8 pr_ctrlr_status:3; /* Controller status */ > >>+ u8 rsvdz3:1; > >>+ u8 pr_host_status:4; /* PR Host status */ > >>+ u64 rsvdz4:36; > >>+ }; > >>+ }; > >>+}; > >>+ > >>+/* FME PR Data Register */ > >>+struct feature_fme_pr_data { > >>+ union { > >>+ u64 csr; > >>+ struct { > >>+ /* PR data from the raw-binary file */ > >>+ u32 pr_data_raw; > >>+ u32 rsvd; > >>+ }; > >>+ }; > >>+}; > >>+ > >> struct feature_fme_pr { > >> struct feature_header header; > >>+ struct feature_fme_pr_ctl control; > >>+ struct feature_fme_pr_status status; > >>+ struct feature_fme_pr_data data; > >>+ u64 error; > >>+ > >>+ u64 rsvd[16]; > >>+ > >>+ u64 intfc_id_l; /* PR interface Id Low */ > >>+ u64 intfc_id_h; /* PR interface Id High */ > >> }; > >> > >> /* PORT Header Register Set */ > >>diff --git a/drivers/fpga/intel/fme-main.c b/drivers/fpga/intel/fme-main.c > >>index 36d0c4c..0d9a7a6 100644 > >>--- a/drivers/fpga/intel/fme-main.c > >>+++ b/drivers/fpga/intel/fme-main.c > >>@@ -23,6 +23,7 @@ > >> #include > >> > >> #include "feature-dev.h" > >>+#include "fme.h" > >> > >> static ssize_t ports_num_show(struct device *dev, > >> struct device_attribute *attr, char *buf) > >>@@ -101,6 +102,10 @@ static struct feature_driver fme_feature_drvs[] = { > >> .ops = &fme_hdr_ops, > >> }, > >> { > >>+ .name = FME_FEATURE_PR_MGMT, > >>+ .ops = &pr_mgmt_ops, > >>+ }, > >>+ { > >> .ops = NULL, > >> }, > >> }; > >>@@ -182,14 +187,48 @@ static const struct file_operations fme_fops = { > >> .unlocked_ioctl = fme_ioctl, > >> }; > >> > >>+static int fme_dev_init(struct platform_device *pdev) > >>+{ > >>+ struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev); > >>+ struct fpga_fme *fme; > >>+ > >>+ fme = devm_kzalloc(&pdev->dev, sizeof(*fme), GFP_KERNEL); > >>+ if (!fme) > >>+ return -ENOMEM; > >>+ > >>+ fme->pdata = pdata; > >>+ > >>+ mutex_lock(&pdata->lock); > >>+ fpga_pdata_set_private(pdata, fme); > >>+ mutex_unlock(&pdata->lock); > >>+ return 0; > >>+} > >>+ > >>+static void fme_dev_destroy(struct platform_device *pdev) > >>+{ > >>+ struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev); > >>+ struct fpga_fme *fme; > >>+ > >>+ mutex_lock(&pdata->lock); > >>+ fme = fpga_pdata_get_private(pdata); > >>+ fpga_pdata_set_private(pdata, NULL); > >>+ mutex_unlock(&pdata->lock); > >>+ > >>+ devm_kfree(&pdev->dev, fme); > >>+} > >>+ > >> static int fme_probe(struct platform_device *pdev) > >> { > >> int ret; > >> > >>- ret = fpga_dev_feature_init(pdev, fme_feature_drvs); > >>+ ret = fme_dev_init(pdev); > >> if (ret) > >> goto exit; > >> > >>+ ret = fpga_dev_feature_init(pdev, fme_feature_drvs); > >>+ if (ret) > >>+ goto dev_destroy; > >>+ > >> ret = fpga_register_dev_ops(pdev, &fme_fops, THIS_MODULE); > >> if (ret) > >> goto feature_uinit; > >>@@ -198,6 +237,8 @@ static int fme_probe(struct platform_device *pdev) > >> > >> feature_uinit: > >> fpga_dev_feature_uinit(pdev); > >>+dev_destroy: > >>+ fme_dev_destroy(pdev); > >> exit: > >> return ret; > >> } > >>@@ -206,6 +247,7 @@ static int fme_remove(struct platform_device *pdev) > >> { > >> fpga_dev_feature_uinit(pdev); > >> fpga_unregister_dev_ops(pdev); > >>+ fme_dev_destroy(pdev); > >> return 0; > >> } > >> > >>diff --git a/drivers/fpga/intel/fme-pr.c b/drivers/fpga/intel/fme-pr.c > >>new file mode 100644 > >>index 0000000..3b44a3e > >>--- /dev/null > >>+++ b/drivers/fpga/intel/fme-pr.c > >>@@ -0,0 +1,400 @@ > >>+/* > >>+ * Driver for Intel FPGA Management Engine (FME) Partial Reconfiguration > >>+ * > >>+ * Copyright (C) 2017 Intel Corporation, Inc. > >>+ * > >>+ * Authors: > >>+ * Kang Luwei > >>+ * Xiao Guangrong > >>+ * Joseph Grecco > >>+ * Enno Luebbers > >>+ * Tim Whisonant > >>+ * Ananda Ravuri > >>+ * Christopher Rauer > >>+ * Henry Mitchel > >>+ * > >>+ * This work is licensed under a dual BSD/GPLv2 license. When using or > >>+ * redistributing this file, you may do so under either license. See the > >>+ * LICENSE.BSD file under this directory for the BSD license and see > >>+ * the COPYING file in the top-level directory for the GPLv2 license. > >>+ */ > >>+ > >>+#include > >>+#include > >>+#include > >>+#include > >>+#include > >>+#include > >>+ > >>+#include "feature-dev.h" > >>+#include "fme.h" > >>+ > >>+#define PR_WAIT_TIMEOUT 8000000 > >>+ > >>+#define PR_HOST_STATUS_IDLE 0 > >>+ > >>+DEFINE_FPGA_PR_ERR_MSG(pr_err_msg); > >>+ > >>+static ssize_t interface_id_show(struct device *dev, > >>+ struct device_attribute *attr, char *buf) > >>+{ > >>+ u64 intfc_id_l, intfc_id_h; > >>+ struct feature_fme_pr *fme_pr > >>+ = get_feature_ioaddr_by_index(dev, FME_FEATURE_ID_PR_MGMT); > >>+ > >>+ intfc_id_l = readq(&fme_pr->intfc_id_l); > >>+ intfc_id_h = readq(&fme_pr->intfc_id_h); > >>+ > >>+ return scnprintf(buf, PAGE_SIZE, "%016llx%016llx\n", > >>+ (unsigned long long)intfc_id_h, > >>+ (unsigned long long)intfc_id_l); > >>+} > >>+static DEVICE_ATTR_RO(interface_id); > >>+ > >>+static struct attribute *pr_mgmt_attrs[] = { > >>+ &dev_attr_interface_id.attr, > >>+ NULL, > >>+}; > >>+ > >>+struct attribute_group pr_mgmt_attr_group = { > >>+ .attrs = pr_mgmt_attrs, > >>+ .name = "pr", > >>+}; > >>+ > >>+static u64 > >>+pr_err_handle(struct platform_device *pdev, struct feature_fme_pr *fme_pr) > >>+{ > >>+ struct feature_fme_pr_status fme_pr_status; > >>+ unsigned long err_code; > >>+ u64 fme_pr_error; > >>+ int i = 0; > >>+ > >>+ fme_pr_status.csr = readq(&fme_pr->status); > >>+ if (!fme_pr_status.pr_status) > >>+ return 0; > >>+ > >>+ err_code = fme_pr_error = readq(&fme_pr->error); > >>+ for_each_set_bit(i, &err_code, PR_MAX_ERR_NUM) > >>+ dev_dbg(&pdev->dev, "%s\n", pr_err_msg[i]); > >>+ writeq(fme_pr_error, &fme_pr->error); > >>+ return fme_pr_error; > >>+} > >>+ > >>+static int fme_pr_write_init(struct fpga_manager *mgr, > >>+ struct fpga_image_info *info, const char *buf, size_t count) > >>+{ > >>+ struct fpga_fme *fme = mgr->priv; > >>+ struct platform_device *pdev; > >>+ struct feature_fme_pr *fme_pr; > >>+ struct feature_fme_pr_ctl fme_pr_ctl; > >>+ struct feature_fme_pr_status fme_pr_status; > >>+ > >>+ pdev = fme->pdata->dev; > >>+ fme_pr = get_feature_ioaddr_by_index(&pdev->dev, > >>+ FME_FEATURE_ID_PR_MGMT); > >>+ if (!fme_pr) > >>+ return -EINVAL; > >>+ > >>+ if (WARN_ON(info->flags != FPGA_MGR_PARTIAL_RECONFIG)) > >>+ return -EINVAL; > > > >flags is bitmapped so please do: > > > >if (WARN_ON(info->flags & FPGA_MGR_PARTIAL_RECONFIG)) > > > >>+ > >>+ dev_dbg(&pdev->dev, "resetting PR before initiated PR\n"); > >>+ > >>+ fme_pr_ctl.csr = readq(&fme_pr->control); > >>+ fme_pr_ctl.pr_reset = 1; > >>+ writeq(fme_pr_ctl.csr, &fme_pr->control); > >>+ > >>+ fme_pr_ctl.pr_reset_ack = 1; > >>+ > >>+ if (fpga_wait_register_field(pr_reset_ack, fme_pr_ctl, > >>+ &fme_pr->control, PR_WAIT_TIMEOUT, 1)) { > >>+ dev_err(&pdev->dev, "maximum PR timeout\n"); > >>+ return -ETIMEDOUT; > >>+ } > >>+ > >>+ fme_pr_ctl.csr = readq(&fme_pr->control); > >>+ fme_pr_ctl.pr_reset = 0; > >>+ writeq(fme_pr_ctl.csr, &fme_pr->control); > >>+ > >>+ dev_dbg(&pdev->dev, > >>+ "waiting for PR resource in HW to be initialized and ready\n"); > >>+ > >>+ fme_pr_status.pr_host_status = PR_HOST_STATUS_IDLE; > >>+ > >>+ if (fpga_wait_register_field(pr_host_status, fme_pr_status, > >>+ &fme_pr->status, PR_WAIT_TIMEOUT, 1)) { > >>+ dev_err(&pdev->dev, "maximum PR timeout\n"); > >>+ return -ETIMEDOUT; > >>+ } > >>+ > >>+ dev_dbg(&pdev->dev, "check if have any previous PR error\n"); > >>+ pr_err_handle(pdev, fme_pr); > >>+ return 0; > >>+} > >>+ > >>+static int fme_pr_write(struct fpga_manager *mgr, > >>+ const char *buf, size_t count) > >>+{ > >>+ struct fpga_fme *fme = mgr->priv; > >>+ struct platform_device *pdev; > >>+ struct feature_fme_pr *fme_pr; > >>+ struct feature_fme_pr_ctl fme_pr_ctl; > >>+ struct feature_fme_pr_status fme_pr_status; > >>+ struct feature_fme_pr_data fme_pr_data; > >>+ int delay, pr_credit, i = 0; > >>+ > >>+ pdev = fme->pdata->dev; > >>+ fme_pr = get_feature_ioaddr_by_index(&pdev->dev, > >>+ FME_FEATURE_ID_PR_MGMT); > >>+ > >>+ dev_dbg(&pdev->dev, "set PR port ID and start request\n"); > >>+ > >>+ fme_pr_ctl.csr = readq(&fme_pr->control); > >>+ fme_pr_ctl.pr_regionid = fme->port_id; > >>+ fme_pr_ctl.pr_start_req = 1; > >>+ writeq(fme_pr_ctl.csr, &fme_pr->control); > >>+ > >>+ dev_dbg(&pdev->dev, "pushing data from bitstream to HW\n"); > >>+ > >>+ fme_pr_status.csr = readq(&fme_pr->status); > >>+ pr_credit = fme_pr_status.pr_credit; > >>+ > >>+ while (count > 0) { > >>+ delay = 0; > >>+ while (pr_credit <= 1) { > >>+ if (delay++ > PR_WAIT_TIMEOUT) { > >>+ dev_err(&pdev->dev, "maximum try\n"); > >>+ return -ETIMEDOUT; > >>+ } > >>+ udelay(1); > >>+ > >>+ fme_pr_status.csr = readq(&fme_pr->status); > >>+ pr_credit = fme_pr_status.pr_credit; > >>+ }; > >>+ > >>+ if (count >= 4) { > >>+ fme_pr_data.rsvd = 0; > >>+ fme_pr_data.pr_data_raw = *(((u32 *)buf) + i); > >>+ writeq(fme_pr_data.csr, &fme_pr->data); > >>+ count -= 4; > >>+ pr_credit--; > >>+ i++; > >>+ } else { > >>+ WARN_ON(1); > >>+ return -EINVAL; > >>+ } > >>+ } > >>+ > >>+ return 0; > >>+} > >>+ > >>+static int fme_pr_write_complete(struct fpga_manager *mgr, > >>+ struct fpga_image_info *info) > >>+{ > >>+ struct fpga_fme *fme = mgr->priv; > >>+ struct platform_device *pdev; > >>+ struct feature_fme_pr *fme_pr; > >>+ struct feature_fme_pr_ctl fme_pr_ctl; > >>+ > >>+ pdev = fme->pdata->dev; > >>+ fme_pr = get_feature_ioaddr_by_index(&pdev->dev, > >>+ FME_FEATURE_ID_PR_MGMT); > >>+ > >>+ fme_pr_ctl.csr = readq(&fme_pr->control); > >>+ fme_pr_ctl.pr_push_complete = 1; > >>+ writeq(fme_pr_ctl.csr, &fme_pr->control); > >>+ > >>+ dev_dbg(&pdev->dev, "green bitstream push complete\n"); > >>+ dev_dbg(&pdev->dev, "waiting for HW to release PR resource\n"); > >>+ > >>+ fme_pr_ctl.pr_start_req = 0; > >>+ > >>+ if (fpga_wait_register_field(pr_start_req, fme_pr_ctl, > >>+ &fme_pr->control, PR_WAIT_TIMEOUT, 1)) { > >>+ dev_err(&pdev->dev, "maximum try.\n"); > >>+ return -ETIMEDOUT; > >>+ } > >>+ > >>+ dev_dbg(&pdev->dev, "PR operation complete, checking status\n"); > >>+ fme->pr_err = pr_err_handle(pdev, fme_pr); > >>+ if (fme->pr_err) > >>+ return -EIO; > >>+ > >>+ dev_dbg(&pdev->dev, "PR done successfully\n"); > >>+ return 0; > >>+} > >>+ > >>+static enum fpga_mgr_states fme_pr_state(struct fpga_manager *mgr) > >>+{ > >>+ return FPGA_MGR_STATE_UNKNOWN; > >>+} > > The functions that implement the fme_pr_ops are really a low level fpga > manager driver for the Altera PR IP component. A standalone version of > such a driver has been reviewed and Acked. See the links below. > Could this file use those functions and remove this code? > > http://marc.info/?l=linux-kernel&m=149019678925564&w=2 > http://marc.info/?l=linux-kernel&m=149019654225457&w=2 > http://marc.info/?l=linux-kernel&m=149019598025274&w=2 > http://marc.info/?l=linux-kernel&m=149013051007149&w=2 > Thanks for the info, I just checked these code quickly, but it seems the register definitions are totally different, so I feel that we may not be able to reuse this driver. Thanks Hao