[PATCH v3 0/3] UIO support for dfl devices

[PATCH v3 0/3] UIO support for dfl devices

[Xu Yilun]

This patchset supports some dfl device drivers written in userspace.

In the patchset v1, the "driver_override" interface should be used to bind
the DFL UIO driver to DFL devices. But there is concern that the
"driver_override" interface is not OK itself. So in v2, we use a new
matching algorithem. The "driver_override" interface is abandoned, the DFL
UIO driver matches any DFL device which could not be handled by other DFL
drivers. So the DFL UIO driver could be used for new DFL devices which are
not supported by kernel, also it will not impact the devices which are
already got supported.

Main changes from v1:
- switch to the new matching algorithem. It matches DFL devices which could
  not be handled by other DFL drivers.
- refacor the code about device resources filling.
- add the documentation.

Main changes from v2:
- split the match ops changes in dfl.c to an independent patch.
- move the declarations needed for dfl-uio-pdev from include/linux/dfl.h
  to driver/fpga/dfl.h
- some minor fixes.


Xu Yilun (3):
  fpga: dfl: add the match() ops for dfl driver
  fpga: dfl: add the userspace I/O device support for DFL devices
  Documentation: fpga: dfl: Add description for DFL UIO support

 Documentation/fpga/dfl.rst  |  27 +++++++++++
 drivers/fpga/Kconfig        |  10 ++++
 drivers/fpga/Makefile       |   1 +
 drivers/fpga/dfl-uio-pdev.c | 110 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl.c          |  22 +++++++--
 drivers/fpga/dfl.h          |   5 ++
 include/linux/dfl.h         |   3 ++
 7 files changed, 173 insertions(+), 5 deletions(-)
 create mode 100644 drivers/fpga/dfl-uio-pdev.c

-- 
2.7.4

[PATCH v3 1/3] fpga: dfl: add the match() ops for dfl driver

[Xu Yilun]

The match ops allows dfl drivers have their own matching algorithem
instead of the standard id_table matching.

This is to support the DFL UIO driver. It intends to match any DFL
device which could not be handled by other DFL drivers.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
---
v3: this patch is splited out from DFL UIO patch.
    move the declarations of exported symbols from include/linux/dfl.h
     to driver/fpga/dfl.h
    fix some comments.
---
 drivers/fpga/dfl.c  | 22 +++++++++++++++++-----
 drivers/fpga/dfl.h  |  5 +++++
 include/linux/dfl.h |  3 +++
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 511b20f..dd90111 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -256,12 +256,13 @@ dfl_match_one_device(const struct dfl_device_id *id, struct dfl_device *ddev)
 	return NULL;
 }
 
-static int dfl_bus_match(struct device *dev, struct device_driver *drv)
+int dfl_match_device(struct dfl_device *ddev, struct dfl_driver *ddrv)
 {
-	struct dfl_device *ddev = to_dfl_dev(dev);
-	struct dfl_driver *ddrv = to_dfl_drv(drv);
 	const struct dfl_device_id *id_entry;
 
+	if (ddrv->match)
+		return ddrv->match(ddev);
+
 	id_entry = ddrv->id_table;
 	if (id_entry) {
 		while (id_entry->feature_id) {
@@ -275,6 +276,15 @@ static int dfl_bus_match(struct device *dev, struct device_driver *drv)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dfl_match_device);
+
+static int dfl_bus_match(struct device *dev, struct device_driver *drv)
+{
+	struct dfl_device *ddev = to_dfl_dev(dev);
+	struct dfl_driver *ddrv = to_dfl_drv(drv);
+
+	return dfl_match_device(ddev, ddrv);
+}
 
 static int dfl_bus_probe(struct device *dev)
 {
@@ -328,7 +338,7 @@ static struct attribute *dfl_dev_attrs[] = {
 };
 ATTRIBUTE_GROUPS(dfl_dev);
 
-static struct bus_type dfl_bus_type = {
+struct bus_type dfl_bus_type = {
 	.name		= "dfl",
 	.match		= dfl_bus_match,
 	.probe		= dfl_bus_probe,
@@ -336,6 +346,7 @@ static struct bus_type dfl_bus_type = {
 	.uevent		= dfl_bus_uevent,
 	.dev_groups	= dfl_dev_groups,
 };
+EXPORT_SYMBOL_GPL(dfl_bus_type);
 
 static void release_dfl_dev(struct device *dev)
 {
@@ -469,7 +480,8 @@ static int dfl_devs_add(struct dfl_feature_platform_data *pdata)
 
 int __dfl_driver_register(struct dfl_driver *dfl_drv, struct module *owner)
 {
-	if (!dfl_drv || !dfl_drv->probe || !dfl_drv->id_table)
+	if (!dfl_drv || !dfl_drv->probe ||
+	    (!dfl_drv->id_table && !dfl_drv->match))
 		return -EINVAL;
 
 	dfl_drv->drv.owner = owner;
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 2b82c96..01c43d8 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -17,6 +17,7 @@
 #include <linux/bitfield.h>
 #include <linux/cdev.h>
 #include <linux/delay.h>
+#include <linux/dfl.h>
 #include <linux/eventfd.h>
 #include <linux/fs.h>
 #include <linux/interrupt.h>
@@ -517,4 +518,8 @@ long dfl_feature_ioctl_set_irq(struct platform_device *pdev,
 			       struct dfl_feature *feature,
 			       unsigned long arg);
 
+extern struct bus_type dfl_bus_type;
+
+int dfl_match_device(struct dfl_device *ddev, struct dfl_driver *ddrv);
+
 #endif /* __FPGA_DFL_H */
diff --git a/include/linux/dfl.h b/include/linux/dfl.h
index 6cc1098..cfd98a4 100644
--- a/include/linux/dfl.h
+++ b/include/linux/dfl.h
@@ -51,6 +51,8 @@ struct dfl_device {
  * @drv: driver model structure.
  * @id_table: pointer to table of device IDs the driver is interested in.
  *	      { } member terminated.
+ * @match: returns one if given device can be handled by the driver and zero
+ *	   otherwise. If NULL, matching is based on id_table.
  * @probe: mandatory callback for device binding.
  * @remove: callback for device unbinding.
  */
@@ -58,6 +60,7 @@ struct dfl_driver {
 	struct device_driver drv;
 	const struct dfl_device_id *id_table;
 
+	int (*match)(struct dfl_device *dfl_dev);
 	int (*probe)(struct dfl_device *dfl_dev);
 	void (*remove)(struct dfl_device *dfl_dev);
 };
-- 
2.7.4

Re: [PATCH v3 1/3] fpga: dfl: add the match() ops for dfl driver

[Tom Rix]

On 12/16/20 9:44 PM, Xu Yilun wrote:
> The match ops allows dfl drivers have their own matching algorithem
> instead of the standard id_table matching.
>
> This is to support the DFL UIO driver. It intends to match any DFL
> device which could not be handled by other DFL drivers.
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>

This looks fine.

Reviewed-by: Tom Rix <trix@redhat.com>

> ---
> v3: this patch is splited out from DFL UIO patch.
>     move the declarations of exported symbols from include/linux/dfl.h
>      to driver/fpga/dfl.h
>     fix some comments.
> ---
>  drivers/fpga/dfl.c  | 22 +++++++++++++++++-----
>  drivers/fpga/dfl.h  |  5 +++++
>  include/linux/dfl.h |  3 +++
>  3 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 511b20f..dd90111 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -256,12 +256,13 @@ dfl_match_one_device(const struct dfl_device_id *id, struct dfl_device *ddev)
>  	return NULL;
>  }
>  
> -static int dfl_bus_match(struct device *dev, struct device_driver *drv)
> +int dfl_match_device(struct dfl_device *ddev, struct dfl_driver *ddrv)
>  {
> -	struct dfl_device *ddev = to_dfl_dev(dev);
> -	struct dfl_driver *ddrv = to_dfl_drv(drv);
>  	const struct dfl_device_id *id_entry;
>  
> +	if (ddrv->match)
> +		return ddrv->match(ddev);
> +
>  	id_entry = ddrv->id_table;
>  	if (id_entry) {
>  		while (id_entry->feature_id) {
> @@ -275,6 +276,15 @@ static int dfl_bus_match(struct device *dev, struct device_driver *drv)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dfl_match_device);
> +
> +static int dfl_bus_match(struct device *dev, struct device_driver *drv)
> +{
> +	struct dfl_device *ddev = to_dfl_dev(dev);
> +	struct dfl_driver *ddrv = to_dfl_drv(drv);
> +
> +	return dfl_match_device(ddev, ddrv);
> +}
>  
>  static int dfl_bus_probe(struct device *dev)
>  {
> @@ -328,7 +338,7 @@ static struct attribute *dfl_dev_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(dfl_dev);
>  
> -static struct bus_type dfl_bus_type = {
> +struct bus_type dfl_bus_type = {
>  	.name		= "dfl",
>  	.match		= dfl_bus_match,
>  	.probe		= dfl_bus_probe,
> @@ -336,6 +346,7 @@ static struct bus_type dfl_bus_type = {
>  	.uevent		= dfl_bus_uevent,
>  	.dev_groups	= dfl_dev_groups,
>  };
> +EXPORT_SYMBOL_GPL(dfl_bus_type);
>  
>  static void release_dfl_dev(struct device *dev)
>  {
> @@ -469,7 +480,8 @@ static int dfl_devs_add(struct dfl_feature_platform_data *pdata)
>  
>  int __dfl_driver_register(struct dfl_driver *dfl_drv, struct module *owner)
>  {
> -	if (!dfl_drv || !dfl_drv->probe || !dfl_drv->id_table)
> +	if (!dfl_drv || !dfl_drv->probe ||
> +	    (!dfl_drv->id_table && !dfl_drv->match))
>  		return -EINVAL;
>  
>  	dfl_drv->drv.owner = owner;
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 2b82c96..01c43d8 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -17,6 +17,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/cdev.h>
>  #include <linux/delay.h>
> +#include <linux/dfl.h>
>  #include <linux/eventfd.h>
>  #include <linux/fs.h>
>  #include <linux/interrupt.h>
> @@ -517,4 +518,8 @@ long dfl_feature_ioctl_set_irq(struct platform_device *pdev,
>  			       struct dfl_feature *feature,
>  			       unsigned long arg);
>  
> +extern struct bus_type dfl_bus_type;
> +
> +int dfl_match_device(struct dfl_device *ddev, struct dfl_driver *ddrv);
> +
>  #endif /* __FPGA_DFL_H */
> diff --git a/include/linux/dfl.h b/include/linux/dfl.h
> index 6cc1098..cfd98a4 100644
> --- a/include/linux/dfl.h
> +++ b/include/linux/dfl.h
> @@ -51,6 +51,8 @@ struct dfl_device {
>   * @drv: driver model structure.
>   * @id_table: pointer to table of device IDs the driver is interested in.
>   *	      { } member terminated.
> + * @match: returns one if given device can be handled by the driver and zero
> + *	   otherwise. If NULL, matching is based on id_table.
>   * @probe: mandatory callback for device binding.
>   * @remove: callback for device unbinding.
>   */
> @@ -58,6 +60,7 @@ struct dfl_driver {
>  	struct device_driver drv;
>  	const struct dfl_device_id *id_table;
>  
> +	int (*match)(struct dfl_device *dfl_dev);
>  	int (*probe)(struct dfl_device *dfl_dev);
>  	void (*remove)(struct dfl_device *dfl_dev);
>  };

[PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for DFL devices

[Xu Yilun]

This patch supports the DFL drivers be written in userspace. This is
realized by exposing the userspace I/O device interfaces.

The driver leverages the uio_pdrv_genirq, it adds the uio_pdrv_genirq
platform device with the DFL device's resources, and let the generic UIO
platform device driver provide support to userspace access to kernel
interrupts and memory locations.

The driver matches DFL devices in a different way. It has no device id
table, instead it matches any DFL device which could not be handled by
other DFL drivers.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
---
v2: switch to the new matching algorithem. It matches DFL devices which
     could not be handled by other DFL drivers.
    refacor the code about device resources filling.
    fix some comments.
v3: split the dfl.c changes out of this patch.
    some minor fixes
---
 drivers/fpga/Kconfig        |  10 ++++
 drivers/fpga/Makefile       |   1 +
 drivers/fpga/dfl-uio-pdev.c | 110 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+)
 create mode 100644 drivers/fpga/dfl-uio-pdev.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 5d7f0ae..7a88af9 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -202,6 +202,16 @@ config FPGA_DFL_NIOS_INTEL_PAC_N3000
 	  the card. It also instantiates the SPI master (spi-altera) for
 	  the card's BMC (Board Management Controller).
 
+config FPGA_DFL_UIO_PDEV
+	tristate "FPGA DFL Driver for Userspace I/O platform devices"
+	depends on FPGA_DFL && UIO_PDRV_GENIRQ
+	help
+	  Enable this to allow some DFL drivers be written in userspace. It
+	  adds the uio_pdrv_genirq platform device with the DFL feature's
+	  resources, and lets the generic UIO platform device driver provide
+	  support for userspace access to kernel interrupts and memory
+	  locations.
+
 config FPGA_DFL_PCI
 	tristate "FPGA DFL PCIe Device Driver"
 	depends on PCI && FPGA_DFL
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 18dc9885..8847fe0 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -45,6 +45,7 @@ dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
 dfl-afu-objs += dfl-afu-error.o
 
 obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-nios.o
+obj-$(CONFIG_FPGA_DFL_UIO_PDEV)		+= dfl-uio-pdev.o
 
 # Drivers for FPGAs which implement DFL
 obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
diff --git a/drivers/fpga/dfl-uio-pdev.c b/drivers/fpga/dfl-uio-pdev.c
new file mode 100644
index 0000000..8c57233
--- /dev/null
+++ b/drivers/fpga/dfl-uio-pdev.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DFL driver for Userspace I/O platform devices
+ *
+ * Copyright (C) 2020 Intel Corporation, Inc.
+ */
+#include <linux/dfl.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/uio_driver.h>
+
+#include "dfl.h"
+
+#define DRIVER_NAME "dfl-uio-pdev"
+
+static struct dfl_driver dfl_uio_pdev_driver;
+
+static int check_for_other_drv_match(struct device_driver *drv, void *data)
+{
+	struct dfl_driver *ddrv = to_dfl_drv(drv);
+	struct dfl_device *ddev = data;
+
+	/* skip myself */
+	if (ddrv == &dfl_uio_pdev_driver)
+		return 0;
+
+	return dfl_match_device(ddev, ddrv);
+}
+
+static int dfl_uio_pdev_match(struct dfl_device *ddev)
+{
+	/*
+	 * If any other driver wants the device, leave the device to this other
+	 * driver.
+	 */
+	if (bus_for_each_drv(&dfl_bus_type, NULL, ddev, check_for_other_drv_match))
+		return 0;
+
+	return 1;
+}
+
+static int dfl_uio_pdev_probe(struct dfl_device *ddev)
+{
+	struct device *dev = &ddev->dev;
+	struct platform_device_info pdevinfo = { 0 };
+	struct uio_info uio_pdata = { 0 };
+	struct platform_device *uio_pdev;
+	struct resource *res;
+	int i;
+
+	pdevinfo.name = "uio_pdrv_genirq";
+
+	res = kcalloc(ddev->num_irqs + 1, sizeof(*res), GFP_KERNEL);
+	if (!res)
+		return -ENOMEM;
+
+	res[0].parent = &ddev->mmio_res;
+	res[0].flags = IORESOURCE_MEM;
+	res[0].start = ddev->mmio_res.start;
+	res[0].end = ddev->mmio_res.end;
+
+	/* then add irq resource */
+	for (i = 0; i < ddev->num_irqs; i++) {
+		res[i + 1].flags = IORESOURCE_IRQ;
+		res[i + 1].start = ddev->irqs[i];
+		res[i + 1].end = ddev->irqs[i];
+	}
+
+	uio_pdata.name = DRIVER_NAME;
+	uio_pdata.version = "0";
+
+	pdevinfo.res = res;
+	pdevinfo.num_res = ddev->num_irqs + 1;
+	pdevinfo.parent = &ddev->dev;
+	pdevinfo.id = PLATFORM_DEVID_AUTO;
+	pdevinfo.data = &uio_pdata;
+	pdevinfo.size_data = sizeof(uio_pdata);
+
+	uio_pdev = platform_device_register_full(&pdevinfo);
+	if (!IS_ERR(uio_pdev))
+		dev_set_drvdata(dev, uio_pdev);
+
+	kfree(res);
+
+	return PTR_ERR_OR_ZERO(uio_pdev);
+}
+
+static void dfl_uio_pdev_remove(struct dfl_device *ddev)
+{
+	struct platform_device *uio_pdev = dev_get_drvdata(&ddev->dev);
+
+	platform_device_unregister(uio_pdev);
+}
+
+static struct dfl_driver dfl_uio_pdev_driver = {
+	.drv	= {
+		.name       = DRIVER_NAME,
+	},
+	.match	= dfl_uio_pdev_match,
+	.probe	= dfl_uio_pdev_probe,
+	.remove	= dfl_uio_pdev_remove,
+};
+module_dfl_driver(dfl_uio_pdev_driver);
+
+MODULE_DESCRIPTION("DFL driver for Userspace I/O platform devices");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for DFL devices

[Tom Rix]

On 12/16/20 9:44 PM, Xu Yilun wrote:
> This patch supports the DFL drivers be written in userspace. This is
> realized by exposing the userspace I/O device interfaces.
>
> The driver leverages the uio_pdrv_genirq, it adds the uio_pdrv_genirq
> platform device with the DFL device's resources, and let the generic UIO
> platform device driver provide support to userspace access to kernel
> interrupts and memory locations.
>
> The driver matches DFL devices in a different way. It has no device id
> table, instead it matches any DFL device which could not be handled by
> other DFL drivers.

Thanks for splitting out the match part, this looks fine.

Reviewed-by: Tom Rix <trix@redhat.com>

> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> ---
> v2: switch to the new matching algorithem. It matches DFL devices which
>      could not be handled by other DFL drivers.
>     refacor the code about device resources filling.
>     fix some comments.
> v3: split the dfl.c changes out of this patch.
>     some minor fixes
> ---
>  drivers/fpga/Kconfig        |  10 ++++
>  drivers/fpga/Makefile       |   1 +
>  drivers/fpga/dfl-uio-pdev.c | 110 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 121 insertions(+)
>  create mode 100644 drivers/fpga/dfl-uio-pdev.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 5d7f0ae..7a88af9 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -202,6 +202,16 @@ config FPGA_DFL_NIOS_INTEL_PAC_N3000
>  	  the card. It also instantiates the SPI master (spi-altera) for
>  	  the card's BMC (Board Management Controller).
>  
> +config FPGA_DFL_UIO_PDEV
> +	tristate "FPGA DFL Driver for Userspace I/O platform devices"
> +	depends on FPGA_DFL && UIO_PDRV_GENIRQ
> +	help
> +	  Enable this to allow some DFL drivers be written in userspace. It
> +	  adds the uio_pdrv_genirq platform device with the DFL feature's
> +	  resources, and lets the generic UIO platform device driver provide
> +	  support for userspace access to kernel interrupts and memory
> +	  locations.
> +
>  config FPGA_DFL_PCI
>  	tristate "FPGA DFL PCIe Device Driver"
>  	depends on PCI && FPGA_DFL
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 18dc9885..8847fe0 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -45,6 +45,7 @@ dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
>  dfl-afu-objs += dfl-afu-error.o
>  
>  obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-nios.o
> +obj-$(CONFIG_FPGA_DFL_UIO_PDEV)		+= dfl-uio-pdev.o
>  
>  # Drivers for FPGAs which implement DFL
>  obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
> diff --git a/drivers/fpga/dfl-uio-pdev.c b/drivers/fpga/dfl-uio-pdev.c
> new file mode 100644
> index 0000000..8c57233
> --- /dev/null
> +++ b/drivers/fpga/dfl-uio-pdev.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DFL driver for Userspace I/O platform devices
> + *
> + * Copyright (C) 2020 Intel Corporation, Inc.
> + */
> +#include <linux/dfl.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/uio_driver.h>
> +
> +#include "dfl.h"
> +
> +#define DRIVER_NAME "dfl-uio-pdev"
> +
> +static struct dfl_driver dfl_uio_pdev_driver;
> +
> +static int check_for_other_drv_match(struct device_driver *drv, void *data)
> +{
> +	struct dfl_driver *ddrv = to_dfl_drv(drv);
> +	struct dfl_device *ddev = data;
> +
> +	/* skip myself */
> +	if (ddrv == &dfl_uio_pdev_driver)
> +		return 0;
> +
> +	return dfl_match_device(ddev, ddrv);
> +}
> +
> +static int dfl_uio_pdev_match(struct dfl_device *ddev)
> +{
> +	/*
> +	 * If any other driver wants the device, leave the device to this other
> +	 * driver.
> +	 */
> +	if (bus_for_each_drv(&dfl_bus_type, NULL, ddev, check_for_other_drv_match))
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int dfl_uio_pdev_probe(struct dfl_device *ddev)
> +{
> +	struct device *dev = &ddev->dev;
> +	struct platform_device_info pdevinfo = { 0 };
> +	struct uio_info uio_pdata = { 0 };
> +	struct platform_device *uio_pdev;
> +	struct resource *res;
> +	int i;
> +
> +	pdevinfo.name = "uio_pdrv_genirq";
> +
> +	res = kcalloc(ddev->num_irqs + 1, sizeof(*res), GFP_KERNEL);
> +	if (!res)
> +		return -ENOMEM;
> +
> +	res[0].parent = &ddev->mmio_res;
> +	res[0].flags = IORESOURCE_MEM;
> +	res[0].start = ddev->mmio_res.start;
> +	res[0].end = ddev->mmio_res.end;
> +
> +	/* then add irq resource */
> +	for (i = 0; i < ddev->num_irqs; i++) {
> +		res[i + 1].flags = IORESOURCE_IRQ;
> +		res[i + 1].start = ddev->irqs[i];
> +		res[i + 1].end = ddev->irqs[i];
> +	}
> +
> +	uio_pdata.name = DRIVER_NAME;
> +	uio_pdata.version = "0";
> +
> +	pdevinfo.res = res;
> +	pdevinfo.num_res = ddev->num_irqs + 1;
> +	pdevinfo.parent = &ddev->dev;
> +	pdevinfo.id = PLATFORM_DEVID_AUTO;
> +	pdevinfo.data = &uio_pdata;
> +	pdevinfo.size_data = sizeof(uio_pdata);
> +
> +	uio_pdev = platform_device_register_full(&pdevinfo);
> +	if (!IS_ERR(uio_pdev))
> +		dev_set_drvdata(dev, uio_pdev);
> +
> +	kfree(res);
> +
> +	return PTR_ERR_OR_ZERO(uio_pdev);
> +}
> +
> +static void dfl_uio_pdev_remove(struct dfl_device *ddev)
> +{
> +	struct platform_device *uio_pdev = dev_get_drvdata(&ddev->dev);
> +
> +	platform_device_unregister(uio_pdev);
> +}
> +
> +static struct dfl_driver dfl_uio_pdev_driver = {
> +	.drv	= {
> +		.name       = DRIVER_NAME,
> +	},
> +	.match	= dfl_uio_pdev_match,
> +	.probe	= dfl_uio_pdev_probe,
> +	.remove	= dfl_uio_pdev_remove,
> +};
> +module_dfl_driver(dfl_uio_pdev_driver);
> +
> +MODULE_DESCRIPTION("DFL driver for Userspace I/O platform devices");
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL v2");

RE: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for DFL devices

[Wu, Hao]

> Subject: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for
> DFL devices
> 
> This patch supports the DFL drivers be written in userspace. This is
> realized by exposing the userspace I/O device interfaces.
> 
> The driver leverages the uio_pdrv_genirq, it adds the uio_pdrv_genirq
> platform device with the DFL device's resources, and let the generic UIO
> platform device driver provide support to userspace access to kernel
> interrupts and memory locations.
> 
> The driver matches DFL devices in a different way. It has no device id
> table, instead it matches any DFL device which could not be handled by
> other DFL drivers.

Looks like we want to build UIO driver as the default/generic driver for DFL,
it seems fine but my concern is that UIO has its own limitation, if some day,
dfl device is extended, but UIO has limitation, then we may need to select 
another one as the default driver.. or we can just match them using 
id_table as we know UIO meets the requirement for those DFL devices?

> 
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> ---
> v2: switch to the new matching algorithem. It matches DFL devices which
>      could not be handled by other DFL drivers.
>     refacor the code about device resources filling.
>     fix some comments.
> v3: split the dfl.c changes out of this patch.
>     some minor fixes
> ---
>  drivers/fpga/Kconfig        |  10 ++++
>  drivers/fpga/Makefile       |   1 +
>  drivers/fpga/dfl-uio-pdev.c | 110
> ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 121 insertions(+)
>  create mode 100644 drivers/fpga/dfl-uio-pdev.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 5d7f0ae..7a88af9 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -202,6 +202,16 @@ config FPGA_DFL_NIOS_INTEL_PAC_N3000
>  	  the card. It also instantiates the SPI master (spi-altera) for
>  	  the card's BMC (Board Management Controller).
> 
> +config FPGA_DFL_UIO_PDEV
> +	tristate "FPGA DFL Driver for Userspace I/O platform devices"
> +	depends on FPGA_DFL && UIO_PDRV_GENIRQ
> +	help
> +	  Enable this to allow some DFL drivers be written in userspace. It
> +	  adds the uio_pdrv_genirq platform device with the DFL feature's
> +	  resources, and lets the generic UIO platform device driver provide
> +	  support for userspace access to kernel interrupts and memory
> +	  locations.

If we consider this as a default driver for everybody in DFL, then we could
consider build it into the core, otherwise it always requires to be loaded
manually, right?

> +
>  config FPGA_DFL_PCI
>  	tristate "FPGA DFL PCIe Device Driver"
>  	depends on PCI && FPGA_DFL
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 18dc9885..8847fe0 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -45,6 +45,7 @@ dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-
> dma-region.o
>  dfl-afu-objs += dfl-afu-error.o
> 
>  obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-nios.o
> +obj-$(CONFIG_FPGA_DFL_UIO_PDEV)		+= dfl-uio-pdev.o
> 
>  # Drivers for FPGAs which implement DFL
>  obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
> diff --git a/drivers/fpga/dfl-uio-pdev.c b/drivers/fpga/dfl-uio-pdev.c
> new file mode 100644
> index 0000000..8c57233
> --- /dev/null
> +++ b/drivers/fpga/dfl-uio-pdev.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DFL driver for Userspace I/O platform devices
> + *
> + * Copyright (C) 2020 Intel Corporation, Inc.
> + */
> +#include <linux/dfl.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/uio_driver.h>
> +
> +#include "dfl.h"
> +
> +#define DRIVER_NAME "dfl-uio-pdev"
> +
> +static struct dfl_driver dfl_uio_pdev_driver;
> +
> +static int check_for_other_drv_match(struct device_driver *drv, void *data)
> +{
> +	struct dfl_driver *ddrv = to_dfl_drv(drv);
> +	struct dfl_device *ddev = data;
> +
> +	/* skip myself */
> +	if (ddrv == &dfl_uio_pdev_driver)
> +		return 0;
> +
> +	return dfl_match_device(ddev, ddrv);
> +}
> +
> +static int dfl_uio_pdev_match(struct dfl_device *ddev)
> +{
> +	/*
> +	 * If any other driver wants the device, leave the device to this other
> +	 * driver.
> +	 */
> +	if (bus_for_each_drv(&dfl_bus_type, NULL, ddev,
> check_for_other_drv_match))
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int dfl_uio_pdev_probe(struct dfl_device *ddev)
> +{
> +	struct device *dev = &ddev->dev;
> +	struct platform_device_info pdevinfo = { 0 };
> +	struct uio_info uio_pdata = { 0 };
> +	struct platform_device *uio_pdev;
> +	struct resource *res;
> +	int i;
> +
> +	pdevinfo.name = "uio_pdrv_genirq";
> +
> +	res = kcalloc(ddev->num_irqs + 1, sizeof(*res), GFP_KERNEL);
> +	if (!res)
> +		return -ENOMEM;
> +
> +	res[0].parent = &ddev->mmio_res;
> +	res[0].flags = IORESOURCE_MEM;
> +	res[0].start = ddev->mmio_res.start;
> +	res[0].end = ddev->mmio_res.end;
> +
> +	/* then add irq resource */
> +	for (i = 0; i < ddev->num_irqs; i++) {
> +		res[i + 1].flags = IORESOURCE_IRQ;
> +		res[i + 1].start = ddev->irqs[i];
> +		res[i + 1].end = ddev->irqs[i];
> +	}

How many interrupts UIO could support? or we need some
warning or just even return error here?

Thanks
Hao

> +
> +	uio_pdata.name = DRIVER_NAME;
> +	uio_pdata.version = "0";
> +
> +	pdevinfo.res = res;
> +	pdevinfo.num_res = ddev->num_irqs + 1;
> +	pdevinfo.parent = &ddev->dev;
> +	pdevinfo.id = PLATFORM_DEVID_AUTO;
> +	pdevinfo.data = &uio_pdata;
> +	pdevinfo.size_data = sizeof(uio_pdata);
> +
> +	uio_pdev = platform_device_register_full(&pdevinfo);
> +	if (!IS_ERR(uio_pdev))
> +		dev_set_drvdata(dev, uio_pdev);
> +
> +	kfree(res);
> +
> +	return PTR_ERR_OR_ZERO(uio_pdev);
> +}
> +
> +static void dfl_uio_pdev_remove(struct dfl_device *ddev)
> +{
> +	struct platform_device *uio_pdev = dev_get_drvdata(&ddev->dev);
> +
> +	platform_device_unregister(uio_pdev);
> +}
> +
> +static struct dfl_driver dfl_uio_pdev_driver = {
> +	.drv	= {
> +		.name       = DRIVER_NAME,
> +	},
> +	.match	= dfl_uio_pdev_match,
> +	.probe	= dfl_uio_pdev_probe,
> +	.remove	= dfl_uio_pdev_remove,
> +};
> +module_dfl_driver(dfl_uio_pdev_driver);
> +
> +MODULE_DESCRIPTION("DFL driver for Userspace I/O platform devices");
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4

Re: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for DFL devices

[Tom Rix]

On 12/18/20 12:05 AM, Wu, Hao wrote:
>> Subject: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for
>> DFL devices
>>
>> This patch supports the DFL drivers be written in userspace. This is
>> realized by exposing the userspace I/O device interfaces.
>>
>> The driver leverages the uio_pdrv_genirq, it adds the uio_pdrv_genirq
>> platform device with the DFL device's resources, and let the generic UIO
>> platform device driver provide support to userspace access to kernel
>> interrupts and memory locations.
>>
>> The driver matches DFL devices in a different way. It has no device id
>> table, instead it matches any DFL device which could not be handled by
>> other DFL drivers.
> Looks like we want to build UIO driver as the default/generic driver for DFL,
> it seems fine but my concern is that UIO has its own limitation, if some day,
> dfl device is extended, but UIO has limitation, then we may need to select 
> another one as the default driver.. or we can just match them using 
> id_table as we know UIO meets the requirement for those DFL devices?

When we have multiple defaults, could this be handled in the configury ?

Tom

>
>> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
>> ---
>> v2: switch to the new matching algorithem. It matches DFL devices which
>>      could not be handled by other DFL drivers.
>>     refacor the code about device resources filling.
>>     fix some comments.
>> v3: split the dfl.c changes out of this patch.
>>     some minor fixes
>> ---
>>  drivers/fpga/Kconfig        |  10 ++++
>>  drivers/fpga/Makefile       |   1 +
>>  drivers/fpga/dfl-uio-pdev.c | 110
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 121 insertions(+)
>>  create mode 100644 drivers/fpga/dfl-uio-pdev.c
>>
>> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
>> index 5d7f0ae..7a88af9 100644
>> --- a/drivers/fpga/Kconfig
>> +++ b/drivers/fpga/Kconfig
>> @@ -202,6 +202,16 @@ config FPGA_DFL_NIOS_INTEL_PAC_N3000
>>  	  the card. It also instantiates the SPI master (spi-altera) for
>>  	  the card's BMC (Board Management Controller).
>>
>> +config FPGA_DFL_UIO_PDEV
>> +	tristate "FPGA DFL Driver for Userspace I/O platform devices"
>> +	depends on FPGA_DFL && UIO_PDRV_GENIRQ
>> +	help
>> +	  Enable this to allow some DFL drivers be written in userspace. It
>> +	  adds the uio_pdrv_genirq platform device with the DFL feature's
>> +	  resources, and lets the generic UIO platform device driver provide
>> +	  support for userspace access to kernel interrupts and memory
>> +	  locations.
> If we consider this as a default driver for everybody in DFL, then we could
> consider build it into the core, otherwise it always requires to be loaded
> manually, right?
>
>> +
>>  config FPGA_DFL_PCI
>>  	tristate "FPGA DFL PCIe Device Driver"
>>  	depends on PCI && FPGA_DFL
>> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
>> index 18dc9885..8847fe0 100644
>> --- a/drivers/fpga/Makefile
>> +++ b/drivers/fpga/Makefile
>> @@ -45,6 +45,7 @@ dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-
>> dma-region.o
>>  dfl-afu-objs += dfl-afu-error.o
>>
>>  obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-nios.o
>> +obj-$(CONFIG_FPGA_DFL_UIO_PDEV)		+= dfl-uio-pdev.o
>>
>>  # Drivers for FPGAs which implement DFL
>>  obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
>> diff --git a/drivers/fpga/dfl-uio-pdev.c b/drivers/fpga/dfl-uio-pdev.c
>> new file mode 100644
>> index 0000000..8c57233
>> --- /dev/null
>> +++ b/drivers/fpga/dfl-uio-pdev.c
>> @@ -0,0 +1,110 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * DFL driver for Userspace I/O platform devices
>> + *
>> + * Copyright (C) 2020 Intel Corporation, Inc.
>> + */
>> +#include <linux/dfl.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/uio_driver.h>
>> +
>> +#include "dfl.h"
>> +
>> +#define DRIVER_NAME "dfl-uio-pdev"
>> +
>> +static struct dfl_driver dfl_uio_pdev_driver;
>> +
>> +static int check_for_other_drv_match(struct device_driver *drv, void *data)
>> +{
>> +	struct dfl_driver *ddrv = to_dfl_drv(drv);
>> +	struct dfl_device *ddev = data;
>> +
>> +	/* skip myself */
>> +	if (ddrv == &dfl_uio_pdev_driver)
>> +		return 0;
>> +
>> +	return dfl_match_device(ddev, ddrv);
>> +}
>> +
>> +static int dfl_uio_pdev_match(struct dfl_device *ddev)
>> +{
>> +	/*
>> +	 * If any other driver wants the device, leave the device to this other
>> +	 * driver.
>> +	 */
>> +	if (bus_for_each_drv(&dfl_bus_type, NULL, ddev,
>> check_for_other_drv_match))
>> +		return 0;
>> +
>> +	return 1;
>> +}
>> +
>> +static int dfl_uio_pdev_probe(struct dfl_device *ddev)
>> +{
>> +	struct device *dev = &ddev->dev;
>> +	struct platform_device_info pdevinfo = { 0 };
>> +	struct uio_info uio_pdata = { 0 };
>> +	struct platform_device *uio_pdev;
>> +	struct resource *res;
>> +	int i;
>> +
>> +	pdevinfo.name = "uio_pdrv_genirq";
>> +
>> +	res = kcalloc(ddev->num_irqs + 1, sizeof(*res), GFP_KERNEL);
>> +	if (!res)
>> +		return -ENOMEM;
>> +
>> +	res[0].parent = &ddev->mmio_res;
>> +	res[0].flags = IORESOURCE_MEM;
>> +	res[0].start = ddev->mmio_res.start;
>> +	res[0].end = ddev->mmio_res.end;
>> +
>> +	/* then add irq resource */
>> +	for (i = 0; i < ddev->num_irqs; i++) {
>> +		res[i + 1].flags = IORESOURCE_IRQ;
>> +		res[i + 1].start = ddev->irqs[i];
>> +		res[i + 1].end = ddev->irqs[i];
>> +	}
> How many interrupts UIO could support? or we need some
> warning or just even return error here?
>
> Thanks
> Hao
>
>> +
>> +	uio_pdata.name = DRIVER_NAME;
>> +	uio_pdata.version = "0";
>> +
>> +	pdevinfo.res = res;
>> +	pdevinfo.num_res = ddev->num_irqs + 1;
>> +	pdevinfo.parent = &ddev->dev;
>> +	pdevinfo.id = PLATFORM_DEVID_AUTO;
>> +	pdevinfo.data = &uio_pdata;
>> +	pdevinfo.size_data = sizeof(uio_pdata);
>> +
>> +	uio_pdev = platform_device_register_full(&pdevinfo);
>> +	if (!IS_ERR(uio_pdev))
>> +		dev_set_drvdata(dev, uio_pdev);
>> +
>> +	kfree(res);
>> +
>> +	return PTR_ERR_OR_ZERO(uio_pdev);
>> +}
>> +
>> +static void dfl_uio_pdev_remove(struct dfl_device *ddev)
>> +{
>> +	struct platform_device *uio_pdev = dev_get_drvdata(&ddev->dev);
>> +
>> +	platform_device_unregister(uio_pdev);
>> +}
>> +
>> +static struct dfl_driver dfl_uio_pdev_driver = {
>> +	.drv	= {
>> +		.name       = DRIVER_NAME,
>> +	},
>> +	.match	= dfl_uio_pdev_match,
>> +	.probe	= dfl_uio_pdev_probe,
>> +	.remove	= dfl_uio_pdev_remove,
>> +};
>> +module_dfl_driver(dfl_uio_pdev_driver);
>> +
>> +MODULE_DESCRIPTION("DFL driver for Userspace I/O platform devices");
>> +MODULE_AUTHOR("Intel Corporation");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.7.4

RE: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for DFL devices

[Wu, Hao]

> Subject: Re: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support
> for DFL devices
> 
> On 12/18/20 12:05 AM, Wu, Hao wrote:
> >> Subject: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support
> for
> >> DFL devices
> >>
> >> This patch supports the DFL drivers be written in userspace. This is
> >> realized by exposing the userspace I/O device interfaces.
> >>
> >> The driver leverages the uio_pdrv_genirq, it adds the uio_pdrv_genirq
> >> platform device with the DFL device's resources, and let the generic UIO
> >> platform device driver provide support to userspace access to kernel
> >> interrupts and memory locations.
> >>
> >> The driver matches DFL devices in a different way. It has no device id
> >> table, instead it matches any DFL device which could not be handled by
> >> other DFL drivers.
> > Looks like we want to build UIO driver as the default/generic driver for DFL,
> > it seems fine but my concern is that UIO has its own limitation, if some day,
> > dfl device is extended, but UIO has limitation, then we may need to select
> > another one as the default driver.. or we can just match them using
> > id_table as we know UIO meets the requirement for those DFL devices?
> 
> When we have multiple defaults, could this be handled in the configury ?

Do you mean select it using configuration options? But if people have already
created the software stack on the old one, then it's a valid requirement that
we need both old and new defaults working at the same time... it's hard to
ask everybody to switch to the new one.

Hao

> 
> Tom
> 

Re: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for DFL devices

[Xu Yilun]

On Fri, Dec 18, 2020 at 05:59:17AM -0800, Tom Rix wrote:
> 
> On 12/18/20 12:05 AM, Wu, Hao wrote:
> >> Subject: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for
> >> DFL devices
> >>
> >> This patch supports the DFL drivers be written in userspace. This is
> >> realized by exposing the userspace I/O device interfaces.
> >>
> >> The driver leverages the uio_pdrv_genirq, it adds the uio_pdrv_genirq
> >> platform device with the DFL device's resources, and let the generic UIO
> >> platform device driver provide support to userspace access to kernel
> >> interrupts and memory locations.
> >>
> >> The driver matches DFL devices in a different way. It has no device id
> >> table, instead it matches any DFL device which could not be handled by
> >> other DFL drivers.
> > Looks like we want to build UIO driver as the default/generic driver for DFL,

I'm not going to make UIO as the default driver for DFL devs, the driver
module will not be autoloaded. I want to provide a choice to operate on
the unhandled devs in userspace. Insmod it if it helps otherwise leave
it.

> > it seems fine but my concern is that UIO has its own limitation, if some day,
> > dfl device is extended, but UIO has limitation, then we may need to select 
> > another one as the default driver.. or we can just match them using 

I think we may not have to select a "default" driver, if we have a
better way for userspace accessing, we could load that module, leave
UIO.

> > id_table as we know UIO meets the requirement for those DFL devices?

As we discussed, the drawback is that we should always change the code to
support a new dfl device for user accessing. But it is OK to me if the
generic UIO match rule is not considered proper.

> 
> When we have multiple defaults, could this be handled in the configury ?

I think we don't have to select a "default".

> 
> Tom
> 
> >
> >> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> >> ---
> >> v2: switch to the new matching algorithem. It matches DFL devices which
> >>      could not be handled by other DFL drivers.
> >>     refacor the code about device resources filling.
> >>     fix some comments.
> >> v3: split the dfl.c changes out of this patch.
> >>     some minor fixes
> >> ---
> >>  drivers/fpga/Kconfig        |  10 ++++
> >>  drivers/fpga/Makefile       |   1 +
> >>  drivers/fpga/dfl-uio-pdev.c | 110
> >> ++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 121 insertions(+)
> >>  create mode 100644 drivers/fpga/dfl-uio-pdev.c
> >>
> >> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> >> index 5d7f0ae..7a88af9 100644
> >> --- a/drivers/fpga/Kconfig
> >> +++ b/drivers/fpga/Kconfig
> >> @@ -202,6 +202,16 @@ config FPGA_DFL_NIOS_INTEL_PAC_N3000
> >>  	  the card. It also instantiates the SPI master (spi-altera) for
> >>  	  the card's BMC (Board Management Controller).
> >>
> >> +config FPGA_DFL_UIO_PDEV
> >> +	tristate "FPGA DFL Driver for Userspace I/O platform devices"
> >> +	depends on FPGA_DFL && UIO_PDRV_GENIRQ
> >> +	help
> >> +	  Enable this to allow some DFL drivers be written in userspace. It
> >> +	  adds the uio_pdrv_genirq platform device with the DFL feature's
> >> +	  resources, and lets the generic UIO platform device driver provide
> >> +	  support for userspace access to kernel interrupts and memory
> >> +	  locations.
> > If we consider this as a default driver for everybody in DFL, then we could
> > consider build it into the core, otherwise it always requires to be loaded
> > manually, right?

It should be loaded manually. I don't want to make this as default.

> >
> >> +
> >>  config FPGA_DFL_PCI
> >>  	tristate "FPGA DFL PCIe Device Driver"
> >>  	depends on PCI && FPGA_DFL
> >> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> >> index 18dc9885..8847fe0 100644
> >> --- a/drivers/fpga/Makefile
> >> +++ b/drivers/fpga/Makefile
> >> @@ -45,6 +45,7 @@ dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-
> >> dma-region.o
> >>  dfl-afu-objs += dfl-afu-error.o
> >>
> >>  obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-nios.o
> >> +obj-$(CONFIG_FPGA_DFL_UIO_PDEV)		+= dfl-uio-pdev.o
> >>
> >>  # Drivers for FPGAs which implement DFL
> >>  obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
> >> diff --git a/drivers/fpga/dfl-uio-pdev.c b/drivers/fpga/dfl-uio-pdev.c
> >> new file mode 100644
> >> index 0000000..8c57233
> >> --- /dev/null
> >> +++ b/drivers/fpga/dfl-uio-pdev.c
> >> @@ -0,0 +1,110 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * DFL driver for Userspace I/O platform devices
> >> + *
> >> + * Copyright (C) 2020 Intel Corporation, Inc.
> >> + */
> >> +#include <linux/dfl.h>
> >> +#include <linux/errno.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/uio_driver.h>
> >> +
> >> +#include "dfl.h"
> >> +
> >> +#define DRIVER_NAME "dfl-uio-pdev"
> >> +
> >> +static struct dfl_driver dfl_uio_pdev_driver;
> >> +
> >> +static int check_for_other_drv_match(struct device_driver *drv, void *data)
> >> +{
> >> +	struct dfl_driver *ddrv = to_dfl_drv(drv);
> >> +	struct dfl_device *ddev = data;
> >> +
> >> +	/* skip myself */
> >> +	if (ddrv == &dfl_uio_pdev_driver)
> >> +		return 0;
> >> +
> >> +	return dfl_match_device(ddev, ddrv);
> >> +}
> >> +
> >> +static int dfl_uio_pdev_match(struct dfl_device *ddev)
> >> +{
> >> +	/*
> >> +	 * If any other driver wants the device, leave the device to this other
> >> +	 * driver.
> >> +	 */
> >> +	if (bus_for_each_drv(&dfl_bus_type, NULL, ddev,
> >> check_for_other_drv_match))
> >> +		return 0;
> >> +
> >> +	return 1;
> >> +}
> >> +
> >> +static int dfl_uio_pdev_probe(struct dfl_device *ddev)
> >> +{
> >> +	struct device *dev = &ddev->dev;
> >> +	struct platform_device_info pdevinfo = { 0 };
> >> +	struct uio_info uio_pdata = { 0 };
> >> +	struct platform_device *uio_pdev;
> >> +	struct resource *res;
> >> +	int i;
> >> +
> >> +	pdevinfo.name = "uio_pdrv_genirq";
> >> +
> >> +	res = kcalloc(ddev->num_irqs + 1, sizeof(*res), GFP_KERNEL);
> >> +	if (!res)
> >> +		return -ENOMEM;
> >> +
> >> +	res[0].parent = &ddev->mmio_res;
> >> +	res[0].flags = IORESOURCE_MEM;
> >> +	res[0].start = ddev->mmio_res.start;
> >> +	res[0].end = ddev->mmio_res.end;
> >> +
> >> +	/* then add irq resource */
> >> +	for (i = 0; i < ddev->num_irqs; i++) {
> >> +		res[i + 1].flags = IORESOURCE_IRQ;
> >> +		res[i + 1].start = ddev->irqs[i];
> >> +		res[i + 1].end = ddev->irqs[i];
> >> +	}
> > How many interrupts UIO could support? or we need some
> > warning or just even return error here?

Yes, I can add some warning here.

Thanks,
Yilun

RE: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support for DFL devices

[Wu, Hao]

> Subject: Re: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support
> for DFL devices
> 
> On Fri, Dec 18, 2020 at 05:59:17AM -0800, Tom Rix wrote:
> >
> > On 12/18/20 12:05 AM, Wu, Hao wrote:
> > >> Subject: [PATCH v3 2/3] fpga: dfl: add the userspace I/O device support
> for
> > >> DFL devices
> > >>
> > >> This patch supports the DFL drivers be written in userspace. This is
> > >> realized by exposing the userspace I/O device interfaces.
> > >>
> > >> The driver leverages the uio_pdrv_genirq, it adds the uio_pdrv_genirq
> > >> platform device with the DFL device's resources, and let the generic UIO
> > >> platform device driver provide support to userspace access to kernel
> > >> interrupts and memory locations.
> > >>
> > >> The driver matches DFL devices in a different way. It has no device id
> > >> table, instead it matches any DFL device which could not be handled by
> > >> other DFL drivers.
> > > Looks like we want to build UIO driver as the default/generic driver for
> DFL,
> 
> I'm not going to make UIO as the default driver for DFL devs, the driver
> module will not be autoloaded. I want to provide a choice to operate on
> the unhandled devs in userspace. Insmod it if it helps otherwise leave
> it.

But once it's loaded, then it will bind with all dfl devices which doesn't have
a matched driver yet. 

> 
> > > it seems fine but my concern is that UIO has its own limitation, if some
> day,
> > > dfl device is extended, but UIO has limitation, then we may need to select
> > > another one as the default driver.. or we can just match them using
> 
> I think we may not have to select a "default" driver, if we have a
> better way for userspace accessing, we could load that module, leave
> UIO.

It would not be an easy decision, it may impact users who have already
built upper layer software on top of UIO, and ask new users to use new
one too. Users may not be happy with this.

> 
> > > id_table as we know UIO meets the requirement for those DFL devices?
> 
> As we discussed, the drawback is that we should always change the code to
> support a new dfl device for user accessing. But it is OK to me if the
> generic UIO match rule is not considered proper.
> 
> >
> > When we have multiple defaults, could this be handled in the configury ?
> 
> I think we don't have to select a "default".

Actually I am fine with the idea of using a default / generic driver for DFL
device, and my only concern is UIO may not be good enough for us per my
understanding. Maybe we can have a more extendable one as the generic
driver, but if we don't have that yet, then using id table may be able to
avoid some problems I think.

Hao

> 
> >
> > Tom
> >
> > >
> > >> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > >> ---
> > >> v2: switch to the new matching algorithem. It matches DFL devices which
> > >>      could not be handled by other DFL drivers.
> > >>     refacor the code about device resources filling.
> > >>     fix some comments.
> > >> v3: split the dfl.c changes out of this patch.
> > >>     some minor fixes
> > >> ---
> > >>  drivers/fpga/Kconfig        |  10 ++++
> > >>  drivers/fpga/Makefile       |   1 +
> > >>  drivers/fpga/dfl-uio-pdev.c | 110
> > >> ++++++++++++++++++++++++++++++++++++++++++++
> > >>  3 files changed, 121 insertions(+)
> > >>  create mode 100644 drivers/fpga/dfl-uio-pdev.c
> > >>
> > >> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > >> index 5d7f0ae..7a88af9 100644
> > >> --- a/drivers/fpga/Kconfig
> > >> +++ b/drivers/fpga/Kconfig
> > >> @@ -202,6 +202,16 @@ config FPGA_DFL_NIOS_INTEL_PAC_N3000
> > >>  	  the card. It also instantiates the SPI master (spi-altera) for
> > >>  	  the card's BMC (Board Management Controller).
> > >>
> > >> +config FPGA_DFL_UIO_PDEV
> > >> +	tristate "FPGA DFL Driver for Userspace I/O platform devices"
> > >> +	depends on FPGA_DFL && UIO_PDRV_GENIRQ
> > >> +	help
> > >> +	  Enable this to allow some DFL drivers be written in userspace. It
> > >> +	  adds the uio_pdrv_genirq platform device with the DFL feature's
> > >> +	  resources, and lets the generic UIO platform device driver provide
> > >> +	  support for userspace access to kernel interrupts and memory
> > >> +	  locations.
> > > If we consider this as a default driver for everybody in DFL, then we could
> > > consider build it into the core, otherwise it always requires to be loaded
> > > manually, right?
> 
> It should be loaded manually. I don't want to make this as default.
> 
> > >
> > >> +
> > >>  config FPGA_DFL_PCI
> > >>  	tristate "FPGA DFL PCIe Device Driver"
> > >>  	depends on PCI && FPGA_DFL
> > >> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > >> index 18dc9885..8847fe0 100644
> > >> --- a/drivers/fpga/Makefile
> > >> +++ b/drivers/fpga/Makefile
> > >> @@ -45,6 +45,7 @@ dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-
> afu-
> > >> dma-region.o
> > >>  dfl-afu-objs += dfl-afu-error.o
> > >>
> > >>  obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-
> nios.o
> > >> +obj-$(CONFIG_FPGA_DFL_UIO_PDEV)		+= dfl-uio-pdev.o
> > >>
> > >>  # Drivers for FPGAs which implement DFL
> > >>  obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
> > >> diff --git a/drivers/fpga/dfl-uio-pdev.c b/drivers/fpga/dfl-uio-pdev.c
> > >> new file mode 100644
> > >> index 0000000..8c57233
> > >> --- /dev/null
> > >> +++ b/drivers/fpga/dfl-uio-pdev.c
> > >> @@ -0,0 +1,110 @@
> > >> +// SPDX-License-Identifier: GPL-2.0
> > >> +/*
> > >> + * DFL driver for Userspace I/O platform devices
> > >> + *
> > >> + * Copyright (C) 2020 Intel Corporation, Inc.
> > >> + */
> > >> +#include <linux/dfl.h>
> > >> +#include <linux/errno.h>
> > >> +#include <linux/kernel.h>
> > >> +#include <linux/module.h>
> > >> +#include <linux/platform_device.h>
> > >> +#include <linux/slab.h>
> > >> +#include <linux/uio_driver.h>
> > >> +
> > >> +#include "dfl.h"
> > >> +
> > >> +#define DRIVER_NAME "dfl-uio-pdev"
> > >> +
> > >> +static struct dfl_driver dfl_uio_pdev_driver;
> > >> +
> > >> +static int check_for_other_drv_match(struct device_driver *drv, void
> *data)
> > >> +{
> > >> +	struct dfl_driver *ddrv = to_dfl_drv(drv);
> > >> +	struct dfl_device *ddev = data;
> > >> +
> > >> +	/* skip myself */
> > >> +	if (ddrv == &dfl_uio_pdev_driver)
> > >> +		return 0;
> > >> +
> > >> +	return dfl_match_device(ddev, ddrv);
> > >> +}
> > >> +
> > >> +static int dfl_uio_pdev_match(struct dfl_device *ddev)
> > >> +{
> > >> +	/*
> > >> +	 * If any other driver wants the device, leave the device to this other
> > >> +	 * driver.
> > >> +	 */
> > >> +	if (bus_for_each_drv(&dfl_bus_type, NULL, ddev,
> > >> check_for_other_drv_match))
> > >> +		return 0;
> > >> +
> > >> +	return 1;
> > >> +}
> > >> +
> > >> +static int dfl_uio_pdev_probe(struct dfl_device *ddev)
> > >> +{
> > >> +	struct device *dev = &ddev->dev;
> > >> +	struct platform_device_info pdevinfo = { 0 };
> > >> +	struct uio_info uio_pdata = { 0 };
> > >> +	struct platform_device *uio_pdev;
> > >> +	struct resource *res;
> > >> +	int i;
> > >> +
> > >> +	pdevinfo.name = "uio_pdrv_genirq";
> > >> +
> > >> +	res = kcalloc(ddev->num_irqs + 1, sizeof(*res), GFP_KERNEL);
> > >> +	if (!res)
> > >> +		return -ENOMEM;
> > >> +
> > >> +	res[0].parent = &ddev->mmio_res;
> > >> +	res[0].flags = IORESOURCE_MEM;
> > >> +	res[0].start = ddev->mmio_res.start;
> > >> +	res[0].end = ddev->mmio_res.end;
> > >> +
> > >> +	/* then add irq resource */
> > >> +	for (i = 0; i < ddev->num_irqs; i++) {
> > >> +		res[i + 1].flags = IORESOURCE_IRQ;
> > >> +		res[i + 1].start = ddev->irqs[i];
> > >> +		res[i + 1].end = ddev->irqs[i];
> > >> +	}
> > > How many interrupts UIO could support? or we need some
> > > warning or just even return error here?
> 
> Yes, I can add some warning here.
> 
> Thanks,
> Yilun

[PATCH v3 3/3] Documentation: fpga: dfl: Add description for DFL UIO support

[Xu Yilun]

This patch adds description for UIO support for dfl devices on DFL
bus.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
---
v2: no doc in v1, add it for v2.
v3: some documentation fixes.
---
 Documentation/fpga/dfl.rst | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index 0404fe6..b298ad9 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -7,6 +7,7 @@ Authors:
 - Enno Luebbers <enno.luebbers@intel.com>
 - Xiao Guangrong <guangrong.xiao@linux.intel.com>
 - Wu Hao <hao.wu@intel.com>
+- Xu Yilun <yilun.xu@intel.com>
 
 The Device Feature List (DFL) FPGA framework (and drivers according to
 this framework) hides the very details of low layer hardwares and provides
@@ -502,6 +503,32 @@ FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c)
 could be a reference.
 
 
+UIO support for DFL devices
+===========================
+The purpose of an FPGA is to be reprogrammed with newly developed hardware
+components. New hardware can instantiate a new private feature in the DFL, and
+then get a DFL device in their system. In some cases users may need a userspace
+driver for the DFL device:
+
+* Users may need to run some diagnostic test for their hardwares.
+* Users may prototype the kernel driver in user space.
+* Some hardware is designed for specific purposes and does not fit into one of
+  the standard kernel subsystems.
+
+This requires the direct access to the MMIO space and interrupt handling in
+userspace. The dfl-uio-pdev module exposes the UIO device interfaces for this
+purpose. It adds the uio_pdrv_genirq platform device with the resources of
+the DFL feature, and lets the generic UIO platform device driver provide UIO
+support to userspace.
+
+FPGA_DFL_UIO_PDEV should be selected to enable this feature.
+
+The DFL UIO driver has a special matching algorithem. It will match any DFL
+device which could not be handled by other DFL drivers. In this way, it will
+not impact the functionality of the features which are already supported by the
+system.
+
+
 Open discussion
 ===============
 FME driver exports one ioctl (DFL_FPGA_FME_PORT_PR) for partial reconfiguration
-- 
2.7.4

Re: [PATCH v3 3/3] Documentation: fpga: dfl: Add description for DFL UIO support

[Tom Rix]

On 12/16/20 9:44 PM, Xu Yilun wrote:
> This patch adds description for UIO support for dfl devices on DFL
> bus.

Thanks for the changes.

Reviewed-by: Tom Rix <trix@redhat.com>

> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> ---
> v2: no doc in v1, add it for v2.
> v3: some documentation fixes.
> ---
>  Documentation/fpga/dfl.rst | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
> index 0404fe6..b298ad9 100644
> --- a/Documentation/fpga/dfl.rst
> +++ b/Documentation/fpga/dfl.rst
> @@ -7,6 +7,7 @@ Authors:
>  - Enno Luebbers <enno.luebbers@intel.com>
>  - Xiao Guangrong <guangrong.xiao@linux.intel.com>
>  - Wu Hao <hao.wu@intel.com>
> +- Xu Yilun <yilun.xu@intel.com>
>  
>  The Device Feature List (DFL) FPGA framework (and drivers according to
>  this framework) hides the very details of low layer hardwares and provides
> @@ -502,6 +503,32 @@ FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c)
>  could be a reference.
>  
>  
> +UIO support for DFL devices
> +===========================
> +The purpose of an FPGA is to be reprogrammed with newly developed hardware
> +components. New hardware can instantiate a new private feature in the DFL, and
> +then get a DFL device in their system. In some cases users may need a userspace
> +driver for the DFL device:
> +
> +* Users may need to run some diagnostic test for their hardwares.
> +* Users may prototype the kernel driver in user space.
> +* Some hardware is designed for specific purposes and does not fit into one of
> +  the standard kernel subsystems.
> +
> +This requires the direct access to the MMIO space and interrupt handling in
> +userspace. The dfl-uio-pdev module exposes the UIO device interfaces for this
> +purpose. It adds the uio_pdrv_genirq platform device with the resources of
> +the DFL feature, and lets the generic UIO platform device driver provide UIO
> +support to userspace.
> +
> +FPGA_DFL_UIO_PDEV should be selected to enable this feature.
> +
> +The DFL UIO driver has a special matching algorithem. It will match any DFL
> +device which could not be handled by other DFL drivers. In this way, it will
> +not impact the functionality of the features which are already supported by the
> +system.
> +
> +
>  Open discussion
>  ===============
>  FME driver exports one ioctl (DFL_FPGA_FME_PORT_PR) for partial reconfiguration

Re: [PATCH v3 0/3] UIO support for dfl devices

[Tom Rix]

On 12/16/20 9:44 PM, Xu Yilun wrote:
> This patchset supports some dfl device drivers written in userspace.
>
> In the patchset v1, the "driver_override" interface should be used to bind
> the DFL UIO driver to DFL devices. But there is concern that the
> "driver_override" interface is not OK itself. So in v2, we use a new
> matching algorithem. The "driver_override" interface is abandoned, the DFL
> UIO driver matches any DFL device which could not be handled by other DFL
> drivers. So the DFL UIO driver could be used for new DFL devices which are
> not supported by kernel, also it will not impact the devices which are
> already got supported.

This set looks good to me.

Moritz,

Is it possible to get this into 5.11 ?

Tom

>
> Main changes from v1:
> - switch to the new matching algorithem. It matches DFL devices which could
>   not be handled by other DFL drivers.
> - refacor the code about device resources filling.
> - add the documentation.
>
> Main changes from v2:
> - split the match ops changes in dfl.c to an independent patch.
> - move the declarations needed for dfl-uio-pdev from include/linux/dfl.h
>   to driver/fpga/dfl.h
> - some minor fixes.
>
>
> Xu Yilun (3):
>   fpga: dfl: add the match() ops for dfl driver
>   fpga: dfl: add the userspace I/O device support for DFL devices
>   Documentation: fpga: dfl: Add description for DFL UIO support
>
>  Documentation/fpga/dfl.rst  |  27 +++++++++++
>  drivers/fpga/Kconfig        |  10 ++++
>  drivers/fpga/Makefile       |   1 +
>  drivers/fpga/dfl-uio-pdev.c | 110 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/fpga/dfl.c          |  22 +++++++--
>  drivers/fpga/dfl.h          |   5 ++
>  include/linux/dfl.h         |   3 ++
>  7 files changed, 173 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/fpga/dfl-uio-pdev.c
>

Re: [PATCH v3 0/3] UIO support for dfl devices

[Greg KH]

On Thu, Dec 17, 2020 at 05:35:51AM -0800, Tom Rix wrote:
> 
> On 12/16/20 9:44 PM, Xu Yilun wrote:
> > This patchset supports some dfl device drivers written in userspace.
> >
> > In the patchset v1, the "driver_override" interface should be used to bind
> > the DFL UIO driver to DFL devices. But there is concern that the
> > "driver_override" interface is not OK itself. So in v2, we use a new
> > matching algorithem. The "driver_override" interface is abandoned, the DFL
> > UIO driver matches any DFL device which could not be handled by other DFL
> > drivers. So the DFL UIO driver could be used for new DFL devices which are
> > not supported by kernel, also it will not impact the devices which are
> > already got supported.
> 
> This set looks good to me.
> 
> Moritz,
> 
> Is it possible to get this into 5.11 ?

How would that be possible?

New features have to be in linux-next successfully, _BEFORE_ the merge
window opens.  Please read Documentation/process/ for how we do all of
this...

thanks,

greg k-h