[PATCH] usb: gadget: f_hid: Fix zero length packet transfer

[PATCH] usb: gadget: f_hid: Fix zero length packet transfer

[William Wu]

If the hid transfer with size divisible to EPs max packet
size, it needs to set the req->zero to true, then the usb
controller can transfer a zero length packet at the end
according to the USB 2.0 spec.

Signed-off-by: William Wu <william.wu@rock-chips.com>
---
 drivers/usb/gadget/function/f_hid.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 8e1d1e8..8021af3 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -511,7 +511,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
 	}
 
 	req->status   = 0;
-	req->zero     = 0;
+	req->zero     = ((count % hidg->in_ep->maxpacket) == 0);
 	req->length   = count;
 	req->complete = f_hidg_req_complete;
 	req->context  = hidg;
@@ -967,7 +967,7 @@ static int hidg_setup(struct usb_function *f,
 	return -EOPNOTSUPP;
 
 respond:
-	req->zero = 0;
+	req->zero = ((length % cdev->gadget->ep0->maxpacket) == 0);
 	req->length = length;
 	status = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
 	if (status < 0)
-- 
2.0.0

Re: [PATCH] usb: gadget: f_hid: Fix zero length packet transfer

[Greg KH]

On Fri, Aug 22, 2025 at 10:01:58AM +0800, William Wu wrote:
> If the hid transfer with size divisible to EPs max packet
> size, it needs to set the req->zero to true, then the usb
> controller can transfer a zero length packet at the end
> according to the USB 2.0 spec.
> 
> Signed-off-by: William Wu <william.wu@rock-chips.com>
> ---
>  drivers/usb/gadget/function/f_hid.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
> index 8e1d1e8..8021af3 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -511,7 +511,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
>  	}
>  
>  	req->status   = 0;
> -	req->zero     = 0;
> +	req->zero     = ((count % hidg->in_ep->maxpacket) == 0);
>  	req->length   = count;
>  	req->complete = f_hidg_req_complete;
>  	req->context  = hidg;
> @@ -967,7 +967,7 @@ static int hidg_setup(struct usb_function *f,
>  	return -EOPNOTSUPP;
>  
>  respond:
> -	req->zero = 0;
> +	req->zero = ((length % cdev->gadget->ep0->maxpacket) == 0);
>  	req->length = length;
>  	status = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
>  	if (status < 0)

What commit id does this fix?

thanks,

greg k-h

Re: [PATCH] usb: gadget: f_hid: Fix zero length packet transfer

[wuliangfeng]

On 8/22/2025 12:45 PM, Greg KH wrote:
> On Fri, Aug 22, 2025 at 10:01:58AM +0800, William Wu wrote:
>> If the hid transfer with size divisible to EPs max packet
>> size, it needs to set the req->zero to true, then the usb
>> controller can transfer a zero length packet at the end
>> according to the USB 2.0 spec.
>>
>> Signed-off-by: William Wu <william.wu@rock-chips.com>
>> ---
>>   drivers/usb/gadget/function/f_hid.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
>> index 8e1d1e8..8021af3 100644
>> --- a/drivers/usb/gadget/function/f_hid.c
>> +++ b/drivers/usb/gadget/function/f_hid.c
>> @@ -511,7 +511,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
>>   	}
>>   
>>   	req->status   = 0;
>> -	req->zero     = 0;
>> +	req->zero     = ((count % hidg->in_ep->maxpacket) == 0);
>>   	req->length   = count;
>>   	req->complete = f_hidg_req_complete;
>>   	req->context  = hidg;
>> @@ -967,7 +967,7 @@ static int hidg_setup(struct usb_function *f,
>>   	return -EOPNOTSUPP;
>>   
>>   respond:
>> -	req->zero = 0;
>> +	req->zero = ((length % cdev->gadget->ep0->maxpacket) == 0);
>>   	req->length = length;
>>   	status = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
>>   	if (status < 0)
> 
> What commit id does this fix?
It seems that the first verison of the f_hid.c had this issue.
commit 71adf1189469 ("USB: gadget: add HID gadget driver")

Best Regards,
William Wu>
> thanks,
> 
> greg k-h
> 
> 
> 

Re: [PATCH] usb: gadget: f_hid: Fix zero length packet transfer

[Alan Stern]

On Fri, Aug 22, 2025 at 10:01:58AM +0800, William Wu wrote:
> If the hid transfer with size divisible to EPs max packet
> size, it needs to set the req->zero to true, then the usb
> controller can transfer a zero length packet at the end
> according to the USB 2.0 spec.
> 
> Signed-off-by: William Wu <william.wu@rock-chips.com>
> ---
>  drivers/usb/gadget/function/f_hid.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
> index 8e1d1e8..8021af3 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -511,7 +511,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
>  	}
>  
>  	req->status   = 0;
> -	req->zero     = 0;
> +	req->zero     = ((count % hidg->in_ep->maxpacket) == 0);

Here and below, there is no need to check whether count is divisible by 
the maxpacket length.  The UDC driver does this for you automatically.  
(See the kerneldoc for struct usb_request.)  Simply set req->zero to 1.

Alan Stern

>  	req->length   = count;
>  	req->complete = f_hidg_req_complete;
>  	req->context  = hidg;
> @@ -967,7 +967,7 @@ static int hidg_setup(struct usb_function *f,
>  	return -EOPNOTSUPP;
>  
>  respond:
> -	req->zero = 0;
> +	req->zero = ((length % cdev->gadget->ep0->maxpacket) == 0);
>  	req->length = length;
>  	status = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
>  	if (status < 0)
> -- 
> 2.0.0

Re: [PATCH] usb: gadget: f_hid: Fix zero length packet transfer

[William Wu]

On 8/22/2025 9:56 PM, Alan Stern wrote:
> On Fri, Aug 22, 2025 at 10:01:58AM +0800, William Wu wrote:
>> If the hid transfer with size divisible to EPs max packet
>> size, it needs to set the req->zero to true, then the usb
>> controller can transfer a zero length packet at the end
>> according to the USB 2.0 spec.
>>
>> Signed-off-by: William Wu <william.wu@rock-chips.com>
>> ---
>>   drivers/usb/gadget/function/f_hid.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
>> index 8e1d1e8..8021af3 100644
>> --- a/drivers/usb/gadget/function/f_hid.c
>> +++ b/drivers/usb/gadget/function/f_hid.c
>> @@ -511,7 +511,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
>>   	}
>>   
>>   	req->status   = 0;
>> -	req->zero     = 0;
>> +	req->zero     = ((count % hidg->in_ep->maxpacket) == 0);
> 
> Here and below, there is no need to check whether count is divisible by
> the maxpacket length.  The UDC driver does this for you automatically.
> (See the kerneldoc for struct usb_request.)  Simply set req->zero to 1.
> 
> Alan Stern
Good idea，I checked the UDC drivers (drivers/usb/gadget/udc and 
dwc2/dwc3), they test if zlp is actually needed if req->zero to 1.>
>>   	req->length   = count;
>>   	req->complete = f_hidg_req_complete;
>>   	req->context  = hidg;
>> @@ -967,7 +967,7 @@ static int hidg_setup(struct usb_function *f,
>>   	return -EOPNOTSUPP;
>>   
>>   respond:
>> -	req->zero = 0;
>> +	req->zero = ((length % cdev->gadget->ep0->maxpacket) == 0);
>>   	req->length = length;
>>   	status = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
>>   	if (status < 0)
>> -- 
>> 2.0.0
> 
> 
>