drivers/usb/usbip/vhci.h | 4 +- drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++----------------- drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++----------- tools/usb/usbip/libsrc/vhci_driver.h | 2 +- 4 files changed, 72 insertions(+), 88 deletions(-)
From: Zongmin Zhou <zhouzongmin@kylinos.cn>
The vhci driver does not need to create a platform device,
it only did so because it was simple to do that in order to
get a place in sysfs to hang some device-specific attributes.
Now the faux device interface is more appropriate,change it
over to use the faux bus instead.
Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
Changes in v2:
- don't change faux create api,just call probe on vhci_hcd_init.
drivers/usb/usbip/vhci.h | 4 +-
drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
4 files changed, 72 insertions(+), 88 deletions(-)
diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
index 5659dce1526e..a73070a3f450 100644
--- a/drivers/usb/usbip/vhci.h
+++ b/drivers/usb/usbip/vhci.h
@@ -93,7 +93,7 @@ enum hub_speed {
struct vhci {
spinlock_t lock;
- struct platform_device *pdev;
+ struct faux_device *fdev;
struct vhci_hcd *vhci_hcd_hs;
struct vhci_hcd *vhci_hcd_ss;
@@ -141,7 +141,7 @@ static inline __u32 port_to_rhport(__u32 port)
return port % VHCI_HC_PORTS;
}
-static inline int port_to_pdev_nr(__u32 port)
+static inline int port_to_fdev_nr(__u32 port)
{
return port / VHCI_PORTS;
}
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index e70fba9f55d6..4e6d7d23e915 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -9,7 +9,7 @@
#include <linux/kernel.h>
#include <linux/kthread.h>
#include <linux/module.h>
-#include <linux/platform_device.h>
+#include <linux/device/faux.h>
#include <linux/slab.h>
#include <linux/string_choices.h>
@@ -1143,7 +1143,7 @@ static int hcd_name_to_id(const char *name)
static int vhci_setup(struct usb_hcd *hcd)
{
- struct vhci *vhci = *((void **)dev_get_platdata(hcd->self.controller));
+ struct vhci *vhci = dev_get_platdata(hcd->self.controller);
if (usb_hcd_is_primary_hcd(hcd)) {
vhci->vhci_hcd_hs = hcd_to_vhci_hcd(hcd);
@@ -1257,7 +1257,7 @@ static int vhci_get_frame_number(struct usb_hcd *hcd)
/* FIXME: suspend/resume */
static int vhci_bus_suspend(struct usb_hcd *hcd)
{
- struct vhci *vhci = *((void **)dev_get_platdata(hcd->self.controller));
+ struct vhci *vhci = dev_get_platdata(hcd->self.controller);
unsigned long flags;
dev_dbg(&hcd->self.root_hub->dev, "%s\n", __func__);
@@ -1271,7 +1271,7 @@ static int vhci_bus_suspend(struct usb_hcd *hcd)
static int vhci_bus_resume(struct usb_hcd *hcd)
{
- struct vhci *vhci = *((void **)dev_get_platdata(hcd->self.controller));
+ struct vhci *vhci = dev_get_platdata(hcd->self.controller);
int rc = 0;
unsigned long flags;
@@ -1336,20 +1336,19 @@ static const struct hc_driver vhci_hc_driver = {
.free_streams = vhci_free_streams,
};
-static int vhci_hcd_probe(struct platform_device *pdev)
+static int vhci_hcd_probe(struct faux_device *fdev)
{
- struct vhci *vhci = *((void **)dev_get_platdata(&pdev->dev));
+ struct vhci *vhci = dev_get_platdata(&fdev->dev);
struct usb_hcd *hcd_hs;
struct usb_hcd *hcd_ss;
int ret;
- usbip_dbg_vhci_hc("name %s id %d\n", pdev->name, pdev->id);
/*
* Allocate and initialize hcd.
* Our private data is also allocated automatically.
*/
- hcd_hs = usb_create_hcd(&vhci_hc_driver, &pdev->dev, dev_name(&pdev->dev));
+ hcd_hs = usb_create_hcd(&vhci_hc_driver, &fdev->dev, dev_name(&fdev->dev));
if (!hcd_hs) {
pr_err("create primary hcd failed\n");
return -ENOMEM;
@@ -1366,8 +1365,8 @@ static int vhci_hcd_probe(struct platform_device *pdev)
goto put_usb2_hcd;
}
- hcd_ss = usb_create_shared_hcd(&vhci_hc_driver, &pdev->dev,
- dev_name(&pdev->dev), hcd_hs);
+ hcd_ss = usb_create_shared_hcd(&vhci_hc_driver, &fdev->dev,
+ dev_name(&fdev->dev), hcd_hs);
if (!hcd_ss) {
ret = -ENOMEM;
pr_err("create shared hcd failed\n");
@@ -1394,9 +1393,9 @@ static int vhci_hcd_probe(struct platform_device *pdev)
return ret;
}
-static void vhci_hcd_remove(struct platform_device *pdev)
+static void vhci_hcd_remove(struct faux_device *fdev)
{
- struct vhci *vhci = *((void **)dev_get_platdata(&pdev->dev));
+ struct vhci *vhci = dev_get_platdata(&fdev->dev);
/*
* Disconnects the root hub,
@@ -1416,7 +1415,7 @@ static void vhci_hcd_remove(struct platform_device *pdev)
#ifdef CONFIG_PM
/* what should happen for USB/IP under suspend/resume? */
-static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state)
+static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
{
struct usb_hcd *hcd;
struct vhci *vhci;
@@ -1425,13 +1424,13 @@ static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state)
int ret = 0;
unsigned long flags;
- dev_dbg(&pdev->dev, "%s\n", __func__);
+ dev_dbg(&fdev->dev, "%s\n", __func__);
- hcd = platform_get_drvdata(pdev);
+ hcd = faux_device_get_drvdata(fdev);
if (!hcd)
return 0;
- vhci = *((void **)dev_get_platdata(hcd->self.controller));
+ vhci = dev_get_platdata(hcd->self.controller);
spin_lock_irqsave(&vhci->lock, flags);
@@ -1448,25 +1447,25 @@ static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state)
spin_unlock_irqrestore(&vhci->lock, flags);
if (connected > 0) {
- dev_info(&pdev->dev,
+ dev_info(&fdev->dev,
"We have %d active connection%s. Do not suspend.\n",
connected, str_plural(connected));
ret = -EBUSY;
} else {
- dev_info(&pdev->dev, "suspend vhci_hcd");
+ dev_info(&fdev->dev, "suspend vhci_hcd");
clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
}
return ret;
}
-static int vhci_hcd_resume(struct platform_device *pdev)
+static int vhci_hcd_resume(struct faux_device *fdev)
{
struct usb_hcd *hcd;
- dev_dbg(&pdev->dev, "%s\n", __func__);
+ dev_dbg(&fdev->dev, "%s\n", __func__);
- hcd = platform_get_drvdata(pdev);
+ hcd = faux_device_get_drvdata(fdev);
if (!hcd)
return 0;
set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
@@ -1482,25 +1481,18 @@ static int vhci_hcd_resume(struct platform_device *pdev)
#endif
-static struct platform_driver vhci_driver = {
- .probe = vhci_hcd_probe,
+static struct faux_device_ops vhci_driver = {
.remove = vhci_hcd_remove,
- .suspend = vhci_hcd_suspend,
- .resume = vhci_hcd_resume,
- .driver = {
- .name = driver_name,
- },
};
-static void del_platform_devices(void)
+static void del_faux_devices(void)
{
int i;
for (i = 0; i < vhci_num_controllers; i++) {
- platform_device_unregister(vhcis[i].pdev);
- vhcis[i].pdev = NULL;
+ faux_device_destroy(vhcis[i].fdev);
+ vhcis[i].fdev = NULL;
}
- sysfs_remove_link(&platform_bus.kobj, driver_name);
}
static int __init vhci_hcd_init(void)
@@ -1517,41 +1509,33 @@ static int __init vhci_hcd_init(void)
if (vhcis == NULL)
return -ENOMEM;
- ret = platform_driver_register(&vhci_driver);
- if (ret)
- goto err_driver_register;
-
for (i = 0; i < vhci_num_controllers; i++) {
void *vhci = &vhcis[i];
- struct platform_device_info pdevinfo = {
- .name = driver_name,
- .id = i,
- .data = &vhci,
- .size_data = sizeof(void *),
- };
-
- vhcis[i].pdev = platform_device_register_full(&pdevinfo);
- ret = PTR_ERR_OR_ZERO(vhcis[i].pdev);
- if (ret < 0) {
+ char vhci_name[16];
+
+ snprintf(vhci_name, 16, "%s.%d", driver_name, i);
+
+ vhcis[i].fdev = faux_device_create_with_groups(vhci_name, NULL, &vhci_driver, NULL);
+ if (!vhcis[i].fdev) {
while (i--)
- platform_device_unregister(vhcis[i].pdev);
+ faux_device_destroy(vhcis[i].fdev);
+ ret = -ENODEV;
goto err_add_hcd;
}
+ vhcis[i].fdev->dev.platform_data = vhci;
+ vhci_hcd_probe(vhcis[i].fdev);
}
return 0;
err_add_hcd:
- platform_driver_unregister(&vhci_driver);
-err_driver_register:
kfree(vhcis);
return ret;
}
static void __exit vhci_hcd_exit(void)
{
- del_platform_devices();
- platform_driver_unregister(&vhci_driver);
+ del_faux_devices();
kfree(vhcis);
}
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index d5865460e82d..d1e6b239399f 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -7,7 +7,7 @@
#include <linux/kthread.h>
#include <linux/file.h>
#include <linux/net.h>
-#include <linux/platform_device.h>
+#include <linux/device/faux.h>
#include <linux/slab.h>
/* Hardening for Spectre-v1 */
@@ -28,7 +28,7 @@
*
* Output includes socket fd instead of socket pointer address to avoid
* leaking kernel memory address in:
- * /sys/devices/platform/vhci_hcd.0/status and in debug output.
+ * /sys/devices/faux/vhci_hcd.0/status and in debug output.
* The socket pointer address is not used at the moment and it was made
* visible as a convenient way to find IP address from socket pointer
* address by looking up /proc/net/{tcp,tcp6}. As this opens a security
@@ -60,9 +60,9 @@ static void port_show_vhci(char **out, int hub, int port, struct vhci_device *vd
}
/* Sysfs entry to show port status */
-static ssize_t status_show_vhci(int pdev_nr, char *out)
+static ssize_t status_show_vhci(int fdev_nr, char *out)
{
- struct platform_device *pdev = vhcis[pdev_nr].pdev;
+ struct faux_device *fdev = vhcis[fdev_nr].fdev;
struct vhci *vhci;
struct usb_hcd *hcd;
struct vhci_hcd *vhci_hcd;
@@ -70,12 +70,12 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)
int i;
unsigned long flags;
- if (!pdev || !out) {
+ if (!fdev || !out) {
usbip_dbg_vhci_sysfs("show status error\n");
return 0;
}
- hcd = platform_get_drvdata(pdev);
+ hcd = faux_device_get_drvdata(fdev);
vhci_hcd = hcd_to_vhci_hcd(hcd);
vhci = vhci_hcd->vhci;
@@ -86,7 +86,7 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)
spin_lock(&vdev->ud.lock);
port_show_vhci(&out, HUB_SPEED_HIGH,
- pdev_nr * VHCI_PORTS + i, vdev);
+ fdev_nr * VHCI_PORTS + i, vdev);
spin_unlock(&vdev->ud.lock);
}
@@ -95,7 +95,7 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)
spin_lock(&vdev->ud.lock);
port_show_vhci(&out, HUB_SPEED_SUPER,
- pdev_nr * VHCI_PORTS + VHCI_HC_PORTS + i, vdev);
+ fdev_nr * VHCI_PORTS + VHCI_HC_PORTS + i, vdev);
spin_unlock(&vdev->ud.lock);
}
@@ -104,14 +104,14 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)
return out - s;
}
-static ssize_t status_show_not_ready(int pdev_nr, char *out)
+static ssize_t status_show_not_ready(int fdev_nr, char *out)
{
char *s = out;
int i = 0;
for (i = 0; i < VHCI_HC_PORTS; i++) {
out += sprintf(out, "hs %04u %03u ",
- (pdev_nr * VHCI_PORTS) + i,
+ (fdev_nr * VHCI_PORTS) + i,
VDEV_ST_NOTASSIGNED);
out += sprintf(out, "000 00000000 0000000000000000 0-0");
out += sprintf(out, "\n");
@@ -119,7 +119,7 @@ static ssize_t status_show_not_ready(int pdev_nr, char *out)
for (i = 0; i < VHCI_HC_PORTS; i++) {
out += sprintf(out, "ss %04u %03u ",
- (pdev_nr * VHCI_PORTS) + VHCI_HC_PORTS + i,
+ (fdev_nr * VHCI_PORTS) + VHCI_HC_PORTS + i,
VDEV_ST_NOTASSIGNED);
out += sprintf(out, "000 00000000 0000000000000000 0-0");
out += sprintf(out, "\n");
@@ -148,16 +148,16 @@ static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *out)
{
char *s = out;
- int pdev_nr;
+ int fdev_nr;
out += sprintf(out,
"hub port sta spd dev sockfd local_busid\n");
- pdev_nr = status_name_to_id(attr->attr.name);
- if (pdev_nr < 0)
- out += status_show_not_ready(pdev_nr, out);
+ fdev_nr = status_name_to_id(attr->attr.name);
+ if (fdev_nr < 0)
+ out += status_show_not_ready(fdev_nr, out);
else
- out += status_show_vhci(pdev_nr, out);
+ out += status_show_vhci(fdev_nr, out);
return out - s;
}
@@ -213,13 +213,13 @@ static int vhci_port_disconnect(struct vhci_hcd *vhci_hcd, __u32 rhport)
return 0;
}
-static int valid_port(__u32 *pdev_nr, __u32 *rhport)
+static int valid_port(__u32 *fdev_nr, __u32 *rhport)
{
- if (*pdev_nr >= vhci_num_controllers) {
- pr_err("pdev %u\n", *pdev_nr);
+ if (*fdev_nr >= vhci_num_controllers) {
+ pr_err("fdev %u\n", *fdev_nr);
return 0;
}
- *pdev_nr = array_index_nospec(*pdev_nr, vhci_num_controllers);
+ *fdev_nr = array_index_nospec(*fdev_nr, vhci_num_controllers);
if (*rhport >= VHCI_HC_PORTS) {
pr_err("rhport %u\n", *rhport);
@@ -233,7 +233,7 @@ static int valid_port(__u32 *pdev_nr, __u32 *rhport)
static ssize_t detach_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- __u32 port = 0, pdev_nr = 0, rhport = 0;
+ __u32 port = 0, fdev_nr = 0, rhport = 0;
struct usb_hcd *hcd;
struct vhci_hcd *vhci_hcd;
int ret;
@@ -241,13 +241,13 @@ static ssize_t detach_store(struct device *dev, struct device_attribute *attr,
if (kstrtoint(buf, 10, &port) < 0)
return -EINVAL;
- pdev_nr = port_to_pdev_nr(port);
+ fdev_nr = port_to_fdev_nr(port);
rhport = port_to_rhport(port);
- if (!valid_port(&pdev_nr, &rhport))
+ if (!valid_port(&fdev_nr, &rhport))
return -EINVAL;
- hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
+ hcd = faux_device_get_drvdata(vhcis[fdev_nr].fdev);
if (hcd == NULL) {
dev_err(dev, "port is not ready %u\n", port);
return -EAGAIN;
@@ -270,10 +270,10 @@ static ssize_t detach_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(detach);
-static int valid_args(__u32 *pdev_nr, __u32 *rhport,
+static int valid_args(__u32 *fdev_nr, __u32 *rhport,
enum usb_device_speed speed)
{
- if (!valid_port(pdev_nr, rhport)) {
+ if (!valid_port(fdev_nr, rhport)) {
return 0;
}
@@ -311,7 +311,7 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr,
{
struct socket *socket;
int sockfd = 0;
- __u32 port = 0, pdev_nr = 0, rhport = 0, devid = 0, speed = 0;
+ __u32 port = 0, fdev_nr = 0, rhport = 0, devid = 0, speed = 0;
struct usb_hcd *hcd;
struct vhci_hcd *vhci_hcd;
struct vhci_device *vdev;
@@ -329,19 +329,19 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr,
*/
if (sscanf(buf, "%u %u %u %u", &port, &sockfd, &devid, &speed) != 4)
return -EINVAL;
- pdev_nr = port_to_pdev_nr(port);
+ fdev_nr = port_to_fdev_nr(port);
rhport = port_to_rhport(port);
- usbip_dbg_vhci_sysfs("port(%u) pdev(%d) rhport(%u)\n",
- port, pdev_nr, rhport);
+ usbip_dbg_vhci_sysfs("port(%u) fdev(%d) rhport(%u)\n",
+ port, fdev_nr, rhport);
usbip_dbg_vhci_sysfs("sockfd(%u) devid(%u) speed(%u)\n",
sockfd, devid, speed);
/* check received parameters */
- if (!valid_args(&pdev_nr, &rhport, speed))
+ if (!valid_args(&fdev_nr, &rhport, speed))
return -EINVAL;
- hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
+ hcd = faux_device_get_drvdata(vhcis[fdev_nr].fdev);
if (hcd == NULL) {
dev_err(dev, "port %d is not ready\n", port);
return -EAGAIN;
@@ -413,8 +413,8 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr,
goto unlock_mutex;
}
- dev_info(dev, "pdev(%u) rhport(%u) sockfd(%d)\n",
- pdev_nr, rhport, sockfd);
+ dev_info(dev, "fdev(%u) rhport(%u) sockfd(%d)\n",
+ fdev_nr, rhport, sockfd);
dev_info(dev, "devid(%u) speed(%u) speed_str(%s)\n",
devid, speed, usb_speed_string(speed));
diff --git a/tools/usb/usbip/libsrc/vhci_driver.h b/tools/usb/usbip/libsrc/vhci_driver.h
index 6c9aca216705..20918e74de59 100644
--- a/tools/usb/usbip/libsrc/vhci_driver.h
+++ b/tools/usb/usbip/libsrc/vhci_driver.h
@@ -11,7 +11,7 @@
#include "usbip_common.h"
-#define USBIP_VHCI_BUS_TYPE "platform"
+#define USBIP_VHCI_BUS_TYPE "faux"
#define USBIP_VHCI_DEVICE_NAME "vhci_hcd.0"
enum hub_speed {
--
2.25.1On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>
> The vhci driver does not need to create a platform device,
> it only did so because it was simple to do that in order to
> get a place in sysfs to hang some device-specific attributes.
> Now the faux device interface is more appropriate,change it
> over to use the faux bus instead.
>
> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> ---
> Changes in v2:
> - don't change faux create api,just call probe on vhci_hcd_init.
>
> drivers/usb/usbip/vhci.h | 4 +-
> drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
> drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
> 4 files changed, 72 insertions(+), 88 deletions(-)
I get the following build errors from this patch:
drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
1462 | static int vhci_hcd_resume(struct faux_device *fdev)
| ^~~~~~~~~~~~~~~
drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
| ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Are you sure you tested this?
thanks,
greg k-h
On 2025/6/19 19:01, Greg KH wrote:
> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>
>> The vhci driver does not need to create a platform device,
>> it only did so because it was simple to do that in order to
>> get a place in sysfs to hang some device-specific attributes.
>> Now the faux device interface is more appropriate,change it
>> over to use the faux bus instead.
>>
>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>> ---
>> Changes in v2:
>> - don't change faux create api,just call probe on vhci_hcd_init.
>>
>> drivers/usb/usbip/vhci.h | 4 +-
>> drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
>> drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>> 4 files changed, 72 insertions(+), 88 deletions(-)
> I get the following build errors from this patch:
>
> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
> | ^~~~~~~~~~~~~~~
> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
> 1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
> | ^~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Are you sure you tested this?
I apologize for not enabling -Werror, which resulted in missing this
error warning.
I have tested usbip feature use the new patch,but not test system
suspend/resume.
The faux bus type don't add pm function,and vhci-hcd driver can't
register it.
Maybe have to add suspend/resume for it.like below:
static const struct bus_type faux_bus_type = {
.name = "faux",
.match = faux_match,
.probe = faux_probe,
.remove = faux_remove,
.resume = faux_resume,
.suspend = faux_suspend,
};
Is that right?
Your expertise would be greatly valued.
Thanks very much.
>
> thanks,
>
> greg k-h
On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>
> On 2025/6/19 19:01, Greg KH wrote:
> > On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
> > > From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> > >
> > > The vhci driver does not need to create a platform device,
> > > it only did so because it was simple to do that in order to
> > > get a place in sysfs to hang some device-specific attributes.
> > > Now the faux device interface is more appropriate,change it
> > > over to use the faux bus instead.
> > >
> > > Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> > > ---
> > > Changes in v2:
> > > - don't change faux create api,just call probe on vhci_hcd_init.
> > >
> > > drivers/usb/usbip/vhci.h | 4 +-
> > > drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
> > > drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
> > > tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
> > > 4 files changed, 72 insertions(+), 88 deletions(-)
> > I get the following build errors from this patch:
> >
> > drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
> > 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
> > | ^~~~~~~~~~~~~~~
> > drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
> > 1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
> > | ^~~~~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> >
> > Are you sure you tested this?
> I apologize for not enabling -Werror, which resulted in missing this error
> warning.
> I have tested usbip feature use the new patch,but not test system
> suspend/resume.
> The faux bus type don't add pm function,and vhci-hcd driver can't register
> it.
> Maybe have to add suspend/resume for it.like below:
> static const struct bus_type faux_bus_type = {
> .name = "faux",
> .match = faux_match,
> .probe = faux_probe,
> .remove = faux_remove,
> .resume = faux_resume,
> .suspend = faux_suspend,
> };
>
> Is that right?
> Your expertise would be greatly valued.
As this is not real hardware, why do you need the suspend/resume
callbacks at all? What is happening here that requires them?
thanks,
greg k-h
On 2025/6/20 12:29, Greg KH wrote:
> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>> On 2025/6/19 19:01, Greg KH wrote:
>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>
>>>> The vhci driver does not need to create a platform device,
>>>> it only did so because it was simple to do that in order to
>>>> get a place in sysfs to hang some device-specific attributes.
>>>> Now the faux device interface is more appropriate,change it
>>>> over to use the faux bus instead.
>>>>
>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>> ---
>>>> Changes in v2:
>>>> - don't change faux create api,just call probe on vhci_hcd_init.
>>>>
>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>> drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
>>>> drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>> I get the following build errors from this patch:
>>>
>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
>>> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>> | ^~~~~~~~~~~~~~~
>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
>>> 1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
>>> | ^~~~~~~~~~~~~~~~
>>> cc1: all warnings being treated as errors
>>>
>>> Are you sure you tested this?
>> I apologize for not enabling -Werror, which resulted in missing this error
>> warning.
>> I have tested usbip feature use the new patch,but not test system
>> suspend/resume.
>> The faux bus type don't add pm function,and vhci-hcd driver can't register
>> it.
>> Maybe have to add suspend/resume for it.like below:
>> static const struct bus_type faux_bus_type = {
>> .name = "faux",
>> .match = faux_match,
>> .probe = faux_probe,
>> .remove = faux_remove,
>> .resume = faux_resume,
>> .suspend = faux_suspend,
>> };
>>
>> Is that right?
>> Your expertise would be greatly valued.
> As this is not real hardware, why do you need the suspend/resume
> callbacks at all? What is happening here that requires them?
@greg,
The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed for
this faux device, but rather to
manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags associated
with the faux device.
For example:
During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
Previously, these two functions were registered through platform_driver,
but faux bus does not have the relevant interface, so they were not called,
resulting in this compilation warning error.
This raises the question: Should the faux bus implement PM-related
interface?
I'm uncertain whether these functions are essential for the vhci-hcd
driver or if they can be safely removed.
However, during system standby/wakeup tests with remote USB devices
bound to the vhci-hcd driver,
I observed consistent failure scenarios across both the original
platform bus and faux bus patch implementations.
Failure Modes
a.Failed standby with auto-wakeup(Log excerpt):
[ 1449.065592][T10238] PM: suspend entry (s2idle)
[ 1449.106146][T10238] Filesystems sync: 0.040 seconds
[ 1449.216189][T10238] Freezing user space processes
[ 1449.219474][T10238] Freezing user space processes completed (elapsed
0.002 seconds)
[ 1449.219887][T10238] OOM killer disabled.
[ 1449.220090][T10238] Freezing remaining freezable tasks
[ 1469.222372][T10238] Freezing remaining freezable tasks failed after
20.002 seconds (0 tasks refusing to freeze, wq_busy=1):
[ 1469.225038][T10238] Showing freezable workqueues that are still busy:
[ 1469.226176][T10238] workqueue events_freezable_pwr_efficient: flags=0x86
[ 1469.227453][T10238] pwq 20: cpus=0-3 node=0 flags=0x4 nice=0
active=1 refcnt=2
[ 1469.227463][T10238] in-flight: 268:disk_events_workfn
[ 1469.233559][T10238] Restarting kernel threads ... done.
[ 1469.235119][T10238] OOM killer enabled.
[ 1469.235849][T10238] Restarting tasks ... done.
[ 1469.240121][T10238] random: crng reseeded on system resumption
[ 1469.241176][T10238] PM: suspend exit
b.Failed standby with black screen freeze:
[ 1820.667073][T11454] PM: suspend entry (s2idle)
@Shuah，
I wonder if you has encountered this issue? When a USB device is
attached to vhci-hcd,
is it not possible to put the system into standby mode?
Thanks.
> thanks,
>
> greg k-h
On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>
> On 2025/6/20 12:29, Greg KH wrote:
> > On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
> > > On 2025/6/19 19:01, Greg KH wrote:
> > > > On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
> > > > > From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> > > > >
> > > > > The vhci driver does not need to create a platform device,
> > > > > it only did so because it was simple to do that in order to
> > > > > get a place in sysfs to hang some device-specific attributes.
> > > > > Now the faux device interface is more appropriate,change it
> > > > > over to use the faux bus instead.
> > > > >
> > > > > Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> > > > > ---
> > > > > Changes in v2:
> > > > > - don't change faux create api,just call probe on vhci_hcd_init.
> > > > >
> > > > > drivers/usb/usbip/vhci.h | 4 +-
> > > > > drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
> > > > > drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
> > > > > tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
> > > > > 4 files changed, 72 insertions(+), 88 deletions(-)
> > > > I get the following build errors from this patch:
> > > >
> > > > drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
> > > > 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
> > > > | ^~~~~~~~~~~~~~~
> > > > drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
> > > > 1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
> > > > | ^~~~~~~~~~~~~~~~
> > > > cc1: all warnings being treated as errors
> > > >
> > > > Are you sure you tested this?
> > > I apologize for not enabling -Werror, which resulted in missing this error
> > > warning.
> > > I have tested usbip feature use the new patch,but not test system
> > > suspend/resume.
> > > The faux bus type don't add pm function,and vhci-hcd driver can't register
> > > it.
> > > Maybe have to add suspend/resume for it.like below:
> > > static const struct bus_type faux_bus_type = {
> > > .name = "faux",
> > > .match = faux_match,
> > > .probe = faux_probe,
> > > .remove = faux_remove,
> > > .resume = faux_resume,
> > > .suspend = faux_suspend,
> > > };
> > >
> > > Is that right?
> > > Your expertise would be greatly valued.
> > As this is not real hardware, why do you need the suspend/resume
> > callbacks at all? What is happening here that requires them?
> @greg,
> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed for this
> faux device, but rather to
> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags associated with
> the faux device.
> For example:
> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>
> Previously, these two functions were registered through platform_driver,
> but faux bus does not have the relevant interface, so they were not called,
> resulting in this compilation warning error.
>
> This raises the question: Should the faux bus implement PM-related
> interface?
> I'm uncertain whether these functions are essential for the vhci-hcd driver
> or if they can be safely removed.
>
> However, during system standby/wakeup tests with remote USB devices bound to
> the vhci-hcd driver,
> I observed consistent failure scenarios across both the original platform
> bus and faux bus patch implementations.
>
> Failure Modes
> a.Failed standby with auto-wakeup(Log excerpt):
> [ 1449.065592][T10238] PM: suspend entry (s2idle)
> [ 1449.106146][T10238] Filesystems sync: 0.040 seconds
> [ 1449.216189][T10238] Freezing user space processes
> [ 1449.219474][T10238] Freezing user space processes completed (elapsed
> 0.002 seconds)
> [ 1449.219887][T10238] OOM killer disabled.
> [ 1449.220090][T10238] Freezing remaining freezable tasks
> [ 1469.222372][T10238] Freezing remaining freezable tasks failed after
> 20.002 seconds (0 tasks refusing to freeze, wq_busy=1):
> [ 1469.225038][T10238] Showing freezable workqueues that are still busy:
> [ 1469.226176][T10238] workqueue events_freezable_pwr_efficient: flags=0x86
> [ 1469.227453][T10238] pwq 20: cpus=0-3 node=0 flags=0x4 nice=0 active=1
> refcnt=2
> [ 1469.227463][T10238] in-flight: 268:disk_events_workfn
> [ 1469.233559][T10238] Restarting kernel threads ... done.
> [ 1469.235119][T10238] OOM killer enabled.
> [ 1469.235849][T10238] Restarting tasks ... done.
> [ 1469.240121][T10238] random: crng reseeded on system resumption
> [ 1469.241176][T10238] PM: suspend exit
>
> b.Failed standby with black screen freeze:
> [ 1820.667073][T11454] PM: suspend entry (s2idle)
Are you sure this is the usbip driver causing suspend to not work? If
you unbind the usbip devices does suspend/resume work?
I would think that we would have gotten some reports by now if this
didn't work at all :)
thanks,
greg k-h
On 6/20/25 03:27, Greg KH wrote:
> On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>>
>> On 2025/6/20 12:29, Greg KH wrote:
>>> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>>>> On 2025/6/19 19:01, Greg KH wrote:
>>>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>
>>>>>> The vhci driver does not need to create a platform device,
>>>>>> it only did so because it was simple to do that in order to
>>>>>> get a place in sysfs to hang some device-specific attributes.
>>>>>> Now the faux device interface is more appropriate,change it
>>>>>> over to use the faux bus instead.
>>>>>>
>>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>> - don't change faux create api,just call probe on vhci_hcd_init.
>>>>>>
>>>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>>>> drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
>>>>>> drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
>>>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>>>> I get the following build errors from this patch:
>>>>>
>>>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
>>>>> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>>>> | ^~~~~~~~~~~~~~~
>>>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
>>>>> 1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
>>>>> | ^~~~~~~~~~~~~~~~
>>>>> cc1: all warnings being treated as errors
>>>>>
>>>>> Are you sure you tested this?
>>>> I apologize for not enabling -Werror, which resulted in missing this error
>>>> warning.
>>>> I have tested usbip feature use the new patch,but not test system
>>>> suspend/resume.
>>>> The faux bus type don't add pm function,and vhci-hcd driver can't register
>>>> it.
>>>> Maybe have to add suspend/resume for it.like below:
>>>> static const struct bus_type faux_bus_type = {
>>>> .name = "faux",
>>>> .match = faux_match,
>>>> .probe = faux_probe,
>>>> .remove = faux_remove,
>>>> .resume = faux_resume,
>>>> .suspend = faux_suspend,
>>>> };
>>>>
>>>> Is that right?
>>>> Your expertise would be greatly valued.
>>> As this is not real hardware, why do you need the suspend/resume
>>> callbacks at all? What is happening here that requires them?
>> @greg,
>> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed for this
>> faux device, but rather to
>> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags associated with
>> the faux device.
>> For example:
>> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>
>> Previously, these two functions were registered through platform_driver,
>> but faux bus does not have the relevant interface, so they were not called,
>> resulting in this compilation warning error.
>>
>> This raises the question: Should the faux bus implement PM-related
>> interface?
>> I'm uncertain whether these functions are essential for the vhci-hcd driver
>> or if they can be safely removed.
>>
>> However, during system standby/wakeup tests with remote USB devices bound to
>> the vhci-hcd driver,
>> I observed consistent failure scenarios across both the original platform
>> bus and faux bus patch implementations.
suspend and resume hooks have been in the code from beginning
in the CONFIG_PM path. The original authors are skeptical about
what should happen during suspend"
/* what should happen for USB/IP under suspend/resume? */
suspend hook checks for active connections and sends EBBUSY
back to pm core.
Active connection means any of the ports are in USB_PORT_STAT_CONNECTION
state. So as long as there are active connections between vhci client
and the host, suspend won't work. So we really don't know what happens
to the active connections if there are no suspend/resume hooks.
If there are no active connections, then it will clear the HCD_FLAG_HW_ACCESSIBLE
bit and returns to pm core to continue with suspend.
resume sets the HCD_FLAG_HW_ACCESSIBLE flag to indicate hardware is accessible
and initiates port status poll.
- suspend hook prevents suspend
With faux device since there is no suspend and resume hook, I would expect
these hooks are not a factor in suspend and resume.
vhci_hcd is a special case virtual driver as it is a proxy for controlling
hardware on the host.
Zongmin,
Do suspend/resume work when vhci_hcd is not loaded?
What happens when when system suspends and resumes with faux device?
Can you send me dmesg logs and pm logs?
thanks,
-- Shuah
On 2025/6/21 01:26, Shuah Khan wrote:
> On 6/20/25 03:27, Greg KH wrote:
>> On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>>>
>>> On 2025/6/20 12:29, Greg KH wrote:
>>>> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>>>>> On 2025/6/19 19:01, Greg KH wrote:
>>>>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>
>>>>>>> The vhci driver does not need to create a platform device,
>>>>>>> it only did so because it was simple to do that in order to
>>>>>>> get a place in sysfs to hang some device-specific attributes.
>>>>>>> Now the faux device interface is more appropriate,change it
>>>>>>> over to use the faux bus instead.
>>>>>>>
>>>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>> ---
>>>>>>> Changes in v2:
>>>>>>> - don't change faux create api,just call probe on vhci_hcd_init.
>>>>>>>
>>>>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>>>>> drivers/usb/usbip/vhci_hcd.c | 86
>>>>>>> +++++++++++-----------------
>>>>>>> drivers/usb/usbip/vhci_sysfs.c | 68
>>>>>>> +++++++++++-----------
>>>>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>>>>> I get the following build errors from this patch:
>>>>>>
>>>>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’
>>>>>> defined but not used [-Werror=unused-function]
>>>>>> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>>>>> | ^~~~~~~~~~~~~~~
>>>>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’
>>>>>> defined but not used [-Werror=unused-function]
>>>>>> 1418 | static int vhci_hcd_suspend(struct faux_device *fdev,
>>>>>> pm_message_t state)
>>>>>> | ^~~~~~~~~~~~~~~~
>>>>>> cc1: all warnings being treated as errors
>>>>>>
>>>>>> Are you sure you tested this?
>>>>> I apologize for not enabling -Werror, which resulted in missing
>>>>> this error
>>>>> warning.
>>>>> I have tested usbip feature use the new patch,but not test system
>>>>> suspend/resume.
>>>>> The faux bus type don't add pm function,and vhci-hcd driver can't
>>>>> register
>>>>> it.
>>>>> Maybe have to add suspend/resume for it.like below:
>>>>> static const struct bus_type faux_bus_type = {
>>>>> .name = "faux",
>>>>> .match = faux_match,
>>>>> .probe = faux_probe,
>>>>> .remove = faux_remove,
>>>>> .resume = faux_resume,
>>>>> .suspend = faux_suspend,
>>>>> };
>>>>>
>>>>> Is that right?
>>>>> Your expertise would be greatly valued.
>>>> As this is not real hardware, why do you need the suspend/resume
>>>> callbacks at all? What is happening here that requires them?
>>> @greg,
>>> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed for
>>> this
>>> faux device, but rather to
>>> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags
>>> associated with
>>> the faux device.
>>> For example:
>>> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>
>>> Previously, these two functions were registered through
>>> platform_driver,
>>> but faux bus does not have the relevant interface, so they were not
>>> called,
>>> resulting in this compilation warning error.
>>>
>>> This raises the question: Should the faux bus implement PM-related
>>> interface?
>>> I'm uncertain whether these functions are essential for the vhci-hcd
>>> driver
>>> or if they can be safely removed.
>>>
>>> However, during system standby/wakeup tests with remote USB devices
>>> bound to
>>> the vhci-hcd driver,
>>> I observed consistent failure scenarios across both the original
>>> platform
>>> bus and faux bus patch implementations.
>
> suspend and resume hooks have been in the code from beginning
> in the CONFIG_PM path. The original authors are skeptical about
> what should happen during suspend"
>
> /* what should happen for USB/IP under suspend/resume? */
> suspend hook checks for active connections and sends EBBUSY
> back to pm core.
>
> Active connection means any of the ports are in USB_PORT_STAT_CONNECTION
> state. So as long as there are active connections between vhci client
> and the host, suspend won't work. So we really don't know what happens
> to the active connections if there are no suspend/resume hooks.
>
> If there are no active connections, then it will clear the
> HCD_FLAG_HW_ACCESSIBLE
> bit and returns to pm core to continue with suspend.
>
> resume sets the HCD_FLAG_HW_ACCESSIBLE flag to indicate hardware is
> accessible
> and initiates port status poll.
>
> - suspend hook prevents suspend
>
> With faux device since there is no suspend and resume hook, I would
> expect
> these hooks are not a factor in suspend and resume.
>
> vhci_hcd is a special case virtual driver as it is a proxy for
> controlling
> hardware on the host.
>
> Zongmin,
>
> Do suspend/resume work when vhci_hcd is not loaded?
> What happens when when system suspends and resumes with faux device?
> Can you send me dmesg logs and pm logs?
>
Hi Greg,shuah
Yes, system with vhci_hcd unload or just load vhci_hcd driver
without usbip devices bound to vhci_hcd,system suspend/resume worked well.
Some logs for you.
1.system with vhci_hcd driver load,but don't bound any usbip devices to
vhci_hcd,suspend/resume worked well.
see dmesg-faux bus-load.log
2.usbip device bound to vhci_hcd,and do system suspend
action,suspend/resume worked failed.
I observed two distinct failure scenario:
Scenario 1: System failed to enter suspend state,and will auto resume;
the log for use platform bus:
dmesg-platform bus-device bound-auto resume.log
the log for use faux bus:
dmesg-faux bus-device bound-auto resume.log
Scenario 2: System resume failed with black screen freeze,a forced
restart of the machine is require.
the log for use platform bus:
dmesg-platform bus-device bound-black screen.log
the log for use faux bus:
dmesg-faux bus-device bound-black screen.log
Hope these message is helpful.
> thanks,
> -- Shuah
>
>
>
On 6/23/25 21:21, Zongmin Zhou wrote:
>
> On 2025/6/21 01:26, Shuah Khan wrote:
>> On 6/20/25 03:27, Greg KH wrote:
>>> On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>>>>
>>>> On 2025/6/20 12:29, Greg KH wrote:
>>>>> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>>>>>> On 2025/6/19 19:01, Greg KH wrote:
>>>>>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>
>>>>>>>> The vhci driver does not need to create a platform device,
>>>>>>>> it only did so because it was simple to do that in order to
>>>>>>>> get a place in sysfs to hang some device-specific attributes.
>>>>>>>> Now the faux device interface is more appropriate,change it
>>>>>>>> over to use the faux bus instead.
>>>>>>>>
>>>>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>> ---
>>>>>>>> Changes in v2:
>>>>>>>> - don't change faux create api,just call probe on vhci_hcd_init.
>>>>>>>>
>>>>>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>>>>>> drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
>>>>>>>> drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
>>>>>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>>>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>>>>>> I get the following build errors from this patch:
>>>>>>>
>>>>>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
>>>>>>> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>>>>>> | ^~~~~~~~~~~~~~~
>>>>>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
>>>>>>> 1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
>>>>>>> | ^~~~~~~~~~~~~~~~
>>>>>>> cc1: all warnings being treated as errors
>>>>>>>
>>>>>>> Are you sure you tested this?
>>>>>> I apologize for not enabling -Werror, which resulted in missing this error
>>>>>> warning.
>>>>>> I have tested usbip feature use the new patch,but not test system
>>>>>> suspend/resume.
>>>>>> The faux bus type don't add pm function,and vhci-hcd driver can't register
>>>>>> it.
>>>>>> Maybe have to add suspend/resume for it.like below:
>>>>>> static const struct bus_type faux_bus_type = {
>>>>>> .name = "faux",
>>>>>> .match = faux_match,
>>>>>> .probe = faux_probe,
>>>>>> .remove = faux_remove,
>>>>>> .resume = faux_resume,
>>>>>> .suspend = faux_suspend,
>>>>>> };
>>>>>>
>>>>>> Is that right?
>>>>>> Your expertise would be greatly valued.
>>>>> As this is not real hardware, why do you need the suspend/resume
>>>>> callbacks at all? What is happening here that requires them?
>>>> @greg,
>>>> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed for this
>>>> faux device, but rather to
>>>> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags associated with
>>>> the faux device.
>>>> For example:
>>>> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>
>>>> Previously, these two functions were registered through platform_driver,
>>>> but faux bus does not have the relevant interface, so they were not called,
>>>> resulting in this compilation warning error.
>>>>
>>>> This raises the question: Should the faux bus implement PM-related
>>>> interface?
>>>> I'm uncertain whether these functions are essential for the vhci-hcd driver
>>>> or if they can be safely removed.
>>>>
>>>> However, during system standby/wakeup tests with remote USB devices bound to
>>>> the vhci-hcd driver,
>>>> I observed consistent failure scenarios across both the original platform
>>>> bus and faux bus patch implementations.
>>
>> suspend and resume hooks have been in the code from beginning
>> in the CONFIG_PM path. The original authors are skeptical about
>> what should happen during suspend"
>>
>> /* what should happen for USB/IP under suspend/resume? */
>> suspend hook checks for active connections and sends EBBUSY
>> back to pm core.
>>
>> Active connection means any of the ports are in USB_PORT_STAT_CONNECTION
>> state. So as long as there are active connections between vhci client
>> and the host, suspend won't work. So we really don't know what happens
>> to the active connections if there are no suspend/resume hooks.
>>
>> If there are no active connections, then it will clear the HCD_FLAG_HW_ACCESSIBLE
>> bit and returns to pm core to continue with suspend.
>>
>> resume sets the HCD_FLAG_HW_ACCESSIBLE flag to indicate hardware is accessible
>> and initiates port status poll.
>>
>> - suspend hook prevents suspend
>>
>> With faux device since there is no suspend and resume hook, I would expect
>> these hooks are not a factor in suspend and resume.
>>
>> vhci_hcd is a special case virtual driver as it is a proxy for controlling
>> hardware on the host.
>>
>> Zongmin,
>>
>> Do suspend/resume work when vhci_hcd is not loaded?
>> What happens when when system suspends and resumes with faux device?
>> Can you send me dmesg logs and pm logs?
>>
> Hi Greg,shuah
>
> Yes, system with vhci_hcd unload or just load vhci_hcd driver
> without usbip devices bound to vhci_hcd,system suspend/resume worked well.
> Some logs for you.
Sorry for the delay. I was at a conference last week.
Thank you for sending these logs.
> 1.system with vhci_hcd driver load,but don't bound any usbip devices to vhci_hcd,suspend/resume worked well.
> see dmesg-faux bus-load.log
>
> 2.usbip device bound to vhci_hcd,and do system suspend action,suspend/resume worked failed.
> I observed two distinct failure scenario:
> Scenario 1: System failed to enter suspend state,and will auto resume;
> the log for use platform bus:
> dmesg-platform bus-device bound-auto resume.log
This is clear from the suspend hook in the vhci_hcd.
When it returns EBUSY, suspend will fail and move to
auto resume.
> the log for use faux bus:
> dmesg-faux bus-device bound-auto resume.log
It is good that the behavior is same with faux bus even though
suspend hook isn't called. I will take a look at the log.
>
> Scenario 2: System resume failed with black screen freeze,a forced restart of the machine is require.
> the log for use platform bus:
> dmesg-platform bus-device bound-black screen.log
> the log for use faux bus:
> dmesg-faux bus-device bound-black screen.log
That is bad. When does this happen? Is there a difference in
configuration?
The behavior same for platform bus and faux bus. Sounds like
an existing problem that needs to be debugged separately.
I will take a look at all these logs and get back to you in
a day or two.
thanks,
-- Shuah
On 2025/7/2 06:56, Shuah Khan wrote:
> On 6/23/25 21:21, Zongmin Zhou wrote:
>>
>> On 2025/6/21 01:26, Shuah Khan wrote:
>>> On 6/20/25 03:27, Greg KH wrote:
>>>> On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>>>>>
>>>>> On 2025/6/20 12:29, Greg KH wrote:
>>>>>> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>>>>>>> On 2025/6/19 19:01, Greg KH wrote:
>>>>>>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>>>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>
>>>>>>>>> The vhci driver does not need to create a platform device,
>>>>>>>>> it only did so because it was simple to do that in order to
>>>>>>>>> get a place in sysfs to hang some device-specific attributes.
>>>>>>>>> Now the faux device interface is more appropriate,change it
>>>>>>>>> over to use the faux bus instead.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>> ---
>>>>>>>>> Changes in v2:
>>>>>>>>> - don't change faux create api,just call probe on vhci_hcd_init.
>>>>>>>>>
>>>>>>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>>>>>>> drivers/usb/usbip/vhci_hcd.c | 86
>>>>>>>>> +++++++++++-----------------
>>>>>>>>> drivers/usb/usbip/vhci_sysfs.c | 68
>>>>>>>>> +++++++++++-----------
>>>>>>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>>>>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>>>>>>> I get the following build errors from this patch:
>>>>>>>>
>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’
>>>>>>>> defined but not used [-Werror=unused-function]
>>>>>>>> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>>>>>>> | ^~~~~~~~~~~~~~~
>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’
>>>>>>>> defined but not used [-Werror=unused-function]
>>>>>>>> 1418 | static int vhci_hcd_suspend(struct faux_device
>>>>>>>> *fdev, pm_message_t state)
>>>>>>>> | ^~~~~~~~~~~~~~~~
>>>>>>>> cc1: all warnings being treated as errors
>>>>>>>>
>>>>>>>> Are you sure you tested this?
>>>>>>> I apologize for not enabling -Werror, which resulted in missing
>>>>>>> this error
>>>>>>> warning.
>>>>>>> I have tested usbip feature use the new patch,but not test system
>>>>>>> suspend/resume.
>>>>>>> The faux bus type don't add pm function,and vhci-hcd driver
>>>>>>> can't register
>>>>>>> it.
>>>>>>> Maybe have to add suspend/resume for it.like below:
>>>>>>> static const struct bus_type faux_bus_type = {
>>>>>>> .name = "faux",
>>>>>>> .match = faux_match,
>>>>>>> .probe = faux_probe,
>>>>>>> .remove = faux_remove,
>>>>>>> .resume = faux_resume,
>>>>>>> .suspend = faux_suspend,
>>>>>>> };
>>>>>>>
>>>>>>> Is that right?
>>>>>>> Your expertise would be greatly valued.
>>>>>> As this is not real hardware, why do you need the suspend/resume
>>>>>> callbacks at all? What is happening here that requires them?
>>>>> @greg,
>>>>> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed
>>>>> for this
>>>>> faux device, but rather to
>>>>> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags
>>>>> associated with
>>>>> the faux device.
>>>>> For example:
>>>>> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>>
>>>>> Previously, these two functions were registered through
>>>>> platform_driver,
>>>>> but faux bus does not have the relevant interface, so they were
>>>>> not called,
>>>>> resulting in this compilation warning error.
>>>>>
>>>>> This raises the question: Should the faux bus implement PM-related
>>>>> interface?
>>>>> I'm uncertain whether these functions are essential for the
>>>>> vhci-hcd driver
>>>>> or if they can be safely removed.
>>>>>
>>>>> However, during system standby/wakeup tests with remote USB
>>>>> devices bound to
>>>>> the vhci-hcd driver,
>>>>> I observed consistent failure scenarios across both the original
>>>>> platform
>>>>> bus and faux bus patch implementations.
>>>
>>> suspend and resume hooks have been in the code from beginning
>>> in the CONFIG_PM path. The original authors are skeptical about
>>> what should happen during suspend"
>>>
>>> /* what should happen for USB/IP under suspend/resume? */
>>> suspend hook checks for active connections and sends EBBUSY
>>> back to pm core.
>>>
>>> Active connection means any of the ports are in
>>> USB_PORT_STAT_CONNECTION
>>> state. So as long as there are active connections between vhci client
>>> and the host, suspend won't work. So we really don't know what happens
>>> to the active connections if there are no suspend/resume hooks.
>>>
>>> If there are no active connections, then it will clear the
>>> HCD_FLAG_HW_ACCESSIBLE
>>> bit and returns to pm core to continue with suspend.
>>>
>>> resume sets the HCD_FLAG_HW_ACCESSIBLE flag to indicate hardware is
>>> accessible
>>> and initiates port status poll.
>>>
>>> - suspend hook prevents suspend
>>>
>>> With faux device since there is no suspend and resume hook, I would
>>> expect
>>> these hooks are not a factor in suspend and resume.
>>>
>>> vhci_hcd is a special case virtual driver as it is a proxy for
>>> controlling
>>> hardware on the host.
>>>
>>> Zongmin,
>>>
>>> Do suspend/resume work when vhci_hcd is not loaded?
>>> What happens when when system suspends and resumes with faux device?
>>> Can you send me dmesg logs and pm logs?
>>>
>> Hi Greg,shuah
>>
>> Yes, system with vhci_hcd unload or just load vhci_hcd driver
>> without usbip devices bound to vhci_hcd,system suspend/resume worked
>> well.
>> Some logs for you.
>
> Sorry for the delay. I was at a conference last week.
> Thank you for sending these logs.
>
>> 1.system with vhci_hcd driver load,but don't bound any usbip devices
>> to vhci_hcd,suspend/resume worked well.
>> see dmesg-faux bus-load.log
>
>
>>
>> 2.usbip device bound to vhci_hcd,and do system suspend
>> action,suspend/resume worked failed.
>> I observed two distinct failure scenario:
>> Scenario 1: System failed to enter suspend state,and will auto resume;
>
>
>> the log for use platform bus:
>> dmesg-platform bus-device bound-auto resume.log
>
> This is clear from the suspend hook in the vhci_hcd.
> When it returns EBUSY, suspend will fail and move to
> auto resume.
>
>> the log for use faux bus:
>> dmesg-faux bus-device bound-auto resume.log
>
> It is good that the behavior is same with faux bus even though
> suspend hook isn't called. I will take a look at the log.
>
>>
>> Scenario 2: System resume failed with black screen freeze,a forced
>> restart of the machine is require.
>> the log for use platform bus:
>> dmesg-platform bus-device bound-black screen.log
>> the log for use faux bus:
>> dmesg-faux bus-device bound-black screen.log
>
> That is bad. When does this happen? Is there a difference in
> configuration?
>
No, there's no difference. The same code is used for two different
failure scenarios.
I just tested many times. These two different situations occur
probabilistically.
But they both happened only when the USBIP device is bound to the
vhci_hcd driver.
> The behavior same for platform bus and faux bus. Sounds like
> an existing problem that needs to be debugged separately.
>
> I will take a look at all these logs and get back to you in
> a day or two.
>
> thanks,
> -- Shuah
On 7/1/25 20:12, Zongmin Zhou wrote:
>
> On 2025/7/2 06:56, Shuah Khan wrote:
>> On 6/23/25 21:21, Zongmin Zhou wrote:
>>>
>>> On 2025/6/21 01:26, Shuah Khan wrote:
>>>> On 6/20/25 03:27, Greg KH wrote:
>>>>> On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>>>>>>
>>>>>> On 2025/6/20 12:29, Greg KH wrote:
>>>>>>> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>>>>>>>> On 2025/6/19 19:01, Greg KH wrote:
>>>>>>>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>>>>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>>
>>>>>>>>>> The vhci driver does not need to create a platform device,
>>>>>>>>>> it only did so because it was simple to do that in order to
>>>>>>>>>> get a place in sysfs to hang some device-specific attributes.
>>>>>>>>>> Now the faux device interface is more appropriate,change it
>>>>>>>>>> over to use the faux bus instead.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>> ---
>>>>>>>>>> Changes in v2:
>>>>>>>>>> - don't change faux create api,just call probe on vhci_hcd_init.
>>>>>>>>>>
>>>>>>>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
>>>>>>>>>> drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
>>>>>>>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>>>>>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>>>>>>>> I get the following build errors from this patch:
>>>>>>>>>
>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
>>>>>>>>> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>>>>>>>> | ^~~~~~~~~~~~~~~
>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
>>>>>>>>> 1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
>>>>>>>>> | ^~~~~~~~~~~~~~~~
>>>>>>>>> cc1: all warnings being treated as errors
>>>>>>>>>
>>>>>>>>> Are you sure you tested this?
>>>>>>>> I apologize for not enabling -Werror, which resulted in missing this error
>>>>>>>> warning.
>>>>>>>> I have tested usbip feature use the new patch,but not test system
>>>>>>>> suspend/resume.
>>>>>>>> The faux bus type don't add pm function,and vhci-hcd driver can't register
>>>>>>>> it.
>>>>>>>> Maybe have to add suspend/resume for it.like below:
>>>>>>>> static const struct bus_type faux_bus_type = {
>>>>>>>> .name = "faux",
>>>>>>>> .match = faux_match,
>>>>>>>> .probe = faux_probe,
>>>>>>>> .remove = faux_remove,
>>>>>>>> .resume = faux_resume,
>>>>>>>> .suspend = faux_suspend,
>>>>>>>> };
>>>>>>>>
>>>>>>>> Is that right?
>>>>>>>> Your expertise would be greatly valued.
>>>>>>> As this is not real hardware, why do you need the suspend/resume
>>>>>>> callbacks at all? What is happening here that requires them?
>>>>>> @greg,
>>>>>> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed for this
>>>>>> faux device, but rather to
>>>>>> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags associated with
>>>>>> the faux device.
>>>>>> For example:
>>>>>> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>>> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>>>
>>>>>> Previously, these two functions were registered through platform_driver,
>>>>>> but faux bus does not have the relevant interface, so they were not called,
>>>>>> resulting in this compilation warning error.
>>>>>>
>>>>>> This raises the question: Should the faux bus implement PM-related
>>>>>> interface?
>>>>>> I'm uncertain whether these functions are essential for the vhci-hcd driver
>>>>>> or if they can be safely removed.
>>>>>>
>>>>>> However, during system standby/wakeup tests with remote USB devices bound to
>>>>>> the vhci-hcd driver,
>>>>>> I observed consistent failure scenarios across both the original platform
>>>>>> bus and faux bus patch implementations.
>>>>
>>>> suspend and resume hooks have been in the code from beginning
>>>> in the CONFIG_PM path. The original authors are skeptical about
>>>> what should happen during suspend"
>>>>
>>>> /* what should happen for USB/IP under suspend/resume? */
>>>> suspend hook checks for active connections and sends EBBUSY
>>>> back to pm core.
>>>>
>>>> Active connection means any of the ports are in USB_PORT_STAT_CONNECTION
>>>> state. So as long as there are active connections between vhci client
>>>> and the host, suspend won't work. So we really don't know what happens
>>>> to the active connections if there are no suspend/resume hooks.
>>>>
>>>> If there are no active connections, then it will clear the HCD_FLAG_HW_ACCESSIBLE
>>>> bit and returns to pm core to continue with suspend.
>>>>
>>>> resume sets the HCD_FLAG_HW_ACCESSIBLE flag to indicate hardware is accessible
>>>> and initiates port status poll.
>>>>
>>>> - suspend hook prevents suspend
>>>>
>>>> With faux device since there is no suspend and resume hook, I would expect
>>>> these hooks are not a factor in suspend and resume.
>>>>
>>>> vhci_hcd is a special case virtual driver as it is a proxy for controlling
>>>> hardware on the host.
>>>>
>>>> Zongmin,
>>>>
>>>> Do suspend/resume work when vhci_hcd is not loaded?
>>>> What happens when when system suspends and resumes with faux device?
>>>> Can you send me dmesg logs and pm logs?
>>>>
>>> Hi Greg,shuah
>>>
>>> Yes, system with vhci_hcd unload or just load vhci_hcd driver
>>> without usbip devices bound to vhci_hcd,system suspend/resume worked well.
>>> Some logs for you.
>>
>> Sorry for the delay. I was at a conference last week.
>> Thank you for sending these logs.
>>
>>> 1.system with vhci_hcd driver load,but don't bound any usbip devices to vhci_hcd,suspend/resume worked well.
>>> see dmesg-faux bus-load.log
>>
>>
>>>
>>> 2.usbip device bound to vhci_hcd,and do system suspend action,suspend/resume worked failed.
>>> I observed two distinct failure scenario:
>>> Scenario 1: System failed to enter suspend state,and will auto resume;
>>
>>
>>> the log for use platform bus:
>>> dmesg-platform bus-device bound-auto resume.log
>>
>> This is clear from the suspend hook in the vhci_hcd.
>> When it returns EBUSY, suspend will fail and move to
>> auto resume.
>>
>>> the log for use faux bus:
>>> dmesg-faux bus-device bound-auto resume.log
>>
>> It is good that the behavior is same with faux bus even though
>> suspend hook isn't called. I will take a look at the log.
>>
>>>
>>> Scenario 2: System resume failed with black screen freeze,a forced restart of the machine is require.
>>> the log for use platform bus:
>>> dmesg-platform bus-device bound-black screen.log
>>> the log for use faux bus:
>>> dmesg-faux bus-device bound-black screen.log
>>
>> That is bad. When does this happen? Is there a difference in
>> configuration?
>>
> No, there's no difference. The same code is used for two different failure scenarios.
> I just tested many times. These two different situations occur probabilistically.
> But they both happened only when the USBIP device is bound to the vhci_hcd driver.
>> The behavior same for platform bus and faux bus. Sounds like
>> an existing problem that needs to be debugged separately.
>>
>> I will take a look at all these logs and get back to you in
>> a day or two.
>>
I looked at the logs and here is what I found:
Scenario 1:
dmesg-faux bus-device bound-auto resume.log
dmesg-platform bus-device bound-auto resume.log
In this case suspend bailed out way before driver suspend
when vhci_hcd is using platform and faux bus.
Freezing remaining freezable tasks failed after 20.006 seconds (0 tasks refusing to freeze, wq_busy=1)
Restarting kernel threads ... done
OOM killer enabled.
Restarting tasks ... done.
random: crng reseeded on system resumption
PM: suspend exit
Auto-resume of the user-space worked. Scenario 1 isn't really
interesting.
Scenario 2:
dmesg-faux bus-device bound-black screen.log
dmesg-platform bus-device bound-black screen.log
Even though the result is the same in seeing blank screen, how
we get there is different.
=================
faux-bus device:
=================
- suspend worked - looks like
usb 4-1: PM: calling usb_dev_suspend @ 6054, parent: usb4
usb 4-1: PM: usb_dev_suspend returned 0 after 13675 usecs
usb usb4: PM: calling usb_dev_suspend @ 6055, parent: vhci_hcd.0
usb usb4: PM: usb_dev_suspend returned 0 after 9 usecs
vhci_hcd suspend isn't in play here. usb_dev_suspend() returns.
See below
usb 4-1: PM: usb_dev_suspend returned message.
-------------------------------------------------------------
- resume started (assuming it has been initiated by user)
[ 650.027491][ T6056] pci 0000:00:01.0: PM: pci_pm_suspend_noirq returned 0 after 304 usecs
See see timestamp difference between the last suspend message and the
first resume message.
[ 674.000257][ T39] pci 0000:00:00.0: PM: calling pci_pm_resume_noirq @ 39, parent: pci0000:00
usb 4-1 usb_dev_resume never returns.
[ 674.071125][ T6117] usb usb4: PM: usb_dev_resume returned 0 after 21110 usecs
[ 674.113991][ T6126] usb 4-1: PM: calling usb_dev_resume @ 6126, parent: usb4
-------------------------------------------------------------
=====================
platform bus device
=====================
- suspend was aborted because vhci_hcd suspend routine returned error
[ 297.854621][ T9402] usb 4-1: PM: calling usb_dev_suspend @ 9402, parent: usb4
[ 297.868524][ T9402] usb 4-1: PM: usb_dev_suspend returned 0 after 13214 usecs
[ 297.869994][ T9403] usb usb4: PM: calling usb_dev_suspend @ 9403, parent: vhci_hcd.0
[ 297.871959][ T9403] usb usb4: PM: usb_dev_suspend returned 0 after 30 usecs
[ 297.873644][ T9394] vhci_hcd vhci_hcd.0: PM: calling platform_pm_suspend @ 9394, parent: platform
[ 297.874738][ T9394] vhci_hcd vhci_hcd.0: We have 1 active connection. Do not suspend.
[ 297.875369][ T9394] vhci_hcd vhci_hcd.0: PM: dpm_run_callback(): platform_pm_suspend returns -16
[ 297.876078][ T9394] vhci_hcd vhci_hcd.0: PM: platform_pm_suspend returned -16 after 1341 usecs
[ 297.876774][ T9394] vhci_hcd vhci_hcd.0: PM: failed to suspend: error -16
- the following triggers resume
[ 297.877321][ T9394] PM: Some devices failed to suspend, or early wake event detected
[ 297.881065][ T9403] usb usb3: PM: usb_dev_resume returned 0 after 19 usecs
[ 297.904551][ T9408] usb usb4: PM: usb_dev_resume returned 0 after 22911 usecs
[ 297.905148][ T9418] usb 4-1: PM: calling usb_dev_resume @ 9418, parent: usb4
usb 4-1 usb_dev_resume never returns.
Note - In both cases, usb_dev_resume doesn't return. When it is called is the
difference.
I don't think suspend/resume works when devices are bound. Suspend exits and
starts resume which seems to fail because it doesn't handle the virtual usb
device resume. There is a missing piece here.
When vhci_hcd imports a device and acts as a proxy between the virtual mass
storage device (e.g in this case) - it appears suspend and resume are
handled as if it is a usb device. Maybe this is incorrect?
usb_dev_suspend() works and usb_dev_resume() on these virtual usb devices?
Do we need to handle this in usb_dev_resume()?
Talking out loud - I have to do some looking into.
thanks,
-- Shuah
On 2025/7/3 07:54, Shuah Khan wrote:
> On 7/1/25 20:12, Zongmin Zhou wrote:
>>
>> On 2025/7/2 06:56, Shuah Khan wrote:
>>> On 6/23/25 21:21, Zongmin Zhou wrote:
>>>>
>>>> On 2025/6/21 01:26, Shuah Khan wrote:
>>>>> On 6/20/25 03:27, Greg KH wrote:
>>>>>> On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>>>>>>>
>>>>>>> On 2025/6/20 12:29, Greg KH wrote:
>>>>>>>> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>>>>>>>>> On 2025/6/19 19:01, Greg KH wrote:
>>>>>>>>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>>>>>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>>>
>>>>>>>>>>> The vhci driver does not need to create a platform device,
>>>>>>>>>>> it only did so because it was simple to do that in order to
>>>>>>>>>>> get a place in sysfs to hang some device-specific attributes.
>>>>>>>>>>> Now the faux device interface is more appropriate,change it
>>>>>>>>>>> over to use the faux bus instead.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>>> ---
>>>>>>>>>>> Changes in v2:
>>>>>>>>>>> - don't change faux create api,just call probe on
>>>>>>>>>>> vhci_hcd_init.
>>>>>>>>>>>
>>>>>>>>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c | 86
>>>>>>>>>>> +++++++++++-----------------
>>>>>>>>>>> drivers/usb/usbip/vhci_sysfs.c | 68
>>>>>>>>>>> +++++++++++-----------
>>>>>>>>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>>>>>>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>>>>>>>>> I get the following build errors from this patch:
>>>>>>>>>>
>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error:
>>>>>>>>>> ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
>>>>>>>>>> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>>>>>>>>> | ^~~~~~~~~~~~~~~
>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error:
>>>>>>>>>> ‘vhci_hcd_suspend’ defined but not used
>>>>>>>>>> [-Werror=unused-function]
>>>>>>>>>> 1418 | static int vhci_hcd_suspend(struct faux_device
>>>>>>>>>> *fdev, pm_message_t state)
>>>>>>>>>> | ^~~~~~~~~~~~~~~~
>>>>>>>>>> cc1: all warnings being treated as errors
>>>>>>>>>>
>>>>>>>>>> Are you sure you tested this?
>>>>>>>>> I apologize for not enabling -Werror, which resulted in
>>>>>>>>> missing this error
>>>>>>>>> warning.
>>>>>>>>> I have tested usbip feature use the new patch,but not test system
>>>>>>>>> suspend/resume.
>>>>>>>>> The faux bus type don't add pm function,and vhci-hcd driver
>>>>>>>>> can't register
>>>>>>>>> it.
>>>>>>>>> Maybe have to add suspend/resume for it.like below:
>>>>>>>>> static const struct bus_type faux_bus_type = {
>>>>>>>>> .name = "faux",
>>>>>>>>> .match = faux_match,
>>>>>>>>> .probe = faux_probe,
>>>>>>>>> .remove = faux_remove,
>>>>>>>>> .resume = faux_resume,
>>>>>>>>> .suspend = faux_suspend,
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> Is that right?
>>>>>>>>> Your expertise would be greatly valued.
>>>>>>>> As this is not real hardware, why do you need the suspend/resume
>>>>>>>> callbacks at all? What is happening here that requires them?
>>>>>>> @greg,
>>>>>>> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed
>>>>>>> for this
>>>>>>> faux device, but rather to
>>>>>>> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags
>>>>>>> associated with
>>>>>>> the faux device.
>>>>>>> For example:
>>>>>>> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE,
>>>>>>> &hcd->flags)
>>>>>>> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>>>>
>>>>>>> Previously, these two functions were registered through
>>>>>>> platform_driver,
>>>>>>> but faux bus does not have the relevant interface, so they were
>>>>>>> not called,
>>>>>>> resulting in this compilation warning error.
>>>>>>>
>>>>>>> This raises the question: Should the faux bus implement PM-related
>>>>>>> interface?
>>>>>>> I'm uncertain whether these functions are essential for the
>>>>>>> vhci-hcd driver
>>>>>>> or if they can be safely removed.
>>>>>>>
>>>>>>> However, during system standby/wakeup tests with remote USB
>>>>>>> devices bound to
>>>>>>> the vhci-hcd driver,
>>>>>>> I observed consistent failure scenarios across both the original
>>>>>>> platform
>>>>>>> bus and faux bus patch implementations.
>>>>>
>>>>> suspend and resume hooks have been in the code from beginning
>>>>> in the CONFIG_PM path. The original authors are skeptical about
>>>>> what should happen during suspend"
>>>>>
>>>>> /* what should happen for USB/IP under suspend/resume? */
>>>>> suspend hook checks for active connections and sends EBBUSY
>>>>> back to pm core.
>>>>>
>>>>> Active connection means any of the ports are in
>>>>> USB_PORT_STAT_CONNECTION
>>>>> state. So as long as there are active connections between vhci client
>>>>> and the host, suspend won't work. So we really don't know what
>>>>> happens
>>>>> to the active connections if there are no suspend/resume hooks.
>>>>>
>>>>> If there are no active connections, then it will clear the
>>>>> HCD_FLAG_HW_ACCESSIBLE
>>>>> bit and returns to pm core to continue with suspend.
>>>>>
>>>>> resume sets the HCD_FLAG_HW_ACCESSIBLE flag to indicate hardware
>>>>> is accessible
>>>>> and initiates port status poll.
>>>>>
>>>>> - suspend hook prevents suspend
>>>>>
>>>>> With faux device since there is no suspend and resume hook, I
>>>>> would expect
>>>>> these hooks are not a factor in suspend and resume.
>>>>>
>>>>> vhci_hcd is a special case virtual driver as it is a proxy for
>>>>> controlling
>>>>> hardware on the host.
>>>>>
>>>>> Zongmin,
>>>>>
>>>>> Do suspend/resume work when vhci_hcd is not loaded?
>>>>> What happens when when system suspends and resumes with faux device?
>>>>> Can you send me dmesg logs and pm logs?
>>>>>
>>>> Hi Greg,shuah
>>>>
>>>> Yes, system with vhci_hcd unload or just load vhci_hcd driver
>>>> without usbip devices bound to vhci_hcd,system suspend/resume
>>>> worked well.
>>>> Some logs for you.
>>>
>>> Sorry for the delay. I was at a conference last week.
>>> Thank you for sending these logs.
>>>
>>>> 1.system with vhci_hcd driver load,but don't bound any usbip
>>>> devices to vhci_hcd,suspend/resume worked well.
>>>> see dmesg-faux bus-load.log
>>>
>>>
>>>>
>>>> 2.usbip device bound to vhci_hcd,and do system suspend
>>>> action,suspend/resume worked failed.
>>>> I observed two distinct failure scenario:
>>>> Scenario 1: System failed to enter suspend state,and will auto resume;
>>>
>>>
>>>> the log for use platform bus:
>>>> dmesg-platform bus-device bound-auto resume.log
>>>
>>> This is clear from the suspend hook in the vhci_hcd.
>>> When it returns EBUSY, suspend will fail and move to
>>> auto resume.
>>>
>>>> the log for use faux bus:
>>>> dmesg-faux bus-device bound-auto resume.log
>>>
>>> It is good that the behavior is same with faux bus even though
>>> suspend hook isn't called. I will take a look at the log.
>>>
>>>>
>>>> Scenario 2: System resume failed with black screen freeze,a forced
>>>> restart of the machine is require.
>>>> the log for use platform bus:
>>>> dmesg-platform bus-device bound-black screen.log
>>>> the log for use faux bus:
>>>> dmesg-faux bus-device bound-black screen.log
>>>
>>> That is bad. When does this happen? Is there a difference in
>>> configuration?
>>>
>> No, there's no difference. The same code is used for two different
>> failure scenarios.
>> I just tested many times. These two different situations occur
>> probabilistically.
>> But they both happened only when the USBIP device is bound to the
>> vhci_hcd driver.
>>> The behavior same for platform bus and faux bus. Sounds like
>>> an existing problem that needs to be debugged separately.
>>>
>>> I will take a look at all these logs and get back to you in
>>> a day or two.
>>>
>
> I looked at the logs and here is what I found:
>
> Scenario 1:
> dmesg-faux bus-device bound-auto resume.log
> dmesg-platform bus-device bound-auto resume.log
>
> In this case suspend bailed out way before driver suspend
> when vhci_hcd is using platform and faux bus.
>
> Freezing remaining freezable tasks failed after 20.006 seconds (0
> tasks refusing to freeze, wq_busy=1)
> Restarting kernel threads ... done
> OOM killer enabled.
> Restarting tasks ... done.
> random: crng reseeded on system resumption
> PM: suspend exit
>
> Auto-resume of the user-space worked. Scenario 1 isn't really
> interesting.
>
> Scenario 2:
> dmesg-faux bus-device bound-black screen.log
> dmesg-platform bus-device bound-black screen.log
>
> Even though the result is the same in seeing blank screen, how
> we get there is different.
>
> =================
> faux-bus device:
> =================
> - suspend worked - looks like
> usb 4-1: PM: calling usb_dev_suspend @ 6054, parent: usb4
> usb 4-1: PM: usb_dev_suspend returned 0 after 13675 usecs
> usb usb4: PM: calling usb_dev_suspend @ 6055, parent: vhci_hcd.0
> usb usb4: PM: usb_dev_suspend returned 0 after 9 usecs
>
> vhci_hcd suspend isn't in play here. usb_dev_suspend() returns.
> See below
>
> usb 4-1: PM: usb_dev_suspend returned message.
>
> -------------------------------------------------------------
>
> - resume started (assuming it has been initiated by user)
>
> [ 650.027491][ T6056] pci 0000:00:01.0: PM: pci_pm_suspend_noirq
> returned 0 after 304 usecs
>
> See see timestamp difference between the last suspend message and the
> first resume message.
> [ 674.000257][ T39] pci 0000:00:00.0: PM: calling
> pci_pm_resume_noirq @ 39, parent: pci0000:00
>
> usb 4-1 usb_dev_resume never returns.
>
> [ 674.071125][ T6117] usb usb4: PM: usb_dev_resume returned 0 after
> 21110 usecs
> [ 674.113991][ T6126] usb 4-1: PM: calling usb_dev_resume @ 6126,
> parent: usb4
>
> -------------------------------------------------------------
>
> =====================
> platform bus device
> =====================
>
> - suspend was aborted because vhci_hcd suspend routine returned error
>
> [ 297.854621][ T9402] usb 4-1: PM: calling usb_dev_suspend @ 9402,
> parent: usb4
> [ 297.868524][ T9402] usb 4-1: PM: usb_dev_suspend returned 0 after
> 13214 usecs
> [ 297.869994][ T9403] usb usb4: PM: calling usb_dev_suspend @ 9403,
> parent: vhci_hcd.0
> [ 297.871959][ T9403] usb usb4: PM: usb_dev_suspend returned 0 after
> 30 usecs
> [ 297.873644][ T9394] vhci_hcd vhci_hcd.0: PM: calling
> platform_pm_suspend @ 9394, parent: platform
> [ 297.874738][ T9394] vhci_hcd vhci_hcd.0: We have 1 active
> connection. Do not suspend.
> [ 297.875369][ T9394] vhci_hcd vhci_hcd.0: PM: dpm_run_callback():
> platform_pm_suspend returns -16
> [ 297.876078][ T9394] vhci_hcd vhci_hcd.0: PM: platform_pm_suspend
> returned -16 after 1341 usecs
> [ 297.876774][ T9394] vhci_hcd vhci_hcd.0: PM: failed to suspend:
> error -16
>
> - the following triggers resume
> [ 297.877321][ T9394] PM: Some devices failed to suspend, or early
> wake event detected
>
> [ 297.881065][ T9403] usb usb3: PM: usb_dev_resume returned 0 after
> 19 usecs
> [ 297.904551][ T9408] usb usb4: PM: usb_dev_resume returned 0 after
> 22911 usecs
> [ 297.905148][ T9418] usb 4-1: PM: calling usb_dev_resume @ 9418,
> parent: usb4
>
> usb 4-1 usb_dev_resume never returns.
>
> Note - In both cases, usb_dev_resume doesn't return. When it is called
> is the
> difference.
>
> I don't think suspend/resume works when devices are bound. Suspend
> exits and
> starts resume which seems to fail because it doesn't handle the
> virtual usb
> device resume. There is a missing piece here.
>
> When vhci_hcd imports a device and acts as a proxy between the virtual
> mass
> storage device (e.g in this case) - it appears suspend and resume are
> handled as if it is a usb device. Maybe this is incorrect?
>
> usb_dev_suspend() works and usb_dev_resume() on these virtual usb
> devices?
> Do we need to handle this in usb_dev_resume()?
>
> Talking out loud - I have to do some looking into.
>
Re:
Yes, your analysis is completely correct.
In fact, I've experimented with adding PM hooks to the faux bus,
and found that faux bus devices then behave identically to platform bus
devices during suspend/resume.
See the attachment.
This is likely a historical legacy issue.
Regarding this matter, is there anything else you'd like me to assist with?
> thanks,
> -- Shuah
>
>
>
On 7/3/25 00:04, Zongmin Zhou wrote:
>
> On 2025/7/3 07:54, Shuah Khan wrote:
>> On 7/1/25 20:12, Zongmin Zhou wrote:
>>>
>>> On 2025/7/2 06:56, Shuah Khan wrote:
>>>> On 6/23/25 21:21, Zongmin Zhou wrote:
>>>>>
>>>>> On 2025/6/21 01:26, Shuah Khan wrote:
>>>>>> On 6/20/25 03:27, Greg KH wrote:
>>>>>>> On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>>>>>>>>
>>>>>>>> On 2025/6/20 12:29, Greg KH wrote:
>>>>>>>>> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>>>>>>>>>> On 2025/6/19 19:01, Greg KH wrote:
>>>>>>>>>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>>>>>>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>>>>
>>>>>>>>>>>> The vhci driver does not need to create a platform device,
>>>>>>>>>>>> it only did so because it was simple to do that in order to
>>>>>>>>>>>> get a place in sysfs to hang some device-specific attributes.
>>>>>>>>>>>> Now the faux device interface is more appropriate,change it
>>>>>>>>>>>> over to use the faux bus instead.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>>>> ---
>>>>>>>>>>>> Changes in v2:
>>>>>>>>>>>> - don't change faux create api,just call probe on vhci_hcd_init.
>>>>>>>>>>>>
>>>>>>>>>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
>>>>>>>>>>>> drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
>>>>>>>>>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>>>>>>>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>>>>>>>>>> I get the following build errors from this patch:
>>>>>>>>>>>
>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
>>>>>>>>>>> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>>>>>>>>>> | ^~~~~~~~~~~~~~~
>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
>>>>>>>>>>> 1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
>>>>>>>>>>> | ^~~~~~~~~~~~~~~~
>>>>>>>>>>> cc1: all warnings being treated as errors
>>>>>>>>>>>
>>>>>>>>>>> Are you sure you tested this?
>>>>>>>>>> I apologize for not enabling -Werror, which resulted in missing this error
>>>>>>>>>> warning.
>>>>>>>>>> I have tested usbip feature use the new patch,but not test system
>>>>>>>>>> suspend/resume.
>>>>>>>>>> The faux bus type don't add pm function,and vhci-hcd driver can't register
>>>>>>>>>> it.
>>>>>>>>>> Maybe have to add suspend/resume for it.like below:
>>>>>>>>>> static const struct bus_type faux_bus_type = {
>>>>>>>>>> .name = "faux",
>>>>>>>>>> .match = faux_match,
>>>>>>>>>> .probe = faux_probe,
>>>>>>>>>> .remove = faux_remove,
>>>>>>>>>> .resume = faux_resume,
>>>>>>>>>> .suspend = faux_suspend,
>>>>>>>>>> };
>>>>>>>>>>
>>>>>>>>>> Is that right?
>>>>>>>>>> Your expertise would be greatly valued.
>>>>>>>>> As this is not real hardware, why do you need the suspend/resume
>>>>>>>>> callbacks at all? What is happening here that requires them?
>>>>>>>> @greg,
>>>>>>>> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed for this
>>>>>>>> faux device, but rather to
>>>>>>>> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags associated with
>>>>>>>> the faux device.
>>>>>>>> For example:
>>>>>>>> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>>>>> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>>>>>
>>>>>>>> Previously, these two functions were registered through platform_driver,
>>>>>>>> but faux bus does not have the relevant interface, so they were not called,
>>>>>>>> resulting in this compilation warning error.
>>>>>>>>
>>>>>>>> This raises the question: Should the faux bus implement PM-related
>>>>>>>> interface?
>>>>>>>> I'm uncertain whether these functions are essential for the vhci-hcd driver
>>>>>>>> or if they can be safely removed.
>>>>>>>>
>>>>>>>> However, during system standby/wakeup tests with remote USB devices bound to
>>>>>>>> the vhci-hcd driver,
>>>>>>>> I observed consistent failure scenarios across both the original platform
>>>>>>>> bus and faux bus patch implementations.
>>>>>>
>>>>>> suspend and resume hooks have been in the code from beginning
>>>>>> in the CONFIG_PM path. The original authors are skeptical about
>>>>>> what should happen during suspend"
>>>>>>
>>>>>> /* what should happen for USB/IP under suspend/resume? */
>>>>>> suspend hook checks for active connections and sends EBBUSY
>>>>>> back to pm core.
>>>>>>
>>>>>> Active connection means any of the ports are in USB_PORT_STAT_CONNECTION
>>>>>> state. So as long as there are active connections between vhci client
>>>>>> and the host, suspend won't work. So we really don't know what happens
>>>>>> to the active connections if there are no suspend/resume hooks.
>>>>>>
>>>>>> If there are no active connections, then it will clear the HCD_FLAG_HW_ACCESSIBLE
>>>>>> bit and returns to pm core to continue with suspend.
>>>>>>
>>>>>> resume sets the HCD_FLAG_HW_ACCESSIBLE flag to indicate hardware is accessible
>>>>>> and initiates port status poll.
>>>>>>
>>>>>> - suspend hook prevents suspend
>>>>>>
>>>>>> With faux device since there is no suspend and resume hook, I would expect
>>>>>> these hooks are not a factor in suspend and resume.
>>>>>>
>>>>>> vhci_hcd is a special case virtual driver as it is a proxy for controlling
>>>>>> hardware on the host.
>>>>>>
>>>>>> Zongmin,
>>>>>>
>>>>>> Do suspend/resume work when vhci_hcd is not loaded?
>>>>>> What happens when when system suspends and resumes with faux device?
>>>>>> Can you send me dmesg logs and pm logs?
>>>>>>
>>>>> Hi Greg,shuah
>>>>>
>>>>> Yes, system with vhci_hcd unload or just load vhci_hcd driver
>>>>> without usbip devices bound to vhci_hcd,system suspend/resume worked well.
>>>>> Some logs for you.
>>>>
>>>> Sorry for the delay. I was at a conference last week.
>>>> Thank you for sending these logs.
>>>>
>>>>> 1.system with vhci_hcd driver load,but don't bound any usbip devices to vhci_hcd,suspend/resume worked well.
>>>>> see dmesg-faux bus-load.log
>>>>
>>>>
>>>>>
>>>>> 2.usbip device bound to vhci_hcd,and do system suspend action,suspend/resume worked failed.
>>>>> I observed two distinct failure scenario:
>>>>> Scenario 1: System failed to enter suspend state,and will auto resume;
>>>>
>>>>
>>>>> the log for use platform bus:
>>>>> dmesg-platform bus-device bound-auto resume.log
>>>>
>>>> This is clear from the suspend hook in the vhci_hcd.
>>>> When it returns EBUSY, suspend will fail and move to
>>>> auto resume.
>>>>
>>>>> the log for use faux bus:
>>>>> dmesg-faux bus-device bound-auto resume.log
>>>>
>>>> It is good that the behavior is same with faux bus even though
>>>> suspend hook isn't called. I will take a look at the log.
>>>>
>>>>>
>>>>> Scenario 2: System resume failed with black screen freeze,a forced restart of the machine is require.
>>>>> the log for use platform bus:
>>>>> dmesg-platform bus-device bound-black screen.log
>>>>> the log for use faux bus:
>>>>> dmesg-faux bus-device bound-black screen.log
>>>>
>>>> That is bad. When does this happen? Is there a difference in
>>>> configuration?
>>>>
>>> No, there's no difference. The same code is used for two different failure scenarios.
>>> I just tested many times. These two different situations occur probabilistically.
>>> But they both happened only when the USBIP device is bound to the vhci_hcd driver.
>>>> The behavior same for platform bus and faux bus. Sounds like
>>>> an existing problem that needs to be debugged separately.
>>>>
>>>> I will take a look at all these logs and get back to you in
>>>> a day or two.
>>>>
>>
>> I looked at the logs and here is what I found:
>>
>> Scenario 1:
>> dmesg-faux bus-device bound-auto resume.log
>> dmesg-platform bus-device bound-auto resume.log
>>
>> In this case suspend bailed out way before driver suspend
>> when vhci_hcd is using platform and faux bus.
>>
>> Freezing remaining freezable tasks failed after 20.006 seconds (0 tasks refusing to freeze, wq_busy=1)
>> Restarting kernel threads ... done
>> OOM killer enabled.
>> Restarting tasks ... done.
>> random: crng reseeded on system resumption
>> PM: suspend exit
>>
>> Auto-resume of the user-space worked. Scenario 1 isn't really
>> interesting.
>>
>> Scenario 2:
>> dmesg-faux bus-device bound-black screen.log
>> dmesg-platform bus-device bound-black screen.log
>>
>> Even though the result is the same in seeing blank screen, how
>> we get there is different.
>>
>> =================
>> faux-bus device:
>> =================
>> - suspend worked - looks like
>> usb 4-1: PM: calling usb_dev_suspend @ 6054, parent: usb4
>> usb 4-1: PM: usb_dev_suspend returned 0 after 13675 usecs
>> usb usb4: PM: calling usb_dev_suspend @ 6055, parent: vhci_hcd.0
>> usb usb4: PM: usb_dev_suspend returned 0 after 9 usecs
>>
>> vhci_hcd suspend isn't in play here. usb_dev_suspend() returns.
>> See below
>>
>> usb 4-1: PM: usb_dev_suspend returned message.
>>
>> -------------------------------------------------------------
>>
>> - resume started (assuming it has been initiated by user)
>>
>> [ 650.027491][ T6056] pci 0000:00:01.0: PM: pci_pm_suspend_noirq returned 0 after 304 usecs
>>
>> See see timestamp difference between the last suspend message and the
>> first resume message.
>> [ 674.000257][ T39] pci 0000:00:00.0: PM: calling pci_pm_resume_noirq @ 39, parent: pci0000:00
>>
>> usb 4-1 usb_dev_resume never returns.
>>
>> [ 674.071125][ T6117] usb usb4: PM: usb_dev_resume returned 0 after 21110 usecs
>> [ 674.113991][ T6126] usb 4-1: PM: calling usb_dev_resume @ 6126, parent: usb4
>>
>> -------------------------------------------------------------
>>
>> =====================
>> platform bus device
>> =====================
>>
>> - suspend was aborted because vhci_hcd suspend routine returned error
>>
>> [ 297.854621][ T9402] usb 4-1: PM: calling usb_dev_suspend @ 9402, parent: usb4
>> [ 297.868524][ T9402] usb 4-1: PM: usb_dev_suspend returned 0 after 13214 usecs
>> [ 297.869994][ T9403] usb usb4: PM: calling usb_dev_suspend @ 9403, parent: vhci_hcd.0
>> [ 297.871959][ T9403] usb usb4: PM: usb_dev_suspend returned 0 after 30 usecs
>> [ 297.873644][ T9394] vhci_hcd vhci_hcd.0: PM: calling platform_pm_suspend @ 9394, parent: platform
>> [ 297.874738][ T9394] vhci_hcd vhci_hcd.0: We have 1 active connection. Do not suspend.
>> [ 297.875369][ T9394] vhci_hcd vhci_hcd.0: PM: dpm_run_callback(): platform_pm_suspend returns -16
>> [ 297.876078][ T9394] vhci_hcd vhci_hcd.0: PM: platform_pm_suspend returned -16 after 1341 usecs
>> [ 297.876774][ T9394] vhci_hcd vhci_hcd.0: PM: failed to suspend: error -16
>>
>> - the following triggers resume
>> [ 297.877321][ T9394] PM: Some devices failed to suspend, or early wake event detected
>>
>> [ 297.881065][ T9403] usb usb3: PM: usb_dev_resume returned 0 after 19 usecs
>> [ 297.904551][ T9408] usb usb4: PM: usb_dev_resume returned 0 after 22911 usecs
>> [ 297.905148][ T9418] usb 4-1: PM: calling usb_dev_resume @ 9418, parent: usb4
>>
>> usb 4-1 usb_dev_resume never returns.
>>
>> Note - In both cases, usb_dev_resume doesn't return. When it is called is the
>> difference.
>>
>> I don't think suspend/resume works when devices are bound. Suspend exits and
>> starts resume which seems to fail because it doesn't handle the virtual usb
>> device resume. There is a missing piece here.
>>
>> When vhci_hcd imports a device and acts as a proxy between the virtual mass
>> storage device (e.g in this case) - it appears suspend and resume are
>> handled as if it is a usb device. Maybe this is incorrect?
>>
>> usb_dev_suspend() works and usb_dev_resume() on these virtual usb devices?
>> Do we need to handle this in usb_dev_resume()?
>>
>> Talking out loud - I have to do some looking into.
>>
> Re:
> Yes, your analysis is completely correct.
>
> In fact, I've experimented with adding PM hooks to the faux bus,
> and found that faux bus devices then behave identically to platform bus devices during suspend/resume.
> See the attachment.
>
Thanks for checking this scenario. No surprises here.
> This is likely a historical legacy issue.
It is an existing problem in the way vhci_hcd and the bound devices
handle suspend/resume. vhci_hcd suspend assumes once it returns
"don't suspend" the rest works. However suspend for the bound device
runs first and a subsequent resume on it fails.
This problem needs fixing - I don't know yet how to.
> Regarding this matter, is there anything else you'd like me to assist with?
>
One thing I am curious about is the status of the bound devices after
"forced restart of the machine" when you see blank screen or hang?
thanks,
-- Shuah
On 2025/7/9 02:16, Shuah Khan wrote:
> On 7/3/25 00:04, Zongmin Zhou wrote:
>>
>> On 2025/7/3 07:54, Shuah Khan wrote:
>>> On 7/1/25 20:12, Zongmin Zhou wrote:
>>>>
>>>> On 2025/7/2 06:56, Shuah Khan wrote:
>>>>> On 6/23/25 21:21, Zongmin Zhou wrote:
>>>>>>
>>>>>> On 2025/6/21 01:26, Shuah Khan wrote:
>>>>>>> On 6/20/25 03:27, Greg KH wrote:
>>>>>>>> On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>>>>>>>>>
>>>>>>>>> On 2025/6/20 12:29, Greg KH wrote:
>>>>>>>>>> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>>>>>>>>>>> On 2025/6/19 19:01, Greg KH wrote:
>>>>>>>>>>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>>>>>>>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>>>>>
>>>>>>>>>>>>> The vhci driver does not need to create a platform device,
>>>>>>>>>>>>> it only did so because it was simple to do that in order to
>>>>>>>>>>>>> get a place in sysfs to hang some device-specific attributes.
>>>>>>>>>>>>> Now the faux device interface is more appropriate,change it
>>>>>>>>>>>>> over to use the faux bus instead.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>> Changes in v2:
>>>>>>>>>>>>> - don't change faux create api,just call probe on
>>>>>>>>>>>>> vhci_hcd_init.
>>>>>>>>>>>>>
>>>>>>>>>>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c | 86
>>>>>>>>>>>>> +++++++++++-----------------
>>>>>>>>>>>>> drivers/usb/usbip/vhci_sysfs.c | 68
>>>>>>>>>>>>> +++++++++++-----------
>>>>>>>>>>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>>>>>>>>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>>>>>>>>>>> I get the following build errors from this patch:
>>>>>>>>>>>>
>>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error:
>>>>>>>>>>>> ‘vhci_hcd_resume’ defined but not used
>>>>>>>>>>>> [-Werror=unused-function]
>>>>>>>>>>>> 1462 | static int vhci_hcd_resume(struct faux_device
>>>>>>>>>>>> *fdev)
>>>>>>>>>>>> | ^~~~~~~~~~~~~~~
>>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error:
>>>>>>>>>>>> ‘vhci_hcd_suspend’ defined but not used
>>>>>>>>>>>> [-Werror=unused-function]
>>>>>>>>>>>> 1418 | static int vhci_hcd_suspend(struct faux_device
>>>>>>>>>>>> *fdev, pm_message_t state)
>>>>>>>>>>>> | ^~~~~~~~~~~~~~~~
>>>>>>>>>>>> cc1: all warnings being treated as errors
>>>>>>>>>>>>
>>>>>>>>>>>> Are you sure you tested this?
>>>>>>>>>>> I apologize for not enabling -Werror, which resulted in
>>>>>>>>>>> missing this error
>>>>>>>>>>> warning.
>>>>>>>>>>> I have tested usbip feature use the new patch,but not test
>>>>>>>>>>> system
>>>>>>>>>>> suspend/resume.
>>>>>>>>>>> The faux bus type don't add pm function,and vhci-hcd driver
>>>>>>>>>>> can't register
>>>>>>>>>>> it.
>>>>>>>>>>> Maybe have to add suspend/resume for it.like below:
>>>>>>>>>>> static const struct bus_type faux_bus_type = {
>>>>>>>>>>> .name = "faux",
>>>>>>>>>>> .match = faux_match,
>>>>>>>>>>> .probe = faux_probe,
>>>>>>>>>>> .remove = faux_remove,
>>>>>>>>>>> .resume = faux_resume,
>>>>>>>>>>> .suspend = faux_suspend,
>>>>>>>>>>> };
>>>>>>>>>>>
>>>>>>>>>>> Is that right?
>>>>>>>>>>> Your expertise would be greatly valued.
>>>>>>>>>> As this is not real hardware, why do you need the suspend/resume
>>>>>>>>>> callbacks at all? What is happening here that requires them?
>>>>>>>>> @greg,
>>>>>>>>> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not
>>>>>>>>> designed for this
>>>>>>>>> faux device, but rather to
>>>>>>>>> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags
>>>>>>>>> associated with
>>>>>>>>> the faux device.
>>>>>>>>> For example:
>>>>>>>>> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE,
>>>>>>>>> &hcd->flags)
>>>>>>>>> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE,
>>>>>>>>> &hcd->flags)
>>>>>>>>>
>>>>>>>>> Previously, these two functions were registered through
>>>>>>>>> platform_driver,
>>>>>>>>> but faux bus does not have the relevant interface, so they
>>>>>>>>> were not called,
>>>>>>>>> resulting in this compilation warning error.
>>>>>>>>>
>>>>>>>>> This raises the question: Should the faux bus implement
>>>>>>>>> PM-related
>>>>>>>>> interface?
>>>>>>>>> I'm uncertain whether these functions are essential for the
>>>>>>>>> vhci-hcd driver
>>>>>>>>> or if they can be safely removed.
>>>>>>>>>
>>>>>>>>> However, during system standby/wakeup tests with remote USB
>>>>>>>>> devices bound to
>>>>>>>>> the vhci-hcd driver,
>>>>>>>>> I observed consistent failure scenarios across both the
>>>>>>>>> original platform
>>>>>>>>> bus and faux bus patch implementations.
>>>>>>>
>>>>>>> suspend and resume hooks have been in the code from beginning
>>>>>>> in the CONFIG_PM path. The original authors are skeptical about
>>>>>>> what should happen during suspend"
>>>>>>>
>>>>>>> /* what should happen for USB/IP under suspend/resume? */
>>>>>>> suspend hook checks for active connections and sends EBBUSY
>>>>>>> back to pm core.
>>>>>>>
>>>>>>> Active connection means any of the ports are in
>>>>>>> USB_PORT_STAT_CONNECTION
>>>>>>> state. So as long as there are active connections between vhci
>>>>>>> client
>>>>>>> and the host, suspend won't work. So we really don't know what
>>>>>>> happens
>>>>>>> to the active connections if there are no suspend/resume hooks.
>>>>>>>
>>>>>>> If there are no active connections, then it will clear the
>>>>>>> HCD_FLAG_HW_ACCESSIBLE
>>>>>>> bit and returns to pm core to continue with suspend.
>>>>>>>
>>>>>>> resume sets the HCD_FLAG_HW_ACCESSIBLE flag to indicate hardware
>>>>>>> is accessible
>>>>>>> and initiates port status poll.
>>>>>>>
>>>>>>> - suspend hook prevents suspend
>>>>>>>
>>>>>>> With faux device since there is no suspend and resume hook, I
>>>>>>> would expect
>>>>>>> these hooks are not a factor in suspend and resume.
>>>>>>>
>>>>>>> vhci_hcd is a special case virtual driver as it is a proxy for
>>>>>>> controlling
>>>>>>> hardware on the host.
>>>>>>>
>>>>>>> Zongmin,
>>>>>>>
>>>>>>> Do suspend/resume work when vhci_hcd is not loaded?
>>>>>>> What happens when when system suspends and resumes with faux
>>>>>>> device?
>>>>>>> Can you send me dmesg logs and pm logs?
>>>>>>>
>>>>>> Hi Greg,shuah
>>>>>>
>>>>>> Yes, system with vhci_hcd unload or just load vhci_hcd driver
>>>>>> without usbip devices bound to vhci_hcd,system suspend/resume
>>>>>> worked well.
>>>>>> Some logs for you.
>>>>>
>>>>> Sorry for the delay. I was at a conference last week.
>>>>> Thank you for sending these logs.
>>>>>
>>>>>> 1.system with vhci_hcd driver load,but don't bound any usbip
>>>>>> devices to vhci_hcd,suspend/resume worked well.
>>>>>> see dmesg-faux bus-load.log
>>>>>
>>>>>
>>>>>>
>>>>>> 2.usbip device bound to vhci_hcd,and do system suspend
>>>>>> action,suspend/resume worked failed.
>>>>>> I observed two distinct failure scenario:
>>>>>> Scenario 1: System failed to enter suspend state,and will auto
>>>>>> resume;
>>>>>
>>>>>
>>>>>> the log for use platform bus:
>>>>>> dmesg-platform bus-device bound-auto resume.log
>>>>>
>>>>> This is clear from the suspend hook in the vhci_hcd.
>>>>> When it returns EBUSY, suspend will fail and move to
>>>>> auto resume.
>>>>>
>>>>>> the log for use faux bus:
>>>>>> dmesg-faux bus-device bound-auto resume.log
>>>>>
>>>>> It is good that the behavior is same with faux bus even though
>>>>> suspend hook isn't called. I will take a look at the log.
>>>>>
>>>>>>
>>>>>> Scenario 2: System resume failed with black screen freeze,a
>>>>>> forced restart of the machine is require.
>>>>>> the log for use platform bus:
>>>>>> dmesg-platform bus-device bound-black screen.log
>>>>>> the log for use faux bus:
>>>>>> dmesg-faux bus-device bound-black screen.log
>>>>>
>>>>> That is bad. When does this happen? Is there a difference in
>>>>> configuration?
>>>>>
>>>> No, there's no difference. The same code is used for two different
>>>> failure scenarios.
>>>> I just tested many times. These two different situations occur
>>>> probabilistically.
>>>> But they both happened only when the USBIP device is bound to the
>>>> vhci_hcd driver.
>>>>> The behavior same for platform bus and faux bus. Sounds like
>>>>> an existing problem that needs to be debugged separately.
>>>>>
>>>>> I will take a look at all these logs and get back to you in
>>>>> a day or two.
>>>>>
>>>
>>> I looked at the logs and here is what I found:
>>>
>>> Scenario 1:
>>> dmesg-faux bus-device bound-auto resume.log
>>> dmesg-platform bus-device bound-auto resume.log
>>>
>>> In this case suspend bailed out way before driver suspend
>>> when vhci_hcd is using platform and faux bus.
>>>
>>> Freezing remaining freezable tasks failed after 20.006 seconds (0
>>> tasks refusing to freeze, wq_busy=1)
>>> Restarting kernel threads ... done
>>> OOM killer enabled.
>>> Restarting tasks ... done.
>>> random: crng reseeded on system resumption
>>> PM: suspend exit
>>>
>>> Auto-resume of the user-space worked. Scenario 1 isn't really
>>> interesting.
>>>
>>> Scenario 2:
>>> dmesg-faux bus-device bound-black screen.log
>>> dmesg-platform bus-device bound-black screen.log
>>>
>>> Even though the result is the same in seeing blank screen, how
>>> we get there is different.
>>>
>>> =================
>>> faux-bus device:
>>> =================
>>> - suspend worked - looks like
>>> usb 4-1: PM: calling usb_dev_suspend @ 6054, parent: usb4
>>> usb 4-1: PM: usb_dev_suspend returned 0 after 13675 usecs
>>> usb usb4: PM: calling usb_dev_suspend @ 6055, parent: vhci_hcd.0
>>> usb usb4: PM: usb_dev_suspend returned 0 after 9 usecs
>>>
>>> vhci_hcd suspend isn't in play here. usb_dev_suspend() returns.
>>> See below
>>>
>>> usb 4-1: PM: usb_dev_suspend returned message.
>>>
>>> -------------------------------------------------------------
>>>
>>> - resume started (assuming it has been initiated by user)
>>>
>>> [ 650.027491][ T6056] pci 0000:00:01.0: PM: pci_pm_suspend_noirq
>>> returned 0 after 304 usecs
>>>
>>> See see timestamp difference between the last suspend message and the
>>> first resume message.
>>> [ 674.000257][ T39] pci 0000:00:00.0: PM: calling
>>> pci_pm_resume_noirq @ 39, parent: pci0000:00
>>>
>>> usb 4-1 usb_dev_resume never returns.
>>>
>>> [ 674.071125][ T6117] usb usb4: PM: usb_dev_resume returned 0 after
>>> 21110 usecs
>>> [ 674.113991][ T6126] usb 4-1: PM: calling usb_dev_resume @ 6126,
>>> parent: usb4
>>>
>>> -------------------------------------------------------------
>>>
>>> =====================
>>> platform bus device
>>> =====================
>>>
>>> - suspend was aborted because vhci_hcd suspend routine returned error
>>>
>>> [ 297.854621][ T9402] usb 4-1: PM: calling usb_dev_suspend @ 9402,
>>> parent: usb4
>>> [ 297.868524][ T9402] usb 4-1: PM: usb_dev_suspend returned 0 after
>>> 13214 usecs
>>> [ 297.869994][ T9403] usb usb4: PM: calling usb_dev_suspend @ 9403,
>>> parent: vhci_hcd.0
>>> [ 297.871959][ T9403] usb usb4: PM: usb_dev_suspend returned 0
>>> after 30 usecs
>>> [ 297.873644][ T9394] vhci_hcd vhci_hcd.0: PM: calling
>>> platform_pm_suspend @ 9394, parent: platform
>>> [ 297.874738][ T9394] vhci_hcd vhci_hcd.0: We have 1 active
>>> connection. Do not suspend.
>>> [ 297.875369][ T9394] vhci_hcd vhci_hcd.0: PM: dpm_run_callback():
>>> platform_pm_suspend returns -16
>>> [ 297.876078][ T9394] vhci_hcd vhci_hcd.0: PM: platform_pm_suspend
>>> returned -16 after 1341 usecs
>>> [ 297.876774][ T9394] vhci_hcd vhci_hcd.0: PM: failed to suspend:
>>> error -16
>>>
>>> - the following triggers resume
>>> [ 297.877321][ T9394] PM: Some devices failed to suspend, or early
>>> wake event detected
>>>
>>> [ 297.881065][ T9403] usb usb3: PM: usb_dev_resume returned 0 after
>>> 19 usecs
>>> [ 297.904551][ T9408] usb usb4: PM: usb_dev_resume returned 0 after
>>> 22911 usecs
>>> [ 297.905148][ T9418] usb 4-1: PM: calling usb_dev_resume @ 9418,
>>> parent: usb4
>>>
>>> usb 4-1 usb_dev_resume never returns.
>>>
>>> Note - In both cases, usb_dev_resume doesn't return. When it is
>>> called is the
>>> difference.
>>>
>>> I don't think suspend/resume works when devices are bound. Suspend
>>> exits and
>>> starts resume which seems to fail because it doesn't handle the
>>> virtual usb
>>> device resume. There is a missing piece here.
>>>
>>> When vhci_hcd imports a device and acts as a proxy between the
>>> virtual mass
>>> storage device (e.g in this case) - it appears suspend and resume are
>>> handled as if it is a usb device. Maybe this is incorrect?
>>>
>>> usb_dev_suspend() works and usb_dev_resume() on these virtual usb
>>> devices?
>>> Do we need to handle this in usb_dev_resume()?
>>>
>>> Talking out loud - I have to do some looking into.
>>>
>> Re:
>> Yes, your analysis is completely correct.
>>
>> In fact, I've experimented with adding PM hooks to the faux bus,
>> and found that faux bus devices then behave identically to platform
>> bus devices during suspend/resume.
>> See the attachment.
>>
>
> Thanks for checking this scenario. No surprises here.
Another part of my purpose in doing this is that the vhci-hcd driver seems
should still retain suspend/resume hooks. Therefore, the faux bus should
add corresponding hooks to allow the driver to call its own pm functions.
Though currently don't know how to fix this problem yet.
>
>> This is likely a historical legacy issue.
>
> It is an existing problem in the way vhci_hcd and the bound devices
> handle suspend/resume. vhci_hcd suspend assumes once it returns
> "don't suspend" the rest works. However suspend for the bound device
> runs first and a subsequent resume on it fails.
>
> This problem needs fixing - I don't know yet how to.
>
>> Regarding this matter, is there anything else you'd like me to assist
>> with?
>>
>
> One thing I am curious about is the status of the bound devices after
> "forced restart of the machine" when you see blank screen or hang?
That's an excellent question.Another persistent problem has surfaced
during troubleshooting：
After a USB/IP device is bound to the vhci-hcd driver, when the machine
reboots or performs a forced restart(include resume fail,forced restart),
attempting to re-bind the device to the vhci-hcd driver will generate
the error message
"usbip: error: Attach Request for 4-2 failed - Device busy (exported)".
At this point, the device must first be explicitly detached
from the usbip-host driver before it can be bound again.
I implemented a shutdown hook (can refer to the attached file) in the
vhci-hcd driver
to perform vhci_hcd remove before system shutdown.
This resolves failures where devices cannot rebind to
the vhci-hcd driver after normal reboots. However,
it remains ineffective for forced shutdown/reboot scenarios
since no shutdown functions are executed in such cases.
That's all details I've gathered yet.
>
> thanks,
> -- Shuah
On 7/9/25 03:07, Zongmin Zhou wrote:
>
> On 2025/7/9 02:16, Shuah Khan wrote:
>> On 7/3/25 00:04, Zongmin Zhou wrote:
>>>
>>> On 2025/7/3 07:54, Shuah Khan wrote:
>>>> On 7/1/25 20:12, Zongmin Zhou wrote:
>>>>>
>>>>> On 2025/7/2 06:56, Shuah Khan wrote:
>>>>>> On 6/23/25 21:21, Zongmin Zhou wrote:
>>>>>>>
>>>>>>> On 2025/6/21 01:26, Shuah Khan wrote:
>>>>>>>> On 6/20/25 03:27, Greg KH wrote:
>>>>>>>>> On Fri, Jun 20, 2025 at 05:19:33PM +0800, Zongmin Zhou wrote:
>>>>>>>>>>
>>>>>>>>>> On 2025/6/20 12:29, Greg KH wrote:
>>>>>>>>>>> On Fri, Jun 20, 2025 at 10:16:16AM +0800, Zongmin Zhou wrote:
>>>>>>>>>>>> On 2025/6/19 19:01, Greg KH wrote:
>>>>>>>>>>>>> On Wed, Jun 04, 2025 at 02:54:10PM +0800, Zongmin Zhou wrote:
>>>>>>>>>>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The vhci driver does not need to create a platform device,
>>>>>>>>>>>>>> it only did so because it was simple to do that in order to
>>>>>>>>>>>>>> get a place in sysfs to hang some device-specific attributes.
>>>>>>>>>>>>>> Now the faux device interface is more appropriate,change it
>>>>>>>>>>>>>> over to use the faux bus instead.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>> Changes in v2:
>>>>>>>>>>>>>> - don't change faux create api,just call probe on vhci_hcd_init.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> drivers/usb/usbip/vhci.h | 4 +-
>>>>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c | 86 +++++++++++-----------------
>>>>>>>>>>>>>> drivers/usb/usbip/vhci_sysfs.c | 68 +++++++++++-----------
>>>>>>>>>>>>>> tools/usb/usbip/libsrc/vhci_driver.h | 2 +-
>>>>>>>>>>>>>> 4 files changed, 72 insertions(+), 88 deletions(-)
>>>>>>>>>>>>> I get the following build errors from this patch:
>>>>>>>>>>>>>
>>>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1462:12: error: ‘vhci_hcd_resume’ defined but not used [-Werror=unused-function]
>>>>>>>>>>>>> 1462 | static int vhci_hcd_resume(struct faux_device *fdev)
>>>>>>>>>>>>> | ^~~~~~~~~~~~~~~
>>>>>>>>>>>>> drivers/usb/usbip/vhci_hcd.c:1418:12: error: ‘vhci_hcd_suspend’ defined but not used [-Werror=unused-function]
>>>>>>>>>>>>> 1418 | static int vhci_hcd_suspend(struct faux_device *fdev, pm_message_t state)
>>>>>>>>>>>>> | ^~~~~~~~~~~~~~~~
>>>>>>>>>>>>> cc1: all warnings being treated as errors
>>>>>>>>>>>>>
>>>>>>>>>>>>> Are you sure you tested this?
>>>>>>>>>>>> I apologize for not enabling -Werror, which resulted in missing this error
>>>>>>>>>>>> warning.
>>>>>>>>>>>> I have tested usbip feature use the new patch,but not test system
>>>>>>>>>>>> suspend/resume.
>>>>>>>>>>>> The faux bus type don't add pm function,and vhci-hcd driver can't register
>>>>>>>>>>>> it.
>>>>>>>>>>>> Maybe have to add suspend/resume for it.like below:
>>>>>>>>>>>> static const struct bus_type faux_bus_type = {
>>>>>>>>>>>> .name = "faux",
>>>>>>>>>>>> .match = faux_match,
>>>>>>>>>>>> .probe = faux_probe,
>>>>>>>>>>>> .remove = faux_remove,
>>>>>>>>>>>> .resume = faux_resume,
>>>>>>>>>>>> .suspend = faux_suspend,
>>>>>>>>>>>> };
>>>>>>>>>>>>
>>>>>>>>>>>> Is that right?
>>>>>>>>>>>> Your expertise would be greatly valued.
>>>>>>>>>>> As this is not real hardware, why do you need the suspend/resume
>>>>>>>>>>> callbacks at all? What is happening here that requires them?
>>>>>>>>>> @greg,
>>>>>>>>>> The vhci_hcd_suspend/vhci_hcd_resume interfaces are not designed for this
>>>>>>>>>> faux device, but rather to
>>>>>>>>>> manipulate the HCD_FLAG_HW_ACCESSIBLE bit in the hcd flags associated with
>>>>>>>>>> the faux device.
>>>>>>>>>> For example:
>>>>>>>>>> During system standby: clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>>>>>>> During system wakeup: set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)
>>>>>>>>>>
>>>>>>>>>> Previously, these two functions were registered through platform_driver,
>>>>>>>>>> but faux bus does not have the relevant interface, so they were not called,
>>>>>>>>>> resulting in this compilation warning error.
>>>>>>>>>>
>>>>>>>>>> This raises the question: Should the faux bus implement PM-related
>>>>>>>>>> interface?
>>>>>>>>>> I'm uncertain whether these functions are essential for the vhci-hcd driver
>>>>>>>>>> or if they can be safely removed.
>>>>>>>>>>
>>>>>>>>>> However, during system standby/wakeup tests with remote USB devices bound to
>>>>>>>>>> the vhci-hcd driver,
>>>>>>>>>> I observed consistent failure scenarios across both the original platform
>>>>>>>>>> bus and faux bus patch implementations.
>>>>>>>>
>>>>>>>> suspend and resume hooks have been in the code from beginning
>>>>>>>> in the CONFIG_PM path. The original authors are skeptical about
>>>>>>>> what should happen during suspend"
>>>>>>>>
>>>>>>>> /* what should happen for USB/IP under suspend/resume? */
>>>>>>>> suspend hook checks for active connections and sends EBBUSY
>>>>>>>> back to pm core.
>>>>>>>>
>>>>>>>> Active connection means any of the ports are in USB_PORT_STAT_CONNECTION
>>>>>>>> state. So as long as there are active connections between vhci client
>>>>>>>> and the host, suspend won't work. So we really don't know what happens
>>>>>>>> to the active connections if there are no suspend/resume hooks.
>>>>>>>>
>>>>>>>> If there are no active connections, then it will clear the HCD_FLAG_HW_ACCESSIBLE
>>>>>>>> bit and returns to pm core to continue with suspend.
>>>>>>>>
>>>>>>>> resume sets the HCD_FLAG_HW_ACCESSIBLE flag to indicate hardware is accessible
>>>>>>>> and initiates port status poll.
>>>>>>>>
>>>>>>>> - suspend hook prevents suspend
>>>>>>>>
>>>>>>>> With faux device since there is no suspend and resume hook, I would expect
>>>>>>>> these hooks are not a factor in suspend and resume.
>>>>>>>>
>>>>>>>> vhci_hcd is a special case virtual driver as it is a proxy for controlling
>>>>>>>> hardware on the host.
>>>>>>>>
>>>>>>>> Zongmin,
>>>>>>>>
>>>>>>>> Do suspend/resume work when vhci_hcd is not loaded?
>>>>>>>> What happens when when system suspends and resumes with faux device?
>>>>>>>> Can you send me dmesg logs and pm logs?
>>>>>>>>
>>>>>>> Hi Greg,shuah
>>>>>>>
>>>>>>> Yes, system with vhci_hcd unload or just load vhci_hcd driver
>>>>>>> without usbip devices bound to vhci_hcd,system suspend/resume worked well.
>>>>>>> Some logs for you.
>>>>>>
>>>>>> Sorry for the delay. I was at a conference last week.
>>>>>> Thank you for sending these logs.
>>>>>>
>>>>>>> 1.system with vhci_hcd driver load,but don't bound any usbip devices to vhci_hcd,suspend/resume worked well.
>>>>>>> see dmesg-faux bus-load.log
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> 2.usbip device bound to vhci_hcd,and do system suspend action,suspend/resume worked failed.
>>>>>>> I observed two distinct failure scenario:
>>>>>>> Scenario 1: System failed to enter suspend state,and will auto resume;
>>>>>>
>>>>>>
>>>>>>> the log for use platform bus:
>>>>>>> dmesg-platform bus-device bound-auto resume.log
>>>>>>
>>>>>> This is clear from the suspend hook in the vhci_hcd.
>>>>>> When it returns EBUSY, suspend will fail and move to
>>>>>> auto resume.
>>>>>>
>>>>>>> the log for use faux bus:
>>>>>>> dmesg-faux bus-device bound-auto resume.log
>>>>>>
>>>>>> It is good that the behavior is same with faux bus even though
>>>>>> suspend hook isn't called. I will take a look at the log.
>>>>>>
>>>>>>>
>>>>>>> Scenario 2: System resume failed with black screen freeze,a forced restart of the machine is require.
>>>>>>> the log for use platform bus:
>>>>>>> dmesg-platform bus-device bound-black screen.log
>>>>>>> the log for use faux bus:
>>>>>>> dmesg-faux bus-device bound-black screen.log
>>>>>>
>>>>>> That is bad. When does this happen? Is there a difference in
>>>>>> configuration?
>>>>>>
>>>>> No, there's no difference. The same code is used for two different failure scenarios.
>>>>> I just tested many times. These two different situations occur probabilistically.
>>>>> But they both happened only when the USBIP device is bound to the vhci_hcd driver.
>>>>>> The behavior same for platform bus and faux bus. Sounds like
>>>>>> an existing problem that needs to be debugged separately.
>>>>>>
>>>>>> I will take a look at all these logs and get back to you in
>>>>>> a day or two.
>>>>>>
>>>>
>>>> I looked at the logs and here is what I found:
>>>>
>>>> Scenario 1:
>>>> dmesg-faux bus-device bound-auto resume.log
>>>> dmesg-platform bus-device bound-auto resume.log
>>>>
>>>> In this case suspend bailed out way before driver suspend
>>>> when vhci_hcd is using platform and faux bus.
>>>>
>>>> Freezing remaining freezable tasks failed after 20.006 seconds (0 tasks refusing to freeze, wq_busy=1)
>>>> Restarting kernel threads ... done
>>>> OOM killer enabled.
>>>> Restarting tasks ... done.
>>>> random: crng reseeded on system resumption
>>>> PM: suspend exit
>>>>
>>>> Auto-resume of the user-space worked. Scenario 1 isn't really
>>>> interesting.
>>>>
>>>> Scenario 2:
>>>> dmesg-faux bus-device bound-black screen.log
>>>> dmesg-platform bus-device bound-black screen.log
>>>>
>>>> Even though the result is the same in seeing blank screen, how
>>>> we get there is different.
>>>>
>>>> =================
>>>> faux-bus device:
>>>> =================
>>>> - suspend worked - looks like
>>>> usb 4-1: PM: calling usb_dev_suspend @ 6054, parent: usb4
>>>> usb 4-1: PM: usb_dev_suspend returned 0 after 13675 usecs
>>>> usb usb4: PM: calling usb_dev_suspend @ 6055, parent: vhci_hcd.0
>>>> usb usb4: PM: usb_dev_suspend returned 0 after 9 usecs
>>>>
>>>> vhci_hcd suspend isn't in play here. usb_dev_suspend() returns.
>>>> See below
>>>>
>>>> usb 4-1: PM: usb_dev_suspend returned message.
>>>>
>>>> -------------------------------------------------------------
>>>>
>>>> - resume started (assuming it has been initiated by user)
>>>>
>>>> [ 650.027491][ T6056] pci 0000:00:01.0: PM: pci_pm_suspend_noirq returned 0 after 304 usecs
>>>>
>>>> See see timestamp difference between the last suspend message and the
>>>> first resume message.
>>>> [ 674.000257][ T39] pci 0000:00:00.0: PM: calling pci_pm_resume_noirq @ 39, parent: pci0000:00
>>>>
>>>> usb 4-1 usb_dev_resume never returns.
>>>>
>>>> [ 674.071125][ T6117] usb usb4: PM: usb_dev_resume returned 0 after 21110 usecs
>>>> [ 674.113991][ T6126] usb 4-1: PM: calling usb_dev_resume @ 6126, parent: usb4
>>>>
>>>> -------------------------------------------------------------
>>>>
>>>> =====================
>>>> platform bus device
>>>> =====================
>>>>
>>>> - suspend was aborted because vhci_hcd suspend routine returned error
>>>>
>>>> [ 297.854621][ T9402] usb 4-1: PM: calling usb_dev_suspend @ 9402, parent: usb4
>>>> [ 297.868524][ T9402] usb 4-1: PM: usb_dev_suspend returned 0 after 13214 usecs
>>>> [ 297.869994][ T9403] usb usb4: PM: calling usb_dev_suspend @ 9403, parent: vhci_hcd.0
>>>> [ 297.871959][ T9403] usb usb4: PM: usb_dev_suspend returned 0 after 30 usecs
>>>> [ 297.873644][ T9394] vhci_hcd vhci_hcd.0: PM: calling platform_pm_suspend @ 9394, parent: platform
>>>> [ 297.874738][ T9394] vhci_hcd vhci_hcd.0: We have 1 active connection. Do not suspend.
>>>> [ 297.875369][ T9394] vhci_hcd vhci_hcd.0: PM: dpm_run_callback(): platform_pm_suspend returns -16
>>>> [ 297.876078][ T9394] vhci_hcd vhci_hcd.0: PM: platform_pm_suspend returned -16 after 1341 usecs
>>>> [ 297.876774][ T9394] vhci_hcd vhci_hcd.0: PM: failed to suspend: error -16
>>>>
>>>> - the following triggers resume
>>>> [ 297.877321][ T9394] PM: Some devices failed to suspend, or early wake event detected
>>>>
>>>> [ 297.881065][ T9403] usb usb3: PM: usb_dev_resume returned 0 after 19 usecs
>>>> [ 297.904551][ T9408] usb usb4: PM: usb_dev_resume returned 0 after 22911 usecs
>>>> [ 297.905148][ T9418] usb 4-1: PM: calling usb_dev_resume @ 9418, parent: usb4
>>>>
>>>> usb 4-1 usb_dev_resume never returns.
>>>>
>>>> Note - In both cases, usb_dev_resume doesn't return. When it is called is the
>>>> difference.
>>>>
>>>> I don't think suspend/resume works when devices are bound. Suspend exits and
>>>> starts resume which seems to fail because it doesn't handle the virtual usb
>>>> device resume. There is a missing piece here.
>>>>
>>>> When vhci_hcd imports a device and acts as a proxy between the virtual mass
>>>> storage device (e.g in this case) - it appears suspend and resume are
>>>> handled as if it is a usb device. Maybe this is incorrect?
>>>>
>>>> usb_dev_suspend() works and usb_dev_resume() on these virtual usb devices?
>>>> Do we need to handle this in usb_dev_resume()?
>>>>
>>>> Talking out loud - I have to do some looking into.
>>>>
>>> Re:
>>> Yes, your analysis is completely correct.
>>>
>>> In fact, I've experimented with adding PM hooks to the faux bus,
>>> and found that faux bus devices then behave identically to platform bus devices during suspend/resume.
>>> See the attachment.
>>>
>>
>> Thanks for checking this scenario. No surprises here.
> Another part of my purpose in doing this is that the vhci-hcd driver seems
> should still retain suspend/resume hooks. Therefore, the faux bus should
> add corresponding hooks to allow the driver to call its own pm functions.
> Though currently don't know how to fix this problem yet.
>>
>>> This is likely a historical legacy issue.
>>
>> It is an existing problem in the way vhci_hcd and the bound devices
>> handle suspend/resume. vhci_hcd suspend assumes once it returns
>> "don't suspend" the rest works. However suspend for the bound device
>> runs first and a subsequent resume on it fails.
>>
>> This problem needs fixing - I don't know yet how to.
>>
>>> Regarding this matter, is there anything else you'd like me to assist with?
>>>
>>
>> One thing I am curious about is the status of the bound devices after
>> "forced restart of the machine" when you see blank screen or hang?
> That's an excellent question.Another persistent problem has surfaced during troubleshooting：
> After a USB/IP device is bound to the vhci-hcd driver, when the machine
> reboots or performs a forced restart(include resume fail,forced restart),
> attempting to re-bind the device to the vhci-hcd driver will generate the error message
> "usbip: error: Attach Request for 4-2 failed - Device busy (exported)".
> At this point, the device must first be explicitly detached
> from the usbip-host driver before it can be bound again.
The error is coming from the usbip host (server) which would have
stayed up during client side reboot.
It requires an extra detach step before the attach. vhci client
doesn't do detach on it own. Attach and detach are user initiated.
So the current behavior is inline with that.
I don't dislike the shutdown use which essentially mimics
a series of "detach" commands from the user-space. However the
solution that works in conjunction with the usbip host side.
usbip_host - stub driver doesn't implement suspend/resume.
> I implemented a shutdown hook (can refer to the attached file) in the vhci-hcd driver
> to perform vhci_hcd remove before system shutdown.
> This resolves failures where devices cannot rebind to
> the vhci-hcd driver after normal reboots. However,
> it remains ineffective for forced shutdown/reboot scenarios
> since no shutdown functions are executed in such cases.
The problem here is that current vhci_suspend/resume are ineffective
usb core initiates usb_dev_suspend on the virtual device.
The solution could be to disable auto-suspend on virtual devices.
This would eliminate the suspend and resume hanging issue.
There is the server side issue - what happens when usbip host
suspends? We have a few components here to think about.
usbipd (user-space)
vhci_hcd and the usb devices it creates
usbip_host, stub driver that proxies between the device
on the server and vhci_client.
PM can be complex and it has to do lot more than it currently
does on both server and client end to support seamlessly.
thanks,
-- Shuah
On Wed, Jul 09, 2025 at 05:07:24PM +0800, Zongmin Zhou wrote: > > > In fact, I've experimented with adding PM hooks to the faux bus, > > > and found that faux bus devices then behave identically to platform > > > bus devices during suspend/resume. > > > See the attachment. > > > > > > > Thanks for checking this scenario. No surprises here. > Another part of my purpose in doing this is that the vhci-hcd driver seems > should still retain suspend/resume hooks. Therefore, the faux bus should > add corresponding hooks to allow the driver to call its own pm functions. > Though currently don't know how to fix this problem yet. I have no problem with adding the pm functions to the faux bus, BUT it needs to make sense as to why they would be needed at all as this is not a "real" device or bus that should need to do anything when suspend/resume happens. thanks, greg k-h
On Wed, Jul 09, 2025 at 12:06:57PM +0200, Greg KH wrote: > On Wed, Jul 09, 2025 at 05:07:24PM +0800, Zongmin Zhou wrote: > > > > In fact, I've experimented with adding PM hooks to the faux bus, > > > > and found that faux bus devices then behave identically to platform > > > > bus devices during suspend/resume. > > > > See the attachment. > > > > > > > > > > Thanks for checking this scenario. No surprises here. > > Another part of my purpose in doing this is that the vhci-hcd driver seems > > should still retain suspend/resume hooks. Therefore, the faux bus should > > add corresponding hooks to allow the driver to call its own pm functions. > > Though currently don't know how to fix this problem yet. > > I have no problem with adding the pm functions to the faux bus, BUT it > needs to make sense as to why they would be needed at all as this is not > a "real" device or bus that should need to do anything when > suspend/resume happens. The unique problem faced by vhci-hcd is that the devices it controls reside on external computer systems that have a lot of their own state, much more than ordinay USB devices have. Consequently vhci-hcd may need to do more work for a PM transition than a normal driver would. As an analogy, suppose you're running a program that has an open TCP connection to an external server. If you suspend your computer, it won't be able to send the TCP keepalive packets that the server expects, and the server will eventually close the connection. Then when your computer resumes, your program may misbehave when it finds its connection has spontaneously been closed for no apparent reason. Alan Stern
On 7/9/25 08:20, Alan Stern wrote: > On Wed, Jul 09, 2025 at 12:06:57PM +0200, Greg KH wrote: >> On Wed, Jul 09, 2025 at 05:07:24PM +0800, Zongmin Zhou wrote: >>>>> In fact, I've experimented with adding PM hooks to the faux bus, >>>>> and found that faux bus devices then behave identically to platform >>>>> bus devices during suspend/resume. >>>>> See the attachment. >>>>> >>>> >>>> Thanks for checking this scenario. No surprises here. >>> Another part of my purpose in doing this is that the vhci-hcd driver seems >>> should still retain suspend/resume hooks. Therefore, the faux bus should >>> add corresponding hooks to allow the driver to call its own pm functions. >>> Though currently don't know how to fix this problem yet. >> >> I have no problem with adding the pm functions to the faux bus, BUT it >> needs to make sense as to why they would be needed at all as this is not >> a "real" device or bus that should need to do anything when >> suspend/resume happens. > > The unique problem faced by vhci-hcd is that the devices it controls > reside on external computer systems that have a lot of their own state, > much more than ordinay USB devices have. Consequently vhci-hcd may need > to do more work for a PM transition than a normal driver would. > > As an analogy, suppose you're running a program that has an open TCP > connection to an external server. If you suspend your computer, it > won't be able to send the TCP keepalive packets that the server expects, > and the server will eventually close the connection. Then when your > computer resumes, your program may misbehave when it finds its > connection has spontaneously been closed for no apparent reason. > Right. We have a few too many moving pieces here: usbipd (user-space) vhci_hcd and the usb devices it creates usbip_host, stub driver that proxies between the device on the server and vhci_client. PM can be complex and it has to do lot more than it currently does on both server and client end to support seamlessly. The current suspend took the approach of refusing suspend which doesn't work since usb devices underneath hang in usb_dev_resume(). Looks like this usb device is treated like a real device bu usb core. Is there a way to have usb core PM (suspend and resume) handle them as virtual? Would it help to use "supports_autosuspend" to disable suspend and resume? This would solve the hang during usb_dev_resume() problem. Maybe vhci_hcd isn't a good candidate for faux bus? It appears we might have a need for a shutdown, suspend at the very least to be able to support reboot/suspend/resume cases? The current code doesn't handle suspend/resume correctly when devices are imported. thanks, -- Shuah
On 7/9/25 15:49, Shuah Khan wrote: > On 7/9/25 08:20, Alan Stern wrote: >> On Wed, Jul 09, 2025 at 12:06:57PM +0200, Greg KH wrote: >>> On Wed, Jul 09, 2025 at 05:07:24PM +0800, Zongmin Zhou wrote: >>>>>> In fact, I've experimented with adding PM hooks to the faux bus, >>>>>> and found that faux bus devices then behave identically to platform >>>>>> bus devices during suspend/resume. >>>>>> See the attachment. >>>>>> >>>>> >>>>> Thanks for checking this scenario. No surprises here. >>>> Another part of my purpose in doing this is that the vhci-hcd driver seems >>>> should still retain suspend/resume hooks. Therefore, the faux bus should >>>> add corresponding hooks to allow the driver to call its own pm functions. >>>> Though currently don't know how to fix this problem yet. >>> >>> I have no problem with adding the pm functions to the faux bus, BUT it >>> needs to make sense as to why they would be needed at all as this is not >>> a "real" device or bus that should need to do anything when >>> suspend/resume happens. >> >> The unique problem faced by vhci-hcd is that the devices it controls >> reside on external computer systems that have a lot of their own state, >> much more than ordinay USB devices have. Consequently vhci-hcd may need >> to do more work for a PM transition than a normal driver would. >> >> As an analogy, suppose you're running a program that has an open TCP >> connection to an external server. If you suspend your computer, it >> won't be able to send the TCP keepalive packets that the server expects, >> and the server will eventually close the connection. Then when your >> computer resumes, your program may misbehave when it finds its >> connection has spontaneously been closed for no apparent reason. >> > > Right. We have a few too many moving pieces here: > > usbipd (user-space) > vhci_hcd and the usb devices it creates > > usbip_host, stub driver that proxies between the device > on the server and vhci_client. > > PM can be complex and it has to do lot more than it currently > does on both server and client end to support seamlessly. > > The current suspend took the approach of refusing suspend > which doesn't work since usb devices underneath hang in > usb_dev_resume(). Looks like this usb device is treated like > a real device bu usb core. Is there a way to have usb core > PM (suspend and resume) handle them as virtual? Would it > help to use "supports_autosuspend" to disable suspend and > resume? Would it work if usb_disable_autosuspend() on the devices that hang off its vitual bus? thanks, -- Shuah
On Wed, Jul 09, 2025 at 03:57:35PM -0600, Shuah Khan wrote: > On 7/9/25 15:49, Shuah Khan wrote: > > Right. We have a few too many moving pieces here: > > > > usbipd (user-space) > > vhci_hcd and the usb devices it creates > > > > usbip_host, stub driver that proxies between the device > > on the server and vhci_client. > > > > PM can be complex and it has to do lot more than it currently > > does on both server and client end to support seamlessly. > > > > The current suspend took the approach of refusing suspend > > which doesn't work since usb devices underneath hang in > > usb_dev_resume(). Looks like this usb device is treated like > > a real device bu usb core. Is there a way to have usb core > > PM (suspend and resume) handle them as virtual? Would it > > help to use "supports_autosuspend" to disable suspend and > > resume? > > Would it work if usb_disable_autosuspend() on the devices > that hang off its vitual bus? You have to consider PM on both the host and client. And you have to consider both runtime PM and system PM (that is, suspend to RAM, hibernate, etc.). On the server, any sort of suspend will interrupt the connection. usb_disable_autosuspend() will prevent runtime suspends, but you shouldn't try to prevent system suspends. Instead, the usbip driver on the server should have its suspend routine close the connection to the client (rather as if the server's user had unplugged the device). On the client, you've got a choice for how to handle runtime PM. You can leave it enabled, and when the client decides to suspend the device, tell the server's driver. The server driver can then attempt to do a runtime suspend on the physical device. (You might need to add a new type of message to the USBIP protocol to accomplish this; I don't know the details.) Alternatively, you can forbid runtime suspend on the client entirely, although it would be nice if you could avoid this. System PM on the client can be handled more less the same as runtime PM. Alan Stern
On 7/10/25 08:06, Alan Stern wrote: > On Wed, Jul 09, 2025 at 03:57:35PM -0600, Shuah Khan wrote: >> On 7/9/25 15:49, Shuah Khan wrote: >>> Right. We have a few too many moving pieces here: >>> >>> usbipd (user-space) >>> vhci_hcd and the usb devices it creates >>> >>> usbip_host, stub driver that proxies between the device >>> on the server and vhci_client. >>> >>> PM can be complex and it has to do lot more than it currently >>> does on both server and client end to support seamlessly. >>> >>> The current suspend took the approach of refusing suspend >>> which doesn't work since usb devices underneath hang in >>> usb_dev_resume(). Looks like this usb device is treated like >>> a real device bu usb core. Is there a way to have usb core >>> PM (suspend and resume) handle them as virtual? Would it >>> help to use "supports_autosuspend" to disable suspend and >>> resume? >> >> Would it work if usb_disable_autosuspend() on the devices >> that hang off its vitual bus? > > You have to consider PM on both the host and client. And you have to > consider both runtime PM and system PM (that is, suspend to RAM, > hibernate, etc.). This would be as a fix for the existing suspend hang issue. > > On the server, any sort of suspend will interrupt the connection. > usb_disable_autosuspend() will prevent runtime suspends, but you > shouldn't try to prevent system suspends. Instead, the usbip driver on > the server should have its suspend routine close the connection to the > client (rather as if the server's user had unplugged the device). > > On the client, you've got a choice for how to handle runtime PM. You > can leave it enabled, and when the client decides to suspend the device, > tell the server's driver. The server driver can then attempt to do a > runtime suspend on the physical device. (You might need to add a new > type of message to the USBIP protocol to accomplish this; I don't know > the details.) Alternatively, you can forbid runtime suspend on the > client entirely, although it would be nice if you could avoid this. > > System PM on the client can be handled more less the same as runtime PM. Correct. This has to be a complete solution that syncs up server and client side. I am going to look into implementing this - it might be possible to do this in user-space. Either way this will require changes to the protocol very likely. Greg, Zongmin Zhou, let's hold off on this conversion yo faux bus for now. I will spend time looking at if we can find PM solution that works end to end for server and client. thanks, -- Shuah
On 2025/7/11 04:33, Shuah Khan wrote: > On 7/10/25 08:06, Alan Stern wrote: >> On Wed, Jul 09, 2025 at 03:57:35PM -0600, Shuah Khan wrote: >>> On 7/9/25 15:49, Shuah Khan wrote: >>>> Right. We have a few too many moving pieces here: >>>> >>>> usbipd (user-space) >>>> vhci_hcd and the usb devices it creates >>>> >>>> usbip_host, stub driver that proxies between the device >>>> on the server and vhci_client. >>>> >>>> PM can be complex and it has to do lot more than it currently >>>> does on both server and client end to support seamlessly. >>>> >>>> The current suspend took the approach of refusing suspend >>>> which doesn't work since usb devices underneath hang in >>>> usb_dev_resume(). Looks like this usb device is treated like >>>> a real device bu usb core. Is there a way to have usb core >>>> PM (suspend and resume) handle them as virtual? Would it >>>> help to use "supports_autosuspend" to disable suspend and >>>> resume? >>> >>> Would it work if usb_disable_autosuspend() on the devices >>> that hang off its vitual bus? >> >> You have to consider PM on both the host and client. And you have to >> consider both runtime PM and system PM (that is, suspend to RAM, >> hibernate, etc.). > > This would be as a fix for the existing suspend hang issue. > >> >> On the server, any sort of suspend will interrupt the connection. >> usb_disable_autosuspend() will prevent runtime suspends, but you >> shouldn't try to prevent system suspends. Instead, the usbip driver on >> the server should have its suspend routine close the connection to the >> client (rather as if the server's user had unplugged the device). >> >> On the client, you've got a choice for how to handle runtime PM. You >> can leave it enabled, and when the client decides to suspend the device, >> tell the server's driver. The server driver can then attempt to do a >> runtime suspend on the physical device. (You might need to add a new >> type of message to the USBIP protocol to accomplish this; I don't know >> the details.) Alternatively, you can forbid runtime suspend on the >> client entirely, although it would be nice if you could avoid this. >> >> System PM on the client can be handled more less the same as runtime PM. > > Correct. This has to be a complete solution that syncs up server and > client > side. I am going to look into implementing this - it might be possible to > do this in user-space. Either way this will require changes to the > protocol > very likely. > > Greg, Zongmin Zhou, let's hold off on this conversion yo faux bus for > now. > I will spend time looking at if we can find PM solution that works end > to end > for server and client. Sorry for the late reply. Ok,I got it. it is necessary to tackle the PM standby issue first before considering next steps. > > thanks, > -- Shuah
On Thu, Jul 10, 2025 at 02:33:42PM -0600, Shuah Khan wrote: > On 7/10/25 08:06, Alan Stern wrote: > > On Wed, Jul 09, 2025 at 03:57:35PM -0600, Shuah Khan wrote: > > > On 7/9/25 15:49, Shuah Khan wrote: > > > > Right. We have a few too many moving pieces here: > > > > > > > > usbipd (user-space) > > > > vhci_hcd and the usb devices it creates > > > > > > > > usbip_host, stub driver that proxies between the device > > > > on the server and vhci_client. > > > > > > > > PM can be complex and it has to do lot more than it currently > > > > does on both server and client end to support seamlessly. > > > > > > > > The current suspend took the approach of refusing suspend > > > > which doesn't work since usb devices underneath hang in > > > > usb_dev_resume(). Looks like this usb device is treated like > > > > a real device bu usb core. Is there a way to have usb core > > > > PM (suspend and resume) handle them as virtual? Would it > > > > help to use "supports_autosuspend" to disable suspend and > > > > resume? > > > > > > Would it work if usb_disable_autosuspend() on the devices > > > that hang off its vitual bus? > > > > You have to consider PM on both the host and client. And you have to > > consider both runtime PM and system PM (that is, suspend to RAM, > > hibernate, etc.). > > This would be as a fix for the existing suspend hang issue. > > > > > On the server, any sort of suspend will interrupt the connection. > > usb_disable_autosuspend() will prevent runtime suspends, but you > > shouldn't try to prevent system suspends. Instead, the usbip driver on > > the server should have its suspend routine close the connection to the > > client (rather as if the server's user had unplugged the device). > > > > On the client, you've got a choice for how to handle runtime PM. You > > can leave it enabled, and when the client decides to suspend the device, > > tell the server's driver. The server driver can then attempt to do a > > runtime suspend on the physical device. (You might need to add a new > > type of message to the USBIP protocol to accomplish this; I don't know > > the details.) Alternatively, you can forbid runtime suspend on the > > client entirely, although it would be nice if you could avoid this. > > > > System PM on the client can be handled more less the same as runtime PM. > > Correct. This has to be a complete solution that syncs up server and client > side. I am going to look into implementing this - it might be possible to > do this in user-space. Either way this will require changes to the protocol > very likely. > > Greg, Zongmin Zhou, let's hold off on this conversion yo faux bus for now. > I will spend time looking at if we can find PM solution that works end to end > for server and client. Ok, thanks!
On 6/4/25 00:54, Zongmin Zhou wrote: > From: Zongmin Zhou <zhouzongmin@kylinos.cn> > > The vhci driver does not need to create a platform device, > it only did so because it was simple to do that in order to > get a place in sysfs to hang some device-specific attributes. > Now the faux device interface is more appropriate,change it > over to use the faux bus instead. > > Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn> > --- > Changes in v2: > - don't change faux create api,just call probe on vhci_hcd_init. I will defer the review to Greg on this. I am fine with the change if Greg is happy with it. :) Acked-by: Shuah Khan <skhan@linuxfoundation.org> thanks, -- Shuah
On Tue, Jun 10, 2025 at 09:15:51AM -0600, Shuah Khan wrote: > On 6/4/25 00:54, Zongmin Zhou wrote: > > From: Zongmin Zhou <zhouzongmin@kylinos.cn> > > > > The vhci driver does not need to create a platform device, > > it only did so because it was simple to do that in order to > > get a place in sysfs to hang some device-specific attributes. > > Now the faux device interface is more appropriate,change it > > over to use the faux bus instead. > > > > Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn> > > --- > > Changes in v2: > > - don't change faux create api,just call probe on vhci_hcd_init. > > I will defer the review to Greg on this. > > I am fine with the change if Greg is happy with it. :) Well the build errors aren't that good, but overall, yes, it's a nice change :) thanks, greg k-h
© 2016 - 2025 Red Hat, Inc.