| Message ID | 20200316092721.301429-2-vicamo.yang@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | All PS/2 ports on PS/2 Serial add-in bracket are not working after S3 | expand |
On Mon, Mar 16, 2020 at 05:27:21PM +0800, You-Sheng Yang wrote: > BugLink: https://bugs.launchpad.net/bugs/1866734 > > 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: You-Sheng Yang <vicamo.yang@canonical.com> > (backported from > https://lore.kernel.org/linux-input/20200310033640.14440-1-vicamo@gmail.com/) > --- > drivers/input/serio/i8042.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c > index 7f4592177607..cc079c5680d6 100644 > --- a/drivers/input/serio/i8042.c > +++ b/drivers/input/serio/i8042.c > @@ -927,25 +927,28 @@ static int i8042_controller_selftest(void) > { > unsigned char param; > int i = 0; > + int ret; > > /* > * We try this 5 times; on some really fragile systems this does not > * take the first time... > */ > - do { > - > - if (i8042_command(¶m, I8042_CMD_CTL_TEST)) { > - pr_info("i8042 controller selftest timeout\n"); > - return -ENODEV; > - } > + while (i++ < 5) { > > - if (param == I8042_RET_CTL_TEST) > + ret = i8042_command(¶m, I8042_CMD_CTL_TEST); > + if (ret) > + pr_info("i8042 controller selftest timeout (%d/5)\n", i); > + else if (param == I8042_RET_CTL_TEST) > return 0; > + else > + dbg("i8042 controller selftest: %#x != %#x\n", > + param, I8042_RET_CTL_TEST); > > - dbg("i8042 controller selftest: %#x != %#x\n", > - param, I8042_RET_CTL_TEST); > msleep(50); > - } while (i++ < 5); > + } > + > + if (ret) > + return -ENODEV; > > #ifdef CONFIG_X86 > /* > -- > 2.25.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 16.03.20 10:27, You-Sheng Yang wrote: > BugLink: https://bugs.launchpad.net/bugs/1866734 > > 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: You-Sheng Yang <vicamo.yang@canonical.com> > (backported from > https://lore.kernel.org/linux-input/20200310033640.14440-1-vicamo@gmail.com/) Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > drivers/input/serio/i8042.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c > index 7f4592177607..cc079c5680d6 100644 > --- a/drivers/input/serio/i8042.c > +++ b/drivers/input/serio/i8042.c > @@ -927,25 +927,28 @@ static int i8042_controller_selftest(void) > { > unsigned char param; > int i = 0; > + int ret; > > /* > * We try this 5 times; on some really fragile systems this does not > * take the first time... > */ > - do { > - > - if (i8042_command(¶m, I8042_CMD_CTL_TEST)) { > - pr_info("i8042 controller selftest timeout\n"); > - return -ENODEV; > - } > + while (i++ < 5) { > > - if (param == I8042_RET_CTL_TEST) > + ret = i8042_command(¶m, I8042_CMD_CTL_TEST); > + if (ret) > + pr_info("i8042 controller selftest timeout (%d/5)\n", i); > + else if (param == I8042_RET_CTL_TEST) > return 0; > + else > + dbg("i8042 controller selftest: %#x != %#x\n", > + param, I8042_RET_CTL_TEST); > > - dbg("i8042 controller selftest: %#x != %#x\n", > - param, I8042_RET_CTL_TEST); > msleep(50); > - } while (i++ < 5); > + } > + > + if (ret) > + return -ENODEV; > > #ifdef CONFIG_X86 > /* >
On 16.03.20 10:27, You-Sheng Yang wrote: > BugLink: https://bugs.launchpad.net/bugs/1866734 > > 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: You-Sheng Yang <vicamo.yang@canonical.com> > (backported from > https://lore.kernel.org/linux-input/20200310033640.14440-1-vicamo@gmail.com/) > --- The subject appears as this should go into Bionic main kernel, too. However the bug report has no nomination for the Bionic main kernel. Which is correct? -Stefan > drivers/input/serio/i8042.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c > index 7f4592177607..cc079c5680d6 100644 > --- a/drivers/input/serio/i8042.c > +++ b/drivers/input/serio/i8042.c > @@ -927,25 +927,28 @@ static int i8042_controller_selftest(void) > { > unsigned char param; > int i = 0; > + int ret; > > /* > * We try this 5 times; on some really fragile systems this does not > * take the first time... > */ > - do { > - > - if (i8042_command(¶m, I8042_CMD_CTL_TEST)) { > - pr_info("i8042 controller selftest timeout\n"); > - return -ENODEV; > - } > + while (i++ < 5) { > > - if (param == I8042_RET_CTL_TEST) > + ret = i8042_command(¶m, I8042_CMD_CTL_TEST); > + if (ret) > + pr_info("i8042 controller selftest timeout (%d/5)\n", i); > + else if (param == I8042_RET_CTL_TEST) > return 0; > + else > + dbg("i8042 controller selftest: %#x != %#x\n", > + param, I8042_RET_CTL_TEST); > > - dbg("i8042 controller selftest: %#x != %#x\n", > - param, I8042_RET_CTL_TEST); > msleep(50); > - } while (i++ < 5); > + } > + > + if (ret) > + return -ENODEV; > > #ifdef CONFIG_X86 > /* >
On 16.03.20 10:27, You-Sheng Yang wrote: > BugLink: https://bugs.launchpad.net/bugs/1866734 > > 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: You-Sheng Yang <vicamo.yang@canonical.com> > (backported from > https://lore.kernel.org/linux-input/20200310033640.14440-1-vicamo@gmail.com/) Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- From what I read already applied to F, unsure nomination for B. So for now only acking for E (and OEM-B fwiw). -Stefan > drivers/input/serio/i8042.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c > index 7f4592177607..cc079c5680d6 100644 > --- a/drivers/input/serio/i8042.c > +++ b/drivers/input/serio/i8042.c > @@ -927,25 +927,28 @@ static int i8042_controller_selftest(void) > { > unsigned char param; > int i = 0; > + int ret; > > /* > * We try this 5 times; on some really fragile systems this does not > * take the first time... > */ > - do { > - > - if (i8042_command(¶m, I8042_CMD_CTL_TEST)) { > - pr_info("i8042 controller selftest timeout\n"); > - return -ENODEV; > - } > + while (i++ < 5) { > > - if (param == I8042_RET_CTL_TEST) > + ret = i8042_command(¶m, I8042_CMD_CTL_TEST); > + if (ret) > + pr_info("i8042 controller selftest timeout (%d/5)\n", i); > + else if (param == I8042_RET_CTL_TEST) > return 0; > + else > + dbg("i8042 controller selftest: %#x != %#x\n", > + param, I8042_RET_CTL_TEST); > > - dbg("i8042 controller selftest: %#x != %#x\n", > - param, I8042_RET_CTL_TEST); > msleep(50); > - } while (i++ < 5); > + } > + > + if (ret) > + return -ENODEV; > > #ifdef CONFIG_X86 > /* >
Hi, it was a bug in launchpad. Once Bionic series was removed for package A, it cannot be added back for package B for the same bug. So I actually don't have a way to nominate all the series proposed on the patch mail. Is it possible to alter settings from emails like Debian's bug tracking system? You-Sheng Yang On 2020-04-03 16:01, Stefan Bader wrote: > On 16.03.20 10:27, You-Sheng Yang wrote: >> BugLink: https://bugs.launchpad.net/bugs/1866734 >> >> 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: You-Sheng Yang <vicamo.yang@canonical.com> >> (backported from >> https://lore.kernel.org/linux-input/20200310033640.14440-1-vicamo@gmail.com/) > Acked-by: Stefan Bader <stefan.bader@canonical.com> >> --- > > From what I read already applied to F, unsure nomination for B. So for now only > acking for E (and OEM-B fwiw). > > -Stefan > >> drivers/input/serio/i8042.c | 23 +++++++++++++---------- >> 1 file changed, 13 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c >> index 7f4592177607..cc079c5680d6 100644 >> --- a/drivers/input/serio/i8042.c >> +++ b/drivers/input/serio/i8042.c >> @@ -927,25 +927,28 @@ static int i8042_controller_selftest(void) >> { >> unsigned char param; >> int i = 0; >> + int ret; >> >> /* >> * We try this 5 times; on some really fragile systems this does not >> * take the first time... >> */ >> - do { >> - >> - if (i8042_command(¶m, I8042_CMD_CTL_TEST)) { >> - pr_info("i8042 controller selftest timeout\n"); >> - return -ENODEV; >> - } >> + while (i++ < 5) { >> >> - if (param == I8042_RET_CTL_TEST) >> + ret = i8042_command(¶m, I8042_CMD_CTL_TEST); >> + if (ret) >> + pr_info("i8042 controller selftest timeout (%d/5)\n", i); >> + else if (param == I8042_RET_CTL_TEST) >> return 0; >> + else >> + dbg("i8042 controller selftest: %#x != %#x\n", >> + param, I8042_RET_CTL_TEST); >> >> - dbg("i8042 controller selftest: %#x != %#x\n", >> - param, I8042_RET_CTL_TEST); >> msleep(50); >> - } while (i++ < 5); >> + } >> + >> + if (ret) >> + return -ENODEV; >> >> #ifdef CONFIG_X86 >> /* >> > > >
Hi Vicamo, I fixed this by removing the nomination to Bionic from all of the affected packages. After this is done the nomination to this series can be done to all packages again. This can be avoided by setting a nomination to 'Invalid' instead of removing it when it's not applicable. As this discussion was not solved before the cut-off date for patches for the current cycle it hasn't been applied yet but it's queued to piggyback on any re-spin that we'll need. Thanks, Kleber On 04.04.20 14:43, You-Sheng Yang wrote: > Hi, it was a bug in launchpad. Once Bionic series was removed for > package A, it cannot be added back for package B for the same bug. So I > actually don't have a way to nominate all the series proposed on the > patch mail. Is it possible to alter settings from emails like Debian's > bug tracking system? > > You-Sheng Yang > > On 2020-04-03 16:01, Stefan Bader wrote: >> On 16.03.20 10:27, You-Sheng Yang wrote: >>> BugLink: https://bugs.launchpad.net/bugs/1866734 >>> >>> 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: You-Sheng Yang <vicamo.yang@canonical.com> >>> (backported from >>> https://lore.kernel.org/linux-input/20200310033640.14440-1-vicamo@gmail.com/) >> Acked-by: Stefan Bader <stefan.bader@canonical.com> >>> --- >> >> From what I read already applied to F, unsure nomination for B. So for now only >> acking for E (and OEM-B fwiw). >> >> -Stefan >> >>> drivers/input/serio/i8042.c | 23 +++++++++++++---------- >>> 1 file changed, 13 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c >>> index 7f4592177607..cc079c5680d6 100644 >>> --- a/drivers/input/serio/i8042.c >>> +++ b/drivers/input/serio/i8042.c >>> @@ -927,25 +927,28 @@ static int i8042_controller_selftest(void) >>> { >>> unsigned char param; >>> int i = 0; >>> + int ret; >>> >>> /* >>> * We try this 5 times; on some really fragile systems this does not >>> * take the first time... >>> */ >>> - do { >>> - >>> - if (i8042_command(¶m, I8042_CMD_CTL_TEST)) { >>> - pr_info("i8042 controller selftest timeout\n"); >>> - return -ENODEV; >>> - } >>> + while (i++ < 5) { >>> >>> - if (param == I8042_RET_CTL_TEST) >>> + ret = i8042_command(¶m, I8042_CMD_CTL_TEST); >>> + if (ret) >>> + pr_info("i8042 controller selftest timeout (%d/5)\n", i); >>> + else if (param == I8042_RET_CTL_TEST) >>> return 0; >>> + else >>> + dbg("i8042 controller selftest: %#x != %#x\n", >>> + param, I8042_RET_CTL_TEST); >>> >>> - dbg("i8042 controller selftest: %#x != %#x\n", >>> - param, I8042_RET_CTL_TEST); >>> msleep(50); >>> - } while (i++ < 5); >>> + } >>> + >>> + if (ret) >>> + return -ENODEV; >>> >>> #ifdef CONFIG_X86 >>> /* >>> >> >> >> > >
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c index 7f4592177607..cc079c5680d6 100644 --- a/drivers/input/serio/i8042.c +++ b/drivers/input/serio/i8042.c @@ -927,25 +927,28 @@ static int i8042_controller_selftest(void) { unsigned char param; int i = 0; + int ret; /* * We try this 5 times; on some really fragile systems this does not * take the first time... */ - do { - - if (i8042_command(¶m, I8042_CMD_CTL_TEST)) { - pr_info("i8042 controller selftest timeout\n"); - return -ENODEV; - } + while (i++ < 5) { - if (param == I8042_RET_CTL_TEST) + ret = i8042_command(¶m, I8042_CMD_CTL_TEST); + if (ret) + pr_info("i8042 controller selftest timeout (%d/5)\n", i); + else if (param == I8042_RET_CTL_TEST) return 0; + else + dbg("i8042 controller selftest: %#x != %#x\n", + param, I8042_RET_CTL_TEST); - dbg("i8042 controller selftest: %#x != %#x\n", - param, I8042_RET_CTL_TEST); msleep(50); - } while (i++ < 5); + } + + if (ret) + return -ENODEV; #ifdef CONFIG_X86 /*
BugLink: https://bugs.launchpad.net/bugs/1866734 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: You-Sheng Yang <vicamo.yang@canonical.com> (backported from https://lore.kernel.org/linux-input/20200310033640.14440-1-vicamo@gmail.com/) --- drivers/input/serio/i8042.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)