* [PATCH] usb: gadget: composite: fix OS descriptors w_value logic
@ 2024-04-04 10:06 Peter Korsgaard
2024-04-22 19:09 ` Andrzej Pietrasiewicz
0 siblings, 1 reply; 3+ messages in thread
From: Peter Korsgaard @ 2024-04-04 10:06 UTC (permalink / raw
To: linux-usb, Greg Kroah-Hartman, Andrzej Pietrasiewicz; +Cc: Peter Korsgaard
The OS descriptors logic had the high/low byte of w_value inverted, causing
the extended properties to not be accessible for interface != 0.
From the Microsoft documentation:
https://learn.microsoft.com/en-us/windows-hardware/drivers/usbcon/microsoft-os-1-0-descriptors-specification
OS_Desc_CompatID.doc (w_index = 0x4):
- wValue:
High Byte = InterfaceNumber. InterfaceNumber is set to the number of the
interface or function that is associated with the descriptor, typically
0x00. Because a device can have only one extended compat ID descriptor,
it should ignore InterfaceNumber, regardless of the value, and simply
return the descriptor.
Low Byte = 0. PageNumber is used to retrieve descriptors that are larger
than 64 KB. The header section is 16 bytes, so PageNumber is set to 0 for
this request.
We currently do not support >64KB compat ID descriptors, so verify that the
low byte is 0.
OS_Desc_Ext_Prop.doc (w_index = 0x5):
- wValue:
High byte = InterfaceNumber. The high byte of wValue is set to the number
of the interface or function that is associated with the descriptor.
Low byte = PageNumber. The low byte of wValue is used to retrieve
descriptors that are larger than 64 KB. The header section is 10 bytes, so
PageNumber is set to 0 for this request.
We also don't support >64KB extended properties, so verify that the low byte
is 0 and use the high byte for the interface number.
Fixes: 37a3a533429e ("usb: gadget: OS Feature Descriptors support")
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
drivers/usb/gadget/composite.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 0ace45b66a31..0e151b54aae8 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2112,7 +2112,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
buf[5] = 0x01;
switch (ctrl->bRequestType & USB_RECIP_MASK) {
case USB_RECIP_DEVICE:
- if (w_index != 0x4 || (w_value >> 8))
+ if (w_index != 0x4 || (w_value & 0xff))
break;
buf[6] = w_index;
/* Number of ext compat interfaces */
@@ -2128,9 +2128,9 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
}
break;
case USB_RECIP_INTERFACE:
- if (w_index != 0x5 || (w_value >> 8))
+ if (w_index != 0x5 || (w_value & 0xff))
break;
- interface = w_value & 0xFF;
+ interface = w_value >> 8;
if (interface >= MAX_CONFIG_INTERFACES ||
!os_desc_cfg->interface[interface])
break;
--
2.39.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: gadget: composite: fix OS descriptors w_value logic
2024-04-04 10:06 [PATCH] usb: gadget: composite: fix OS descriptors w_value logic Peter Korsgaard
@ 2024-04-22 19:09 ` Andrzej Pietrasiewicz
2024-04-23 6:50 ` Peter Korsgaard
0 siblings, 1 reply; 3+ messages in thread
From: Andrzej Pietrasiewicz @ 2024-04-22 19:09 UTC (permalink / raw
To: Peter Korsgaard, linux-usb, Greg Kroah-Hartman
Hi Peter,
W dniu 4.04.2024 o 12:06, Peter Korsgaard pisze:
> The OS descriptors logic had the high/low byte of w_value inverted, causing
> the extended properties to not be accessible for interface != 0.
Out of curiosity - did you run into problems running some USB gadget,
and if yes, what was it? Or is this patch a result of a code and documentation
review without actually running a gadget?
>
> From the Microsoft documentation:
> https://learn.microsoft.com/en-us/windows-hardware/drivers/usbcon/microsoft-os-1-0-descriptors-specification
>
> OS_Desc_CompatID.doc (w_index = 0x4):
>
> - wValue:
>
> High Byte = InterfaceNumber. InterfaceNumber is set to the number of the
> interface or function that is associated with the descriptor, typically
> 0x00. Because a device can have only one extended compat ID descriptor,
> it should ignore InterfaceNumber, regardless of the value, and simply
> return the descriptor.
>
> Low Byte = 0. PageNumber is used to retrieve descriptors that are larger
> than 64 KB. The header section is 16 bytes, so PageNumber is set to 0 for
> this request.
>
> We currently do not support >64KB compat ID descriptors, so verify that the
> low byte is 0.
>
> OS_Desc_Ext_Prop.doc (w_index = 0x5):
>
> - wValue:
>
> High byte = InterfaceNumber. The high byte of wValue is set to the number
> of the interface or function that is associated with the descriptor.
>
> Low byte = PageNumber. The low byte of wValue is used to retrieve
> descriptors that are larger than 64 KB. The header section is 10 bytes, so
> PageNumber is set to 0 for this request.
>
FWIW w_value is:
u16 w_value = le16_to_cpu(ctrl->wValue);
Regards,
Andrzej
> We also don't support >64KB extended properties, so verify that the low byte
> is 0 and use the high byte for the interface number.
>
> Fixes: 37a3a533429e ("usb: gadget: OS Feature Descriptors support")
>
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> ---
> drivers/usb/gadget/composite.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 0ace45b66a31..0e151b54aae8 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -2112,7 +2112,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> buf[5] = 0x01;
> switch (ctrl->bRequestType & USB_RECIP_MASK) {
> case USB_RECIP_DEVICE:
> - if (w_index != 0x4 || (w_value >> 8))
> + if (w_index != 0x4 || (w_value & 0xff))
> break;
> buf[6] = w_index;
> /* Number of ext compat interfaces */
> @@ -2128,9 +2128,9 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> }
> break;
> case USB_RECIP_INTERFACE:
> - if (w_index != 0x5 || (w_value >> 8))
> + if (w_index != 0x5 || (w_value & 0xff))
> break;
> - interface = w_value & 0xFF;
> + interface = w_value >> 8;
> if (interface >= MAX_CONFIG_INTERFACES ||
> !os_desc_cfg->interface[interface])
> break;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: gadget: composite: fix OS descriptors w_value logic
2024-04-22 19:09 ` Andrzej Pietrasiewicz
@ 2024-04-23 6:50 ` Peter Korsgaard
0 siblings, 0 replies; 3+ messages in thread
From: Peter Korsgaard @ 2024-04-23 6:50 UTC (permalink / raw
To: Andrzej Pietrasiewicz; +Cc: linux-usb, Greg Kroah-Hartman
>>>>> "Andrzej" == Andrzej Pietrasiewicz <andrzej.p@collabora.com> writes:
> Hi Peter,
> W dniu 4.04.2024 o 12:06, Peter Korsgaard pisze:
>> The OS descriptors logic had the high/low byte of w_value inverted, causing
>> the extended properties to not be accessible for interface != 0.
> Out of curiosity - did you run into problems running some USB gadget,
> and if yes, what was it? Or is this patch a result of a code and documentation
> review without actually running a gadget?
I had issues with getting Windows to accept the OS descriptors when the
function I wanted to use with WinUSB was not the first (= interface 0)
function in a composite device together with HID and mass storage.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-23 6:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-04 10:06 [PATCH] usb: gadget: composite: fix OS descriptors w_value logic Peter Korsgaard
2024-04-22 19:09 ` Andrzej Pietrasiewicz
2024-04-23 6:50 ` Peter Korsgaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).