From: Moritz Fischer <mdf@kernel.org>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Xu Yilun <yilun.xu@intel.com>,
mdf@kernel.org, linux-fpga@vger.kernel.org,
linux-kernel@vger.kernel.org, trix@redhat.com,
lgoncalv@redhat.com, hao.wu@intel.com
Subject: Re: [PATCH v6 1/2] fpga: dfl: add the userspace I/O device support for DFL devices
Date: Sun, 17 Jan 2021 08:22:36 -0800 [thread overview]
Message-ID: <YARkTFMrotPo45ic@epycbox.lan> (raw)
In-Reply-To: <YARbgGU6lr3zZaKP@kroah.com>
Greg,
On Sun, Jan 17, 2021 at 04:45:04PM +0100, Greg KH wrote:
> On Wed, Jan 13, 2021 at 09:54:07AM +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 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.
>
> Why doesn't the existing uio driver work for this, why do you need a new
> one?
>
> > ---
> > drivers/fpga/Kconfig | 10 +++++
> > drivers/fpga/Makefile | 1 +
> > drivers/fpga/dfl-uio-pdev.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
>
> uio drivers traditionally go in drivers/uio/ and start with "uio", so
> shouldn't this be drivers/uio/uio_dfl_pdev.c to match the same naming
> scheme?
I had considered suggesting that, but ultimately this driver only
creates a 'uio_pdrv_genirq' platform device, so it didn't seem like a
good fit.
>
> But again, you need to explain in detail, why the existing uio driver
> doesn't work properly, or why you can't just add a few lines to an
> existing one.
Ultimately there are three options I see:
1) Do what Xu does, which is re-use the 'uio_pdrv_genirq' uio driver by
creating a platform device for it as sub-device of the dfl device that
we bind to uio_pdrv_genirq
2) Add a module_dfl_driver part to drivers/uio/uio_pdrv_genirq.c and
corresponding id table
3) Create a new uio_dfl_genirq kind of driver that uses the dfl bus and
that would make sense to then put into drivers/uio. (This would
duplicate code in uio_pdrv_genirq to some extend)
Overall I think in terms of code re-use I think Xu's choice might be
less new code as it simply wraps the uio platform device driver, and
allows for defining the resources passed to the UIO driver to be defined
by hardware through a DFL.
I've seen the pattern that Xu proposed used in other places like the
macb network driver where you'd have macb_main (the platform driver) and
macb_pci that wraps it for a pci usage.
- Moritz
next prev parent reply other threads:[~2021-01-17 16:23 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-13 1:54 [PATCH v6 0/2] UIO support for dfl devices Xu Yilun
2021-01-13 1:54 ` [PATCH v6 1/2] fpga: dfl: add the userspace I/O device support for DFL devices Xu Yilun
2021-01-17 3:56 ` Moritz Fischer
2021-01-18 2:48 ` Xu Yilun
2021-01-17 15:45 ` Greg KH
2021-01-17 16:22 ` Moritz Fischer [this message]
2021-01-21 14:30 ` Tom Rix
2021-01-21 20:03 ` Moritz Fischer
2021-01-22 5:46 ` Xu Yilun
2021-01-13 1:54 ` [PATCH v6 2/2] Documentation: fpga: dfl: Add description for DFL UIO support Xu Yilun
2021-01-17 3:57 ` Moritz Fischer
2021-01-18 3:10 ` Xu Yilun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YARkTFMrotPo45ic@epycbox.lan \
--to=mdf@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=hao.wu@intel.com \
--cc=lgoncalv@redhat.com \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=trix@redhat.com \
--cc=yilun.xu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).