Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752521AbdGRCaF (ORCPT ); Mon, 17 Jul 2017 22:30:05 -0400 Received: from mga07.intel.com ([134.134.136.100]:2685 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751842AbdGRCaC (ORCPT ); Mon, 17 Jul 2017 22:30:02 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,376,1496127600"; d="scan'208";a="127876938" From: "Wu, Hao" To: Alan Tull CC: Moritz Fischer , "linux-fpga@vger.kernel.org" , linux-kernel , "linux-api@vger.kernel.org" , "Kang, Luwei" , "Zhang, Yi Z" , Xiao Guangrong , "Whisonant, Tim" , "Luebbers, Enno" , "Rao, Shiva" , "Rauer, Christopher" Subject: RE: [PATCH v2 07/22] fpga: intel: pcie: parse feature list and create platform device for features. Thread-Topic: [PATCH v2 07/22] fpga: intel: pcie: parse feature list and create platform device for features. Thread-Index: AQHS/zEpb3xruJOwsU6kIZ/4YQ7swKJYyQ7A Date: Tue, 18 Jul 2017 02:29:57 +0000 Message-ID: References: <1498441938-14046-1-git-send-email-hao.wu@intel.com> <1498441938-14046-8-git-send-email-hao.wu@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id v6I2VUcS025851 Content-Length: 8959 Lines: 254 > On Sun, Jun 25, 2017 at 8:52 PM, Wu Hao wrote: > > From: Xiao Guangrong > > > > Device Feature List structure creates a link list of feature headers > > within the MMIO space to provide an extensible way of adding features. > > > > The Intel FPGA PCIe driver walks through the feature headers to enumerate > > feature devices, FPGA Management Engine (FME) and FPGA Port for > Accelerated > > Function Unit (AFU), and their private sub features. For feature devices, > > it creates the platform devices and linked the private sub features into > > their platform data. > > > > Signed-off-by: Tim Whisonant > > Signed-off-by: Enno Luebbers > > Signed-off-by: Shiva Rao > > Signed-off-by: Christopher Rauer > > Signed-off-by: Kang Luwei > > Signed-off-by: Zhang Yi > > Signed-off-by: Xiao Guangrong > > Signed-off-by: Wu Hao > > --- > > v2: moved the code to drivers/fpga folder as suggested by Alan Tull. > > switched to GPLv2 license. > > fixed comments from Moritz Fischer. > > fixed kbuild warning, typos and clean up the code. > > --- > > drivers/fpga/Makefile | 2 +- > > drivers/fpga/intel-feature-dev.c | 130 ++++++ > > drivers/fpga/intel-feature-dev.h | 341 ++++++++++++++++ > > drivers/fpga/intel-pcie.c | 841 > ++++++++++++++++++++++++++++++++++++++- > > 4 files changed, 1311 insertions(+), 3 deletions(-) > > create mode 100644 drivers/fpga/intel-feature-dev.c > > create mode 100644 drivers/fpga/intel-feature-dev.h > > > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > > index 5613133..ad24b3d 100644 > > --- a/drivers/fpga/Makefile > > +++ b/drivers/fpga/Makefile > > @@ -31,4 +31,4 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga- > region.o > > # Intel FPGA Support > > obj-$(CONFIG_INTEL_FPGA_PCI) += intel-fpga-pci.o > > > > -intel-fpga-pci-objs := intel-pcie.o > > +intel-fpga-pci-objs := intel-pcie.o intel-feature-dev.o > > diff --git a/drivers/fpga/intel-feature-dev.c b/drivers/fpga/intel-feature-dev.c > > new file mode 100644 > > index 0000000..68f9cba > > --- /dev/null > > +++ b/drivers/fpga/intel-feature-dev.c > > @@ -0,0 +1,130 @@ > > +/* > > + * Intel FPGA Feature Device Driver > > + * > > + * Copyright (C) 2017 Intel Corporation, Inc. > > + * > > + * Authors: > > + * Kang Luwei > > + * Zhang Yi > > + * Wu Hao > > + * Xiao Guangrong > > + * > > + * This work is licensed under the terms of the GNU GPL version 2. See > > + * the COPYING file in the top-level directory. > > + */ > > + > > +#include "intel-feature-dev.h" > > + > > +void feature_platform_data_add(struct feature_platform_data *pdata, > > + int index, const char *name, > > + int resource_index, void __iomem *ioaddr) > > +{ > > + WARN_ON(index >= pdata->num); > > + > > + pdata->features[index].name = name; > > + pdata->features[index].resource_index = resource_index; > > + pdata->features[index].ioaddr = ioaddr; > > +} > > + > > +struct feature_platform_data * > > +feature_platform_data_alloc_and_init(struct platform_device *dev, int num) > > +{ > > + struct feature_platform_data *pdata; > > + > > + pdata = kzalloc(feature_platform_data_size(num), GFP_KERNEL); > > + if (pdata) { > > + pdata->dev = dev; > > + pdata->num = num; > > + mutex_init(&pdata->lock); > > + } > > + > > + return pdata; > > +} > > + > > +int fme_feature_num(void) > > +{ > > + return FME_FEATURE_ID_MAX; > > +} > > + > > +int port_feature_num(void) > > +{ > > + return PORT_FEATURE_ID_MAX; > > +} > > + > > +int fpga_port_id(struct platform_device *pdev) > > +{ > > + struct feature_port_header *port_hdr; > > + struct feature_port_capability capability; > > + > > + port_hdr = get_feature_ioaddr_by_index(&pdev->dev, > > + PORT_FEATURE_ID_HEADER); > > + WARN_ON(!port_hdr); > > + > > + capability.csr = readq(&port_hdr->capability); > > + return capability.port_number; > > +} > > +EXPORT_SYMBOL_GPL(fpga_port_id); > > + > > +/* > > + * Enable Port by clear the port soft reset bit, which is set by default. > > + * The User AFU is unable to respond to any MMIO access while in reset. > > + * __fpga_port_enable function should only be used after > __fpga_port_disable > > + * function. > > + */ > > +void __fpga_port_enable(struct platform_device *pdev) > > +{ > > + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev); > > + struct feature_port_header *port_hdr; > > + struct feature_port_control control; > > + > > + WARN_ON(!pdata->disable_count); > > + > > + if (--pdata->disable_count != 0) > > + return; > > + > > + port_hdr = get_feature_ioaddr_by_index(&pdev->dev, > > + PORT_FEATURE_ID_HEADER); > > + WARN_ON(!port_hdr); > > + > > + control.csr = readq(&port_hdr->control); > > + control.port_sftrst = 0x0; > > + writeq(control.csr, &port_hdr->control); > > +} > > +EXPORT_SYMBOL_GPL(__fpga_port_enable); > > + > > +#define RST_POLL_INVL 10 /* us */ > > +#define RST_POLL_TIMEOUT 1000 /* us */ > > + > > +int __fpga_port_disable(struct platform_device *pdev) > > +{ > > + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev); > > + struct feature_port_header *port_hdr; > > + struct feature_port_control control; > > + > > + if (pdata->disable_count++ != 0) > > + return 0; > > + > > + port_hdr = get_feature_ioaddr_by_index(&pdev->dev, > > + PORT_FEATURE_ID_HEADER); > > + WARN_ON(!port_hdr); > > + > > + /* Set port soft reset */ > > + control.csr = readq(&port_hdr->control); > > + control.port_sftrst = 0x1; > > + writeq(control.csr, &port_hdr->control); > > + > > + /* > > + * HW sets ack bit to 1 when all outstanding requests have been drained > > + * on this port and minimum soft reset pulse width has elapsed. > > + * Driver polls port_soft_reset_ack to determine if reset done by HW. > > + */ > > + if (readq_poll_timeout(&port_hdr->control, control.csr, > > + (control.port_sftrst_ack == 1), > > + RST_POLL_INVL, RST_POLL_TIMEOUT)) { > > + dev_err(&pdev->dev, "timeout, fail to reset device\n"); > > + return -ETIMEDOUT; > > + } > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(__fpga_port_disable); > > diff --git a/drivers/fpga/intel-feature-dev.h b/drivers/fpga/intel-feature-dev.h > > new file mode 100644 > > index 0000000..f67784a > > --- /dev/null > > +++ b/drivers/fpga/intel-feature-dev.h > > @@ -0,0 +1,341 @@ > > +/* > > + * Intel FPGA Feature Device Driver Header File > > + * > > + * Copyright (C) 2017 Intel Corporation, Inc. > > + * > > + * Authors: > > + * Kang Luwei > > + * Zhang Yi > > + * Wu Hao > > + * Xiao Guangrong > > + * > > + * This work is licensed under the terms of the GNU GPL version 2. See > > + * the COPYING file in the top-level directory. > > + */ > > + > > +#ifndef __INTEL_FPGA_FEATURE_H > > +#define __INTEL_FPGA_FEATURE_H > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#ifndef readq > > +static inline u64 readq(void __iomem *addr) > > +{ > > + return readl(addr) + ((u64)readl(addr + 4) << 32); > > +} > > +#endif > > + > > +#ifndef writeq > > +static inline void writeq(u64 val, void __iomem *addr) > > +{ > > + writel((u32) (val), addr); > > + writel((u32) (val >> 32), (addr + 4)); > > +} > > +#endif > > Hi Hao, > > Was there a reason readq and writeq had to be created for this? Do > these get used? > Hi Alan Thanks for your review. Driver uses writeq and readq to access HW registers. Actually this is a problem reported by kbuild against the patch set v1[1]. Writeq/readq are missed in some arch, so I added writeq/readq definitions to resolve this problem following the same way done by some existing drivers. After recheck this today, I found I should use linux/io-64-nonatomic-lo-hi.h instead of rewriting them. I will fix it in the next version. [1] http://marc.info/?l=linux-kernel&m=149100396816882&w=2 Thanks Hao > Alan