[PATCH usb v5 0/6] usb/core/phy fixes for v4.17

[Keerthy]

On Monday 09 April 2018 02:57 AM, Martin Blumenstingl wrote:
> This is a follow-up to my previous series "initialize (multiple) PHYs
> for a HCD": [0].
> 
> Roger Quadros reported [1] that it "is breaking low power cases on TI
> SoCs when USB is in host mode". He further explains that "Not doing the
> phy_exit() here [when entering suspend] leaves the clocks enabled on
> our SoC and we're no longer able to reach low power states on system
> suspend."
> Chunfeng Yun from Mediatek noted [2] that we cannot unconditionally call
> phy_exit while entering system suspend, because this would "disconnect
> plugged devices on MTK platforms, due to re-initialize u2 phys when
> resume"
> 
> In the discussion (which followed Roger's bug report: [1]) Roger,
> Chunfeng and me came to the conclusion that we can fix suspend on the
> TI SoCs without breaking it on the Mediatek SoCs by extending the
> suspend and resume code in usb/core/phy.c by checking whether the USB
> controller can wake up the system (which is the case for the Mediatek
> MTU3 controller, but now for the dwc3 controller used on the TI SoCs):
> - if the controller can wake up the system (Mediatek MTU3 use-case) we
>   only call usb_phy_roothub_power_off (which calls phy_power_off) when
>   entering system suspend
> - if the controller however cannot wake up the system (dwc3 on TI SoCs)
>   we additionally call usb_phy_roothub_exit (which calls phy_exit) when
>   entering system suspend
> - (we undo the previous steps during system resume)
> 
> The goal of this series is to fix the issue reported by Roger without
> breaking suspend/resume on the Mediatek SoCs.
> Since I neither have a TI nor a Mediatek device I am sending this as
> RFC. I have tested it on an Amlogic Meson GXM board (Khadas VIM2) which
> does NOT support suspend/resume yet.
> 
> Additionally Stefan Wahren reported [7] that booting on a Raspberry Pi
> with CONFIG_GENERIC_PHY being disabled (which is the case for
> bcm2835_defconfig) breaks USB. A fix for this is also included.
> 

FWIW

The series fixes suspend/resume on TI AM437X-GP-EVM platform.

Tested-by: Keerthy <j-keerthy at ti.com>

Regards,
Keerthy
> 
> changes since v4 at [8]
> - updated series title since it now includes fixes for other
>   functionality than suspend
> - rebased on top of f8cf2f16a7c95a from Linus' tree -> I will re-send
>   an updated version once v4.17-rc1 is out, I just sent it early to
>   get some feedback early!
> - included patch [3] "usb: core: phy: fix return value of
>   usb_phy_roothub_exit()" in this series
> - fix the logic if CONFIG_GENERIC_PHY is disabled (as reported by
>   Stefan Wahren, new patch #4)
> - two minor coding style fixes as suggested by Masahiro Yamada (new
>   patches #4 and #5)
> 
> changes since RFC v3 at [6]:
> - added Chunfeng Yun's Tested-by and Roger Quadros' Reviewed-by (thank
>   you!)
> - dropped RFC prefix
> 
> changes since RFC v2 at [5]:
> - add missing INIT_LIST_HEAD call in usb_phy_roothub_add_phy (affects
>   patch #1 - spotted by Roger Quadros, thank you!)
> - fixed swapped conditions using device_may_wakeup() in
>   usb_phy_roothub_resume because we need to call usb_phy_roothub_init
>   if the controller cannot wake up the device (affects patch #2, spotted
>   by Chunfeng Yun, thank you!)
> - simplified the error condition to "undo" usb_phy_roothub_init if
>   usb_phy_roothub_power_on failed in usb_phy_roothub_resume (suggested
>   by Chunfeng Yun)
> - updated the commit message (using Roger's wording) because (quote from
>   Roger "it doesn't prevent the system from entering suspend but just
>   prevents the system from reaching lowest power levels in the suspend
>   state."
> 
> Changes since RFC v1 (blob attachments) at [4]:
> - use device_may_wakeup instead of device_can_wakeup as suggested by
>   Roger Quadros
> - use the controller device from hcd->self.controller as suggested by
>   Chunfeng Yun
> - compile time fixes thanks to Roger Quadros
> - if usb_phy_roothub_power_on in usb_phy_roothub_resume failes then
>   we now call usb_phy_roothub_exit to keep the PHYs in the correct
>   state if usb_phy_roothub_resume partially failed
> 
> 
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006599.html
> [1] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006737.html
> [2] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006758.html
> [3] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006819.html
> [4] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006794.html
> [5] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006820.html
> [6] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006847.html
> [7] https://www.spinics.net/lists/linux-usb/msg167472.html
> [8] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006882.html
> 
> 
> Martin Blumenstingl (6):
>   usb: core: phy: fix return value of usb_phy_roothub_exit()
>   usb: core: split usb_phy_roothub_{init,alloc}
>   usb: core: use phy_exit during suspend if wake up is not supported
>   usb: core: phy: make it a no-op if CONFIG_GENERIC_PHY is disabled
>   usb: core: phy: add missing forward declaration for "struct device"
>   usb: core: phy: add the SPDX-License-Identifier and include guard
> 
>  drivers/usb/core/hcd.c | 18 +++++---
>  drivers/usb/core/phy.c | 93 ++++++++++++++++++++++++++++++------------
>  drivers/usb/core/phy.h | 22 +++++++++-
>  3 files changed, 99 insertions(+), 34 deletions(-)
>