[PATCH v9 0/2] UIO support for dfl devices

[PATCH v9 0/2] 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.

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. The concern is the UIO may
not be suitable as a default/generic driver for all dfl features, such as
features with multiple interrupts.

In v4, we specify each matching device in the id_table of the UIO driver,
just the same as other dfl drivers do. Now the UIO driver supports Ether
Group feature. To support more DFL features, their feature ids should be
added to the driver's id_table.

Before v9, we create a "uio_pdrv_genirq" platform device using DFL devices'
resources. Then we leverage the uio_pdrv_genirq driver for UIO support. It
is suggested that we implement a driver in drivers/uio that directly calls
UIO framework APIs. So we implement the uio_dfl driver in v9. The driver
now only binds the ether group feature, which has no irq. So the irq
support is not implemented yet.


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.

Main changes from v3:
- switch to specifying each matching device in the driver's id_table.
- refactor the irq handling code.

Main changes from v4:
- refactor the irq handling code.

Main changes from v5:
- fix the res[] zero initialization issue.
- improve the return code for probe().
- some doc improvement.

Main changes from v6:
- use platform_device_register_resndata() for pdev creation.

Main changes from v7:
- some doc fixes.

Main changes from v9:
- switch to add a uio driver in drivers/uio 

Xu Yilun (2):
  uio: uio_dfl: add userspace i/o driver for DFL bus
  Documentation: fpga: dfl: Add description for DFL UIO support

 Documentation/fpga/dfl.rst | 23 ++++++++++++++++
 drivers/uio/Kconfig        | 13 +++++++++
 drivers/uio/Makefile       |  1 +
 drivers/uio/uio_dfl.c      | 66 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 103 insertions(+)
 create mode 100644 drivers/uio/uio_dfl.c

-- 
2.7.4

[PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

[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 now only binds the ether group feature, which has no irq. So
the irq support is not implemented yet.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
---
v9: switch to add a uio driver in drivers/uio
---
 drivers/uio/Kconfig   | 13 ++++++++++
 drivers/uio/Makefile  |  1 +
 drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 drivers/uio/uio_dfl.c

diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 202ee81..44778f8 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -165,4 +165,17 @@ config UIO_HV_GENERIC
 	  to network and storage devices from userspace.
 
 	  If you compile this as a module, it will be called uio_hv_generic.
+
+config UIO_DFL
+	tristate "Generic driver for DFL bus"
+	depends on FPGA_DFL
+	help
+	  Generic DFL (Device Feature List) driver for Userspace I/O devices.
+	  It is useful to provide direct access to DFL devices from userspace.
+	  A sample userspace application using this driver is available for
+	  download in a git repository:
+
+	    git clone https://github.com/OPAE/opae-sdk.git
+
+	  If you compile this as a module, it will be called uio_dfl.
 endif
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index c285dd2..f2f416a1 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_UIO_PRUSS)         += uio_pruss.o
 obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
 obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
 obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
+obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
new file mode 100644
index 0000000..89c0fc7
--- /dev/null
+++ b/drivers/uio/uio_dfl.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Generic DFL driver for Userspace I/O devicess
+ *
+ * Copyright (C) 2021 Intel Corporation, Inc.
+ */
+#include <linux/dfl.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/uio_driver.h>
+
+#define DRIVER_NAME "uio_dfl"
+
+static int uio_dfl_probe(struct dfl_device *ddev)
+{
+	struct resource *r = &ddev->mmio_res;
+	struct device *dev = &ddev->dev;
+	struct uio_info *uioinfo;
+	struct uio_mem *uiomem;
+	int ret;
+
+	uioinfo = devm_kzalloc(dev, sizeof(struct uio_info), GFP_KERNEL);
+	if (!uioinfo)
+		return -ENOMEM;
+
+	uioinfo->name = DRIVER_NAME;
+	uioinfo->version = "0";
+
+	uiomem = &uioinfo->mem[0];
+	uiomem->memtype = UIO_MEM_PHYS;
+	uiomem->addr = r->start & PAGE_MASK;
+	uiomem->offs = r->start & ~PAGE_MASK;
+	uiomem->size = (uiomem->offs + resource_size(r)
+			+ PAGE_SIZE - 1) & PAGE_MASK;
+	uiomem->name = r->name;
+
+	/* Irq is yet to be supported */
+	uioinfo->irq = UIO_IRQ_NONE;
+
+	ret = devm_uio_register_device(dev, uioinfo);
+	if (ret)
+		dev_err(dev, "unable to register uio device\n");
+
+	return ret;
+}
+
+#define FME_FEATURE_ID_ETH_GROUP	0x10
+
+static const struct dfl_device_id uio_dfl_ids[] = {
+	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
+	{ }
+};
+MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
+
+static struct dfl_driver uio_dfl_driver = {
+	.drv = {
+		.name = DRIVER_NAME,
+	},
+	.id_table	= uio_dfl_ids,
+	.probe		= uio_dfl_probe,
+};
+module_dfl_driver(uio_dfl_driver);
+
+MODULE_DESCRIPTION("Generic DFL driver for Userspace I/O devices");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH v9 2/2] 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.
v4: documentation change since the driver matching is changed.
v5: no change.
v6: improve the title of the userspace driver support section.
    some word improvement.
v7: rebased to next-20210119
v8: some doc fixes.
v9: some doc change since we switch to the driver in drivers/uio.
---
 Documentation/fpga/dfl.rst | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index c41ac76..e35cf87 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
@@ -530,6 +531,28 @@ Being able to specify more than one DFL per BAR has been considered, but it
 was determined the use case did not provide value.  Specifying a single DFL
 per BAR simplifies the implementation and allows for extra error checking.
 
+
+Userspace driver 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 present a DFL device in the system. In some cases users may need a
+userspace driver for the DFL device:
+
+* Users may need to run some diagnostic test for their hardware.
+* 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 direct access to MMIO space and interrupt handling from
+userspace. The uio_dfl module exposes the UIO device interfaces for this
+purpose.
+
+UIO_DFL should be selected to enable the uio_dfl module driver. To support a
+new DFL feature via UIO direct access, its feature id should be added to the
+driver's id_table.
+
+
 Open discussion
 ===============
 FME driver exports one ioctl (DFL_FPGA_FME_PORT_PR) for partial reconfiguration
-- 
2.7.4

Re: [PATCH v9 2/2] Documentation: fpga: dfl: Add description for DFL UIO support

[Tom Rix]

On 1/25/21 12:49 AM, 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.
> v3: some documentation fixes.
> v4: documentation change since the driver matching is changed.
> v5: no change.
> v6: improve the title of the userspace driver support section.
>     some word improvement.
> v7: rebased to next-20210119
> v8: some doc fixes.
> v9: some doc change since we switch to the driver in drivers/uio.
> ---
>  Documentation/fpga/dfl.rst | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)

This looks fine.

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

Re: [PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

[Tom Rix]

On 1/25/21 12:49 AM, 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 now only binds the ether group feature, which has no irq. So
> the irq support is not implemented yet.
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> ---
> v9: switch to add a uio driver in drivers/uio
> ---
>  drivers/uio/Kconfig   | 13 ++++++++++
>  drivers/uio/Makefile  |  1 +
>  drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++

You should add this to the MAINTAINERS file.

>  3 files changed, 80 insertions(+)
>  create mode 100644 drivers/uio/uio_dfl.c
>
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 202ee81..44778f8 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -165,4 +165,17 @@ config UIO_HV_GENERIC
>  	  to network and storage devices from userspace.
>  
>  	  If you compile this as a module, it will be called uio_hv_generic.
> +
> +config UIO_DFL
> +	tristate "Generic driver for DFL bus"

The term 'DFL' will be unknown to folks in drivers/uio

I think it would be better if DFL was always prefixed 'FPGA DFL'

> +	depends on FPGA_DFL
> +	help
> +	  Generic DFL (Device Feature List) driver for Userspace I/O devices.
> +	  It is useful to provide direct access to DFL devices from userspace.
> +	  A sample userspace application using this driver is available for
> +	  download in a git repository:
> +
> +	    git clone https://github.com/OPAE/opae-sdk.git
> +
> +	  If you compile this as a module, it will be called uio_dfl.

opae-sdk is pretty large and uncovered in the Documentation/fpga/dfl.rst.

Where in opae-sdk is this example ?

If you can point me at the example, I will turn it into a selftest.

Tom

>  endif
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index c285dd2..f2f416a1 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_UIO_PRUSS)         += uio_pruss.o
>  obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
>  obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
> +obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
> diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
> new file mode 100644
> index 0000000..89c0fc7
> --- /dev/null
> +++ b/drivers/uio/uio_dfl.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Generic DFL driver for Userspace I/O devicess
> + *
> + * Copyright (C) 2021 Intel Corporation, Inc.
> + */
> +#include <linux/dfl.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/uio_driver.h>
> +
> +#define DRIVER_NAME "uio_dfl"
> +
> +static int uio_dfl_probe(struct dfl_device *ddev)
> +{
> +	struct resource *r = &ddev->mmio_res;
> +	struct device *dev = &ddev->dev;
> +	struct uio_info *uioinfo;
> +	struct uio_mem *uiomem;
> +	int ret;
> +
> +	uioinfo = devm_kzalloc(dev, sizeof(struct uio_info), GFP_KERNEL);
> +	if (!uioinfo)
> +		return -ENOMEM;
> +
> +	uioinfo->name = DRIVER_NAME;
> +	uioinfo->version = "0";
> +
> +	uiomem = &uioinfo->mem[0];
> +	uiomem->memtype = UIO_MEM_PHYS;
> +	uiomem->addr = r->start & PAGE_MASK;
> +	uiomem->offs = r->start & ~PAGE_MASK;
> +	uiomem->size = (uiomem->offs + resource_size(r)
> +			+ PAGE_SIZE - 1) & PAGE_MASK;
> +	uiomem->name = r->name;
> +
> +	/* Irq is yet to be supported */
> +	uioinfo->irq = UIO_IRQ_NONE;
> +
> +	ret = devm_uio_register_device(dev, uioinfo);
> +	if (ret)
> +		dev_err(dev, "unable to register uio device\n");
> +
> +	return ret;
> +}
> +
> +#define FME_FEATURE_ID_ETH_GROUP	0x10
> +
> +static const struct dfl_device_id uio_dfl_ids[] = {
> +	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
> +
> +static struct dfl_driver uio_dfl_driver = {
> +	.drv = {
> +		.name = DRIVER_NAME,
> +	},
> +	.id_table	= uio_dfl_ids,
> +	.probe		= uio_dfl_probe,
> +};
> +module_dfl_driver(uio_dfl_driver);
> +
> +MODULE_DESCRIPTION("Generic DFL driver for Userspace I/O devices");
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

[Xu Yilun]

On Mon, Jan 25, 2021 at 11:00:38AM -0800, Tom Rix wrote:
> 
> On 1/25/21 12:49 AM, 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 now only binds the ether group feature, which has no irq. So
> > the irq support is not implemented yet.
> >
> > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > ---
> > v9: switch to add a uio driver in drivers/uio
> > ---
> >  drivers/uio/Kconfig   | 13 ++++++++++
> >  drivers/uio/Makefile  |  1 +
> >  drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> You should add this to the MAINTAINERS file.

OK. I could add recored the file under "FPGA DFL DRIVERS"

> 
> >  3 files changed, 80 insertions(+)
> >  create mode 100644 drivers/uio/uio_dfl.c
> >
> > diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> > index 202ee81..44778f8 100644
> > --- a/drivers/uio/Kconfig
> > +++ b/drivers/uio/Kconfig
> > @@ -165,4 +165,17 @@ config UIO_HV_GENERIC
> >  	  to network and storage devices from userspace.
> >  
> >  	  If you compile this as a module, it will be called uio_hv_generic.
> > +
> > +config UIO_DFL
> > +	tristate "Generic driver for DFL bus"
> 
> The term 'DFL' will be unknown to folks in drivers/uio
> 
> I think it would be better if DFL was always prefixed 'FPGA DFL'

The bus is named as "DFL bus", how about we add the full name like the
following help message.

  tristate "Generic driver for DFL (Device Feature List) bus"

> 
> > +	depends on FPGA_DFL
> > +	help
> > +	  Generic DFL (Device Feature List) driver for Userspace I/O devices.
> > +	  It is useful to provide direct access to DFL devices from userspace.
> > +	  A sample userspace application using this driver is available for
> > +	  download in a git repository:
> > +
> > +	    git clone https://github.com/OPAE/opae-sdk.git
> > +
> > +	  If you compile this as a module, it will be called uio_dfl.
> 
> opae-sdk is pretty large and uncovered in the Documentation/fpga/dfl.rst.
> 
> Where in opae-sdk is this example ?

You can find the example in tools/libopaeuio/. I could add the path in
help message.

Thanks,
Yilun

> 
> If you can point me at the example, I will turn it into a selftest.

Re: [PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

[Moritz Fischer]

On Mon, Jan 25, 2021 at 11:00:38AM -0800, Tom Rix wrote:
> 
> On 1/25/21 12:49 AM, 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 now only binds the ether group feature, which has no irq. So
> > the irq support is not implemented yet.
> >
> > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > ---
> > v9: switch to add a uio driver in drivers/uio
> > ---
> >  drivers/uio/Kconfig   | 13 ++++++++++
> >  drivers/uio/Makefile  |  1 +
> >  drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> You should add this to the MAINTAINERS file.

This is covered by MAINTAINERS under drivers/uio.
> 
> >  3 files changed, 80 insertions(+)
> >  create mode 100644 drivers/uio/uio_dfl.c
> >
> > diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> > index 202ee81..44778f8 100644
> > --- a/drivers/uio/Kconfig
> > +++ b/drivers/uio/Kconfig
> > @@ -165,4 +165,17 @@ config UIO_HV_GENERIC
> >  	  to network and storage devices from userspace.
> >  
> >  	  If you compile this as a module, it will be called uio_hv_generic.
> > +
> > +config UIO_DFL
> > +	tristate "Generic driver for DFL bus"
> 
> The term 'DFL' will be unknown to folks in drivers/uio
> 
> I think it would be better if DFL was always prefixed 'FPGA DFL'
> 
> > +	depends on FPGA_DFL
> > +	help
> > +	  Generic DFL (Device Feature List) driver for Userspace I/O devices.
> > +	  It is useful to provide direct access to DFL devices from userspace.
> > +	  A sample userspace application using this driver is available for
> > +	  download in a git repository:
> > +
> > +	    git clone https://github.com/OPAE/opae-sdk.git
> > +
> > +	  If you compile this as a module, it will be called uio_dfl.

I'm not sure KConfig is the right place for this.
> 
> opae-sdk is pretty large and uncovered in the Documentation/fpga/dfl.rst.
> 
> Where in opae-sdk is this example ?
> 
> If you can point me at the example, I will turn it into a selftest.
> 
> Tom
> 
> >  endif
> > diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> > index c285dd2..f2f416a1 100644
> > --- a/drivers/uio/Makefile
> > +++ b/drivers/uio/Makefile
> > @@ -11,3 +11,4 @@ obj-$(CONFIG_UIO_PRUSS)         += uio_pruss.o
> >  obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
> >  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
> >  obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
> > +obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
> > diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
> > new file mode 100644
> > index 0000000..89c0fc7
> > --- /dev/null
> > +++ b/drivers/uio/uio_dfl.c
> > @@ -0,0 +1,66 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Generic DFL driver for Userspace I/O devicess
> > + *
> > + * Copyright (C) 2021 Intel Corporation, Inc.
> > + */
> > +#include <linux/dfl.h>
> > +#include <linux/errno.h>
> > +#include <linux/module.h>
> > +#include <linux/uio_driver.h>
> > +
> > +#define DRIVER_NAME "uio_dfl"
> > +
> > +static int uio_dfl_probe(struct dfl_device *ddev)
> > +{
> > +	struct resource *r = &ddev->mmio_res;
> > +	struct device *dev = &ddev->dev;
> > +	struct uio_info *uioinfo;
> > +	struct uio_mem *uiomem;
> > +	int ret;
> > +
> > +	uioinfo = devm_kzalloc(dev, sizeof(struct uio_info), GFP_KERNEL);
> > +	if (!uioinfo)
> > +		return -ENOMEM;
> > +
> > +	uioinfo->name = DRIVER_NAME;
> > +	uioinfo->version = "0";
> > +
> > +	uiomem = &uioinfo->mem[0];
> > +	uiomem->memtype = UIO_MEM_PHYS;
> > +	uiomem->addr = r->start & PAGE_MASK;
> > +	uiomem->offs = r->start & ~PAGE_MASK;
> > +	uiomem->size = (uiomem->offs + resource_size(r)
> > +			+ PAGE_SIZE - 1) & PAGE_MASK;
> > +	uiomem->name = r->name;
> > +
> > +	/* Irq is yet to be supported */
> > +	uioinfo->irq = UIO_IRQ_NONE;
> > +
> > +	ret = devm_uio_register_device(dev, uioinfo);
> > +	if (ret)
> > +		dev_err(dev, "unable to register uio device\n");
> > +
> > +	return ret;
> > +}
> > +
> > +#define FME_FEATURE_ID_ETH_GROUP	0x10
> > +
> > +static const struct dfl_device_id uio_dfl_ids[] = {
> > +	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
> > +
> > +static struct dfl_driver uio_dfl_driver = {
> > +	.drv = {
> > +		.name = DRIVER_NAME,
> > +	},
> > +	.id_table	= uio_dfl_ids,
> > +	.probe		= uio_dfl_probe,
> > +};
> > +module_dfl_driver(uio_dfl_driver);
> > +
> > +MODULE_DESCRIPTION("Generic DFL driver for Userspace I/O devices");
> > +MODULE_AUTHOR("Intel Corporation");
> > +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

[Xu Yilun]

On Mon, Jan 25, 2021 at 06:22:55PM -0800, Moritz Fischer wrote:
> On Mon, Jan 25, 2021 at 11:00:38AM -0800, Tom Rix wrote:
> > 
> > On 1/25/21 12:49 AM, 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 now only binds the ether group feature, which has no irq. So
> > > the irq support is not implemented yet.
> > >
> > > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > > ---
> > > v9: switch to add a uio driver in drivers/uio
> > > ---
> > >  drivers/uio/Kconfig   | 13 ++++++++++
> > >  drivers/uio/Makefile  |  1 +
> > >  drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 
> > You should add this to the MAINTAINERS file.
> 
> This is covered by MAINTAINERS under drivers/uio.

Yes. But is it OK I also add the file in "FPGA DFL DRIVERS"? So DFL
developers would also be aware if there is change. It is a little
different from other feature driver, it is like a generic driver for
DFL bus.

> > 
> > >  3 files changed, 80 insertions(+)
> > >  create mode 100644 drivers/uio/uio_dfl.c
> > >
> > > diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> > > index 202ee81..44778f8 100644
> > > --- a/drivers/uio/Kconfig
> > > +++ b/drivers/uio/Kconfig
> > > @@ -165,4 +165,17 @@ config UIO_HV_GENERIC
> > >  	  to network and storage devices from userspace.
> > >  
> > >  	  If you compile this as a module, it will be called uio_hv_generic.
> > > +
> > > +config UIO_DFL
> > > +	tristate "Generic driver for DFL bus"
> > 
> > The term 'DFL' will be unknown to folks in drivers/uio
> > 
> > I think it would be better if DFL was always prefixed 'FPGA DFL'
> > 
> > > +	depends on FPGA_DFL
> > > +	help
> > > +	  Generic DFL (Device Feature List) driver for Userspace I/O devices.
> > > +	  It is useful to provide direct access to DFL devices from userspace.
> > > +	  A sample userspace application using this driver is available for
> > > +	  download in a git repository:
> > > +
> > > +	    git clone https://github.com/OPAE/opae-sdk.git
> > > +
> > > +	  If you compile this as a module, it will be called uio_dfl.
> 
> I'm not sure KConfig is the right place for this.

Do you mean the OPAE link? I see several uio drivers provide their
userspace application link in Kconfig. I guess the uio drivers are
selected for these applications so it may be better pointing out where
they are.

Thanks,
Yilun

Re: [PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

[Moritz Fischer]

On Tue, Jan 26, 2021 at 10:40:05AM +0800, Xu Yilun wrote:
> On Mon, Jan 25, 2021 at 06:22:55PM -0800, Moritz Fischer wrote:
> > On Mon, Jan 25, 2021 at 11:00:38AM -0800, Tom Rix wrote:
> > > 
> > > On 1/25/21 12:49 AM, 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 now only binds the ether group feature, which has no irq. So
> > > > the irq support is not implemented yet.
> > > >
> > > > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > > > ---
> > > > v9: switch to add a uio driver in drivers/uio
> > > > ---
> > > >  drivers/uio/Kconfig   | 13 ++++++++++
> > > >  drivers/uio/Makefile  |  1 +
> > > >  drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 
> > > You should add this to the MAINTAINERS file.
> > 
> > This is covered by MAINTAINERS under drivers/uio.
> 
> Yes. But is it OK I also add the file in "FPGA DFL DRIVERS"? So DFL
> developers would also be aware if there is change. It is a little
> different from other feature driver, it is like a generic driver for
> DFL bus.
Fair enough. I meant you don't have to if you don't want to. If you want
to feel free to do so :)
> 
> > > 
> > > >  3 files changed, 80 insertions(+)
> > > >  create mode 100644 drivers/uio/uio_dfl.c
> > > >
> > > > diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> > > > index 202ee81..44778f8 100644
> > > > --- a/drivers/uio/Kconfig
> > > > +++ b/drivers/uio/Kconfig
> > > > @@ -165,4 +165,17 @@ config UIO_HV_GENERIC
> > > >  	  to network and storage devices from userspace.
> > > >  
> > > >  	  If you compile this as a module, it will be called uio_hv_generic.
> > > > +
> > > > +config UIO_DFL
> > > > +	tristate "Generic driver for DFL bus"
> > > 
> > > The term 'DFL' will be unknown to folks in drivers/uio
> > > 
> > > I think it would be better if DFL was always prefixed 'FPGA DFL'
> > > 
> > > > +	depends on FPGA_DFL
> > > > +	help
> > > > +	  Generic DFL (Device Feature List) driver for Userspace I/O devices.
> > > > +	  It is useful to provide direct access to DFL devices from userspace.
> > > > +	  A sample userspace application using this driver is available for
> > > > +	  download in a git repository:
> > > > +
> > > > +	    git clone https://github.com/OPAE/opae-sdk.git
> > > > +
> > > > +	  If you compile this as a module, it will be called uio_dfl.
> > 
> > I'm not sure KConfig is the right place for this.
> 
> Do you mean the OPAE link? I see several uio drivers provide their
> userspace application link in Kconfig. I guess the uio drivers are
> selected for these applications so it may be better pointing out where
> they are.

Ok, works for me.

- Moritz

Re: [PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

[Tom Rix]

On 1/25/21 6:40 PM, Xu Yilun wrote:
> On Mon, Jan 25, 2021 at 06:22:55PM -0800, Moritz Fischer wrote:
>> On Mon, Jan 25, 2021 at 11:00:38AM -0800, Tom Rix wrote:
>>> On 1/25/21 12:49 AM, 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 now only binds the ether group feature, which has no irq. So
>>>> the irq support is not implemented yet.
>>>>
>>>> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
>>>> ---
>>>> v9: switch to add a uio driver in drivers/uio
>>>> ---
>>>>  drivers/uio/Kconfig   | 13 ++++++++++
>>>>  drivers/uio/Makefile  |  1 +
>>>>  drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>> You should add this to the MAINTAINERS file.
>> This is covered by MAINTAINERS under drivers/uio.
> Yes. But is it OK I also add the file in "FPGA DFL DRIVERS"? So DFL
> developers would also be aware if there is change. It is a little
> different from other feature driver, it is like a generic driver for
> DFL bus.

I think the issue is which maintainer branch this gets merged into.

It would not be linux-fpga.

It is this sort of driver I want to add to FPGA SUBDEVICES list described here

https://lore.kernel.org/linux-fpga/96a9d3d9-6091-47c9-21f9-0cfdd9464732@redhat.com/

Where the driver is maintained in the subsystem but reviewed in linux-fpga.

Tom

>>>>  3 files changed, 80 insertions(+)
>>>>  create mode 100644 drivers/uio/uio_dfl.c
>>>>
>>>> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
>>>> index 202ee81..44778f8 100644
>>>> --- a/drivers/uio/Kconfig
>>>> +++ b/drivers/uio/Kconfig
>>>> @@ -165,4 +165,17 @@ config UIO_HV_GENERIC
>>>>  	  to network and storage devices from userspace.
>>>>  
>>>>  	  If you compile this as a module, it will be called uio_hv_generic.
>>>> +
>>>> +config UIO_DFL
>>>> +	tristate "Generic driver for DFL bus"
>>> The term 'DFL' will be unknown to folks in drivers/uio
>>>
>>> I think it would be better if DFL was always prefixed 'FPGA DFL'
>>>
>>>> +	depends on FPGA_DFL
>>>> +	help
>>>> +	  Generic DFL (Device Feature List) driver for Userspace I/O devices.
>>>> +	  It is useful to provide direct access to DFL devices from userspace.
>>>> +	  A sample userspace application using this driver is available for
>>>> +	  download in a git repository:
>>>> +
>>>> +	    git clone https://github.com/OPAE/opae-sdk.git
>>>> +
>>>> +	  If you compile this as a module, it will be called uio_dfl.
>> I'm not sure KConfig is the right place for this.
> Do you mean the OPAE link? I see several uio drivers provide their
> userspace application link in Kconfig. I guess the uio drivers are
> selected for these applications so it may be better pointing out where
> they are.
>
> Thanks,
> Yilun
>

Re: [PATCH v9 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus

[Tom Rix]

On 1/25/21 6:22 PM, Moritz Fischer wrote:
> On Mon, Jan 25, 2021 at 11:00:38AM -0800, Tom Rix wrote:
Snip
>>
>>> +	depends on FPGA_DFL
>>> +	help
>>> +	  Generic DFL (Device Feature List) driver for Userspace I/O devices.
>>> +	  It is useful to provide direct access to DFL devices from userspace.
>>> +	  A sample userspace application using this driver is available for
>>> +	  download in a git repository:
>>> +
>>> +	    git clone https://github.com/OPAE/opae-sdk.git
>>> +
>>> +	  If you compile this as a module, it will be called uio_dfl.
> I'm not sure KConfig is the right place for this.

More than half of the other configs do this.

ex/

config UIO_MF624
    tristate "Humusoft MF624 DAQ PCI card driver"
    depends on PCI
    help
      Userspace I/O interface for the Humusoft MF624 PCI card.
      A sample userspace application using this driver is available
      (among other MF624 related information and software components)
      for download in a git repository:

        git clone git://rtime.felk.cvut.cz/mf6xx.git

      If you compile this as a module, it will be called uio_mf624.

Tom

>> opae-sdk is pretty large and uncovered in the Documentation/fpga/dfl.rst.
>>
>> Where in opae-sdk is this example ?
>>
>> If you can point me at the example, I will turn it into a selftest.
>>
>> Tom
>>
>>>  endif
>>> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
>>> index c285dd2..f2f416a1 100644
>>> --- a/drivers/uio/Makefile
>>> +++ b/drivers/uio/Makefile
>>> @@ -11,3 +11,4 @@ obj-$(CONFIG_UIO_PRUSS)         += uio_pruss.o
>>>  obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>>>  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
>>>  obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
>>> +obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
>>> diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
>>> new file mode 100644
>>> index 0000000..89c0fc7
>>> --- /dev/null
>>> +++ b/drivers/uio/uio_dfl.c
>>> @@ -0,0 +1,66 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Generic DFL driver for Userspace I/O devicess
>>> + *
>>> + * Copyright (C) 2021 Intel Corporation, Inc.
>>> + */
>>> +#include <linux/dfl.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/module.h>
>>> +#include <linux/uio_driver.h>
>>> +
>>> +#define DRIVER_NAME "uio_dfl"
>>> +
>>> +static int uio_dfl_probe(struct dfl_device *ddev)
>>> +{
>>> +	struct resource *r = &ddev->mmio_res;
>>> +	struct device *dev = &ddev->dev;
>>> +	struct uio_info *uioinfo;
>>> +	struct uio_mem *uiomem;
>>> +	int ret;
>>> +
>>> +	uioinfo = devm_kzalloc(dev, sizeof(struct uio_info), GFP_KERNEL);
>>> +	if (!uioinfo)
>>> +		return -ENOMEM;
>>> +
>>> +	uioinfo->name = DRIVER_NAME;
>>> +	uioinfo->version = "0";
>>> +
>>> +	uiomem = &uioinfo->mem[0];
>>> +	uiomem->memtype = UIO_MEM_PHYS;
>>> +	uiomem->addr = r->start & PAGE_MASK;
>>> +	uiomem->offs = r->start & ~PAGE_MASK;
>>> +	uiomem->size = (uiomem->offs + resource_size(r)
>>> +			+ PAGE_SIZE - 1) & PAGE_MASK;
>>> +	uiomem->name = r->name;
>>> +
>>> +	/* Irq is yet to be supported */
>>> +	uioinfo->irq = UIO_IRQ_NONE;
>>> +
>>> +	ret = devm_uio_register_device(dev, uioinfo);
>>> +	if (ret)
>>> +		dev_err(dev, "unable to register uio device\n");
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +#define FME_FEATURE_ID_ETH_GROUP	0x10
>>> +
>>> +static const struct dfl_device_id uio_dfl_ids[] = {
>>> +	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
>>> +
>>> +static struct dfl_driver uio_dfl_driver = {
>>> +	.drv = {
>>> +		.name = DRIVER_NAME,
>>> +	},
>>> +	.id_table	= uio_dfl_ids,
>>> +	.probe		= uio_dfl_probe,
>>> +};
>>> +module_dfl_driver(uio_dfl_driver);
>>> +
>>> +MODULE_DESCRIPTION("Generic DFL driver for Userspace I/O devices");
>>> +MODULE_AUTHOR("Intel Corporation");
>>> +MODULE_LICENSE("GPL v2");