The USB/IP Virtual Host Controller (VHCI) platform driver is expected to
prevent entering system suspend when at least one remote device is
attached to the virtual USB root hub.

However, in some cases, the detection logic for active USB/IP
connections doesn't seem to work reliably, e.g. when all devices
attached to the virtual hub have been already suspended.  This will
normally lead to a broken suspend state, with unrecoverable resume.

The first patch of the series provides a workaround to ensure the
virtually attached devices do not enter suspend.  Note this is currently
limited to the client side (vhci_hcd) only, since the server side
(usbip_host) doesn't implement system suspend prevention.

Additionally, during the investigation I noticed and fixed a bunch of
coding style issues, hence the subsequent patches contain all the
changes needed to make checkpatch happy for the entire driver.

IMPORTANT:

Please note commit aa7a9275ab81 ("PM: sleep: Suspend async parents after
suspending children") from v6.16-rc1 introduced a regression which
breaks the suspend cancellation and hangs the system.

A fix [1] has been already provided, which also landed soon after in
v6.16-rc7 under commit ebd6884167ea ("PM: sleep: Update power.completion
for all devices on errors").

[1] https://lore.kernel.org/all/6191258.lOV4Wx5bFT@rjwysocki.net/

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v2:
- Updated cover letter to indicate the PM core fix has landed in
  v6.16-rc7
- Also made it clear that the patch fixing up suspend prevention only
  applies to the client side (vhci_hcd), since the server side
  (usbip_host) doesn't implement this functionality
- Documented the usage of dev_pm_syscore_device() in vhci_urb_enqueue()
- Reworked most of the cleanup patches according to the feedback
  received from Greg
- Link to v1: https://lore.kernel.org/r/20250717-vhci-hcd-suspend-fix-v1-0-2b000cd05952@collabora.com

---
Cristian Ciocaltea (18):
      usb: vhci-hcd: Prevent suspending virtually attached devices
      usb: vhci-hcd: Ensure lines do not end with '('
      usb: vhci-hcd: Consistently use the braces
      usb: vhci-hcd: Avoid unnecessary use of braces
      usb: vhci-hcd: Consistently use blank lines
      usb: vhci-hcd: Drop spaces after casts
      usb: vhci-hcd: Add spaces around operators
      usb: vhci-hcd: Drop unnecessary parentheses
      usb: vhci-hcd: Fix open parenthesis alignment
      usb: vhci-hcd: Simplify NULL comparison
      usb: vhci-hcd: Simplify kzalloc usage
      usb: vhci-hcd: Use the paranthesized form of sizeof
      usb: vhci-hcd: Fix block comments
      usb: vhci-hcd: Remove ftrace-like logging
      usb: vhci-hcd: Consistently use __func__
      usb: vhci-hcd: Do not split quoted strings
      usb: vhci-hcd: Switch to dev_err_probe() in probe path
      usb: vhci-hcd: Replace pr_*() with dev_*() logging

 drivers/usb/usbip/vhci_hcd.c | 274 +++++++++++++++++++++++--------------------
 1 file changed, 146 insertions(+), 128 deletions(-)
---
base-commit: 024e09e444bd2b06aee9d1f3fe7b313c7a2df1bb
change-id: 20250714-vhci-hcd-suspend-fix-7db5c25c509d

On Sat, Jul 26, 2025 at 01:08:02AM +0300, Cristian Ciocaltea wrote:
> The USB/IP Virtual Host Controller (VHCI) platform driver is expected to
> prevent entering system suspend when at least one remote device is
> attached to the virtual USB root hub.
> 
> However, in some cases, the detection logic for active USB/IP
> connections doesn't seem to work reliably, e.g. when all devices
> attached to the virtual hub have been already suspended.  This will
> normally lead to a broken suspend state, with unrecoverable resume.
> 
> The first patch of the series provides a workaround to ensure the
> virtually attached devices do not enter suspend.  Note this is currently
> limited to the client side (vhci_hcd) only, since the server side
> (usbip_host) doesn't implement system suspend prevention.
> 
> Additionally, during the investigation I noticed and fixed a bunch of
> coding style issues, hence the subsequent patches contain all the
> changes needed to make checkpatch happy for the entire driver.

You are doing two major things here, fixing suspend, and cleaning up
checkpatch issues.  Please make that two different patch sets as those
are not logical things to put together at all.  Work on the suspend
issue first, and after that is all done and working, then consider
checkpatch cleanups, those are not that important overall :)

thanks,

greg k-h

Hi Greg,

On 7/26/25 9:43 AM, Greg Kroah-Hartman wrote:
> On Sat, Jul 26, 2025 at 01:08:02AM +0300, Cristian Ciocaltea wrote:
>> The USB/IP Virtual Host Controller (VHCI) platform driver is expected to
>> prevent entering system suspend when at least one remote device is
>> attached to the virtual USB root hub.
>>
>> However, in some cases, the detection logic for active USB/IP
>> connections doesn't seem to work reliably, e.g. when all devices
>> attached to the virtual hub have been already suspended.  This will
>> normally lead to a broken suspend state, with unrecoverable resume.
>>
>> The first patch of the series provides a workaround to ensure the
>> virtually attached devices do not enter suspend.  Note this is currently
>> limited to the client side (vhci_hcd) only, since the server side
>> (usbip_host) doesn't implement system suspend prevention.
>>
>> Additionally, during the investigation I noticed and fixed a bunch of
>> coding style issues, hence the subsequent patches contain all the
>> changes needed to make checkpatch happy for the entire driver.
> 
> You are doing two major things here, fixing suspend, and cleaning up
> checkpatch issues.  Please make that two different patch sets as those
> are not logical things to put together at all.  Work on the suspend
> issue first, and after that is all done and working, then consider
> checkpatch cleanups, those are not that important overall :)

Yeah, the cleanup part ended up larger than initially anticipated, but I
don't really expect further changes on the fixup side.  I can handle the
split if another revision would be still required, or would you like me to
do this regardless?  I've just made a quick test moving the first patch to
the end of the series and it didn't cause any conflicts, hence there won't 
be any dependencies between the two patch sets.

Thanks,
Cristian

On 7/28/25 12:41 PM, Cristian Ciocaltea wrote:
> Hi Greg,
> 
> On 7/26/25 9:43 AM, Greg Kroah-Hartman wrote:
>> On Sat, Jul 26, 2025 at 01:08:02AM +0300, Cristian Ciocaltea wrote:
>>> The USB/IP Virtual Host Controller (VHCI) platform driver is expected to
>>> prevent entering system suspend when at least one remote device is
>>> attached to the virtual USB root hub.
>>>
>>> However, in some cases, the detection logic for active USB/IP
>>> connections doesn't seem to work reliably, e.g. when all devices
>>> attached to the virtual hub have been already suspended.  This will
>>> normally lead to a broken suspend state, with unrecoverable resume.
>>>
>>> The first patch of the series provides a workaround to ensure the
>>> virtually attached devices do not enter suspend.  Note this is currently
>>> limited to the client side (vhci_hcd) only, since the server side
>>> (usbip_host) doesn't implement system suspend prevention.
>>>
>>> Additionally, during the investigation I noticed and fixed a bunch of
>>> coding style issues, hence the subsequent patches contain all the
>>> changes needed to make checkpatch happy for the entire driver.
>>
>> You are doing two major things here, fixing suspend, and cleaning up
>> checkpatch issues.  Please make that two different patch sets as those
>> are not logical things to put together at all.  Work on the suspend
>> issue first, and after that is all done and working, then consider
>> checkpatch cleanups, those are not that important overall :)
> 
> Yeah, the cleanup part ended up larger than initially anticipated, but I
> don't really expect further changes on the fixup side.  I can handle the
> split if another revision would be still required, or would you like me to
> do this regardless?  I've just made a quick test moving the first patch to
> the end of the series and it didn't cause any conflicts, hence there won't 
> be any dependencies between the two patch sets.

This continues to apply cleanly on recent linux-next, hence I'm not sure if
there's still a need to resend as two separate patch sets.

Please let me know how should we move further.

Thanks,
Cristian

On Wed, Aug 27, 2025 at 12:14:15PM +0300, Cristian Ciocaltea wrote:
> On 7/28/25 12:41 PM, Cristian Ciocaltea wrote:
> > Hi Greg,
> > 
> > On 7/26/25 9:43 AM, Greg Kroah-Hartman wrote:
> >> On Sat, Jul 26, 2025 at 01:08:02AM +0300, Cristian Ciocaltea wrote:
> >>> The USB/IP Virtual Host Controller (VHCI) platform driver is expected to
> >>> prevent entering system suspend when at least one remote device is
> >>> attached to the virtual USB root hub.
> >>>
> >>> However, in some cases, the detection logic for active USB/IP
> >>> connections doesn't seem to work reliably, e.g. when all devices
> >>> attached to the virtual hub have been already suspended.  This will
> >>> normally lead to a broken suspend state, with unrecoverable resume.
> >>>
> >>> The first patch of the series provides a workaround to ensure the
> >>> virtually attached devices do not enter suspend.  Note this is currently
> >>> limited to the client side (vhci_hcd) only, since the server side
> >>> (usbip_host) doesn't implement system suspend prevention.
> >>>
> >>> Additionally, during the investigation I noticed and fixed a bunch of
> >>> coding style issues, hence the subsequent patches contain all the
> >>> changes needed to make checkpatch happy for the entire driver.
> >>
> >> You are doing two major things here, fixing suspend, and cleaning up
> >> checkpatch issues.  Please make that two different patch sets as those
> >> are not logical things to put together at all.  Work on the suspend
> >> issue first, and after that is all done and working, then consider
> >> checkpatch cleanups, those are not that important overall :)
> > 
> > Yeah, the cleanup part ended up larger than initially anticipated, but I
> > don't really expect further changes on the fixup side.  I can handle the
> > split if another revision would be still required, or would you like me to
> > do this regardless?  I've just made a quick test moving the first patch to
> > the end of the series and it didn't cause any conflicts, hence there won't 
> > be any dependencies between the two patch sets.
> 
> This continues to apply cleanly on recent linux-next, hence I'm not sure if
> there's still a need to resend as two separate patch sets.

As I no longer have any of these in my review queue, yes, you are going
to have to resend whatever you want us to accept :)

thanks,

greg k-h