Re: [PATCH] usb: gadget: f_hid: convert polling interval from ms to bInterval units

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Jul 16, 2026 at 04:49:21PM +0200, Arthur Crépin Leblond wrote:
> The HID polling interval is different depending on the USB speed.
> 
> - Full speed: the units are milliseconds (1-255)
> - High/Super speed: the units are micro-frames of 0.125ms
> 
> Exposing directly the bInterval means that the user needs to know
> which speed is currently in use and has to convert it accordingly
> themselves.
> 
> The solution is to expose the interval in units of milliseconds and
> do the conversion in the driver.
> 
> Cc: Ben Hoff <hoff.benjamin.k@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Arthur Crépin Leblond <arthur@marmottus.net>
> ---
>  Documentation/ABI/testing/configfs-usb-gadget-hid | 10 ++++++++++
>  drivers/usb/gadget/function/f_hid.c               |  8 ++++----
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-hid b/Documentation/ABI/testing/configfs-usb-gadget-hid
> index 748705c4cb58..006d479b7428 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-hid
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-hid
> @@ -11,3 +11,13 @@ Description:
>  		report_length	HID report length
>  		subclass	HID device subclass to use
>  		=============	============================================
> +
> +What:		/config/usb-gadget/gadget/functions/hid.name/interval
> +Date:		Jul 2026
> +KernelVersion:	7.2
> +Description:
> +		The polling interval for the HID endpoint(s), expressed in
> +		milliseconds in the range 1-255.
> +
> +		The value is speed-independent: the driver converts it to the
> +		appropriate bInterval.
> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
> index 3c6b43d06a6d..49548a96c1e8 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -1211,8 +1211,8 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
>  		hidg_ss_in_ep_desc.bInterval = 4;
>  	} else {
>  		hidg_fs_in_ep_desc.bInterval = hidg->interval;
> -		hidg_hs_in_ep_desc.bInterval = hidg->interval;
> -		hidg_ss_in_ep_desc.bInterval = hidg->interval;
> +		hidg_hs_in_ep_desc.bInterval = USB_MS_TO_HS_INTERVAL(hidg->interval);
> +		hidg_ss_in_ep_desc.bInterval = USB_MS_TO_HS_INTERVAL(hidg->interval);
>  	}
>  
>  	hidg_ss_out_comp_desc.wBytesPerInterval =
> @@ -1245,8 +1245,8 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
>  			hidg_ss_out_ep_desc.bInterval = 4;
>  		} else {
>  			hidg_fs_out_ep_desc.bInterval = hidg->interval;
> -			hidg_hs_out_ep_desc.bInterval = hidg->interval;
> -			hidg_ss_out_ep_desc.bInterval = hidg->interval;
> +			hidg_hs_out_ep_desc.bInterval = USB_MS_TO_HS_INTERVAL(hidg->interval);
> +			hidg_ss_out_ep_desc.bInterval = USB_MS_TO_HS_INTERVAL(hidg->interval);

So this breaks the current usage, as the value previously used now
operates differently, right?

Yes, it was never documented, but by changing this, what are you now
causing to be changed when kernels are updated and userspace isn't
modified?

thanks,

greg k-h