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

[Kai-Heng Feng]

On Fri, Aug 18, 2023 at 11:19 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Aug 18, 2023 at 08:00:39AM +0800, Kai-Heng Feng wrote:
> > On Thu, Aug 17, 2023 at 10:07 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Thu, Aug 17, 2023 at 05:33:05PM +0800, Kai-Heng Feng wrote:
> > > > The system is designed to let display and touchpanel share the same
> > > > power source, so when the display becomes off, the USB touchpanel also
> > > > lost its power and disconnect itself from USB bus. That doesn't play
> > > > well when most Desktop Environment lock and turnoff the display right
> > > > before entering system suspend.
> > >
> > > I don't see why that should cause any trouble.  The display gets locked
> > > and turned off, the touchpanel disconnects from the USB bus, and then
> > > the system goes into suspend.  Why would there be a wakeup signal at
> > > this point?
> >
> > The disconnecting can happens during the system suspend process, so
> > the suspend process is aborted.
>
> Maybe these systems need to add a little delay when the display is
> turned off, in order to give the touchpanel time to disconnect before
> the system suspend begins.

Unfortunately the hardware can't be changed.

>
> > > > So for system-wide suspend, also disable connect, disconnect and
> > > > over-current wakeup to prevent spurious wakeup.
> > >
> > > Whether to disable these things is part of the userspace policy.  The
> > > kernel should not make the decision; the user does by enabling or
> > > disabling wakeups.
> >
> > The power/wakeup is already disabled.
>
> In that case the root hub should not generate a wakeup request in
> response to the touchpanel disconnecting.

Here's the wakeup setting when the issue happens:
controller - wakeup enabled
root hub: wakeup disabled
touchpanel: wakeup disabled

>
> > The disconnecting event is from roothub and if roothub wakeup is
> > disabled, other USB devices lose the ability to wake the system up
> > from system suspend.
>
> That shouldn't happen either.  Disabling wakeup on the root hub should
> not prevent the root hub from relaying wakeup requests it receives from
> downstream devices.  It should merely prevent the root hub from
> generating its own wakeup requests for connects, disconnects, and
> over-current events.

Sorry, it was meant to be the xHCI controller. The didn't make the
difference clear.

>
> It sounds like the xhci root-hub code isn't doing the right thing, at
> least, not on your systems.

I still don't fully understand why removing PORT_WAKE_BITS is not
right in this case.

Kai-Heng

>
> Alan Stern