Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Alan Cooper]

[-- Attachment #1: Type: text/plain, Size: 7207 bytes --]

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;
>

[-- Attachment #2: Type: text/html, Size: 9488 bytes --]

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Chunfeng Yun]

hi,
On Tue, 2018-10-30 at 18:30 -0400, Alan Cooper wrote:
> 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(-)
> >>


> >>       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.
Just use device_init_wakeup(dev, true) instead of
device_wakeup_enable(dev) for your controller driver, you can try it.

Sorry for the late replay
> 
> >
> >> +
> >>       /* 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;
> >

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Chunfeng Yun]

hi,
On Tue, 2018-10-30 at 18:30 -0400, Alan Cooper wrote:
> 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(-)
> >>


> >>       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.
Just use device_init_wakeup(dev, true) instead of
device_wakeup_enable(dev) for your controller driver, you can try it.

Sorry for the late replay
> 
> >
> >> +
> >>       /* 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;
> >

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Alan Cooper]

On Wed, Nov 7, 2018 at 8:43 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> hi,
> On Tue, 2018-10-30 at 18:30 -0400, Alan Cooper wrote:
> > 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(-)
> > >>
>
>
> > >>       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.
> Just use device_init_wakeup(dev, true) instead of
> device_wakeup_enable(dev) for your controller driver, you can try it.
>
> Sorry for the late replay

What about the case where sysfs is used to disable wakeup?

Al
> >
> > >
> > >> +
> > >>       /* 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;
> > >
>
>

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Alan Cooper]

On Wed, Nov 7, 2018 at 8:43 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> hi,
> On Tue, 2018-10-30 at 18:30 -0400, Alan Cooper wrote:
> > 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(-)
> > >>
>
>
> > >>       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.
> Just use device_init_wakeup(dev, true) instead of
> device_wakeup_enable(dev) for your controller driver, you can try it.
>
> Sorry for the late replay

What about the case where sysfs is used to disable wakeup?

Al
> >
> > >
> > >> +
> > >>       /* 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;
> > >
>
>

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Chunfeng Yun]

On Thu, 2018-11-08 at 17:50 -0500, Alan Cooper wrote:
> On Wed, Nov 7, 2018 at 8:43 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > hi,
> > On Tue, 2018-10-30 at 18:30 -0400, Alan Cooper wrote:
> > > 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(-)
> > > >>
> >
> >
> > > >>       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.
> > Just use device_init_wakeup(dev, true) instead of
> > device_wakeup_enable(dev) for your controller driver, you can try it.
> >
> > Sorry for the late replay
> 
> What about the case where sysfs is used to disable wakeup?
Oh, It's a problem in this case.

BTW: Why the host driver will crash when access host controller's
register after power-off phy? they are usually independent.

> 
> Al
> > >
> > > >
> > > >> +
> > > >>       /* 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;
> > > >
> >
> >

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Chunfeng Yun]

On Thu, 2018-11-08 at 17:50 -0500, Alan Cooper wrote:
> On Wed, Nov 7, 2018 at 8:43 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > hi,
> > On Tue, 2018-10-30 at 18:30 -0400, Alan Cooper wrote:
> > > 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(-)
> > > >>
> >
> >
> > > >>       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.
> > Just use device_init_wakeup(dev, true) instead of
> > device_wakeup_enable(dev) for your controller driver, you can try it.
> >
> > Sorry for the late replay
> 
> What about the case where sysfs is used to disable wakeup?
Oh, It's a problem in this case.

BTW: Why the host driver will crash when access host controller's
register after power-off phy? they are usually independent.

> 
> Al
> > >
> > > >
> > > >> +
> > > >>       /* 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;
> > > >
> >
> >

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Alan Cooper]

On Fri, Nov 9, 2018 at 3:08 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Thu, 2018-11-08 at 17:50 -0500, Alan Cooper wrote:
> > On Wed, Nov 7, 2018 at 8:43 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > hi,
> > > On Tue, 2018-10-30 at 18:30 -0400, Alan Cooper wrote:
> > > > 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(-)
> > > > >>
> > >
> > >
> > > > >>       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.
> > > Just use device_init_wakeup(dev, true) instead of
> > > device_wakeup_enable(dev) for your controller driver, you can try it.
> > >
> > > Sorry for the late replay
> >
> > What about the case where sysfs is used to disable wakeup?
> Oh, It's a problem in this case.
>
> BTW: Why the host driver will crash when access host controller's
> register after power-off phy? they are usually independent.

Unfortunately, they are not independent in our design. The PHY
supplies the clock needed to read/write the OHCI, EHCI and XHCI
registers.

Al

>
> >
> > Al
> > > >
> > > > >
> > > > >> +
> > > > >>       /* 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;
> > > > >
> > >
> > >
>
>

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Alan Cooper]

On Fri, Nov 9, 2018 at 3:08 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Thu, 2018-11-08 at 17:50 -0500, Alan Cooper wrote:
> > On Wed, Nov 7, 2018 at 8:43 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > hi,
> > > On Tue, 2018-10-30 at 18:30 -0400, Alan Cooper wrote:
> > > > 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(-)
> > > > >>
> > >
> > >
> > > > >>       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.
> > > Just use device_init_wakeup(dev, true) instead of
> > > device_wakeup_enable(dev) for your controller driver, you can try it.
> > >
> > > Sorry for the late replay
> >
> > What about the case where sysfs is used to disable wakeup?
> Oh, It's a problem in this case.
>
> BTW: Why the host driver will crash when access host controller's
> register after power-off phy? they are usually independent.

Unfortunately, they are not independent in our design. The PHY
supplies the clock needed to read/write the OHCI, EHCI and XHCI
registers.

Al

>
> >
> > Al
> > > >
> > > > >
> > > > >> +
> > > > >>       /* 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;
> > > > >
> > >
> > >
>
>

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Chunfeng Yun]

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.

> +
>  	/* 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;

Re: [PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Chunfeng Yun]

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.

> +
>  	/* 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;

[PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Al Cooper]

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;
+
 	/* 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;
-- 
1.9.0.138.g2de3478

[PATCH V2 2/6] usb: core: Add ability to skip phy exit on suspend and init on resume

[Al Cooper]

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;
+
 	/* 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;
-- 
1.9.0.138.g2de3478