Re: [PATCH 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case

2013-02-28 Thread Alan Stern 
On Thu, 28 Feb 2013, Lan Tianyu wrote:

> Hi Alan:
>   Further thinking, the device should be disconnected since the port
> can't be resumed and the device will not work normally. Something like
> following. Does this make sense?
> ---
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index d5d3de4..cf36b11 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -3170,6 +3170,7 @@ int usb_port_resume(struct usb_device *udev,
> pm_message_t msg)
> if (status < 0) {
> dev_dbg(>dev, "can't resume usb port,
> status %d\n",
> status);
> +   hub_port_logical_disconnect(hub, port1);
> return status;
> }
> }

I don't know.  If you do this, will the port ever get powered on again?  
This sort of thing is hard to test.

As far as the device is concerned, it won't make much difference.  
Either way, the device won't work.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case

2013-02-28 Thread Li, Fei 
> -Original Message-
> From: Lan, Tianyu
> Sent: Thursday, February 28, 2013 4:37 PM
> To: Li, Fei; st...@rowland.harvard.edu
> Cc: gre...@linuxfoundation.org; sarah.a.sh...@linux.intel.com; r...@sisk.pl;
> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Liu, Chuansheng
> Subject: Re: [PATCH 4/5] usb: call pm_runtime_put_sync in
> pm_runtime_get_sync failed case
> 
> On 2013年02月28日 15:57, Li Fei wrote:
> >
> > Even in failed case of pm_runtime_get_sync, the usage_count
> > is incremented. In order to keep the usage_count with correct
> > value and runtime power management to behave correctly, call
> > pm_runtime_put(_sync) in such case.
> 
> Hi Fei:
>   It's not necessary. Because the did_runtime_put == true means the
> port's usage count has already been decreased during
> usb_port_suspend().So to keep usage count balance, we should increase
> the usage count in the usb_port_resume() whatever.

Thanks for your reminder.
Sorry, I forget to keep did_runtime_put as false in pm_runtime_get_sync
failed case. I'll submit patch V2 to update it.

> >
> > Signed-off-by Liu Chuansheng 
> > Signed-off-by: Li Fei 
> > ---
> >  drivers/usb/core/hub.c |1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 5480352..b68493b 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -3152,6 +3152,7 @@ int usb_port_resume(struct usb_device *udev,
> pm_message_t msg)
> > if (status < 0) {
> > dev_dbg(>dev, "can't resume usb port, status 
> > %d\n",
> > status);
> > +   pm_runtime_put_sync(_dev->dev);
> > return status;
> > }
> > }
> >
> Hi Alan:
>   Further thinking, the device should be disconnected since the port
> can't be resumed and the device will not work normally. Something like
> following. Does this make sense?
> ---
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index d5d3de4..cf36b11 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -3170,6 +3170,7 @@ int usb_port_resume(struct usb_device *udev,
> pm_message_t msg)
> if (status < 0) {
> dev_dbg(>dev, "can't resume usb port,
> status %d\n",
> status);
> +   hub_port_logical_disconnect(hub, port1);

IMHO, this can't keep the usage_count balance.

Best Regards,
Li Fei 

> return status;
> }
> }
> 
> 
> --
> Best regards
> Tianyu Lan

Re: [PATCH 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case

2013-02-28 Thread Lan Tianyu 
On 2013年02月28日 15:57, Li Fei wrote:
> 
> Even in failed case of pm_runtime_get_sync, the usage_count
> is incremented. In order to keep the usage_count with correct
> value and runtime power management to behave correctly, call
> pm_runtime_put(_sync) in such case.

Hi Fei:
It's not necessary. Because the did_runtime_put == true means the
port's usage count has already been decreased during
usb_port_suspend().So to keep usage count balance, we should increase
the usage count in the usb_port_resume() whatever.

> 
> Signed-off-by Liu Chuansheng 
> Signed-off-by: Li Fei 
> ---
>  drivers/usb/core/hub.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 5480352..b68493b 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -3152,6 +3152,7 @@ int usb_port_resume(struct usb_device *udev, 
> pm_message_t msg)
>   if (status < 0) {
>   dev_dbg(>dev, "can't resume usb port, status 
> %d\n",
>   status);
> + pm_runtime_put_sync(_dev->dev);
>   return status;
>   }
>   }
> 
Hi Alan:
Further thinking, the device should be disconnected since the port
can't be resumed and the device will not work normally. Something like
following. Does this make sense?
---
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d5d3de4..cf36b11 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3170,6 +3170,7 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (status < 0) {
dev_dbg(>dev, "can't resume usb port,
status %d\n",
status);
+   hub_port_logical_disconnect(hub, port1);
return status;
}
}


-- 
Best regards
Tianyu Lan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case

2013-02-28 Thread Lan Tianyu 
On 2013年02月28日 15:57, Li Fei wrote:
 
 Even in failed case of pm_runtime_get_sync, the usage_count
 is incremented. In order to keep the usage_count with correct
 value and runtime power management to behave correctly, call
 pm_runtime_put(_sync) in such case.

Hi Fei:
It's not necessary. Because the did_runtime_put == true means the
port's usage count has already been decreased during
usb_port_suspend().So to keep usage count balance, we should increase
the usage count in the usb_port_resume() whatever.

 
 Signed-off-by Liu Chuansheng chuansheng@intel.com
 Signed-off-by: Li Fei fei...@intel.com
 ---
  drivers/usb/core/hub.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
 index 5480352..b68493b 100644
 --- a/drivers/usb/core/hub.c
 +++ b/drivers/usb/core/hub.c
 @@ -3152,6 +3152,7 @@ int usb_port_resume(struct usb_device *udev, 
 pm_message_t msg)
   if (status  0) {
   dev_dbg(udev-dev, can't resume usb port, status 
 %d\n,
   status);
 + pm_runtime_put_sync(port_dev-dev);
   return status;
   }
   }
 
Hi Alan:
Further thinking, the device should be disconnected since the port
can't be resumed and the device will not work normally. Something like
following. Does this make sense?
---
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d5d3de4..cf36b11 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3170,6 +3170,7 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (status  0) {
dev_dbg(udev-dev, can't resume usb port,
status %d\n,
status);
+   hub_port_logical_disconnect(hub, port1);
return status;
}
}


-- 
Best regards
Tianyu Lan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case

2013-02-28 Thread Li, Fei 
 -Original Message-
 From: Lan, Tianyu
 Sent: Thursday, February 28, 2013 4:37 PM
 To: Li, Fei; st...@rowland.harvard.edu
 Cc: gre...@linuxfoundation.org; sarah.a.sh...@linux.intel.com; r...@sisk.pl;
 linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Liu, Chuansheng
 Subject: Re: [PATCH 4/5] usb: call pm_runtime_put_sync in
 pm_runtime_get_sync failed case
 
 On 2013年02月28日 15:57, Li Fei wrote:
 
  Even in failed case of pm_runtime_get_sync, the usage_count
  is incremented. In order to keep the usage_count with correct
  value and runtime power management to behave correctly, call
  pm_runtime_put(_sync) in such case.
 
 Hi Fei:
   It's not necessary. Because the did_runtime_put == true means the
 port's usage count has already been decreased during
 usb_port_suspend().So to keep usage count balance, we should increase
 the usage count in the usb_port_resume() whatever.

Thanks for your reminder.
Sorry, I forget to keep did_runtime_put as false in pm_runtime_get_sync
failed case. I'll submit patch V2 to update it.

 
  Signed-off-by Liu Chuansheng chuansheng@intel.com
  Signed-off-by: Li Fei fei...@intel.com
  ---
   drivers/usb/core/hub.c |1 +
   1 files changed, 1 insertions(+), 0 deletions(-)
 
  diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
  index 5480352..b68493b 100644
  --- a/drivers/usb/core/hub.c
  +++ b/drivers/usb/core/hub.c
  @@ -3152,6 +3152,7 @@ int usb_port_resume(struct usb_device *udev,
 pm_message_t msg)
  if (status  0) {
  dev_dbg(udev-dev, can't resume usb port, status 
  %d\n,
  status);
  +   pm_runtime_put_sync(port_dev-dev);
  return status;
  }
  }
 
 Hi Alan:
   Further thinking, the device should be disconnected since the port
 can't be resumed and the device will not work normally. Something like
 following. Does this make sense?
 ---
 diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
 index d5d3de4..cf36b11 100644
 --- a/drivers/usb/core/hub.c
 +++ b/drivers/usb/core/hub.c
 @@ -3170,6 +3170,7 @@ int usb_port_resume(struct usb_device *udev,
 pm_message_t msg)
 if (status  0) {
 dev_dbg(udev-dev, can't resume usb port,
 status %d\n,
 status);
 +   hub_port_logical_disconnect(hub, port1);

IMHO, this can't keep the usage_count balance.

Best Regards,
Li Fei 

 return status;
 }
 }
 
 
 --
 Best regards
 Tianyu Lan

Re: [PATCH 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case

2013-02-28 Thread Alan Stern 
On Thu, 28 Feb 2013, Lan Tianyu wrote:

 Hi Alan:
   Further thinking, the device should be disconnected since the port
 can't be resumed and the device will not work normally. Something like
 following. Does this make sense?
 ---
 diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
 index d5d3de4..cf36b11 100644
 --- a/drivers/usb/core/hub.c
 +++ b/drivers/usb/core/hub.c
 @@ -3170,6 +3170,7 @@ int usb_port_resume(struct usb_device *udev,
 pm_message_t msg)
 if (status  0) {
 dev_dbg(udev-dev, can't resume usb port,
 status %d\n,
 status);
 +   hub_port_logical_disconnect(hub, port1);
 return status;
 }
 }

I don't know.  If you do this, will the port ever get powered on again?  
This sort of thing is hard to test.

As far as the device is concerned, it won't make much difference.  
Either way, the device won't work.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case

2013-02-27 Thread Li Fei 

Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.

Signed-off-by Liu Chuansheng 
Signed-off-by: Li Fei 
---
 drivers/usb/core/hub.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..b68493b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3152,6 +3152,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t 
msg)
if (status < 0) {
dev_dbg(>dev, "can't resume usb port, status 
%d\n",
status);
+   pm_runtime_put_sync(_dev->dev);
return status;
}
}
-- 
1.7.4.1



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case

2013-02-27 Thread Li Fei 

Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.

Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
 drivers/usb/core/hub.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..b68493b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3152,6 +3152,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t 
msg)
if (status  0) {
dev_dbg(udev-dev, can't resume usb port, status 
%d\n,
status);
+   pm_runtime_put_sync(port_dev-dev);
return status;
}
}
-- 
1.7.4.1



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/