All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] USB: Increment wakeup count on remote wakeup.
@ 2018-04-19  0:18 ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-19  0:18 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: ravisadineni@chromium.org
---
 drivers/usb/core/hcd.c |  1 +
 drivers/usb/core/hub.c | 12 ++++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..79f95a878fb6e 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2375,6 +2375,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 {
 	unsigned long flags;
 
+	pm_wakeup_event(dev, 0);
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..6abc5be1bcbf5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3432,10 +3437,13 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	usb_lock_port(port_dev);
 
-	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	/* Skip the initial Clear-Suspend step for a remote wakeup */
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* USB: Increment wakeup count on remote wakeup.
@ 2018-04-19  0:18 ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-19  0:18 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: ravisadineni@chromium.org
---
 drivers/usb/core/hcd.c |  1 +
 drivers/usb/core/hub.c | 12 ++++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..79f95a878fb6e 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2375,6 +2375,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 {
 	unsigned long flags;
 
+	pm_wakeup_event(dev, 0);
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..6abc5be1bcbf5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3432,10 +3437,13 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	usb_lock_port(port_dev);
 
-	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	/* Skip the initial Clear-Suspend step for a remote wakeup */
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: Increment wakeup count on remote wakeup.
@ 2018-04-19  9:30   ` kbuild test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kbuild test robot @ 2018-04-19  9:30 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: kbuild-all, stern, gregkh, martin.blumenstingl, ravisadineni,
	chunfeng.yun, johan, arvind.yadav.cs, dtor,
	anton.bondarenko.sama, f.fainelli, keescook, mathias.nyman,
	felipe.balbi, ekorenevsky, peter.chen, joe, tbroch, linux-usb,
	linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

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

Hi Ravi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.17-rc1 next-20180419]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ravi-Chandra-Sadineni/USB-Increment-wakeup-count-on-remote-wakeup/20180419-165317
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-randconfig-x013-201815 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/usb//core/hcd.c: In function 'usb_hcd_resume_root_hub':
>> drivers/usb//core/hcd.c:2378:18: error: 'dev' undeclared (first use in this function); did you mean 'cdev'?
     pm_wakeup_event(dev, 0);
                     ^~~
                     cdev
   drivers/usb//core/hcd.c:2378:18: note: each undeclared identifier is reported only once for each function it appears in

vim +2378 drivers/usb//core/hcd.c

  2364	
  2365	/**
  2366	 * usb_hcd_resume_root_hub - called by HCD to resume its root hub
  2367	 * @hcd: host controller for this root hub
  2368	 *
  2369	 * The USB host controller calls this function when its root hub is
  2370	 * suspended (with the remote wakeup feature enabled) and a remote
  2371	 * wakeup request is received.  The routine submits a workqueue request
  2372	 * to resume the root hub (that is, manage its downstream ports again).
  2373	 */
  2374	void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
  2375	{
  2376		unsigned long flags;
  2377	
> 2378		pm_wakeup_event(dev, 0);
  2379		spin_lock_irqsave (&hcd_root_hub_lock, flags);
  2380		if (hcd->rh_registered) {
  2381			set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
  2382			queue_work(pm_wq, &hcd->wakeup_work);
  2383		}
  2384		spin_unlock_irqrestore (&hcd_root_hub_lock, flags);
  2385	}
  2386	EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
  2387	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29813 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: Increment wakeup count on remote wakeup.
@ 2018-04-19  9:30   ` kbuild test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kbuild test robot @ 2018-04-19  9:30 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: kbuild-all, stern, gregkh, martin.blumenstingl, ravisadineni,
	chunfeng.yun, johan, arvind.yadav.cs, dtor,
	anton.bondarenko.sama, f.fainelli, keescook, mathias.nyman,
	felipe.balbi, ekorenevsky, peter.chen, joe, tbroch, linux-usb,
	linux-kernel, rajatja, bleung

Hi Ravi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.17-rc1 next-20180419]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ravi-Chandra-Sadineni/USB-Increment-wakeup-count-on-remote-wakeup/20180419-165317
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-randconfig-x013-201815 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/usb//core/hcd.c: In function 'usb_hcd_resume_root_hub':
>> drivers/usb//core/hcd.c:2378:18: error: 'dev' undeclared (first use in this function); did you mean 'cdev'?
     pm_wakeup_event(dev, 0);
                     ^~~
                     cdev
   drivers/usb//core/hcd.c:2378:18: note: each undeclared identifier is reported only once for each function it appears in

vim +2378 drivers/usb//core/hcd.c

  2364	
  2365	/**
  2366	 * usb_hcd_resume_root_hub - called by HCD to resume its root hub
  2367	 * @hcd: host controller for this root hub
  2368	 *
  2369	 * The USB host controller calls this function when its root hub is
  2370	 * suspended (with the remote wakeup feature enabled) and a remote
  2371	 * wakeup request is received.  The routine submits a workqueue request
  2372	 * to resume the root hub (that is, manage its downstream ports again).
  2373	 */
  2374	void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
  2375	{
  2376		unsigned long flags;
  2377	
> 2378		pm_wakeup_event(dev, 0);
  2379		spin_lock_irqsave (&hcd_root_hub_lock, flags);
  2380		if (hcd->rh_registered) {
  2381			set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
  2382			queue_work(pm_wq, &hcd->wakeup_work);
  2383		}
  2384		spin_unlock_irqrestore (&hcd_root_hub_lock, flags);
  2385	}
  2386	EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
  2387
---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: Increment wakeup count on remote wakeup.
@ 2018-04-19  9:33   ` kbuild test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kbuild test robot @ 2018-04-19  9:33 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: kbuild-all, stern, gregkh, martin.blumenstingl, ravisadineni,
	chunfeng.yun, johan, arvind.yadav.cs, dtor,
	anton.bondarenko.sama, f.fainelli, keescook, mathias.nyman,
	felipe.balbi, ekorenevsky, peter.chen, joe, tbroch, linux-usb,
	linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

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

Hi Ravi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.17-rc1 next-20180419]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ravi-Chandra-Sadineni/USB-Increment-wakeup-count-on-remote-wakeup/20180419-165317
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-randconfig-s0-201815 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/usb/core/hcd.c: In function 'usb_hcd_resume_root_hub':
>> drivers/usb/core/hcd.c:2378:18: error: 'dev' undeclared (first use in this function)
     pm_wakeup_event(dev, 0);
                     ^~~
   drivers/usb/core/hcd.c:2378:18: note: each undeclared identifier is reported only once for each function it appears in

vim +/dev +2378 drivers/usb/core/hcd.c

  2364	
  2365	/**
  2366	 * usb_hcd_resume_root_hub - called by HCD to resume its root hub
  2367	 * @hcd: host controller for this root hub
  2368	 *
  2369	 * The USB host controller calls this function when its root hub is
  2370	 * suspended (with the remote wakeup feature enabled) and a remote
  2371	 * wakeup request is received.  The routine submits a workqueue request
  2372	 * to resume the root hub (that is, manage its downstream ports again).
  2373	 */
  2374	void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
  2375	{
  2376		unsigned long flags;
  2377	
> 2378		pm_wakeup_event(dev, 0);
  2379		spin_lock_irqsave (&hcd_root_hub_lock, flags);
  2380		if (hcd->rh_registered) {
  2381			set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
  2382			queue_work(pm_wq, &hcd->wakeup_work);
  2383		}
  2384		spin_unlock_irqrestore (&hcd_root_hub_lock, flags);
  2385	}
  2386	EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
  2387	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31325 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: Increment wakeup count on remote wakeup.
@ 2018-04-19  9:33   ` kbuild test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kbuild test robot @ 2018-04-19  9:33 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: kbuild-all, stern, gregkh, martin.blumenstingl, ravisadineni,
	chunfeng.yun, johan, arvind.yadav.cs, dtor,
	anton.bondarenko.sama, f.fainelli, keescook, mathias.nyman,
	felipe.balbi, ekorenevsky, peter.chen, joe, tbroch, linux-usb,
	linux-kernel, rajatja, bleung

Hi Ravi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.17-rc1 next-20180419]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ravi-Chandra-Sadineni/USB-Increment-wakeup-count-on-remote-wakeup/20180419-165317
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-randconfig-s0-201815 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/usb/core/hcd.c: In function 'usb_hcd_resume_root_hub':
>> drivers/usb/core/hcd.c:2378:18: error: 'dev' undeclared (first use in this function)
     pm_wakeup_event(dev, 0);
                     ^~~
   drivers/usb/core/hcd.c:2378:18: note: each undeclared identifier is reported only once for each function it appears in

vim +/dev +2378 drivers/usb/core/hcd.c

  2364	
  2365	/**
  2366	 * usb_hcd_resume_root_hub - called by HCD to resume its root hub
  2367	 * @hcd: host controller for this root hub
  2368	 *
  2369	 * The USB host controller calls this function when its root hub is
  2370	 * suspended (with the remote wakeup feature enabled) and a remote
  2371	 * wakeup request is received.  The routine submits a workqueue request
  2372	 * to resume the root hub (that is, manage its downstream ports again).
  2373	 */
  2374	void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
  2375	{
  2376		unsigned long flags;
  2377	
> 2378		pm_wakeup_event(dev, 0);
  2379		spin_lock_irqsave (&hcd_root_hub_lock, flags);
  2380		if (hcd->rh_registered) {
  2381			set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
  2382			queue_work(pm_wq, &hcd->wakeup_work);
  2383		}
  2384		spin_unlock_irqrestore (&hcd_root_hub_lock, flags);
  2385	}
  2386	EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
  2387
---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: Increment wakeup count on remote wakeup.
@ 2018-04-19 15:01   ` Alan Stern
  0 siblings, 0 replies; 34+ messages in thread
From: Alan Stern @ 2018-04-19 15:01 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Wed, 18 Apr 2018, Ravi Chandra Sadineni wrote:

> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: ravisadineni@chromium.org
> ---
>  drivers/usb/core/hcd.c |  1 +
>  drivers/usb/core/hub.c | 12 ++++++++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 777036ae63674..79f95a878fb6e 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2375,6 +2375,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>  {
>  	unsigned long flags;
>  
> +	pm_wakeup_event(dev, 0);

Instead of dev, you probably want to use hcd->self.sysdev.  Or maybe
hcd->self.controller, although the difference probably doesn't matter 
for your purposes.

On the other hand, this wakeup event may already have been counted by
the host controller's bus subsystem.  Does it matter if the same wakeup
event gets counted twice?

(This is inevitable with USB devices, in any case.  If a device sends a 
wakeup request, it will be counted for that device, for all the 
intermediate hubs, and for the host controller.)

> @@ -3432,10 +3437,13 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>  
>  	usb_lock_port(port_dev);
>  
> -	/* Skip the initial Clear-Suspend step for a remote wakeup */
>  	status = hub_port_status(hub, port1, &portstatus, &portchange);
> -	if (status == 0 && !port_is_suspended(hub, portstatus))
> +	/* Skip the initial Clear-Suspend step for a remote wakeup */

What is the reason for moving the comment line down after the 
hub_port_status() call?

Alan Stern

> +	if (status == 0 && !port_is_suspended(hub, portstatus)) {
> +		if (portchange & USB_PORT_STAT_C_SUSPEND)
> +			pm_wakeup_event(&udev->dev, 0);
>  		goto SuspendCleared;
> +	}
>  
>  	/* see 7.1.7.7; affects power usage, but not budgeting */
>  	if (hub_is_superspeed(hub->hdev))
> 


^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: Increment wakeup count on remote wakeup.
@ 2018-04-19 15:01   ` Alan Stern
  0 siblings, 0 replies; 34+ messages in thread
From: Alan Stern @ 2018-04-19 15:01 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Wed, 18 Apr 2018, Ravi Chandra Sadineni wrote:

> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: ravisadineni@chromium.org
> ---
>  drivers/usb/core/hcd.c |  1 +
>  drivers/usb/core/hub.c | 12 ++++++++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 777036ae63674..79f95a878fb6e 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2375,6 +2375,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>  {
>  	unsigned long flags;
>  
> +	pm_wakeup_event(dev, 0);

Instead of dev, you probably want to use hcd->self.sysdev.  Or maybe
hcd->self.controller, although the difference probably doesn't matter 
for your purposes.

On the other hand, this wakeup event may already have been counted by
the host controller's bus subsystem.  Does it matter if the same wakeup
event gets counted twice?

(This is inevitable with USB devices, in any case.  If a device sends a 
wakeup request, it will be counted for that device, for all the 
intermediate hubs, and for the host controller.)

> @@ -3432,10 +3437,13 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>  
>  	usb_lock_port(port_dev);
>  
> -	/* Skip the initial Clear-Suspend step for a remote wakeup */
>  	status = hub_port_status(hub, port1, &portstatus, &portchange);
> -	if (status == 0 && !port_is_suspended(hub, portstatus))
> +	/* Skip the initial Clear-Suspend step for a remote wakeup */

What is the reason for moving the comment line down after the 
hub_port_status() call?

Alan Stern

> +	if (status == 0 && !port_is_suspended(hub, portstatus)) {
> +		if (portchange & USB_PORT_STAT_C_SUSPEND)
> +			pm_wakeup_event(&udev->dev, 0);
>  		goto SuspendCleared;
> +	}
>  
>  	/* see 7.1.7.7; affects power usage, but not budgeting */
>  	if (hub_is_superspeed(hub->hdev))
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: Increment wakeup count on remote wakeup.
@ 2018-04-19 16:17     ` Rajat Jain
  0 siblings, 0 replies; 34+ messages in thread
From: Rajat Jain @ 2018-04-19 16:17 UTC (permalink / raw)
  To: Alan Stern
  Cc: Ravi Chandra Sadineni, Greg Kroah-Hartman, martin.blumenstingl,
	Ravi Chandra Sadineni, chunfeng.yun, johan, arvind.yadav.cs,
	Dmitry Torokhov, anton.bondarenko.sama, f.fainelli, keescook,
	mathias.nyman, felipe.balbi, ekorenevsky, peter.chen, joe,
	Todd Broch, linux-usb, Linux Kernel Mailing List, Benson Leung

On Thu, Apr 19, 2018 at 8:01 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 18 Apr 2018, Ravi Chandra Sadineni wrote:
>
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: ravisadineni@chromium.org
>> ---
>>  drivers/usb/core/hcd.c |  1 +
>>  drivers/usb/core/hub.c | 12 ++++++++++--
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 777036ae63674..79f95a878fb6e 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2375,6 +2375,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>>  {
>>       unsigned long flags;
>>
>> +     pm_wakeup_event(dev, 0);
>
> Instead of dev, you probably want to use hcd->self.sysdev.  Or maybe
> hcd->self.controller, although the difference probably doesn't matter
> for your purposes.
>
> On the other hand, this wakeup event may already have been counted by
> the host controller's bus subsystem.  Does it matter if the same wakeup
> event gets counted twice?

No. The context is that we're interested in making user space behave
differently if the wake up source was a user input device (e.g. USB
keyboard) v/s some other kind of USB device. So all that matters is
that the USB device wakeup count gets incremented.

>
> (This is inevitable with USB devices, in any case.  If a device sends a
> wakeup request, it will be counted for that device, for all the
> intermediate hubs, and for the host controller.)
>
>> @@ -3432,10 +3437,13 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>
>>       usb_lock_port(port_dev);
>>
>> -     /* Skip the initial Clear-Suspend step for a remote wakeup */
>>       status = hub_port_status(hub, port1, &portstatus, &portchange);
>> -     if (status == 0 && !port_is_suspended(hub, portstatus))
>> +     /* Skip the initial Clear-Suspend step for a remote wakeup */
>
> What is the reason for moving the comment line down after the
> hub_port_status() call?
>
> Alan Stern
>
>> +     if (status == 0 && !port_is_suspended(hub, portstatus)) {
>> +             if (portchange & USB_PORT_STAT_C_SUSPEND)
>> +                     pm_wakeup_event(&udev->dev, 0);
>>               goto SuspendCleared;
>> +     }
>>
>>       /* see 7.1.7.7; affects power usage, but not budgeting */
>>       if (hub_is_superspeed(hub->hdev))
>>
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: Increment wakeup count on remote wakeup.
@ 2018-04-19 16:17     ` Rajat Jain
  0 siblings, 0 replies; 34+ messages in thread
From: Rajat Jain @ 2018-04-19 16:17 UTC (permalink / raw)
  To: Alan Stern
  Cc: Ravi Chandra Sadineni, Greg Kroah-Hartman, martin.blumenstingl,
	Ravi Chandra Sadineni, chunfeng.yun, johan, arvind.yadav.cs,
	Dmitry Torokhov, anton.bondarenko.sama, f.fainelli, keescook,
	mathias.nyman, felipe.balbi, ekorenevsky, peter.chen, joe,
	Todd Broch, linux-usb, Linux Kernel Mailing List, Benson Leung

On Thu, Apr 19, 2018 at 8:01 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 18 Apr 2018, Ravi Chandra Sadineni wrote:
>
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: ravisadineni@chromium.org
>> ---
>>  drivers/usb/core/hcd.c |  1 +
>>  drivers/usb/core/hub.c | 12 ++++++++++--
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 777036ae63674..79f95a878fb6e 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2375,6 +2375,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>>  {
>>       unsigned long flags;
>>
>> +     pm_wakeup_event(dev, 0);
>
> Instead of dev, you probably want to use hcd->self.sysdev.  Or maybe
> hcd->self.controller, although the difference probably doesn't matter
> for your purposes.
>
> On the other hand, this wakeup event may already have been counted by
> the host controller's bus subsystem.  Does it matter if the same wakeup
> event gets counted twice?

No. The context is that we're interested in making user space behave
differently if the wake up source was a user input device (e.g. USB
keyboard) v/s some other kind of USB device. So all that matters is
that the USB device wakeup count gets incremented.

>
> (This is inevitable with USB devices, in any case.  If a device sends a
> wakeup request, it will be counted for that device, for all the
> intermediate hubs, and for the host controller.)
>
>> @@ -3432,10 +3437,13 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>
>>       usb_lock_port(port_dev);
>>
>> -     /* Skip the initial Clear-Suspend step for a remote wakeup */
>>       status = hub_port_status(hub, port1, &portstatus, &portchange);
>> -     if (status == 0 && !port_is_suspended(hub, portstatus))
>> +     /* Skip the initial Clear-Suspend step for a remote wakeup */
>
> What is the reason for moving the comment line down after the
> hub_port_status() call?
>
> Alan Stern
>
>> +     if (status == 0 && !port_is_suspended(hub, portstatus)) {
>> +             if (portchange & USB_PORT_STAT_C_SUSPEND)
>> +                     pm_wakeup_event(&udev->dev, 0);
>>               goto SuspendCleared;
>> +     }
>>
>>       /* see 7.1.7.7; affects power usage, but not budgeting */
>>       if (hub_is_superspeed(hub->hdev))
>>
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH V2] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20  0:27     ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20  0:27 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
 drivers/usb/core/hcd.c |  2 ++
 drivers/usb/core/hub.c | 10 +++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..ee0f3ec57ce49 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2374,7 +2374,9 @@ static void hcd_resume_work(struct work_struct *work)
 void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 {
 	unsigned long flags;
+	struct device *dev = &hcd->self.root_hub->dev;
 
+	pm_wakeup_event(dev, 0);
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..aa9968d90a48c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))
-- 
2.13.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [V2] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20  0:27     ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20  0:27 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
 drivers/usb/core/hcd.c |  2 ++
 drivers/usb/core/hub.c | 10 +++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..ee0f3ec57ce49 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2374,7 +2374,9 @@ static void hcd_resume_work(struct work_struct *work)
 void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 {
 	unsigned long flags;
+	struct device *dev = &hcd->self.root_hub->dev;
 
+	pm_wakeup_event(dev, 0);
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..aa9968d90a48c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20  0:50     ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20  0:50 UTC (permalink / raw)
  To: Alan Stern
  Cc: Ravi Chandra Sadineni, gregkh, martin.blumenstingl, chunfeng.yun,
	johan, arvind.yadav.cs, Dmitry Torokhov, anton.bondarenko.sama,
	f.fainelli, keescook, mathias.nyman, felipe.balbi, ekorenevsky,
	peter.chen, joe, Todd Broch, linux-usb, linux-kernel, Rajat Jain,
	Benson Leung

Hi Alan,
Thanks for reviewing the change. Appreciate your time.  I tried to
address your comments in the V2 of the patch.

On Thu, Apr 19, 2018 at 8:01 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 18 Apr 2018, Ravi Chandra Sadineni wrote:
>
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: ravisadineni@chromium.org
>> ---
>>  drivers/usb/core/hcd.c |  1 +
>>  drivers/usb/core/hub.c | 12 ++++++++++--
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 777036ae63674..79f95a878fb6e 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2375,6 +2375,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>>  {
>>       unsigned long flags;
>>
>> +     pm_wakeup_event(dev, 0);
>
> Instead of dev, you probably want to use hcd->self.sysdev.  Or maybe
> hcd->self.controller, although the difference probably doesn't matter
> for your purposes.

Trying to increment the wakeup count for the roothub. So pointed dev
to &hcd->self.root_hub->dev. Hope this is fine.

>
> On the other hand, this wakeup event may already have been counted by
> the host controller's bus subsystem.  Does it matter if the same wakeup
> event gets counted twice?
>
> (This is inevitable with USB devices, in any case.  If a device sends a
> wakeup request, it will be counted for that device, for all the
> intermediate hubs, and for the host controller.)

We are o.k. with the wake-up count getting incremented for the
intermediate hubs. We just want to identify the leaf hid devices, if
they are cause of the remote wake. This way, we can differentiate user
triggered wake from a automatic wakes (Ex: wakes triggered by WOLAN
packets from USB ethernet adapter).

We are also o.k. with the wake-up count getting incremented twice. All
we look for is a change in the wake-up count for the interested
devices.
>
>> @@ -3432,10 +3437,13 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>
>>       usb_lock_port(port_dev);
>>
>> -     /* Skip the initial Clear-Suspend step for a remote wakeup */
>>       status = hub_port_status(hub, port1, &portstatus, &portchange);
>> -     if (status == 0 && !port_is_suspended(hub, portstatus))
>> +     /* Skip the initial Clear-Suspend step for a remote wakeup */
>
> What is the reason for moving the comment line down after the
> hub_port_status() call?
Sorry. This was a mistake. Changed this back in V2.
>
> Alan Stern
>
>> +     if (status == 0 && !port_is_suspended(hub, portstatus)) {
>> +             if (portchange & USB_PORT_STAT_C_SUSPEND)
>> +                     pm_wakeup_event(&udev->dev, 0);
>>               goto SuspendCleared;
>> +     }
>>
>>       /* see 7.1.7.7; affects power usage, but not budgeting */
>>       if (hub_is_superspeed(hub->hdev))
>>
>

Thanks,
Ravi

^ permalink raw reply	[flat|nested] 34+ messages in thread

* USB: Increment wakeup count on remote wakeup.
@ 2018-04-20  0:50     ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20  0:50 UTC (permalink / raw)
  To: Alan Stern
  Cc: Ravi Chandra Sadineni, gregkh, martin.blumenstingl, chunfeng.yun,
	johan, arvind.yadav.cs, Dmitry Torokhov, anton.bondarenko.sama,
	f.fainelli, keescook, mathias.nyman, felipe.balbi, ekorenevsky,
	peter.chen, joe, Todd Broch, linux-usb, linux-kernel, Rajat Jain,
	Benson Leung

Hi Alan,
Thanks for reviewing the change. Appreciate your time.  I tried to
address your comments in the V2 of the patch.

On Thu, Apr 19, 2018 at 8:01 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 18 Apr 2018, Ravi Chandra Sadineni wrote:
>
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: ravisadineni@chromium.org
>> ---
>>  drivers/usb/core/hcd.c |  1 +
>>  drivers/usb/core/hub.c | 12 ++++++++++--
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 777036ae63674..79f95a878fb6e 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2375,6 +2375,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>>  {
>>       unsigned long flags;
>>
>> +     pm_wakeup_event(dev, 0);
>
> Instead of dev, you probably want to use hcd->self.sysdev.  Or maybe
> hcd->self.controller, although the difference probably doesn't matter
> for your purposes.

Trying to increment the wakeup count for the roothub. So pointed dev
to &hcd->self.root_hub->dev. Hope this is fine.

>
> On the other hand, this wakeup event may already have been counted by
> the host controller's bus subsystem.  Does it matter if the same wakeup
> event gets counted twice?
>
> (This is inevitable with USB devices, in any case.  If a device sends a
> wakeup request, it will be counted for that device, for all the
> intermediate hubs, and for the host controller.)

We are o.k. with the wake-up count getting incremented for the
intermediate hubs. We just want to identify the leaf hid devices, if
they are cause of the remote wake. This way, we can differentiate user
triggered wake from a automatic wakes (Ex: wakes triggered by WOLAN
packets from USB ethernet adapter).

We are also o.k. with the wake-up count getting incremented twice. All
we look for is a change in the wake-up count for the interested
devices.
>
>> @@ -3432,10 +3437,13 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>
>>       usb_lock_port(port_dev);
>>
>> -     /* Skip the initial Clear-Suspend step for a remote wakeup */
>>       status = hub_port_status(hub, port1, &portstatus, &portchange);
>> -     if (status == 0 && !port_is_suspended(hub, portstatus))
>> +     /* Skip the initial Clear-Suspend step for a remote wakeup */
>
> What is the reason for moving the comment line down after the
> hub_port_status() call?
Sorry. This was a mistake. Changed this back in V2.
>
> Alan Stern
>
>> +     if (status == 0 && !port_is_suspended(hub, portstatus)) {
>> +             if (portchange & USB_PORT_STAT_C_SUSPEND)
>> +                     pm_wakeup_event(&udev->dev, 0);
>>               goto SuspendCleared;
>> +     }
>>
>>       /* see 7.1.7.7; affects power usage, but not budgeting */
>>       if (hub_is_superspeed(hub->hdev))
>>
>

Thanks,
Ravi
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V2] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 14:12       ` Alan Stern
  0 siblings, 0 replies; 34+ messages in thread
From: Alan Stern @ 2018-04-20 14:12 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Thu, 19 Apr 2018, Ravi Chandra Sadineni wrote:

> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
>  drivers/usb/core/hcd.c |  2 ++
>  drivers/usb/core/hub.c | 10 +++++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 777036ae63674..ee0f3ec57ce49 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2374,7 +2374,9 @@ static void hcd_resume_work(struct work_struct *work)
>  void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>  {
>  	unsigned long flags;
> +	struct device *dev = &hcd->self.root_hub->dev;
>  
> +	pm_wakeup_event(dev, 0);
>  	spin_lock_irqsave (&hcd_root_hub_lock, flags);
>  	if (hcd->rh_registered) {
>  		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);

In general, hcd->self.root_hub is guaranteed to be non-NULL only when 
hcd->rh_registered is set.  Therefore the assignment to dev and the 
call to pm_wakeup_event() should be moved within this "if" statement.

The rest of the patch looks okay.

Alan Stern

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index f6ea16e9f6bb9..aa9968d90a48c 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
>  		unsigned int portnum)
>  {
>  	struct usb_hub *hub;
> +	struct usb_port *port_dev;
>  
>  	if (!hdev)
>  		return;
>  
>  	hub = usb_hub_to_struct_hub(hdev);
>  	if (hub) {
> +		port_dev = hub->ports[portnum - 1];
> +		if (port_dev && port_dev->child)
> +			pm_wakeup_event(&port_dev->child->dev, 0);
> +
>  		set_bit(portnum, hub->wakeup_bits);
>  		kick_hub_wq(hub);
>  	}
> @@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>  
>  	/* Skip the initial Clear-Suspend step for a remote wakeup */
>  	status = hub_port_status(hub, port1, &portstatus, &portchange);
> -	if (status == 0 && !port_is_suspended(hub, portstatus))
> +	if (status == 0 && !port_is_suspended(hub, portstatus)) {
> +		if (portchange & USB_PORT_STAT_C_SUSPEND)
> +			pm_wakeup_event(&udev->dev, 0);
>  		goto SuspendCleared;
> +	}
>  
>  	/* see 7.1.7.7; affects power usage, but not budgeting */
>  	if (hub_is_superspeed(hub->hdev))
> 


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [V2] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 14:12       ` Alan Stern
  0 siblings, 0 replies; 34+ messages in thread
From: Alan Stern @ 2018-04-20 14:12 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Thu, 19 Apr 2018, Ravi Chandra Sadineni wrote:

> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
>  drivers/usb/core/hcd.c |  2 ++
>  drivers/usb/core/hub.c | 10 +++++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 777036ae63674..ee0f3ec57ce49 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2374,7 +2374,9 @@ static void hcd_resume_work(struct work_struct *work)
>  void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>  {
>  	unsigned long flags;
> +	struct device *dev = &hcd->self.root_hub->dev;
>  
> +	pm_wakeup_event(dev, 0);
>  	spin_lock_irqsave (&hcd_root_hub_lock, flags);
>  	if (hcd->rh_registered) {
>  		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);

In general, hcd->self.root_hub is guaranteed to be non-NULL only when 
hcd->rh_registered is set.  Therefore the assignment to dev and the 
call to pm_wakeup_event() should be moved within this "if" statement.

The rest of the patch looks okay.

Alan Stern

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index f6ea16e9f6bb9..aa9968d90a48c 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
>  		unsigned int portnum)
>  {
>  	struct usb_hub *hub;
> +	struct usb_port *port_dev;
>  
>  	if (!hdev)
>  		return;
>  
>  	hub = usb_hub_to_struct_hub(hdev);
>  	if (hub) {
> +		port_dev = hub->ports[portnum - 1];
> +		if (port_dev && port_dev->child)
> +			pm_wakeup_event(&port_dev->child->dev, 0);
> +
>  		set_bit(portnum, hub->wakeup_bits);
>  		kick_hub_wq(hub);
>  	}
> @@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>  
>  	/* Skip the initial Clear-Suspend step for a remote wakeup */
>  	status = hub_port_status(hub, port1, &portstatus, &portchange);
> -	if (status == 0 && !port_is_suspended(hub, portstatus))
> +	if (status == 0 && !port_is_suspended(hub, portstatus)) {
> +		if (portchange & USB_PORT_STAT_C_SUSPEND)
> +			pm_wakeup_event(&udev->dev, 0);
>  		goto SuspendCleared;
> +	}
>  
>  	/* see 7.1.7.7; affects power usage, but not budgeting */
>  	if (hub_is_superspeed(hub->hdev))
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH V3] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 17:05         ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 17:05 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
 drivers/usb/core/hcd.c |  2 ++
 drivers/usb/core/hub.c | 10 +++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..b8024ae4fdcaa 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2375,6 +2375,8 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 {
 	unsigned long flags;
 
+	if (hcd->rh_registered)
+		pm_wakeup_event(&hcd->self.root_hub->dev, 0);
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..aa9968d90a48c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))
-- 
2.17.0.484.g0c8726318c-goog


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [V3] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 17:05         ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 17:05 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
 drivers/usb/core/hcd.c |  2 ++
 drivers/usb/core/hub.c | 10 +++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..b8024ae4fdcaa 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2375,6 +2375,8 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 {
 	unsigned long flags;
 
+	if (hcd->rh_registered)
+		pm_wakeup_event(&hcd->self.root_hub->dev, 0);
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..aa9968d90a48c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH V2] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 17:07         ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 17:07 UTC (permalink / raw)
  To: Alan Stern
  Cc: Ravi Chandra Sadineni, gregkh, martin.blumenstingl, chunfeng.yun,
	johan, arvind.yadav.cs, Dmitry Torokhov, anton.bondarenko.sama,
	f.fainelli, keescook, mathias.nyman, felipe.balbi, ekorenevsky,
	peter.chen, joe, Todd Broch, linux-usb, linux-kernel, Rajat Jain,
	Benson Leung

On Fri, Apr 20, 2018 at 7:12 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 19 Apr 2018, Ravi Chandra Sadineni wrote:
>
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
>> ---
>>  drivers/usb/core/hcd.c |  2 ++
>>  drivers/usb/core/hub.c | 10 +++++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 777036ae63674..ee0f3ec57ce49 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2374,7 +2374,9 @@ static void hcd_resume_work(struct work_struct *work)
>>  void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>>  {
>>       unsigned long flags;
>> +     struct device *dev = &hcd->self.root_hub->dev;
>>
>> +     pm_wakeup_event(dev, 0);
>>       spin_lock_irqsave (&hcd_root_hub_lock, flags);
>>       if (hcd->rh_registered) {
>>               set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
>
> In general, hcd->self.root_hub is guaranteed to be non-NULL only when
> hcd->rh_registered is set.  Therefore the assignment to dev and the
> call to pm_wakeup_event() should be moved within this "if" statement.
>
> The rest of the patch looks okay.

Added the check in V3.  Thanks.
>
> Alan Stern
>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index f6ea16e9f6bb9..aa9968d90a48c 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
>>               unsigned int portnum)
>>  {
>>       struct usb_hub *hub;
>> +     struct usb_port *port_dev;
>>
>>       if (!hdev)
>>               return;
>>
>>       hub = usb_hub_to_struct_hub(hdev);
>>       if (hub) {
>> +             port_dev = hub->ports[portnum - 1];
>> +             if (port_dev && port_dev->child)
>> +                     pm_wakeup_event(&port_dev->child->dev, 0);
>> +
>>               set_bit(portnum, hub->wakeup_bits);
>>               kick_hub_wq(hub);
>>       }
>> @@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>
>>       /* Skip the initial Clear-Suspend step for a remote wakeup */
>>       status = hub_port_status(hub, port1, &portstatus, &portchange);
>> -     if (status == 0 && !port_is_suspended(hub, portstatus))
>> +     if (status == 0 && !port_is_suspended(hub, portstatus)) {
>> +             if (portchange & USB_PORT_STAT_C_SUSPEND)
>> +                     pm_wakeup_event(&udev->dev, 0);
>>               goto SuspendCleared;
>> +     }
>>
>>       /* see 7.1.7.7; affects power usage, but not budgeting */
>>       if (hub_is_superspeed(hub->hdev))
>>
>

Thanks,
Ravi

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [V2] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 17:07         ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 17:07 UTC (permalink / raw)
  To: Alan Stern
  Cc: Ravi Chandra Sadineni, gregkh, martin.blumenstingl, chunfeng.yun,
	johan, arvind.yadav.cs, Dmitry Torokhov, anton.bondarenko.sama,
	f.fainelli, keescook, mathias.nyman, felipe.balbi, ekorenevsky,
	peter.chen, joe, Todd Broch, linux-usb, linux-kernel, Rajat Jain,
	Benson Leung

On Fri, Apr 20, 2018 at 7:12 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 19 Apr 2018, Ravi Chandra Sadineni wrote:
>
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
>> ---
>>  drivers/usb/core/hcd.c |  2 ++
>>  drivers/usb/core/hub.c | 10 +++++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 777036ae63674..ee0f3ec57ce49 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2374,7 +2374,9 @@ static void hcd_resume_work(struct work_struct *work)
>>  void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>>  {
>>       unsigned long flags;
>> +     struct device *dev = &hcd->self.root_hub->dev;
>>
>> +     pm_wakeup_event(dev, 0);
>>       spin_lock_irqsave (&hcd_root_hub_lock, flags);
>>       if (hcd->rh_registered) {
>>               set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
>
> In general, hcd->self.root_hub is guaranteed to be non-NULL only when
> hcd->rh_registered is set.  Therefore the assignment to dev and the
> call to pm_wakeup_event() should be moved within this "if" statement.
>
> The rest of the patch looks okay.

Added the check in V3.  Thanks.
>
> Alan Stern
>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index f6ea16e9f6bb9..aa9968d90a48c 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
>>               unsigned int portnum)
>>  {
>>       struct usb_hub *hub;
>> +     struct usb_port *port_dev;
>>
>>       if (!hdev)
>>               return;
>>
>>       hub = usb_hub_to_struct_hub(hdev);
>>       if (hub) {
>> +             port_dev = hub->ports[portnum - 1];
>> +             if (port_dev && port_dev->child)
>> +                     pm_wakeup_event(&port_dev->child->dev, 0);
>> +
>>               set_bit(portnum, hub->wakeup_bits);
>>               kick_hub_wq(hub);
>>       }
>> @@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>
>>       /* Skip the initial Clear-Suspend step for a remote wakeup */
>>       status = hub_port_status(hub, port1, &portstatus, &portchange);
>> -     if (status == 0 && !port_is_suspended(hub, portstatus))
>> +     if (status == 0 && !port_is_suspended(hub, portstatus)) {
>> +             if (portchange & USB_PORT_STAT_C_SUSPEND)
>> +                     pm_wakeup_event(&udev->dev, 0);
>>               goto SuspendCleared;
>> +     }
>>
>>       /* see 7.1.7.7; affects power usage, but not budgeting */
>>       if (hub_is_superspeed(hub->hdev))
>>
>

Thanks,
Ravi
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V3] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 17:29           ` Alan Stern
  0 siblings, 0 replies; 34+ messages in thread
From: Alan Stern @ 2018-04-20 17:29 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Fri, 20 Apr 2018, Ravi Chandra Sadineni wrote:

> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---

At this point you're supposed to list the differences between this 
patch and the preceding versions.

>  drivers/usb/core/hcd.c |  2 ++
>  drivers/usb/core/hub.c | 10 +++++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 777036ae63674..b8024ae4fdcaa 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2375,6 +2375,8 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>  {
>  	unsigned long flags;
>  
> +	if (hcd->rh_registered)
> +		pm_wakeup_event(&hcd->self.root_hub->dev, 0);
>  	spin_lock_irqsave (&hcd_root_hub_lock, flags);
>  	if (hcd->rh_registered) {
>  		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);

This isn't good enough.  hcd->rh_registered can change at any time;  
it is protected by the hcd_root_hub_lock spinlock.  That's why I said
your code should be moved inside the existing "if" statement.

Alan Stern

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index f6ea16e9f6bb9..aa9968d90a48c 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
>  		unsigned int portnum)
>  {
>  	struct usb_hub *hub;
> +	struct usb_port *port_dev;
>  
>  	if (!hdev)
>  		return;
>  
>  	hub = usb_hub_to_struct_hub(hdev);
>  	if (hub) {
> +		port_dev = hub->ports[portnum - 1];
> +		if (port_dev && port_dev->child)
> +			pm_wakeup_event(&port_dev->child->dev, 0);
> +
>  		set_bit(portnum, hub->wakeup_bits);
>  		kick_hub_wq(hub);
>  	}
> @@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>  
>  	/* Skip the initial Clear-Suspend step for a remote wakeup */
>  	status = hub_port_status(hub, port1, &portstatus, &portchange);
> -	if (status == 0 && !port_is_suspended(hub, portstatus))
> +	if (status == 0 && !port_is_suspended(hub, portstatus)) {
> +		if (portchange & USB_PORT_STAT_C_SUSPEND)
> +			pm_wakeup_event(&udev->dev, 0);
>  		goto SuspendCleared;
> +	}
>  
>  	/* see 7.1.7.7; affects power usage, but not budgeting */
>  	if (hub_is_superspeed(hub->hdev))
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [V3] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 17:29           ` Alan Stern
  0 siblings, 0 replies; 34+ messages in thread
From: Alan Stern @ 2018-04-20 17:29 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Fri, 20 Apr 2018, Ravi Chandra Sadineni wrote:

> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---

At this point you're supposed to list the differences between this 
patch and the preceding versions.

>  drivers/usb/core/hcd.c |  2 ++
>  drivers/usb/core/hub.c | 10 +++++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 777036ae63674..b8024ae4fdcaa 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2375,6 +2375,8 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>  {
>  	unsigned long flags;
>  
> +	if (hcd->rh_registered)
> +		pm_wakeup_event(&hcd->self.root_hub->dev, 0);
>  	spin_lock_irqsave (&hcd_root_hub_lock, flags);
>  	if (hcd->rh_registered) {
>  		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);

This isn't good enough.  hcd->rh_registered can change at any time;  
it is protected by the hcd_root_hub_lock spinlock.  That's why I said
your code should be moved inside the existing "if" statement.

Alan Stern

> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index f6ea16e9f6bb9..aa9968d90a48c 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
>  		unsigned int portnum)
>  {
>  	struct usb_hub *hub;
> +	struct usb_port *port_dev;
>  
>  	if (!hdev)
>  		return;
>  
>  	hub = usb_hub_to_struct_hub(hdev);
>  	if (hub) {
> +		port_dev = hub->ports[portnum - 1];
> +		if (port_dev && port_dev->child)
> +			pm_wakeup_event(&port_dev->child->dev, 0);
> +
>  		set_bit(portnum, hub->wakeup_bits);
>  		kick_hub_wq(hub);
>  	}
> @@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>  
>  	/* Skip the initial Clear-Suspend step for a remote wakeup */
>  	status = hub_port_status(hub, port1, &portstatus, &portchange);
> -	if (status == 0 && !port_is_suspended(hub, portstatus))
> +	if (status == 0 && !port_is_suspended(hub, portstatus)) {
> +		if (portchange & USB_PORT_STAT_C_SUSPEND)
> +			pm_wakeup_event(&udev->dev, 0);
>  		goto SuspendCleared;
> +	}
>  
>  	/* see 7.1.7.7; affects power usage, but not budgeting */
>  	if (hub_is_superspeed(hub->hdev))
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH V4] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 17:54             ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 17:54 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
 drivers/usb/core/hcd.c |  1 +
 drivers/usb/core/hub.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..00bb8417050ff 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2377,6 +2377,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
+		pm_wakeup_event(&hcd->self.root_hub->dev, 0);
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
 		queue_work(pm_wq, &hcd->wakeup_work);
 	}
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..aa9968d90a48c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))
-- 
2.17.0.484.g0c8726318c-goog

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [V4] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 17:54             ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 17:54 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
 drivers/usb/core/hcd.c |  1 +
 drivers/usb/core/hub.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..00bb8417050ff 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2377,6 +2377,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
+		pm_wakeup_event(&hcd->self.root_hub->dev, 0);
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
 		queue_work(pm_wq, &hcd->wakeup_work);
 	}
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..aa9968d90a48c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH V5] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 18:08             ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 18:08 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@vger.kernel.org>
---

V5: Added the description of changes between different versions of patches.
V4: Moved the wakeup count increment logic to the existing if which is
safegaurded by hcd_root_hub_lock spinlock.
V3: Added a gaurd to check if rh_registered is set before accessing
root_hub pointer.
V2: Fixed the build failure error due to uninitialized dev pointer.

drivers/usb/core/hcd.c |  1 +
 drivers/usb/core/hub.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..00bb8417050ff 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2377,6 +2377,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
+		pm_wakeup_event(&hcd->self.root_hub->dev, 0);
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
 		queue_work(pm_wq, &hcd->wakeup_work);
 	}
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..aa9968d90a48c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))
-- 
2.17.0.484.g0c8726318c-goog


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [V5] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 18:08             ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 18:08 UTC (permalink / raw)
  To: stern, gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun,
	johan, arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe
  Cc: tbroch, linux-usb, linux-kernel, rajatja, bleung, Ravi Chandra Sadineni

On chromebooks we depend on wakeup count to identify the wakeup source.
But currently USB devices do not increment the wakeup count when they
trigger the remote wake. This patch addresses the same.

Resume condition is reported differently on USB 2.0 and USB 3.0 devices.

On USB 2.0 devices, a wake capable device, if wake enabled, drives
resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
The upstream facing port then sets C_PORT_SUSPEND bit and reports a
port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
has resumed before driving the resume signal from the host and
C_PORT_SUSPEND is set, then the device attached to the given port might
be the reason for the last system wakeup. Increment the wakeup count for
the same.

On USB 3.0 devices, a function may signal that it wants to exit from device
suspend by sending a Function Wake Device Notification to the host (USB3.0
spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
wakeup count.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---

V5: Added the description of changes between different versions of patches.
V4: Moved the wakeup count increment logic to the existing if which is
safegaurded by hcd_root_hub_lock spinlock.
V3: Added a gaurd to check if rh_registered is set before accessing
root_hub pointer.
V2: Fixed the build failure error due to uninitialized dev pointer.

drivers/usb/core/hcd.c |  1 +
 drivers/usb/core/hub.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae63674..00bb8417050ff 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2377,6 +2377,7 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
+		pm_wakeup_event(&hcd->self.root_hub->dev, 0);
 		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
 		queue_work(pm_wq, &hcd->wakeup_work);
 	}
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f6ea16e9f6bb9..aa9968d90a48c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		unsigned int portnum)
 {
 	struct usb_hub *hub;
+	struct usb_port *port_dev;
 
 	if (!hdev)
 		return;
 
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
+		port_dev = hub->ports[portnum - 1];
+		if (port_dev && port_dev->child)
+			pm_wakeup_event(&port_dev->child->dev, 0);
+
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
@@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 	/* Skip the initial Clear-Suspend step for a remote wakeup */
 	status = hub_port_status(hub, port1, &portstatus, &portchange);
-	if (status == 0 && !port_is_suspended(hub, portstatus))
+	if (status == 0 && !port_is_suspended(hub, portstatus)) {
+		if (portchange & USB_PORT_STAT_C_SUSPEND)
+			pm_wakeup_event(&udev->dev, 0);
 		goto SuspendCleared;
+	}
 
 	/* see 7.1.7.7; affects power usage, but not budgeting */
 	if (hub_is_superspeed(hub->hdev))

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH V3] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 18:12             ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 18:12 UTC (permalink / raw)
  To: Alan Stern
  Cc: Ravi Chandra Sadineni, gregkh, Martin Blumenstingl, chunfeng.yun,
	johan, Arvind Yadav, Dmitry Torokhov, Anton Bondarenko,
	Florian Fainelli, Kees Cook, mathias.nyman, felipe.balbi,
	Eugene Korenevsky, peter.chen, joe, Todd Broch, linux-usb,
	linux-kernel, Rajat Jain, Benson Leung

On Fri, Apr 20, 2018 at 10:29 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Fri, 20 Apr 2018, Ravi Chandra Sadineni wrote:
>
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
>> ---
>
> At this point you're supposed to list the differences between this
> patch and the preceding versions.

Mentioned the changes between different versions in V5. Thanks.

>
>>  drivers/usb/core/hcd.c |  2 ++
>>  drivers/usb/core/hub.c | 10 +++++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 777036ae63674..b8024ae4fdcaa 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2375,6 +2375,8 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>>  {
>>       unsigned long flags;
>>
>> +     if (hcd->rh_registered)
>> +             pm_wakeup_event(&hcd->self.root_hub->dev, 0);
>>       spin_lock_irqsave (&hcd_root_hub_lock, flags);
>>       if (hcd->rh_registered) {
>>               set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
>
> This isn't good enough.  hcd->rh_registered can change at any time;
> it is protected by the hcd_root_hub_lock spinlock.  That's why I said
> your code should be moved inside the existing "if" statement.

Sorry about this.  Fixed it now. Hope this is fine. Thanks.
>
> Alan Stern
>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index f6ea16e9f6bb9..aa9968d90a48c 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
>>               unsigned int portnum)
>>  {
>>       struct usb_hub *hub;
>> +     struct usb_port *port_dev;
>>
>>       if (!hdev)
>>               return;
>>
>>       hub = usb_hub_to_struct_hub(hdev);
>>       if (hub) {
>> +             port_dev = hub->ports[portnum - 1];
>> +             if (port_dev && port_dev->child)
>> +                     pm_wakeup_event(&port_dev->child->dev, 0);
>> +
>>               set_bit(portnum, hub->wakeup_bits);
>>               kick_hub_wq(hub);
>>       }
>> @@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>
>>       /* Skip the initial Clear-Suspend step for a remote wakeup */
>>       status = hub_port_status(hub, port1, &portstatus, &portchange);
>> -     if (status == 0 && !port_is_suspended(hub, portstatus))
>> +     if (status == 0 && !port_is_suspended(hub, portstatus)) {
>> +             if (portchange & USB_PORT_STAT_C_SUSPEND)
>> +                     pm_wakeup_event(&udev->dev, 0);
>>               goto SuspendCleared;
>> +     }
>>
>>       /* see 7.1.7.7; affects power usage, but not budgeting */
>>       if (hub_is_superspeed(hub->hdev))
>>
>
Thanks,
Ravi

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [V3] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 18:12             ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-20 18:12 UTC (permalink / raw)
  To: Alan Stern
  Cc: Ravi Chandra Sadineni, gregkh, Martin Blumenstingl, chunfeng.yun,
	johan, Arvind Yadav, Dmitry Torokhov, Anton Bondarenko,
	Florian Fainelli, Kees Cook, mathias.nyman, felipe.balbi,
	Eugene Korenevsky, peter.chen, joe, Todd Broch, linux-usb,
	linux-kernel, Rajat Jain, Benson Leung

On Fri, Apr 20, 2018 at 10:29 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Fri, 20 Apr 2018, Ravi Chandra Sadineni wrote:
>
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
>> ---
>
> At this point you're supposed to list the differences between this
> patch and the preceding versions.

Mentioned the changes between different versions in V5. Thanks.

>
>>  drivers/usb/core/hcd.c |  2 ++
>>  drivers/usb/core/hub.c | 10 +++++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 777036ae63674..b8024ae4fdcaa 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2375,6 +2375,8 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
>>  {
>>       unsigned long flags;
>>
>> +     if (hcd->rh_registered)
>> +             pm_wakeup_event(&hcd->self.root_hub->dev, 0);
>>       spin_lock_irqsave (&hcd_root_hub_lock, flags);
>>       if (hcd->rh_registered) {
>>               set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
>
> This isn't good enough.  hcd->rh_registered can change at any time;
> it is protected by the hcd_root_hub_lock spinlock.  That's why I said
> your code should be moved inside the existing "if" statement.

Sorry about this.  Fixed it now. Hope this is fine. Thanks.
>
> Alan Stern
>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index f6ea16e9f6bb9..aa9968d90a48c 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -653,12 +653,17 @@ void usb_wakeup_notification(struct usb_device *hdev,
>>               unsigned int portnum)
>>  {
>>       struct usb_hub *hub;
>> +     struct usb_port *port_dev;
>>
>>       if (!hdev)
>>               return;
>>
>>       hub = usb_hub_to_struct_hub(hdev);
>>       if (hub) {
>> +             port_dev = hub->ports[portnum - 1];
>> +             if (port_dev && port_dev->child)
>> +                     pm_wakeup_event(&port_dev->child->dev, 0);
>> +
>>               set_bit(portnum, hub->wakeup_bits);
>>               kick_hub_wq(hub);
>>       }
>> @@ -3434,8 +3439,11 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>>
>>       /* Skip the initial Clear-Suspend step for a remote wakeup */
>>       status = hub_port_status(hub, port1, &portstatus, &portchange);
>> -     if (status == 0 && !port_is_suspended(hub, portstatus))
>> +     if (status == 0 && !port_is_suspended(hub, portstatus)) {
>> +             if (portchange & USB_PORT_STAT_C_SUSPEND)
>> +                     pm_wakeup_event(&udev->dev, 0);
>>               goto SuspendCleared;
>> +     }
>>
>>       /* see 7.1.7.7; affects power usage, but not budgeting */
>>       if (hub_is_superspeed(hub->hdev))
>>
>
Thanks,
Ravi
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V5] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 18:22               ` Alan Stern
  0 siblings, 0 replies; 34+ messages in thread
From: Alan Stern @ 2018-04-20 18:22 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Fri, 20 Apr 2018, Ravi Chandra Sadineni wrote:

> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
> 
> V5: Added the description of changes between different versions of patches.
> V4: Moved the wakeup count increment logic to the existing if which is
> safegaurded by hcd_root_hub_lock spinlock.
> V3: Added a gaurd to check if rh_registered is set before accessing
> root_hub pointer.
> V2: Fixed the build failure error due to uninitialized dev pointer.

Acked-by: Alan Stern <stern@rowland.harvard.edu>


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [V5] USB: Increment wakeup count on remote wakeup.
@ 2018-04-20 18:22               ` Alan Stern
  0 siblings, 0 replies; 34+ messages in thread
From: Alan Stern @ 2018-04-20 18:22 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: gregkh, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Fri, 20 Apr 2018, Ravi Chandra Sadineni wrote:

> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
> 
> V5: Added the description of changes between different versions of patches.
> V4: Moved the wakeup count increment logic to the existing if which is
> safegaurded by hcd_root_hub_lock spinlock.
> V3: Added a gaurd to check if rh_registered is set before accessing
> root_hub pointer.
> V2: Fixed the build failure error due to uninitialized dev pointer.

Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V5] USB: Increment wakeup count on remote wakeup.
@ 2018-04-21  8:59               ` Greg Kroah-Hartman
  0 siblings, 0 replies; 34+ messages in thread
From: Greg KH @ 2018-04-21  8:59 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: stern, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Fri, Apr 20, 2018 at 11:08:21AM -0700, Ravi Chandra Sadineni wrote:
> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
> 
> V5: Added the description of changes between different versions of patches.
> V4: Moved the wakeup count increment logic to the existing if which is
> safegaurded by hcd_root_hub_lock spinlock.
> V3: Added a gaurd to check if rh_registered is set before accessing
> root_hub pointer.
> V2: Fixed the build failure error due to uninitialized dev pointer.

Is this needed in older kernels?  Should I submit it to the stable
trees?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [V5] USB: Increment wakeup count on remote wakeup.
@ 2018-04-21  8:59               ` Greg Kroah-Hartman
  0 siblings, 0 replies; 34+ messages in thread
From: Greg Kroah-Hartman @ 2018-04-21  8:59 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: stern, martin.blumenstingl, ravisadineni, chunfeng.yun, johan,
	arvind.yadav.cs, dtor, anton.bondarenko.sama, f.fainelli,
	keescook, mathias.nyman, felipe.balbi, ekorenevsky, peter.chen,
	joe, tbroch, linux-usb, linux-kernel, rajatja, bleung

On Fri, Apr 20, 2018 at 11:08:21AM -0700, Ravi Chandra Sadineni wrote:
> On chromebooks we depend on wakeup count to identify the wakeup source.
> But currently USB devices do not increment the wakeup count when they
> trigger the remote wake. This patch addresses the same.
> 
> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
> 
> On USB 2.0 devices, a wake capable device, if wake enabled, drives
> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
> has resumed before driving the resume signal from the host and
> C_PORT_SUSPEND is set, then the device attached to the given port might
> be the reason for the last system wakeup. Increment the wakeup count for
> the same.
> 
> On USB 3.0 devices, a function may signal that it wants to exit from device
> suspend by sending a Function Wake Device Notification to the host (USB3.0
> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
> wakeup count.
> 
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
> 
> V5: Added the description of changes between different versions of patches.
> V4: Moved the wakeup count increment logic to the existing if which is
> safegaurded by hcd_root_hub_lock spinlock.
> V3: Added a gaurd to check if rh_registered is set before accessing
> root_hub pointer.
> V2: Fixed the build failure error due to uninitialized dev pointer.

Is this needed in older kernels?  Should I submit it to the stable
trees?

thanks,

greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V5] USB: Increment wakeup count on remote wakeup.
@ 2018-04-21 13:37                 ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-21 13:37 UTC (permalink / raw)
  To: Greg KH
  Cc: Ravi Chandra Sadineni, Alan Stern, Martin Blumenstingl,
	chunfeng.yun, johan, Arvind Yadav, Dmitry Torokhov,
	Anton Bondarenko, Florian Fainelli, Kees Cook, mathias.nyman,
	felipe.balbi, Eugene Korenevsky, peter.chen, joe, Todd Broch,
	linux-usb, linux-kernel, Rajat Jain, Benson Leung

Sure. Pushing it to the older kernels will definitely help.

Thanks,
Ravi

On Sat, Apr 21, 2018 at 1:59 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Apr 20, 2018 at 11:08:21AM -0700, Ravi Chandra Sadineni wrote:
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
>> ---
>>
>> V5: Added the description of changes between different versions of patches.
>> V4: Moved the wakeup count increment logic to the existing if which is
>> safegaurded by hcd_root_hub_lock spinlock.
>> V3: Added a gaurd to check if rh_registered is set before accessing
>> root_hub pointer.
>> V2: Fixed the build failure error due to uninitialized dev pointer.
>
> Is this needed in older kernels?  Should I submit it to the stable
> trees?
>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [V5] USB: Increment wakeup count on remote wakeup.
@ 2018-04-21 13:37                 ` Ravi Chandra Sadineni
  0 siblings, 0 replies; 34+ messages in thread
From: Ravi Chandra Sadineni @ 2018-04-21 13:37 UTC (permalink / raw)
  To: Greg KH
  Cc: Ravi Chandra Sadineni, Alan Stern, Martin Blumenstingl,
	chunfeng.yun, johan, Arvind Yadav, Dmitry Torokhov,
	Anton Bondarenko, Florian Fainelli, Kees Cook, mathias.nyman,
	felipe.balbi, Eugene Korenevsky, peter.chen, joe, Todd Broch,
	linux-usb, linux-kernel, Rajat Jain, Benson Leung

Sure. Pushing it to the older kernels will definitely help.

Thanks,
Ravi

On Sat, Apr 21, 2018 at 1:59 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Apr 20, 2018 at 11:08:21AM -0700, Ravi Chandra Sadineni wrote:
>> On chromebooks we depend on wakeup count to identify the wakeup source.
>> But currently USB devices do not increment the wakeup count when they
>> trigger the remote wake. This patch addresses the same.
>>
>> Resume condition is reported differently on USB 2.0 and USB 3.0 devices.
>>
>> On USB 2.0 devices, a wake capable device, if wake enabled, drives
>> resume signal to indicate a remote wake (USB 2.0 spec section 7.1.7.7).
>> The upstream facing port then sets C_PORT_SUSPEND bit and reports a
>> port change event (USB 2.0 spec section 11.24.2.7.2.3). Thus if a port
>> has resumed before driving the resume signal from the host and
>> C_PORT_SUSPEND is set, then the device attached to the given port might
>> be the reason for the last system wakeup. Increment the wakeup count for
>> the same.
>>
>> On USB 3.0 devices, a function may signal that it wants to exit from device
>> suspend by sending a Function Wake Device Notification to the host (USB3.0
>> spec section 8.5.6.4) Thus on receiving the Function Wake, increment the
>> wakeup count.
>>
>> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
>> ---
>>
>> V5: Added the description of changes between different versions of patches.
>> V4: Moved the wakeup count increment logic to the existing if which is
>> safegaurded by hcd_root_hub_lock spinlock.
>> V3: Added a gaurd to check if rh_registered is set before accessing
>> root_hub pointer.
>> V2: Fixed the build failure error due to uninitialized dev pointer.
>
> Is this needed in older kernels?  Should I submit it to the stable
> trees?
>
> thanks,
>
> greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2018-04-21 13:38 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19  0:18 [PATCH] USB: Increment wakeup count on remote wakeup Ravi Chandra Sadineni
2018-04-19  0:18 ` Ravi Chandra Sadineni
2018-04-19  9:30 ` [PATCH] " kbuild test robot
2018-04-19  9:30   ` kbuild test robot
2018-04-19  9:33 ` [PATCH] " kbuild test robot
2018-04-19  9:33   ` kbuild test robot
2018-04-19 15:01 ` [PATCH] " Alan Stern
2018-04-19 15:01   ` Alan Stern
2018-04-19 16:17   ` [PATCH] " Rajat Jain
2018-04-19 16:17     ` Rajat Jain
2018-04-20  0:27   ` [PATCH V2] " Ravi Chandra Sadineni
2018-04-20  0:27     ` [V2] " Ravi Chandra Sadineni
2018-04-20 14:12     ` [PATCH V2] " Alan Stern
2018-04-20 14:12       ` [V2] " Alan Stern
2018-04-20 17:05       ` [PATCH V3] " Ravi Chandra Sadineni
2018-04-20 17:05         ` [V3] " Ravi Chandra Sadineni
2018-04-20 17:29         ` [PATCH V3] " Alan Stern
2018-04-20 17:29           ` [V3] " Alan Stern
2018-04-20 17:54           ` [PATCH V4] " Ravi Chandra Sadineni
2018-04-20 17:54             ` [V4] " Ravi Chandra Sadineni
2018-04-20 18:08           ` [PATCH V5] " Ravi Chandra Sadineni
2018-04-20 18:08             ` [V5] " Ravi Chandra Sadineni
2018-04-20 18:22             ` [PATCH V5] " Alan Stern
2018-04-20 18:22               ` [V5] " Alan Stern
2018-04-21  8:59             ` [PATCH V5] " Greg KH
2018-04-21  8:59               ` [V5] " Greg Kroah-Hartman
2018-04-21 13:37               ` [PATCH V5] " Ravi Chandra Sadineni
2018-04-21 13:37                 ` [V5] " Ravi Chandra Sadineni
2018-04-20 18:12           ` [PATCH V3] " Ravi Chandra Sadineni
2018-04-20 18:12             ` [V3] " Ravi Chandra Sadineni
2018-04-20 17:07       ` [PATCH V2] " Ravi Chandra Sadineni
2018-04-20 17:07         ` [V2] " Ravi Chandra Sadineni
2018-04-20  0:50   ` [PATCH] " Ravi Chandra Sadineni
2018-04-20  0:50     ` Ravi Chandra Sadineni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.