Re: [PATCH] usb: common: add driver for USB Billboard devices

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Feb 06, 2024 at 02:56:23PM +0200, Niklas Neronin wrote:
> This patch introduces the USB Billboard Driver. Its purpose is to display,
> via debugfs, basic information about connected Billboard devices.

Very cool, I was wondering if/when someone was going to write a kernel
driver for this type of hardware.

But why debugfs?  Normally that is locked down for root-access-only by
the system (rightfully so), why is this information restricted?

And why is this a kernel driver at all?  Why can't you just do this in
userspace and add support to 'lsusb' for it?

> USB-C devices that support Alternate Modes (AUMs), such as DisplayPort
> and HDMI, can expose a simple USB 2 billboard device that describes the
> Alternate Modes the USB-C device supports. This enables users to see
> which Alternate Modes are supported by the USB-C device, even if the
> host system doesn't support them. All USB-C hosts support USB 2 devices.
> 
> The AUM information is communicated through a 'Billboard Capability
> Descriptor' and one or more 'Billboard AUM Capability Descriptors'. The
> values described in the aforementioned descriptors are exposed by this
> driver via debugfs
> 
> The driver will create a "billboards" directory within
> '/sys/kernel/debug/usb'. Each connected billboard device will have a
> corresponding file added to this "billboards" directory.
> 
> Example:
> 
> $ cat /sys/kernel/debug/usb/billboards/1-1:1.0
> Billboard:
> iAddtionalInfoURL               USB-C ADAPTOR
> bNumberOfAlternateOrUSB4Modes   1
> bPreferredAlternateOrUSB4Modes  0
> VCONNPower                      1W
> bvdVersion                      v1.21
> bAdditionalFailureInfo          0
> bReserved                       0
> 
> AUM-00:
> bwAlternateModesVdo             0x405
> bmConfigured                    AUM configuration not attempted or exited
> wSVID                           0xff01
> bAlternateOrUSB4Mode            0x0
> iAlternateOrUSB4ModeString      Generic

Looks like lsusb output, so again, why is this needed in the kernel and
only accessable by root users?

> 
> Link: https://www.usb.org/document-library/billboard-device-class-spec-revision-122-and-adopters-agreement
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>

I'm not going to actually review this patch, as you didn't follow the
rules that all Intel kernel developers need to follow, sorry.  Please
work with your Linux kernel team to do this correctly, otherwise I have
can't do anything with it even if I did want to merge it :(

thanks,

greg k-h