[RFC PATCH 0/1] hibernate and roothub port power

[RFC PATCH 0/1] hibernate and roothub port power

[Mathias Nyman]

Hi

Looking for comments on an issue where a self-powered usb device can
survive in suspended U3 state even if the host system is restarted.

This causes additional boot delay as boot firmware is not designed
to handle usb devices in U3.
Boot firmware waits for a usb state change in vain until it times out.

In shutdown (S5), with xHCI as host, this can be solved fairly easily
by turning off roothub port power in the .shutdown path.

This is discussed in xhci spec 4.19.4 for driver unload:
"Before the xHC driver is unloaded, the driver should clear the
Port Power (PP) flag of all Root Hub ports to place them into
the Disabled state and reduce port power consumption."

But for S4 hibernate things get more complicated.
We can't just turn off port power, especially if the usb device can
generate remote wake, and host should resume the system from S4.

But I can't come up with a better solution, so this RFC patch
does exactly that. It turns off port power for xHC roothub ports
in the hibernate poweroff_late stage, but only if the host isn't set
to wake up the system from S4.

This RFC workaround is specific to PCI xHC hosts, but if this solution
makes sense at all, shuold it be turned into a more generic solution?
Maybe calling hc_driver hcd->port_power(hcd, i, false) for each roothub
port in dev_pm_ops usb_hcd_pci_pm_ops .poweroff or .poweroff_late
callbacks?

Mathias Nyman (1):
  xhci: pci: power off roothub ports in hibernate poweroff_late stage

 drivers/usb/host/xhci-hub.c |  4 ++--
 drivers/usb/host/xhci-pci.c | 32 +++++++++++++++++++++++++++++++-
 drivers/usb/host/xhci.h     |  2 ++
 3 files changed, 35 insertions(+), 3 deletions(-)

-- 
2.25.1

[RFC PATCH 1/1] xhci: pci: power off roothub ports in hibernate poweroff_late stage

[Mathias Nyman]

Power off roothub ports in hibernate S4 poweroff_late stage to avoid
self-powered usb devices from surviving in U3 suspended state into
next reboot. Bootloader/firmware can't handle usb ports in U3, and
will timeout, causing extra delay during reboot/restore from S4.

Only turn off ports if xhci isn't capable of waking up from S4, as
turning off port power will disable remote wake for connected devices.

This is a bit of a hack, we create a custom non-constant dev_pm_ops
strucure for xhci-pci driver where we first copy the standard
usb_hcd_pci_pm_ops content, and then add the poweroff_late callback
to turn of port power for xhci-pci

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c |  4 ++--
 drivers/usb/host/xhci-pci.c | 32 +++++++++++++++++++++++++++++++-
 drivers/usb/host/xhci.h     |  2 ++
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index c54f2bc23d3f..dc404107f4d9 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -652,8 +652,8 @@ struct xhci_hub *xhci_get_rhub(struct usb_hcd *hcd)
  * It will release and re-aquire the lock while calling ACPI
  * method.
  */
-static void xhci_set_port_power(struct xhci_hcd *xhci, struct usb_hcd *hcd,
-				u16 index, bool on, unsigned long *flags)
+void xhci_set_port_power(struct xhci_hcd *xhci, struct usb_hcd *hcd, u16 index,
+			 bool on, unsigned long *flags)
 	__must_hold(&xhci->lock)
 {
 	struct xhci_hub *rhub;
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index fac9492a8bda..89c4c3f07a99 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -85,6 +85,7 @@
 static const char hcd_name[] = "xhci_hcd";
 
 static struct hc_driver __read_mostly xhci_pci_hc_driver;
+static struct dev_pm_ops xhci_hcd_pci_pm_ops;
 
 static int xhci_pci_setup(struct usb_hcd *hcd);
 
@@ -630,6 +631,32 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
 	return retval;
 }
 
+static int xhci_pci_poweroff_late(struct device *dev)
+{
+	struct pci_dev          *pci_dev = to_pci_dev(dev);
+	struct usb_hcd          *hcd = pci_get_drvdata(pci_dev);
+	struct xhci_hcd		*xhci = hcd_to_xhci(hcd);
+	unsigned long		flags;
+	int			i;
+
+	if (!device_may_wakeup(dev)) {
+		spin_lock_irqsave(&xhci->lock, flags);
+
+		/* Power off USB2 ports*/
+		for (i = 0; i < xhci->usb2_rhub.num_ports; i++)
+			xhci_set_port_power(xhci, xhci->main_hcd, i, false,
+					    &flags);
+		/* Power off USB3 ports*/
+		for (i = 0; i < xhci->usb3_rhub.num_ports; i++)
+			xhci_set_port_power(xhci, xhci->shared_hcd, i, false,
+					    &flags);
+
+		spin_unlock_irqrestore(&xhci->lock, flags);
+	}
+
+	return 0;
+}
+
 static void xhci_pci_shutdown(struct usb_hcd *hcd)
 {
 	struct xhci_hcd		*xhci = hcd_to_xhci(hcd);
@@ -685,7 +712,7 @@ static struct pci_driver xhci_pci_driver = {
 	.shutdown = 	usb_hcd_pci_shutdown,
 #ifdef CONFIG_PM
 	.driver = {
-		.pm = &usb_hcd_pci_pm_ops
+		.pm = &xhci_hcd_pci_pm_ops
 	},
 #endif
 };
@@ -697,6 +724,9 @@ static int __init xhci_pci_init(void)
 	xhci_pci_hc_driver.pci_suspend = xhci_pci_suspend;
 	xhci_pci_hc_driver.pci_resume = xhci_pci_resume;
 	xhci_pci_hc_driver.shutdown = xhci_pci_shutdown;
+
+	xhci_hcd_pci_pm_ops = usb_hcd_pci_pm_ops;
+	xhci_hcd_pci_pm_ops.poweroff_late = xhci_pci_poweroff_late;
 #endif
 	return pci_register_driver(&xhci_pci_driver);
 }
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 0bd76c94a4b1..28aaf031f9a8 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2196,6 +2196,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex,
 int xhci_hub_status_data(struct usb_hcd *hcd, char *buf);
 int xhci_find_raw_port_number(struct usb_hcd *hcd, int port1);
 struct xhci_hub *xhci_get_rhub(struct usb_hcd *hcd);
+void xhci_set_port_power(struct xhci_hcd *xhci, struct usb_hcd *hcd, u16 index,
+			 bool on, unsigned long *flags);
 
 void xhci_hc_died(struct xhci_hcd *xhci);
 
-- 
2.25.1

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Oliver Neukum]

On 07.06.22 15:58, Mathias Nyman wrote:

Hi,

> In shutdown (S5), with xHCI as host, this can be solved fairly easily
> by turning off roothub port power in the .shutdown path.

That would suck for the people who charge their phone from their
computer.

> This is discussed in xhci spec 4.19.4 for driver unload:
> "Before the xHC driver is unloaded, the driver should clear the
> Port Power (PP) flag of all Root Hub ports to place them into
> the Disabled state and reduce port power consumption."

Yes, you could say that the standard calls for this.

> But I can't come up with a better solution, so this RFC patch
> does exactly that. It turns off port power for xHC roothub ports
> in the hibernate poweroff_late stage, but only if the host isn't set
> to wake up the system from S4.

In general this looks like the sane strategy.
However, what will this do if the port is in an alternate mode
or if this is the port the system's battery is to be charged through?

	Regards
		Oliver

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Mathias Nyman]

On 8.6.2022 11.19, Oliver Neukum wrote:
> 
> 
> On 07.06.22 15:58, Mathias Nyman wrote:
> 
> Hi,
> 
>> In shutdown (S5), with xHCI as host, this can be solved fairly easily
>> by turning off roothub port power in the .shutdown path.
> 
> That would suck for the people who charge their phone from their
> computer.

Good point.
My guess is that xHC port power bits won't actually turn off VBus for those
Sleep-and-charge, or Always-on ports.
VBus is allowed to be on even if port is in power-off state, but usb link state
should be forced to ss.Disabled from other states, like U3.

Need to try it out, it's possible this turns off VBus for some usb-A ports
on some older systems that earlier (accidentally?) supplied VBus on
"normal" ports after shutdown.

> 
>> This is discussed in xhci spec 4.19.4 for driver unload:
>> "Before the xHC driver is unloaded, the driver should clear the
>> Port Power (PP) flag of all Root Hub ports to place them into
>> the Disabled state and reduce port power consumption."
> 
> Yes, you could say that the standard calls for this.
> 
>> But I can't come up with a better solution, so this RFC patch
>> does exactly that. It turns off port power for xHC roothub ports
>> in the hibernate poweroff_late stage, but only if the host isn't set
>> to wake up the system from S4.
> 
> In general this looks like the sane strategy.
> However, what will this do if the port is in an alternate mode
> or if this is the port the system's battery is to be charged through?

I have to double check this, but my understanding is that xHC port power bits
don't have any impact here. These cases are handled by other parts like the
power delivery controller before the connector may even be muxed to a xHC.
Most likely turning off xHC port power bits here will only force a connected
usb device link state to ss.Disabled (or usb2 equivalent state).

Thanks for the feedback

-Mathias

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Alan Stern]

On Wed, Jun 08, 2022 at 02:47:22PM +0300, Mathias Nyman wrote:
> On 8.6.2022 11.19, Oliver Neukum wrote:
> > 
> > 
> > On 07.06.22 15:58, Mathias Nyman wrote:
> > 
> > Hi,
> > 
> > > In shutdown (S5), with xHCI as host, this can be solved fairly easily
> > > by turning off roothub port power in the .shutdown path.
> > 
> > That would suck for the people who charge their phone from their
> > computer.
> 
> Good point.
> My guess is that xHC port power bits won't actually turn off VBus for those
> Sleep-and-charge, or Always-on ports.
> VBus is allowed to be on even if port is in power-off state, but usb link state
> should be forced to ss.Disabled from other states, like U3.
> 
> Need to try it out, it's possible this turns off VBus for some usb-A ports
> on some older systems that earlier (accidentally?) supplied VBus on
> "normal" ports after shutdown.

How about turning off port power _only_ in the shutdown or unbind path, 
and setting the port link states to ss.Disabled in the poweroff or 
poweroff_noirq stage of hibernation (if wakeup is disabled)?  Would that 
solve the problem of the firmware needing to time out on reboot?

Alan Stern

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Mathias Nyman]

On 8.6.2022 16.43, Alan Stern wrote:
> On Wed, Jun 08, 2022 at 02:47:22PM +0300, Mathias Nyman wrote:
>> On 8.6.2022 11.19, Oliver Neukum wrote:
>>>
>>>
>>> On 07.06.22 15:58, Mathias Nyman wrote:
>>>
>>> Hi,
>>>
>>>> In shutdown (S5), with xHCI as host, this can be solved fairly easily
>>>> by turning off roothub port power in the .shutdown path.
>>>
>>> That would suck for the people who charge their phone from their
>>> computer.
>>
>> Good point.
>> My guess is that xHC port power bits won't actually turn off VBus for those
>> Sleep-and-charge, or Always-on ports.
>> VBus is allowed to be on even if port is in power-off state, but usb link state
>> should be forced to ss.Disabled from other states, like U3.
>>
>> Need to try it out, it's possible this turns off VBus for some usb-A ports
>> on some older systems that earlier (accidentally?) supplied VBus on
>> "normal" ports after shutdown.
> 
> How about turning off port power _only_ in the shutdown or unbind path,
> and setting the port link states to ss.Disabled in the poweroff or
> poweroff_noirq stage of hibernation (if wakeup is disabled)?  Would that
> solve the problem of the firmware needing to time out on reboot?
> 

That would be optimal, but unfortunately xHCI doesn't support setting link
state directly to ss.Disabled. Only way is to clear port power (PP) bit.

To avoid turning off VBus in hibernate we could limit port power bit clearing
to xHC hosts that don't have the Port Power Control (PPC) capability flag.

We know these xHC hosts don't control power switches, and clearing PP won't turn
off VBus (xhci 5.4.8, PORTRSC)

This could be a solution for some hosts, but probably not cover all.
Not sure if the hardware this was reported on has PPC flag set.

Thanks
Mathias

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Alan Stern]

On Thu, Jun 09, 2022 at 10:59:37AM +0300, Mathias Nyman wrote:
> On 8.6.2022 16.43, Alan Stern wrote:
> > On Wed, Jun 08, 2022 at 02:47:22PM +0300, Mathias Nyman wrote:
> > > On 8.6.2022 11.19, Oliver Neukum wrote:
> > > > 
> > > > 
> > > > On 07.06.22 15:58, Mathias Nyman wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > > In shutdown (S5), with xHCI as host, this can be solved fairly easily
> > > > > by turning off roothub port power in the .shutdown path.
> > > > 
> > > > That would suck for the people who charge their phone from their
> > > > computer.
> > > 
> > > Good point.
> > > My guess is that xHC port power bits won't actually turn off VBus for those
> > > Sleep-and-charge, or Always-on ports.
> > > VBus is allowed to be on even if port is in power-off state, but usb link state
> > > should be forced to ss.Disabled from other states, like U3.
> > > 
> > > Need to try it out, it's possible this turns off VBus for some usb-A ports
> > > on some older systems that earlier (accidentally?) supplied VBus on
> > > "normal" ports after shutdown.
> > 
> > How about turning off port power _only_ in the shutdown or unbind path,
> > and setting the port link states to ss.Disabled in the poweroff or
> > poweroff_noirq stage of hibernation (if wakeup is disabled)?  Would that
> > solve the problem of the firmware needing to time out on reboot?
> > 
> 
> That would be optimal, but unfortunately xHCI doesn't support setting link
> state directly to ss.Disabled. Only way is to clear port power (PP) bit.

What would happen if you clear the PP bit, wait for the link state to 
become ss.Disabled, and then turn PP back on?

> To avoid turning off VBus in hibernate we could limit port power bit clearing
> to xHC hosts that don't have the Port Power Control (PPC) capability flag.
> 
> We know these xHC hosts don't control power switches, and clearing PP won't turn
> off VBus (xhci 5.4.8, PORTRSC)
> 
> This could be a solution for some hosts, but probably not cover all.
> Not sure if the hardware this was reported on has PPC flag set.

In theory the problem could affect systems having any kind of xHCI 
hardware, since it's really a defect in the system firmware.

Doesn't Windows have a hibernation mode?  Do you know what state it 
leaves the xHCI ports in during hibernation?

Alan Stern

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Evan Green]

On Thu, Jun 9, 2022 at 12:58 AM Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:
>
> On 8.6.2022 16.43, Alan Stern wrote:
> > On Wed, Jun 08, 2022 at 02:47:22PM +0300, Mathias Nyman wrote:
> >> On 8.6.2022 11.19, Oliver Neukum wrote:
> >>>
> >>>
> >>> On 07.06.22 15:58, Mathias Nyman wrote:
> >>>
> >>> Hi,
> >>>
> >>>> In shutdown (S5), with xHCI as host, this can be solved fairly easily
> >>>> by turning off roothub port power in the .shutdown path.
> >>>
> >>> That would suck for the people who charge their phone from their
> >>> computer.
> >>
> >> Good point.
> >> My guess is that xHC port power bits won't actually turn off VBus for those
> >> Sleep-and-charge, or Always-on ports.
> >> VBus is allowed to be on even if port is in power-off state, but usb link state
> >> should be forced to ss.Disabled from other states, like U3.
> >>
> >> Need to try it out, it's possible this turns off VBus for some usb-A ports
> >> on some older systems that earlier (accidentally?) supplied VBus on
> >> "normal" ports after shutdown.
> >
> > How about turning off port power _only_ in the shutdown or unbind path,
> > and setting the port link states to ss.Disabled in the poweroff or
> > poweroff_noirq stage of hibernation (if wakeup is disabled)?  Would that
> > solve the problem of the firmware needing to time out on reboot?
> >
>
> That would be optimal, but unfortunately xHCI doesn't support setting link
> state directly to ss.Disabled. Only way is to clear port power (PP) bit.
>
> To avoid turning off VBus in hibernate we could limit port power bit clearing
> to xHC hosts that don't have the Port Power Control (PPC) capability flag.
>
> We know these xHC hosts don't control power switches, and clearing PP won't turn
> off VBus (xhci 5.4.8, PORTRSC)
>
> This could be a solution for some hosts, but probably not cover all.
> Not sure if the hardware this was reported on has PPC flag set.

It appears it does not, HCCPARAMS1 for both USB controllers seems to
be 0x20007fc1 (missing bit 3). You can check my work in case I made an
error here: https://pastebin.com/9raZc63N
-Evan

>
> Thanks
> Mathias

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Mathias Nyman]

On 9.6.2022 16.48, Alan Stern wrote:
> On Thu, Jun 09, 2022 at 10:59:37AM +0300, Mathias Nyman wrote:
>> On 8.6.2022 16.43, Alan Stern wrote:
>>> On Wed, Jun 08, 2022 at 02:47:22PM +0300, Mathias Nyman wrote:
>>>> On 8.6.2022 11.19, Oliver Neukum wrote:
>>>>>
>>>>>
>>>>> On 07.06.22 15:58, Mathias Nyman wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>>> In shutdown (S5), with xHCI as host, this can be solved fairly easily
>>>>>> by turning off roothub port power in the .shutdown path.
>>>>>
>>>>> That would suck for the people who charge their phone from their
>>>>> computer.
>>>>
>>>> Good point.
>>>> My guess is that xHC port power bits won't actually turn off VBus for those
>>>> Sleep-and-charge, or Always-on ports.
>>>> VBus is allowed to be on even if port is in power-off state, but usb link state
>>>> should be forced to ss.Disabled from other states, like U3.
>>>>
>>>> Need to try it out, it's possible this turns off VBus for some usb-A ports
>>>> on some older systems that earlier (accidentally?) supplied VBus on
>>>> "normal" ports after shutdown.
>>>
>>> How about turning off port power _only_ in the shutdown or unbind path,
>>> and setting the port link states to ss.Disabled in the poweroff or
>>> poweroff_noirq stage of hibernation (if wakeup is disabled)?  Would that
>>> solve the problem of the firmware needing to time out on reboot?
>>>
>>
>> That would be optimal, but unfortunately xHCI doesn't support setting link
>> state directly to ss.Disabled. Only way is to clear port power (PP) bit.

Correction,  we can get USB 3 links to ss.Disabled and port to Disabled state
by setting the PORTSC PED bit.
USB 2 link goes to Polling, port Disabled.

Port power is left untouched.
This could work.

> 
> What would happen if you clear the PP bit, wait for the link state to
> become ss.Disabled, and then turn PP back on?

For USB 3 devices host will detect the device, and do all steps until device is
in an enabled U0 state. All without driver interaction, and even if host is not running.

USB 2 devices probably stop in Disabled/link:Polling waiting for driver port reset

> 
>> To avoid turning off VBus in hibernate we could limit port power bit clearing
>> to xHC hosts that don't have the Port Power Control (PPC) capability flag.
>>
>> We know these xHC hosts don't control power switches, and clearing PP won't turn
>> off VBus (xhci 5.4.8, PORTRSC)
>>
>> This could be a solution for some hosts, but probably not cover all.
>> Not sure if the hardware this was reported on has PPC flag set.
> 
> In theory the problem could affect systems having any kind of xHCI
> hardware, since it's really a defect in the system firmware.
> 
> Doesn't Windows have a hibernation mode?  Do you know what state it
> leaves the xHCI ports in during hibernation?

Good question, haven't looked into what windows does here.
I think this problematic boot firmware is specific for non-windows systems.
but again, not sure.
Worth looking into

Thanks

-Mathias

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Mathias Nyman]

On 9.6.2022 18.08, Evan Green wrote:
> On Thu, Jun 9, 2022 at 12:58 AM Mathias Nyman
> <mathias.nyman@linux.intel.com> wrote:
>>
>> On 8.6.2022 16.43, Alan Stern wrote:
>>> On Wed, Jun 08, 2022 at 02:47:22PM +0300, Mathias Nyman wrote:
>>>> On 8.6.2022 11.19, Oliver Neukum wrote:
>>>>>
>>>>>
>>>>> On 07.06.22 15:58, Mathias Nyman wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>>> In shutdown (S5), with xHCI as host, this can be solved fairly easily
>>>>>> by turning off roothub port power in the .shutdown path.
>>>>>
>>>>> That would suck for the people who charge their phone from their
>>>>> computer.
>>>>
>>>> Good point.
>>>> My guess is that xHC port power bits won't actually turn off VBus for those
>>>> Sleep-and-charge, or Always-on ports.
>>>> VBus is allowed to be on even if port is in power-off state, but usb link state
>>>> should be forced to ss.Disabled from other states, like U3.
>>>>
>>>> Need to try it out, it's possible this turns off VBus for some usb-A ports
>>>> on some older systems that earlier (accidentally?) supplied VBus on
>>>> "normal" ports after shutdown.
>>>
>>> How about turning off port power _only_ in the shutdown or unbind path,
>>> and setting the port link states to ss.Disabled in the poweroff or
>>> poweroff_noirq stage of hibernation (if wakeup is disabled)?  Would that
>>> solve the problem of the firmware needing to time out on reboot?
>>>
>>
>> That would be optimal, but unfortunately xHCI doesn't support setting link
>> state directly to ss.Disabled. Only way is to clear port power (PP) bit.
>>
>> To avoid turning off VBus in hibernate we could limit port power bit clearing
>> to xHC hosts that don't have the Port Power Control (PPC) capability flag.
>>
>> We know these xHC hosts don't control power switches, and clearing PP won't turn
>> off VBus (xhci 5.4.8, PORTRSC)
>>
>> This could be a solution for some hosts, but probably not cover all.
>> Not sure if the hardware this was reported on has PPC flag set.
> 
> It appears it does not, HCCPARAMS1 for both USB controllers seems to
> be 0x20007fc1 (missing bit 3). You can check my work in case I made an
> error here: https://pastebin.com/9raZc63N
> -Evan

Thanks, good to know.
So if disabling ports in hibernate doesn't work then we could turn off port power for
hosts with PPC==0. It should at least solve the issue for this particular system,
and not change current VBus policy in hibernate.

-Mathias

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Evan Green]

On Tue, Jun 14, 2022 at 3:06 AM Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:
>
> On 9.6.2022 18.08, Evan Green wrote:
> > On Thu, Jun 9, 2022 at 12:58 AM Mathias Nyman
> > <mathias.nyman@linux.intel.com> wrote:
> >>
> >> On 8.6.2022 16.43, Alan Stern wrote:
> >>> On Wed, Jun 08, 2022 at 02:47:22PM +0300, Mathias Nyman wrote:
> >>>> On 8.6.2022 11.19, Oliver Neukum wrote:
> >>>>>
> >>>>>
> >>>>> On 07.06.22 15:58, Mathias Nyman wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>>> In shutdown (S5), with xHCI as host, this can be solved fairly easily
> >>>>>> by turning off roothub port power in the .shutdown path.
> >>>>>
> >>>>> That would suck for the people who charge their phone from their
> >>>>> computer.
> >>>>
> >>>> Good point.
> >>>> My guess is that xHC port power bits won't actually turn off VBus for those
> >>>> Sleep-and-charge, or Always-on ports.
> >>>> VBus is allowed to be on even if port is in power-off state, but usb link state
> >>>> should be forced to ss.Disabled from other states, like U3.
> >>>>
> >>>> Need to try it out, it's possible this turns off VBus for some usb-A ports
> >>>> on some older systems that earlier (accidentally?) supplied VBus on
> >>>> "normal" ports after shutdown.
> >>>
> >>> How about turning off port power _only_ in the shutdown or unbind path,
> >>> and setting the port link states to ss.Disabled in the poweroff or
> >>> poweroff_noirq stage of hibernation (if wakeup is disabled)?  Would that
> >>> solve the problem of the firmware needing to time out on reboot?
> >>>
> >>
> >> That would be optimal, but unfortunately xHCI doesn't support setting link
> >> state directly to ss.Disabled. Only way is to clear port power (PP) bit.
> >>
> >> To avoid turning off VBus in hibernate we could limit port power bit clearing
> >> to xHC hosts that don't have the Port Power Control (PPC) capability flag.
> >>
> >> We know these xHC hosts don't control power switches, and clearing PP won't turn
> >> off VBus (xhci 5.4.8, PORTRSC)
> >>
> >> This could be a solution for some hosts, but probably not cover all.
> >> Not sure if the hardware this was reported on has PPC flag set.
> >
> > It appears it does not, HCCPARAMS1 for both USB controllers seems to
> > be 0x20007fc1 (missing bit 3). You can check my work in case I made an
> > error here: https://pastebin.com/9raZc63N
> > -Evan
>
> Thanks, good to know.
> So if disabling ports in hibernate doesn't work then we could turn off port power for
> hosts with PPC==0. It should at least solve the issue for this particular system,
> and not change current VBus policy in hibernate.

Did a new version of this ever make it out?
-Evan

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Mathias Nyman]

On 21.7.2022 0.53, Evan Green wrote:
> On Tue, Jun 14, 2022 at 3:06 AM Mathias Nyman
> <mathias.nyman@linux.intel.com> wrote:
>>
>> On 9.6.2022 18.08, Evan Green wrote:
>>> On Thu, Jun 9, 2022 at 12:58 AM Mathias Nyman
>>> <mathias.nyman@linux.intel.com> wrote:
>>>>
>>>> On 8.6.2022 16.43, Alan Stern wrote:
>>>>> On Wed, Jun 08, 2022 at 02:47:22PM +0300, Mathias Nyman wrote:
>>>>>> On 8.6.2022 11.19, Oliver Neukum wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 07.06.22 15:58, Mathias Nyman wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>>> In shutdown (S5), with xHCI as host, this can be solved fairly easily
>>>>>>>> by turning off roothub port power in the .shutdown path.
>>>>>>>
>>>>>>> That would suck for the people who charge their phone from their
>>>>>>> computer.
>>>>>>
>>>>>> Good point.
>>>>>> My guess is that xHC port power bits won't actually turn off VBus for those
>>>>>> Sleep-and-charge, or Always-on ports.
>>>>>> VBus is allowed to be on even if port is in power-off state, but usb link state
>>>>>> should be forced to ss.Disabled from other states, like U3.
>>>>>>
>>>>>> Need to try it out, it's possible this turns off VBus for some usb-A ports
>>>>>> on some older systems that earlier (accidentally?) supplied VBus on
>>>>>> "normal" ports after shutdown.
>>>>>
>>>>> How about turning off port power _only_ in the shutdown or unbind path,
>>>>> and setting the port link states to ss.Disabled in the poweroff or
>>>>> poweroff_noirq stage of hibernation (if wakeup is disabled)?  Would that
>>>>> solve the problem of the firmware needing to time out on reboot?
>>>>>
>>>>
>>>> That would be optimal, but unfortunately xHCI doesn't support setting link
>>>> state directly to ss.Disabled. Only way is to clear port power (PP) bit.
>>>>
>>>> To avoid turning off VBus in hibernate we could limit port power bit clearing
>>>> to xHC hosts that don't have the Port Power Control (PPC) capability flag.
>>>>
>>>> We know these xHC hosts don't control power switches, and clearing PP won't turn
>>>> off VBus (xhci 5.4.8, PORTRSC)
>>>>
>>>> This could be a solution for some hosts, but probably not cover all.
>>>> Not sure if the hardware this was reported on has PPC flag set.
>>>
>>> It appears it does not, HCCPARAMS1 for both USB controllers seems to
>>> be 0x20007fc1 (missing bit 3). You can check my work in case I made an
>>> error here: https://pastebin.com/9raZc63N
>>> -Evan
>>
>> Thanks, good to know.
>> So if disabling ports in hibernate doesn't work then we could turn off port power for
>> hosts with PPC==0. It should at least solve the issue for this particular system,
>> and not change current VBus policy in hibernate.
> 
> Did a new version of this ever make it out?
> -Evan

started writing a patch before vacation that sets port link to ss.Disabled instead of
turning off power power.

Still has some FIXME lines and extra prints, but should be testable.
pushed to a fix_port_disable_s4 topic branch on top of 5.19:

git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git fix_port_disable_s4
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=fix_port_disable_s4

There was also one reported regression with powering off ports in shutdown S5, probably need
to sort that out before pushing this.

Thanks
-Mathias

Re: [RFC PATCH 0/1] hibernate and roothub port power

[Evan Green]

On Tue, Aug 9, 2022 at 6:47 AM Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:
>
> On 21.7.2022 0.53, Evan Green wrote:
> > On Tue, Jun 14, 2022 at 3:06 AM Mathias Nyman
> > <mathias.nyman@linux.intel.com> wrote:
> >>
> >> On 9.6.2022 18.08, Evan Green wrote:
> >>> On Thu, Jun 9, 2022 at 12:58 AM Mathias Nyman
> >>> <mathias.nyman@linux.intel.com> wrote:
> >>>>
> >>>> On 8.6.2022 16.43, Alan Stern wrote:
> >>>>> On Wed, Jun 08, 2022 at 02:47:22PM +0300, Mathias Nyman wrote:
> >>>>>> On 8.6.2022 11.19, Oliver Neukum wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 07.06.22 15:58, Mathias Nyman wrote:
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>>> In shutdown (S5), with xHCI as host, this can be solved fairly easily
> >>>>>>>> by turning off roothub port power in the .shutdown path.
> >>>>>>>
> >>>>>>> That would suck for the people who charge their phone from their
> >>>>>>> computer.
> >>>>>>
> >>>>>> Good point.
> >>>>>> My guess is that xHC port power bits won't actually turn off VBus for those
> >>>>>> Sleep-and-charge, or Always-on ports.
> >>>>>> VBus is allowed to be on even if port is in power-off state, but usb link state
> >>>>>> should be forced to ss.Disabled from other states, like U3.
> >>>>>>
> >>>>>> Need to try it out, it's possible this turns off VBus for some usb-A ports
> >>>>>> on some older systems that earlier (accidentally?) supplied VBus on
> >>>>>> "normal" ports after shutdown.
> >>>>>
> >>>>> How about turning off port power _only_ in the shutdown or unbind path,
> >>>>> and setting the port link states to ss.Disabled in the poweroff or
> >>>>> poweroff_noirq stage of hibernation (if wakeup is disabled)?  Would that
> >>>>> solve the problem of the firmware needing to time out on reboot?
> >>>>>
> >>>>
> >>>> That would be optimal, but unfortunately xHCI doesn't support setting link
> >>>> state directly to ss.Disabled. Only way is to clear port power (PP) bit.
> >>>>
> >>>> To avoid turning off VBus in hibernate we could limit port power bit clearing
> >>>> to xHC hosts that don't have the Port Power Control (PPC) capability flag.
> >>>>
> >>>> We know these xHC hosts don't control power switches, and clearing PP won't turn
> >>>> off VBus (xhci 5.4.8, PORTRSC)
> >>>>
> >>>> This could be a solution for some hosts, but probably not cover all.
> >>>> Not sure if the hardware this was reported on has PPC flag set.
> >>>
> >>> It appears it does not, HCCPARAMS1 for both USB controllers seems to
> >>> be 0x20007fc1 (missing bit 3). You can check my work in case I made an
> >>> error here: https://pastebin.com/9raZc63N
> >>> -Evan
> >>
> >> Thanks, good to know.
> >> So if disabling ports in hibernate doesn't work then we could turn off port power for
> >> hosts with PPC==0. It should at least solve the issue for this particular system,
> >> and not change current VBus policy in hibernate.
> >
> > Did a new version of this ever make it out?
> > -Evan
>
> started writing a patch before vacation that sets port link to ss.Disabled instead of
> turning off power power.
>
> Still has some FIXME lines and extra prints, but should be testable.
> pushed to a fix_port_disable_s4 topic branch on top of 5.19:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git fix_port_disable_s4
> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=fix_port_disable_s4

Thanks! I was OOO last week, but I'm back and should be able to give
it a try shortly.

>
> There was also one reported regression with powering off ports in shutdown S5, probably need
> to sort that out before pushing this.

Uh oh, I was just made aware of a regression as well on 4.19 that got
narrowed down to that. I'm trying to learn more now.

>
> Thanks
> -Mathias