| Message ID | CAH0vN5JonW+PWTS4mY9Jiwp60DuMQsiYCPnrgu+i5YFgU764rA@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 26, 2016 at 6:13 PM, Marcos Souza <marcos.souza.org@gmail.com> wrote: > Hi Dmitry, > > On Mon, Jul 25, 2016 at 3:48 PM, Marcos Souza > <marcos.souza.org@gmail.com> wrote: >> Hi Dmitry, >> >> On Mon, Jul 25, 2016 at 3:22 PM Dmitry Torokhov <dmitry.torokhov@gmail.com> >> wrote: >>> >>> [ 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: >>> > 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 :( >> >>> >>> >>> " >>> commit 1ca56e513a9fd356d5a9e0de45dbe0e189e00386 >>> Author: Dmitry Torokhov <dmitry.torokhov@gmail.com> >>> 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 <jerome.lacoste@gmail.com> >>> Signed-off-by: Dmitry Torokhov <dtor@mail.ru> >>> " >>> >>> 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 think you will have to provide custom parameter parsing method. psmouse module has one, so you can use it as an example. Thanks.
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 82b42c9..ac0d5cd 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1431,6 +1431,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted. controllers i8042.notimeout [HW] Ignore timeout condition signalled by controller i8042.reset [HW] Reset the controller during init and cleanup + i8042.reset_mode + [HW] Reset the controller, always, never or just on + resume + Format: { 1 | Y | y | 0 | N | n } + 1, Y, y: Always reset controller (same as the i8042.reset) + 0, N, n: Never reset controller + Default: Just reset on S3 resume i8042.unlock [HW] Unlock (ignore) the keylock i8042.kbdreset [HW] Reset device connected to KBD port diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h index 68f5f4a..40a78d1 100644 --- a/drivers/input/serio/i8042-x86ia64io.h +++ b/drivers/input/serio/i8042-x86ia64io.h @@ -510,6 +510,60 @@ static const struct dmi_system_id __initconst i8042_dmi_nomux_table[] = { { } }; +/* + * On some hardware just running the self test causes problems. + */ +static const struct dmi_system_id __initconst i8042_dmi_noselftest_table[] = { + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "K401LB"), + }, + }, + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "K501LX"), + }, + }, + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "K501LD"), + }, + }, + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "X302LA"), + }, + }, + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "X450LCP"), + }, + }, + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "X450LD"), + }, + }, + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "X455LAB"), + }, + }, + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "X455LDB"), + }, + }, + { } +}; static const struct dmi_system_id __initconst i8042_dmi_reset_table[] = { { /* MSI Wind U-100 */ @@ -1077,7 +1131,11 @@ static int __init i8042_platform_init(void) #ifdef CONFIG_X86 if (dmi_check_system(i8042_dmi_reset_table)) - i8042_reset = true; + i8042_reset_mode = I8042_RESET_ALWAYS; + + // just disable controller reset + //if (dmi_check_system(i8042_dmi_noselftest_table)) + // i8042_reset_mode = I8042_RESET_NEVER; if (dmi_check_system(i8042_dmi_noloop_table)) i8042_noloop = true; diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c index 4541957..cfc1590 100644 --- a/drivers/input/serio/i8042.c +++ b/drivers/input/serio/i8042.c @@ -52,6 +52,17 @@ static bool i8042_reset; module_param_named(reset, i8042_reset, bool, 0); MODULE_PARM_DESC(reset, "Reset controller during init and cleanup."); +enum i8042_controller_reset_mode { + I8042_RESET_NEVER, + I8042_RESET_ALWAYS, + I8042_RESET_ON_RESUME +}; + +static unsigned int i8042_reset_mode = I8042_RESET_ON_RESUME; +static char *i8042_reset_param; +module_param_named(reset_mode, i8042_reset_param, charp, 0); +MODULE_PARM_DESC(reset_mode, "Reset controller mode, being always, never or just on resume"); + static bool i8042_direct; module_param_named(direct, i8042_direct, bool, 0); MODULE_PARM_DESC(direct, "Put keyboard port into non-translated mode."); @@ -890,6 +901,9 @@ static int i8042_controller_selftest(void) unsigned char param; int i = 0; + if (i8042_reset_mode == I8042_RESET_NEVER) + return 0; + /* * We try this 5 times; on some really fragile systems this does not * take the first time... @@ -1495,7 +1509,20 @@ static int __init i8042_probe(struct platform_device *dev) i8042_platform_device = dev; - if (i8042_reset) { + if (i8042_reset_param != NULL) { + if (!strncmp(i8042_reset_param, "1", 1) + || !strncasecmp(i8042_reset_param, "y", 1)) { + i8042_reset_mode = I8042_RESET_ALWAYS; + } else if (!strncmp(i8042_reset_param, "0", 1) + || !strncasecmp(i8042_reset_param, "n", 1)) { + i8042_reset_mode = I8042_RESET_NEVER; + } + } + + if (i8042_reset) + i8042_reset_mode = I8042_RESET_ALWAYS; + + if (i8042_reset_mode == I8042_RESET_ALWAYS) { error = i8042_controller_selftest(); if (error) return error;