[PATCH] doc: admin-guide/kernel-parameters: remove useless comment

[Vegard Nossum]

This comment about DRM drivers has been there since the first git
commit. It simply doesn't belong in kernel-parameters; remove it.

Signed-off-by: Vegard Nossum 
---
 Documentation/admin-guide/kernel-parameters.rst | 5 -
 1 file changed, 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst 
b/Documentation/admin-guide/kernel-parameters.rst
index 102937bc8443..4410384596a9 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -218,8 +218,3 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted:
 
 .. include:: kernel-parameters.txt
:literal:
-
-Todo
-
-
-   Add more DRM drivers.
-- 
2.34.1

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

[Vegard Nossum]

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 

---
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  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

Re: [PATCH -next] drm/nouveau: uapi: fix kerneldoc warnings

[Vegard Nossum]

On 25/12/2023 08:40, Randy Dunlap wrote:

I do see one thing that I don't like in the generated html output.
It's not a problem with this patch.
The #defines for DRM_NOUVEAU_VM_BIND_OP_MAP etc. have a ';' at the
end of each line:

struct drm_nouveau_vm_bind_op {
 __u32 op;
#define DRM_NOUVEAU_VM_BIND_OP_MAP 0x0;
#define DRM_NOUVEAU_VM_BIND_OP_UNMAP 0x1;
 __u32 flags;
#define DRM_NOUVEAU_VM_BIND_SPARSE (1 << 8);
 __u32 handle;
 __u32 pad;
 __u64 addr;
 __u64 bo_offset;
 __u64 range;
};


Do we actually ever want preprocessor directives to appear inside
definitions in the output? If not, I think this should work:

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 3cdc7dba37e3..61425fc9645e 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1259,6 +1259,8 @@ sub dump_struct($$) {
$clause =~ s/\s+$//;
$clause =~ s/\s+/ /;
next if (!$clause);
+   # skip preprocessor directives
+   next if $clause =~ m/^#/;
$level-- if ($clause =~ m/(\})/ && $level > 1);
if (!($clause =~ m/^\s*#/)) {
$declaration .= "\t" x $level;


Vegard

[PATCH -next] drm/nouveau: uapi: fix kerneldoc warnings

[Vegard Nossum]

As of commit b77fdd6a48e6 ("scripts/kernel-doc: restore warning for
Excess struct/union"), we see the following warnings when running 'make
htmldocs':

  ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member 
'DRM_NOUVEAU_VM_BIND_OP_MAP' description in 'drm_nouveau_vm_bind_op'
  ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member 
'DRM_NOUVEAU_VM_BIND_OP_UNMAP' description in 'drm_nouveau_vm_bind_op'
  ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member 
'DRM_NOUVEAU_VM_BIND_SPARSE' description in 'drm_nouveau_vm_bind_op'
  ./include/uapi/drm/nouveau_drm.h:336: warning: Excess struct member 
'DRM_NOUVEAU_VM_BIND_RUN_ASYNC' description in 'drm_nouveau_vm_bind'

The problem is that these values are #define constants, but had kerneldoc
comments attached to them as if they were actual struct members.

There are a number of ways we could fix this, but I chose to draw
inspiration from include/uapi/drm/i915_drm.h, which pulls them into the
corresponding kerneldoc comment for the struct member that they are
intended to be used with.

To keep the diff readable, there are a number of things I _didn't_ do in
this patch, but which we should also consider:

- This is pretty good documentation, but it ends up in gpu/driver-uapi,
  which is part of subsystem-apis/ when it really ought to display under
  userspace-api/ (the "Linux kernel user-space API guide" book of the
  documentation).

- More generally, we might want a warning if include/uapi/ files are
  kerneldoc'd outside userspace-api/.

- I'd consider it cleaner if the #defines appeared between the kerneldoc
  for the member and the member itself (which is something other DRM-
  related UAPI docs do).

- The %IDENTIFIER kerneldoc syntax is intended for "constants", and is
  more appropriate in this context than ``IDENTIFIER`` or &IDENTIFIER.
  The DRM docs aren't very consistent on this.

Cc: Randy Dunlap 
Cc: Jonathan Corbet 
Signed-off-by: Vegard Nossum 
---
 include/uapi/drm/nouveau_drm.h | 56 --
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
index 0bade1592f34..c95ef8a4d94a 100644
--- a/include/uapi/drm/nouveau_drm.h
+++ b/include/uapi/drm/nouveau_drm.h
@@ -238,34 +238,32 @@ struct drm_nouveau_vm_init {
 struct drm_nouveau_vm_bind_op {
/**
 * @op: the operation type
+*
+* Supported values:
+*
+* %DRM_NOUVEAU_VM_BIND_OP_MAP - Map a GEM object to the GPU's VA
+* space. Optionally, the &DRM_NOUVEAU_VM_BIND_SPARSE flag can be
+* passed to instruct the kernel to create sparse mappings for the
+* given range.
+*
+* %DRM_NOUVEAU_VM_BIND_OP_UNMAP - Unmap an existing mapping in the
+* GPU's VA space. If the region the mapping is located in is a
+* sparse region, new sparse mappings are created where the unmapped
+* (memory backed) mapping was mapped previously. To remove a sparse
+* region the &DRM_NOUVEAU_VM_BIND_SPARSE must be set.
 */
__u32 op;
-/**
- * @DRM_NOUVEAU_VM_BIND_OP_MAP:
- *
- * Map a GEM object to the GPU's VA space. Optionally, the
- * &DRM_NOUVEAU_VM_BIND_SPARSE flag can be passed to instruct the kernel to
- * create sparse mappings for the given range.
- */
 #define DRM_NOUVEAU_VM_BIND_OP_MAP 0x0
-/**
- * @DRM_NOUVEAU_VM_BIND_OP_UNMAP:
- *
- * Unmap an existing mapping in the GPU's VA space. If the region the mapping
- * is located in is a sparse region, new sparse mappings are created where the
- * unmapped (memory backed) mapping was mapped previously. To remove a sparse
- * region the &DRM_NOUVEAU_VM_BIND_SPARSE must be set.
- */
 #define DRM_NOUVEAU_VM_BIND_OP_UNMAP 0x1
/**
 * @flags: the flags for a &drm_nouveau_vm_bind_op
+*
+* Supported values:
+*
+* %DRM_NOUVEAU_VM_BIND_SPARSE - Indicates that an allocated VA
+* space region should be sparse.
 */
__u32 flags;
-/**
- * @DRM_NOUVEAU_VM_BIND_SPARSE:
- *
- * Indicates that an allocated VA space region should be sparse.
- */
 #define DRM_NOUVEAU_VM_BIND_SPARSE (1 << 8)
/**
 * @handle: the handle of the DRM GEM object to map
@@ -301,17 +299,17 @@ struct drm_nouveau_vm_bind {
__u32 op_count;
/**
 * @flags: the flags for a &drm_nouveau_vm_bind ioctl
+*
+* Supported values:
+*
+* %DRM_NOUVEAU_VM_BIND_RUN_ASYNC - Indicates that the given VM_BIND
+* operation should be executed asynchronously by the kernel.
+*
+* If this flag is not supplied the kernel executes the associated
+* operations synchronously and doesn't accept any &drm_nouvea

Re: [PATCH] drm/panel: olimex-lcd-olinuxino: select CRC32

[Vegard Nossum]

Hi Thierry,

I tried sending the patch below to Stefan Mavrodiev 
(listed in MAINTAINERS for DRM DRIVER FOR OLIMEX LCD-OLINUXINO PANELS)
but it bounced with "No such user at that domain".

Will you take the patch and/or update the MAINTAINERS entry?

Thanks,


Vegard

On 10/12/21 1:52 PM, Vegard Nossum wrote:
> Fix the following build/link error by adding a dependency on the CRC32
> routines:
> 
>   ld: drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.o: in function 
> `lcd_olinuxino_probe':
>   panel-olimex-lcd-olinuxino.c:(.text+0x303): undefined reference to 
> `crc32_le'
> 
> Fixes: 17fd7a9d324fd ("drm/panel: Add support for Olimex LCD-OLinuXino panel")
> Cc: Arnd Bergmann 
> Signed-off-by: Vegard Nossum 
> ---
>  drivers/gpu/drm/panel/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index beb581b96ecdc..418638e6e3b0a 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -295,6 +295,7 @@ config DRM_PANEL_OLIMEX_LCD_OLINUXINO
>   depends on OF
>   depends on I2C
>   depends on BACKLIGHT_CLASS_DEVICE
> + select CRC32
>   help
> The panel is used with different sizes LCDs, from 480x272 to
> 1280x800, and 24 bit per pixel.
> 

[PATCH] drm/panel: olimex-lcd-olinuxino: select CRC32

[Vegard Nossum]

Fix the following build/link error by adding a dependency on the CRC32
routines:

  ld: drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.o: in function 
`lcd_olinuxino_probe':
  panel-olimex-lcd-olinuxino.c:(.text+0x303): undefined reference to `crc32_le'

Fixes: 17fd7a9d324fd ("drm/panel: Add support for Olimex LCD-OLinuXino panel")
Cc: Arnd Bergmann 
Signed-off-by: Vegard Nossum 
---
 drivers/gpu/drm/panel/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index beb581b96ecdc..418638e6e3b0a 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -295,6 +295,7 @@ config DRM_PANEL_OLIMEX_LCD_OLINUXINO
depends on OF
depends on I2C
depends on BACKLIGHT_CLASS_DEVICE
+   select CRC32
help
  The panel is used with different sizes LCDs, from 480x272 to
  1280x800, and 24 bit per pixel.
-- 
2.23.0.718.g5ad94255a8