Message-ID: <48ab511e-2847-4daa-98de-a234b8584b78@163.com>
Date: Tue, 24 Jun 2025 11:21:28 +0800
From: Zongmin Zhou <min_halo@....com>
To: Shuah Khan <skhan@...uxfoundation.org>,
 Greg KH <gregkh@...uxfoundation.org>
Cc: shuah@...nel.org, valentina.manea.m@...il.com, i@...ithal.me,
 linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
 zhouzongmin@...inos.cn
Subject: Re: [PATCH v2] usbip: convert to use faux_device


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@...inos.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@...inos.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
>
>
>
View attachment "dmesg-faux bus-device bound-auto
 resume.log" of type "text/x-log" (180277 bytes)

View attachment "dmesg-faux bus-device bound-black
 screen.log" of type "text/x-log" (222208 bytes)

View attachment "dmesg-faux bus-load.log" of type "text/x-log" (215679 bytes)

View attachment "dmesg-platform bus-device
 bound-auto resume.log" of type "text/x-log" (175540 bytes)

View attachment "dmesg-platform bus-device
 bound-black screen.log" of type "text/x-log" (178300 bytes)

Powered by blists - more mailing lists