On Fri, Apr 16, 2021 at 11:39 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Fri, Apr 16, 2021 at 09:24:30AM +0800, Chris Chiu wrote: > > On Fri, Apr 16, 2021 at 2:46 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > On Fri, Apr 16, 2021 at 12:13:43AM +0800, Chris Chiu wrote: > > > > One thing worth mentioning here, I never hit the hub_ext_port_status -71 > > > > problem if I resume by waking up from the keyboard connected to the hub. > > > > > > I thought you said earlier that the port got into trouble while it was > > > suspending, not while it was resuming. You wrote: > > > > > > > [ 2789.679812] usb 3-4-port3: can't suspend, status -110 > > > > > > So if the problem occurs during suspend, how can it be possible to avoid > > > the problem by taking some particular action later while resuming? > > > > > > > The ETIMEDOUT is still there, but the port can resume w/o problems and I > > don't really know what the hub did. I can only reset_resume the hub to bring it > > back if I'm not waking up from the connected keyboard. > > What if two devices are plugged into the hub, only one of them is > runtime suspended, and you need to runtime resume that device? Then you > can't do a reset-resume of the hub, because the hub isn't suspended. > > > > > But the usbcore kernel log shows me wPortStatus: 0503 wPortChane: 0004 > > > > of that port while resuming. In normal cases, they are 0507:0000. > > > > > > The 0004 bit of wPortChange means that the suspend status has changed; > > > the port is no longer suspended because the device attached to that port > > > (your keyboard) issued a wakeup request. > > > > > > > I don't know how to SetPortFeature() with setting the status change bit only. > > > > > > You can't. Only the hub itself can set the wPortChange bits. > > > > > > > Or maybe it's just some kind of timing issue of the > > > > idle/suspend/resume signaling. > > > > > > Not timing. It's because you woke the system up from the attached > > > keyboard. > > > > > > Alan Stern > > > > Got it. I'm just confused by the USB 2.0 spec 11.24.2.7.2 that > > "Hubs may allow setting of the status change bits with a SetPortFeature() > > request for diagnostic purposes." > > Yeah, I don't think very many hubs actually do that. > > > So for this case, I think USB_QUIRK_RESET_RESUME would be a > > better option to at least bring back the port. Any suggestion about what > > kind of test I can do on this hub? Thanks > > I'm not sure what you're proposing. > > If (as mentioned above) the hub doesn't handle the > Set-Port-Feature(suspend) request properly, then we should avoid issuing > that request. Which means runtime suspend attempts should not be > allowed, as in your most recent patch. > > Alan Stern Sorry that I didn't make myself clear. I found that if I applied RESET_RESUME quirk on the problematic hub, the Set-Port-Feature(suspend) timeout error disappeared. SInce the timeout is not happening for each suspend by default, I suspect maybe reset-resume take everything back to clean state for the hub and the Set-Port-Feature(suspend) can be taken care of w/o problems. I didn't like RESET_RESUME because runtime PM would not work on the quirked device. But if the Set-Port-Feature(suspend) can't be handled and skipped, I can't expect the runtime PM to work for all devices connected to the hub either. Is that right? If what I proposed in the patch can not get better result than existing quirk, I think using the RESET_RESUME would be a better option. Any suggestions? Chris
