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.
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`?
Kind regards,
Paul
