Message-ID: <9748c70b1e506f17ad986e4fe6b20a291a980205.camel@vmware.com>
Date:   Fri, 3 Feb 2023 02:53:24 +0000
From:   Zack Rusin <zackr@...are.com>
To:     "dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>
CC:     "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Michael Banack <banackm@...are.com>
Subject: Re: Suspend with ps2 keyboard/mice

On Wed, 2023-02-01 at 22:12 -0800, dmitry.torokhov@...il.com wrote:
> On Thu, Feb 02, 2023 at 03:23:34AM +0000, Zack Rusin wrote:
> > So the kernels i8042 module specifically calls device_set_wakeup_enable only on
> > the
> > keyboard port. Then because of the a1cec0617738 psbase leaves the mouse enabled
> > on
> > suspend which, in case of a mouse move/click right before the suspend makes sure
> > Linux will never wake up.
> > 
> > That's because the dataport will be effectively latched to the mouse (mouse irq
> > wasn't serviced in the guest) and it won't be possible to deliver a keyboard
> > interrupt anymore, and mouse interrupts won't wake Linux.
> > 
> > Our suggestion would be to simply remove the:
> > ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> > added in a1cec0617738 . I don't have access to HP nx7400 from 2007 but it seems
> > like
> > that code breaks every other well formed ps/2 controller with suspend/wakeup.
> > 
> > I think the ps2 mouse has to be disabled on suspend in order to be able to get
> > the
> > keyboard interrupts required to wake up. Otherwise if the mouse is supposed to
> > be
> > really enabled (but mouse irq's will be ignored), then how does the PS/2
> > controller
> > decide whether to return mouse or keyboard data when they do the data port read?
> 
> IIRC the reason we leave mouse enabled is because firmware on multiple
> devices was unhappy if the mouse was disabled on suspend. It was not
> only nx7400, but others as well. I am not sure if newer generation
> devices exhibit the same problem or not.
> 
> Regarding inability to deliver keyboard interrupt: I am not sure why you
> are saying that Embedded Controller (EC) can not raise IRQ1 and IRQ12
> together. Also, they do not have to be wakeup sources, I think on some
> designs these signals were wired to other pins as well, and those were
> actually configured to wake up the AP.
> 
> Both mouse and keyboard data are coming from the same data port, so it
> really up to the Embedded Controller to figure out what data it wants to
> send first, and Linux is using the same interrupt handler for both IRQ1
> and IRQ12, and can dispatch data as needed. But you do not need to read
> the data to wake up the system. You just need EC to actually signal
> interrupt that is configured as a wakeup source.
> 
> I think you should look into how you implement EC and make sure it does
> not get stuck.

That makes sense. Thanks a lot for going over this. We were being overly cautious
with the IRQ delivery and would not deliver IRQ's on PS/2 unless the dataport had
data from the respective device. We went back now and checked Win98 and WinXP to
make sure that they actually don't depend on the IRQ number that's delivered to
figure out the routing and it looks like they properly do check the status register.
So we went ahead and fixed it in our code. 
For the kernel I will write a small patch just for vmmouse that overwrites the serio
cleanup to avoid the enabling of the mouse on suspend just to fix it for folks who
won't be running our latest hypervisors and everything should be fine.

z

Powered by blists - more mailing lists