On 10/17/18 9:46 PM, Chunfeng Yun wrote:> hi,
>
> On Wed, 2018-10-17 at 18:29 -0400, Al Cooper wrote:
>> Add the ability to skip calling the PHY's exit routine on suspend
>> and the PHY's init routine on resume. This is to handle a USB PHY
>> that should have it's power_off function called on suspend but cannot
>> have it's exit function called because on exit it will disable the
>> PHY to the point where register accesses to the Host Controllers
>> using the PHY will be disabled and the host drivers will crash.
>>
>> This is enabled with the HCD flag "suspend_without_phy_exit" which
>> can be set from any HCD driver.
>>
>> Signed-off-by: Al Cooper<alcooperx@gmail.com>
>> ---
>>   drivers/usb/core/hcd.c  |  8 ++++----
>>   drivers/usb/core/phy.c  | 18 ++++++++++++------
>>   drivers/usb/core/phy.h  |  9 ++++++---
>>   include/linux/usb/hcd.h |  3 +++
>>   4 files changed, 25 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 1c21955fe7c0..e67e4d6b3d21 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2263,7 +2263,7 @@ int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg)
>>           hcd->state = HC_STATE_SUSPENDED;
>>  
>>           if (!PMSG_IS_AUTO(msg))
>> -            usb_phy_roothub_suspend(hcd->self.sysdev,
>> +            usb_phy_roothub_suspend(hcd,
>>                           hcd->phy_roothub);
>>  
>>           /* Did we race with a root-hub wakeup event? */
>> @@ -2304,7 +2304,7 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg)
>>       }
>>  
>>       if (!PMSG_IS_AUTO(msg)) {
>> -        status = usb_phy_roothub_resume(hcd->self.sysdev,
>> +        status = usb_phy_roothub_resume(hcd,
>>                           hcd->phy_roothub);
>>           if (status)
>>               return status;
>> @@ -2347,7 +2347,7 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg)
>>           }
>>       } else {
>>           hcd->state = old_state;
>> -        usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
>> +        usb_phy_roothub_suspend(hcd, hcd->phy_roothub);
>>           dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
>>                   "resume", status);
>>           if (status != -ESHUTDOWN)
>> @@ -2744,7 +2744,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>>       struct usb_device *rhdev;
>>  
>>       if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
>> -        hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
>> +        hcd->phy_roothub = usb_phy_roothub_alloc(hcd);
>>           if (IS_ERR(hcd->phy_roothub))
>>               return PTR_ERR(hcd->phy_roothub);
>>  
>> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
>> index 9879767452a2..b54db39eee4c 100644
>> --- a/drivers/usb/core/phy.c
>> +++ b/drivers/usb/core/phy.c
>> @@ -45,10 +45,11 @@ static int usb_phy_roothub_add_phy(struct device *dev, int index,
>>       return 0;
>>   }
>>  
>> -struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
>> +struct usb_phy_roothub *usb_phy_roothub_alloc(struct usb_hcd *hcd)
>>   {
>>       struct usb_phy_roothub *phy_roothub;
>>       int i, num_phys, err;
>> +    struct device *dev = hcd->self.sysdev;
>>  
>>       if (!IS_ENABLED(CONFIG_GENERIC_PHY))
>>           return NULL;
>> @@ -161,26 +162,30 @@ void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub)
>>   }
>>   EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);
>>  
>> -int usb_phy_roothub_suspend(struct device *controller_dev,
>> +int usb_phy_roothub_suspend(struct usb_hcd *hcd,
>>                   struct usb_phy_roothub *phy_roothub)
>>   {
>> +    struct device *controller_dev = hcd->self.sysdev;
>> +
>>       usb_phy_roothub_power_off(phy_roothub);
>>  
>>       /* keep the PHYs initialized so the device can wake up the system */
>> -    if (device_may_wakeup(controller_dev))
>> +    if (device_may_wakeup(controller_dev) || hcd->suspend_without_phy_exit)
>>           return 0;
>>  
>>       return usb_phy_roothub_exit(phy_roothub);
>>   }
>>   EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);
>>  
>> -int usb_phy_roothub_resume(struct device *controller_dev,
>> +int usb_phy_roothub_resume(struct usb_hcd *hcd,
>>                  struct usb_phy_roothub *phy_roothub)
>>   {
>> +    struct device *controller_dev = hcd->self.sysdev;
>>       int err;
>>  
>>       /* if the device can't wake up the system _exit was called */
>> -    if (!device_may_wakeup(controller_dev)) {
>> +    if (!device_may_wakeup(controller_dev) &&
>> +        !hcd->suspend_without_phy_exit) {
>>           err = usb_phy_roothub_init(phy_roothub);
>>           if (err)
>>               return err;
>> @@ -189,7 +194,8 @@ int usb_phy_roothub_resume(struct device *controller_dev,
>>       err = usb_phy_roothub_power_on(phy_roothub);
>>  
>>       /* undo _init if _power_on failed */
>> -    if (err && !device_may_wakeup(controller_dev))
>> +    if (err && !device_may_wakeup(controller_dev)
>> +        && !hcd->suspend_without_phy_exit)
>>           usb_phy_roothub_exit(phy_roothub);
>>  
>>       return err;
>> diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
>> index 88a3c037e9df..34293e11a917 100644
>> --- a/drivers/usb/core/phy.h
>> +++ b/drivers/usb/core/phy.h
>> @@ -5,13 +5,16 @@
>>    * Copyright (C) 2018 Martin Blumenstingl<martin.blumenstingl@googlemail.com>
>>    */
>>  
>> +#include <linux/usb.h>
>> +#include <linux/usb/hcd.h>
>> +
>>   #ifndef __USB_CORE_PHY_H_
>>   #define __USB_CORE_PHY_H_
>>  
>>   struct device;
>>   struct usb_phy_roothub;
>>  
>> -struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev);
>> +struct usb_phy_roothub *usb_phy_roothub_alloc(struct usb_hcd *hcd);
>>  
>>   int usb_phy_roothub_init(struct usb_phy_roothub *phy_roothub);
>>   int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub);
>> @@ -19,9 +22,9 @@
>>   int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub);
>>   void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub);
>>  
>> -int usb_phy_roothub_suspend(struct device *controller_dev,
>> +int usb_phy_roothub_suspend(struct usb_hcd *hcd,
>>                   struct usb_phy_roothub *phy_roothub);
>> -int usb_phy_roothub_resume(struct device *controller_dev,
>> +int usb_phy_roothub_resume(struct usb_hcd *hcd,
>>                  struct usb_phy_roothub *phy_roothub);
>>  
>>   #endif /* __USB_CORE_PHY_H_ */
>> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
>> index 97e2ddec18b1..87a104055b5e 100644
>> --- a/include/linux/usb/hcd.h
>> +++ b/include/linux/usb/hcd.h
>> @@ -157,6 +157,9 @@ struct usb_hcd {
>>        */
>>       unsigned        skip_phy_initialization:1;
>>  
>> +    /* Some phys don't want the phy's exit/init called on suspend/resume */
>> +    unsigned        suspend_without_phy_exit:1;
> As suggested before, you can skip phy's exit/init during suspend/resume
> by enabling wakeup of hcd, so needn't add a new variable for it.

I still need to be able to enable and disable wakeup for this driver.

>
>> +
>>       /* The next flag is a stopgap, to be removed when all the HCDs
>>        * support the new root-hub polling mechanism. */
>>       unsigned        uses_new_polling:1;
>