From: Champion Chen <[email protected]>
Suspend could fail for some platforms because
btusb_suspend==> btusb_stop_traffic ==> usb_kill_anchored_urbs,
When btusb_bulk_complete returns before system suspend and resubmits an urb,
the system cannot enter suspend state.
Signed-off-by: Champion Chen <[email protected]>
Signed-off-by: Larry Finger <[email protected]>
Cc: Stable <[email protected]>
---
Johan,
To help Champion with the process, I have formatted the patch in
the correct manner. I hope I understand the issue correctly and
stated it in a coherent manner in the commit message.
Larry
---
drivers/bluetooth/btusb.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 292c38e..45a7211 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -330,6 +330,9 @@ static void btusb_intr_complete(struct urb *urb)
BT_ERR("%s corrupted event packet", hdev->name);
hdev->stat.err_rx++;
}
+ } else if (urb->status == -ENOENT) {
+ /* Avoid suspend failed when usb_kill_urb */
+ return;
}
if (!test_bit(BTUSB_INTR_RUNNING, &data->flags))
@@ -417,6 +420,9 @@ static void btusb_bulk_complete(struct urb *urb)
urb->actual_length) < 0) {
BT_ERR("%s corrupted ACL packet", hdev->name);
hdev->stat.err_rx++;
+ } else if (urb->status == -ENOENT) {
+ /* Avoid suspend failed when usb_kill_urb */
+ return;
}
}
@@ -512,6 +518,9 @@ static void btusb_isoc_complete(struct urb *urb)
hdev->stat.err_rx++;
}
}
+ } else if (urb->status == -ENOENT) {
+ /* Avoid suspend failed when usb_kill_urb */
+ return;
}
if (!test_bit(BTUSB_ISOC_RUNNING, &data->flags))
--
1.8.4.5
On Sat, Sep 06, 2014 at 12:12:44PM -0700, Marcel Holtmann wrote:
> Hi Larry,
>
> >> one other thing. Please let maintainers decide when a patch is
> >> useful for stable and when not. Patches need to go upstream first
> >> anyway. People spamming [email protected] with patches that
> >> are not reviewed yet is pretty much a waste of time.
> >
> > Sorry. In the wireless tree, the patch author suggests that the
> > patch is suitable for stable. If the maintainer disagrees, he strips
> > the Cc to stable. It seems that you do it the other way. In either
> > case, I do not think the stable patches are picked up by the
> > maintainers of stable versions until the patch hits mainline.
>
> I have no idea what the [email protected] guys do with patches
> that are not yet upstream.
I ignore them :)
> However it is a waste of their time sending stuff to their mailing
> list that has not even seen a maintainer review. You can just mention
> it in the comment section that this might qualify for stable.
It doesn't bother me at all for the cc:, they are easy to filter away,
and it keeps me informed as to where patches will be coming from in the
future, and helps to track down things that get dropped.
thanks,
greg k-h
Hi Larry,
>> one other thing. Please let maintainers decide when a patch is useful for stable and when not. Patches need to go upstream first anyway. People spamming [email protected] with patches that are not reviewed yet is pretty much a waste of time.
>
> Sorry. In the wireless tree, the patch author suggests that the patch is suitable for stable. If the maintainer disagrees, he strips the Cc to stable. It seems that you do it the other way. In either case, I do not think the stable patches are picked up by the maintainers of stable versions until the patch hits mainline.
I have no idea what the [email protected] guys do with patches that are not yet upstream. However it is a waste of their time sending stuff to their mailing list that has not even seen a maintainer review. You can just mention it in the comment section that this might qualify for stable.
Regards
Marcel
On 09/06/2014 12:35 PM, Marcel Holtmann wrote:
> Hi Larry,
>
>
> one other thing. Please let maintainers decide when a patch is useful for stable and when not. Patches need to go upstream first anyway. People spamming [email protected] with patches that are not reviewed yet is pretty much a waste of time.
Sorry. In the wireless tree, the patch author suggests that the patch is
suitable for stable. If the maintainer disagrees, he strips the Cc to stable. It
seems that you do it the other way. In either case, I do not think the stable
patches are picked up by the maintainers of stable versions until the patch hits
mainline.
Larry
Hi Larry,
>>> Suspend could fail for some platforms because
>>> btusb_suspend==> btusb_stop_traffic ==> usb_kill_anchored_urbs,
>>>
>>> When btusb_bulk_complete returns before system suspend and resubmits an urb,
>>> the system cannot enter suspend state.
>>>
>>> Signed-off-by: Champion Chen <[email protected]>
>>> Signed-off-by: Larry Finger <[email protected]>
>>> Cc: Stable <[email protected]>
>>> ---
>>> Johan,
>>>
>>> To help Champion with the process, I have formatted the patch in
>>> the correct manner. I hope I understand the issue correctly and
>>> stated it in a coherent manner in the commit message.
>>>
>>> Larry
>>> ---
>>> drivers/bluetooth/btusb.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>> index 292c38e..45a7211 100644
>>> --- a/drivers/bluetooth/btusb.c
>>> +++ b/drivers/bluetooth/btusb.c
>>> @@ -330,6 +330,9 @@ static void btusb_intr_complete(struct urb *urb)
>>> BT_ERR("%s corrupted event packet", hdev->name);
>>> hdev->stat.err_rx++;
>>> }
>>> + } else if (urb->status == -ENOENT) {
>>> + /* Avoid suspend failed when usb_kill_urb */
>>> + return;
>>> }
>>>
>>> if (!test_bit(BTUSB_INTR_RUNNING, &data->flags))
>>> @@ -417,6 +420,9 @@ static void btusb_bulk_complete(struct urb *urb)
>>> urb->actual_length) < 0) {
>>> BT_ERR("%s corrupted ACL packet", hdev->name);
>>> hdev->stat.err_rx++;
>>> + } else if (urb->status == -ENOENT) {
>>> + /* Avoid suspend failed when usb_kill_urb */
>>> + return;
>>> }
>>
>> this one is utterly bogus. Either urb->status is 0 or it is -ENOENT, but it will not be both. I think you put it to the wrong if clause.
>
> Thanks for the review. Obviously, you are correct. A revised patch will be sent soon.
one other thing. Please let maintainers decide when a patch is useful for stable and when not. Patches need to go upstream first anyway. People spamming [email protected] with patches that are not reviewed yet is pretty much a waste of time.
Regards
Marcel
On 09/06/2014 12:02 PM, Marcel Holtmann wrote:
> Hi Larry,
>
>> Suspend could fail for some platforms because
>> btusb_suspend==> btusb_stop_traffic ==> usb_kill_anchored_urbs,
>>
>> When btusb_bulk_complete returns before system suspend and resubmits an urb,
>> the system cannot enter suspend state.
>>
>> Signed-off-by: Champion Chen <[email protected]>
>> Signed-off-by: Larry Finger <[email protected]>
>> Cc: Stable <[email protected]>
>> ---
>> Johan,
>>
>> To help Champion with the process, I have formatted the patch in
>> the correct manner. I hope I understand the issue correctly and
>> stated it in a coherent manner in the commit message.
>>
>> Larry
>> ---
>> drivers/bluetooth/btusb.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index 292c38e..45a7211 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -330,6 +330,9 @@ static void btusb_intr_complete(struct urb *urb)
>> BT_ERR("%s corrupted event packet", hdev->name);
>> hdev->stat.err_rx++;
>> }
>> + } else if (urb->status == -ENOENT) {
>> + /* Avoid suspend failed when usb_kill_urb */
>> + return;
>> }
>>
>> if (!test_bit(BTUSB_INTR_RUNNING, &data->flags))
>> @@ -417,6 +420,9 @@ static void btusb_bulk_complete(struct urb *urb)
>> urb->actual_length) < 0) {
>> BT_ERR("%s corrupted ACL packet", hdev->name);
>> hdev->stat.err_rx++;
>> + } else if (urb->status == -ENOENT) {
>> + /* Avoid suspend failed when usb_kill_urb */
>> + return;
>> }
>
> this one is utterly bogus. Either urb->status is 0 or it is -ENOENT, but it will not be both. I think you put it to the wrong if clause.
Thanks for the review. Obviously, you are correct. A revised patch will be sent
soon.
The code sent by Champion is now posted at
http://github.com/lwfinger/rtl8723au_bt/new. It works for everyone that has
tested it.
Larry
Hi Larry,
> Suspend could fail for some platforms because
> btusb_suspend==> btusb_stop_traffic ==> usb_kill_anchored_urbs,
>
> When btusb_bulk_complete returns before system suspend and resubmits an urb,
> the system cannot enter suspend state.
>
> Signed-off-by: Champion Chen <[email protected]>
> Signed-off-by: Larry Finger <[email protected]>
> Cc: Stable <[email protected]>
> ---
> Johan,
>
> To help Champion with the process, I have formatted the patch in
> the correct manner. I hope I understand the issue correctly and
> stated it in a coherent manner in the commit message.
>
> Larry
> ---
> drivers/bluetooth/btusb.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 292c38e..45a7211 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -330,6 +330,9 @@ static void btusb_intr_complete(struct urb *urb)
> BT_ERR("%s corrupted event packet", hdev->name);
> hdev->stat.err_rx++;
> }
> + } else if (urb->status == -ENOENT) {
> + /* Avoid suspend failed when usb_kill_urb */
> + return;
> }
>
> if (!test_bit(BTUSB_INTR_RUNNING, &data->flags))
> @@ -417,6 +420,9 @@ static void btusb_bulk_complete(struct urb *urb)
> urb->actual_length) < 0) {
> BT_ERR("%s corrupted ACL packet", hdev->name);
> hdev->stat.err_rx++;
> + } else if (urb->status == -ENOENT) {
> + /* Avoid suspend failed when usb_kill_urb */
> + return;
> }
this one is utterly bogus. Either urb->status is 0 or it is -ENOENT, but it will not be both. I think you put it to the wrong if clause.
> }
>
> @@ -512,6 +518,9 @@ static void btusb_isoc_complete(struct urb *urb)
> hdev->stat.err_rx++;
> }
> }
> + } else if (urb->status == -ENOENT) {
> + /* Avoid suspend failed when usb_kill_urb */
> + return;
> }
>
> if (!test_bit(BTUSB_ISOC_RUNNING, &data->flags))
Regards
Marcel