[PATCH v5] Bluetooth: btintel_pcie: Add support for _suspend() / _resume()

[PATCH v5] Bluetooth: btintel_pcie: Add support for _suspend() / _resume()

[Kiran K]

From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>

This patch implements _suspend() and _resume() functions for the
Bluetooth controller. When the system enters a suspended state, the
driver notifies the controller to perform necessary housekeeping tasks
by writing to the sleep control register and waits for an alive
interrupt. The firmware raises the alive interrupt when it has
transitioned to the D3 state. The same flow occurs when the system
resumes.

Command to test host initiated wakeup after 60 seconds
sudo rtcwake -m mem -s 60

dmesg log (tested on Whale Peak2 on Panther Lake platform)
On system suspend:
[  516.418316] Bluetooth: hci0: device entered into d3 state from d0 in 81 us

On system resume:
[  542.174128] Bluetooth: hci0: device entered into d0 state from d3 in 357 us

Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
changes in v5:
     - Address review comments

changes in v4:
     - Moved document and section details from the commit message as comment in code.

changes in v3:
     - Corrected the typo's
     - Updated the CC list as suggested.
     - Corrected the format specifiers in the logs.

changes in v2:
     - Updated the commit message with test steps and logs.
     - Added logs to include the timeout message.
     - Fixed a potential race condition during suspend and resume.

 drivers/bluetooth/btintel_pcie.c | 102 +++++++++++++++++++++++++++++++
 drivers/bluetooth/btintel_pcie.h |   4 ++
 2 files changed, 106 insertions(+)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 6e7bbbd35279..a96975a55cbe 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -540,6 +540,12 @@ static int btintel_pcie_reset_bt(struct btintel_pcie_data *data)
 	return reg == 0 ? 0 : -ENODEV;
 }
 
+static void btintel_pcie_set_persistence_mode(struct btintel_pcie_data *data)
+{
+	btintel_pcie_set_reg_bits(data, BTINTEL_PCIE_CSR_HW_BOOT_CONFIG,
+				  BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON);
+}
+
 static void btintel_pcie_mac_init(struct btintel_pcie_data *data)
 {
 	u32 reg;
@@ -829,6 +835,8 @@ static int btintel_pcie_enable_bt(struct btintel_pcie_data *data)
 	 */
 	data->boot_stage_cache = 0x0;
 
+	btintel_pcie_set_persistence_mode(data);
+
 	/* Set MAC_INIT bit to start primary bootloader */
 	reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
 	reg &= ~(BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_INIT |
@@ -2573,11 +2581,105 @@ static void btintel_pcie_coredump(struct device *dev)
 }
 #endif
 
+#ifdef CONFIG_PM
+static int btintel_pcie_suspend_late(struct device *dev, pm_message_t mesg)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct btintel_pcie_data *data;
+	ktime_t start;
+	u32 dxstate;
+	s64 delta;
+	int err;
+
+	data = pci_get_drvdata(pdev);
+
+	if (mesg.event == PM_EVENT_SUSPEND)
+		dxstate = BTINTEL_PCIE_STATE_D3_HOT;
+	else
+		dxstate = BTINTEL_PCIE_STATE_D3_COLD;
+
+	data->gp0_received = false;
+
+	start = ktime_get();
+
+	/* Refer: 6.4.11.7 -> Platform power management */
+	btintel_pcie_wr_sleep_cntrl(data, dxstate);
+	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
+				 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
+	delta = ktime_to_ns(ktime_get() - start) / 1000;
+
+	if (err == 0) {
+		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D3 entry",
+				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
+		return -EBUSY;
+	}
+	bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld us",
+		    delta);
+	return 0;
+}
+
+static int btintel_pcie_suspend(struct device *dev)
+{
+	return btintel_pcie_suspend_late(dev, PMSG_SUSPEND);
+}
+
+static int btintel_pcie_hibernate(struct device *dev)
+{
+	return btintel_pcie_suspend_late(dev, PMSG_HIBERNATE);
+}
+
+static int btintel_pcie_freeze(struct device *dev)
+{
+	return btintel_pcie_suspend_late(dev, PMSG_FREEZE);
+}
+
+static int btintel_pcie_resume(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct btintel_pcie_data *data;
+	ktime_t start;
+	int err;
+	s64 delta;
+
+	data = pci_get_drvdata(pdev);
+	data->gp0_received = false;
+
+	start = ktime_get();
+
+	/* Refer: 6.4.11.7 -> Platform power management */
+	btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
+	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
+				 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
+	delta = ktime_to_ns(ktime_get() - start) / 1000;
+
+	if (err == 0) {
+		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D0 entry",
+				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
+		return -EBUSY;
+	}
+	bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld us",
+		    delta);
+	return 0;
+}
+
+const struct dev_pm_ops btintel_pcie_pm_ops = {
+	.suspend = btintel_pcie_suspend,
+	.resume = btintel_pcie_resume,
+	.freeze = btintel_pcie_freeze,
+	.thaw = btintel_pcie_resume,
+	.poweroff = btintel_pcie_hibernate,
+	.restore = btintel_pcie_resume,
+};
+#define BTINTELPCIE_PM_OPS	(&btintel_pcie_pm_ops)
+#else
+#define BTINTELPCIE_PM_OPS	NULL
+#endif
 static struct pci_driver btintel_pcie_driver = {
 	.name = KBUILD_MODNAME,
 	.id_table = btintel_pcie_table,
 	.probe = btintel_pcie_probe,
 	.remove = btintel_pcie_remove,
+	.driver.pm = BTINTELPCIE_PM_OPS,
 #ifdef CONFIG_DEV_COREDUMP
 	.driver.coredump = btintel_pcie_coredump
 #endif
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index 0fa876c5b954..5bc69004b692 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -8,6 +8,7 @@
 
 /* Control and Status Register(BTINTEL_PCIE_CSR) */
 #define BTINTEL_PCIE_CSR_BASE			(0x000)
+#define BTINTEL_PCIE_CSR_HW_BOOT_CONFIG		(BTINTEL_PCIE_CSR_BASE + 0x000)
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_REG		(BTINTEL_PCIE_CSR_BASE + 0x024)
 #define BTINTEL_PCIE_CSR_HW_REV_REG		(BTINTEL_PCIE_CSR_BASE + 0x028)
 #define BTINTEL_PCIE_CSR_RF_ID_REG		(BTINTEL_PCIE_CSR_BASE + 0x09C)
@@ -55,6 +56,9 @@
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_ALIVE		(BIT(23))
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY	(BIT(24))
 
+/* CSR HW BOOT CONFIG Register */
+#define BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON		(BIT(31))
+
 /* Registers for MSI-X */
 #define BTINTEL_PCIE_CSR_MSIX_BASE		(0x2000)
 #define BTINTEL_PCIE_CSR_MSIX_FH_INT_CAUSES	(BTINTEL_PCIE_CSR_MSIX_BASE + 0x0800)
-- 
2.43.0

Re: [PATCH v5] Bluetooth: btintel_pcie: Add support for _suspend() / _resume()

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed, 23 Jul 2025 19:27:15 +0530 you wrote:
> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> 
> This patch implements _suspend() and _resume() functions for the
> Bluetooth controller. When the system enters a suspended state, the
> driver notifies the controller to perform necessary housekeeping tasks
> by writing to the sleep control register and waits for an alive
> interrupt. The firmware raises the alive interrupt when it has
> transitioned to the D3 state. The same flow occurs when the system
> resumes.
> 
> [...]

Here is the summary with links:
  - [v5] Bluetooth: btintel_pcie: Add support for _suspend() / _resume()
    https://git.kernel.org/bluetooth/bluetooth-next/c/33bb9b1ce6f6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v5] Bluetooth: btintel_pcie: Add support for _suspend() / _resume()

[Manivannan Sadhasivam]

On Wed, Jul 23, 2025 at 07:27:15PM GMT, Kiran K wrote:
> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> 
> This patch implements _suspend() and _resume() functions for the
> Bluetooth controller. When the system enters a suspended state, the
> driver notifies the controller to perform necessary housekeeping tasks
> by writing to the sleep control register and waits for an alive
> interrupt. The firmware raises the alive interrupt when it has
> transitioned to the D3 state. The same flow occurs when the system
> resumes.
> 
> Command to test host initiated wakeup after 60 seconds
> sudo rtcwake -m mem -s 60
> 
> dmesg log (tested on Whale Peak2 on Panther Lake platform)
> On system suspend:
> [  516.418316] Bluetooth: hci0: device entered into d3 state from d0 in 81 us
> 
> On system resume:
> [  542.174128] Bluetooth: hci0: device entered into d0 state from d3 in 357 us
> 
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
> changes in v5:
>      - Address review comments
> 
> changes in v4:
>      - Moved document and section details from the commit message as comment in code.
> 
> changes in v3:
>      - Corrected the typo's
>      - Updated the CC list as suggested.
>      - Corrected the format specifiers in the logs.
> 
> changes in v2:
>      - Updated the commit message with test steps and logs.
>      - Added logs to include the timeout message.
>      - Fixed a potential race condition during suspend and resume.
> 
>  drivers/bluetooth/btintel_pcie.c | 102 +++++++++++++++++++++++++++++++
>  drivers/bluetooth/btintel_pcie.h |   4 ++
>  2 files changed, 106 insertions(+)
> 
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 6e7bbbd35279..a96975a55cbe 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -540,6 +540,12 @@ static int btintel_pcie_reset_bt(struct btintel_pcie_data *data)
>  	return reg == 0 ? 0 : -ENODEV;
>  }
>  
> +static void btintel_pcie_set_persistence_mode(struct btintel_pcie_data *data)
> +{
> +	btintel_pcie_set_reg_bits(data, BTINTEL_PCIE_CSR_HW_BOOT_CONFIG,
> +				  BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON);
> +}
> +
>  static void btintel_pcie_mac_init(struct btintel_pcie_data *data)
>  {
>  	u32 reg;
> @@ -829,6 +835,8 @@ static int btintel_pcie_enable_bt(struct btintel_pcie_data *data)
>  	 */
>  	data->boot_stage_cache = 0x0;
>  
> +	btintel_pcie_set_persistence_mode(data);
> +
>  	/* Set MAC_INIT bit to start primary bootloader */
>  	reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>  	reg &= ~(BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_INIT |
> @@ -2573,11 +2581,105 @@ static void btintel_pcie_coredump(struct device *dev)
>  }
>  #endif
>  
> +#ifdef CONFIG_PM
> +static int btintel_pcie_suspend_late(struct device *dev, pm_message_t mesg)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct btintel_pcie_data *data;
> +	ktime_t start;
> +	u32 dxstate;
> +	s64 delta;
> +	int err;
> +
> +	data = pci_get_drvdata(pdev);
> +
> +	if (mesg.event == PM_EVENT_SUSPEND)
> +		dxstate = BTINTEL_PCIE_STATE_D3_HOT;
> +	else
> +		dxstate = BTINTEL_PCIE_STATE_D3_COLD;
> +
> +	data->gp0_received = false;
> +
> +	start = ktime_get();
> +
> +	/* Refer: 6.4.11.7 -> Platform power management */
> +	btintel_pcie_wr_sleep_cntrl(data, dxstate);
> +	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
> +				 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
> +	delta = ktime_to_ns(ktime_get() - start) / 1000;
> +
> +	if (err == 0) {
> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D3 entry",
> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> +		return -EBUSY;
> +	}
> +	bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld us",
> +		    delta);
> +	return 0;
> +}
> +
> +static int btintel_pcie_suspend(struct device *dev)
> +{
> +	return btintel_pcie_suspend_late(dev, PMSG_SUSPEND);
> +}
> +
> +static int btintel_pcie_hibernate(struct device *dev)
> +{
> +	return btintel_pcie_suspend_late(dev, PMSG_HIBERNATE);
> +}
> +
> +static int btintel_pcie_freeze(struct device *dev)
> +{
> +	return btintel_pcie_suspend_late(dev, PMSG_FREEZE);
> +}
> +
> +static int btintel_pcie_resume(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct btintel_pcie_data *data;
> +	ktime_t start;
> +	int err;
> +	s64 delta;
> +
> +	data = pci_get_drvdata(pdev);
> +	data->gp0_received = false;
> +
> +	start = ktime_get();
> +
> +	/* Refer: 6.4.11.7 -> Platform power management */
> +	btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
> +	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
> +				 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
> +	delta = ktime_to_ns(ktime_get() - start) / 1000;
> +
> +	if (err == 0) {
> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D0 entry",
> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> +		return -EBUSY;
> +	}
> +	bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld us",
> +		    delta);
> +	return 0;
> +}
> +
> +const struct dev_pm_ops btintel_pcie_pm_ops = {
> +	.suspend = btintel_pcie_suspend,
> +	.resume = btintel_pcie_resume,
> +	.freeze = btintel_pcie_freeze,
> +	.thaw = btintel_pcie_resume,
> +	.poweroff = btintel_pcie_hibernate,
> +	.restore = btintel_pcie_resume,

You should use pm_sleep_ptr() to avoid the ifdef clutter. It will ensure that
the compiler drops the function definitions when CONFIG_PM_SLEEP is not enabled.

> +};
> +#define BTINTELPCIE_PM_OPS	(&btintel_pcie_pm_ops)
> +#else
> +#define BTINTELPCIE_PM_OPS	NULL
> +#endif
>  static struct pci_driver btintel_pcie_driver = {
>  	.name = KBUILD_MODNAME,
>  	.id_table = btintel_pcie_table,
>  	.probe = btintel_pcie_probe,
>  	.remove = btintel_pcie_remove,
> +	.driver.pm = BTINTELPCIE_PM_OPS,

Here, you should use pm_ptr() for the same reason.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v5] Bluetooth: btintel_pcie: Add support for _suspend() / _resume()

[Paul Menzel]

Dear Kiran, dear Chandrashekar,


Thank you for your patch.

Am 23.07.25 um 15:57 schrieb Kiran K:
> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> 
> This patch implements _suspend() and _resume() functions for the
> Bluetooth controller. When the system enters a suspended state, the
> driver notifies the controller to perform necessary housekeeping tasks
> by writing to the sleep control register and waits for an alive
> interrupt. The firmware raises the alive interrupt when it has
> transitioned to the D3 state. The same flow occurs when the system
> resumes.
> 
> Command to test host initiated wakeup after 60 seconds
> sudo rtcwake -m mem -s 60
> 
> dmesg log (tested on Whale Peak2 on Panther Lake platform)
> On system suspend:
> [  516.418316] Bluetooth: hci0: device entered into d3 state from d0 in 81 us
> 
> On system resume:
> [  542.174128] Bluetooth: hci0: device entered into d0 state from d3 in 357 us

Just to avoid confusion, is the timestamp correct, as this is only a 26 
s difference and your command says 60 s. (I am only aware of inaccurate 
timestamps using human-readable format (`dmesg -T`).)

> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
> changes in v5:
>       - Address review comments

This should be more detailed, if code was changed.

> changes in v4:
>       - Moved document and section details from the commit message as comment in code.
> 
> changes in v3:
>       - Corrected the typo's
>       - Updated the CC list as suggested.
>       - Corrected the format specifiers in the logs.
> 
> changes in v2:
>       - Updated the commit message with test steps and logs.
>       - Added logs to include the timeout message.
>       - Fixed a potential race condition during suspend and resume.
> 
>   drivers/bluetooth/btintel_pcie.c | 102 +++++++++++++++++++++++++++++++
>   drivers/bluetooth/btintel_pcie.h |   4 ++
>   2 files changed, 106 insertions(+)
> 
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 6e7bbbd35279..a96975a55cbe 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -540,6 +540,12 @@ static int btintel_pcie_reset_bt(struct btintel_pcie_data *data)
>   	return reg == 0 ? 0 : -ENODEV;
>   }
>   
> +static void btintel_pcie_set_persistence_mode(struct btintel_pcie_data *data)
> +{
> +	btintel_pcie_set_reg_bits(data, BTINTEL_PCIE_CSR_HW_BOOT_CONFIG,
> +				  BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON);
> +}
> +
>   static void btintel_pcie_mac_init(struct btintel_pcie_data *data)
>   {
>   	u32 reg;
> @@ -829,6 +835,8 @@ static int btintel_pcie_enable_bt(struct btintel_pcie_data *data)
>   	 */
>   	data->boot_stage_cache = 0x0;
>   
> +	btintel_pcie_set_persistence_mode(data);
> +

This hunk is not described in the commit message.

>   	/* Set MAC_INIT bit to start primary bootloader */
>   	reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>   	reg &= ~(BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_INIT |
> @@ -2573,11 +2581,105 @@ static void btintel_pcie_coredump(struct device *dev)
>   }
>   #endif
>   
> +#ifdef CONFIG_PM
> +static int btintel_pcie_suspend_late(struct device *dev, pm_message_t mesg)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct btintel_pcie_data *data;
> +	ktime_t start;
> +	u32 dxstate;
> +	s64 delta;

Append the unit to the name? `delta_us`?

> +	int err;
> +
> +	data = pci_get_drvdata(pdev);
> +
> +	if (mesg.event == PM_EVENT_SUSPEND)
> +		dxstate = BTINTEL_PCIE_STATE_D3_HOT;
> +	else
> +		dxstate = BTINTEL_PCIE_STATE_D3_COLD;

I’d use the ternary operator.

> +
> +	data->gp0_received = false;
> +
> +	start = ktime_get();
> +
> +	/* Refer: 6.4.11.7 -> Platform power management */
> +	btintel_pcie_wr_sleep_cntrl(data, dxstate);
> +	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
> +				 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
> +	delta = ktime_to_ns(ktime_get() - start) / 1000;

Move it below right before `bt_dev_info`?

> +
> +	if (err == 0) {
> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D3 entry",
> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> +		return -EBUSY;
> +	}
> +	bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld us",
> +		    delta);
> +	return 0;
> +}
> +
> +static int btintel_pcie_suspend(struct device *dev)
> +{
> +	return btintel_pcie_suspend_late(dev, PMSG_SUSPEND);
> +}
> +
> +static int btintel_pcie_hibernate(struct device *dev)
> +{
> +	return btintel_pcie_suspend_late(dev, PMSG_HIBERNATE);
> +}
> +
> +static int btintel_pcie_freeze(struct device *dev)
> +{
> +	return btintel_pcie_suspend_late(dev, PMSG_FREEZE);
> +}
> +
> +static int btintel_pcie_resume(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct btintel_pcie_data *data;
> +	ktime_t start;
> +	int err;
> +	s64 delta;
> +
> +	data = pci_get_drvdata(pdev);
> +	data->gp0_received = false;
> +
> +	start = ktime_get();
> +
> +	/* Refer: 6.4.11.7 -> Platform power management */
> +	btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
> +	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
> +				 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
> +	delta = ktime_to_ns(ktime_get() - start) / 1000;
> +
> +	if (err == 0) {
> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D0 entry",
> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> +		return -EBUSY;
> +	}
> +	bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld us",
> +		    delta);
> +	return 0;
> +}
> +
> +const struct dev_pm_ops btintel_pcie_pm_ops = {
> +	.suspend = btintel_pcie_suspend,
> +	.resume = btintel_pcie_resume,
> +	.freeze = btintel_pcie_freeze,
> +	.thaw = btintel_pcie_resume,
> +	.poweroff = btintel_pcie_hibernate,
> +	.restore = btintel_pcie_resume,
> +};
> +#define BTINTELPCIE_PM_OPS	(&btintel_pcie_pm_ops)
> +#else
> +#define BTINTELPCIE_PM_OPS	NULL
> +#endif
>   static struct pci_driver btintel_pcie_driver = {
>   	.name = KBUILD_MODNAME,
>   	.id_table = btintel_pcie_table,
>   	.probe = btintel_pcie_probe,
>   	.remove = btintel_pcie_remove,
> +	.driver.pm = BTINTELPCIE_PM_OPS,
>   #ifdef CONFIG_DEV_COREDUMP
>   	.driver.coredump = btintel_pcie_coredump
>   #endif
> diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
> index 0fa876c5b954..5bc69004b692 100644
> --- a/drivers/bluetooth/btintel_pcie.h
> +++ b/drivers/bluetooth/btintel_pcie.h
> @@ -8,6 +8,7 @@
>   
>   /* Control and Status Register(BTINTEL_PCIE_CSR) */
>   #define BTINTEL_PCIE_CSR_BASE			(0x000)
> +#define BTINTEL_PCIE_CSR_HW_BOOT_CONFIG		(BTINTEL_PCIE_CSR_BASE + 0x000)
>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_REG		(BTINTEL_PCIE_CSR_BASE + 0x024)
>   #define BTINTEL_PCIE_CSR_HW_REV_REG		(BTINTEL_PCIE_CSR_BASE + 0x028)
>   #define BTINTEL_PCIE_CSR_RF_ID_REG		(BTINTEL_PCIE_CSR_BASE + 0x09C)
> @@ -55,6 +56,9 @@
>   #define BTINTEL_PCIE_CSR_BOOT_STAGE_ALIVE		(BIT(23))
>   #define BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY	(BIT(24))
>   
> +/* CSR HW BOOT CONFIG Register */
> +#define BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON		(BIT(31))
> +
>   /* Registers for MSI-X */
>   #define BTINTEL_PCIE_CSR_MSIX_BASE		(0x2000)
>   #define BTINTEL_PCIE_CSR_MSIX_FH_INT_CAUSES	(BTINTEL_PCIE_CSR_MSIX_BASE + 0x0800)


Kind regards,

Paul

RE: [PATCH v5] Bluetooth: btintel_pcie: Add support for _suspend() / _resume()

[K, Kiran]

Hi Manivannan,

Thanks for the comments.

>-----Original Message-----
>From: Manivannan Sadhasivam <mani@kernel.org>
>Sent: Thursday, July 24, 2025 10:37 AM
>To: K, Kiran <kiran.k@intel.com>
>Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar
><ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan
><chethan.tumkur.narayan@intel.com>; bhelgaas@google.com; linux-
>pci@vger.kernel.org; Devegowda, Chandrashekar
><chandrashekar.devegowda@intel.com>
>Subject: Re: [PATCH v5] Bluetooth: btintel_pcie: Add support for _suspend() /
>_resume()
>
>On Wed, Jul 23, 2025 at 07:27:15PM GMT, Kiran K wrote:
>> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
>>
>> This patch implements _suspend() and _resume() functions for the
>> Bluetooth controller. When the system enters a suspended state, the
>> driver notifies the controller to perform necessary housekeeping tasks
>> by writing to the sleep control register and waits for an alive
>> interrupt. The firmware raises the alive interrupt when it has
>> transitioned to the D3 state. The same flow occurs when the system
>> resumes.
>>
>> Command to test host initiated wakeup after 60 seconds sudo rtcwake -m
>> mem -s 60
>>
>> dmesg log (tested on Whale Peak2 on Panther Lake platform) On system
>> suspend:
>> [  516.418316] Bluetooth: hci0: device entered into d3 state from d0
>> in 81 us
>>
>> On system resume:
>> [  542.174128] Bluetooth: hci0: device entered into d0 state from d3
>> in 357 us
>>
>> Signed-off-by: Chandrashekar Devegowda
>> <chandrashekar.devegowda@intel.com>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>> changes in v5:
>>      - Address review comments
>>
>> changes in v4:
>>      - Moved document and section details from the commit message as
>comment in code.
>>
>> changes in v3:
>>      - Corrected the typo's
>>      - Updated the CC list as suggested.
>>      - Corrected the format specifiers in the logs.
>>
>> changes in v2:
>>      - Updated the commit message with test steps and logs.
>>      - Added logs to include the timeout message.
>>      - Fixed a potential race condition during suspend and resume.
>>
>>  drivers/bluetooth/btintel_pcie.c | 102 +++++++++++++++++++++++++++++++
>>  drivers/bluetooth/btintel_pcie.h |   4 ++
>>  2 files changed, 106 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index 6e7bbbd35279..a96975a55cbe 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -540,6 +540,12 @@ static int btintel_pcie_reset_bt(struct
>btintel_pcie_data *data)
>>  	return reg == 0 ? 0 : -ENODEV;
>>  }
>>
>> +static void btintel_pcie_set_persistence_mode(struct
>> +btintel_pcie_data *data) {
>> +	btintel_pcie_set_reg_bits(data,
>BTINTEL_PCIE_CSR_HW_BOOT_CONFIG,
>> +
>BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON);
>> +}
>> +
>>  static void btintel_pcie_mac_init(struct btintel_pcie_data *data)  {
>>  	u32 reg;
>> @@ -829,6 +835,8 @@ static int btintel_pcie_enable_bt(struct
>btintel_pcie_data *data)
>>  	 */
>>  	data->boot_stage_cache = 0x0;
>>
>> +	btintel_pcie_set_persistence_mode(data);
>> +
>>  	/* Set MAC_INIT bit to start primary bootloader */
>>  	reg = btintel_pcie_rd_reg32(data,
>BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>>  	reg &= ~(BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_INIT | @@ -2573,11
>> +2581,105 @@ static void btintel_pcie_coredump(struct device *dev)  }
>> #endif
>>
>> +#ifdef CONFIG_PM
>> +static int btintel_pcie_suspend_late(struct device *dev, pm_message_t
>> +mesg) {
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct btintel_pcie_data *data;
>> +	ktime_t start;
>> +	u32 dxstate;
>> +	s64 delta;
>> +	int err;
>> +
>> +	data = pci_get_drvdata(pdev);
>> +
>> +	if (mesg.event == PM_EVENT_SUSPEND)
>> +		dxstate = BTINTEL_PCIE_STATE_D3_HOT;
>> +	else
>> +		dxstate = BTINTEL_PCIE_STATE_D3_COLD;
>> +
>> +	data->gp0_received = false;
>> +
>> +	start = ktime_get();
>> +
>> +	/* Refer: 6.4.11.7 -> Platform power management */
>> +	btintel_pcie_wr_sleep_cntrl(data, dxstate);
>> +	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
>> +
>msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
>> +	delta = ktime_to_ns(ktime_get() - start) / 1000;
>> +
>> +	if (err == 0) {
>> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt
>for D3 entry",
>> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
>> +		return -EBUSY;
>> +	}
>> +	bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld
>us",
>> +		    delta);
>> +	return 0;
>> +}
>> +
>> +static int btintel_pcie_suspend(struct device *dev) {
>> +	return btintel_pcie_suspend_late(dev, PMSG_SUSPEND); }
>> +
>> +static int btintel_pcie_hibernate(struct device *dev) {
>> +	return btintel_pcie_suspend_late(dev, PMSG_HIBERNATE); }
>> +
>> +static int btintel_pcie_freeze(struct device *dev) {
>> +	return btintel_pcie_suspend_late(dev, PMSG_FREEZE); }
>> +
>> +static int btintel_pcie_resume(struct device *dev) {
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct btintel_pcie_data *data;
>> +	ktime_t start;
>> +	int err;
>> +	s64 delta;
>> +
>> +	data = pci_get_drvdata(pdev);
>> +	data->gp0_received = false;
>> +
>> +	start = ktime_get();
>> +
>> +	/* Refer: 6.4.11.7 -> Platform power management */
>> +	btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
>> +	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
>> +
>msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
>> +	delta = ktime_to_ns(ktime_get() - start) / 1000;
>> +
>> +	if (err == 0) {
>> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt
>for D0 entry",
>> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
>> +		return -EBUSY;
>> +	}
>> +	bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld
>us",
>> +		    delta);
>> +	return 0;
>> +}
>> +
>> +const struct dev_pm_ops btintel_pcie_pm_ops = {
>> +	.suspend = btintel_pcie_suspend,
>> +	.resume = btintel_pcie_resume,
>> +	.freeze = btintel_pcie_freeze,
>> +	.thaw = btintel_pcie_resume,
>> +	.poweroff = btintel_pcie_hibernate,
>> +	.restore = btintel_pcie_resume,
>
>You should use pm_sleep_ptr() to avoid the ifdef clutter. It will ensure that the
>compiler drops the function definitions when CONFIG_PM_SLEEP is not
>enabled.
Ack.

>
>> +};
>> +#define BTINTELPCIE_PM_OPS	(&btintel_pcie_pm_ops)
>> +#else
>> +#define BTINTELPCIE_PM_OPS	NULL
>> +#endif
>>  static struct pci_driver btintel_pcie_driver = {
>>  	.name = KBUILD_MODNAME,
>>  	.id_table = btintel_pcie_table,
>>  	.probe = btintel_pcie_probe,
>>  	.remove = btintel_pcie_remove,
>> +	.driver.pm = BTINTELPCIE_PM_OPS,
>
>Here, you should use pm_ptr() for the same reason.
Ack.  I will incorporate the comments.
>
>- Mani
>
>--
>மணிவண்ணன் சதாசிவம்

Thanks,
Kiran

RE: [PATCH v5] Bluetooth: btintel_pcie: Add support for _suspend() / _resume()

[K, Kiran]

Hi Paul,

Appreciate your comments.

>-----Original Message-----
>From: Paul Menzel <pmenzel@molgen.mpg.de>
>Sent: Thursday, July 24, 2025 11:48 AM
>To: K, Kiran <kiran.k@intel.com>; Devegowda, Chandrashekar
><chandrashekar.devegowda@intel.com>
>Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar
><ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan
><chethan.tumkur.narayan@intel.com>; bhelgaas@google.com; linux-
>pci@vger.kernel.org
>Subject: Re: [PATCH v5] Bluetooth: btintel_pcie: Add support for _suspend() /
>_resume()
>
>Dear Kiran, dear Chandrashekar,
>
>
>Thank you for your patch.
>
>Am 23.07.25 um 15:57 schrieb Kiran K:
>> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
>>
>> This patch implements _suspend() and _resume() functions for the
>> Bluetooth controller. When the system enters a suspended state, the
>> driver notifies the controller to perform necessary housekeeping tasks
>> by writing to the sleep control register and waits for an alive
>> interrupt. The firmware raises the alive interrupt when it has
>> transitioned to the D3 state. The same flow occurs when the system
>> resumes.
>>
>> Command to test host initiated wakeup after 60 seconds sudo rtcwake -m
>> mem -s 60
>>
>> dmesg log (tested on Whale Peak2 on Panther Lake platform) On system
>> suspend:
>> [  516.418316] Bluetooth: hci0: device entered into d3 state from d0
>> in 81 us
>>
>> On system resume:
>> [  542.174128] Bluetooth: hci0: device entered into d0 state from d3
>> in 357 us
>
>Just to avoid confusion, is the timestamp correct, as this is only a 26 s
>difference and your command says 60 s. (I am only aware of inaccurate
>timestamps using human-readable format (`dmesg -T`).)
The system was woken up by key press before the sleep timeout. To avoid the confusion, I will update the traces to reflect the system waking up independently.
>
>> Signed-off-by: Chandrashekar Devegowda
>> <chandrashekar.devegowda@intel.com>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>> changes in v5:
>>       - Address review comments
>
>This should be more detailed, if code was changed.
Ack.

>
>> changes in v4:
>>       - Moved document and section details from the commit message as
>comment in code.
>>
>> changes in v3:
>>       - Corrected the typo's
>>       - Updated the CC list as suggested.
>>       - Corrected the format specifiers in the logs.
>>
>> changes in v2:
>>       - Updated the commit message with test steps and logs.
>>       - Added logs to include the timeout message.
>>       - Fixed a potential race condition during suspend and resume.
>>
>>   drivers/bluetooth/btintel_pcie.c | 102 +++++++++++++++++++++++++++++++
>>   drivers/bluetooth/btintel_pcie.h |   4 ++
>>   2 files changed, 106 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index 6e7bbbd35279..a96975a55cbe 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -540,6 +540,12 @@ static int btintel_pcie_reset_bt(struct
>btintel_pcie_data *data)
>>   	return reg == 0 ? 0 : -ENODEV;
>>   }
>>
>> +static void btintel_pcie_set_persistence_mode(struct
>> +btintel_pcie_data *data) {
>> +	btintel_pcie_set_reg_bits(data,
>BTINTEL_PCIE_CSR_HW_BOOT_CONFIG,
>> +
>BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON);
>> +}
>> +
>>   static void btintel_pcie_mac_init(struct btintel_pcie_data *data)
>>   {
>>   	u32 reg;
>> @@ -829,6 +835,8 @@ static int btintel_pcie_enable_bt(struct
>btintel_pcie_data *data)
>>   	 */
>>   	data->boot_stage_cache = 0x0;
>>
>> +	btintel_pcie_set_persistence_mode(data);
>> +
>
>This hunk is not described in the commit message.
This register write is intended to retain firmware state across a warm reboot (S5) and is not relevant to this patch. Apologies for overlooking this detail.
>
>>   	/* Set MAC_INIT bit to start primary bootloader */
>>   	reg = btintel_pcie_rd_reg32(data,
>BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>>   	reg &= ~(BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_INIT | @@ -2573,11
>> +2581,105 @@ static void btintel_pcie_coredump(struct device *dev)
>>   }
>>   #endif
>>
>> +#ifdef CONFIG_PM
>> +static int btintel_pcie_suspend_late(struct device *dev, pm_message_t
>> +mesg) {
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct btintel_pcie_data *data;
>> +	ktime_t start;
>> +	u32 dxstate;
>> +	s64 delta;
>
>Append the unit to the name? `delta_us`?
Ack
>
>> +	int err;
>> +
>> +	data = pci_get_drvdata(pdev);
>> +
>> +	if (mesg.event == PM_EVENT_SUSPEND)
>> +		dxstate = BTINTEL_PCIE_STATE_D3_HOT;
>> +	else
>> +		dxstate = BTINTEL_PCIE_STATE_D3_COLD;
>
>I’d use the ternary operator.
Ack.

>
>> +
>> +	data->gp0_received = false;
>> +
>> +	start = ktime_get();
>> +
>> +	/* Refer: 6.4.11.7 -> Platform power management */
>> +	btintel_pcie_wr_sleep_cntrl(data, dxstate);
>> +	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
>> +
>msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
>> +	delta = ktime_to_ns(ktime_get() - start) / 1000;
>
>Move it below right before `bt_dev_info`?
Ack.

>
>> +
>> +	if (err == 0) {
>> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt
>for D3 entry",
>> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
>> +		return -EBUSY;
>> +	}
>> +	bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld
>us",
>> +		    delta);
>> +	return 0;
>> +}
>> +
>> +static int btintel_pcie_suspend(struct device *dev) {
>> +	return btintel_pcie_suspend_late(dev, PMSG_SUSPEND); }
>> +
>> +static int btintel_pcie_hibernate(struct device *dev) {
>> +	return btintel_pcie_suspend_late(dev, PMSG_HIBERNATE); }
>> +
>> +static int btintel_pcie_freeze(struct device *dev) {
>> +	return btintel_pcie_suspend_late(dev, PMSG_FREEZE); }
>> +
>> +static int btintel_pcie_resume(struct device *dev) {
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct btintel_pcie_data *data;
>> +	ktime_t start;
>> +	int err;
>> +	s64 delta;
>> +
>> +	data = pci_get_drvdata(pdev);
>> +	data->gp0_received = false;
>> +
>> +	start = ktime_get();
>> +
>> +	/* Refer: 6.4.11.7 -> Platform power management */
>> +	btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
>> +	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
>> +
>msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
>> +	delta = ktime_to_ns(ktime_get() - start) / 1000;
>> +
>> +	if (err == 0) {
>> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt
>for D0 entry",
>> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
>> +		return -EBUSY;
>> +	}
>> +	bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld
>us",
>> +		    delta);
>> +	return 0;
>> +}
>> +
>> +const struct dev_pm_ops btintel_pcie_pm_ops = {
>> +	.suspend = btintel_pcie_suspend,
>> +	.resume = btintel_pcie_resume,
>> +	.freeze = btintel_pcie_freeze,
>> +	.thaw = btintel_pcie_resume,
>> +	.poweroff = btintel_pcie_hibernate,
>> +	.restore = btintel_pcie_resume,
>> +};
>> +#define BTINTELPCIE_PM_OPS	(&btintel_pcie_pm_ops)
>> +#else
>> +#define BTINTELPCIE_PM_OPS	NULL
>> +#endif
>>   static struct pci_driver btintel_pcie_driver = {
>>   	.name = KBUILD_MODNAME,
>>   	.id_table = btintel_pcie_table,
>>   	.probe = btintel_pcie_probe,
>>   	.remove = btintel_pcie_remove,
>> +	.driver.pm = BTINTELPCIE_PM_OPS,
>>   #ifdef CONFIG_DEV_COREDUMP
>>   	.driver.coredump = btintel_pcie_coredump
>>   #endif
>> diff --git a/drivers/bluetooth/btintel_pcie.h
>> b/drivers/bluetooth/btintel_pcie.h
>> index 0fa876c5b954..5bc69004b692 100644
>> --- a/drivers/bluetooth/btintel_pcie.h
>> +++ b/drivers/bluetooth/btintel_pcie.h
>> @@ -8,6 +8,7 @@
>>
>>   /* Control and Status Register(BTINTEL_PCIE_CSR) */
>>   #define BTINTEL_PCIE_CSR_BASE			(0x000)
>> +#define BTINTEL_PCIE_CSR_HW_BOOT_CONFIG
>	(BTINTEL_PCIE_CSR_BASE + 0x000)
>>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_REG
>	(BTINTEL_PCIE_CSR_BASE + 0x024)
>>   #define BTINTEL_PCIE_CSR_HW_REV_REG
>	(BTINTEL_PCIE_CSR_BASE + 0x028)
>>   #define BTINTEL_PCIE_CSR_RF_ID_REG
>	(BTINTEL_PCIE_CSR_BASE + 0x09C)
>> @@ -55,6 +56,9 @@
>>   #define BTINTEL_PCIE_CSR_BOOT_STAGE_ALIVE		(BIT(23))
>>   #define BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY	(BIT(24))
>>
>> +/* CSR HW BOOT CONFIG Register */
>> +#define BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON
>	(BIT(31))
>> +
>>   /* Registers for MSI-X */
>>   #define BTINTEL_PCIE_CSR_MSIX_BASE		(0x2000)
>>   #define BTINTEL_PCIE_CSR_MSIX_FH_INT_CAUSES
>	(BTINTEL_PCIE_CSR_MSIX_BASE + 0x0800)
>
>
>Kind regards,
>
>Paul

Thanks,
Kiran