| Message ID | 20230125143425.85268-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2bf40502badf9bc15a487244dd23ce0b08c306c0 |
| Headers | show |
| Series | [v1,1/1] usb: gadget: Use correct APIs and data types for UUID handling | expand |
On Wed, Jan 25, 2023 at 3:34 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > We have two types for UUIDs depending on the byte ordering. > Instead of explaining how bytes should go over the wire, > use dedicated APIs and data types. This removes a confusion > over the byte ordering. Thanks for pointing this out. I was unaware of the exact UUID functions, as I'm still quite a newbie here. I compiled and tested your patch in my test setup and it works perfectly. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/usb/gadget/composite.c | 4 ++-- > include/linux/usb/webusb.h | 9 +++------ > 2 files changed, 5 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c > index 8e2603688016..fa7dd6cf014d 100644 > --- a/drivers/usb/gadget/composite.c > +++ b/drivers/usb/gadget/composite.c > @@ -829,7 +829,7 @@ static int bos_desc(struct usb_composite_dev *cdev) > if (cdev->use_webusb) { > struct usb_plat_dev_cap_descriptor *webusb_cap; > struct usb_webusb_cap_data *webusb_cap_data; > - uuid_t webusb_uuid = WEBUSB_UUID; > + guid_t webusb_uuid = WEBUSB_UUID; > > webusb_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength); > webusb_cap_data = (struct usb_webusb_cap_data *) webusb_cap->CapabilityData; > @@ -841,7 +841,7 @@ static int bos_desc(struct usb_composite_dev *cdev) > webusb_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY; > webusb_cap->bDevCapabilityType = USB_PLAT_DEV_CAP_TYPE; > webusb_cap->bReserved = 0; > - export_uuid(webusb_cap->UUID, &webusb_uuid); > + export_guid(webusb_cap->UUID, &webusb_uuid); > > if (cdev->bcd_webusb_version != 0) > webusb_cap_data->bcdVersion = cpu_to_le16(cdev->bcd_webusb_version); > diff --git a/include/linux/usb/webusb.h b/include/linux/usb/webusb.h > index b430d84357f3..fe43020b4a48 100644 > --- a/include/linux/usb/webusb.h > +++ b/include/linux/usb/webusb.h > @@ -11,15 +11,12 @@ > #include "uapi/linux/usb/ch9.h" > > /* > - * little endian PlatformCapablityUUID for WebUSB > + * Little Endian PlatformCapablityUUID for WebUSB > * 3408b638-09a9-47a0-8bfd-a0768815b665 > - * to identify Platform Device Capability descriptors as referring to WebUSB > - * > - * the UUID above MUST be sent over the wire as the byte sequence: > - * {0x38, 0xB6, 0x08, 0x34, 0xA9, 0x09, 0xA0, 0x47, 0x8B, 0xFD, 0xA0, 0x76, 0x88, 0x15, 0xB6, 0x65}. This is actually explicitly spelled out this way in the specification: https://wicg.github.io/webusb/#webusb-platform-capability-descriptor But I agree, the way you rewrote it is much clearer! > + * to identify Platform Device Capability descriptors as referring to WebUSB. > */ > #define WEBUSB_UUID \ > - UUID_INIT(0x38b60834, 0xa909, 0xa047, 0x8b, 0xfd, 0xa0, 0x76, 0x88, 0x15, 0xb6, 0x65) > + GUID_INIT(0x3408b638, 0x09a9, 0x47a0, 0x8b, 0xfd, 0xa0, 0x76, 0x88, 0x15, 0xb6, 0x65) Yes, this is definitely more readable. > > /* > * WebUSB Platform Capability data > -- > 2.39.0 >
On Wed, Jan 25, 2023 at 06:31:36PM +0100, Jó Ágila Bitsch wrote: > On Wed, Jan 25, 2023 at 3:34 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > We have two types for UUIDs depending on the byte ordering. > > Instead of explaining how bytes should go over the wire, > > use dedicated APIs and data types. This removes a confusion > > over the byte ordering. > > Thanks for pointing this out. I was unaware of the exact UUID > functions, as I'm still quite a newbie here. > > I compiled and tested your patch in my test setup and it works perfectly. Thanks for the testing. According to Submitting Patches documentation you can provide a formal Tested-by tag.
On Wed, Jan 25, 2023 at 9:01 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Jan 25, 2023 at 06:31:36PM +0100, Jó Ágila Bitsch wrote: > > On Wed, Jan 25, 2023 at 3:34 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > We have two types for UUIDs depending on the byte ordering. > > > Instead of explaining how bytes should go over the wire, > > > use dedicated APIs and data types. This removes a confusion > > > over the byte ordering. > > > > Thanks for pointing this out. I was unaware of the exact UUID > > functions, as I'm still quite a newbie here. > > > > I compiled and tested your patch in my test setup and it works perfectly. > > Thanks for the testing. According to Submitting Patches documentation > you can provide a formal Tested-by tag. Thanks for pointing this out to me. I'm not really sure how to do that though. On https://docs.kernel.org/process/submitting-patches.html#reviewer-s-statement-of-oversight, it says: > Both Tested-by and Reviewed-by tags, once received on mailing list from tester or reviewer, should be added by author to the applicable patches when sending next versions. So I guess you could do that at your convenience on any next version. Or is it already ok, if I just add the following line in my comment? Tested-By: Jó Ágila Bitsch <jgilab@gmail.com> I'm still quite a newbie in the kernel development community, so thanks for bearing with my ignorance :-) Best regards and thanks a lot, Jó > > -- > With Best Regards, > Andy Shevchenko > >
On Wed, Jan 25, 2023 at 10:54:58PM +0100, Jó Ágila Bitsch wrote: > On Wed, Jan 25, 2023 at 9:01 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Jan 25, 2023 at 06:31:36PM +0100, Jó Ágila Bitsch wrote: > > > On Wed, Jan 25, 2023 at 3:34 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: ... > > > I compiled and tested your patch in my test setup and it works perfectly. > > > > Thanks for the testing. According to Submitting Patches documentation > > you can provide a formal Tested-by tag. > > Thanks for pointing this out to me. > > I'm not really sure how to do that though. > On https://docs.kernel.org/process/submitting-patches.html#reviewer-s-statement-of-oversight, > it says: > > Both Tested-by and Reviewed-by tags, once received on mailing list from tester or reviewer, should be added by author to the applicable patches when sending next versions. > > So I guess you could do that at your convenience on any next version. In mine I can only do if you give me that tag. Till this line the tag is not given, but... > Or is it already ok, if I just add the following line in my comment? ...this is what is expected to have. > Tested-By: Jó Ágila Bitsch <jgilab@gmail.com> And now _if_ I need a new version I will bear this with it. Otherwise maintainer, who picks up the patch, takes care of gathering these all together. > I'm still quite a newbie in the kernel development community, so > thanks for bearing with my ignorance :-) So far you are doing good, thanks!
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index 8e2603688016..fa7dd6cf014d 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -829,7 +829,7 @@ static int bos_desc(struct usb_composite_dev *cdev) if (cdev->use_webusb) { struct usb_plat_dev_cap_descriptor *webusb_cap; struct usb_webusb_cap_data *webusb_cap_data; - uuid_t webusb_uuid = WEBUSB_UUID; + guid_t webusb_uuid = WEBUSB_UUID; webusb_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength); webusb_cap_data = (struct usb_webusb_cap_data *) webusb_cap->CapabilityData; @@ -841,7 +841,7 @@ static int bos_desc(struct usb_composite_dev *cdev) webusb_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY; webusb_cap->bDevCapabilityType = USB_PLAT_DEV_CAP_TYPE; webusb_cap->bReserved = 0; - export_uuid(webusb_cap->UUID, &webusb_uuid); + export_guid(webusb_cap->UUID, &webusb_uuid); if (cdev->bcd_webusb_version != 0) webusb_cap_data->bcdVersion = cpu_to_le16(cdev->bcd_webusb_version); diff --git a/include/linux/usb/webusb.h b/include/linux/usb/webusb.h index b430d84357f3..fe43020b4a48 100644 --- a/include/linux/usb/webusb.h +++ b/include/linux/usb/webusb.h @@ -11,15 +11,12 @@ #include "uapi/linux/usb/ch9.h" /* - * little endian PlatformCapablityUUID for WebUSB + * Little Endian PlatformCapablityUUID for WebUSB * 3408b638-09a9-47a0-8bfd-a0768815b665 - * to identify Platform Device Capability descriptors as referring to WebUSB - * - * the UUID above MUST be sent over the wire as the byte sequence: - * {0x38, 0xB6, 0x08, 0x34, 0xA9, 0x09, 0xA0, 0x47, 0x8B, 0xFD, 0xA0, 0x76, 0x88, 0x15, 0xB6, 0x65}. + * to identify Platform Device Capability descriptors as referring to WebUSB. */ #define WEBUSB_UUID \ - UUID_INIT(0x38b60834, 0xa909, 0xa047, 0x8b, 0xfd, 0xa0, 0x76, 0x88, 0x15, 0xb6, 0x65) + GUID_INIT(0x3408b638, 0x09a9, 0x47a0, 0x8b, 0xfd, 0xa0, 0x76, 0x88, 0x15, 0xb6, 0x65) /* * WebUSB Platform Capability data
We have two types for UUIDs depending on the byte ordering. Instead of explaining how bytes should go over the wire, use dedicated APIs and data types. This removes a confusion over the byte ordering. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/usb/gadget/composite.c | 4 ++-- include/linux/usb/webusb.h | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-)