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: [v4] USB: Don't enable LPM if it's already enabled From: Greg Kroah-Hartman Message-Id: <20190111091207.GE15610@kroah.com> Date: Fri, 11 Jan 2019 10:12:07 +0100 To: Alan Stern Cc: Kai Heng Feng , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org List-ID: T24gVHVlLCBKYW4gMDgsIDIwMTkgYXQgMTI6MzQ6MTdQTSAtMDUwMCwgQWxhbiBTdGVybiB3cm90 ZToKPiBPbiBXZWQsIDkgSmFuIDIwMTksIEthaSBIZW5nIEZlbmcgd3JvdGU6Cj4gCj4gPiA+IE9u IEphbiA4LCAyMDE5LCBhdCAxMTo0MSBQTSwgR3JlZyBLSCA8Z3JlZ2toQGxpbnV4Zm91bmRhdGlv bi5vcmc+IHdyb3RlOgo+ID4gPiAKPiA+ID4gT24gTW9uLCBEZWMgMDMsIDIwMTggYXQgMDY6MjY6 NDNQTSArMDgwMCwgS2FpLUhlbmcgRmVuZyB3cm90ZToKPiA+ID4+IFVTQiBCbHVldG9vdGggY29u dHJvbGxlciBRQ0EgUk9NRSAoMGNmMzplMDA3KSBzb21ldGltZXMgc3RvcHMgd29ya2luZwo+ID4g Pj4gYWZ0ZXIgUzM6Cj4gPiA+PiBbIDE2NS4xMTA3NDJdIEJsdWV0b290aDogaGNpMDogdXNpbmcg TlZNIGZpbGU6IHFjYS9udm1fdXNiXzAwMDAwMzAyLmJpbgo+ID4gPj4gWyAxNjguNDMyMDY1XSBC bHVldG9vdGg6IGhjaTA6IEZhaWxlZCB0byBzZW5kIGJvZHkgYXQgNCBvZiAxOTUzICgtMTEwKQo+ ID4gPj4gCj4gPiA+PiBBZnRlciBzb21lIGV4cGVyaW1lbnRzLCBJIGZvdW5kIHRoYXQgZGlzYWJs aW5nIExQTSBjYW4gd29ya2Fyb3VuZCB0aGUKPiA+ID4+IGlzc3VlLgo+ID4gPj4gCj4gPiA+PiBP biBzb21lIHBsYXRmb3JtcywgdGhlIFVTQiBwb3dlciBpcyBjdXQgZHVyaW5nIFMzLCBzbyB0aGUg ZHJpdmVyIHVzZXMKPiA+ID4+IHJlc2V0LXJlc3VtZSB0byByZXN1bWUgdGhlIGRldmljZS4gRHVy aW5nIHBvcnQgcmVzdW1lLCBMUE0gZ2V0cyBlbmFibGVkCj4gPiA+PiB0d2ljZSwgYnkgdXNiX3Jl c2V0X2FuZF92ZXJpZnlfZGV2aWNlKCkgYW5kIHVzYl9wb3J0X3Jlc3VtZSgpLgo+ID4gPj4gCj4g PiA+PiBTbyBsZXQncyBlbmFibGUgTFBNIGZvciBqdXN0IG9uY2UsIGFzIHRoaXMgc29sdmVzIHRo ZSBpc3N1ZSBmb3IgdGhlCj4gPiA+PiBkZXZpY2UgaW4gcXVlc3Rpb24uCj4gPiA+PiAKPiA+ID4+ IEFsc28gY29uc29saWRhdGUgVVNCMiBMUE0gZnVuY3Rpb25zIHRvIHVzYl9lbmFibGVfdXNiMl9o YXJkd2FyZV9scG0oKQo+ID4gPj4gYW5kIHVzYl9kaXNhYmxlX3VzYjJfaGFyZHdhcmVfbHBtKCku Cj4gPiA+IAo+ID4gPiBJIHRob3VnaHQgSSBhc2tlZCBmb3IgdGhpcyB0byBiZSB0d28gZGlmZmVy ZW50IHBhdGNoZXMuICBPbmUgdGhhdCBkb2VzCj4gPiA+IHRoZSAiY29uc29saWRhdGlvbiIsIGFu ZCB0aGVuIG9uZSB0aGF0IGZpeGVzIHRoZSBidWcuICBZb3UgYXJlIG1peGluZwo+ID4gPiB0d28g ZGlmZmVyZW50IHRoaW5ncyBoZXJlIHRvZ2V0aGVyLCBtYWtpbmcgaXQgaGFyZGVyIHRvIHJldmll dy4KPiA+ID4gCj4gPiA+IENhbiB5b3UgcGxlYXNlIGJyZWFrIHRoaXMgdXAgYW5kIHNlbmQgYSBw YXRjaCBzZXJpZXMsIHdpdGggdGhlIGNvcnJlY3QKPiA+ID4gIkZpeGVzOiIgdGFnIGFkZGVkIHRv IHRoZSBzZWNvbmQgcGF0Y2ggdGhhdCBhY3R1YWxseSBmaXhlcyB0aGUgaXNzdWU/Cj4gPiAKPiA+ IFRoZSBjb25zb2xpZGF0aW9uIGl0c2VsZiBpcyB0aGUgZml4LCBzbyBJIGFtIG5vdCBzdXJlIGhv dyB0byBicmVhayB0aGlzIHVwLgo+ID4gCj4gPiBJbiByZXNldC1yZXN1bWUgY2FzZSwgTFBNIGdl dHMgZW5hYmxlZCB0d2ljZSwgYnkKPiA+IHVzYl9yZXNldF9hbmRfdmVyaWZ5X2RldmljZSgpIGFu ZCB1c2JfcG9ydF9yZXN1bWUoKS4KPiA+IAo+ID4gSWYgaXTigJlzIGEgbm9ybWFsIHJlc3VtZSwg TFBNIG9ubHkgZ2V0cyBlbmFibGVkIG9uY2UsIGJ5Cj4gPiB1c2JfcG9ydF9yZXN1bWUoKS4KPiA+ IAo+ID4gU2luY2UgYWxsIHRocmVlIGNoZWNrcyAoY2FwYWJsZSwgYWxsb3dlZCBhbmQgZW5hYmxl ZCkgYXJlIG1lcmdlZCB0bwo+ID4gYSBzaW5nbGUgcGxhY2UsIGVuYWJsaW5nIExQTSB0d2ljZSBj YW4gYmUgYXZvaWRlZCwgaGVuY2UgZml4aW5nIHRoZQo+ID4gaXNzdWUuCj4gCj4gT25lIGFwcHJv YWNoIHdvdWxkIGJlIHRvIGhhdmUgdGhlIGZpcnN0IHBhdGNoIGFkZCB0aGUgbmV3IGZ1bmN0aW9u cyBhbmQKPiBjaGFuZ2UgdGhlIGNvZGUgdG8gY2FsbCB0aGVtIGluc3RlYWQgb2YgdGhlIG9yaWdp bmFsIGZ1bmN0aW9uLCBidXQKPiBsZWF2ZXMgdGhlIGNoZWNrcyB0aGUgd2F5IHRoZXkgYXJlIG5v dy4gIFRoZW4gdGhlIHNlY29uZCBwYXRjaCBjb3VsZAo+IGFkZCB0aGUgY2hlY2tzIHRvIHRoZSBu ZXcgZnVuY3Rpb25zIGFuZCByZW1vdmUgdGhlbSBmcm9tIHRoZSBjYWxsCj4gc2l0ZXMuCgpZZXMs IHRoYXQgaXMgd2hhdCBJIHdhcyBsb29raW5nIGZvci4gIFRoYXQgd2F5LCBpZiB0aGUgImNoYW5n ZSIgcmVhbGx5CmRvZXMgY2F1c2UgcHJvYmxlbXMsIGl0IGlzIGVhc2llciB0byByZXZlcnQvZml4 LgoKdGhhbmtzLAoKZ3JlZyBrLWgK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 483CBC43387 for ; Fri, 11 Jan 2019 09:12:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 14E3220870 for ; Fri, 11 Jan 2019 09:12:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1547197933; bh=a5vdxkiCxGUC+CWiZ4OxDgPAJO9dU4j9H28OhIY1ne8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=gvUn3SQ6DjUfObyUVxdwWRdUPd6iSOKCp+NaloVmuZG2dUOI7UoH9JCP5bxsVu3Ho LRpDQnAhrDXjFU2oTQPuinDHPFaELwTSX2A3LjzFwzu7JlARyaLvOsOtFy2TJWrkO5 dE72MXre2AKOUUFKBeZrqxLGqGUa4yKcq9vseqtU= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731105AbfAKJML (ORCPT ); Fri, 11 Jan 2019 04:12:11 -0500 Received: from mail.kernel.org ([198.145.29.99]:55858 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725601AbfAKJML (ORCPT ); Fri, 11 Jan 2019 04:12:11 -0500 Received: from localhost (unknown [178.228.36.116]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 2CF1620870; Fri, 11 Jan 2019 09:12:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1547197930; bh=a5vdxkiCxGUC+CWiZ4OxDgPAJO9dU4j9H28OhIY1ne8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=MG2nfaeJ9VDaX+mrUM/veVXudH7F6LDfKEi02hmTB8AUTffVEhFUCHXrMZZKEK6g8 MvTwd/mG7iHvIA+tsrA72G1O+QCl2xIY3ipDn7GvxkLYdf3xoh+zTEsIWcs8LV4frI 86LD1fsxGwKTaH6K8G3L6aLhJNCt+k/qKkitSQUI= Date: Fri, 11 Jan 2019 10:12:07 +0100 From: Greg KH To: Alan Stern Cc: Kai Heng Feng , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4] USB: Don't enable LPM if it's already enabled Message-ID: <20190111091207.GE15610@kroah.com> References: <98BF2E31-2F7F-47FA-B591-40B2FFBC65F1@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.11.2 (2019-01-07) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 08, 2019 at 12:34:17PM -0500, Alan Stern wrote: > On Wed, 9 Jan 2019, Kai Heng Feng wrote: > > > > On Jan 8, 2019, at 11:41 PM, Greg KH wrote: > > > > > > On Mon, Dec 03, 2018 at 06:26:43PM +0800, Kai-Heng Feng wrote: > > >> USB Bluetooth controller QCA ROME (0cf3:e007) sometimes stops working > > >> after S3: > > >> [ 165.110742] Bluetooth: hci0: using NVM file: qca/nvm_usb_00000302.bin > > >> [ 168.432065] Bluetooth: hci0: Failed to send body at 4 of 1953 (-110) > > >> > > >> After some experiments, I found that disabling LPM can workaround the > > >> issue. > > >> > > >> On some platforms, the USB power is cut during S3, so the driver uses > > >> reset-resume to resume the device. During port resume, LPM gets enabled > > >> twice, by usb_reset_and_verify_device() and usb_port_resume(). > > >> > > >> So let's enable LPM for just once, as this solves the issue for the > > >> device in question. > > >> > > >> Also consolidate USB2 LPM functions to usb_enable_usb2_hardware_lpm() > > >> and usb_disable_usb2_hardware_lpm(). > > > > > > I thought I asked for this to be two different patches. One that does > > > the "consolidation", and then one that fixes the bug. You are mixing > > > two different things here together, making it harder to review. > > > > > > Can you please break this up and send a patch series, with the correct > > > "Fixes:" tag added to the second patch that actually fixes the issue? > > > > The consolidation itself is the fix, so I am not sure how to break this up. > > > > In reset-resume case, LPM gets enabled twice, by > > usb_reset_and_verify_device() and usb_port_resume(). > > > > If it’s a normal resume, LPM only gets enabled once, by > > usb_port_resume(). > > > > Since all three checks (capable, allowed and enabled) are merged to > > a single place, enabling LPM twice can be avoided, hence fixing the > > issue. > > One approach would be to have the first patch add the new functions and > change the code to call them instead of the original function, but > leaves the checks the way they are now. Then the second patch could > add the checks to the new functions and remove them from the call > sites. Yes, that is what I was looking for. That way, if the "change" really does cause problems, it is easier to revert/fix. thanks, greg k-h