From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752130AbdCaGNi (ORCPT ); Fri, 31 Mar 2017 02:13:38 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:49148 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750859AbdCaGNh (ORCPT ); Fri, 31 Mar 2017 02:13:37 -0400 X-ME-Sender: X-Sasl-enc: 7VtIm2Ybmxa6/6YBim+D9LLjDgQrcLq31J79u9W+Mewd 1490940815 Date: Fri, 31 Mar 2017 08:13:22 +0200 From: Greg KH To: Wu Hao Cc: atull@kernel.org, moritz.fischer@ettus.com, linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org, luwei.kang@intel.com, yi.z.zhang@intel.com, Tim Whisonant , Enno Luebbers , Shiva Rao , Christopher Rauer , Xiao Guangrong Subject: Re: [PATCH 02/16] fpga: add FPGA device framework Message-ID: <20170331061322.GB7621@kroah.com> References: <1490875696-15145-1-git-send-email-hao.wu@intel.com> <1490875696-15145-3-git-send-email-hao.wu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1490875696-15145-3-git-send-email-hao.wu@intel.com> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 30, 2017 at 08:08:02PM +0800, Wu Hao wrote: > During FPGA device (e.g PCI-based) discovery, platform devices are > registered for different FPGA function units. But the device node path > isn't quite friendly to applications. > > Consider this case, applications want to access child device's sysfs file > for some information. > > 1) Access using bus-based path (e.g PCI) > > /sys/bus/pci/devices/xxxxx/fpga_func_a.0/sysfs_file > > From the path, it's clear which PCI device is the parent, but not perfect > solution for applications. PCI device BDF is not fixed, application may > need to search all PCI device to find the actual FPGA Device. > > 2) Or access using platform device path > > /sys/bus/platform/devices/fpga_func_a.0/sysfs_file > > Applications find the actual function by name easily, but no information > about which fpga device it belongs to. It's quite confusing if multiple > FPGA devices are in one system. > > 'FPGA Device' class is introduced to resolve this problem. Each node under > this class represents a fpga device, which may have one or more child > devices. Applications only need to search under this FPGA Device class > folder to find the child device node it needs. > > For example, for the platform has 2 fpga devices, each fpga device has > 3 child devices, the hierarchy looks like this. > > Two nodes are under /sys/class/fpga/: > /sys/class/fpga/fpga.0 > /sys/class/fpga/fpga.1 > > Each node has 1 function A device and 2 function B devices: > /sys/class/fpga/fpga.0/func_a.0 > /sys/class/fpga/fpga.0/func_b.0 > /sys/class/fpga/fpga.0/func_b.1 > > /sys/class/fpga/fpga.1/func_a.1 > /sys/class/fpga/fpga.1/func_b.2 > /sys/class/fpga/fpga.1/func_b.3 > > This following APIs are provided by FPGA device framework: > * fpga_dev_create > Create fpga device under the given parent device. > * fpga_dev_destroy > Destroy fpga device > > The following sysfs files are created: > * /sys/class/fpga//name > Name of the fpga device. > > Signed-off-by: Tim Whisonant > Signed-off-by: Enno Luebbers > Signed-off-by: Shiva Rao > Signed-off-by: Christopher Rauer > Signed-off-by: Xiao Guangrong > Signed-off-by: Wu Hao > --- > drivers/fpga/Kconfig | 6 +++ > drivers/fpga/Makefile | 3 ++ > drivers/fpga/fpga-dev.c | 120 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/fpga/fpga-dev.h | 34 ++++++++++++ > 4 files changed, 163 insertions(+) > create mode 100644 drivers/fpga/fpga-dev.c > create mode 100644 include/linux/fpga/fpga-dev.h > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index ce861a2..d99b640 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -12,6 +12,12 @@ config FPGA > manager drivers. > > if FPGA > +config FPGA_DEVICE > + tristate "FPGA Device Framework" > + help > + Say Y here if you want support for FPGA Devices from the kernel. > + The FPGA Device Framework adds a FPGA device class and provide > + interfaces to create FPGA devices. > > config FPGA_REGION > tristate "FPGA Region" > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > index 8df07bc..53a41d2 100644 > --- a/drivers/fpga/Makefile > +++ b/drivers/fpga/Makefile > @@ -5,6 +5,9 @@ > # Core FPGA Manager Framework > obj-$(CONFIG_FPGA) += fpga-mgr.o > > +# FPGA Device Framework > +obj-$(CONFIG_FPGA_DEVICE) += fpga-dev.o > + > # FPGA Manager Drivers > obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o > obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10) += socfpga-a10.o > diff --git a/drivers/fpga/fpga-dev.c b/drivers/fpga/fpga-dev.c > new file mode 100644 > index 0000000..0f4c0ed > --- /dev/null > +++ b/drivers/fpga/fpga-dev.c > @@ -0,0 +1,120 @@ > +/* > + * FPGA Device Framework Driver > + * > + * Copyright (C) 2017 Intel Corporation, Inc. > + * > + * 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 drivers/fpga/intel for the BSD license and see > + * the COPYING file in the top-level directory for the GPLv2 license. Really? A BSD licened file that does EXPORT_SYMBOL_GPL and interacts directly with the driver core? Please go talk to some of your lawyers about this before you resubmit this correctly... > + */ > +#include > +#include > +#include > +#include > + > +static DEFINE_IDA(fpga_dev_ida); > +static struct class *fpga_dev_class; > + > +static ssize_t name_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct fpga_dev *fdev = to_fpga_dev(dev); > + > + return sprintf(buf, "%s\n", fdev->name); > +} > +static DEVICE_ATTR_RO(name); There already is a name for the device, it's the directory name. > + > +static struct attribute *fpga_dev_attrs[] = { > + &dev_attr_name.attr, > + NULL, > +}; > +ATTRIBUTE_GROUPS(fpga_dev); > + > +/** > + * fpga_dev_create - create a fpga device > + * @parent: parent device > + * @name: fpga device name > + * > + * Return fpga_dev struct for success, error code otherwise. > + */ > +struct fpga_dev *fpga_dev_create(struct device *parent, const char *name) > +{ > + struct fpga_dev *fdev; > + int id, ret = 0; > + > + if (!name || !strlen(name)) { > + dev_err(parent, "Attempt to register with no name!\n"); > + return ERR_PTR(-EINVAL); > + } > + > + fdev = kzalloc(sizeof(*fdev), GFP_KERNEL); > + if (!fdev) > + return ERR_PTR(-ENOMEM); > + > + id = ida_simple_get(&fpga_dev_ida, 0, 0, GFP_KERNEL); > + if (id < 0) { > + ret = id; > + goto error_kfree; > + } > + > + fdev->name = name; > + > + device_initialize(&fdev->dev); > + fdev->dev.class = fpga_dev_class; > + fdev->dev.parent = parent; > + fdev->dev.id = id; > + > + ret = dev_set_name(&fdev->dev, "fpga.%d", id); > + if (ret) > + goto error_device; > + > + ret = device_add(&fdev->dev); > + if (ret) > + goto error_device; > + > + dev_dbg(fdev->dev.parent, "fpga device [%s] created\n", fdev->name); > + > + return fdev; > + > +error_device: > + ida_simple_remove(&fpga_dev_ida, id); > +error_kfree: > + kfree(fdev); > + > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL_GPL(fpga_dev_create); > + > +static void fpga_dev_release(struct device *dev) > +{ > + struct fpga_dev *fdev = to_fpga_dev(dev); > + > + ida_simple_remove(&fpga_dev_ida, fdev->dev.id); > + kfree(fdev); > +} > + > +static int __init fpga_dev_class_init(void) > +{ > + pr_info("FPGA Device framework\n"); > + > + fpga_dev_class = class_create(THIS_MODULE, "fpga"); > + if (IS_ERR(fpga_dev_class)) > + return PTR_ERR(fpga_dev_class); > + > + fpga_dev_class->dev_groups = fpga_dev_groups; > + fpga_dev_class->dev_release = fpga_dev_release; > + > + return 0; > +} > + > +static void __exit fpga_dev_class_exit(void) > +{ > + class_destroy(fpga_dev_class); > +} > + > +MODULE_DESCRIPTION("FPGA Device framework"); > +MODULE_LICENSE("Dual BSD/GPL"); > + > +subsys_initcall(fpga_dev_class_init); > +module_exit(fpga_dev_class_exit); > diff --git a/include/linux/fpga/fpga-dev.h b/include/linux/fpga/fpga-dev.h > new file mode 100644 > index 0000000..7b58356 > --- /dev/null > +++ b/include/linux/fpga/fpga-dev.h > @@ -0,0 +1,34 @@ > +/* > + * FPGA Device Driver Header > + * > + * Copyright (C) 2017 Intel Corporation, Inc. > + * > + * 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 drivers/fpga/intel for the BSD license and see > + * the COPYING file in the top-level directory for the GPLv2 license. Again with the dual license, please fix up. > + * > + */ > +#ifndef _LINUX_FPGA_DEV_H > +#define _LINUX_FPGA_DEV_H > + > +/** > + * struct fpga_dev - fpga device structure > + * @name: name of fpga device > + * @dev: fpga device > + */ > +struct fpga_dev { > + const char *name; > + struct device dev; struct device already has a name, why duplicate it here? thanks, greg k-h