From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 20 Dec 2017 10:53:09 -0800 From: Brian Norris To: Kai-Heng Feng Cc: gregkh@linuxfoundation.org, marcel@holtmann.org, linux-usb@vger.kernel.org, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, Leif Liddy , Matthias Kaehlcke , Daniel Drake , Guenter Roeck Subject: Re: [PATCH 1/2] Revert "Bluetooth: btusb: fix QCA Rome suspend/resume" Message-ID: <20171220185308.GA176644@google.com> References: <20171220110008.11856-1-kai.heng.feng@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20171220110008.11856-1-kai.heng.feng@canonical.com> List-ID: On Wed, Dec 20, 2017 at 07:00:07PM +0800, Kai-Heng Feng wrote: > This reverts commit fd865802c66bc451dc515ed89360f84376ce1a56. > > This commit causes a regression on some QCA ROME chips. The USB device > reset happens in btusb_open(), hence firmware loading gets interrupted. Oh, did you really confirm that's the root of the problem? I was only hypothesizing, with some informed observation and code review; but I didn't fully convince myself. If so, that's interesting. > Furthermore, this commit stops working after commit > ("a0085f2510e8976614ad8f766b209448b385492f Bluetooth: btusb: driver to > enable the usb-wakeup feature"). Reset-resume quirk only gets enabled in > btusb_suspend() when it's not a wakeup source. > > If we really want to reset the USB device, we need to do it before > btusb_open(). Let's handle it in drivers/usb/core/quirks.c. > > Cc: stable@vger.kernel.org > Cc: Leif Liddy > Cc: Matthias Kaehlcke > Cc: Brian Norris > Cc: Daniel Drake > Signed-off-by: Kai-Heng Feng Looks good to me. Definitely a regression and we need to clear that up in mainline and stable before reintroducing the intended fix: Reviewed-by: Brian Norris Tested-by: Brian Norris Thanks! > --- > > Daniel, Cc you because this also affects your original quirk patch for > Realtek btusb. > > drivers/bluetooth/btusb.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index f7120c9eb9bd..da353c4acdc7 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -3117,12 +3117,6 @@ static int btusb_probe(struct usb_interface *intf, > if (id->driver_info & BTUSB_QCA_ROME) { > data->setup_on_usb = btusb_setup_qca; > hdev->set_bdaddr = btusb_set_bdaddr_ath3012; > - > - /* QCA Rome devices lose their updated firmware over suspend, > - * but the USB hub doesn't notice any status change. > - * Explicitly request a device reset on resume. > - */ > - set_bit(BTUSB_RESET_RESUME, &data->flags); > } > > #ifdef CONFIG_BT_HCIBTUSB_RTL > -- > 2.14.1 > From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [1/2] Revert "Bluetooth: btusb: fix QCA Rome suspend/resume" From: Brian Norris Message-Id: <20171220185308.GA176644@google.com> Date: Wed, 20 Dec 2017 10:53:09 -0800 To: Kai-Heng Feng Cc: gregkh@linuxfoundation.org, marcel@holtmann.org, linux-usb@vger.kernel.org, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, Leif Liddy , Matthias Kaehlcke , Daniel Drake , Guenter Roeck List-ID: T24gV2VkLCBEZWMgMjAsIDIwMTcgYXQgMDc6MDA6MDdQTSArMDgwMCwgS2FpLUhlbmcgRmVuZyB3 cm90ZToKPiBUaGlzIHJldmVydHMgY29tbWl0IGZkODY1ODAyYzY2YmM0NTFkYzUxNWVkODkzNjBm ODQzNzZjZTFhNTYuCj4gCj4gVGhpcyBjb21taXQgY2F1c2VzIGEgcmVncmVzc2lvbiBvbiBzb21l IFFDQSBST01FIGNoaXBzLiBUaGUgVVNCIGRldmljZQo+IHJlc2V0IGhhcHBlbnMgaW4gYnR1c2Jf b3BlbigpLCBoZW5jZSBmaXJtd2FyZSBsb2FkaW5nIGdldHMgaW50ZXJydXB0ZWQuCgpPaCwgZGlk IHlvdSByZWFsbHkgY29uZmlybSB0aGF0J3MgdGhlIHJvb3Qgb2YgdGhlIHByb2JsZW0/IEkgd2Fz IG9ubHkKaHlwb3RoZXNpemluZywgd2l0aCBzb21lIGluZm9ybWVkIG9ic2VydmF0aW9uIGFuZCBj b2RlIHJldmlldzsgYnV0IEkKZGlkbid0IGZ1bGx5IGNvbnZpbmNlIG15c2VsZi4gSWYgc28sIHRo YXQncyBpbnRlcmVzdGluZy4KCj4gRnVydGhlcm1vcmUsIHRoaXMgY29tbWl0IHN0b3BzIHdvcmtp bmcgYWZ0ZXIgY29tbWl0Cj4gKCJhMDA4NWYyNTEwZTg5NzY2MTRhZDhmNzY2YjIwOTQ0OGIzODU0 OTJmIEJsdWV0b290aDogYnR1c2I6IGRyaXZlciB0bwo+IGVuYWJsZSB0aGUgdXNiLXdha2V1cCBm ZWF0dXJlIikuIFJlc2V0LXJlc3VtZSBxdWlyayBvbmx5IGdldHMgZW5hYmxlZCBpbgo+IGJ0dXNi X3N1c3BlbmQoKSB3aGVuIGl0J3Mgbm90IGEgd2FrZXVwIHNvdXJjZS4KPiAKPiBJZiB3ZSByZWFs bHkgd2FudCB0byByZXNldCB0aGUgVVNCIGRldmljZSwgd2UgbmVlZCB0byBkbyBpdCBiZWZvcmUK PiBidHVzYl9vcGVuKCkuIExldCdzIGhhbmRsZSBpdCBpbiBkcml2ZXJzL3VzYi9jb3JlL3F1aXJr cy5jLgo+IAo+IENjOiBzdGFibGVAdmdlci5rZXJuZWwub3JnCj4gQ2M6IExlaWYgTGlkZHkgPGxl aWYubGludXhAZ21haWwuY29tPgo+IENjOiBNYXR0aGlhcyBLYWVobGNrZSA8bWthQGNocm9taXVt Lm9yZz4KPiBDYzogQnJpYW4gTm9ycmlzIDxicmlhbm5vcnJpc0BjaHJvbWl1bS5vcmc+Cj4gQ2M6 IERhbmllbCBEcmFrZSA8ZHJha2VAZW5kbGVzc20uY29tPgo+IFNpZ25lZC1vZmYtYnk6IEthaS1I ZW5nIEZlbmcgPGthaS5oZW5nLmZlbmdAY2Fub25pY2FsLmNvbT4KCkxvb2tzIGdvb2QgdG8gbWUu IERlZmluaXRlbHkgYSByZWdyZXNzaW9uIGFuZCB3ZSBuZWVkIHRvIGNsZWFyIHRoYXQgdXAKaW4g bWFpbmxpbmUgYW5kIHN0YWJsZSBiZWZvcmUgcmVpbnRyb2R1Y2luZyB0aGUgaW50ZW5kZWQgZml4 OgoKUmV2aWV3ZWQtYnk6IEJyaWFuIE5vcnJpcyA8YnJpYW5ub3JyaXNAY2hyb21pdW0ub3JnPgpU ZXN0ZWQtYnk6IEJyaWFuIE5vcnJpcyA8YnJpYW5ub3JyaXNAY2hyb21pdW0ub3JnPgoKVGhhbmtz IQoKPiAtLS0KPiAKPiBEYW5pZWwsIENjIHlvdSBiZWNhdXNlIHRoaXMgYWxzbyBhZmZlY3RzIHlv dXIgb3JpZ2luYWwgcXVpcmsgcGF0Y2ggZm9yCj4gUmVhbHRlayBidHVzYi4KPiAKPiAgZHJpdmVy cy9ibHVldG9vdGgvYnR1c2IuYyB8IDYgLS0tLS0tCj4gIDEgZmlsZSBjaGFuZ2VkLCA2IGRlbGV0 aW9ucygtKQo+IAo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2JsdWV0b290aC9idHVzYi5jIGIvZHJp dmVycy9ibHVldG9vdGgvYnR1c2IuYwo+IGluZGV4IGY3MTIwYzllYjliZC4uZGEzNTNjNGFjZGM3 IDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvYmx1ZXRvb3RoL2J0dXNiLmMKPiArKysgYi9kcml2ZXJz L2JsdWV0b290aC9idHVzYi5jCj4gQEAgLTMxMTcsMTIgKzMxMTcsNiBAQCBzdGF0aWMgaW50IGJ0 dXNiX3Byb2JlKHN0cnVjdCB1c2JfaW50ZXJmYWNlICppbnRmLAo+ICAJaWYgKGlkLT5kcml2ZXJf aW5mbyAmIEJUVVNCX1FDQV9ST01FKSB7Cj4gIAkJZGF0YS0+c2V0dXBfb25fdXNiID0gYnR1c2Jf c2V0dXBfcWNhOwo+ICAJCWhkZXYtPnNldF9iZGFkZHIgPSBidHVzYl9zZXRfYmRhZGRyX2F0aDMw MTI7Cj4gLQo+IC0JCS8qIFFDQSBSb21lIGRldmljZXMgbG9zZSB0aGVpciB1cGRhdGVkIGZpcm13 YXJlIG92ZXIgc3VzcGVuZCwKPiAtCQkgKiBidXQgdGhlIFVTQiBodWIgZG9lc24ndCBub3RpY2Ug YW55IHN0YXR1cyBjaGFuZ2UuCj4gLQkJICogRXhwbGljaXRseSByZXF1ZXN0IGEgZGV2aWNlIHJl c2V0IG9uIHJlc3VtZS4KPiAtCQkgKi8KPiAtCQlzZXRfYml0KEJUVVNCX1JFU0VUX1JFU1VNRSwg JmRhdGEtPmZsYWdzKTsKPiAgCX0KPiAgCj4gICNpZmRlZiBDT05GSUdfQlRfSENJQlRVU0JfUlRM Cj4gLS0gCj4gMi4xNC4xCj4KLS0tClRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5k IHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC11c2IiIGluCnRoZSBib2R5IG9mIGEgbWVzc2Fn ZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnCk1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0 dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbAo=