[v1] usb: core: Add boot delay for DH34 board in restore mode

[PATCH v1] usb: core: Add boot delay for DH34 board in restore mode

Posted by Jiayi Li 11 months, 1 week ago

On certain DH34-model motherboards, USB keyboards may fail to respond
during the restore mode confirmation prompt due to the usbhid driver
not being fully initialized when device registration occurs. This
results in inability to input 'y'/'n' confirmation.

Detect this scenario by:
1. Checking DMI_BOARD_NAME for "DH34" substring
2. Verifying "restore" in kernel command line

Introduce a 200ms delay before device registration when both conditions
are met. This allows sufficient time for the usbhid driver to properly
initialize before user interaction is required.

Signed-off-by: Jiayi Li <lijiayi@kylinos.cn>
---
 drivers/usb/core/hub.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a76bb50b6202..b81b518f438b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -36,6 +36,7 @@
 #include <linux/bitfield.h>
 #include <linux/uaccess.h>
 #include <asm/byteorder.h>
+#include <linux/dmi.h>
 
 #include "hub.h"
 #include "phy.h"
@@ -2610,6 +2611,7 @@ static void set_usb_port_removable(struct usb_device *udev)
 int usb_new_device(struct usb_device *udev)
 {
 	int err;
+	const char *board_name;
 
 	if (udev->parent) {
 		/* Initialize non-root-hub device wakeup to disabled;
@@ -2656,6 +2658,17 @@ int usb_new_device(struct usb_device *udev)
 	/* check whether the hub or firmware marks this port as non-removable */
 	set_usb_port_removable(udev);
 
+	/* get board manufacturer information (DMI_BOARD_VENDOR) */
+	board_name = dmi_get_system_info(DMI_BOARD_NAME);
+
+	/* In order to load the usbhid driver on a specific model motherboards
+	 * before the restore mode confirmation, add 200ms of latancy.
+	 */
+	if (board_name && strstr(board_name, "DH34") &&
+		(strstr(saved_command_line, "restore") != NULL))
+		msleep(200);
+
+
 	/* Register the device.  The device driver is responsible
 	 * for configuring the device and invoking the add-device
 	 * notifier chain (used by usbfs and possibly others).
-- 
2.47.1

Re: [PATCH v1] usb: core: Add boot delay for DH34 board in restore mode

Posted by kernel test robot 11 months, 1 week ago

Hi Jiayi,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.14-rc5 next-20250306]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiayi-Li/usb-core-Add-boot-delay-for-DH34-board-in-restore-mode/20250306-141857
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20250306061749.1502029-1-lijiayi%40kylinos.cn
patch subject: [PATCH v1] usb: core: Add boot delay for DH34 board in restore mode
config: arm64-randconfig-002-20250307 (https://download.01.org/0day-ci/archive/20250307/202503071748.Paav3L6j-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250307/202503071748.Paav3L6j-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503071748.Paav3L6j-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in fs/exportfs/exportfs.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/zlib_inflate/zlib_inflate.o
>> ERROR: modpost: "saved_command_line" [drivers/usb/core/usbcore.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1] usb: core: Add boot delay for DH34 board in restore mode

Posted by Greg KH 11 months, 1 week ago

On Thu, Mar 06, 2025 at 02:17:49PM +0800, Jiayi Li wrote:
> On certain DH34-model motherboards, USB keyboards may fail to respond
> during the restore mode confirmation prompt due to the usbhid driver
> not being fully initialized when device registration occurs. This
> results in inability to input 'y'/'n' confirmation.
> 
> Detect this scenario by:
> 1. Checking DMI_BOARD_NAME for "DH34" substring
> 2. Verifying "restore" in kernel command line
> 
> Introduce a 200ms delay before device registration when both conditions
> are met. This allows sufficient time for the usbhid driver to properly
> initialize before user interaction is required.
> 
> Signed-off-by: Jiayi Li <lijiayi@kylinos.cn>
> ---
>  drivers/usb/core/hub.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index a76bb50b6202..b81b518f438b 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -36,6 +36,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/uaccess.h>
>  #include <asm/byteorder.h>
> +#include <linux/dmi.h>
>  
>  #include "hub.h"
>  #include "phy.h"
> @@ -2610,6 +2611,7 @@ static void set_usb_port_removable(struct usb_device *udev)
>  int usb_new_device(struct usb_device *udev)
>  {
>  	int err;
> +	const char *board_name;
>  
>  	if (udev->parent) {
>  		/* Initialize non-root-hub device wakeup to disabled;
> @@ -2656,6 +2658,17 @@ int usb_new_device(struct usb_device *udev)
>  	/* check whether the hub or firmware marks this port as non-removable */
>  	set_usb_port_removable(udev);
>  
> +	/* get board manufacturer information (DMI_BOARD_VENDOR) */
> +	board_name = dmi_get_system_info(DMI_BOARD_NAME);

What about platforms that do not have DMI?

> +
> +	/* In order to load the usbhid driver on a specific model motherboards
> +	 * before the restore mode confirmation, add 200ms of latancy.
> +	 */
> +	if (board_name && strstr(board_name, "DH34") &&
> +		(strstr(saved_command_line, "restore") != NULL))
> +		msleep(200);
> +
> +

Always use scripts/checkpatch.pl before sending patches out.

thanks,

greg k-h

Re: [PATCH v1] usb: core: Add boot delay for DH34 board in restore mode

Posted by Alan Stern 11 months, 1 week ago

On Thu, Mar 06, 2025 at 02:17:49PM +0800, Jiayi Li wrote:
> On certain DH34-model motherboards, USB keyboards may fail to respond
> during the restore mode confirmation prompt due to the usbhid driver
> not being fully initialized when device registration occurs.

Why does that make any difference?  The driver core will probe the 
usbhid driver when it is fully registered (assuming the keyboard device 
hasn't been bound to a different driver in the meantime).

>  This
> results in inability to input 'y'/'n' confirmation.
> 
> Detect this scenario by:
> 1. Checking DMI_BOARD_NAME for "DH34" substring
> 2. Verifying "restore" in kernel command line
> 
> Introduce a 200ms delay before device registration when both conditions
> are met. This allows sufficient time for the usbhid driver to properly
> initialize before user interaction is required.

Why does delaying device registration help?  In theory it should make 
things worse: When user interaction is required, the keyboard device 
won't even be registered yet, let alone bound to the usbhid driver.  

Furthermore, your patch delays registration of _all_ devices, not just 
the keyboard device.  This will slow down the restore-mode boot 
procedure considerably.

It sounds like what you really need to do is delay the user interaction, 
leaving the device and driver initialization and registration unchanged.

Alan Stern

> Signed-off-by: Jiayi Li <lijiayi@kylinos.cn>
> ---
>  drivers/usb/core/hub.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index a76bb50b6202..b81b518f438b 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -36,6 +36,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/uaccess.h>
>  #include <asm/byteorder.h>
> +#include <linux/dmi.h>
>  
>  #include "hub.h"
>  #include "phy.h"
> @@ -2610,6 +2611,7 @@ static void set_usb_port_removable(struct usb_device *udev)
>  int usb_new_device(struct usb_device *udev)
>  {
>  	int err;
> +	const char *board_name;
>  
>  	if (udev->parent) {
>  		/* Initialize non-root-hub device wakeup to disabled;
> @@ -2656,6 +2658,17 @@ int usb_new_device(struct usb_device *udev)
>  	/* check whether the hub or firmware marks this port as non-removable */
>  	set_usb_port_removable(udev);
>  
> +	/* get board manufacturer information (DMI_BOARD_VENDOR) */
> +	board_name = dmi_get_system_info(DMI_BOARD_NAME);
> +
> +	/* In order to load the usbhid driver on a specific model motherboards
> +	 * before the restore mode confirmation, add 200ms of latancy.
> +	 */
> +	if (board_name && strstr(board_name, "DH34") &&
> +		(strstr(saved_command_line, "restore") != NULL))
> +		msleep(200);
> +
> +
>  	/* Register the device.  The device driver is responsible
>  	 * for configuring the device and invoking the add-device
>  	 * notifier chain (used by usbfs and possibly others).
> -- 
> 2.47.1
>

[PATCH WITHDRAWN] usb: core: Add boot delay for DH34 board in restore mode

Posted by Jiayi Li 10 months, 3 weeks ago

On certain DH34-model motherboards, USB keyboards may fail to respond
during the restore mode confirmation prompt due to the usbhid driver
not being fully initialized when device registration occurs. This
results in inability to input 'y'/'n' confirmation.

Detect this scenario by:
1. Checking DMI_BOARD_NAME for "DH34" substring
2. Verifying "restore" in kernel command line

Introduce a 200ms delay before device registration when both conditions
are met. This allows sufficient time for the usbhid driver to properly
initialize before user interaction is required.

Signed-off-by: Jiayi Li <lijiayi@kylinos.cn>
---
 drivers/usb/core/hub.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a76bb50b6202..b81b518f438b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -36,6 +36,7 @@
 #include <linux/bitfield.h>
 #include <linux/uaccess.h>
 #include <asm/byteorder.h>
+#include <linux/dmi.h>
 
 #include "hub.h"
 #include "phy.h"
@@ -2610,6 +2611,7 @@ static void set_usb_port_removable(struct usb_device *udev)
 int usb_new_device(struct usb_device *udev)
 {
 	int err;
+	const char *board_name;
 
 	if (udev->parent) {
 		/* Initialize non-root-hub device wakeup to disabled;
@@ -2656,6 +2658,17 @@ int usb_new_device(struct usb_device *udev)
 	/* check whether the hub or firmware marks this port as non-removable */
 	set_usb_port_removable(udev);
 
+	/* get board manufacturer information (DMI_BOARD_VENDOR) */
+	board_name = dmi_get_system_info(DMI_BOARD_NAME);
+
+	/* In order to load the usbhid driver on a specific model motherboards
+	 * before the restore mode confirmation, add 200ms of latancy.
+	 */
+	if (board_name && strstr(board_name, "DH34") &&
+		(strstr(saved_command_line, "restore") != NULL))
+		msleep(200);
+
+
 	/* Register the device.  The device driver is responsible
 	 * for configuring the device and invoking the add-device
 	 * notifier chain (used by usbfs and possibly others).
-- 
2.47.1