On Mon, Mar 04, 2024 at 04:23:01PM -0800, Elbert Mai wrote: > Motivation > ---------- > > The binary device object store (BOS) of a USB device consists of the BOS > descriptor followed by a set of device capability descriptors. One that is > of interest to users is the platform descriptor. This contains a 128-bit > UUID and arbitrary data, and it allows parties outside of USB-IF to add > additional metadata about a USB device in a standards-compliant manner. > Notable examples include the WebUSB and Microsoft OS 2.0 descriptors. > > The kernel already retrieves and caches the BOS from USB devices if its > bcdUSB is >= 0x0201. Because the BOS is flexible and extensible, we export > the entire BOS to sysfs so users can retrieve whatever device capabilities > they desire, without requiring USB I/O or elevated permissions. > > Implementation > -------------- > > Add bos_descriptors attribute to sysfs. This is a binary file and it works > the same way as the existing descriptors attribute. The file exists only if > the BOS is present in the USB device. > > Also create a binary attribute group, so the driver core can handle the > creation of both the descriptors and bos_descriptors attributes in sysfs. > > Signed-off-by: Elbert Mai <code@xxxxxxxxxxxxx> > --- > Changes in v2: > - Rename to bos_descriptors (plural) since the attribute contains the > entire BOS, not just the first descriptor in it. > - Use binary attribute groups to let driver core handle attribute > creation for both descriptors and bos_descriptors. > - The attribute is visible in sysfs only if the BOS is present in the > USB device. Very nice, thanks for this! One very minor comment, you can send a follow-on patch for this if you like: > +static umode_t dev_bin_attrs_are_visible(struct kobject *kobj, > + struct bin_attribute *a, int n) > +{ > + struct device *dev = kobj_to_dev(kobj); > + struct usb_device *udev = to_usb_device(dev); > + > + /* All USB devices have a device descriptor, so the descriptors > + * attribute always exists. No need to check for its visibility. > + */ This comment is in the "wrong" style, I think checkpatch will complain about that, right? But it's a bit confusing, you say "no need to check", and then you: > + if (a == &bin_attr_bos_descriptors) { > + if (udev->bos == NULL) > + return 0; > + } check something :) How about this as a comment instead: /* * If this is the BOS descriptor, check to verify if the device * has that descriptor at all or not. */ That's all you need here, right? Anyway, again, very nice, I'll go queue this up and run it through the 0-day tests. thanks! greg k -h