Input: i8042 - Fix the selftest retry logic
diff mbox series

Message ID 20200305064423.16196-1-acelan.kao@canonical.com
State New
Headers show
Series
  • Input: i8042 - Fix the selftest retry logic
Related show

Commit Message

AceLan Kao March 5, 2020, 6:44 a.m. UTC
It returns -NODEV at the first selftest timeout, so the retry logic
doesn't work. Move the return outside of the while loop to make it real
retry 5 times before returns -ENODEV.

BTW, the origin loop will retry 6 times, also fix this.

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/input/serio/i8042.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Dmitry Torokhov March 6, 2020, 4:16 a.m. UTC | #1
Hi AceLan,

On Thu, Mar 05, 2020 at 02:44:23PM +0800, AceLan Kao wrote:
> It returns -NODEV at the first selftest timeout, so the retry logic
> doesn't work. Move the return outside of the while loop to make it real
> retry 5 times before returns -ENODEV.

The retry logic here was for the controller not returning the expected
selftest value, not the controller refusing to communicate at all.

Could you pease tell me what device requires this change?

Thanks.
AceLan Kao March 6, 2020, 6:40 a.m. UTC | #2
Hi Dmitry,

We have a Dell desktop with ps2 addon card, after S3, the ps2 keyboard
lost function and got below errors
Jan 15 07:10:08 Rh-MT kernel: [  346.575353] i8042: i8042 controller
selftest timeout
Jan 15 07:10:08 Rh-MT kernel: [  346.575358] PM: Device i8042 failed
to resume: error -19

Adding this patch, I found the selftest passes at the second retry and
the keyboard continue working fine.

Best regards,
AceLan Kao.

Dmitry Torokhov <dmitry.torokhov@gmail.com> 於 2020年3月6日 週五 下午12:16寫道:
>
> Hi AceLan,
>
> On Thu, Mar 05, 2020 at 02:44:23PM +0800, AceLan Kao wrote:
> > It returns -NODEV at the first selftest timeout, so the retry logic
> > doesn't work. Move the return outside of the while loop to make it real
> > retry 5 times before returns -ENODEV.
>
> The retry logic here was for the controller not returning the expected
> selftest value, not the controller refusing to communicate at all.
>
> Could you pease tell me what device requires this change?
>
> Thanks.
>
> --
> Dmitry
You-Sheng Yang March 10, 2020, 3:35 a.m. UTC | #3
On 2020-03-05 14:44, AceLan Kao wrote:
> @@ -955,7 +954,9 @@ static int i8042_controller_selftest(void)
>  		dbg("i8042 controller selftest: %#x != %#x\n",
>  		    param, I8042_RET_CTL_TEST);
>  		msleep(50);
> -	} while (i++ < 5);
> +	} while (++i < 5);
> +	if (i == 5)
> +		return -ENODEV;

I would like to propose a V2 for this. The original logic allows
continuation to device probe when selftest returns a different value
than expected, and this is no longer available with this patch.

>  #ifdef CONFIG_X86
>  	/*
> 

You-Sheng Yang

Patch
diff mbox series

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 20ff2bed3917..3f6433a5c8e6 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -944,10 +944,9 @@  static int i8042_controller_selftest(void)
 	 */
 	do {
 
-		if (i8042_command(&param, I8042_CMD_CTL_TEST)) {
-			pr_err("i8042 controller selftest timeout\n");
-			return -ENODEV;
-		}
+		if (i8042_command(&param, I8042_CMD_CTL_TEST))
+			pr_err("i8042 controller selftest timeout (%d/5)\n",
+			       i+1);
 
 		if (param == I8042_RET_CTL_TEST)
 			return 0;
@@ -955,7 +954,9 @@  static int i8042_controller_selftest(void)
 		dbg("i8042 controller selftest: %#x != %#x\n",
 		    param, I8042_RET_CTL_TEST);
 		msleep(50);
-	} while (i++ < 5);
+	} while (++i < 5);
+	if (i == 5)
+		return -ENODEV;
 
 #ifdef CONFIG_X86
 	/*