| Message ID | CO1PR17MB54197F118CBC8783D289B97DE1102@CO1PR17MB5419.namprd17.prod.outlook.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] usb: gadget: f_fs: add capability for dfu run-time descriptor | expand |
On Wed, Apr 24, 2024 at 10:14:58PM +0000, Chris Wulff wrote: > From: David Sands <david.sands@biamp.com> > > Add the ability for FunctionFS driver to be able to create DFU Run-Time > descriptors. Don't you need some userspace documentation for this as well? > --- a/include/uapi/linux/usb/ch9.h > +++ b/include/uapi/linux/usb/ch9.h > @@ -254,6 +254,9 @@ struct usb_ctrlrequest { > #define USB_DT_DEVICE_CAPABILITY 0x10 > #define USB_DT_WIRELESS_ENDPOINT_COMP 0x11 > #define USB_DT_WIRE_ADAPTER 0x21 > +/* From USB Device Firmware Upgrade Specification, Revision 1.1 */ > +#define USB_DT_DFU_FUNCTIONAL 0x21 So USB_DT_WIRE_ADAPTER and USB_DT_DFU_FUNCTIONAL are the same? That seems wrong. > +/* these are from the Wireless USB spec */ What spec? What "these"? > #define USB_DT_RPIPE 0x22 > #define USB_DT_CS_RADIO_CONTROL 0x23 > /* From the T10 UAS specification */ > @@ -263,6 +266,7 @@ struct usb_ctrlrequest { > /* From the USB 3.1 spec */ > #define USB_DT_SSP_ISOC_ENDPOINT_COMP 0x31 > > + > /* Conventional codes for class-specific descriptors. The convention is > * defined in the USB "Common Class" Spec (3.11). Individual class specs > * are authoritative for their usage, not the "common class" writeup. Unneeded change? > @@ -329,9 +333,10 @@ struct usb_device_descriptor { > #define USB_CLASS_USB_TYPE_C_BRIDGE 0x12 > #define USB_CLASS_MISC 0xef > #define USB_CLASS_APP_SPEC 0xfe > -#define USB_CLASS_VENDOR_SPEC 0xff > +#define USB_SUBCLASS_DFU 0x01 > > -#define USB_SUBCLASS_VENDOR_SPEC 0xff > +#define USB_CLASS_VENDOR_SPEC 0xff > +#define USB_SUBCLASS_VENDOR_SPEC 0xff Why reorder these? > > /*-------------------------------------------------------------------------*/ > > diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h > index 9f88de9c3d66..6d2061500184 100644 > --- a/include/uapi/linux/usb/functionfs.h > +++ b/include/uapi/linux/usb/functionfs.h > @@ -37,6 +37,31 @@ struct usb_endpoint_descriptor_no_audio { > __u8 bInterval; > } __attribute__((packed)); > > +/** > + * struct usb_dfu_functional_descriptor - DFU Functional descriptor > + * @bLength: Size of the descriptor (bytes) > + * @bDescriptorType: USB_DT_DFU_FUNCTIONAL > + * @bmAttributes: DFU attributes > + * @wDetachTimeOut: Maximum time to wait after DFU_DETACH (ms, le16) > + * @wTransferSize: Maximum number of bytes per control-write (le16) > + * @bcdDFUVersion: DFU Spec version (BCD, le16) > + */ > +struct usb_dfu_functional_descriptor { > + __u8 bLength; > + __u8 bDescriptorType; > + __u8 bmAttributes; > + __le16 wDetachTimeOut; > + __le16 wTransferSize; > + __le16 bcdDFUVersion; > +} __attribute__ ((packed)); > + > +/* from DFU functional descriptor bmAttributes */ > +#define DFU_FUNC_ATT_WILL_DETACH (1 << 3) > +#define DFU_FUNC_ATT_MANIFEST_TOLERANT (1 << 2) > +#define DFU_FUNC_ATT_CAN_UPLOAD (1 << 1) > +#define DFU_FUNC_ATT_CAN_DOWNLOAD (1 << 0) Please use proper BIT macros here to make this more obvious. And in sorted order? thanks, greg k-h
On Wed, Jul 31, 2024 at 4:28 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Wed, Apr 24, 2024 at 10:14:58PM +0000, Chris Wulff wrote: > > From: David Sands <david.sands@biamp.com> > > > > Add the ability for FunctionFS driver to be able to create DFU Run-Time > > descriptors. > > Don't you need some userspace documentation for this as well? Yes, I will add some. > > > --- a/include/uapi/linux/usb/ch9.h > > +++ b/include/uapi/linux/usb/ch9.h > > @@ -254,6 +254,9 @@ struct usb_ctrlrequest { > > #define USB_DT_DEVICE_CAPABILITY 0x10 > > #define USB_DT_WIRELESS_ENDPOINT_COMP 0x11 > > #define USB_DT_WIRE_ADAPTER 0x21 > > +/* From USB Device Firmware Upgrade Specification, Revision 1.1 */ > > +#define USB_DT_DFU_FUNCTIONAL 0x21 > > So USB_DT_WIRE_ADAPTER and USB_DT_DFU_FUNCTIONAL are the same? That > seems wrong. > > > +/* these are from the Wireless USB spec */ > > What spec? What "these"? I inserted the DFU constant in numerical order, splitting the original section, so I duplicated the comment. (Partly to make it obvious that there were two with the same number.) It looks like possibly wireless usb is a dead spec. You can find wireless usb v1.0 on usb.org in the wayback machine, but it looks like most of the references have been purged from the current site, and have been gone for a few years. There was apparently a v1.1 too, but I never found a copy of that anywhere. https://web.archive.org/web/20090325042850/http://www.usb.org/developers/wusb/docs/ A few references remain but say the specification is no longer available. Eg. https://www.usb.org/bos-descriptor-types The original section of code that I inserted the new constant into was: /* these are from the Wireless USB spec */ #define USB_DT_SECURITY 0x0c #define USB_DT_KEY 0x0d #define USB_DT_ENCRYPTION_TYPE 0x0e #define USB_DT_BOS 0x0f #define USB_DT_DEVICE_CAPABILITY 0x10 #define USB_DT_WIRELESS_ENDPOINT_COMP 0x11 #define USB_DT_WIRE_ADAPTER 0x21 #define USB_DT_RPIPE 0x22 #define USB_DT_CS_RADIO_CONTROL 0x23 /* From the T10 UAS specification */ > > > #define USB_DT_RPIPE 0x22 > > #define USB_DT_CS_RADIO_CONTROL 0x23 > > /* From the T10 UAS specification */ > > @@ -263,6 +266,7 @@ struct usb_ctrlrequest { > > /* From the USB 3.1 spec */ > > #define USB_DT_SSP_ISOC_ENDPOINT_COMP 0x31 > > > > + > > /* Conventional codes for class-specific descriptors. The convention is > > * defined in the USB "Common Class" Spec (3.11). Individual class specs > > * are authoritative for their usage, not the "common class" writeup. > > Unneeded change? Yeah, I'll clean that up. > > > @@ -329,9 +333,10 @@ struct usb_device_descriptor { > > #define USB_CLASS_USB_TYPE_C_BRIDGE 0x12 > > #define USB_CLASS_MISC 0xef > > #define USB_CLASS_APP_SPEC 0xfe > > -#define USB_CLASS_VENDOR_SPEC 0xff > > +#define USB_SUBCLASS_DFU 0x01 > > > > -#define USB_SUBCLASS_VENDOR_SPEC 0xff > > +#define USB_CLASS_VENDOR_SPEC 0xff > > +#define USB_SUBCLASS_VENDOR_SPEC 0xff > > Why reorder these? The subclasses are specific to the class they are under. USB_SUBCLASS_DFU is part of USB_CLASS_APP_SPEC USB_SUBCLASS_VENDOR_SPEC is part of USB_CLASS_VENDOR_SPEC > > > > > /*-------------------------------------------------------------------------*/ > > > > diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h > > index 9f88de9c3d66..6d2061500184 100644 > > --- a/include/uapi/linux/usb/functionfs.h > > +++ b/include/uapi/linux/usb/functionfs.h > > @@ -37,6 +37,31 @@ struct usb_endpoint_descriptor_no_audio { > > __u8 bInterval; > > } __attribute__((packed)); > > > > +/** > > + * struct usb_dfu_functional_descriptor - DFU Functional descriptor > > + * @bLength: Size of the descriptor (bytes) > > + * @bDescriptorType: USB_DT_DFU_FUNCTIONAL > > + * @bmAttributes: DFU attributes > > + * @wDetachTimeOut: Maximum time to wait after DFU_DETACH (ms, le16) > > + * @wTransferSize: Maximum number of bytes per control-write (le16) > > + * @bcdDFUVersion: DFU Spec version (BCD, le16) > > + */ > > +struct usb_dfu_functional_descriptor { > > + __u8 bLength; > > + __u8 bDescriptorType; > > + __u8 bmAttributes; > > + __le16 wDetachTimeOut; > > + __le16 wTransferSize; > > + __le16 bcdDFUVersion; > > +} __attribute__ ((packed)); > > + > > +/* from DFU functional descriptor bmAttributes */ > > +#define DFU_FUNC_ATT_WILL_DETACH (1 << 3) > > +#define DFU_FUNC_ATT_MANIFEST_TOLERANT (1 << 2) > > +#define DFU_FUNC_ATT_CAN_UPLOAD (1 << 1) > > +#define DFU_FUNC_ATT_CAN_DOWNLOAD (1 << 0) > > Please use proper BIT macros here to make this more obvious. And in > sorted order? Ok. I'll fix this up > > thanks, > > greg k-h >
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index bffbc1dc651f..4cc3f3601cf0 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -2467,7 +2467,7 @@ typedef int (*ffs_os_desc_callback)(enum ffs_os_desc_type entity, static int __must_check ffs_do_single_desc(char *data, unsigned len, ffs_entity_callback entity, - void *priv, int *current_class) + void *priv, int *current_class, int *current_subclass) { struct usb_descriptor_header *_ds = (void *)data; u8 length; @@ -2524,6 +2524,7 @@ static int __must_check ffs_do_single_desc(char *data, unsigned len, if (ds->iInterface) __entity(STRING, ds->iInterface); *current_class = ds->bInterfaceClass; + *current_subclass = ds->bInterfaceSubClass; } break; @@ -2548,6 +2549,12 @@ static int __must_check ffs_do_single_desc(char *data, unsigned len, if (length != sizeof(struct ccid_descriptor)) goto inv_length; break; + } else if (*current_class == USB_CLASS_APP_SPEC && + *current_subclass == USB_SUBCLASS_DFU) { + pr_vdebug("dfu functional descriptor\n"); + if (length != sizeof(struct usb_dfu_functional_descriptor)) + goto inv_length; + break; } else { pr_vdebug("unknown descriptor: %d for class %d\n", _ds->bDescriptorType, *current_class); @@ -2610,6 +2617,7 @@ static int __must_check ffs_do_descs(unsigned count, char *data, unsigned len, const unsigned _len = len; unsigned long num = 0; int current_class = -1; + int current_subclass = -1; for (;;) { int ret; @@ -2629,7 +2637,7 @@ static int __must_check ffs_do_descs(unsigned count, char *data, unsigned len, return _len - len; ret = ffs_do_single_desc(data, len, entity, priv, - ¤t_class); + ¤t_class, ¤t_subclass); if (ret < 0) { pr_debug("%s returns %d\n", __func__, ret); return ret; diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h index 44d73ba8788d..7f155fba0c1f 100644 --- a/include/uapi/linux/usb/ch9.h +++ b/include/uapi/linux/usb/ch9.h @@ -254,6 +254,9 @@ struct usb_ctrlrequest { #define USB_DT_DEVICE_CAPABILITY 0x10 #define USB_DT_WIRELESS_ENDPOINT_COMP 0x11 #define USB_DT_WIRE_ADAPTER 0x21 +/* From USB Device Firmware Upgrade Specification, Revision 1.1 */ +#define USB_DT_DFU_FUNCTIONAL 0x21 +/* these are from the Wireless USB spec */ #define USB_DT_RPIPE 0x22 #define USB_DT_CS_RADIO_CONTROL 0x23 /* From the T10 UAS specification */ @@ -263,6 +266,7 @@ struct usb_ctrlrequest { /* From the USB 3.1 spec */ #define USB_DT_SSP_ISOC_ENDPOINT_COMP 0x31 + /* Conventional codes for class-specific descriptors. The convention is * defined in the USB "Common Class" Spec (3.11). Individual class specs * are authoritative for their usage, not the "common class" writeup. @@ -329,9 +333,10 @@ struct usb_device_descriptor { #define USB_CLASS_USB_TYPE_C_BRIDGE 0x12 #define USB_CLASS_MISC 0xef #define USB_CLASS_APP_SPEC 0xfe -#define USB_CLASS_VENDOR_SPEC 0xff +#define USB_SUBCLASS_DFU 0x01 -#define USB_SUBCLASS_VENDOR_SPEC 0xff +#define USB_CLASS_VENDOR_SPEC 0xff +#define USB_SUBCLASS_VENDOR_SPEC 0xff /*-------------------------------------------------------------------------*/ diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h index 9f88de9c3d66..6d2061500184 100644 --- a/include/uapi/linux/usb/functionfs.h +++ b/include/uapi/linux/usb/functionfs.h @@ -37,6 +37,31 @@ struct usb_endpoint_descriptor_no_audio { __u8 bInterval; } __attribute__((packed)); +/** + * struct usb_dfu_functional_descriptor - DFU Functional descriptor + * @bLength: Size of the descriptor (bytes) + * @bDescriptorType: USB_DT_DFU_FUNCTIONAL + * @bmAttributes: DFU attributes + * @wDetachTimeOut: Maximum time to wait after DFU_DETACH (ms, le16) + * @wTransferSize: Maximum number of bytes per control-write (le16) + * @bcdDFUVersion: DFU Spec version (BCD, le16) + */ +struct usb_dfu_functional_descriptor { + __u8 bLength; + __u8 bDescriptorType; + __u8 bmAttributes; + __le16 wDetachTimeOut; + __le16 wTransferSize; + __le16 bcdDFUVersion; +} __attribute__ ((packed)); + +/* from DFU functional descriptor bmAttributes */ +#define DFU_FUNC_ATT_WILL_DETACH (1 << 3) +#define DFU_FUNC_ATT_MANIFEST_TOLERANT (1 << 2) +#define DFU_FUNC_ATT_CAN_UPLOAD (1 << 1) +#define DFU_FUNC_ATT_CAN_DOWNLOAD (1 << 0) + + struct usb_functionfs_descs_head_v2 { __le32 magic; __le32 length;