mbox series

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

Message ID 20220607135836.627711-1-mathias.nyman@linux.intel.com (mailing list archive)
Headers show
Series hibernate and roothub port power | expand

Message

Mathias Nyman June 7, 2022, 1:58 p.m. UTC
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(-)

Comments

Oliver Neukum June 8, 2022, 8:19 a.m. UTC | #1
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
Mathias Nyman June 8, 2022, 11:47 a.m. UTC | #2
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
Alan Stern June 8, 2022, 1:43 p.m. UTC | #3
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
Mathias Nyman June 9, 2022, 7:59 a.m. UTC | #4
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
Alan Stern June 9, 2022, 1:48 p.m. UTC | #5
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
Evan Green June 9, 2022, 3:08 p.m. UTC | #6
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
Mathias Nyman June 10, 2022, 10:30 a.m. UTC | #7
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
Mathias Nyman June 14, 2022, 10:07 a.m. UTC | #8
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
Evan Green July 20, 2022, 9:53 p.m. UTC | #9
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
Mathias Nyman Aug. 9, 2022, 1:49 p.m. UTC | #10
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
Evan Green Aug. 15, 2022, 7:01 p.m. UTC | #11
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