On Wed, Jul 24, 2024 at 11:00:42PM +0200, Paul Menzel wrote: > Dear Alan, > > > Am 24.07.24 um 20:52 schrieb Alan Stern: > > On Wed, Jul 24, 2024 at 08:14:34PM +0200, Paul Menzel wrote: > > […] > > > > Am 24.07.24 um 16:10 schrieb Alan Stern: > > > > On Wed, Jul 24, 2024 at 01:15:23PM +0200, Paul Menzel wrote: > > > > > This basically reverts commit b789696af8b4102b7cc26dec30c2c51ce51ee18b > > > > > ("[PATCH] USB: relax usbcore reset timings") from 2005. > > > > > > > > > > This adds unneeded 40 ms during resume from suspend on a majority of > > > > > > > > Wrong. It adds 40 ms to the recovery time from a port reset -- see the > > > > commit's title. Suspend and resume do not in general involve port > > > > resets (although sometimes they do). > > > > > > It looks like on my system the ports are reset: > > > > > > ``` > > > $ grep suspend-240501-063619/hub_port_reset abreu_mem_ftrace.txt > > > 6416.257589 | 3) kworker-9023 | | hub_port_reset [usbcore]() { > > > 6416.387182 | 2) kworker-9023 | 129593.0 us | } /* hub_port_reset [usbcore] */ > > > > > ``` > > > > It depends on the hardware and the kind of suspend. > > It is ACPI S3 suspend. Can I find out, why the ports are reset? Not > resetting the ports would be even better to reduce the resume time. It's probably an xHCI thing -- the hardware may stop providing power to the ports during S3 suspend, or something like that. The xHCI people may have a better idea of what's going on. > > > > > devices, where it’s not needed, like the Dell XPS 13 9360/0596KF, BIOS > > > > > 2.21.0 06/02/2022 with > > > > > > > > > The commit messages unfortunately does not list the devices needing this. > > > > > Should they surface again, these should be added to the quirk list for > > > > > USB_QUIRK_HUB_SLOW_RESET. > > > > > > > > This quirk applies to hubs that need extra time when one of their ports > > > > gets reset. However, it seems likely that the patch you are reverting > > > > was meant to help the device attached to the port, not the hub itself. > > > > Which would mean that the adding hubs to the quirk list won't help > > > > unless every hub is added -- in which case there's no point reverting > > > > the patch. > > > > > > > > Furthermore, should any of these bad hubs or devices still be in use, > > > > your change would cause them to stop working reliably. It would be a > > > > regression. > > > > > > > > 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. Alan Stern