[PATCH 3/3][SRU][OEM-5.10/U] UBUNTU: SAUCE: (no-up) USB: reset-resume the Realtek hub if suspend timeout

[Chris Chiu]

On Mon, May 17, 2021 at 8:41 PM Krzysztof Kozlowski
<krzysztof.kozlowski at canonical.com> wrote:
>
> On 16/05/2021 14:14, chris.chiu at canonical.com wrote:
> > From: Chris Chiu <chris.chiu at canonical.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1928242
> >
> > In Dell WD19 dock, there're a super-speed hub and a high-speed hub.
> > When the high-speed hub hit the timeout problem when suspending the
> > port which has wakeup enabled descendants, the super-speed hub will
> > also hit the timeout when there's super-speed device connecting to
> > the exposed Type-A port for unknown reason. If the super speed device
> > connects to Type-C port, there's no such problem.
> >
> > The port status of the super-speed hub doesn't indicate it's suspended.
> > However, if no ClearPortFeature invoked during resume for the port,
> > the hub will then fail to activate and all devices connected to the
> > super-speedhub will be lost.
> >
> > This commit reset-resume the downstream hub of the superspeed hub
> > only if suspend timeout happens even the port is not suspended.
> > Since it only happens in particular circumstance, we do not like
> > RESET_RESUME quirk to reset-resume all the time which lose the
> > runtime pm capability. And it's unable to be upstream because no
> > good explanation for why the Type-A port with a super-speed device
> > connected cause the super-speed hub suspend timeout at the same
> > time when high-speed hub timeout during suspend.
> >
> > Signed-off-by: Chris Chiu <chris.chiu at canonical.com>
> > ---
> >  drivers/usb/core/hub.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index e739f7b5991a..740d84028757 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -3391,6 +3391,7 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
> >               if (status == -ETIMEDOUT) {
> >                       int ret;
> >                       u16 portstatus, portchange;
> > +                     u16 vid = le16_to_cpu(udev->descriptor.idVendor);
> >
> >                       portstatus = portchange = 0;
> >                       ret = hub_port_status(hub, port1, &portstatus,
> > @@ -3403,6 +3404,12 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
> >                               status = 0;
> >                               goto suspend_done;
> >                       }
> > +
> > +                     // reset_resume the downstream Realtek hub
> > +                     if (vid == 0x0bda && usb_hub_to_struct_hub(udev)) {
>
> This looks very hacky. I think an upstreamable solution would be better
> than hard-coding Realtek VID in generic hub code..
>
> Best regards,
> Krzysztof

That's the difficult part and I don't like this hacky code, either.
This should be caused by the special design of the WD19. The
super-speed device connected to the Type-A port would be routed to the
super-speed hub. When the timeout happens on the high-speed hub, we
can still handle it because the port status shows that it's suspended.
But the super-speed hub will then timeout suspending the port
connecting to the super-speed device. High-speed hub timeout is the
cause, Super-speed hub timeout is the consequence. It's hard to simply
have a quirk for the super-speed hub w/o explaining the cause. And the
super-speed hub timeout won't occur if the timeout problem didn't
happen on the high-speed hub first, so quirking only the super-speed
hub makes no sense.

That's why we have this hacky code here. We need to take care of the
super-speed hub timeout only for the "WD19 dock". And there's no
useful Dell device info we can use for quirk. The Dell devices of the
WD19 are listed below.
Bus 003 Device 007: ID 413c:b06e Dell Computer Corp. 58200   (HID
device, connect to high-speed hub 0bda:5487)
Bus 003 Device 009: ID 413c:b06f Dell Computer Corp. 58200    (HID
device, connect to high-speed hub 0bda:5413, which is the downstream
hub of 0bda:5487)

This is the minimal impact code that I can come out. We don't need a
quirk for the Realtek super-speed because it could be a standalone hub
w/o problem. If some other peripherals have the same Realtek Hub, the
reset_resume will cause no harm. If there's any better option, please
advise and I would be happy to replace the current hacky code.

Chris