2023-08-18 20:13:43

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] xhci: Disable connect, disconnect and over-current wakeup on system suspend

On Fri, Aug 18, 2023 at 08:01:54AM +0800, Kai-Heng Feng wrote:
> On Thu, Aug 17, 2023 at 10:22 PM Alan Stern <[email protected]> wrote:
> >
> > On Thu, Aug 17, 2023 at 10:07:37AM -0400, Alan Stern wrote:
> > > On Thu, Aug 17, 2023 at 05:33:05PM +0800, Kai-Heng Feng wrote:
> > > > HP ProOne 440 G10 AIO sometimes cannot suspend as xHCI wakes up the
> > > > system:
> > > > [ 445.814574] hub 2-0:1.0: hub_suspend
> > > > [ 445.814652] usb usb2: bus suspend, wakeup 0
> > > > [ 445.824629] xhci_hcd 0000:00:14.0: Port change event, 1-11, id 11, portsc: 0x202a0
> > >
> > > What is the meaning of the 0x202a0 bits? What caused this wakeup?
> >
> > And more to the point, given that the previous line says "wakeup 0", why
> > should any port change event cause a wakeup?
>
> I think the controller and roothub have to deal with the interrupt
> about disconnecting regardless of the remote wakeup setting.

This seems to contradict what you wrote in an earlier email:

> > If remote wakeup isn't enabled then the do_wakeup variable will be 0,
> > so your patch wouldn't make any difference. The question is what
> > happens when remote wakeup _is_ enabled.
>
> Nothing happens either per my testing.
>
> For USB keyboard, the remote wakeup is enabled, unplugging it when
> suspend is suspended doesn't wake the system up, despite of PORT_WKDISC_E being set.
> Plugging it back doesn't wake the system up either, despite of PORT_WKCONN_E.

You appear to be saying that when wakeup is disabled, unplugging a
device will wake up the system -- but when wakeup is enabled, unplugging
a device will not wake up the system!

Is that really what you meant? It doesn't make sense. Probably means
there's a bug in the HCD.

The point I'm trying to get at is that if wakeups are disabled for both
the host controller and the root hub then _nothing_ should generate an
interrupt or wakeup request. Not pressing a key, not unplugging a
device... nothing. But if wakeup _is_ enabled for both the controller
and the root hub, then any of those actions should generate an interrupt
and wake up the system.

If wakeup is enabled for the host controller but not for the root hub,
then unplugging a device from the root hub should not generate a wakeup
request or an interrupt. But things like pressing a key on a
wakeup-enabled keyboard should. (In other words, the root hub shouldn't
generate any wakeup requests on its own but it should relay wakeup
requests that it receives from downstream devices.) However, it's
understandable if the system doesn't behave properly in this case since
it's kind of an odd situation.

Alan Stern


2023-08-20 08:38:04

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] xhci: Disable connect, disconnect and over-current wakeup on system suspend

On Fri, Aug 18, 2023 at 11:08 AM Alan Stern <[email protected]> wrote:
>
> On Fri, Aug 18, 2023 at 08:01:54AM +0800, Kai-Heng Feng wrote:
> > On Thu, Aug 17, 2023 at 10:22 PM Alan Stern <[email protected]> wrote:
> > >
> > > On Thu, Aug 17, 2023 at 10:07:37AM -0400, Alan Stern wrote:
> > > > On Thu, Aug 17, 2023 at 05:33:05PM +0800, Kai-Heng Feng wrote:
> > > > > HP ProOne 440 G10 AIO sometimes cannot suspend as xHCI wakes up the
> > > > > system:
> > > > > [ 445.814574] hub 2-0:1.0: hub_suspend
> > > > > [ 445.814652] usb usb2: bus suspend, wakeup 0
> > > > > [ 445.824629] xhci_hcd 0000:00:14.0: Port change event, 1-11, id 11, portsc: 0x202a0
> > > >
> > > > What is the meaning of the 0x202a0 bits? What caused this wakeup?
> > >
> > > And more to the point, given that the previous line says "wakeup 0", why
> > > should any port change event cause a wakeup?
> >
> > I think the controller and roothub have to deal with the interrupt
> > about disconnecting regardless of the remote wakeup setting.
>
> This seems to contradict what you wrote in an earlier email:

Nothing is contradicting, what I mentioned is the wakeup of USB
touchpanel, not the controller or root hub.

>
> > > If remote wakeup isn't enabled then the do_wakeup variable will be 0,
> > > so your patch wouldn't make any difference. The question is what
> > > happens when remote wakeup _is_ enabled.
> >
> > Nothing happens either per my testing.
> >
> > For USB keyboard, the remote wakeup is enabled, unplugging it when
> > suspend is suspended doesn't wake the system up, despite of PORT_WKDISC_E being set.
> > Plugging it back doesn't wake the system up either, despite of PORT_WKCONN_E.
>
> You appear to be saying that when wakeup is disabled, unplugging a
> device will wake up the system -- but when wakeup is enabled, unplugging
> a device will not wake up the system!

No, what I was saying is that when PORT_WKCONN_E and PORT_WKDISC_E are
set, plugging/unplugging USB doesn't wake up the system from suspended
state.
What it really does for this case is to hinder the suspending process.

>
> Is that really what you meant? It doesn't make sense. Probably means
> there's a bug in the HCD.
>
> The point I'm trying to get at is that if wakeups are disabled for both
> the host controller and the root hub then _nothing_ should generate an
> interrupt or wakeup request. Not pressing a key, not unplugging a
> device... nothing. But if wakeup _is_ enabled for both the controller
> and the root hub, then any of those actions should generate an interrupt
> and wake up the system.

Like above, the wakeup I mentioned is on the USB touchpanel itself,
not on the controller and roothub.
There's no IRQ generated when controller's wakeup is disabled.

>
> If wakeup is enabled for the host controller but not for the root hub,
> then unplugging a device from the root hub should not generate a wakeup
> request or an interrupt. But things like pressing a key on a
> wakeup-enabled keyboard should. (In other words, the root hub shouldn't
> generate any wakeup requests on its own but it should relay wakeup
> requests that it receives from downstream devices.) However, it's
> understandable if the system doesn't behave properly in this case since
> it's kind of an odd situation.

Do you mean when the system is suspended, or system is still suspending?
The issue only happens when the system is suspending.

Kai-Heng

>
> Alan Stern