[PATCHv3 0/2] Skip selftest on Asus laptops

[PATCHv3 0/2] Skip selftest on Asus laptops

[Marcos Paulo de Souza]

So, here we are, third version.

Changes since v2:
* Due to the number of devices with problems related to selftest, let's
  disable selftest for any laptop device
* Fix condition on reset/resume calls of i8042 controller
Changes since v1: 
* Use reset parameter, instead of adding reset_mode parameter.
* Change reset parameter from bool to unsigned int.

Please let me know if I missed something.

Marcos Paulo de Souza (2):
  kernel-parameters: Update i8042.reset parameter documentation
  input/serio/i8042.c: Skipt selftest on ASUS laptops

 Documentation/kernel-parameters.txt   |  7 ++++++-
 drivers/input/serio/i8042-x86ia64io.h | 21 +++++++++++++++++--
 drivers/input/serio/i8042.c           | 38 +++++++++++++++++++++++++++++------
 3 files changed, 57 insertions(+), 9 deletions(-)

-- 
2.7.4

[PATCHv3 1/2] kernel-parameters: Update i8042.reset parameter documentation

[Marcos Paulo de Souza]

---
 Documentation/kernel-parameters.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 46c030a..9526854 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1457,7 +1457,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	i8042.nopnp	[HW] Don't use ACPIPnP / PnPBIOS to discover KBD/AUX
 			     controllers
 	i8042.notimeout	[HW] Ignore timeout condition signalled by controller
-	i8042.reset	[HW] Reset the controller during init and cleanup
+	i8042.reset	[HW] Reset the controller during init and cleanup, only
+			     on cleanup, or never reset
+			Format: { 1 | Y | y | 0 | N | n }
+			1, Y, y: init and cleanup
+			0, N, n: don't reset controller
+			Default: only on cleanup
 	i8042.unlock	[HW] Unlock (ignore) the keylock
 	i8042.kbdreset  [HW] Reset device connected to KBD port
 
-- 
2.7.4

[PATCHv3 2/2] input/serio/i8042.c: Skipt selftest on ASUS laptops

[Marcos Paulo de Souza]

On suspend/resume cycle, selftest is executed to reset i8042 controller. But
when this is done in Asus 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 Asus laptops, it'll make the system to start with the
touchpad already stuck, since psmouse_probe forcibly calls the
selftest function.

This patch was inspired by John Hiesey's change[1], but, since this problem
affects a lot of models of Asus (A455LD, K401LB, K501LX, V502LX, X302LA, X450LCP,
X455LAB, X455LDB, X455LF, V502LX), let's just disable self tests for any Asus laptop.

[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=107971)

Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
---
 drivers/input/serio/i8042-x86ia64io.h | 21 +++++++++++++++++--
 drivers/input/serio/i8042.c           | 38 +++++++++++++++++++++++++++++------
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index 68f5f4a..43008b6 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -510,6 +510,17 @@ static const struct dmi_system_id __initconst i8042_dmi_nomux_table[] = {
 	{ }
 };
 
+/*
+ * On Asus laptops, just running self tests cause problems.
+ */
+static const struct dmi_system_id __initconst i8042_dmi_noselftest_table[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+		},
+	},
+	{ }
+};
 static const struct dmi_system_id __initconst i8042_dmi_reset_table[] = {
 	{
 		/* MSI Wind U-100 */
@@ -1076,8 +1087,14 @@ static int __init i8042_platform_init(void)
 #endif
 
 #ifdef CONFIG_X86
-	if (dmi_check_system(i8042_dmi_reset_table))
-		i8042_reset = true;
+	/* Honor module parameter when value is not default */
+	if (i8042_reset == I8042_RESET_ON_RESUME) {
+		if (dmi_check_system(i8042_dmi_reset_table))
+			i8042_reset = I8042_RESET_ALWAYS;
+
+		if (dmi_check_system(i8042_dmi_noselftest_table))
+			i8042_reset = 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 b4d3408..0dfe754 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -48,9 +48,32 @@ static bool i8042_unlock;
 module_param_named(unlock, i8042_unlock, bool, 0);
 MODULE_PARM_DESC(unlock, "Ignore keyboard lock.");
 
-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 = I8042_RESET_ON_RESUME;
+static int i8042_set_reset(const char *val, const struct kernel_param *kp)
+{
+	unsigned int ret = I8042_RESET_ON_RESUME;
+        if (!val || !strncmp(val, "1", 1) || !strncasecmp(val, "y", 1))
+		ret = I8042_RESET_ALWAYS;
+	else if (!strncmp(val, "0", 1) || !strncasecmp(val, "n", 1))
+		ret = I8042_RESET_NEVER;
+
+        *((unsigned int *)kp->arg) = ret;
+
+        return 0;
+}
+
+static const struct kernel_param_ops param_ops_reset_param = {
+	.flags = KERNEL_PARAM_OPS_FL_NOARG,
+	.set = i8042_set_reset,
+};
+#define param_check_reset_param(name, p) __param_check(name, p, unsigned int)
+module_param_named(reset, i8042_reset, reset_param, 0);
+MODULE_PARM_DESC(reset, "Reset controller on resume, cleanup or both");
 
 static bool i8042_direct;
 module_param_named(direct, i8042_direct, bool, 0);
@@ -890,6 +913,9 @@ static int i8042_controller_selftest(void)
 	unsigned char param;
 	int i = 0;
 
+	if (i8042_reset == I8042_RESET_NEVER)
+		return 0;
+
 	/*
 	 * We try this 5 times; on some really fragile systems this does not
 	 * take the first time...
@@ -1044,7 +1070,7 @@ static void i8042_controller_reset(bool force_reset)
  * Reset the controller if requested.
  */
 
-	if (i8042_reset || force_reset)
+	if (i8042_reset != I8042_RESET_NEVER || force_reset)
 		i8042_controller_selftest();
 
 /*
@@ -1118,7 +1144,7 @@ static int i8042_controller_resume(bool force_reset)
 	if (error)
 		return error;
 
-	if (i8042_reset || force_reset) {
+	if (i8042_reset != I8042_RESET_NEVER || force_reset) {
 		error = i8042_controller_selftest();
 		if (error)
 			return error;
@@ -1481,7 +1507,7 @@ static int __init i8042_probe(struct platform_device *dev)
 
 	i8042_platform_device = dev;
 
-	if (i8042_reset) {
+	if (i8042_reset == I8042_RESET_ALWAYS) {
 		error = i8042_controller_selftest();
 		if (error)
 			return error;
-- 
2.7.4

Re: [PATCHv3 2/2] input/serio/i8042.c: Skipt selftest on ASUS laptops

[ulrik.debie-os]

Hi Marcos,

Given the fact that there are already Asus laptops that do not have a problem
with self test, is there no chance of regression on those that they would stop functioning with this patch ?

Anyone with a Asus K53SV, G46VW, G750JX, TP500LN, X750JN, UX31 or UX32VD wants to give this patch a try ?

Thanks,
Kind regards

On Wed, Aug 10, 2016 at 12:25:45AM -0300, Marcos Paulo de Souza wrote:
> Date:	Wed, 10 Aug 2016 00:25:45 -0300
> From: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> To: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org
> Cc: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> Subject: [PATCHv3 2/2] input/serio/i8042.c: Skipt selftest on ASUS laptops
> X-Mailer: git-send-email 2.7.4
> X-Mailing-List:	linux-input@vger.kernel.org
> 
> On suspend/resume cycle, selftest is executed to reset i8042 controller. But
> when this is done in Asus 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 Asus laptops, it'll make the system to start with the
> touchpad already stuck, since psmouse_probe forcibly calls the
> selftest function.
> 
> This patch was inspired by John Hiesey's change[1], but, since this problem
> affects a lot of models of Asus (A455LD, K401LB, K501LX, V502LX, X302LA, X450LCP,
> X455LAB, X455LDB, X455LF, V502LX), let's just disable self tests for any Asus laptop.
> 
> [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=107971)
> 
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> ---
>  drivers/input/serio/i8042-x86ia64io.h | 21 +++++++++++++++++--
>  drivers/input/serio/i8042.c           | 38 +++++++++++++++++++++++++++++------
>  2 files changed, 51 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> index 68f5f4a..43008b6 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -510,6 +510,17 @@ static const struct dmi_system_id __initconst i8042_dmi_nomux_table[] = {
>  	{ }
>  };
>  
> +/*
> + * On Asus laptops, just running self tests cause problems.
> + */
> +static const struct dmi_system_id __initconst i8042_dmi_noselftest_table[] = {
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +		},
> +	},
> +	{ }
> +};
>  static const struct dmi_system_id __initconst i8042_dmi_reset_table[] = {
>  	{
>  		/* MSI Wind U-100 */
> @@ -1076,8 +1087,14 @@ static int __init i8042_platform_init(void)
>  #endif
>  
>  #ifdef CONFIG_X86
> -	if (dmi_check_system(i8042_dmi_reset_table))
> -		i8042_reset = true;
> +	/* Honor module parameter when value is not default */
> +	if (i8042_reset == I8042_RESET_ON_RESUME) {
> +		if (dmi_check_system(i8042_dmi_reset_table))
> +			i8042_reset = I8042_RESET_ALWAYS;
> +
> +		if (dmi_check_system(i8042_dmi_noselftest_table))
> +			i8042_reset = 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 b4d3408..0dfe754 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -48,9 +48,32 @@ static bool i8042_unlock;
>  module_param_named(unlock, i8042_unlock, bool, 0);
>  MODULE_PARM_DESC(unlock, "Ignore keyboard lock.");
>  
> -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 = I8042_RESET_ON_RESUME;
> +static int i8042_set_reset(const char *val, const struct kernel_param *kp)
> +{
> +	unsigned int ret = I8042_RESET_ON_RESUME;
> +        if (!val || !strncmp(val, "1", 1) || !strncasecmp(val, "y", 1))
> +		ret = I8042_RESET_ALWAYS;
> +	else if (!strncmp(val, "0", 1) || !strncasecmp(val, "n", 1))
> +		ret = I8042_RESET_NEVER;
> +
> +        *((unsigned int *)kp->arg) = ret;
> +
> +        return 0;
> +}
> +
> +static const struct kernel_param_ops param_ops_reset_param = {
> +	.flags = KERNEL_PARAM_OPS_FL_NOARG,
> +	.set = i8042_set_reset,
> +};
> +#define param_check_reset_param(name, p) __param_check(name, p, unsigned int)
> +module_param_named(reset, i8042_reset, reset_param, 0);
> +MODULE_PARM_DESC(reset, "Reset controller on resume, cleanup or both");
>  
>  static bool i8042_direct;
>  module_param_named(direct, i8042_direct, bool, 0);
> @@ -890,6 +913,9 @@ static int i8042_controller_selftest(void)
>  	unsigned char param;
>  	int i = 0;
>  
> +	if (i8042_reset == I8042_RESET_NEVER)
> +		return 0;
> +
>  	/*
>  	 * We try this 5 times; on some really fragile systems this does not
>  	 * take the first time...
> @@ -1044,7 +1070,7 @@ static void i8042_controller_reset(bool force_reset)
>   * Reset the controller if requested.
>   */
>  
> -	if (i8042_reset || force_reset)
> +	if (i8042_reset != I8042_RESET_NEVER || force_reset)
>  		i8042_controller_selftest();
>  
>  /*
> @@ -1118,7 +1144,7 @@ static int i8042_controller_resume(bool force_reset)
>  	if (error)
>  		return error;
>  
> -	if (i8042_reset || force_reset) {
> +	if (i8042_reset != I8042_RESET_NEVER || force_reset) {
>  		error = i8042_controller_selftest();
>  		if (error)
>  			return error;
> @@ -1481,7 +1507,7 @@ static int __init i8042_probe(struct platform_device *dev)
>  
>  	i8042_platform_device = dev;
>  
> -	if (i8042_reset) {
> +	if (i8042_reset == I8042_RESET_ALWAYS) {
>  		error = i8042_controller_selftest();
>  		if (error)
>  			return error;
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 2/2] input/serio/i8042.c: Skipt selftest on ASUS laptops

[Marcos Paulo de Souza]

Hi Ulrik,

On Tue, Aug 23, 2016 at 10:20:08PM +0200, ulrik.debie-os@e2big.org wrote:
> Hi Marcos,
> 
> Given the fact that there are already Asus laptops that do not have a problem
> with self test, is there no chance of regression on those that they would stop functioning with this patch ?

I really don't know. This just worked for me, and as it is a really
annoying bug (that can be seen be the large number of models affected),
I just wanted to check with more people about other models, if this
patch break anything.

> 
> Anyone with a Asus K53SV, G46VW, G750JX, TP500LN, X750JN, UX31 or UX32VD wants to give this patch a try ?

That would solve our doubts about fixing some models and breaking new
ones. I hope that someone could make a test on any of these devices.

Thanks,

> 
> Thanks,
> Kind regards
> 
> On Wed, Aug 10, 2016 at 12:25:45AM -0300, Marcos Paulo de Souza wrote:
> > Date:	Wed, 10 Aug 2016 00:25:45 -0300
> > From: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> > To: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org
> > Cc: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> > Subject: [PATCHv3 2/2] input/serio/i8042.c: Skipt selftest on ASUS laptops
> > X-Mailer: git-send-email 2.7.4
> > X-Mailing-List:	linux-input@vger.kernel.org
> > 
> > On suspend/resume cycle, selftest is executed to reset i8042 controller. But
> > when this is done in Asus 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 Asus laptops, it'll make the system to start with the
> > touchpad already stuck, since psmouse_probe forcibly calls the
> > selftest function.
> > 
> > This patch was inspired by John Hiesey's change[1], but, since this problem
> > affects a lot of models of Asus (A455LD, K401LB, K501LX, V502LX, X302LA, X450LCP,
> > X455LAB, X455LDB, X455LF, V502LX), let's just disable self tests for any Asus laptop.
> > 
> > [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=107971)
> > 
> > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> > ---
> >  drivers/input/serio/i8042-x86ia64io.h | 21 +++++++++++++++++--
> >  drivers/input/serio/i8042.c           | 38 +++++++++++++++++++++++++++++------
> >  2 files changed, 51 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> > index 68f5f4a..43008b6 100644
> > --- a/drivers/input/serio/i8042-x86ia64io.h
> > +++ b/drivers/input/serio/i8042-x86ia64io.h
> > @@ -510,6 +510,17 @@ static const struct dmi_system_id __initconst i8042_dmi_nomux_table[] = {
> >  	{ }
> >  };
> >  
> > +/*
> > + * On Asus laptops, just running self tests cause problems.
> > + */
> > +static const struct dmi_system_id __initconst i8042_dmi_noselftest_table[] = {
> > +	{
> > +		.matches = {
> > +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> > +		},
> > +	},
> > +	{ }
> > +};
> >  static const struct dmi_system_id __initconst i8042_dmi_reset_table[] = {
> >  	{
> >  		/* MSI Wind U-100 */
> > @@ -1076,8 +1087,14 @@ static int __init i8042_platform_init(void)
> >  #endif
> >  
> >  #ifdef CONFIG_X86
> > -	if (dmi_check_system(i8042_dmi_reset_table))
> > -		i8042_reset = true;
> > +	/* Honor module parameter when value is not default */
> > +	if (i8042_reset == I8042_RESET_ON_RESUME) {
> > +		if (dmi_check_system(i8042_dmi_reset_table))
> > +			i8042_reset = I8042_RESET_ALWAYS;
> > +
> > +		if (dmi_check_system(i8042_dmi_noselftest_table))
> > +			i8042_reset = 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 b4d3408..0dfe754 100644
> > --- a/drivers/input/serio/i8042.c
> > +++ b/drivers/input/serio/i8042.c
> > @@ -48,9 +48,32 @@ static bool i8042_unlock;
> >  module_param_named(unlock, i8042_unlock, bool, 0);
> >  MODULE_PARM_DESC(unlock, "Ignore keyboard lock.");
> >  
> > -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 = I8042_RESET_ON_RESUME;
> > +static int i8042_set_reset(const char *val, const struct kernel_param *kp)
> > +{
> > +	unsigned int ret = I8042_RESET_ON_RESUME;
> > +        if (!val || !strncmp(val, "1", 1) || !strncasecmp(val, "y", 1))
> > +		ret = I8042_RESET_ALWAYS;
> > +	else if (!strncmp(val, "0", 1) || !strncasecmp(val, "n", 1))
> > +		ret = I8042_RESET_NEVER;
> > +
> > +        *((unsigned int *)kp->arg) = ret;
> > +
> > +        return 0;
> > +}
> > +
> > +static const struct kernel_param_ops param_ops_reset_param = {
> > +	.flags = KERNEL_PARAM_OPS_FL_NOARG,
> > +	.set = i8042_set_reset,
> > +};
> > +#define param_check_reset_param(name, p) __param_check(name, p, unsigned int)
> > +module_param_named(reset, i8042_reset, reset_param, 0);
> > +MODULE_PARM_DESC(reset, "Reset controller on resume, cleanup or both");
> >  
> >  static bool i8042_direct;
> >  module_param_named(direct, i8042_direct, bool, 0);
> > @@ -890,6 +913,9 @@ static int i8042_controller_selftest(void)
> >  	unsigned char param;
> >  	int i = 0;
> >  
> > +	if (i8042_reset == I8042_RESET_NEVER)
> > +		return 0;
> > +
> >  	/*
> >  	 * We try this 5 times; on some really fragile systems this does not
> >  	 * take the first time...
> > @@ -1044,7 +1070,7 @@ static void i8042_controller_reset(bool force_reset)
> >   * Reset the controller if requested.
> >   */
> >  
> > -	if (i8042_reset || force_reset)
> > +	if (i8042_reset != I8042_RESET_NEVER || force_reset)
> >  		i8042_controller_selftest();
> >  
> >  /*
> > @@ -1118,7 +1144,7 @@ static int i8042_controller_resume(bool force_reset)
> >  	if (error)
> >  		return error;
> >  
> > -	if (i8042_reset || force_reset) {
> > +	if (i8042_reset != I8042_RESET_NEVER || force_reset) {
> >  		error = i8042_controller_selftest();
> >  		if (error)
> >  			return error;
> > @@ -1481,7 +1507,7 @@ static int __init i8042_probe(struct platform_device *dev)
> >  
> >  	i8042_platform_device = dev;
> >  
> > -	if (i8042_reset) {
> > +	if (i8042_reset == I8042_RESET_ALWAYS) {
> >  		error = i8042_controller_selftest();
> >  		if (error)
> >  			return error;
> > -- 
> > 2.7.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-input" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html