diff mbox series

[v2] Bluetooth: btintel_pcie: Support suspend-resume

Message ID 20241108143931.2422947-1-chandrashekar.devegowda@intel.com
State New
Headers show
Series [v2] Bluetooth: btintel_pcie: Support suspend-resume | expand

Commit Message

Devegowda, Chandrashekar Nov. 8, 2024, 2:39 p.m. UTC 
This patch contains the changes in driver for vendor specific handshake
during enter of D3 and D0 exit.
Command to test host initiated wake up after 60seconds
    sudo rtcwake -m mem-s 60
logs from testing:
    Bluetooth: hci0: BT device resumed to D0 in 1016 usecs

Signed-off-by: Kiran K <kiran.k@intel.com>
Signed-off-by: ChandraShekar Devegowda <chandrashekar.devegowda@intel.com>
---
 drivers/bluetooth/btintel_pcie.c | 58 ++++++++++++++++++++++++++++++++
 drivers/bluetooth/btintel_pcie.h |  4 +++
 2 files changed, 62 insertions(+)

Comments

Devegowda, Chandrashekar Nov. 11, 2024, 6:33 a.m. UTC | #1 
Hi Paul,
     Thank you for your comments,

> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Friday, November 08, 2024 2:49 PM
> To: 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>; K, Kiran <kiran.k@intel.com>
> Subject: Re: [PATCH v2] Bluetooth: btintel_pcie: Support suspend-resume
> 
> Dear Chandra,
> 
> 
> Thank you for sending the second iteration. Please also include the previous
> reviewers in the Cc: list.
> 

Ack have added them back in the CC list and also will add them for the next version of patches.

> 
> Am 08.11.24 um 15:39 schrieb ChandraShekar Devegowda:
> 
> The space in your name is still missing.
> 

I have added my second name , my first name doesn’t have a space so please consider ChandraShekar as a single name.

> > This patch contains the changes in driver for vendor specific handshake
> > during enter of D3 and D0 exit.
> 
> Please document the datasheet name and revision.
> 

Datasheet is internal to Intel, sorry wouldn't  be able to share at the moment.

> > Command to test host initiated wake up after 60seconds
> 
> Please remove the space in wakeup, and add a space in 60 seconds.
> 

Ack will change in the next version of the patch. 

> >      sudo rtcwake -m mem-s 60
> 
> Please add space before the switch -s.
> 

Ack will change in the next version of the patch.

> > logs from testing:
> >      Bluetooth: hci0: BT device resumed to D0 in 1016 usecs
> 
> Thank you for providing the logs.
> 
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Signed-off-by: ChandraShekar Devegowda
> <chandrashekar.devegowda@intel.com>
> > ---
> 
> It’s common to add a change-log between the different versions below the
> `---` line.
> 

Ack will add the change-log in the next patch. 

> >   drivers/bluetooth/btintel_pcie.c | 58
> ++++++++++++++++++++++++++++++++
> >   drivers/bluetooth/btintel_pcie.h |  4 +++
> >   2 files changed, 62 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel_pcie.c
> b/drivers/bluetooth/btintel_pcie.c
> > index 2b79952f3628..49b78d3ecdf9 100644
> > --- a/drivers/bluetooth/btintel_pcie.c
> > +++ b/drivers/bluetooth/btintel_pcie.c
> > @@ -273,6 +273,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);
> > +}
> > +
> >   /* This function enables BT function by setting
> BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_INIT bit in
> >    * BTINTEL_PCIE_CSR_FUNC_CTRL_REG register and wait for MSI-X with
> >    * BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0.
> > @@ -297,6 +303,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 |
> > @@ -1647,11 +1655,61 @@ static void btintel_pcie_remove(struct pci_dev
> *pdev)
> >   	pci_set_drvdata(pdev, NULL);
> >   }
> >
> > +static int btintel_pcie_suspend(struct device *dev)
> > +{
> > +	struct btintel_pcie_data *data;
> > +	int err;
> > +	struct  pci_dev *pdev = to_pci_dev(dev);
> > +
> > +	data = pci_get_drvdata(pdev);
> > +	data->gp0_received = false;
> > +	btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D3_HOT);
> > +	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
> > +
> msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
> > +	if (!err) {
> > +		bt_dev_err(data->hdev, "failed to receive gp0 interrupt for
> suspend in %lu msecs",
> > +			   BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> > +		return -EBUSY;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int btintel_pcie_resume(struct device *dev)
> > +{
> > +	struct btintel_pcie_data *data;
> > +	struct  pci_dev *pdev = to_pci_dev(dev);
> > +	ktime_t calltime, delta, rettime;
> > +	unsigned long long duration;
> > +	int err;
> > +
> > +	data = pci_get_drvdata(pdev);
> > +	data->gp0_received = false;
> > +	calltime = ktime_get();
> > +	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) {
> > +		bt_dev_err(data->hdev, "failed to receive gp0 interrupt for
> resume in %lu msecs",
> > +			   BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
> > +		return -EBUSY;
> > +	}
> > +	rettime = ktime_get();
> > +	delta = ktime_sub(rettime, calltime);
> > +	duration = (unsigned long long)ktime_to_ns(delta) >> 10;
> > +	bt_dev_info(data->hdev, "BT device resumed to D0 in %llu usecs",
> duration);
> > +
> > +	return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(btintel_pcie_pm_ops, btintel_pcie_suspend,
> > +		btintel_pcie_resume);
> > +
> >   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 = &btintel_pcie_pm_ops,
> >   };
> >   module_pci_driver(btintel_pcie_driver);
> >
> > diff --git a/drivers/bluetooth/btintel_pcie.h
> b/drivers/bluetooth/btintel_pcie.h
> > index f9aada0543c4..38d0c8ea2b6f 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)
> > @@ -48,6 +49,9 @@
> >   #define BTINTEL_PCIE_CSR_MSIX_IVAR_BASE
> 	(BTINTEL_PCIE_CSR_MSIX_BASE + 0x0880)
> >   #define BTINTEL_PCIE_CSR_MSIX_IVAR(cause)
> 	(BTINTEL_PCIE_CSR_MSIX_IVAR_BASE + (cause))
> >
> > +/* CSR HW BOOT CONFIG Register */
> > +#define BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON
> 	(BIT(31))
> > +
> >   /* Causes for the FH register interrupts */
> >   enum msix_fh_int_causes {
> >   	BTINTEL_PCIE_MSIX_FH_INT_CAUSES_0	= BIT(0),	/*
> cause 0 */
> 
> 
> Kind regards,
> 
> Paul
Paul Menzel Nov. 11, 2024, 7:21 a.m. UTC | #2 
Dear Chandrashekar,


Thank you for your space.


Am 11.11.24 um 07:33 schrieb Devegowda, Chandrashekar:

>> -----Original Message-----

>> Sent: Friday, November 08, 2024 2:49 PM
>> To: Devegowda, Chandrashekar <chandrashekar.devegowda@intel.com>

[…]

>> Am 08.11.24 um 15:39 schrieb ChandraShekar Devegowda:
>>
>> The space in your name is still missing.
> 
> I have added my second name, my first name doesn’t have a space so
> please consider ChandraShekar as a single name.
Thank you. In your email you now do *not* use CamelCase, which is more 
common in Western culture. (Of course you can write your name as you 
want, and I just pointed it out.)

>>> This patch contains the changes in driver for vendor specific handshake
>>> during enter of D3 and D0 exit.
>>
>> Please document the datasheet name and revision.
> 
> Datasheet is internal to Intel, sorry wouldn't  be able to share at
> the moment.

Although it is not public, the name would still be good to have. Intel 
employees can probably get access more easily, and non-Intel folks could 
directly approach with the name, and in the future it might be even made 
public.

[…]

Thank you for acknowledging the other comments.


Kind regards

Paul
diff mbox series

Patch

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 2b79952f3628..49b78d3ecdf9 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -273,6 +273,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);
+}
+
 /* This function enables BT function by setting BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_INIT bit in
  * BTINTEL_PCIE_CSR_FUNC_CTRL_REG register and wait for MSI-X with
  * BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0.
@@ -297,6 +303,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 |
@@ -1647,11 +1655,61 @@  static void btintel_pcie_remove(struct pci_dev *pdev)
 	pci_set_drvdata(pdev, NULL);
 }
 
+static int btintel_pcie_suspend(struct device *dev)
+{
+	struct btintel_pcie_data *data;
+	int err;
+	struct  pci_dev *pdev = to_pci_dev(dev);
+
+	data = pci_get_drvdata(pdev);
+	data->gp0_received = false;
+	btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D3_HOT);
+	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
+				 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
+	if (!err) {
+		bt_dev_err(data->hdev, "failed to receive gp0 interrupt for suspend in %lu msecs",
+			   BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
+		return -EBUSY;
+	}
+	return 0;
+}
+
+static int btintel_pcie_resume(struct device *dev)
+{
+	struct btintel_pcie_data *data;
+	struct  pci_dev *pdev = to_pci_dev(dev);
+	ktime_t calltime, delta, rettime;
+	unsigned long long duration;
+	int err;
+
+	data = pci_get_drvdata(pdev);
+	data->gp0_received = false;
+	calltime = ktime_get();
+	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) {
+		bt_dev_err(data->hdev, "failed to receive gp0 interrupt for resume in %lu msecs",
+			   BTINTEL_DEFAULT_INTR_TIMEOUT_MS);
+		return -EBUSY;
+	}
+	rettime = ktime_get();
+	delta = ktime_sub(rettime, calltime);
+	duration = (unsigned long long)ktime_to_ns(delta) >> 10;
+	bt_dev_info(data->hdev, "BT device resumed to D0 in %llu usecs", duration);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(btintel_pcie_pm_ops, btintel_pcie_suspend,
+		btintel_pcie_resume);
+
 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 = &btintel_pcie_pm_ops,
 };
 module_pci_driver(btintel_pcie_driver);
 
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index f9aada0543c4..38d0c8ea2b6f 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)
@@ -48,6 +49,9 @@ 
 #define BTINTEL_PCIE_CSR_MSIX_IVAR_BASE		(BTINTEL_PCIE_CSR_MSIX_BASE + 0x0880)
 #define BTINTEL_PCIE_CSR_MSIX_IVAR(cause)	(BTINTEL_PCIE_CSR_MSIX_IVAR_BASE + (cause))
 
+/* CSR HW BOOT CONFIG Register */
+#define BTINTEL_PCIE_CSR_HW_BOOT_CONFIG_KEEP_ON		(BIT(31))
+
 /* Causes for the FH register interrupts */
 enum msix_fh_int_causes {
 	BTINTEL_PCIE_MSIX_FH_INT_CAUSES_0	= BIT(0),	/* cause 0 */