[PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Abhijeet, Kumar]

From: Abhijeet Kumar <abhijeet.kumar@intel.com>

Runtime resume USB device in order to ensure that PM framework knows
that the we might be using the device in a short time and doesn't
autosuspend the device while we update it's interface. Without this
change, if device autosuspends and the kernel polling for block
devices is disabled through sysfs at runtime or through bootargs, then
storage devices might never unmount since the disconnect IRQ wont be
kicked at all.

Signed-off-by: Abhijeet Kumar <abhijeet.kumar@intel.com>
---
 drivers/usb/core/message.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 371a07d874a3..54a9accf88cb 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1305,6 +1305,11 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
 	if (iface->unregistering)
 		return -ENODEV;
 
+	/*Letting runtime PM know that we wish to use the device in a
+	 * short time.
+	 */
+	pm_runtime_get(&iface->dev);
+
 	alt = usb_altnum_to_altsetting(iface, alternate);
 	if (!alt) {
 		dev_warn(&dev->dev, "selecting invalid altsetting %d\n",
-- 
1.9.1

Re: [PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Greg Kroah-Hartman]

On Fri, Nov 10, 2017 at 09:32:07PM +0530, Abhijeet@vger.kernel.org wrote:
> From: Abhijeet Kumar <abhijeet.kumar@intel.com>
> 
> Runtime resume USB device in order to ensure that PM framework knows
> that the we might be using the device in a short time and doesn't
> autosuspend the device while we update it's interface. Without this
> change, if device autosuspends and the kernel polling for block
> devices is disabled through sysfs at runtime or through bootargs, then
> storage devices might never unmount since the disconnect IRQ wont be
> kicked at all.
> 
> Signed-off-by: Abhijeet Kumar <abhijeet.kumar@intel.com>
> ---
>  drivers/usb/core/message.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 371a07d874a3..54a9accf88cb 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1305,6 +1305,11 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
>  	if (iface->unregistering)
>  		return -ENODEV;
>  
> +	/*Letting runtime PM know that we wish to use the device in a
> +	 * short time.
> +	 */

Please place comments int the correct style.

> +	pm_runtime_get(&iface->dev);
> +

No dropping of the pm_runtime when we are finished?

This feels really wrong to me...

greg k-h

Re: [PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Alan Stern]

On Fri, 10 Nov 2017 Abhijeet@vger.kernel.org wrote:

> From: Abhijeet Kumar <abhijeet.kumar@intel.com>
> 
> Runtime resume USB device in order to ensure that PM framework knows
> that the we might be using the device in a short time and doesn't
> autosuspend the device while we update it's interface. Without this
> change, if device autosuspends and the kernel polling for block
> devices is disabled through sysfs at runtime or through bootargs, then
> storage devices might never unmount since the disconnect IRQ wont be
> kicked at all.
> 
> Signed-off-by: Abhijeet Kumar <abhijeet.kumar@intel.com>

I don't see any reason for doing this.  The places that call this 
subroutine will already have made sure that the device is active.  
There's no need for an additional runtime_get.

As for disconnect IRQs, they occur whether the device is suspended or 
not (they are generated by the host controller, not by the device).

Alan Stern

Re: [PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Felipe Balbi]

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


Hi,

Abhijeet@vger.kernel.org, Kumar@vger.kernel.org writes:

these emails don't exist. Fix your email client.

> From: abhijeet kumar <abhijeet.kumar@intel.com>

capitalize names

> Runtime resume USB device in order to ensure that PM framework
> knows that the we might be using the device in a short time and doesn't
> autosuspend the device while we updating it's interface.

this doesn't tell me about any problem. What, exactly, are you trying to
fix?

> Signed-off-by: abhijeet kumar <abhijeet.kumar@intel.com>

capitalize names

> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 371a07d874a3..658603ed779e 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1305,6 +1305,9 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
>  	if (iface->unregistering)
>  		return -ENODEV;
>  
> +	/*Letting runtime PM now that we wish to use the device in a short time
> +	 *pm_runtime_get(&iface->dev);
> +	 */

why is it so that adding commented out code help? Did you *really* test
this at all?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Abhijeet, Kumar]

From: abhijeet kumar <abhijeet.kumar@intel.com>

Runtime resume USB device in order to ensure that PM framework
knows that the we might be using the device in a short time and doesn't
autosuspend the device while we updating it's interface.

Signed-off-by: abhijeet kumar <abhijeet.kumar@intel.com>
---
 drivers/usb/core/message.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 371a07d874a3..658603ed779e 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1305,6 +1305,9 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
 	if (iface->unregistering)
 		return -ENODEV;
 
+	/*Letting runtime PM now that we wish to use the device in a short time
+	 *pm_runtime_get(&iface->dev);
+	 */
+
 	alt = usb_altnum_to_altsetting(iface, alternate);
 	if (!alt) {
 		dev_warn(&dev->dev, "selecting invalid altsetting %d\n",
-- 
1.9.1