lkml.org 
[lkml]   [2021]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RESEND PATCH v11 1/2] uio: uio_dfl: add userspace i/o driver for DFL bus
Hi Greg,

On Sat, Feb 27, 2021 at 04:42:55PM +0100, Greg KH wrote:
> On Sat, Feb 27, 2021 at 11:27:03PM +0800, 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>
> > Reviewed-by: Tom Rix <trix@redhat.com>
> > ---
> > v9: switch to add a uio driver in drivers/uio
> > v10: add the source file in MAINTAINERS
> > more descriptive Kconfig header
> > add detailed path for opae uio example
> > v11: no change
> > ---
> > MAINTAINERS | 1 +
> > drivers/uio/Kconfig | 17 +++++++++++++
> > drivers/uio/Makefile | 1 +
> > drivers/uio/uio_dfl.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 85 insertions(+)
> > create mode 100644 drivers/uio/uio_dfl.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 000fe0b..4dc0354 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6943,6 +6943,7 @@ S: Maintained
> > F: Documentation/ABI/testing/sysfs-bus-dfl*
> > F: Documentation/fpga/dfl.rst
> > F: drivers/fpga/dfl*
> > +F: drivers/uio/uio_dfl.c
> > F: include/linux/dfl.h
> > F: include/uapi/linux/fpga-dfl.h
> >
> > diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> > index 202ee81..5531f3a 100644
> > --- a/drivers/uio/Kconfig
> > +++ b/drivers/uio/Kconfig
> > @@ -165,4 +165,21 @@ 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 (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
> > +
> > + It could be found at:
> > +
> > + opae-sdk/tools/libopaeuio/
> > +
> > + 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
>
> Why are you saying that an ethernet driver should be using the UIO
> interface?
>
> And why can't you use the existing UIO drivers that bind to memory
> regions specified by firmware? Without an interrupt being used, why is
> UIO needed at all?

That was an earlier version -- which from your comments you didn't seem
to like :) Maybe miscommunication.

Essentially I see two options:
- Have a DFL bus driver instantiate a platform driver (uio_pdrv_genirq)
which I *think* you described above?
- What this patch implements -- a UIO driver on the DFL bus

These FPGA devices can on the fly change their contents and -- even if
just for test -- being able to expose a bunch of registers via UIO can
be extremely useful.

Whether a device should expose registers or not should be up to the
implemeneter of the FPGA design I think (policy). This patch (or the
previous version) provides a mechanism to do so via DFL.

This is similar in nature to uio_pdrv_genirq on a DT based platform, to
expose the registers you instantiate the DT node.

Re-implementing a new driver for each of these instances doesn't seem
desirable and tying DFL as enumeration mechanism to UIO seems like a
good compromise for enabling this kind of functionality.

Note this is *not* an attempt to bypass the network stack or other
existing subsystems.

Cheers,

Moritz

\
 
 \ /
  Last update: 2021-02-28 18:22    [W:0.061 / U:10.940 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site