Re: [PATCH 1/2] Bluetooth: hci_bcm: Don't change runtime-status when not using runtime-pm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marcel,

On 03-04-18 16:16, Marcel Holtmann wrote:
Hi Hans,

The pm_runtime_set_suspended() call in bcm_close() is intended as a
counterpart to the pm_runtime_set_active() call from bcm_request_irq()
(which gets called from bcm_setup()).

The pm_runtime_set_active() call only happens if our pre-requisites for
doing runtime-pm are met. This commit adds the same check to the
pm_runtime_set_suspended() call to avoid the runtime-pm state getting
stuck in suspended after after a bcm_close().

Have you actually observed the state getting stuck in suspended?
I'm asking because I don't quite see how this could happen:
pm_runtime_set_suspended() becomes a no-op unless runtime PM is disabled.
Now, a few lines further up in bcm_close(), pm_runtime_disable() is only
called under the conditions you're adding here.  If it's not called,
pm_runtime_set_suspended() doesn't have any effect anyway so adding the
conditions is pointless.  Am I missing something?

All devices start with runtime-pm disabled and on devices without
an IRQ we never enable it. So when the pm_runtime_set_suspended()
gets called runtime-pm is still in its initial disabled state.
Okay but the device's initial state is set to RPM_SUSPENDED by
pm_runtime_init() and AFAICS neither the platform nor acpi buses
subsequently set it to RPM_ACTIVE, so the device is in suspended
state anyway, unless I've overlooked some detail.

Ah, no you're right, I assumed that devices would default to
RPM_ACTIVE (which corresponds to value 0), but you are right
they are set to suspended by default.

So this patch can be dropped.

Marcel: It would still be nice to get the second patch from
this series merged.

can you resent that patch?

Done.

And should it also go into stable?

I'm not sure, the hardware which needs this is somewhat rare I guess,
so I did not add a Cc: stable myself. But if you think this makes sense
for stable feel free to add a Cc: stable I've no objections against that.

Regards,

Hans



--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux