[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <35a9d8ae152deea8a79392bb4b2a9b2053daae92.camel@crapouillou.net>
Date: Wed, 17 Jan 2024 13:09:40 +0100
From: Paul Cercueil <paul@...pouillou.net>
To: Vegard Nossum <vegard.nossum@...cle.com>, Greg Kroah-Hartman
<gregkh@...uxfoundation.org>, Sumit Semwal <sumit.semwal@...aro.org>,
Christian König
<christian.koenig@....com>, Jonathan Corbet <corbet@....net>
Cc: Jonathan Cameron <jic23@...nel.org>, Nuno Sá
<noname.nuno@...il.com>, Michael Hennerich <Michael.Hennerich@...log.com>,
Andrzej Pietrasiewicz <andrzej.p@...labora.com>, linux-usb@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v3 4/4] Documentation: usb: Document FunctionFS DMABUF
API
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@...pouillou.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
Powered by blists - more mailing lists