From: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
To: "Arthur Crépin Leblond" <arthur@marmottus.net>
Cc: Ben Hoff <hoff.benjamin.k@gmail.com>,
linux-usb@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v2] usb: gadget: f_hid: allow setting different interval for each speed
Date: Wed, 29 Jul 2026 13:45:48 +0530 [thread overview]
Message-ID: <a9197221-99d3-45c8-90c6-3ceeeade77e0@oss.qualcomm.com> (raw)
In-Reply-To: <20260727152705.79384-1-arthur@marmottus.net>
On 7/27/2026 8:57 PM, Arthur Crépin Leblond wrote:
> The HID polling interval unit depends on the USB speed.
>
> - Full speed: the units are in milliseconds
> - High/Super speed: the units are in 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.
>
> The solution is to expose the interval for each speed:
> - interval_fs: bInterval for full-speed
> - interval_hs: bInterval for high-speed
> - interval_ss: bInterval for super-speed
> - interval: legacy attribute that takes precedence over the per-speed
> attributes and sets the same bInterval for each speed.
>
> 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>
> ---
> v2:
> - Expose a specific interval attribute per USB speed
> - Use a define for the default values
> - Update ABI documentation
>
> v1: 20260716144921.1120265-1-arthur@marmottus.net
> .../ABI/testing/configfs-usb-gadget-hid | 18 ++
> drivers/usb/gadget/function/f_hid.c | 191 +++++++++++-------
> drivers/usb/gadget/function/u_hid.h | 9 +-
> 3 files changed, 142 insertions(+), 76 deletions(-)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-hid b/Documentation/ABI/testing/configfs-usb-gadget-hid
> index 748705c4cb58..a81f5b527b59 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-hid
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-hid
> @@ -11,3 +11,21 @@ Description:
> report_length HID report length
> subclass HID device subclass to use
> ============= ============================================
> +
> +What: /config/usb-gadget/gadget/functions/hid.name
> +Date: Jul 2026
> +KernelVersion: 7.3
> +Description:
> + The attributes:
> +
> + ============= ============================================
> + interval HID endpoint bInterval for all speeds; if
> + set, it takes precedence over the
> + per-speed attributes below
> + interval_fs HID endpoint bInterval for full-speed
> + (in milliseconds, default: 10)
> + interval_hs HID endpoint bInterval for high-speed
> + (in 125 us units, default: 4)
> + interval_ss HID endpoint bInterval for super-speed
> + (in 125 us units, default: 4)
> + ============= ============================================
> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
> index 3c6b43d06a6d..f5ad3ae3e02b 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -30,6 +30,11 @@
> */
> #define GET_REPORT_TIMEOUT_MS 2500
>
> +/* Default bInterval values */
> +#define HIDG_DEFAULT_FS_BINTERVAL 10
> +#define HIDG_DEFAULT_HS_BINTERVAL 4
> +#define HIDG_DEFAULT_SS_BINTERVAL 4
> +
> static int major, minors;
>
> static const struct class hidg_class = {
> @@ -63,7 +68,9 @@ struct f_hidg {
> char *report_desc;
> unsigned short report_length;
> unsigned char interval;
> - bool interval_user_set;
> + struct f_hid_opt_interval interval_fs;
> + struct f_hid_opt_interval interval_hs;
> + struct f_hid_opt_interval interval_ss;
>
> /*
> * use_out_ep - if true, the OUT Endpoint (interrupt out method)
> @@ -1204,16 +1211,24 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
> hidg_fs_in_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
> hidg_ss_out_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
>
> - /* IN endpoints: FS default=10ms, HS default=4µ-frame; user override if set */
> - if (!hidg->interval_user_set) {
> - hidg_fs_in_ep_desc.bInterval = 10;
> - hidg_hs_in_ep_desc.bInterval = 4;
> - 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;
> - }
> + /* IN endpoints: set defaults unless user set */
> + if (hidg->interval_fs.user_set)
> + hidg_fs_in_ep_desc.bInterval = hidg->interval_fs.value;
> + else
> + hidg_fs_in_ep_desc.bInterval =
> + HIDG_DEFAULT_FS_BINTERVAL;
> +
> + if (hidg->interval_hs.user_set)
> + hidg_hs_in_ep_desc.bInterval = hidg->interval_hs.value;
> + else
> + hidg_hs_in_ep_desc.bInterval =
> + HIDG_DEFAULT_HS_BINTERVAL;
> +
> + if (hidg->interval_ss.user_set)
> + hidg_ss_in_ep_desc.bInterval = hidg->interval_ss.value;
> + else
> + hidg_ss_in_ep_desc.bInterval =
> + HIDG_DEFAULT_SS_BINTERVAL;
>
> hidg_ss_out_comp_desc.wBytesPerInterval =
> cpu_to_le16(hidg->report_length);
> @@ -1238,16 +1253,28 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
> hidg_fs_out_ep_desc.bEndpointAddress;
>
> if (hidg->use_out_ep) {
> - /* OUT endpoints: same defaults (FS=10, HS=4) unless user set */
> - if (!hidg->interval_user_set) {
> - hidg_fs_out_ep_desc.bInterval = 10;
> - hidg_hs_out_ep_desc.bInterval = 4;
> - 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;
> - }
> + /* OUT endpoints: set defaults unless user set */
> + if (hidg->interval_fs.user_set)
> + hidg_fs_out_ep_desc.bInterval =
> + hidg->interval_fs.value;
> + else
> + hidg_fs_out_ep_desc.bInterval =
> + HIDG_DEFAULT_FS_BINTERVAL;
> +
> + if (hidg->interval_hs.user_set)
> + hidg_hs_out_ep_desc.bInterval =
> + hidg->interval_hs.value;
> + else
> + hidg_hs_out_ep_desc.bInterval =
> + HIDG_DEFAULT_HS_BINTERVAL;
> +
> + if (hidg->interval_ss.user_set)
> + hidg_ss_out_ep_desc.bInterval =
> + hidg->interval_ss.value;
> + else
> + hidg_ss_out_ep_desc.bInterval =
> + HIDG_DEFAULT_SS_BINTERVAL;
> +
> status = usb_assign_descriptors(f,
> hidg_fs_descriptors_intout,
> hidg_hs_descriptors_intout,
> @@ -1334,14 +1361,17 @@ static const struct configfs_item_operations hidg_item_ops = {
> .release = hid_attr_release,
> };
>
> -#define F_HID_OPT(name, prec, limit) \
> -static ssize_t f_hid_opts_##name##_show(struct config_item *item, char *page)\
> +#define F_HID_OPT_U(name, prec, limit) \
> +static ssize_t f_hid_opts_##name##_show(struct config_item *item, \
> + char *page) \
> { \
> struct f_hid_opts *opts = to_f_hid_opts(item); \
> int result; \
> + u##prec value; \
> \
> mutex_lock(&opts->lock); \
> - result = sprintf(page, "%d\n", opts->name); \
> + value = get_hid_opt_##name(opts); \
> + result = sprintf(page, "%d\n", value); \
> mutex_unlock(&opts->lock); \
> \
> return result; \
> @@ -1368,7 +1398,7 @@ static ssize_t f_hid_opts_##name##_store(struct config_item *item, \
> ret = -EINVAL; \
> goto end; \
> } \
> - opts->name = num; \
> + set_hid_opt_##name(opts, num); \
> ret = len; \
> \
> end: \
> @@ -1378,11 +1408,54 @@ end: \
> \
> CONFIGFS_ATTR(f_hid_opts_, name)
>
> +#define F_HID_OPT(name, prec, limit) \
> +static void set_hid_opt_##name(struct f_hid_opts *opts, u##prec val) \
> +{ \
> + opts->name = val; \
> +} \
> +static u##prec get_hid_opt_##name(struct f_hid_opts *opts) \
> +{ \
> + return opts->name; \
> +} \
> +F_HID_OPT_U(name, prec, limit)
> +
> +#define F_HID_OPT_INTERVAL(name) \
> +static void set_hid_opt_##name(struct f_hid_opts *opts, u8 val) \
> +{ \
> + opts->name.value = val; \
> + opts->name.user_set = true; \
> +} \
> +static u8 get_hid_opt_##name(struct f_hid_opts *opts) \
> +{ \
> + return opts->name.value; \
> +} \
> +F_HID_OPT_U(name, 8, 255)
> +
> F_HID_OPT(subclass, 8, 255);
> F_HID_OPT(protocol, 8, 255);
> F_HID_OPT(no_out_endpoint, 8, 1);
> F_HID_OPT(report_length, 16, 65535);
>
> +F_HID_OPT_INTERVAL(interval_fs);
> +F_HID_OPT_INTERVAL(interval_hs);
> +F_HID_OPT_INTERVAL(interval_ss);
> +
> +static void set_hid_opt_interval(struct f_hid_opts *opts, u8 val)
> +{
> + set_hid_opt_interval_fs(opts, val);
> + set_hid_opt_interval_hs(opts, val);
> + set_hid_opt_interval_ss(opts, val);
> +
> + opts->interval = val;
> +}
> +
> +static u8 get_hid_opt_interval(struct f_hid_opts *opts)
> +{
> + return opts->interval;
> +}
> +
> +F_HID_OPT_U(interval, 8, 255);
> +
> static ssize_t f_hid_opts_report_desc_show(struct config_item *item, char *page)
> {
> struct f_hid_opts *opts = to_f_hid_opts(item);
> @@ -1428,53 +1501,6 @@ static ssize_t f_hid_opts_report_desc_store(struct config_item *item,
>
> CONFIGFS_ATTR(f_hid_opts_, report_desc);
>
> -static ssize_t f_hid_opts_interval_show(struct config_item *item, char *page)
> -{
> - struct f_hid_opts *opts = to_f_hid_opts(item);
> - int result;
> -
> - mutex_lock(&opts->lock);
> - result = sprintf(page, "%d\n", opts->interval);
use sysfs_emit instead here.
> - mutex_unlock(&opts->lock);
> -
> - return result;
> -}
> -
> -static ssize_t f_hid_opts_interval_store(struct config_item *item,
> - const char *page, size_t len)
> -{
> - struct f_hid_opts *opts = to_f_hid_opts(item);
> - int ret;
> - unsigned int tmp;
> -
> - mutex_lock(&opts->lock);
> - if (opts->refcnt) {
> - ret = -EBUSY;
> - goto end;
> - }
> -
> - /* parse into a wider type first */
> - ret = kstrtouint(page, 0, &tmp);
> - if (ret)
> - goto end;
> -
> - /* range-check against unsigned char max */
> - if (tmp > 255) {
> - ret = -EINVAL;
> - goto end;
> - }
> -
> - opts->interval = (unsigned char)tmp;
> - opts->interval_user_set = true;
> - ret = len;
> -
> -end:
> - mutex_unlock(&opts->lock);
> - return ret;
> -}
> -
> -CONFIGFS_ATTR(f_hid_opts_, interval);
> -
> static ssize_t f_hid_opts_dev_show(struct config_item *item, char *page)
> {
> struct f_hid_opts *opts = to_f_hid_opts(item);
> @@ -1490,6 +1516,9 @@ static struct configfs_attribute *hid_attrs[] = {
> &f_hid_opts_attr_no_out_endpoint,
> &f_hid_opts_attr_report_length,
> &f_hid_opts_attr_interval,
> + &f_hid_opts_attr_interval_hs,
> + &f_hid_opts_attr_interval_fs,
> + &f_hid_opts_attr_interval_ss,
> &f_hid_opts_attr_report_desc,
> &f_hid_opts_attr_dev,
> NULL,
> @@ -1537,8 +1566,13 @@ static struct usb_function_instance *hidg_alloc_inst(void)
> return ERR_PTR(-ENOMEM);
> mutex_init(&opts->lock);
>
> - opts->interval = 4;
> - opts->interval_user_set = false;
> + opts->interval = HIDG_DEFAULT_HS_BINTERVAL;
> + opts->interval_fs.value = HIDG_DEFAULT_FS_BINTERVAL;
> + opts->interval_fs.user_set = false;
> + opts->interval_hs.value = HIDG_DEFAULT_HS_BINTERVAL;
> + opts->interval_hs.user_set = false;
> + opts->interval_ss.value = HIDG_DEFAULT_SS_BINTERVAL;
> + opts->interval_ss.user_set = false;
>
> opts->func_inst.free_func_inst = hidg_free_inst;
> ret = &opts->func_inst;
> @@ -1628,8 +1662,15 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi)
> hidg->bInterfaceProtocol = opts->protocol;
> hidg->report_length = opts->report_length;
> hidg->report_desc_length = opts->report_desc_length;
> +
> hidg->interval = opts->interval;
> - hidg->interval_user_set = opts->interval_user_set;
> + hidg->interval_fs.value = opts->interval_fs.value;
> + hidg->interval_fs.user_set = opts->interval_fs.user_set;
> + hidg->interval_hs.value = opts->interval_hs.value;
> + hidg->interval_hs.user_set = opts->interval_hs.user_set;
> + hidg->interval_ss.value = opts->interval_ss.value;
> + hidg->interval_ss.user_set = opts->interval_ss.user_set;
> +
> if (opts->report_desc) {
> hidg->report_desc = kmemdup(opts->report_desc,
> opts->report_desc_length,
> diff --git a/drivers/usb/gadget/function/u_hid.h b/drivers/usb/gadget/function/u_hid.h
> index a9ed9720caee..e666cd9968eb 100644
> --- a/drivers/usb/gadget/function/u_hid.h
> +++ b/drivers/usb/gadget/function/u_hid.h
> @@ -15,6 +15,11 @@
>
> #include <linux/usb/composite.h>
>
> +struct f_hid_opt_interval {
> + unsigned char value;
> + bool user_set;
> +};
> +
> struct f_hid_opts {
> struct usb_function_instance func_inst;
> int minor;
> @@ -26,7 +31,9 @@ struct f_hid_opts {
> unsigned char *report_desc;
> bool report_desc_alloc;
> unsigned char interval;
> - bool interval_user_set;
> + struct f_hid_opt_interval interval_fs;
> + struct f_hid_opt_interval interval_hs;
> + struct f_hid_opt_interval interval_ss;
>
> /*
> * Protect the data form concurrent access by read/write
prev parent reply other threads:[~2026-07-29 8:15 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-27 15:27 [PATCH v2] usb: gadget: f_hid: allow setting different interval for each speed Arthur Crépin Leblond
2026-07-29 8:15 ` Krishna Kurapati [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a9197221-99d3-45c8-90c6-3ceeeade77e0@oss.qualcomm.com \
--to=krishna.kurapati@oss.qualcomm.com \
--cc=arthur@marmottus.net \
--cc=gregkh@linuxfoundation.org \
--cc=hoff.benjamin.k@gmail.com \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox