Re: [PATCH v2 4/4] Bluetooth: hci_bcm: Add suspend/resume runtime PM functions

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

 



Hello Ilya,

On 07/09/2015 23:32, Ilya Faenson wrote:
<snip>
@@ -726,7 +826,10 @@ MODULE_DEVICE_TABLE(acpi, bcm_acpi_match);
>>#endif
>>
>>/* Platform suspend and resume callbacks */
>>-static SIMPLE_DEV_PM_OPS(bcm_pm_ops, bcm_suspend, bcm_resume);
>>+static const struct dev_pm_ops bcm_pm_ops = {
>>+	SET_SYSTEM_SLEEP_PM_OPS(bcm_suspend, bcm_resume)
>>+	SET_RUNTIME_PM_OPS(bcm_runtime_suspend, bcm_runtime_resume, NULL)
>
>I think you really need to explain why runtime suspend and system sleep is actually different for Broadcom based devices. Since for the Intel ones it turned out to be the same (at least for now).
I dissociate system sleep and runtime suspend functions for the
following reasons:

1) IRQ is enabled as a wake source only during system sleep, while this
is not needed for runtime suspend.

IF: The ideal implementation should suspend both the UART and the BT device at runtime. UART should be suspended in a state where the device is flow-controlled so the device is unable to send. BT device GPIO based interrupt is then needed for runtime resume initiated by the device. Upon the interrupt, your code will resume the UART and allow the device to transmit again. If you leave the UART on w/o flow-controlling the device in suspend you in fact don't need that interrupt . However, OEMs will not like that due to the unnecessary UART power consumption. Speaking from experience.:-)

Sorry if I was not quite clear enough.

Both bcm_runtime_suspend() and bcm_suspend() enable flow control and deassert device_wakeup GPIO in the same way, so that device can resume the link via the irq. The bcm_suspend just adds the capability for the IRQ to wake-up the platform (if device_may_wakeup). On resume we disable only this capability, meaning that IRQ remains enabled.


Thanks,
  -Ilya

2) runtime suspend functions do not need to protect usage of dev->hu as
these functions are called with a valid dev->hu (these functions are
disabled before dev->hu is set to NULL when BCM driver is closed).
System sleep functions need this protection as they can be called even
when driver is not opened, and need to ensure that dev->hu is valid
until GPIO and UART flow control management is performed.

Common part (GPIO and UART flow control management) for runtime suspend
and system sleep moved to __bcm_suspend() and __bcm_resume() in previous
patch.

Regards

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



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux