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