linux-kernel - Re: [PATCH] USB: core: hub_port_reset: Remove extra 40 ms reset recovery time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5b1d4bea-8d39-4f29-85e6-bd36c12510ce@rowland.harvard.edu>
Date: Mon, 5 Aug 2024 09:38:57 -0400
From: Alan Stern <stern@...land.harvard.edu>
To: Paul Menzel <pmenzel@...gen.mpg.de>
Cc: Nicolas Boichat <drinkcat@...omium.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Kai-Heng Feng <kai.heng.feng@...onical.com>,
	Hans de Goede <hdegoede@...hat.com>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] USB: core: hub_port_reset: Remove extra 40 ms reset
 recovery time

On Mon, Aug 05, 2024 at 10:19:17AM +0200, Paul Menzel wrote:
> > > > > > > A better approach would be to add a sysfs boolean
> > > > > > > attribute to the hub
> > > > > > > driver to enable the 40-ms reset-recovery delay, and
> > > > > > > make it default to
> > > > > > > True.  Then people who don't need the delay could disable it from
> > > > > > > userspace, say by a udev rule.
> > > > > > 
> > > > > > How would you name it?
> > > > > 
> > > > > You could call it "long_reset_recovery".  Anything like that would be
> > > > > okay.
> > > > 
> > > > Would it be useful to makes it an integer instead of a boolean,
> > > > and allow to configure the delay:
> > > > `extra_reset_recovery_delay_ms`?
> > > 
> > > Sure, why not?  Just so long as the default value matches the current
> > > behavior.
> > 
> > I hope, I am going to find time to take a stab at it.
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 4b93c0bd1d4b..72dd16eaa73a 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -120,9 +120,16 @@ MODULE_PARM_DESC(use_both_schemes,
>                 "try the other device initialization scheme if the "
>                 "first one fails");
> 
> +static int extra_reset_recovery_delay_ms = 40;
> +module_param(extra_reset_recovery_delay_ms, int, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(extra_reset_recovery_delay_ms,
> +               "extra recovery delay for USB devices after reset in
> milliseconds "
> +               "(default 40 ms");

This is a module parameter, not a sysfs attribute.  Module parameters 
should be avoided if possible, because they are much less flexible than 
sysfs attributes.

Alan Stern