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/<fpga.x>/name
> Name of the fpga device.
>
> 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: Xiao Guangrong <guangrong.x...@linux.intel.com>
> Signed-off-by: Wu Hao <hao...@intel.com>
> ---
> 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 <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/fpga/fpga-dev.h>
> +
> +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
