[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2024073122-shakable-photo-67d1@gregkh>
Date: Wed, 31 Jul 2024 10:27:59 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Chris Wulff <Chris.Wulff@...mp.com>
Cc: "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
Christian Brauner <brauner@...nel.org>,
Paul Cercueil <paul@...pouillou.net>, Jan Kara <jack@...e.cz>,
Jeff Layton <jlayton@...nel.org>,
Dmitry Antipov <dmantipov@...dex.ru>,
David Sands <david.sands@...mp.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>
Subject: Re: [PATCH v3] usb: gadget: f_fs: add capability for dfu run-time
descriptor
On Wed, Apr 24, 2024 at 10:14:58PM +0000, Chris Wulff wrote:
> From: David Sands <david.sands@...mp.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
Powered by blists - more mailing lists