Thread (2 messages) 2 messages, 2 authors, 2016-07-27

Re: [PATCH 1/2] input/serio/i8042: Skip selftest on i8042 controller in some ASUS laptops

From: Marcos Souza <hidden>
Date: 2016-07-27 01:13:52

Possibly related (same subject, not in this thread)

Hi Dmitry,

On Mon, Jul 25, 2016 at 3:48 PM, Marcos Souza
[off-list ref] wrote:
Hi Dmitry,

On Mon, Jul 25, 2016 at 3:22 PM Dmitry Torokhov [off-list ref]
wrote:
quoted
[ resending to all - mutt is for some reason confused about recipent
list ]
On Mon, Jul 25, 2016 at 02:20:16PM -0300, Marcos Paulo de Souza wrote:
quoted
On suspend/resume cycle, selftest is executed on to reset i8042
controller. But
when this is done in these devices, posterior calls to detect/init
functions
to elantech driver fails. Skipping selftest fixes this problem.

An easier step to reproduce this problem is adding i8042.reset=1 as a
kernel
parameter. On problematic laptops, it'll make the system to start with
the
touchpad already stucked, since psmouse_probe forcibly calls the
selftest function.

This patch was inspired by John Hiesey's change[1].

[1]: https://marc.info/?l=linux-input&m=144312209020616&w=2

Fixes: "ETPS/2 Elantech Touchpad dies after resume from suspend"
(https://bugzilla.kernel.org/show_bug.cgi?id=7739)
*sigh* You just cant win, they'll manage to mess up the firmware one way
or another :( Witness:

They're experts on messing things up :(
quoted

"
commit 1ca56e513a9fd356d5a9e0de45dbe0e189e00386
Author: Dmitry Torokhov [off-list ref]
Date:   Tue Jul 20 20:25:34 2010 -0700

    Input: i8042 - reset keyboard controller wehen resuming from S2R

    Some laptops, such as Lenovo 3000 N100, require keyboard controller
reset
    in order to have touchpad operable after suspend to RAM. Even if box
does
    not need the reset it should be safe to do so, so instead of chasing
    after misbehaving boxes and grow DMI tables, let's reset the
controller
    unconditionally.

    Reported-and-tested-by: Jerome Lacoste [off-list ref]
    Signed-off-by: Dmitry Torokhov [off-list ref]
"

so apparently it is not safe to reset the controller on resume. Oh joy.

The issue I have here is selftest is the same as reset, and we already
have i8042_reset flag, so 2 flags controlling the same functionality is
not great. Could we extend i8042 to an enum, like:

I8042_RESET_ALWAYS
I8042_RESET_NEVER
I8042_RESET_ON_RESUME (default)

and have a custom module parameter for it?

i8042.reset and i8042_reset={1|Y|y} would result in I8042_RESET_ALWAYS
i8042.reset={0|N|n} would result in I8042_RESET_NEVER
Without i8042.reset we just reset it when resuming from S3.

What do you think?

Seems to be a very good approach. I'll send a new version.
You asked me to adapt my previous patch, making it accept:

i8042.reset
i8042.reset={1,y,Y}
i8042.reset={0,n,N}

But, when using a charp as module parameter, it doesn't allocate
memory to i8042_reset when we don't use the equal sign. So, in this
case I can't tell if the user is using i8042.reset, or don't any
parameter (in this case it means to use I8042_RESET_ON_RESUME)

So, in this case, should we just change i8042.reset to accept 1,y,n?
This should solve the problem too (although I think people who already
use such parameter will complain...)

What do you think?

I attached my current diff here, by using a new kernel parameter
reset_mode. Besides the new parameter,It works great if I remove the
comment about the ASUS DMI info.

Thanks,
quoted

Thanks.

--
Dmitry


-- 
Att,

Marcos Paulo de Souza
Github: https://github.com/marcosps/

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help