2021-01-26 10:56:09

by Xu Yilun

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

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 <[email protected]>
> > > ---
> > > 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


2021-01-26 13:55:18

by Tom Rix

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


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 <[email protected]>
>>>> ---
>>>> 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/[email protected]/

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
>

2021-01-27 19:48:41

by Moritz Fischer

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

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 <[email protected]>
> > > > ---
> > > > 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