Messages in this thread | | | Subject | Re: [PATCH v4] USB: Don't enable LPM if it's already enabled | From | Kai Heng Feng <> | Date | Wed, 9 Jan 2019 00:04:08 +0800 |
| |
> On Jan 8, 2019, at 11:41 PM, Greg KH <gregkh@linuxfoundation.org> 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.
Kai-Heng
> > thanks, > > greg k-h
| |