diff mbox series

[v3,4/4] Documentation: usb: Document FunctionFS DMABUF API

Message ID 20240108120056.22165-5-paul@crapouillou.net (mailing list archive)
State Superseded
Headers show
Series usb: gadget: functionfs: DMABUF import interface | expand

Commit Message

Paul Cercueil Jan. 8, 2024, noon UTC
Add documentation for the three ioctls used to attach or detach
externally-created DMABUFs, and to request transfers from/to previously
attached DMABUFs.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>

---
v3: New patch
---
 Documentation/usb/functionfs.rst | 36 ++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Vegard Nossum Jan. 9, 2024, 1:08 p.m. UTC | #1
On 08/01/2024 13:00, Paul Cercueil wrote:
> Add documentation for the three ioctls used to attach or detach
> externally-created DMABUFs, and to request transfers from/to previously
> attached DMABUFs.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> 
> ---
> v3: New patch
> ---
>   Documentation/usb/functionfs.rst | 36 ++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)

Hi,

I'd like to point out that this file (usb/functionfs.rst) is currently
included by Documentation/subsystem-apis.rst, the top-level file for the
"Kernel subsystem documentation" set of books, which describe internal
APIs: "These books get into the details of how specific kernel
subsystems work from the point of view of a kernel developer".

However, functionfs.rst (and especially your new additions) are
documenting a userspace API, so it really belongs somewhere in
Documentation/userspace-api/ -- that's where /proc, /sys, /dev and ioctl
descriptions for userspace programmers belong.

I'm not NAKing the patch -- I just want to draw attention to this
discrepancy. Maybe we can separate the kernel-implementation details
(stuff about __init sections and stuff) from the new ioctl() info?

Looking at <https://docs.kernel.org/usb/> I see that there are many
other adjacent documents that are also not really documenting kernel
implementation details, rough categorization as follows:

USB support
-----------

- Linux ACM driver v0.16 ==> admin/user info
- Authorizing (or not) your USB devices to connect to the system ==> 
admin/user info
- ChipIdea Highspeed Dual Role Controller Driver => admin/user info
- DWC3 driver ==> driver TODOs (can be moved into source code?)
- EHCI driver ==> technical info + driver details
- How FunctionFS works
- Linux USB gadget configured through configfs ==> userspace API + 
implementation
- Linux USB HID gadget driver ==> implementation + userspace API
- Multifunction Composite Gadget ==> technical + user info
- Linux USB Printer Gadget Driver ==> userspace API
- Linux Gadget Serial Driver v2.0 ==> user/admin + userspace API
- Linux UVC Gadget Driver ==> user/admin + userspace API
- Gadget Testing ==> user/admin + userspace API
- Infinity Usb Unlimited Readme ==> user/admin
- Mass Storage Gadget (MSG) ==> user/admin
- USB 7-Segment Numeric Display ==> user/admin
- mtouchusb driver ==> user/admin
- OHCI ==> technical info
- USB Raw Gadget ==> userspace API
- USB/IP protocol ==> technical info
- usbmon ==> user/admin + userspace API
- USB serial ==> user/admin + technical info
- USB references
- Linux CDC ACM inf
- Linux inf
- USB devfs drop permissions source
- Credits

By "admin/user info", I mean things that a user would have to do or run
(e.g. modprobe + flags) to make use of a driver; "technical info" is
more like device specifications (transfer speeds, modes of operation,
etc.); "userspace API" is stuff like configfs and ioctls; "driver
details" is really implementation details and internal considerations.

The last ones I don't even really know how to categorize.

I'm guessing nobody is really enthralled by the idea of splitting
Documentation/usb/ up like this?

   Documentation/admin-guide/usb/
   Documentation/driver-api/usb/ (this one actually exists already)
   Documentation/userspace-api/usb/

For the stuff that is _actually_ internal to a specific driver (so not
useful for end users, not useful for admins, not generic USB info, and
not useful for userspace programmers), I would honestly propose to just
move it directly into the driver's source code, or, if the text is
obsolete, just get rid of it completely.

The distinction between user/admin and userspace API is pretty clear
(one is for end users, the other is for userspace _programmers_), but it
can sometimes be hard to determine whether something falls in one or the
other category.

In any case -- it looks like almost all of the usb/ directory does not
document "how specific kernel subsystems work from the point of view of
a kernel developer" so maybe we should just move the include to
userspace-api/ for now as an obvious improvement (if still not 100%
correct):

diff --git a/Documentation/subsystem-apis.rst 
b/Documentation/subsystem-apis.rst
index 2d353fb8ea26..fe972f57bf4c 100644
--- a/Documentation/subsystem-apis.rst
+++ b/Documentation/subsystem-apis.rst
@@ -81,7 +81,6 @@ Storage interfaces
     security/index
     crypto/index
     bpf/index
-   usb/index
     PCI/index
     misc-devices/index
     peci/index
diff --git a/Documentation/userspace-api/index.rst 
b/Documentation/userspace-api/index.rst
index 82f9dbd228f5..e60cd9174ada 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -41,6 +41,7 @@ Subsystem-specific documentation:
     tee
     isapnp
     dcdbas
+   ../usb/index

  Kernel ABIs: These documents describe the the ABI between the Linux
  kernel and userspace, and the relative stability of these interfaces.


Thoughts?


Vegard
Paul Cercueil Jan. 17, 2024, 12:09 p.m. UTC | #2
Hi Vegard,

Le mardi 09 janvier 2024 à 14:08 +0100, Vegard Nossum a écrit :
> On 08/01/2024 13:00, Paul Cercueil wrote:
> > Add documentation for the three ioctls used to attach or detach
> > externally-created DMABUFs, and to request transfers from/to
> > previously
> > attached DMABUFs.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > 
> > ---
> > v3: New patch
> > ---
> >   Documentation/usb/functionfs.rst | 36
> > ++++++++++++++++++++++++++++++++
> >   1 file changed, 36 insertions(+)
> 
> Hi,
> 
> I'd like to point out that this file (usb/functionfs.rst) is
> currently
> included by Documentation/subsystem-apis.rst, the top-level file for
> the
> "Kernel subsystem documentation" set of books, which describe
> internal
> APIs: "These books get into the details of how specific kernel
> subsystems work from the point of view of a kernel developer".
> 
> However, functionfs.rst (and especially your new additions) are
> documenting a userspace API, so it really belongs somewhere in
> Documentation/userspace-api/ -- that's where /proc, /sys, /dev and
> ioctl
> descriptions for userspace programmers belong.

Agreed. Even the original content prior to my additions describe a
userspace API.

> 
> I'm not NAKing the patch -- I just want to draw attention to this
> discrepancy. Maybe we can separate the kernel-implementation details
> (stuff about __init sections and stuff) from the new ioctl() info?
> 
> Looking at <https://docs.kernel.org/usb/> I see that there are many
> other adjacent documents that are also not really documenting kernel
> implementation details, rough categorization as follows:
> 
> USB support
> -----------
> 
> - Linux ACM driver v0.16 ==> admin/user info
> - Authorizing (or not) your USB devices to connect to the system ==> 
> admin/user info
> - ChipIdea Highspeed Dual Role Controller Driver => admin/user info
> - DWC3 driver ==> driver TODOs (can be moved into source code?)
> - EHCI driver ==> technical info + driver details
> - How FunctionFS works
> - Linux USB gadget configured through configfs ==> userspace API + 
> implementation
> - Linux USB HID gadget driver ==> implementation + userspace API
> - Multifunction Composite Gadget ==> technical + user info
> - Linux USB Printer Gadget Driver ==> userspace API
> - Linux Gadget Serial Driver v2.0 ==> user/admin + userspace API
> - Linux UVC Gadget Driver ==> user/admin + userspace API
> - Gadget Testing ==> user/admin + userspace API
> - Infinity Usb Unlimited Readme ==> user/admin
> - Mass Storage Gadget (MSG) ==> user/admin
> - USB 7-Segment Numeric Display ==> user/admin
> - mtouchusb driver ==> user/admin
> - OHCI ==> technical info
> - USB Raw Gadget ==> userspace API
> - USB/IP protocol ==> technical info
> - usbmon ==> user/admin + userspace API
> - USB serial ==> user/admin + technical info
> - USB references
> - Linux CDC ACM inf
> - Linux inf
> - USB devfs drop permissions source
> - Credits
> 
> By "admin/user info", I mean things that a user would have to do or
> run
> (e.g. modprobe + flags) to make use of a driver; "technical info" is
> more like device specifications (transfer speeds, modes of operation,
> etc.); "userspace API" is stuff like configfs and ioctls; "driver
> details" is really implementation details and internal
> considerations.
> 
> The last ones I don't even really know how to categorize.
> 
> I'm guessing nobody is really enthralled by the idea of splitting
> Documentation/usb/ up like this?
> 
>    Documentation/admin-guide/usb/
>    Documentation/driver-api/usb/ (this one actually exists already)
>    Documentation/userspace-api/usb/
> 
> For the stuff that is _actually_ internal to a specific driver (so
> not
> useful for end users, not useful for admins, not generic USB info,
> and
> not useful for userspace programmers), I would honestly propose to
> just
> move it directly into the driver's source code, or, if the text is
> obsolete, just get rid of it completely.
> 
> The distinction between user/admin and userspace API is pretty clear
> (one is for end users, the other is for userspace _programmers_), but
> it
> can sometimes be hard to determine whether something falls in one or
> the
> other category.
> 
> In any case -- it looks like almost all of the usb/ directory does
> not
> document "how specific kernel subsystems work from the point of view
> of
> a kernel developer" so maybe we should just move the include to
> userspace-api/ for now as an obvious improvement (if still not 100%
> correct):
> 
> diff --git a/Documentation/subsystem-apis.rst 
> b/Documentation/subsystem-apis.rst
> index 2d353fb8ea26..fe972f57bf4c 100644
> --- a/Documentation/subsystem-apis.rst
> +++ b/Documentation/subsystem-apis.rst
> @@ -81,7 +81,6 @@ Storage interfaces
>      security/index
>      crypto/index
>      bpf/index
> -   usb/index
>      PCI/index
>      misc-devices/index
>      peci/index
> diff --git a/Documentation/userspace-api/index.rst 
> b/Documentation/userspace-api/index.rst
> index 82f9dbd228f5..e60cd9174ada 100644
> --- a/Documentation/userspace-api/index.rst
> +++ b/Documentation/userspace-api/index.rst
> @@ -41,6 +41,7 @@ Subsystem-specific documentation:
>      tee
>      isapnp
>      dcdbas
> +   ../usb/index
> 
>   Kernel ABIs: These documents describe the the ABI between the Linux
>   kernel and userspace, and the relative stability of these
> interfaces.
> 
> 
> Thoughts?

Makes sense to me. There's definitely some cleanup to be done in the
USB documentation.

> Vegard

Cheers,
-Paul
diff mbox series

Patch

diff --git a/Documentation/usb/functionfs.rst b/Documentation/usb/functionfs.rst
index a3054bea38f3..d05a775bc45b 100644
--- a/Documentation/usb/functionfs.rst
+++ b/Documentation/usb/functionfs.rst
@@ -2,6 +2,9 @@ 
 How FunctionFS works
 ====================
 
+Overview
+========
+
 From kernel point of view it is just a composite function with some
 unique behaviour.  It may be added to an USB configuration only after
 the user space driver has registered by writing descriptors and
@@ -66,3 +69,36 @@  have been written to their ep0's.
 
 Conversely, the gadget is unregistered after the first USB function
 closes its endpoints.
+
+DMABUF interface
+================
+
+FunctionFS additionally supports a DMABUF based interface, where the
+userspace can attach DMABUF objects (externally created) to an endpoint,
+and subsequently use them for data transfers.
+
+A userspace application can then use this interface to share DMABUF
+objects between several interfaces, allowing it to transfer data in a
+zero-copy fashion, for instance between IIO and the USB stack.
+
+As part of this interface, three new IOCTLs have been added. These three
+IOCTLs have to be performed on a data endpoint (ie. not ep0). They are:
+
+  ``FUNCTIONFS_DMABUF_ATTACH(int)``
+    Attach the DMABUF object, identified by its file descriptor, to the
+    data endpoint. Returns zero on success, and a negative errno value
+    on error.
+
+  ``FUNCTIONFS_DMABUF_DETACH(int)``
+    Detach the given DMABUF object, identified by its file descriptor,
+    from the data endpoint. Returns zero on success, and a negative
+    errno value on error. Note that closing the endpoint's file
+    descriptor will automatically detach all attached DMABUFs.
+
+  ``FUNCTIONFS_DMABUF_TRANSFER(struct usb_ffs_dmabuf_transfer_req *)``
+    Enqueue the previously attached DMABUF to the transfer queue.
+    The argument is a structure that packs the DMABUF's file descriptor,
+    the size in bytes to transfer (which should generally correspond to
+    the size of the DMABUF), and a 'flags' field which is unused
+    for now. Returns zero on success, and a negative errno value on
+    error.