diff mbox series

[v4] usb: gadget: f_fs: add capability for dfu run-time descriptor

Message ID 20240805000639.619232-2-crwulff@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v4] usb: gadget: f_fs: add capability for dfu run-time descriptor | expand

Commit Message

Chris Wulff Aug. 5, 2024, 12:06 a.m. UTC
From: David Sands <david.sands@biamp.com>

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>
Co-developed-by: Chris Wulff <crwulff@gmail.com>
Signed-off-by: Chris Wulff <crwulff@gmail.com>
---
v4: Clean up unneeded change, switch to BIT macros, more documentation
v3: Documentation, additional constants and constant order fixed
https://lore.kernel.org/all/CO1PR17MB54197F118CBC8783D289B97DE1102@CO1PR17MB5419.namprd17.prod.outlook.com/
v2: https://lore.kernel.org/linux-usb/CO1PR17MB54198D086B61F7392FA9075FE10E2@CO1PR17MB5419.namprd17.prod.outlook.com/
v1: https://lore.kernel.org/linux-usb/CO1PR17MB5419AC3907C74E28D80C5021E1082@CO1PR17MB5419.namprd17.prod.outlook.com/
---
 Documentation/usb/functionfs-desc.rst | 22 ++++++++++++++++++++++
 Documentation/usb/functionfs.rst      |  2 ++
 Documentation/usb/index.rst           |  1 +
 drivers/usb/gadget/function/f_fs.c    | 12 ++++++++++--
 include/uapi/linux/usb/ch9.h          |  8 ++++++--
 include/uapi/linux/usb/functionfs.h   | 25 +++++++++++++++++++++++++
 6 files changed, 66 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/usb/functionfs-desc.rst

Comments

Matthew Wilcox (Oracle) Aug. 5, 2024, 3:39 a.m. UTC | #1
On Sun, Aug 04, 2024 at 08:06:40PM -0400, crwulff@gmail.com wrote:
> diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
> index 44d73ba8788d..91f0f7e214a5 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

This is the only place in the entire patch where you explain what "DFU"
means.  Is this really such a well-known acronym that it doesn't need to
be in the documentation or the commit message?
Greg Kroah-Hartman Aug. 5, 2024, 7:01 a.m. UTC | #2
On Sun, Aug 04, 2024 at 08:06:40PM -0400, crwulff@gmail.com wrote:
> From: David Sands <david.sands@biamp.com>
> 
> From: David Sands <david.sands@biamp.com>

Twice?

> Add the ability for FunctionFS driver to be able to create DFU Run-Time
> descriptors.

As others said, please spell out "DFU" and I do not think that
"Run-Time" needs Capital letters, or a '-', right?

Also include here a lot more description of how this is to be used.

> 
> Signed-off-by: David Sands <david.sands@biamp.com>
> Co-developed-by: Chris Wulff <crwulff@gmail.com>
> Signed-off-by: Chris Wulff <crwulff@gmail.com>
> ---
> v4: Clean up unneeded change, switch to BIT macros, more documentation
> v3: Documentation, additional constants and constant order fixed
> https://lore.kernel.org/all/CO1PR17MB54197F118CBC8783D289B97DE1102@CO1PR17MB5419.namprd17.prod.outlook.com/
> v2: https://lore.kernel.org/linux-usb/CO1PR17MB54198D086B61F7392FA9075FE10E2@CO1PR17MB5419.namprd17.prod.outlook.com/
> v1: https://lore.kernel.org/linux-usb/CO1PR17MB5419AC3907C74E28D80C5021E1082@CO1PR17MB5419.namprd17.prod.outlook.com/
> ---
>  Documentation/usb/functionfs-desc.rst | 22 ++++++++++++++++++++++
>  Documentation/usb/functionfs.rst      |  2 ++
>  Documentation/usb/index.rst           |  1 +
>  drivers/usb/gadget/function/f_fs.c    | 12 ++++++++++--
>  include/uapi/linux/usb/ch9.h          |  8 ++++++--
>  include/uapi/linux/usb/functionfs.h   | 25 +++++++++++++++++++++++++
>  6 files changed, 66 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/usb/functionfs-desc.rst
> 
> diff --git a/Documentation/usb/functionfs-desc.rst b/Documentation/usb/functionfs-desc.rst
> new file mode 100644
> index 000000000000..73d2b8a3f02c
> --- /dev/null
> +++ b/Documentation/usb/functionfs-desc.rst
> @@ -0,0 +1,22 @@
> +======================
> +FunctionFS Descriptors
> +======================
> +
> +Interface Descriptors
> +---------------------
> +
> +Standard USB interface descriptors may be added. The class/subclass of the
> +most recent interface descriptor determines what type of class-specific
> +descriptors are accepted.
> +
> +Class-Specific Descriptors
> +--------------------------
> +

Why an empty section?

> +DFU Functional Descriptor
> +~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +When the interface class is USB_CLASS_APP_SPEC and  the interface subclass

Extra space?


> +is USB_SUBCLASS_DFU, a DFU functional descriptor can be provided.

Provided how?

> +
> +.. kernel-doc:: include/uapi/linux/usb/functionfs.h
> +   :doc: usb_dfu_functional_descriptor
> diff --git a/Documentation/usb/functionfs.rst b/Documentation/usb/functionfs.rst
> index d05a775bc45b..4f96e4b93d7b 100644
> --- a/Documentation/usb/functionfs.rst
> +++ b/Documentation/usb/functionfs.rst
> @@ -70,6 +70,8 @@ have been written to their ep0's.
>  Conversely, the gadget is unregistered after the first USB function
>  closes its endpoints.
>  
> +For more information about FunctionFS descriptors see :doc:`functionfs-desc`
> +
>  DMABUF interface
>  ================
>  
> diff --git a/Documentation/usb/index.rst b/Documentation/usb/index.rst
> index 27955dad95e1..826492c813ac 100644
> --- a/Documentation/usb/index.rst
> +++ b/Documentation/usb/index.rst
> @@ -11,6 +11,7 @@ USB support
>      dwc3
>      ehci
>      functionfs
> +    functionfs-desc

That's an odd name for a DFU-specific file, right?

Where are the Documentation/ABI/ entries?

>      gadget_configfs
>      gadget_hid
>      gadget_multi
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index d8b096859337..ba5c6e4827ba 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -2478,7 +2478,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;
> @@ -2535,6 +2535,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;
>  
> @@ -2559,6 +2560,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);
> @@ -2621,6 +2628,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;
> @@ -2640,7 +2648,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,
> -			&current_class);
> +			&current_class, &current_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..91f0f7e214a5 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 */
> @@ -329,9 +332,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..40f87cbabf7a 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_CAN_DOWNLOAD	BIT(0)
> +#define DFU_FUNC_ATT_CAN_UPLOAD		BIT(1)
> +#define DFU_FUNC_ATT_MANIFEST_TOLERANT	BIT(2)
> +#define DFU_FUNC_ATT_WILL_DETACH	BIT(3)

Wrong macro for bit fields for uapi .h files :(

thanks,

greg k-h
Chris Wulff Aug. 5, 2024, 11:39 p.m. UTC | #3
On Sun, Aug 4, 2024 at 11:39 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sun, Aug 04, 2024 at 08:06:40PM -0400, crwulff@gmail.com wrote:
> > diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
> > index 44d73ba8788d..91f0f7e214a5 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
>
> This is the only place in the entire patch where you explain what "DFU"
> means.  Is this really such a well-known acronym that it doesn't need to
> be in the documentation or the commit message?

Yeah, it would probably be good to add that to the documentation. I'll
add that info.

(It stands for "Device Firmware Upgrade")

  -- Chris Wulff
Chris Wulff Aug. 6, 2024, 12:21 a.m. UTC | #4
On Mon, Aug 5, 2024 at 3:01 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, Aug 04, 2024 at 08:06:40PM -0400, crwulff@gmail.com wrote:
> > From: David Sands <david.sands@biamp.com>
> >
> > From: David Sands <david.sands@biamp.com>
>
> Twice?

Oops. Not sure what happened, but I'll try to make it not happen next time.

>
> > Add the ability for FunctionFS driver to be able to create DFU Run-Time
> > descriptors.
>
> As others said, please spell out "DFU" and I do not think that
> "Run-Time" needs Capital letters, or a '-', right?
>
> Also include here a lot more description of how this is to be used.

Ok, I will expand on this (and the associated documentation.)

>
> >
> > Signed-off-by: David Sands <david.sands@biamp.com>
> > Co-developed-by: Chris Wulff <crwulff@gmail.com>
> > Signed-off-by: Chris Wulff <crwulff@gmail.com>
> > ---
> > v4: Clean up unneeded change, switch to BIT macros, more documentation
> > v3: Documentation, additional constants and constant order fixed
> > https://lore.kernel.org/all/CO1PR17MB54197F118CBC8783D289B97DE1102@CO1PR17MB5419.namprd17.prod.outlook.com/
> > v2: https://lore.kernel.org/linux-usb/CO1PR17MB54198D086B61F7392FA9075FE10E2@CO1PR17MB5419.namprd17.prod.outlook.com/
> > v1: https://lore.kernel.org/linux-usb/CO1PR17MB5419AC3907C74E28D80C5021E1082@CO1PR17MB5419.namprd17.prod.outlook.com/
> > ---
> >  Documentation/usb/functionfs-desc.rst | 22 ++++++++++++++++++++++
> >  Documentation/usb/functionfs.rst      |  2 ++
> >  Documentation/usb/index.rst           |  1 +
> >  drivers/usb/gadget/function/f_fs.c    | 12 ++++++++++--
> >  include/uapi/linux/usb/ch9.h          |  8 ++++++--
> >  include/uapi/linux/usb/functionfs.h   | 25 +++++++++++++++++++++++++
> >  6 files changed, 66 insertions(+), 4 deletions(-)
> >  create mode 100644 Documentation/usb/functionfs-desc.rst
> >
> > diff --git a/Documentation/usb/functionfs-desc.rst b/Documentation/usb/functionfs-desc.rst
> > new file mode 100644
> > index 000000000000..73d2b8a3f02c
> > --- /dev/null
> > +++ b/Documentation/usb/functionfs-desc.rst
> > @@ -0,0 +1,22 @@
> > +======================
> > +FunctionFS Descriptors
> > +======================
> > +
> > +Interface Descriptors
> > +---------------------
> > +
> > +Standard USB interface descriptors may be added. The class/subclass of the
> > +most recent interface descriptor determines what type of class-specific
> > +descriptors are accepted.
> > +
> > +Class-Specific Descriptors
> > +--------------------------
> > +
>
> Why an empty section?

It was just a heading-2 for the class-specific descriptor section (with each
of the class-specific descriptors being heading-3). I can add a bit of
text though.

>
> > +DFU Functional Descriptor
> > +~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +When the interface class is USB_CLASS_APP_SPEC and  the interface subclass
>
> Extra space?
>
>
> > +is USB_SUBCLASS_DFU, a DFU functional descriptor can be provided.
>
> Provided how?

I will expand on this a bit more. Most of the functionfs descriptor
behavior wasn't documented. The functionfs page talks about how
these are written to the ep0 file, but doesn't mention anything about
what descriptors can be written other than mentioning that ep# files
are created when endpoint descriptors are written.

>
> > +
> > +.. kernel-doc:: include/uapi/linux/usb/functionfs.h
> > +   :doc: usb_dfu_functional_descriptor
> > diff --git a/Documentation/usb/functionfs.rst b/Documentation/usb/functionfs.rst
> > index d05a775bc45b..4f96e4b93d7b 100644
> > --- a/Documentation/usb/functionfs.rst
> > +++ b/Documentation/usb/functionfs.rst
> > @@ -70,6 +70,8 @@ have been written to their ep0's.
> >  Conversely, the gadget is unregistered after the first USB function
> >  closes its endpoints.
> >
> > +For more information about FunctionFS descriptors see :doc:`functionfs-desc`
> > +
> >  DMABUF interface
> >  ================
> >
> > diff --git a/Documentation/usb/index.rst b/Documentation/usb/index.rst
> > index 27955dad95e1..826492c813ac 100644
> > --- a/Documentation/usb/index.rst
> > +++ b/Documentation/usb/index.rst
> > @@ -11,6 +11,7 @@ USB support
> >      dwc3
> >      ehci
> >      functionfs
> > +    functionfs-desc
>
> That's an odd name for a DFU-specific file, right?
>
> Where are the Documentation/ABI/ entries?

functionfs-desc was intended to be for more than DFU. I was thinking
it would be nice to also talk about other descriptors that can be
written to functionfs since I couldn't find any documentation on
that, but I didn't want to add documentation for a bunch of existing
stuff to this same patch. It seems like that would be better submitted
separately (which I can work on if you think it's useful.) I only included
the descriptors that were relevant to DFU.

I will see what I can add for the ABI documentation as well.

>
> >      gadget_configfs
> >      gadget_hid
> >      gadget_multi
> > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> > index d8b096859337..ba5c6e4827ba 100644
> > --- a/drivers/usb/gadget/function/f_fs.c
> > +++ b/drivers/usb/gadget/function/f_fs.c
> > @@ -2478,7 +2478,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;
> > @@ -2535,6 +2535,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;
> >
> > @@ -2559,6 +2560,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);
> > @@ -2621,6 +2628,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;
> > @@ -2640,7 +2648,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,
> > -                     &current_class);
> > +                     &current_class, &current_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..91f0f7e214a5 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 */
> > @@ -329,9 +332,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..40f87cbabf7a 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_CAN_DOWNLOAD    BIT(0)
> > +#define DFU_FUNC_ATT_CAN_UPLOAD              BIT(1)
> > +#define DFU_FUNC_ATT_MANIFEST_TOLERANT       BIT(2)
> > +#define DFU_FUNC_ATT_WILL_DETACH     BIT(3)
>
> Wrong macro for bit fields for uapi .h files :(

Oh. I'm surprised there is more than one macro for bits. I will
change this to _BITUL().

>
> thanks,
>
> greg k-h
diff mbox series

Patch

diff --git a/Documentation/usb/functionfs-desc.rst b/Documentation/usb/functionfs-desc.rst
new file mode 100644
index 000000000000..73d2b8a3f02c
--- /dev/null
+++ b/Documentation/usb/functionfs-desc.rst
@@ -0,0 +1,22 @@ 
+======================
+FunctionFS Descriptors
+======================
+
+Interface Descriptors
+---------------------
+
+Standard USB interface descriptors may be added. The class/subclass of the
+most recent interface descriptor determines what type of class-specific
+descriptors are accepted.
+
+Class-Specific Descriptors
+--------------------------
+
+DFU Functional Descriptor
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+When the interface class is USB_CLASS_APP_SPEC and  the interface subclass
+is USB_SUBCLASS_DFU, a DFU functional descriptor can be provided.
+
+.. kernel-doc:: include/uapi/linux/usb/functionfs.h
+   :doc: usb_dfu_functional_descriptor
diff --git a/Documentation/usb/functionfs.rst b/Documentation/usb/functionfs.rst
index d05a775bc45b..4f96e4b93d7b 100644
--- a/Documentation/usb/functionfs.rst
+++ b/Documentation/usb/functionfs.rst
@@ -70,6 +70,8 @@  have been written to their ep0's.
 Conversely, the gadget is unregistered after the first USB function
 closes its endpoints.
 
+For more information about FunctionFS descriptors see :doc:`functionfs-desc`
+
 DMABUF interface
 ================
 
diff --git a/Documentation/usb/index.rst b/Documentation/usb/index.rst
index 27955dad95e1..826492c813ac 100644
--- a/Documentation/usb/index.rst
+++ b/Documentation/usb/index.rst
@@ -11,6 +11,7 @@  USB support
     dwc3
     ehci
     functionfs
+    functionfs-desc
     gadget_configfs
     gadget_hid
     gadget_multi
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index d8b096859337..ba5c6e4827ba 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -2478,7 +2478,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;
@@ -2535,6 +2535,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;
 
@@ -2559,6 +2560,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);
@@ -2621,6 +2628,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;
@@ -2640,7 +2648,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,
-			&current_class);
+			&current_class, &current_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..91f0f7e214a5 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 */
@@ -329,9 +332,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..40f87cbabf7a 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_CAN_DOWNLOAD	BIT(0)
+#define DFU_FUNC_ATT_CAN_UPLOAD		BIT(1)
+#define DFU_FUNC_ATT_MANIFEST_TOLERANT	BIT(2)
+#define DFU_FUNC_ATT_WILL_DETACH	BIT(3)
+
+
 struct usb_functionfs_descs_head_v2 {
 	__le32 magic;
 	__le32 length;