diff mbox series

[2/7] miscdevice: adding support for MPI3MR_MINOR(243)

Message ID 20210921184600.64427-3-kashyap.desai@broadcom.com (mailing list archive)
State Changes Requested
Headers show
Series adding application support | expand

Commit Message

Kashyap Desai Sept. 21, 2021, 6:45 p.m. UTC
---
 include/linux/miscdevice.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Martin K. Petersen Oct. 5, 2021, 4:27 a.m. UTC | #1
Kashyap,

> @@ -71,6 +71,7 @@
>  #define USERIO_MINOR		240
>  #define VHOST_VSOCK_MINOR	241
>  #define RFKILL_MINOR		242
> +#define MPI3MR_MINOR		243
>  #define MISC_DYNAMIC_MINOR	255

Has this been officially assigned by LANANA?
Kashyap Desai Oct. 26, 2021, 11:16 a.m. UTC | #2
>
>
> Kashyap,
>
> > @@ -71,6 +71,7 @@
> >  #define USERIO_MINOR		240
> >  #define VHOST_VSOCK_MINOR	241
> >  #define RFKILL_MINOR		242
> > +#define MPI3MR_MINOR		243
> >  #define MISC_DYNAMIC_MINOR	255
>
> Has this been officially assigned by LANANA?

Martin -

<mpi3mr> Driver Development team understood the request and we will work
to accommodate <bsg> interface in <mpi3mr> driver and remove ioctl hooks.
Currently there are many in-house application and library developed that
depend upon ioctl interface. Immediately dropping ioctl support will
create lots of issues for Development/Test (within a org +  OEM testing).
How about accepting updated ioctl patch-set after reviewed-by tag (which
will not use static MAJOR number) for time being  ?

Once we have <bsg> interface developed and test qualified, we will post
patch-set.
We will also keep posting some intermediate <bsg> interface patch (for
mpi3mr driver) as PATCH or RFC so that, we can make sure changes are
in-line with upstream design goal.

>
> --
> Martin K. Petersen	Oracle Linux Engineering
Martin K. Petersen Oct. 27, 2021, 3:38 a.m. UTC | #3
Kashyap,

> Immediately dropping ioctl support will create lots of issues for
> Development/Test (within a org + OEM testing).  How about accepting
> updated ioctl patch-set after reviewed-by tag (which will not use
> static MAJOR number) for time being ?

If we were to introduce an ioctl interface for mpi3mr we would never be
able to deprecate it without breaking existing applications.

While I appreciate that it is inconvenient to have to update your
tooling, this is the only chance we have to get the interface right.
Kashyap Desai Oct. 28, 2021, 6:34 p.m. UTC | #4
>
>
> Kashyap,
>
> > Immediately dropping ioctl support will create lots of issues for
> > Development/Test (within a org + OEM testing).  How about accepting
> > updated ioctl patch-set after reviewed-by tag (which will not use
> > static MAJOR number) for time being ?
>
> If we were to introduce an ioctl interface for mpi3mr we would never be
able
> to deprecate it without breaking existing applications.

Martin -

Understood that best case scenario is not to have IOCTL interface code at
all in kernel tree (for new drivers), but we need this interface to be
there for couple of
months.
As of now, There is only in-house application development happened on
<mpi3mr> since product is under development and OEM has access to the h/w
for pre-GA testing.
We are also planning to document such interface change for those who wants
to develop their own application in future.  Most of the application which
need interopt check of ioctl vs bsg will be Broadcom in-house and we are
planning to take care the same.

How about providing unlocked_ioctl under module parameter  ?  By default
parameter will be OFF (this will avoid interopt issue as you mentioned)
and at least user who really have dependency on Test vehicle for time
being can enable it.
Once <bsg> interface is available, we will remove whole IOCTL code from
tree.


Kashyap
>
> While I appreciate that it is inconvenient to have to update your
tooling, this is
> the only chance we have to get the interface right.
>
> --
> Martin K. Petersen	Oracle Linux Engineering
Sumit Saxena Dec. 13, 2021, 12:23 p.m. UTC | #5
Hi Martin,

We are working on adding bsg interface support in mpi3mr driver. In
bsg, we can use SG_IO command type for synchronous ioctls,
but I don't see the infrastructure for asynchronous commands. I
searched a bit around it and noticed that "poll" interface was
supported
earlier but removed through this patch-
https://www.spinics.net/lists/kernel/msg2853766.html

We have a requirement wherein the userland application/daemon
waits/listens for asynchronous driver/firmware events.
With driver IOCTLs, we were using the "fasync" interface for this. How
can we achieve the same with bsg interface ?

Thanks,
Sumit

On Fri, Oct 29, 2021 at 12:04 AM Kashyap Desai
<kashyap.desai@broadcom.com> wrote:
>
> >
> >
> > Kashyap,
> >
> > > Immediately dropping ioctl support will create lots of issues for
> > > Development/Test (within a org + OEM testing).  How about accepting
> > > updated ioctl patch-set after reviewed-by tag (which will not use
> > > static MAJOR number) for time being ?
> >
> > If we were to introduce an ioctl interface for mpi3mr we would never be
> able
> > to deprecate it without breaking existing applications.
>
> Martin -
>
> Understood that best case scenario is not to have IOCTL interface code at
> all in kernel tree (for new drivers), but we need this interface to be
> there for couple of
> months.
> As of now, There is only in-house application development happened on
> <mpi3mr> since product is under development and OEM has access to the h/w
> for pre-GA testing.
> We are also planning to document such interface change for those who wants
> to develop their own application in future.  Most of the application which
> need interopt check of ioctl vs bsg will be Broadcom in-house and we are
> planning to take care the same.
>
> How about providing unlocked_ioctl under module parameter  ?  By default
> parameter will be OFF (this will avoid interopt issue as you mentioned)
> and at least user who really have dependency on Test vehicle for time
> being can enable it.
> Once <bsg> interface is available, we will remove whole IOCTL code from
> tree.
>
>
> Kashyap
> >
> > While I appreciate that it is inconvenient to have to update your
> tooling, this is
> > the only chance we have to get the interface right.
> >
> > --
> > Martin K. Petersen    Oracle Linux Engineering
Martin K. Petersen Dec. 17, 2021, 3:57 a.m. UTC | #6
Sumit,

> We have a requirement wherein the userland application/daemon
> waits/listens for asynchronous driver/firmware events.  With driver
> IOCTLs, we were using the "fasync" interface for this. How can we
> achieve the same with bsg interface ?

Going forward I suspect io_uring is going to be the preferred interface
for async I/O. But for now can't you just wait for events in a separate
thread?
diff mbox series

Patch

diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 0676f18093f9..2d4d1502c1aa 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -71,6 +71,7 @@ 
 #define USERIO_MINOR		240
 #define VHOST_VSOCK_MINOR	241
 #define RFKILL_MINOR		242
+#define MPI3MR_MINOR		243
 #define MISC_DYNAMIC_MINOR	255
 
 struct device;