On Thu, May 8, 2014 at 9:01 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 7 May 2014, Dan Williams wrote:
>
>> Per Alan:
>> "You mean from within hub_handle_remote_wakeup()? That routine will
>> never get called if CONFIG_PM_RUNTIME isn't enabled, because khubd
>> never sees wakeup requests if they arise during system suspend.
>>
>> In fact, that routine ought to go inside the "#ifdef CONFIG_PM_RUNTIME"
>> portion of hub.c, along with the other suspend/resume code."
>>
>> Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>> ---
>> drivers/usb/core/hub.c | 4 ++++
>> drivers/usb/core/usb.h | 5 -----
>> 2 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index d91e6493abf7..45ded752c2d8 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -4700,6 +4700,7 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
>> usb_lock_port(port_dev);
>> }
>>
>> +#ifdef CONFIG_PM_RUNTIME
>> /* Returns 1 if there was a remote wakeup and a connect status change. */
>> static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
>> u16 portstatus, u16 portchange)
>> @@ -4740,6 +4741,7 @@ static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
>> dev_dbg(&port_dev->dev, "resume, status %d\n", ret);
>> return connect_change;
>> }
>> +#endif
>
> I meant that instead of adding a new #ifdef section here, you could
> move the subroutine inside the #ifdef that protects
> usb_remote_wakeup().
>
>> static void port_event(struct usb_hub *hub, int port1)
>> __must_hold(&port_dev->status_lock)
>> @@ -4818,8 +4820,10 @@ static void port_event(struct usb_hub *hub, int port1)
>> if (!pm_runtime_active(&port_dev->dev))
>> return;
>>
>> +#ifdef CONFIG_PM_RUNTIME
>> if (hub_handle_remote_wakeup(hub, port1, portstatus, portchange))
>> connect_change = 1;
>> +#endif
>
> Also, instead of adding another #ifdef here, you could add a #else
> section to the existing #ifdef in which you define an inline version of
> hub_handle_remote_wakeup() (or a macro version) that always returns 0.
I originally started down that path, and then noticed:
#ifdef CONFIG_PM_RUNTIME
} else if (udev->state == USB_STATE_SUSPENDED &&
udev->persist_enabled) {
/* For a suspended device, treat this as a
* remote wakeup event.
*/
usb_unlock_port(port_dev);
status = usb_remote_wakeup(udev);
usb_lock_port(port_dev);
#endif
...right above and made the wrong call to add more ugliness. Will
fix, but will leave the other ifdef excursions alone for now.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
