Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <0bdfdd52-4e73-0554-ef94-5da5af0ae01b@gmx.com>
Date:   Sun, 4 Sep 2022 23:38:31 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.0
From:   jflf_kernel@gmx.com
Subject: Re: [PATCH] usb: add quirks for Lenovo OneLink+ Dock
To:     Oliver Neukum <oneukum@suse.com>, gregkh@linuxfoundation.org,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220824160946.10128-1-jflf_kernel@gmx.com>
 <31aeee3c-f4f1-16a8-272b-96da5d4a565e@suse.com>
 <d1a5f149-50a1-49fc-9a6d-eceffa23311b@gmx.com>
 <4b92eee0-b020-9211-2039-18ac3ac72a7b@suse.com>
 <edd97137-74a1-ee0b-d475-7c5b36197155@gmx.com>
 <60a08cc9-a96d-9453-1cef-a440527a79f6@suse.com>
Content-Language: en-US
In-Reply-To: <60a08cc9-a96d-9453-1cef-a440527a79f6@suse.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk


On 31/08/2022 11.16, Oliver Neukum wrote:

>> I'll run some more tests out of curiosity to see how things behave in c=
orner cases.
>
> While you do so, which is a good idea, could you restate your current
> results in a more precise way? We should have 4 results for each hub.
> It is not the case that RESET_RESUME has no effect if you give
> usbcore.autosuspend=3D-1 if the system has been suspended. Hence we have
> the cases of
>
> no RESET_RESUME/default autosuspend
> no RESET_RESUME/autosuspend=3D-1
> with RESET_RESUME/default autosuspend
> with RESET_RESUME/autosuspend=3D-1


Hi Oliver,

Apologies for my last unclear message.

Both VL812 hubs behave the same way so I have a single table for both. The=
 settings are applied to both hubs, on both USB2 and USB3.

I have separated USB2 and USB3 as they behave slightly differently.


=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
Settings                        USB2 hotplug    USB3 hotplug    State afte=
r waking up
=2D-----------------------------------------------------------------------=
---------------

power/control=3Dauto              works (1)       fails           broken (=
2)

usbcore.autosuspend=3D-1          works           works           broken (=
2)
OR power/control=3Don

power/control=3Dauto              works (3)       works (3)       works
and USB_QUIRK_RESET_RESUME

power/control=3Don                works           works           works
and USB_QUIRK_RESET_RESUME

HUB_QUIRK_DISABLE_AUTOSUSPEND   works           works           works
and USB_QUIRK_RESET_RESUME

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D


(1) I have yet to find a device that would be rejected by the USB2 side, w=
hile I have reliable reproducers for USB3. The problem appears with specif=
ic devices only, which otherwise work flawlessly on other hubs, other syst=
ems, etc.

(2) After a system wake-up the hubs are in a seemingly random state. Numer=
ous errors appear in the syslog, and some ports may be dead (changes from =
wake-up to wake-up). This can affect both internal (to dock built-in devic=
es) and external hub ports.

(3) With RESET_RESUME the hubs (both USB2 and USB3) stay active and don't =
autosuspend, even when power/control=3D=3Dauto.


In all situations above a keyboard and the integrated ethernet controller =
can wake up the laptop from suspend. In situations (2) they may not work a=
nymore after waking up.


The bottom line is: we need USB_QUIRK_RESET_RESUME to deal with the spurio=
us hub state after waking up, and we need to prevent the hubs from autosus=
pending to work around the USB3 issue. RESET_RESUME does all of that right=
 now, but can its behaviour be considered stable? Is there any chance that=
 some day it would no longer block autosuspend?

If RESET_RESUME is stable, then my current patch is enough and it can be r=
e-merged. If not, I have an updated patch ready with HUB_QUIRK_DISABLE_AUT=
OSUSPEND.


Also, RESET_RESUME keeps the device nodes active, but not the hub interfac=
e nodes. HUB_QUIRK_DISABLE_AUTOSUSPEND and usbcore.autosuspend=3D-1 keep b=
oth of them active:

    $ grep active /sys/bus/usb/devices/2*/power/runtime_status    # USB_QU=
IRK_RESET_RESUME
    /sys/bus/usb/devices/2-1.1/power/runtime_status:active
    /sys/bus/usb/devices/2-1/power/runtime_status:active

    $ grep active /sys/bus/usb/devices/2*/power/runtime_status    # HUB_QU=
IRK_DISABLE_AUTOSUSPEND
    /sys/bus/usb/devices/2-1:1.0/power/runtime_status:active
    /sys/bus/usb/devices/2-1.1:1.0/power/runtime_status:active
    /sys/bus/usb/devices/2-1.1/power/runtime_status:active
    /sys/bus/usb/devices/2-1/power/runtime_status:active

I do not know what impact this could have. In my tests both accepted the h=
otplugged USB3 devices, so keeping the device nodes active is enough.


Thanks!
JF