On Thu, Mar 30, 2017 at 7:08 AM, Wu Hao <hao...@intel.com> wrote: > From: Xiao Guangrong <guangrong.x...@linux.intel.com> > > Device Featuer List structure creates a link list of feature headers > within the MMIO space to provide an extensiable 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 <tim.whison...@intel.com> > Signed-off-by: Enno Luebbers <enno.luebb...@intel.com> > Signed-off-by: Shiva Rao <shiva....@intel.com> > Signed-off-by: Christopher Rauer <christopher.ra...@intel.com> > Signed-off-by: Kang Luwei <luwei.k...@intel.com> > Signed-off-by: Zhang Yi <yi.z.zh...@intel.com> > Signed-off-by: Xiao Guangrong <guangrong.x...@linux.intel.com> > Signed-off-by: Wu Hao <hao...@intel.com> > --- > drivers/fpga/intel/Makefile | 2 +- > drivers/fpga/intel/feature-dev.c | 139 +++++++ > drivers/fpga/intel/feature-dev.h | 342 ++++++++++++++++ > drivers/fpga/intel/pcie.c | 841 > ++++++++++++++++++++++++++++++++++++++- > 4 files changed, 1321 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/intel/Makefile b/drivers/fpga/intel/Makefile > index 61fd8ea..c029940 100644 > --- a/drivers/fpga/intel/Makefile > +++ b/drivers/fpga/intel/Makefile > @@ -1,3 +1,3 @@ > obj-$(CONFIG_INTEL_FPGA_PCI) += intel-fpga-pci.o > > -intel-fpga-pci-objs := pcie.o > +intel-fpga-pci-objs := pcie.o 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..6952566 > --- /dev/null > +++ b/drivers/fpga/intel/feature-dev.c > @@ -0,0 +1,139 @@ > +/* > + * Intel FPGA Feature Device Driver > + * > + * Copyright (C) 2017 Intel Corporation, Inc. > + * > + * Authors: > + * Kang Luwei <luwei.k...@intel.com> > + * Zhang Yi <yi.z.zh...@intel.com> > + * Wu Hao <hao...@intel.com> > + * Xiao Guangrong <guangrong.x...@linux.intel.com> > + * > + * 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 "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; > +} > + > +int feature_platform_data_size(int num) > +{ > + return sizeof(struct feature_platform_data) + > + num * sizeof(struct feature); > +} > + > +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) > +{
feature-dev.c is handling enumeration and adding port enable/disable/etc functions for a specific port device. I see the port as a fpga-bridge. The enumeration code should be separate from the bridge code. Especially separate from a very specific bridge low level device driver implementation, otherwise this becomes obsolete as soon as you have another port device with a different register implementation. Even if you handle that, then this enumeration code isn't useable by other people who are using fpga-bridge. The fpga-bridge framework exists to separate low level things like how to enable/disable a specific bridge device from upper level code that knows when to enable/disable it (fpga-region). Alan