| Message ID | CO1PR17MB54198D086B61F7392FA9075FE10E2@CO1PR17MB5419.namprd17.prod.outlook.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: gadget: f_fs: add capability for dfu run-time descriptor | expand |
On Thu, Apr 18, 2024 at 08:39:03PM +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. > > Signed-off-by: David Sands <david.sands@biamp.com> Why didn't you also cc: them? > Co-developed-by: Chris Wulff <chris.wulff@biamp.com> > Signed-off-by: Chris Wulff <chris.wulff@biamp.com> > > --- > v2: Fixed attribution and signoff > > drivers/usb/gadget/function/f_fs.c | 11 +++++++++-- > include/uapi/linux/usb/ch9.h | 4 ++++ > include/uapi/linux/usb/functionfs.h | 10 ++++++++++ > 3 files changed, 23 insertions(+), 2 deletions(-) Does this require a Documentation/ABI/ update? > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > index bffbc1dc651f..4d39e5e30b73 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,11 @@ 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 +2616,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 +2636,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..dcd962d1a75a 100644 > --- a/include/uapi/linux/usb/ch9.h > +++ b/include/uapi/linux/usb/ch9.h > @@ -263,6 +263,9 @@ struct usb_ctrlrequest { > /* From the USB 3.1 spec */ > #define USB_DT_SSP_ISOC_ENDPOINT_COMP 0x31 > > +/* From USB Device Firmware Upgrade Specification, Revision 1.1 */ > +#define USB_DT_DFU_FUNCTIONAL 0x21 Why is this not in sorted order? And it really conflicts with USB_DT_WIRE_ADAPTER? That seems odd given that DFU came first. Hm, it is that value, odd. > + > /* 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. > @@ -332,6 +335,7 @@ struct usb_device_descriptor { > #define USB_CLASS_VENDOR_SPEC 0xff > > #define USB_SUBCLASS_VENDOR_SPEC 0xff > +#define USB_SUBCLASS_DFU 0x01 This should be up next to the other DFU stuff, right? WHat about the DFU interface type? > > /*-------------------------------------------------------------------------*/ > > diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h > index 9f88de9c3d66..cf3f55234a5e 100644 > --- a/include/uapi/linux/usb/functionfs.h > +++ b/include/uapi/linux/usb/functionfs.h > @@ -37,6 +37,16 @@ struct usb_endpoint_descriptor_no_audio { > __u8 bInterval; > } __attribute__((packed)); > > +/* DFU Functional descriptor */ > +struct usb_dfu_functional_descriptor { > + __u8 bLength; > + __u8 bDescriptorType; > + __u8 bmAttributes; Do you need #defines for the different attribute bits that DFU wants here? thanks, greg k-h
From: Greg KH <gregkh@linuxfoundation.org> >> drivers/usb/gadget/function/f_fs.c | 11 +++++++++-- >> include/uapi/linux/usb/ch9.h | 4 ++++ >> include/uapi/linux/usb/functionfs.h | 10 ++++++++++ >> 3 files changed, 23 insertions(+), 2 deletions(-) > > Does this require a Documentation/ABI/ update? I haven't found anything in Documentation that talks about any of these structures or constants. I will add kerneldoc sytle comments for the fields in the usb_dfu_functional_descriptor structure though. >> diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h >> index 44d73ba8788d..dcd962d1a75a 100644 >> --- a/include/uapi/linux/usb/ch9.h >> +++ b/include/uapi/linux/usb/ch9.h >> @@ -263,6 +263,9 @@ struct usb_ctrlrequest { >> /* From the USB 3.1 spec */ >> #define USB_DT_SSP_ISOC_ENDPOINT_COMP 0x31 >> >> +/* From USB Device Firmware Upgrade Specification, Revision 1.1 */ >> +#define USB_DT_DFU_FUNCTIONAL 0x21 > >Why is this not in sorted order? > I will move it up so it is in order. >And it really conflicts with USB_DT_WIRE_ADAPTER? That seems odd given >that DFU came first. Hm, it is that value, odd. > >> + >> /* 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. >> @@ -332,6 +335,7 @@ struct usb_device_descriptor { >> #define USB_CLASS_VENDOR_SPEC 0xff >> >> #define USB_SUBCLASS_VENDOR_SPEC 0xff >> +#define USB_SUBCLASS_DFU 0x01 > >This should be up next to the other DFU stuff, right? WHat about the >DFU interface type? I will re-group USB_SUBCLASS_DFU so it is associated with its corresponding class code (USB_CLASS_APP_SPEC). The DFU interface descriptor just uses the standard usb_interface_descriptor structure so nothing new needed for that. >> >> /*-------------------------------------------------------------------------*/ >> >> diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h >> index 9f88de9c3d66..cf3f55234a5e 100644 >> --- a/include/uapi/linux/usb/functionfs.h >> +++ b/include/uapi/linux/usb/functionfs.h >> @@ -37,6 +37,16 @@ struct usb_endpoint_descriptor_no_audio { >> __u8 bInterval; >> } __attribute__((packed)); >> >> +/* DFU Functional descriptor */ >> +struct usb_dfu_functional_descriptor { >> + __u8 bLength; >> + __u8 bDescriptorType; >> + __u8 bmAttributes; > >Do you need #defines for the different attribute bits that DFU wants >here? I have them defined in the associated userspace program, but I will include them here instead. The kernel doesn't need any of those constants, but I see a few other structs that have bmAttributes related defines below them. -- Chris Wulff
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index bffbc1dc651f..4d39e5e30b73 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,11 @@ 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 +2616,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 +2636,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..dcd962d1a75a 100644 --- a/include/uapi/linux/usb/ch9.h +++ b/include/uapi/linux/usb/ch9.h @@ -263,6 +263,9 @@ struct usb_ctrlrequest { /* From the USB 3.1 spec */ #define USB_DT_SSP_ISOC_ENDPOINT_COMP 0x31 +/* From USB Device Firmware Upgrade Specification, Revision 1.1 */ +#define USB_DT_DFU_FUNCTIONAL 0x21 + /* 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. @@ -332,6 +335,7 @@ struct usb_device_descriptor { #define USB_CLASS_VENDOR_SPEC 0xff #define USB_SUBCLASS_VENDOR_SPEC 0xff +#define USB_SUBCLASS_DFU 0x01 /*-------------------------------------------------------------------------*/ diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h index 9f88de9c3d66..cf3f55234a5e 100644 --- a/include/uapi/linux/usb/functionfs.h +++ b/include/uapi/linux/usb/functionfs.h @@ -37,6 +37,16 @@ struct usb_endpoint_descriptor_no_audio { __u8 bInterval; } __attribute__((packed)); +/* DFU Functional descriptor */ +struct usb_dfu_functional_descriptor { + __u8 bLength; + __u8 bDescriptorType; + __u8 bmAttributes; + __le16 wDetachTimeOut; + __le16 wTransferSize; + __le16 bcdDFUVersion; +} __attribute__ ((packed)); + struct usb_functionfs_descs_head_v2 { __le32 magic; __le32 length;