[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cee9630e-781e-49b1-82c5-9066552f71b1@molgen.mpg.de>
Date: Wed, 24 Jul 2024 23:00:42 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Alan Stern <stern@...land.harvard.edu>
Cc: 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
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
Powered by blists - more mailing lists