diff mbox

[02/16] fpga: add FPGA device framework

Message ID 1490875696-15145-3-git-send-email-hao.wu@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Wu, Hao March 30, 2017, 12:08 p.m. UTC
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.whisonant@intel.com>
Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
Signed-off-by: Shiva Rao <shiva.rao@intel.com>
Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@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

Comments

Greg KH March 31, 2017, 6:09 a.m. UTC | #1
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.

How does this interact with the existing "fpga class" that is in the
kernel already?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH March 31, 2017, 6:13 a.m. UTC | #2
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.whisonant@intel.com>
> Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
> Signed-off-by: Shiva Rao <shiva.rao@intel.com>
> Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> Signed-off-by: Wu Hao <hao.wu@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
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao March 31, 2017, 7:48 a.m. UTC | #3
On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
> 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.
> 
> How does this interact with the existing "fpga class" that is in the
> kernel already?

The fpga-dev introduced by this patch, is only a container device, and
drivers could register different functions under it. Per my understanding,
the existing "fpga class", including fpga-region, fpga-bridge and 
fpga-manager, is used to provide reconfiguration function for FPGA. So
driver can create child node using this existing "fpga class" to provide
FPGA reconfiguration function, and more nodes under this container for
different functions for given FPGA device.

For Intel FPGA device, partial reconfiguration is only one function of 
Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
below path for partial reconfiguration, and other interfaces for more
functions, e.g power management, virtualization support and etc.

/sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager

Thanks
Hao

> 
> thanks,
> 
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH March 31, 2017, 9:03 a.m. UTC | #4
On Fri, Mar 31, 2017 at 03:48:42PM +0800, Wu Hao wrote:
> On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
> > 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.
> > 
> > How does this interact with the existing "fpga class" that is in the
> > kernel already?
> 
> The fpga-dev introduced by this patch, is only a container device, and
> drivers could register different functions under it. Per my understanding,
> the existing "fpga class", including fpga-region, fpga-bridge and 
> fpga-manager, is used to provide reconfiguration function for FPGA. So
> driver can create child node using this existing "fpga class" to provide
> FPGA reconfiguration function, and more nodes under this container for
> different functions for given FPGA device.
> 
> For Intel FPGA device, partial reconfiguration is only one function of 
> Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
> below path for partial reconfiguration, and other interfaces for more
> functions, e.g power management, virtualization support and etc.
> 
> /sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager

So there is now two different levels of fpga class interfaces?

I'm not disagreeing with this, just that it seems a bit confusing, don't
you think?

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao March 31, 2017, 12:19 p.m. UTC | #5
On Fri, Mar 31, 2017 at 11:03:28AM +0200, Greg KH wrote:
> On Fri, Mar 31, 2017 at 03:48:42PM +0800, Wu Hao wrote:
> > On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
> > > 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.
> > > 
> > > How does this interact with the existing "fpga class" that is in the
> > > kernel already?
> > 
> > The fpga-dev introduced by this patch, is only a container device, and
> > drivers could register different functions under it. Per my understanding,
> > the existing "fpga class", including fpga-region, fpga-bridge and 
> > fpga-manager, is used to provide reconfiguration function for FPGA. So
> > driver can create child node using this existing "fpga class" to provide
> > FPGA reconfiguration function, and more nodes under this container for
> > different functions for given FPGA device.
> > 
> > For Intel FPGA device, partial reconfiguration is only one function of 
> > Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
> > below path for partial reconfiguration, and other interfaces for more
> > functions, e.g power management, virtualization support and etc.
> > 
> > /sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager
> 
> So there is now two different levels of fpga class interfaces?
> 
> I'm not disagreeing with this, just that it seems a bit confusing, don't
> you think?

I am not so sure, but the main purpose of fpga-dev, is trying to provide
enduser a more clear sysfs hierarchy reflecting the real hardware. And
fpga-things can be registered to fpga-dev directly if the hardware arch
is simple.

From enduser point of view, he could find everything of this FPGA device
under /sys/class/fpga/<fpga.x>/, including all fpga-regions, fpga-bridges
and fpga-managers. I feel it is not a bad choice. :)

Thanks
Hao

> 
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao March 31, 2017, 1:31 p.m. UTC | #6
> 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.whisonant@intel.com>
> > Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
> > Signed-off-by: Shiva Rao <shiva.rao@intel.com>
> > Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
> > Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> > Signed-off-by: Wu Hao <hao.wu@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...

Sorry, will check and fix this in next version.

> 
> 
> > + */
> > +#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.

For current implementation, the directory will have a common name like

/sys/class/fpga/fpga.0
/sys/class/fpga/fpga.1
/sys/class/fpga/fpga.2
...

For the 'name' sysfs interface, driver can put more device specific info
into this 'name', e.g intel-fpga-dev. Userspace can use this information
to know which kind of FPGA device it is. e.g if applications read the
/sys/class/fpga/fpga.5/name as intel-fpga-dev, it means the 5th fpga
device on the system is a Intel FPGA device, and then application applies
related method to enumerate the accelerators for it.
And other existing fpga class has similar sysfs interface too, so I would
like to keep it aligned with others.

> 
> > +
> > +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.
>

Sorry, will fix this in the next version.
 
> > + *
> > + */
> > +#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?

As mentioned above, dev has common name as fpga.x, but 'name' could have
more meaningful information, e.g venodr and device information.

Thanks
Hao

> 
> thanks,
> 
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH March 31, 2017, 2:10 p.m. UTC | #7
On Fri, Mar 31, 2017 at 09:31:09PM +0800, Wu Hao wrote:
> > On Thu, Mar 30, 2017 at 08:08:02PM +0800, Wu Hao wrote:
> > > +#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.
> 
> For current implementation, the directory will have a common name like
> 
> /sys/class/fpga/fpga.0
> /sys/class/fpga/fpga.1
> /sys/class/fpga/fpga.2
> ...
> 
> For the 'name' sysfs interface, driver can put more device specific info
> into this 'name', e.g intel-fpga-dev. Userspace can use this information
> to know which kind of FPGA device it is. e.g if applications read the
> /sys/class/fpga/fpga.5/name as intel-fpga-dev, it means the 5th fpga
> device on the system is a Intel FPGA device, and then application applies
> related method to enumerate the accelerators for it.
> And other existing fpga class has similar sysfs interface too, so I would
> like to keep it aligned with others.

Ok, then document the heck out of this in Documentation/ABI/ which I
don't think you did for the sysfs files you are creating.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Gerlach March 31, 2017, 7:01 p.m. UTC | #8
On Fri, 31 Mar 2017, Wu Hao wrote:

> On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
>> 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.
>>
>> How does this interact with the existing "fpga class" that is in the
>> kernel already?
>
> The fpga-dev introduced by this patch, is only a container device, and

I completely understand the need for a container device.  The fpga-region 
is also primarily a container, and in some cases the fpga-region may 
represent the entire fpga.  Over time this code may become redundant.

> drivers could register different functions under it. Per my understanding,
> the existing "fpga class", including fpga-region, fpga-bridge and
> fpga-manager, is used to provide reconfiguration function for FPGA. So
> driver can create child node using this existing "fpga class" to provide
> FPGA reconfiguration function, and more nodes under this container for
> different functions for given FPGA device.
>
> For Intel FPGA device, partial reconfiguration is only one function of
> Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
> below path for partial reconfiguration, and other interfaces for more
> functions, e.g power management, virtualization support and etc.
>
> /sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager
>
> Thanks
> Hao
>
>>
>> thanks,
>>
>> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao April 1, 2017, 11:36 a.m. UTC | #9
On Fri, Mar 31, 2017 at 04:10:09PM +0200, Greg KH wrote:
> On Fri, Mar 31, 2017 at 09:31:09PM +0800, Wu Hao wrote:
> > > On Thu, Mar 30, 2017 at 08:08:02PM +0800, Wu Hao wrote:
> > > > +#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.
> > 
> > For current implementation, the directory will have a common name like
> > 
> > /sys/class/fpga/fpga.0
> > /sys/class/fpga/fpga.1
> > /sys/class/fpga/fpga.2
> > ...
> > 
> > For the 'name' sysfs interface, driver can put more device specific info
> > into this 'name', e.g intel-fpga-dev. Userspace can use this information
> > to know which kind of FPGA device it is. e.g if applications read the
> > /sys/class/fpga/fpga.5/name as intel-fpga-dev, it means the 5th fpga
> > device on the system is a Intel FPGA device, and then application applies
> > related method to enumerate the accelerators for it.
> > And other existing fpga class has similar sysfs interface too, so I would
> > like to keep it aligned with others.
> 
> Ok, then document the heck out of this in Documentation/ABI/ which I
> don't think you did for the sysfs files you are creating.

Sure, thanks a lot for the reminder.

I will prepare the sysfs docs in the next version.

Thanks
Hao

> 
> thanks,
> 
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao April 1, 2017, 12:18 p.m. UTC | #10
On Fri, Mar 31, 2017 at 12:01:13PM -0700, matthew.gerlach@linux.intel.com wrote:
> On Fri, 31 Mar 2017, Wu Hao wrote:
> >On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
> >>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.
> >>
> >>How does this interact with the existing "fpga class" that is in the
> >>kernel already?
> >
> >The fpga-dev introduced by this patch, is only a container device, and
> 
> I completely understand the need for a container device.  The fpga-region is
> also primarily a container, and in some cases the fpga-region may represent
> the entire fpga.  Over time this code may become redundant.

Thanks a lot for your review and comments.

I feel that the fpga-region implies that it supports reconfiguration, but
in our cases, the Intel FPGA device, doesn't have base fpga-region for
full reconfiguration, but many accelerators with partial reconfiguration
support. A fpga-region brings together everything needed for the
reconfiguration, and a fpga-dev is trying to brings everything on a FPGA
device together, including fpga-region/bridge/manager, access different
accelerators and other function units.

I think it's not mandatory to use fpga-dev, as fpga-dev is just trying to
provide one more option here for some complex hardware.

Thanks
Hao
 
> >drivers could register different functions under it. Per my understanding,
> >the existing "fpga class", including fpga-region, fpga-bridge and
> >fpga-manager, is used to provide reconfiguration function for FPGA. So
> >driver can create child node using this existing "fpga class" to provide
> >FPGA reconfiguration function, and more nodes under this container for
> >different functions for given FPGA device.
> >
> >For Intel FPGA device, partial reconfiguration is only one function of
> >Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
> >below path for partial reconfiguration, and other interfaces for more
> >functions, e.g power management, virtualization support and etc.
> >
> >/sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager
> >
> >Thanks
> >Hao
> >
> >>
> >>thanks,
> >>
> >>greg k-h
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Tull July 25, 2017, 9:32 p.m. UTC | #11
On Sat, Apr 1, 2017 at 7:18 AM, Wu Hao <hao.wu@intel.com> wrote:

Hi Hao,

> On Fri, Mar 31, 2017 at 12:01:13PM -0700, matthew.gerlach@linux.intel.com wrote:
>> On Fri, 31 Mar 2017, Wu Hao wrote:
>> >On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
>> >>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.
>> >>
>> >>How does this interact with the existing "fpga class" that is in the
>> >>kernel already?
>> >
>> >The fpga-dev introduced by this patch, is only a container device, and
>>
>> I completely understand the need for a container device.  The fpga-region is
>> also primarily a container, and in some cases the fpga-region may represent
>> the entire fpga.  Over time this code may become redundant.
>
> Thanks a lot for your review and comments.
>
> I feel that the fpga-region implies that it supports reconfiguration,

On Arria10, we create base fpga region which does not support full
reconfiguration.  It corresponds to the whole FPGA area, which was
loaded with a static FPGA image in the bootloader.  The partial
reconfiguration regions are children of the base FPGA region.  Any
devices in the FPGA are child devices of either the base region or a
region which is a child of it.

> but
> in our cases, the Intel FPGA device, doesn't have base fpga-region for
> full reconfiguration, but many accelerators with partial reconfiguration
> support. A fpga-region brings together everything needed for the
> reconfiguration, and a fpga-dev is trying to brings everything on a FPGA
> device together, including fpga-region/bridge/manager, access different
> accelerators and other function units.
>
> I think it's not mandatory to use fpga-dev, as fpga-dev is just trying to
> provide one more option here for some complex hardware.

Now that you've put out v2 which uses fpga-regions, do you still need
fpga-dev class?

Alan

>
> Thanks
> Hao
>
>> >drivers could register different functions under it. Per my understanding,
>> >the existing "fpga class", including fpga-region, fpga-bridge and
>> >fpga-manager, is used to provide reconfiguration function for FPGA. So
>> >driver can create child node using this existing "fpga class" to provide
>> >FPGA reconfiguration function, and more nodes under this container for
>> >different functions for given FPGA device.
>> >
>> >For Intel FPGA device, partial reconfiguration is only one function of
>> >Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
>> >below path for partial reconfiguration, and other interfaces for more
>> >functions, e.g power management, virtualization support and etc.
>> >
>> >/sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager
>> >
>> >Thanks
>> >Hao
>> >
>> >>
>> >>thanks,
>> >>
>> >>greg k-h
>> >--
>> >To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
>> >the body of a message to majordomo@vger.kernel.org
>> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao July 26, 2017, 9:50 a.m. UTC | #12
On Tue, Jul 25, 2017 at 04:32:10PM -0500, Alan Tull wrote:
> On Sat, Apr 1, 2017 at 7:18 AM, Wu Hao <hao.wu@intel.com> wrote:
> 
> Hi Hao,
> 
> > On Fri, Mar 31, 2017 at 12:01:13PM -0700, matthew.gerlach@linux.intel.com wrote:
> >> On Fri, 31 Mar 2017, Wu Hao wrote:
> >> >On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
> >> >>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.
> >> >>
> >> >>How does this interact with the existing "fpga class" that is in the
> >> >>kernel already?
> >> >
> >> >The fpga-dev introduced by this patch, is only a container device, and
> >>
> >> I completely understand the need for a container device.  The fpga-region is
> >> also primarily a container, and in some cases the fpga-region may represent
> >> the entire fpga.  Over time this code may become redundant.
> >
> > Thanks a lot for your review and comments.
> >
> > I feel that the fpga-region implies that it supports reconfiguration,
> 
> On Arria10, we create base fpga region which does not support full
> reconfiguration.  It corresponds to the whole FPGA area, which was
> loaded with a static FPGA image in the bootloader.  The partial
> reconfiguration regions are children of the base FPGA region.  Any
> devices in the FPGA are child devices of either the base region or a
> region which is a child of it.
> 
> > but
> > in our cases, the Intel FPGA device, doesn't have base fpga-region for
> > full reconfiguration, but many accelerators with partial reconfiguration
> > support. A fpga-region brings together everything needed for the
> > reconfiguration, and a fpga-dev is trying to brings everything on a FPGA
> > device together, including fpga-region/bridge/manager, access different
> > accelerators and other function units.
> >
> > I think it's not mandatory to use fpga-dev, as fpga-dev is just trying to
> > provide one more option here for some complex hardware.
> 
> Now that you've put out v2 which uses fpga-regions, do you still need
> fpga-dev class?

Hi Alan

Thanks for the comments.

In v2, I have updated the driver organization section in intel-fpga.txt[1].
The fpga-regions/bridges/manager are created as children of FME module, as
the partial reconfiguration function is only a sub feature of FME module.

If switch to fpga-region as container device, it may not be easy for user
space applications to know which one represents a FPGA device and which one
represents a reconfigurable region as all have the similar name 'regionx'
in the same sysfs folder. Please consider this case, if we have 5 fpga
devices on one system and each fpga device has multiple PR regions (e.g 20+).
Then user space applications need to search all regions to locate the ones
represent the FPGA device, even we add some attributes to it.

[1]http://marc.info/?l=linux-fpga&m=149844234509825&w=2

Thanks
Hao

> 
> Alan
> 
> >
> > Thanks
> > Hao
> >
> >> >drivers could register different functions under it. Per my understanding,
> >> >the existing "fpga class", including fpga-region, fpga-bridge and
> >> >fpga-manager, is used to provide reconfiguration function for FPGA. So
> >> >driver can create child node using this existing "fpga class" to provide
> >> >FPGA reconfiguration function, and more nodes under this container for
> >> >different functions for given FPGA device.
> >> >
> >> >For Intel FPGA device, partial reconfiguration is only one function of
> >> >Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
> >> >below path for partial reconfiguration, and other interfaces for more
> >> >functions, e.g power management, virtualization support and etc.
> >> >
> >> >/sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager
> >> >
> >> >Thanks
> >> >Hao
> >> >
> >> >>
> >> >>thanks,
> >> >>
> >> >>greg k-h
> >> >--
> >> >To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
> >> >the body of a message to majordomo@vger.kernel.org
> >> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> >
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Tull July 26, 2017, 2:20 p.m. UTC | #13
On Wed, Jul 26, 2017 at 4:50 AM, Wu Hao <hao.wu@intel.com> wrote:
> On Tue, Jul 25, 2017 at 04:32:10PM -0500, Alan Tull wrote:
>> On Sat, Apr 1, 2017 at 7:18 AM, Wu Hao <hao.wu@intel.com> wrote:
>>
>> Hi Hao,
>>
>> > On Fri, Mar 31, 2017 at 12:01:13PM -0700, matthew.gerlach@linux.intel.com wrote:
>> >> On Fri, 31 Mar 2017, Wu Hao wrote:
>> >> >On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
>> >> >>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.
>> >> >>
>> >> >>How does this interact with the existing "fpga class" that is in the
>> >> >>kernel already?
>> >> >
>> >> >The fpga-dev introduced by this patch, is only a container device, and
>> >>
>> >> I completely understand the need for a container device.  The fpga-region is
>> >> also primarily a container, and in some cases the fpga-region may represent
>> >> the entire fpga.  Over time this code may become redundant.
>> >
>> > Thanks a lot for your review and comments.
>> >
>> > I feel that the fpga-region implies that it supports reconfiguration,
>>
>> On Arria10, we create base fpga region which does not support full
>> reconfiguration.  It corresponds to the whole FPGA area, which was
>> loaded with a static FPGA image in the bootloader.  The partial
>> reconfiguration regions are children of the base FPGA region.  Any
>> devices in the FPGA are child devices of either the base region or a
>> region which is a child of it.
>>
>> > but
>> > in our cases, the Intel FPGA device, doesn't have base fpga-region for
>> > full reconfiguration, but many accelerators with partial reconfiguration
>> > support. A fpga-region brings together everything needed for the
>> > reconfiguration, and a fpga-dev is trying to brings everything on a FPGA
>> > device together, including fpga-region/bridge/manager, access different
>> > accelerators and other function units.
>> >
>> > I think it's not mandatory to use fpga-dev, as fpga-dev is just trying to
>> > provide one more option here for some complex hardware.
>>
>> Now that you've put out v2 which uses fpga-regions, do you still need
>> fpga-dev class?
>
> Hi Alan
>
> Thanks for the comments.
>
> In v2, I have updated the driver organization section in intel-fpga.txt[1].


I've read your v2 of this document.  It's changed as you've said, but
not that much.  I'm just continuing the previous conversation.  I'll
add further comments on the v2 version.

Alan

> The fpga-regions/bridges/manager are created as children of FME module, as
> the partial reconfiguration function is only a sub feature of FME module.
>
> If switch to fpga-region as container device, it may not be easy for user
> space applications to know which one represents a FPGA device and which one
> represents a reconfigurable region as all have the similar name 'regionx'
> in the same sysfs folder. Please consider this case, if we have 5 fpga
> devices on one system and each fpga device has multiple PR regions (e.g 20+).
> Then user space applications need to search all regions to locate the ones
> represent the FPGA device, even we add some attributes to it.
>
> [1]http://marc.info/?l=linux-fpga&m=149844234509825&w=2
>
> Thanks
> Hao
>
>>
>> Alan
>>
>> >
>> > Thanks
>> > Hao
>> >
>> >> >drivers could register different functions under it. Per my understanding,
>> >> >the existing "fpga class", including fpga-region, fpga-bridge and
>> >> >fpga-manager, is used to provide reconfiguration function for FPGA. So
>> >> >driver can create child node using this existing "fpga class" to provide
>> >> >FPGA reconfiguration function, and more nodes under this container for
>> >> >different functions for given FPGA device.
>> >> >
>> >> >For Intel FPGA device, partial reconfiguration is only one function of
>> >> >Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
>> >> >below path for partial reconfiguration, and other interfaces for more
>> >> >functions, e.g power management, virtualization support and etc.
>> >> >
>> >> >/sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager
>> >> >
>> >> >Thanks
>> >> >Hao
>> >> >
>> >> >>
>> >> >>thanks,
>> >> >>
>> >> >>greg k-h
>> >> >--
>> >> >To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
>> >> >the body of a message to majordomo@vger.kernel.org
>> >> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >> >
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Tull July 26, 2017, 10:29 p.m. UTC | #14
On Wed, Jul 26, 2017 at 9:20 AM, Alan Tull <atull@kernel.org> wrote:
> On Wed, Jul 26, 2017 at 4:50 AM, Wu Hao <hao.wu@intel.com> wrote:
>> On Tue, Jul 25, 2017 at 04:32:10PM -0500, Alan Tull wrote:
>>> On Sat, Apr 1, 2017 at 7:18 AM, Wu Hao <hao.wu@intel.com> wrote:
>>>
>>> Hi Hao,
>>>
>>> > On Fri, Mar 31, 2017 at 12:01:13PM -0700, matthew.gerlach@linux.intel.com wrote:
>>> >> On Fri, 31 Mar 2017, Wu Hao wrote:
>>> >> >On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
>>> >> >>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.
>>> >> >>
>>> >> >>How does this interact with the existing "fpga class" that is in the
>>> >> >>kernel already?
>>> >> >
>>> >> >The fpga-dev introduced by this patch, is only a container device, and
>>> >>
>>> >> I completely understand the need for a container device.  The fpga-region is
>>> >> also primarily a container, and in some cases the fpga-region may represent
>>> >> the entire fpga.  Over time this code may become redundant.
>>> >
>>> > Thanks a lot for your review and comments.
>>> >
>>> > I feel that the fpga-region implies that it supports reconfiguration,
>>>
>>> On Arria10, we create base fpga region which does not support full
>>> reconfiguration.  It corresponds to the whole FPGA area, which was
>>> loaded with a static FPGA image in the bootloader.  The partial
>>> reconfiguration regions are children of the base FPGA region.  Any
>>> devices in the FPGA are child devices of either the base region or a
>>> region which is a child of it.
>>>
>>> > but
>>> > in our cases, the Intel FPGA device, doesn't have base fpga-region for
>>> > full reconfiguration, but many accelerators with partial reconfiguration
>>> > support. A fpga-region brings together everything needed for the
>>> > reconfiguration, and a fpga-dev is trying to brings everything on a FPGA
>>> > device together, including fpga-region/bridge/manager, access different
>>> > accelerators and other function units.
>>> >
>>> > I think it's not mandatory to use fpga-dev, as fpga-dev is just trying to
>>> > provide one more option here for some complex hardware.
>>>
>>> Now that you've put out v2 which uses fpga-regions, do you still need
>>> fpga-dev class?
>>
>> Hi Alan
>>
>> Thanks for the comments.
>>
>> In v2, I have updated the driver organization section in intel-fpga.txt[1].
>
>
> I've read your v2 of this document.  It's changed as you've said, but
> not that much.

I should clarify here that, yes I see that in v2 you're now using
regions and bridges and I appreciate that.  I'm just trying to see
what a good relationship between the existing fpga classes and the new
fpga-dev class would be.

> I'm just continuing the previous conversation.  I'll
> add further comments on the v2 version.
>
> Alan
>
>> The fpga-regions/bridges/manager are created as children of FME module, as
>> the partial reconfiguration function is only a sub feature of FME module.
>>
>> If switch to fpga-region as container device, it may not be easy for user
>> space applications to know which one represents a FPGA device and which one
>> represents a reconfigurable region as all have the similar name 'regionx'
>> in the same sysfs folder. Please consider this case, if we have 5 fpga
>> devices on one system and each fpga device has multiple PR regions (e.g 20+).
>> Then user space applications need to search all regions to locate the ones
>> represent the FPGA device, even we add some attributes to it.
>>
>> [1]http://marc.info/?l=linux-fpga&m=149844234509825&w=2
>>
>> Thanks
>> Hao
>>
>>>
>>> Alan
>>>
>>> >
>>> > Thanks
>>> > Hao
>>> >
>>> >> >drivers could register different functions under it. Per my understanding,
>>> >> >the existing "fpga class", including fpga-region, fpga-bridge and
>>> >> >fpga-manager, is used to provide reconfiguration function for FPGA. So
>>> >> >driver can create child node using this existing "fpga class" to provide
>>> >> >FPGA reconfiguration function, and more nodes under this container for
>>> >> >different functions for given FPGA device.
>>> >> >
>>> >> >For Intel FPGA device, partial reconfiguration is only one function of
>>> >> >Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
>>> >> >below path for partial reconfiguration, and other interfaces for more
>>> >> >functions, e.g power management, virtualization support and etc.
>>> >> >
>>> >> >/sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager
>>> >> >
>>> >> >Thanks
>>> >> >Hao
>>> >> >
>>> >> >>
>>> >> >>thanks,
>>> >> >>
>>> >> >>greg k-h
>>> >> >--
>>> >> >To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
>>> >> >the body of a message to majordomo@vger.kernel.org
>>> >> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> >> >
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao July 27, 2017, 4:54 a.m. UTC | #15
On Wed, Jul 26, 2017 at 05:29:11PM -0500, Alan Tull wrote:
> On Wed, Jul 26, 2017 at 9:20 AM, Alan Tull <atull@kernel.org> wrote:
> > On Wed, Jul 26, 2017 at 4:50 AM, Wu Hao <hao.wu@intel.com> wrote:
> >> On Tue, Jul 25, 2017 at 04:32:10PM -0500, Alan Tull wrote:
> >>> On Sat, Apr 1, 2017 at 7:18 AM, Wu Hao <hao.wu@intel.com> wrote:
> >>>
> >>> Hi Hao,
> >>>
> >>> > On Fri, Mar 31, 2017 at 12:01:13PM -0700, matthew.gerlach@linux.intel.com wrote:
> >>> >> On Fri, 31 Mar 2017, Wu Hao wrote:
> >>> >> >On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
> >>> >> >>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.
> >>> >> >>
> >>> >> >>How does this interact with the existing "fpga class" that is in the
> >>> >> >>kernel already?
> >>> >> >
> >>> >> >The fpga-dev introduced by this patch, is only a container device, and
> >>> >>
> >>> >> I completely understand the need for a container device.  The fpga-region is
> >>> >> also primarily a container, and in some cases the fpga-region may represent
> >>> >> the entire fpga.  Over time this code may become redundant.
> >>> >
> >>> > Thanks a lot for your review and comments.
> >>> >
> >>> > I feel that the fpga-region implies that it supports reconfiguration,
> >>>
> >>> On Arria10, we create base fpga region which does not support full
> >>> reconfiguration.  It corresponds to the whole FPGA area, which was
> >>> loaded with a static FPGA image in the bootloader.  The partial
> >>> reconfiguration regions are children of the base FPGA region.  Any
> >>> devices in the FPGA are child devices of either the base region or a
> >>> region which is a child of it.
> >>>
> >>> > but
> >>> > in our cases, the Intel FPGA device, doesn't have base fpga-region for
> >>> > full reconfiguration, but many accelerators with partial reconfiguration
> >>> > support. A fpga-region brings together everything needed for the
> >>> > reconfiguration, and a fpga-dev is trying to brings everything on a FPGA
> >>> > device together, including fpga-region/bridge/manager, access different
> >>> > accelerators and other function units.
> >>> >
> >>> > I think it's not mandatory to use fpga-dev, as fpga-dev is just trying to
> >>> > provide one more option here for some complex hardware.
> >>>
> >>> Now that you've put out v2 which uses fpga-regions, do you still need
> >>> fpga-dev class?
> >>
> >> Hi Alan
> >>
> >> Thanks for the comments.
> >>
> >> In v2, I have updated the driver organization section in intel-fpga.txt[1].
> >
> >
> > I've read your v2 of this document.  It's changed as you've said, but
> > not that much.
> 
> I should clarify here that, yes I see that in v2 you're now using
> regions and bridges and I appreciate that.  I'm just trying to see
> what a good relationship between the existing fpga classes and the new
> fpga-dev class would be.

Sure, I think these existing fpga classes could be used as child nodes
under fpga-dev, or directly used without it. If we use fpga-dev as container
then user may be able to find the target region easily from sysfs hierarchy
(e.g user wants to do PR on 3rd fpga device's 7th PR region via its fpga
region user interface.). : )

Thanks
Hao

> 
> > I'm just continuing the previous conversation.  I'll
> > add further comments on the v2 version.
> >
> > Alan
> >
> >> The fpga-regions/bridges/manager are created as children of FME module, as
> >> the partial reconfiguration function is only a sub feature of FME module.
> >>
> >> If switch to fpga-region as container device, it may not be easy for user
> >> space applications to know which one represents a FPGA device and which one
> >> represents a reconfigurable region as all have the similar name 'regionx'
> >> in the same sysfs folder. Please consider this case, if we have 5 fpga
> >> devices on one system and each fpga device has multiple PR regions (e.g 20+).
> >> Then user space applications need to search all regions to locate the ones
> >> represent the FPGA device, even we add some attributes to it.
> >>
> >> [1]http://marc.info/?l=linux-fpga&m=149844234509825&w=2
> >>
> >> Thanks
> >> Hao
> >>
> >>>
> >>> Alan
> >>>
> >>> >
> >>> > Thanks
> >>> > Hao
> >>> >
> >>> >> >drivers could register different functions under it. Per my understanding,
> >>> >> >the existing "fpga class", including fpga-region, fpga-bridge and
> >>> >> >fpga-manager, is used to provide reconfiguration function for FPGA. So
> >>> >> >driver can create child node using this existing "fpga class" to provide
> >>> >> >FPGA reconfiguration function, and more nodes under this container for
> >>> >> >different functions for given FPGA device.
> >>> >> >
> >>> >> >For Intel FPGA device, partial reconfiguration is only one function of
> >>> >> >Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
> >>> >> >below path for partial reconfiguration, and other interfaces for more
> >>> >> >functions, e.g power management, virtualization support and etc.
> >>> >> >
> >>> >> >/sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager
> >>> >> >
> >>> >> >Thanks
> >>> >> >Hao
> >>> >> >
> >>> >> >>
> >>> >> >>thanks,
> >>> >> >>
> >>> >> >>greg k-h
> >>> >> >--
> >>> >> >To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
> >>> >> >the body of a message to majordomo@vger.kernel.org
> >>> >> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>> >> >
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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.
+ */
+#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);
+
+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.
+ *
+ */
+#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;
+};
+
+#define to_fpga_dev(d) container_of(d, struct fpga_dev, dev)
+
+struct fpga_dev *fpga_dev_create(struct device *parent, const char *name);
+
+static inline void fpga_dev_destroy(struct fpga_dev *fdev)
+{
+	device_unregister(&fdev->dev);
+}
+
+#endif