Re: [PATCH v3] usb:gadget:hid: {GET,SET} PROTOCOL Support

[Alan Stern]

On Wed, 19 Jul 2017, Abdulhadi Mohamed wrote:

> Currently linux HID gadgets do not support remote wakeup (the ability
> to wake up a host from suspend). This is an important feature for
> gadgets that want to properly emulate the normal operation of a mouse
> and keyboard. Ultimately, This feature has to be enabled/supported by
> the underlying UDC driver to have any impact. This version fixes all 
> previous formatting errors.

This description has no connection to the patch contents!

The patch is about adding support for the Get-Protocol and Set-Protocol 
requests to the f_hid driver.  It is not about remote wakeup or UDC 
drivers.

A patch cannot be accepted without a reasonable description.

Alan Stern

> 
> 
> Signed-off-by: Abdulhadi Mohamed 
> 
> ---
>  drivers/usb/gadget/function/f_hid.c | 17 -
>  include/linux/hid.h |  6 ++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/function/f_hid.c 
> b/drivers/usb/gadget/function/f_hid.c
> index 5eea448..c60b882 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -44,6 +44,7 @@ struct f_hidg {
>   /* configuration */
>   unsigned char   bInterfaceSubClass;
>   unsigned char   bInterfaceProtocol;
> + unsigned char   protocol;
>   unsigned short  report_desc_length;
>   char*report_desc;
>   unsigned short  report_length;
> @@ -527,7 +528,9 @@ static int hidg_setup(struct usb_function *f,
>   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 int, length, 1);
> + ((u8 *) req->buf)[0] = hidg->protocol;
> + goto respond;
>   break;
>  
>   case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
> @@ -539,6 +542,17 @@ static int hidg_setup(struct usb_function *f,
>   case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
> | HID_REQ_SET_PROTOCOL):
>   VDBG(cdev, "set_protocol\n");
> + if (value > HID_REPORT_PROTOCOL)
> + 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 = value;
> + goto respond;
> + }
>   goto stall;
>   break;
>  
> @@ -768,6 +782,7 @@ static int hidg_bind(struct usb_configuration *c, struct 
> usb_function *f)
>   /* set descriptor dynamic values */
>   hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
>   hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
> + hidg->protocol = HID_REPORT_PROTOCOL;
>   hidg_ss_in_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
>   hidg_ss_in_comp_desc.wBytesPerInterval =
>   cpu_to_le16(hidg->report_length);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index b2ec827..29d06c8 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -347,6 +347,12 @@ struct hid_item {
>  #define HID_GROUP_LOGITECH_DJ_DEVICE 0x0102
>  
>  /*
> + * HID protocol status
> + */
> +#define HID_REPORT_PROTOCOL  1
> +#define HID_BOOT_PROTOCOL0
> +
> +/*
>   * This is the global environment of the parser. This information is
>   * persistent for main-items. The global environment can be saved and
>   * restored with PUSH/POP statements.
> 

--
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

Re: [PATCH v3] usb:gadget:hid: {GET,SET} PROTOCOL Support

[Greg KH]

On Wed, Jul 19, 2017 at 02:41:56PM +0100, Abdulhadi Mohamed wrote:
> Currently linux HID gadgets do not support remote wakeup (the ability
> to wake up a host from suspend). This is an important feature for
> gadgets that want to properly emulate the normal operation of a mouse
> and keyboard. Ultimately, This feature has to be enabled/supported by
> the underlying UDC driver to have any impact. This version fixes all 
> previous formatting errors.
> 
> 
> Signed-off-by: Abdulhadi Mohamed 
> 
> ---
>  drivers/usb/gadget/function/f_hid.c | 17 -
>  include/linux/hid.h |  6 ++
>  2 files changed, 22 insertions(+), 1 deletion(-)

What changed from v2?  Always put that below the --- line as the
Documentation tells you to.

v4?

thanks,

greg k-h
--
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

[PATCH v3] usb:gadget:hid: {GET,SET} PROTOCOL Support

[Abdulhadi Mohamed]

Currently linux HID gadgets do not support remote wakeup (the ability
to wake up a host from suspend). This is an important feature for
gadgets that want to properly emulate the normal operation of a mouse
and keyboard. Ultimately, This feature has to be enabled/supported by
the underlying UDC driver to have any impact. This version fixes all 
previous formatting errors.


Signed-off-by: Abdulhadi Mohamed 

---
 drivers/usb/gadget/function/f_hid.c | 17 -
 include/linux/hid.h |  6 ++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index 5eea448..c60b882 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -44,6 +44,7 @@ struct f_hidg {
/* configuration */
unsigned char   bInterfaceSubClass;
unsigned char   bInterfaceProtocol;
+   unsigned char   protocol;
unsigned short  report_desc_length;
char*report_desc;
unsigned short  report_length;
@@ -527,7 +528,9 @@ static int hidg_setup(struct usb_function *f,
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 int, length, 1);
+   ((u8 *) req->buf)[0] = hidg->protocol;
+   goto respond;
break;
 
case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
@@ -539,6 +542,17 @@ static int hidg_setup(struct usb_function *f,
case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
  | HID_REQ_SET_PROTOCOL):
VDBG(cdev, "set_protocol\n");
+   if (value > HID_REPORT_PROTOCOL)
+   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 = value;
+   goto respond;
+   }
goto stall;
break;
 
@@ -768,6 +782,7 @@ static int hidg_bind(struct usb_configuration *c, struct 
usb_function *f)
/* set descriptor dynamic values */
hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
+   hidg->protocol = HID_REPORT_PROTOCOL;
hidg_ss_in_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
hidg_ss_in_comp_desc.wBytesPerInterval =
cpu_to_le16(hidg->report_length);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index b2ec827..29d06c8 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -347,6 +347,12 @@ struct hid_item {
 #define HID_GROUP_LOGITECH_DJ_DEVICE   0x0102
 
 /*
+ * HID protocol status
+ */
+#define HID_REPORT_PROTOCOL1
+#define HID_BOOT_PROTOCOL  0
+
+/*
  * This is the global environment of the parser. This information is
  * persistent for main-items. The global environment can be saved and
  * restored with PUSH/POP statements.
-- 
2.9.3

--
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