On Tue, 26 May 2015, Golmer Palmer wrote:
> Alan Stern <stern@...> writes:
>
> > > Please, take into account that I mentioned that the ASSUMPTION that
> > > keyboard and mouse drivers are compatible with BOTH protocols is
> really
> > > TRUE.
> >
> > No, it isn't. You said "several Keyboard and Mouse implementations
> > using this framework are in fact fully compatible with BOOT mode."
> > This means that other implementations are _not_ compatible with Boot
> > mode. The assumption is _sometimes_ true and _sometimes_ false.
>
> Yes. You're right. And you put the solution in your patch: if the
> descriptor of the device contains the USB_INTERFACE_SUBCLASS_BOOT then
> this device has implemented the BOOT protocol. If not listed as boot
> compatible, then no option for working in BOOT mode.
>
> The assumption about I speak is this: implementations that advertise as
> BOOT compatible AND are using a DIFFERENT implementation for BOOT and
> REPORT modes are... INEXISTENT. This type of implementations don't
> exists in the Linux gadget land because with the current driver is
> impossible to change from REPORT to BOOT (remember that the function
> Set_Protocol() *always* return ERROR). Then this case can be put
> out-of-scope. In any case can be possible to modify the code to include
> a define option for enable it. But the current case is that neither
> implementation with boot=report and boot!=report can work because the
> lack of Set_Protocol() and Get_protocol().
> So the best, at minimum, is support the most common case (boot=report).
>
> Personally, I only know a very few devices that supports the report mode
> with expanded functions and supporting also the boot mode. Mainly
> advanced keyboards (multi-key) that all of them are implemented using
> custom stacks (not the Linux gadget). So this a very *uncommon* case.
> Correct. When someone needs this in the future a new function for enable
> it should needed in the kernel driver. But for fully support the BOOT
> mode a simple implementation of the Set_Protocol() and Get_Protocol()
> are the only requirement.
>
> So please, can you add also some minimal support inside Set_Protocol()
> to "change" from mode "0" to mode "1" (and viceversa) assuming that both
> modes are equal?
Okay, a revised patch is below. Let me know how it works out.
> Thank you Alan for your support and comments. I really appreciate them!
You're welcome.
Alan Stern
Index: usb-4.0/drivers/usb/gadget/function/f_hid.c
===================================================================
--- usb-4.0.orig/drivers/usb/gadget/function/f_hid.c
+++ usb-4.0/drivers/usb/gadget/function/f_hid.c
@@ -44,6 +44,7 @@ struct f_hidg {
/* configuration */
unsigned char bInterfaceSubClass;
unsigned char bInterfaceProtocol;
+ unsigned char protocol_is_report;
unsigned short report_desc_length;
char *report_desc;
unsigned short report_length;
@@ -418,7 +419,9 @@ static int hidg_setup(struct usb_functio
case ((USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
| HID_REQ_GET_PROTOCOL):
VDBG(cdev, "get_protocol\n");
- goto stall;
+ length = min_t(unsigned, length, 1);
+ ((u8 *) req->buf)[0] = hidg->protocol_is_report;
+ goto respond;
break;
case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
@@ -430,6 +433,17 @@ static int hidg_setup(struct usb_functio
case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
| HID_REQ_SET_PROTOCOL):
VDBG(cdev, "set_protocol\n");
+ if (value > 1)
+ goto stall;
+ length = 0;
+ /*
+ * We assume that programs implementing the Boot protocol
+ * are also compatible with the Report protocol.
+ */
+ if (hidg->bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT) {
+ hidg->protocol_is_report = value;
+ goto respond;
+ }
goto stall;
break;
@@ -933,6 +947,8 @@ static struct usb_function *hidg_alloc(s
hidg->minor = opts->minor;
hidg->bInterfaceSubClass = opts->subclass;
hidg->bInterfaceProtocol = opts->protocol;
+ hidg->protocol_is_report = (hidg->bInterfaceSubClass !=
+ USB_INTERFACE_SUBCLASS_BOOT);
hidg->report_length = opts->report_length;
hidg->report_desc_length = opts->report_desc_length;
if (opts->report_desc) {
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
