[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 10:30 PM Chris Chiu <chris.chiu at canonical.com> wrote:
>
> 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

Gentle ping. Any suggestions will be appreciated. Thanks

Chris