Re: [PATCH 4.16 REGRESSION fix 1/2] Revert "Bluetooth: hci_bcm: Streamline runtime PM code"

[Lukas Wunner <lukas@wunner.de>]

On Thu, Mar 15, 2018 at 08:49:04AM +0100, Hans de Goede wrote:
> On 14-03-18 23:38, Lukas Wunner wrote:
> > On Wed, Mar 14, 2018 at 11:23:12PM +0100, Hans de Goede wrote:
> > >We're quite far into the cycle already and this is a serious regression,
> > >also nothing of great value is lost by the revert, the original commit
> > >was a minor cleanup which turns out to have bad side-effects, a simple
> > >revert really is the best solution here, esp. in this point of the cycle.
> >
> > Just an hour ago he sent me the patch to look over it.  And we're at
> > least two and a half weeks away from v4.16.
> 
> No we are *only* two and a half weeks away from v4.16 (worst case scenario)
> and Linus does not like getting last minute fixes.

That doesn't preclude allowing a few hours to discuss things.
There is never such a rush.  In the present case, a new contributor
was willing to debug the issue and submit a patch.  Onboarding new
contributors is important and IMO it's worth waiting a few days for
them to sort things out, even if it means a regression stays present
a little longer.  I'm sorry that it meant you wasted time debugging
it in parallel.

That said, when submitting the patch I clearly failed to notice that
for devices using autosuspend, pm_request_resume() doesn't update
the last usage timestamp.  While that could be fixed by calling
pm_runtime_mark_last_busy() before pm_request_resume(), it doesn't
seem to be customary as a look at all the call sites of
pm_request_resume() shows.  The original three-line sequence,
although quite verbose, appears to be what is commonly used in such
a case.  For this reason reverting back to the original version
seems justified.

Thanks,

Lukas