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

William Wu posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/usb/gadget/function/f_hid.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] usb: gadget: f_hid: Fix zero length packet transfer
Posted by William Wu 1 month, 1 week ago
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
Posted by Alan Stern 1 month, 1 week ago
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
Posted by William Wu 1 month, 1 week ago

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

Re: [PATCH] usb: gadget: f_hid: Fix zero length packet transfer
Posted by Greg KH 1 month, 1 week ago
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
Posted by wuliangfeng 1 month, 1 week ago

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