linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] UIO support for dfl devices
@ 2020-12-14  3:36 Xu Yilun
  2020-12-14  3:36 ` [PATCH v2 1/2] fpga: dfl: add the userspace I/O device support for DFL devices Xu Yilun
  2020-12-14  3:36 ` [PATCH v2 2/2] Documentation: fpga: dfl: Add description for DFL UIO support Xu Yilun
  0 siblings, 2 replies; 9+ messages in thread
From: Xu Yilun @ 2020-12-14  3:36 UTC (permalink / raw)
  To: mdf, linux-fpga, linux-kernel; +Cc: gregkh, trix, lgoncalv, yilun.xu, hao.wu

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.


Xu Yilun (2):
  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  |  23 ++++++++++
 drivers/fpga/Kconfig        |  10 ++++
 drivers/fpga/Makefile       |   1 +
 drivers/fpga/dfl-uio-pdev.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl.c          |  22 +++++++--
 include/linux/dfl.h         |   7 +++
 6 files changed, 166 insertions(+), 5 deletions(-)
 create mode 100644 drivers/fpga/dfl-uio-pdev.c

-- 
2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/2] fpga: dfl: add the userspace I/O device support for DFL devices
  2020-12-14  3:36 [PATCH v2 0/2] UIO support for dfl devices Xu Yilun
@ 2020-12-14  3:36 ` Xu Yilun
  2020-12-14 22:03   ` Tom Rix
  2020-12-14  3:36 ` [PATCH v2 2/2] Documentation: fpga: dfl: Add description for DFL UIO support Xu Yilun
  1 sibling, 1 reply; 9+ messages in thread
From: Xu Yilun @ 2020-12-14  3:36 UTC (permalink / raw)
  To: mdf, linux-fpga, linux-kernel; +Cc: gregkh, trix, lgoncalv, yilun.xu, hao.wu

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. For this purpose, a match() ops is added to the dfl
driver, to allow dfl drivers have their own matching algorithem instead
of the standard id_table matching.

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.
---
 drivers/fpga/Kconfig        |  10 ++++
 drivers/fpga/Makefile       |   1 +
 drivers/fpga/dfl-uio-pdev.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl.c          |  22 +++++++--
 include/linux/dfl.h         |   7 +++
 5 files changed, 143 insertions(+), 5 deletions(-)
 create mode 100644 drivers/fpga/dfl-uio-pdev.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 5d7f0ae..eb8a616 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 device's
+	  resources, and lets the generic UIO platform device driver provide
+	  support to 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..e07b3d5 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..68ede04
--- /dev/null
+++ b/drivers/fpga/dfl-uio-pdev.c
@@ -0,0 +1,108 @@
+// 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>
+
+#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");
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/include/linux/dfl.h b/include/linux/dfl.h
index 6cc1098..7bd97ec 100644
--- a/include/linux/dfl.h
+++ b/include/linux/dfl.h
@@ -11,6 +11,8 @@
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
 
+extern struct bus_type dfl_bus_type;
+
 /**
  * enum dfl_id_type - define the DFL FIU types
  */
@@ -51,6 +53,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 a positive value 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 +62,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);
 };
@@ -65,6 +70,8 @@ struct dfl_driver {
 #define to_dfl_dev(d) container_of(d, struct dfl_device, dev)
 #define to_dfl_drv(d) container_of(d, struct dfl_driver, drv)
 
+int dfl_match_device(struct dfl_device *ddev, struct dfl_driver *ddrv);
+
 /*
  * use a macro to avoid include chaining to get THIS_MODULE.
  */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] Documentation: fpga: dfl: Add description for DFL UIO support
  2020-12-14  3:36 [PATCH v2 0/2] UIO support for dfl devices Xu Yilun
  2020-12-14  3:36 ` [PATCH v2 1/2] fpga: dfl: add the userspace I/O device support for DFL devices Xu Yilun
@ 2020-12-14  3:36 ` Xu Yilun
  2020-12-14 22:14   ` Tom Rix
  1 sibling, 1 reply; 9+ messages in thread
From: Xu Yilun @ 2020-12-14  3:36 UTC (permalink / raw)
  To: mdf, linux-fpga, linux-kernel; +Cc: gregkh, trix, lgoncalv, yilun.xu, hao.wu

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.
---
 Documentation/fpga/dfl.rst | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index 0404fe6..a15e81e 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -502,6 +502,29 @@ FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c)
 could be a reference.
 
 
+UIO support for DFL devices
+===========================
+The FPGA is open for users to be reprogramed with newly developed hardware
+components. They could 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.
+* 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. We implemented a dfl-uio-pdev module which exposes the UIO device
+interfaces. It adds the uio_pdrv_genirq platform device with the resources of
+the DFL device, and let the generic UIO platform device driver provide UIO
+support to userspace.
+
+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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] fpga: dfl: add the userspace I/O device support for DFL devices
  2020-12-14  3:36 ` [PATCH v2 1/2] fpga: dfl: add the userspace I/O device support for DFL devices Xu Yilun
@ 2020-12-14 22:03   ` Tom Rix
  2020-12-15  2:06     ` Xu Yilun
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Rix @ 2020-12-14 22:03 UTC (permalink / raw)
  To: Xu Yilun, mdf, linux-fpga, linux-kernel; +Cc: gregkh, lgoncalv, hao.wu



On 12/13/20 7:36 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. For this purpose, a match() ops is added to the dfl
> driver, to allow dfl drivers have their own matching algorithem instead
> of the standard id_table matching.

Because (*match) is added, this change is doing two things.

In the next version, please split the new interface out, i believe these are

the dfl.c/h changes.

>
> 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.
> ---
>  drivers/fpga/Kconfig        |  10 ++++
>  drivers/fpga/Makefile       |   1 +
>  drivers/fpga/dfl-uio-pdev.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/fpga/dfl.c          |  22 +++++++--
>  include/linux/dfl.h         |   7 +++
>  5 files changed, 143 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/fpga/dfl-uio-pdev.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 5d7f0ae..eb8a616 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 device's
DFL feature's
> +	  resources, and lets the generic UIO platform device driver provide
> +	  support to userspace access to kernel interrupts and memory
support for userspace
> +	  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..e07b3d5 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
This does not really align with += dfl-pci.o, add another tab.
>  
>  # 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..68ede04
> --- /dev/null
> +++ b/drivers/fpga/dfl-uio-pdev.c
> @@ -0,0 +1,108 @@
> +// 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>
> +
> +#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,
> +};
Could move this to the top and remove the forward decl.
> +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");
> 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/include/linux/dfl.h b/include/linux/dfl.h
> index 6cc1098..7bd97ec 100644
> --- a/include/linux/dfl.h
> +++ b/include/linux/dfl.h
> @@ -11,6 +11,8 @@
>  #include <linux/device.h>
>  #include <linux/mod_devicetable.h>
>  
> +extern struct bus_type dfl_bus_type;

Why is this needed ?

This is a public interface, likely should happen somewhere in drivers/fpga

> +
>  /**
>   * enum dfl_id_type - define the DFL FIU types
>   */
> @@ -51,6 +53,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 a positive value if given device can be handled by the
returns one
> + *	   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 +62,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);
>  };
> @@ -65,6 +70,8 @@ struct dfl_driver {
>  #define to_dfl_dev(d) container_of(d, struct dfl_device, dev)
>  #define to_dfl_drv(d) container_of(d, struct dfl_driver, drv)
>  
> +int dfl_match_device(struct dfl_device *ddev, struct dfl_driver *ddrv);

Is it really necessary put this as a public interface ?

Tom

> +
>  /*
>   * use a macro to avoid include chaining to get THIS_MODULE.
>   */


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] Documentation: fpga: dfl: Add description for DFL UIO support
  2020-12-14  3:36 ` [PATCH v2 2/2] Documentation: fpga: dfl: Add description for DFL UIO support Xu Yilun
@ 2020-12-14 22:14   ` Tom Rix
  2020-12-15  2:22     ` Xu Yilun
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Rix @ 2020-12-14 22:14 UTC (permalink / raw)
  To: Xu Yilun, mdf, linux-fpga, linux-kernel; +Cc: gregkh, lgoncalv, hao.wu


On 12/13/20 7:36 PM, Xu Yilun wrote:
> 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.
> ---
>  Documentation/fpga/dfl.rst | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
Should add your self in the authors list.
>
> diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
> index 0404fe6..a15e81e 100644
> --- a/Documentation/fpga/dfl.rst
> +++ b/Documentation/fpga/dfl.rst
> @@ -502,6 +502,29 @@ FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c)
>  could be a reference.
>  
>  
> +UIO support for DFL devices
> +===========================
> +The FPGA is open for users to be reprogramed with newly developed hardware

The purpose of an FPGA is to be reprogrammed with newly developed hardware

components.  New hardware can

> +components. They could 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. We implemented a dfl-uio-pdev module which exposes the UIO device
The dfl-uio-pdev module exposes
> +interfaces. It adds the uio_pdrv_genirq platform device with the resources of
> +the DFL device, and let the generic UIO platform device driver provide UIO
the DLF device, and lets
> +support to userspace.
Use FPGA_DFL_UIO_PDEV 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.
(not sure if this section is needed)

> +
> +
>  Open discussion
>  ===============
>  FME driver exports one ioctl (DFL_FPGA_FME_PORT_PR) for partial reconfiguration


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] fpga: dfl: add the userspace I/O device support for DFL devices
  2020-12-14 22:03   ` Tom Rix
@ 2020-12-15  2:06     ` Xu Yilun
  0 siblings, 0 replies; 9+ messages in thread
From: Xu Yilun @ 2020-12-15  2:06 UTC (permalink / raw)
  To: Tom Rix; +Cc: mdf, linux-fpga, linux-kernel, gregkh, lgoncalv, hao.wu

On Mon, Dec 14, 2020 at 02:03:03PM -0800, Tom Rix wrote:
> 
> 
> On 12/13/20 7:36 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. For this purpose, a match() ops is added to the dfl
> > driver, to allow dfl drivers have their own matching algorithem instead
> > of the standard id_table matching.
> 
> Because (*match) is added, this change is doing two things.
> 
> In the next version, please split the new interface out, i believe these are
> 
> the dfl.c/h changes.

Yes, I'll split it out.

> 
> >
> > 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.
> > ---
> >  drivers/fpga/Kconfig        |  10 ++++
> >  drivers/fpga/Makefile       |   1 +
> >  drivers/fpga/dfl-uio-pdev.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/fpga/dfl.c          |  22 +++++++--
> >  include/linux/dfl.h         |   7 +++
> >  5 files changed, 143 insertions(+), 5 deletions(-)
> >  create mode 100644 drivers/fpga/dfl-uio-pdev.c
> >
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index 5d7f0ae..eb8a616 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 device's
> DFL feature's

Will change it.

> > +	  resources, and lets the generic UIO platform device driver provide
> > +	  support to userspace access to kernel interrupts and memory
> support for userspace

Will change it.

> > +	  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..e07b3d5 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
> This does not really align with += dfl-pci.o, add another tab.

Will change it.

> >  
> >  # 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..68ede04
> > --- /dev/null
> > +++ b/drivers/fpga/dfl-uio-pdev.c
> > @@ -0,0 +1,108 @@
> > +// 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>
> > +
> > +#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,
> > +};
> Could move this to the top and remove the forward decl.

I think it may be unnecessary. If we move it to the top, then we should
forward declare the 3 callbacks.

> > +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");
> > 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/include/linux/dfl.h b/include/linux/dfl.h
> > index 6cc1098..7bd97ec 100644
> > --- a/include/linux/dfl.h
> > +++ b/include/linux/dfl.h
> > @@ -11,6 +11,8 @@
> >  #include <linux/device.h>
> >  #include <linux/mod_devicetable.h>
> >  
> > +extern struct bus_type dfl_bus_type;
> 
> Why is this needed ?
> 
> This is a public interface, likely should happen somewhere in drivers/fpga

The dfl-uio-pdev module is using this variable. Do you mean we should
move it to DFL internal header file, drivers/fpga/dfl.h ?

> 
> > +
> >  /**
> >   * enum dfl_id_type - define the DFL FIU types
> >   */
> > @@ -51,6 +53,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 a positive value if given device can be handled by the
> returns one

Will change it.

> > + *	   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 +62,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);
> >  };
> > @@ -65,6 +70,8 @@ struct dfl_driver {
> >  #define to_dfl_dev(d) container_of(d, struct dfl_device, dev)
> >  #define to_dfl_drv(d) container_of(d, struct dfl_driver, drv)
> >  
> > +int dfl_match_device(struct dfl_device *ddev, struct dfl_driver *ddrv);
> 
> Is it really necessary put this as a public interface ?

I may move it to drivers/fpga/dfl.h

Thanks,
Yilun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] Documentation: fpga: dfl: Add description for DFL UIO support
  2020-12-14 22:14   ` Tom Rix
@ 2020-12-15  2:22     ` Xu Yilun
  2020-12-15  4:43       ` Tom Rix
  0 siblings, 1 reply; 9+ messages in thread
From: Xu Yilun @ 2020-12-15  2:22 UTC (permalink / raw)
  To: Tom Rix; +Cc: mdf, linux-fpga, linux-kernel, gregkh, lgoncalv, hao.wu

On Mon, Dec 14, 2020 at 02:14:56PM -0800, Tom Rix wrote:
> 
> On 12/13/20 7:36 PM, Xu Yilun wrote:
> > 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.
> > ---
> >  Documentation/fpga/dfl.rst | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> Should add your self in the authors list.

OK.

> >
> > diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
> > index 0404fe6..a15e81e 100644
> > --- a/Documentation/fpga/dfl.rst
> > +++ b/Documentation/fpga/dfl.rst
> > @@ -502,6 +502,29 @@ FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c)
> >  could be a reference.
> >  
> >  
> > +UIO support for DFL devices
> > +===========================
> > +The FPGA is open for users to be reprogramed with newly developed hardware
> 
> The purpose of an FPGA is to be reprogrammed with newly developed hardware
> 
> components.  New hardware can

Will change it.

> 
> > +components. They could 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.

Could we just add the line rather than replacing the previous line? I think this
comment is describing a different usecase.

> > +* 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. We implemented a dfl-uio-pdev module which exposes the UIO device
> The dfl-uio-pdev module exposes

Will change it.

> > +interfaces. It adds the uio_pdrv_genirq platform device with the resources of
> > +the DFL device, and let the generic UIO platform device driver provide UIO
> the DLF device, and lets

Will change it.

> > +support to userspace.
> Use FPGA_DFL_UIO_PDEV to enable this feature.

I didn't get your idea for this.

> > +
> > +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.
> (not sure if this section is needed)

I think we may keep it.

Thanks,
Yilun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] Documentation: fpga: dfl: Add description for DFL UIO support
  2020-12-15  2:22     ` Xu Yilun
@ 2020-12-15  4:43       ` Tom Rix
  2020-12-15  5:15         ` Xu Yilun
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Rix @ 2020-12-15  4:43 UTC (permalink / raw)
  To: Xu Yilun; +Cc: mdf, linux-fpga, linux-kernel, gregkh, lgoncalv, hao.wu


On 12/14/20 6:22 PM, Xu Yilun wrote:
> On Mon, Dec 14, 2020 at 02:14:56PM -0800, Tom Rix wrote:
>> On 12/13/20 7:36 PM, Xu Yilun wrote:
>>> 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.
>>> ---
>>
>>> +components. They could 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.
> Could we just add the line rather than replacing the previous line? I think this
> comment is describing a different usecase.

Yes, this is what i ment, please use your original.

I am offering another usecase, one I will use.

Add mine as well, if you want.

>
>>> +* 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. We implemented a dfl-uio-pdev module which exposes the UIO device
>> The dfl-uio-pdev module exposes
> Will change it.
>
>>> +interfaces. It adds the uio_pdrv_genirq platform device with the resources of
>>> +the DFL device, and let the generic UIO platform device driver provide UIO
>> the DLF device, and lets
> Will change it.
>
>>> +support to userspace.
>> Use FPGA_DFL_UIO_PDEV to enable this feature.
> I didn't get your idea for this.

I wanted the user to know which kconfig controls this feature.

Leave it out if you don't think it fits.

>
>>> +
>>> +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.
>> (not sure if this section is needed)
> I think we may keep it.

Ok.

Tom

>
> Thanks,
> Yilun
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] Documentation: fpga: dfl: Add description for DFL UIO support
  2020-12-15  4:43       ` Tom Rix
@ 2020-12-15  5:15         ` Xu Yilun
  0 siblings, 0 replies; 9+ messages in thread
From: Xu Yilun @ 2020-12-15  5:15 UTC (permalink / raw)
  To: Tom Rix; +Cc: mdf, linux-fpga, linux-kernel, gregkh, lgoncalv, hao.wu

On Mon, Dec 14, 2020 at 08:43:31PM -0800, Tom Rix wrote:
> 
> On 12/14/20 6:22 PM, Xu Yilun wrote:
> > On Mon, Dec 14, 2020 at 02:14:56PM -0800, Tom Rix wrote:
> >> On 12/13/20 7:36 PM, Xu Yilun wrote:
> >>> 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.
> >>> ---
> >>
> >>> +components. They could 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.
> > Could we just add the line rather than replacing the previous line? I think this
> > comment is describing a different usecase.
> 
> Yes, this is what i ment, please use your original.
> 
> I am offering another usecase, one I will use.
> 
> Add mine as well, if you want.

I'll add your usecase.

> 
> >
> >>> +* 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. We implemented a dfl-uio-pdev module which exposes the UIO device
> >> The dfl-uio-pdev module exposes
> > Will change it.
> >
> >>> +interfaces. It adds the uio_pdrv_genirq platform device with the resources of
> >>> +the DFL device, and let the generic UIO platform device driver provide UIO
> >> the DLF device, and lets
> > Will change it.
> >
> >>> +support to userspace.
> >> Use FPGA_DFL_UIO_PDEV to enable this feature.
> > I didn't get your idea for this.
> 
> I wanted the user to know which kconfig controls this feature.

Understood. I'll add it.

Thanks,
Yilun

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-12-15  5:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14  3:36 [PATCH v2 0/2] UIO support for dfl devices Xu Yilun
2020-12-14  3:36 ` [PATCH v2 1/2] fpga: dfl: add the userspace I/O device support for DFL devices Xu Yilun
2020-12-14 22:03   ` Tom Rix
2020-12-15  2:06     ` Xu Yilun
2020-12-14  3:36 ` [PATCH v2 2/2] Documentation: fpga: dfl: Add description for DFL UIO support Xu Yilun
2020-12-14 22:14   ` Tom Rix
2020-12-15  2:22     ` Xu Yilun
2020-12-15  4:43       ` Tom Rix
2020-12-15  5:15         ` Xu Yilun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).