| Message ID | 20210422091910.83560-1-pbrobinson@gmail.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Revert "usb: kbd: destroy device after console is stopped" | expand |
On Thu, Apr 22, 2021 at 10:19:10AM +0100, Peter Robinson wrote: Thanks for the report. > Reverts commit eb5fd9e46c1, it causes ARMv7 devices to stop booting > Linux when a USB keyboard is attached. The kernels starts but there's > no output. Reverting it makes things work again. It's not good. When we revert, we introduce another (I consider more serious) bug. > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > > This has caused us issues on a number of ARMv7 deviices. I'm not sure > why specifically ARMv7 because it happens on a RPi3 running Fedora > armhfp, but doesn't happen on aarch64. Issue seen on RPis, Cubietruck, > and others. Can we conduct a bit of investigation here? Is it using ATAGs or command line? How Linux kernel gets its parameters?
On Thu, Apr 22, 2021 at 11:52 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Apr 22, 2021 at 10:19:10AM +0100, Peter Robinson wrote: > > Thanks for the report. > > > Reverts commit eb5fd9e46c1, it causes ARMv7 devices to stop booting > > Linux when a USB keyboard is attached. The kernels starts but there's > > no output. Reverting it makes things work again. > > It's not good. When we revert, we introduce another (I consider more serious) > bug. Sure, looking at the fixes tags I figured as much, but it also causes regressions itself. I figured sending a revert would at least start the discussion off. > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > > Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > > > This has caused us issues on a number of ARMv7 deviices. I'm not sure > > why specifically ARMv7 because it happens on a RPi3 running Fedora > > armhfp, but doesn't happen on aarch64. Issue seen on RPis, Cubietruck, > > and others. > > Can we conduct a bit of investigation here? > Is it using ATAGs or command line? How Linux kernel gets its parameters? I believe command line, I've been testing booting using UEFI. TBH I'm unsure why I see the problem on ARMv7 and not using aarchc64 on the same device.
On Thu, Apr 22, 2021 at 12:11:44PM +0100, Peter Robinson wrote: > On Thu, Apr 22, 2021 at 11:52 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Apr 22, 2021 at 10:19:10AM +0100, Peter Robinson wrote: ... > > > This has caused us issues on a number of ARMv7 deviices. I'm not sure > > > why specifically ARMv7 because it happens on a RPi3 running Fedora > > > armhfp, but doesn't happen on aarch64. Issue seen on RPis, Cubietruck, > > > and others. > > > > Can we conduct a bit of investigation here? > > Is it using ATAGs or command line? How Linux kernel gets its parameters? > > I believe command line, I've been testing booting using UEFI. TBH I'm > unsure why I see the problem on ARMv7 and not using aarchc64 on the > same device. What is the command line in both cases? (I'm not so familiar with ARM.aarch64 boards, but does it mean that the hardware is absolutely the same, just the CPU mode is different?)
On Thu, Apr 22, 2021 at 12:11:44PM +0100, Peter Robinson wrote: > On Thu, Apr 22, 2021 at 11:52 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Thu, Apr 22, 2021 at 10:19:10AM +0100, Peter Robinson wrote: > > > > Thanks for the report. > > > > > Reverts commit eb5fd9e46c1, it causes ARMv7 devices to stop booting > > > Linux when a USB keyboard is attached. The kernels starts but there's > > > no output. Reverting it makes things work again. > > > > It's not good. When we revert, we introduce another (I consider more serious) > > bug. > > Sure, looking at the fixes tags I figured as much, but it also causes > regressions itself. I figured sending a revert would at least start > the discussion off. > > > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > > > Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > --- > > > > > > This has caused us issues on a number of ARMv7 deviices. I'm not sure > > > why specifically ARMv7 because it happens on a RPi3 running Fedora > > > armhfp, but doesn't happen on aarch64. Issue seen on RPis, Cubietruck, > > > and others. > > > > Can we conduct a bit of investigation here? > > Is it using ATAGs or command line? How Linux kernel gets its parameters? > > I believe command line, I've been testing booting using UEFI. TBH I'm > unsure why I see the problem on ARMv7 and not using aarchc64 on the > same device. And what is the U-Boot environment in both cases? (Can you share it somewhere?)
On 4/22/21 1:30 PM, Andy Shevchenko wrote: > On Thu, Apr 22, 2021 at 12:11:44PM +0100, Peter Robinson wrote: >> On Thu, Apr 22, 2021 at 11:52 AM Andy Shevchenko >> <andriy.shevchenko@linux.intel.com> wrote: >>> >>> On Thu, Apr 22, 2021 at 10:19:10AM +0100, Peter Robinson wrote: >>> >>> Thanks for the report. >>> >>>> Reverts commit eb5fd9e46c1, it causes ARMv7 devices to stop booting >>>> Linux when a USB keyboard is attached. The kernels starts but there's >>>> no output. Reverting it makes things work again. >>> >>> It's not good. When we revert, we introduce another (I consider more serious) >>> bug. >> >> Sure, looking at the fixes tags I figured as much, but it also causes >> regressions itself. I figured sending a revert would at least start >> the discussion off. >> >>>> Signed-off-by: Peter Robinson <pbrobinson@gmail.com> >>>> Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> >>>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>>> --- >>>> >>>> This has caused us issues on a number of ARMv7 deviices. I'm not sure >>>> why specifically ARMv7 because it happens on a RPi3 running Fedora >>>> armhfp, but doesn't happen on aarch64. Issue seen on RPis, Cubietruck, >>>> and others. >>> >>> Can we conduct a bit of investigation here? >>> Is it using ATAGs or command line? How Linux kernel gets its parameters? >> >> I believe command line, I've been testing booting using UEFI. TBH I'm >> unsure why I see the problem on ARMv7 and not using aarchc64 on the >> same device. > > And what is the U-Boot environment in both cases? (Can you share it somewhere?) Could it be related to CONFIG_SYS_DEVICE_NULLDEV / CONFIG_NULLDEV_SERIAL not being set in one of the cases ?
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index afad260d3d..515f37136f 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -622,12 +622,12 @@ int usb_kbd_deregister(int force) if (dev) { usb_kbd_dev = (struct usb_device *)dev->priv; data = usb_kbd_dev->privptr; + if (stdio_deregister_dev(dev, force) != 0) + return 1; #if CONFIG_IS_ENABLED(CONSOLE_MUX) - if (iomux_replace_device(stdin, DEVNAME, force ? "nulldev" : "")) + if (iomux_doenv(stdin, env_get("stdin")) != 0) return 1; #endif - if (stdio_deregister_dev(dev, force) != 0) - return 1; #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE destroy_int_queue(usb_kbd_dev, data->intq); #endif @@ -665,16 +665,16 @@ static int usb_kbd_remove(struct udevice *dev) goto err; } data = udev->privptr; + if (stdio_deregister_dev(sdev, true)) { + ret = -EPERM; + goto err; + } #if CONFIG_IS_ENABLED(CONSOLE_MUX) - if (iomux_replace_device(stdin, DEVNAME, "nulldev")) { + if (iomux_doenv(stdin, env_get("stdin"))) { ret = -ENOLINK; goto err; } #endif - if (stdio_deregister_dev(sdev, true)) { - ret = -EPERM; - goto err; - } #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE destroy_int_queue(udev, data->intq); #endif
Reverts commit eb5fd9e46c1, it causes ARMv7 devices to stop booting Linux when a USB keyboard is attached. The kernels starts but there's no output. Reverting it makes things work again. Signed-off-by: Peter Robinson <pbrobinson@gmail.com> Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- This has caused us issues on a number of ARMv7 deviices. I'm not sure why specifically ARMv7 because it happens on a RPi3 running Fedora armhfp, but doesn't happen on aarch64. Issue seen on RPis, Cubietruck, and others. common/usb_kbd.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)