[PATCH] Revert "usb: kbd: destroy device after console is stopped"

[PATCH] Revert "usb: kbd: destroy device after console is stopped"

[Peter Robinson]

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(-)

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
-- 
2.31.1

[PATCH] Revert "usb: kbd: destroy device after console is stopped"

[Andy Shevchenko]

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?

-- 
With Best Regards,
Andy Shevchenko

[PATCH] Revert "usb: kbd: destroy device after console is stopped"

[Peter Robinson]

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.

[PATCH] Revert "usb: kbd: destroy device after console is stopped"

[Andy Shevchenko]

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?)

-- 
With Best Regards,
Andy Shevchenko

[PATCH] Revert "usb: kbd: destroy device after console is stopped"

[Andy Shevchenko]

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?)

-- 
With Best Regards,
Andy Shevchenko

[PATCH] Revert "usb: kbd: destroy device after console is stopped"

[Marek Vasut]

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 ?