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

[PATCH v6] 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:
[Fri Jul 25 11:05:37 2025] Bluetooth: hci0: device entered into d3 state from d0 in 80 us

On system resume:
[Fri Jul 25 11:06:36 2025] Bluetooth: hci0: device entered into d0 state from d3 in 7117 us

Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
changes in v6:
     - s/delta/delta_us/g
     - s/CONFIG_PM/CONFIG_PM_SLEEP/g
     - use pm_sleep_pr()/pm_str() to avoid #ifdefs
     - remove the code to set persistance mode as its not relevant to this patch

changes in v5:
     - refactor _suspend() / _resume() to set the D3HOT/D3COLD based on power
       event
     - remove SIMPLE_DEV_PM_OPS and define the required pm_ops callback
       functions

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 | 90 ++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 6e7bbbd35279..c419521493fe 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -2573,11 +2573,101 @@ static void btintel_pcie_coredump(struct device *dev)
 }
 #endif
 
+#ifdef CONFIG_PM_SLEEP
+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_us;
+	int err;
+
+	data = pci_get_drvdata(pdev);
+
+	dxstate = (mesg.event == PM_EVENT_SUSPEND ?
+		   BTINTEL_PCIE_STATE_D3_HOT : 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));
+	if (err == 0) {
+		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D3 entry",
+				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
+		return -EBUSY;
+	}
+
+	delta_us = ktime_to_ns(ktime_get() - start) / 1000;
+	bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld us",
+		    delta_us);
+	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_us;
+
+	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));
+	if (err == 0) {
+		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D0 entry",
+				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
+		return -EBUSY;
+	}
+
+	delta_us = ktime_to_ns(ktime_get() - start) / 1000;
+	bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld us",
+		    delta_us);
+	return 0;
+}
+
+const struct dev_pm_ops btintel_pcie_pm_ops = {
+	.suspend = pm_sleep_ptr(btintel_pcie_suspend),
+	.resume = pm_sleep_ptr(btintel_pcie_resume),
+	.freeze = pm_sleep_ptr(btintel_pcie_freeze),
+	.thaw = pm_sleep_ptr(btintel_pcie_resume),
+	.poweroff = pm_sleep_ptr(btintel_pcie_hibernate),
+	.restore = pm_sleep_ptr(btintel_pcie_resume),
+};
+#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 = pm_ptr(&btintel_pcie_pm_ops),
 #ifdef CONFIG_DEV_COREDUMP
 	.driver.coredump = btintel_pcie_coredump
 #endif
-- 
2.43.0

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

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch failed: drivers/bluetooth/btintel_pcie.c:2573
error: drivers/bluetooth/btintel_pcie.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth

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

[Paul Menzel]

Dear Kiran,


Thank you for sending the improved version 6.

Am 25.07.25 um 11:01 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:
> [Fri Jul 25 11:05:37 2025] Bluetooth: hci0: device entered into d3 state from d0 in 80 us
> 
> On system resume:
> [Fri Jul 25 11:06:36 2025] Bluetooth: hci0: device entered into d0 state from d3 in 7117 us
> 
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
> changes in v6:
>       - s/delta/delta_us/g
>       - s/CONFIG_PM/CONFIG_PM_SLEEP/g
>       - use pm_sleep_pr()/pm_str() to avoid #ifdefs
>       - remove the code to set persistance mode as its not relevant to this patch
> 
> changes in v5:
>       - refactor _suspend() / _resume() to set the D3HOT/D3COLD based on power
>         event
>       - remove SIMPLE_DEV_PM_OPS and define the required pm_ops callback
>         functions
> 
> 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 | 90 ++++++++++++++++++++++++++++++++
>   1 file changed, 90 insertions(+)
> 
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 6e7bbbd35279..c419521493fe 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -2573,11 +2573,101 @@ static void btintel_pcie_coredump(struct device *dev)
>   }
>   #endif
>   
> +#ifdef CONFIG_PM_SLEEP
> +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_us;
> +	int err;
> +
> +	data = pci_get_drvdata(pdev);
> +
> +	dxstate = (mesg.event == PM_EVENT_SUSPEND ?
> +		   BTINTEL_PCIE_STATE_D3_HOT : BTINTEL_PCIE_STATE_D3_COLD);

I believe the brackets are uncommon. You actually can leave it out, or, 
if brackets need to be used, only surround the condition.

> +
> +	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));
> +	if (err == 0) {
> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D3 entry",
> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> +		return -EBUSY;
> +	}
> +
> +	delta_us = ktime_to_ns(ktime_get() - start) / 1000;
> +	bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld us",
> +		    delta_us);
> +	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_us;
> +
> +	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));
> +	if (err == 0) {
> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D0 entry",
> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> +		return -EBUSY;
> +	}
> +
> +	delta_us = ktime_to_ns(ktime_get() - start) / 1000;
> +	bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld us",
> +		    delta_us);
> +	return 0;
> +}
> +
> +const struct dev_pm_ops btintel_pcie_pm_ops = {
> +	.suspend = pm_sleep_ptr(btintel_pcie_suspend),
> +	.resume = pm_sleep_ptr(btintel_pcie_resume),
> +	.freeze = pm_sleep_ptr(btintel_pcie_freeze),
> +	.thaw = pm_sleep_ptr(btintel_pcie_resume),
> +	.poweroff = pm_sleep_ptr(btintel_pcie_hibernate),
> +	.restore = pm_sleep_ptr(btintel_pcie_resume),
> +};
> +#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 = pm_ptr(&btintel_pcie_pm_ops),
>   #ifdef CONFIG_DEV_COREDUMP
>   	.driver.coredump = btintel_pcie_coredump
>   #endif

With the above comment fixed, please feel free to add:

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

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

[K, Kiran]

Hi Paul,

>-----Original Message-----
>From: Paul Menzel <pmenzel@molgen.mpg.de>
>Sent: Friday, July 25, 2025 2:35 PM
>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 v6] Bluetooth: btintel_pcie: Add support for _suspend() /
>_resume()
>
>Dear Kiran,
>
>
>Thank you for sending the improved version 6.
>
>Am 25.07.25 um 11:01 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:
>> [Fri Jul 25 11:05:37 2025] Bluetooth: hci0: device entered into d3
>> state from d0 in 80 us
>>
>> On system resume:
>> [Fri Jul 25 11:06:36 2025] Bluetooth: hci0: device entered into d0
>> state from d3 in 7117 us
>>
>> Signed-off-by: Chandrashekar Devegowda
>> <chandrashekar.devegowda@intel.com>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>> changes in v6:
>>       - s/delta/delta_us/g
>>       - s/CONFIG_PM/CONFIG_PM_SLEEP/g
>>       - use pm_sleep_pr()/pm_str() to avoid #ifdefs
>>       - remove the code to set persistance mode as its not relevant to
>> this patch
>>
>> changes in v5:
>>       - refactor _suspend() / _resume() to set the D3HOT/D3COLD based on
>power
>>         event
>>       - remove SIMPLE_DEV_PM_OPS and define the required pm_ops callback
>>         functions
>>
>> 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 | 90 ++++++++++++++++++++++++++++++++
>>   1 file changed, 90 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index 6e7bbbd35279..c419521493fe 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -2573,11 +2573,101 @@ static void btintel_pcie_coredump(struct device
>*dev)
>>   }
>>   #endif
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +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_us;
>> +	int err;
>> +
>> +	data = pci_get_drvdata(pdev);
>> +
>> +	dxstate = (mesg.event == PM_EVENT_SUSPEND ?
>> +		   BTINTEL_PCIE_STATE_D3_HOT :
>BTINTEL_PCIE_STATE_D3_COLD);
>
>I believe the brackets are uncommon. You actually can leave it out, or, if
>brackets need to be used, only surround the condition.
>
>> +
>> +	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));
>> +	if (err == 0) {
>> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt
>for D3 entry",
>> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
>> +		return -EBUSY;
>> +	}
>> +
>> +	delta_us = ktime_to_ns(ktime_get() - start) / 1000;
>> +	bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld
>us",
>> +		    delta_us);
>> +	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_us;
>> +
>> +	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));
>> +	if (err == 0) {
>> +		bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt
>for D0 entry",
>> +				BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
>> +		return -EBUSY;
>> +	}
>> +
>> +	delta_us = ktime_to_ns(ktime_get() - start) / 1000;
>> +	bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld
>us",
>> +		    delta_us);
>> +	return 0;
>> +}
>> +
>> +const struct dev_pm_ops btintel_pcie_pm_ops = {
>> +	.suspend = pm_sleep_ptr(btintel_pcie_suspend),
>> +	.resume = pm_sleep_ptr(btintel_pcie_resume),
>> +	.freeze = pm_sleep_ptr(btintel_pcie_freeze),
>> +	.thaw = pm_sleep_ptr(btintel_pcie_resume),
>> +	.poweroff = pm_sleep_ptr(btintel_pcie_hibernate),
>> +	.restore = pm_sleep_ptr(btintel_pcie_resume),
>> +};
>> +#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 = pm_ptr(&btintel_pcie_pm_ops),
>>   #ifdef CONFIG_DEV_COREDUMP
>>   	.driver.coredump = btintel_pcie_coredump
>>   #endif
>
>With the above comment fixed, please feel free to add:
>
>Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Ack.

>
>
>Kind regards,
>
>Paul

Thanks,
Kiran

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

[Luiz Augusto von Dentz]

Hi Kiran,

On Fri, Jul 25, 2025 at 4:45 AM Kiran K <kiran.k@intel.com> 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:
> [Fri Jul 25 11:05:37 2025] Bluetooth: hci0: device entered into d3 state from d0 in 80 us
>
> On system resume:
> [Fri Jul 25 11:06:36 2025] Bluetooth: hci0: device entered into d0 state from d3 in 7117 us
>
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
> changes in v6:
>      - s/delta/delta_us/g
>      - s/CONFIG_PM/CONFIG_PM_SLEEP/g
>      - use pm_sleep_pr()/pm_str() to avoid #ifdefs
>      - remove the code to set persistance mode as its not relevant to this patch
>
> changes in v5:
>      - refactor _suspend() / _resume() to set the D3HOT/D3COLD based on power
>        event
>      - remove SIMPLE_DEV_PM_OPS and define the required pm_ops callback
>        functions
>
> 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 | 90 ++++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 6e7bbbd35279..c419521493fe 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -2573,11 +2573,101 @@ static void btintel_pcie_coredump(struct device *dev)
>  }
>  #endif
>
> +#ifdef CONFIG_PM_SLEEP
> +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_us;
> +       int err;
> +
> +       data = pci_get_drvdata(pdev);
> +
> +       dxstate = (mesg.event == PM_EVENT_SUSPEND ?
> +                  BTINTEL_PCIE_STATE_D3_HOT : 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));
> +       if (err == 0) {
> +               bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D3 entry",
> +                               BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> +               return -EBUSY;
> +       }
> +
> +       delta_us = ktime_to_ns(ktime_get() - start) / 1000;
> +       bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld us",
> +                   delta_us);
> +       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_us;
> +
> +       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));
> +       if (err == 0) {
> +               bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D0 entry",
> +                               BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> +               return -EBUSY;
> +       }
> +
> +       delta_us = ktime_to_ns(ktime_get() - start) / 1000;
> +       bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld us",
> +                   delta_us);
> +       return 0;
> +}
> +
> +const struct dev_pm_ops btintel_pcie_pm_ops = {
> +       .suspend = pm_sleep_ptr(btintel_pcie_suspend),
> +       .resume = pm_sleep_ptr(btintel_pcie_resume),
> +       .freeze = pm_sleep_ptr(btintel_pcie_freeze),
> +       .thaw = pm_sleep_ptr(btintel_pcie_resume),
> +       .poweroff = pm_sleep_ptr(btintel_pcie_hibernate),
> +       .restore = pm_sleep_ptr(btintel_pcie_resume),
> +};
> +#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 = pm_ptr(&btintel_pcie_pm_ops),

This doesn't seem quite right, btintel_pcie_pm_ops is behind
CONFIG_PM_SLEEP not just CONFIG_PM, so it would be undefined if just
CONFIG_PM is set, so we might as well do:

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 5b32f5a6b0b0..2f1b1be94080 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -2654,12 +2654,12 @@ static int btintel_pcie_resume(struct device *dev)
 }

 const struct dev_pm_ops btintel_pcie_pm_ops = {
-       .suspend = pm_sleep_ptr(btintel_pcie_suspend),
-       .resume = pm_sleep_ptr(btintel_pcie_resume),
-       .freeze = pm_sleep_ptr(btintel_pcie_freeze),
-       .thaw = pm_sleep_ptr(btintel_pcie_resume),
-       .poweroff = pm_sleep_ptr(btintel_pcie_hibernate),
-       .restore = pm_sleep_ptr(btintel_pcie_resume),
+       .suspend = btintel_pcie_suspend,
+       .resume = btintel_pcie_resume,
+       .freeze = btintel_pcie_freeze,
+       .thaw = btintel_pcie_resume,
+       .poweroff = btintel_pcie_hibernate,
+       .restore = btintel_pcie_resume,
 };
 #endif

@@ -2668,7 +2668,7 @@ static struct pci_driver btintel_pcie_driver = {
        .id_table = btintel_pcie_table,
        .probe = btintel_pcie_probe,
        .remove = btintel_pcie_remove,
-       .driver.pm = pm_ptr(&btintel_pcie_pm_ops),
+       .driver.pm = pm_sleep_ptr(&btintel_pcie_pm_ops),
 #ifdef CONFIG_DEV_COREDUMP
        .driver.coredump = btintel_pcie_coredump
 #endif


>  #ifdef CONFIG_DEV_COREDUMP
>         .driver.coredump = btintel_pcie_coredump
>  #endif
> --
> 2.43.0
>
>


-- 
Luiz Augusto von Dentz

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

[K, Kiran]

Hi Luiz,

>-----Original Message-----
>From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
>Sent: Friday, July 25, 2025 7:24 PM
>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 v6] Bluetooth: btintel_pcie: Add support for _suspend() /
>_resume()
>
>Hi Kiran,
>
>On Fri, Jul 25, 2025 at 4:45 AM Kiran K <kiran.k@intel.com> 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:
>> [Fri Jul 25 11:05:37 2025] Bluetooth: hci0: device entered into d3
>> state from d0 in 80 us
>>
>> On system resume:
>> [Fri Jul 25 11:06:36 2025] Bluetooth: hci0: device entered into d0
>> state from d3 in 7117 us
>>
>> Signed-off-by: Chandrashekar Devegowda
>> <chandrashekar.devegowda@intel.com>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>> changes in v6:
>>      - s/delta/delta_us/g
>>      - s/CONFIG_PM/CONFIG_PM_SLEEP/g
>>      - use pm_sleep_pr()/pm_str() to avoid #ifdefs
>>      - remove the code to set persistance mode as its not relevant to
>> this patch
>>
>> changes in v5:
>>      - refactor _suspend() / _resume() to set the D3HOT/D3COLD based on
>power
>>        event
>>      - remove SIMPLE_DEV_PM_OPS and define the required pm_ops callback
>>        functions
>>
>> 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 | 90
>> ++++++++++++++++++++++++++++++++
>>  1 file changed, 90 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index 6e7bbbd35279..c419521493fe 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -2573,11 +2573,101 @@ static void btintel_pcie_coredump(struct
>> device *dev)  }  #endif
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +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_us;
>> +       int err;
>> +
>> +       data = pci_get_drvdata(pdev);
>> +
>> +       dxstate = (mesg.event == PM_EVENT_SUSPEND ?
>> +                  BTINTEL_PCIE_STATE_D3_HOT :
>> + 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));
>> +       if (err == 0) {
>> +               bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D3
>entry",
>> +                               BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
>> +               return -EBUSY;
>> +       }
>> +
>> +       delta_us = ktime_to_ns(ktime_get() - start) / 1000;
>> +       bt_dev_info(data->hdev, "device entered into d3 state from d0 in %lld
>us",
>> +                   delta_us);
>> +       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_us;
>> +
>> +       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));
>> +       if (err == 0) {
>> +               bt_dev_err(data->hdev, "Timeout (%u ms) on alive interrupt for D0
>entry",
>> +                               BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
>> +               return -EBUSY;
>> +       }
>> +
>> +       delta_us = ktime_to_ns(ktime_get() - start) / 1000;
>> +       bt_dev_info(data->hdev, "device entered into d0 state from d3 in %lld
>us",
>> +                   delta_us);
>> +       return 0;
>> +}
>> +
>> +const struct dev_pm_ops btintel_pcie_pm_ops = {
>> +       .suspend = pm_sleep_ptr(btintel_pcie_suspend),
>> +       .resume = pm_sleep_ptr(btintel_pcie_resume),
>> +       .freeze = pm_sleep_ptr(btintel_pcie_freeze),
>> +       .thaw = pm_sleep_ptr(btintel_pcie_resume),
>> +       .poweroff = pm_sleep_ptr(btintel_pcie_hibernate),
>> +       .restore = pm_sleep_ptr(btintel_pcie_resume),
>> +};
>> +#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 = pm_ptr(&btintel_pcie_pm_ops),
>
>This doesn't seem quite right, btintel_pcie_pm_ops is behind
>CONFIG_PM_SLEEP not just CONFIG_PM, so it would be undefined if just
>CONFIG_PM is set, so we might as well do:
>
Agree.  I will remove the usage of CONFIG_PM_SLEEP and instead rely on pm_sleep_ptr() and pm_ptr().

>diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
>index 5b32f5a6b0b0..2f1b1be94080 100644
>--- a/drivers/bluetooth/btintel_pcie.c
>+++ b/drivers/bluetooth/btintel_pcie.c
>@@ -2654,12 +2654,12 @@ static int btintel_pcie_resume(struct device *dev)
>}
>
> const struct dev_pm_ops btintel_pcie_pm_ops = {
>-       .suspend = pm_sleep_ptr(btintel_pcie_suspend),
>-       .resume = pm_sleep_ptr(btintel_pcie_resume),
>-       .freeze = pm_sleep_ptr(btintel_pcie_freeze),
>-       .thaw = pm_sleep_ptr(btintel_pcie_resume),
>-       .poweroff = pm_sleep_ptr(btintel_pcie_hibernate),
>-       .restore = pm_sleep_ptr(btintel_pcie_resume),
>+       .suspend = btintel_pcie_suspend,
>+       .resume = btintel_pcie_resume,
>+       .freeze = btintel_pcie_freeze,
>+       .thaw = btintel_pcie_resume,
>+       .poweroff = btintel_pcie_hibernate,
>+       .restore = btintel_pcie_resume,
> };
> #endif
>
>@@ -2668,7 +2668,7 @@ static struct pci_driver btintel_pcie_driver = {
>        .id_table = btintel_pcie_table,
>        .probe = btintel_pcie_probe,
>        .remove = btintel_pcie_remove,
>-       .driver.pm = pm_ptr(&btintel_pcie_pm_ops),
>+       .driver.pm = pm_sleep_ptr(&btintel_pcie_pm_ops),
> #ifdef CONFIG_DEV_COREDUMP
>        .driver.coredump = btintel_pcie_coredump  #endif
>
>
>>  #ifdef CONFIG_DEV_COREDUMP
>>         .driver.coredump = btintel_pcie_coredump  #endif
>> --
>> 2.43.0
>>
>>
>
>
>--
>Luiz Augusto von Dentz

Thanks,
Kiran