* Re: [PATCH v1] Bluetooth: btintel_pcie: Device suspend-resume support added
2024-10-23 7:18 ` [PATCH v1] Bluetooth: btintel_pcie: Device suspend-resume support added Paul Menzel
@ 2024-10-23 19:19 ` Luiz Augusto von Dentz
2024-10-24 9:06 ` Ilpo Järvinen
2024-11-04 9:18 ` Devegowda, Chandrashekar
2 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-23 19:19 UTC (permalink / raw)
To: Paul Menzel
Cc: ChandraShekar, Kiran K, linux-bluetooth, ravishankar.srivatsa,
chethan.tumkur.narayan, Bjorn Helgaas, linux-pci
Hi,
On Wed, Oct 23, 2024 at 3:19 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> [Cc: +Bjorn, +linux-pci]
>
> Dear Chandra,
>
>
> Thank you for the patch.
>
> First something minor: Should there be a space in your name?
>
> ChandraShekar → Chandra Shekar
>
> `git config --global user.name "…"` can configure this for your git setup.
>
> Also for the summary/title, it’d be great if you used a statement by
> using a verb (in imperative mood):
>
> Add device suspend-resume support
>
> or shorter
>
> Support suspend-resume
>
> Am 23.10.24 um 13:46 schrieb ChandraShekar:
> > This patch contains the changes in driver to support the suspend and
> > resume i.e move the controller to D3 state when the platform is entering
> > into suspend and move the controller to D0 on resume.
>
> It’d be great if you elaborated. Please start by the history, since when
> Intel Bluetooth PCIe have been there, and why until now this support was
> missing.
>
> Then please describe, what is needed, and what documentation you used to
> implement the support.
>
> Also, please document, how you tested this, including the log messages,
> and also the time it takes to resume.
>
> Is it also possible to use Bluetooth as a wakeup source from suspend?
This is not a system-suspend though or are you asking if the device
can wakeup itself?
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Signed-off-by: ChandraShekar <chandrashekar.devegowda@intel.com>
> > ---
> > drivers/bluetooth/btintel_pcie.c | 52 ++++++++++++++++++++++++++++++++
> > drivers/bluetooth/btintel_pcie.h | 4 +++
> > 2 files changed, 56 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> > index fd4a8bd056fa..f2c44b9d7328 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 |
> > @@ -1653,11 +1661,55 @@ 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);
> > + btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D3_HOT);
> > + data->gp0_received = false;
> > + 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");
>
> Please include the timeout in the message.
>
> > + goto fail;
> > + }
> > + return 0;
> > +fail:
> > + return -EBUSY;
> > +}
> > +
> > +static int btintel_pcie_resume(struct device *dev)
> > +{
> > + struct btintel_pcie_data *data;
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + int err;
> > +
> > + data = pci_get_drvdata(pdev);
> > + btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
> > + data->gp0_received = false;
> > + 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");
>
> Ditto.
>
> > + goto fail;
> > + }
> > + return 0;
> > +fail:
> > + return -EBUSY;
> > +}
> > +
> > +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
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] Bluetooth: btintel_pcie: Device suspend-resume support added
2024-10-23 7:18 ` [PATCH v1] Bluetooth: btintel_pcie: Device suspend-resume support added Paul Menzel
2024-10-23 19:19 ` Luiz Augusto von Dentz
@ 2024-10-24 9:06 ` Ilpo Järvinen
2024-11-04 9:18 ` Devegowda, Chandrashekar
2 siblings, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2024-10-24 9:06 UTC (permalink / raw)
To: Paul Menzel, ChandraShekar
Cc: Kiran K, linux-bluetooth, ravishankar.srivatsa,
chethan.tumkur.narayan, Bjorn Helgaas, linux-pci
[-- Attachment #1: Type: text/plain, Size: 6107 bytes --]
On Wed, 23 Oct 2024, Paul Menzel wrote:
> [Cc: +Bjorn, +linux-pci]
>
> Dear Chandra,
>
>
> Thank you for the patch.
>
> First something minor: Should there be a space in your name?
>
> ChandraShekar → Chandra Shekar
>
> `git config --global user.name "…"` can configure this for your git setup.
>
> Also for the summary/title, it’d be great if you used a statement by using a
> verb (in imperative mood):
>
> Add device suspend-resume support
>
> or shorter
>
> Support suspend-resume
>
> Am 23.10.24 um 13:46 schrieb ChandraShekar:
> > This patch contains the changes in driver to support the suspend and
> > resume i.e move the controller to D3 state when the platform is entering
> > into suspend and move the controller to D0 on resume.
>
> It’d be great if you elaborated. Please start by the history, since when Intel
> Bluetooth PCIe have been there, and why until now this support was missing.
>
> Then please describe, what is needed, and what documentation you used to
> implement the support.
>
> Also, please document, how you tested this, including the log messages, and
> also the time it takes to resume.
>
> Is it also possible to use Bluetooth as a wakeup source from suspend?
>
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Signed-off-by: ChandraShekar <chandrashekar.devegowda@intel.com>
> > ---
> > drivers/bluetooth/btintel_pcie.c | 52 ++++++++++++++++++++++++++++++++
> > drivers/bluetooth/btintel_pcie.h | 4 +++
> > 2 files changed, 56 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel_pcie.c
> > b/drivers/bluetooth/btintel_pcie.c
> > index fd4a8bd056fa..f2c44b9d7328 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 |
> > @@ -1653,11 +1661,55 @@ 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);
Extra space.
> > +
> > + data = pci_get_drvdata(pdev);
> > + btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D3_HOT);
> > + data->gp0_received = false;
> > + 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");
>
> Please include the timeout in the message.
>
> > + goto fail;
> > + }
> > + return 0;
> > +fail:
> > + return -EBUSY;
> > +}
> > +
> > +static int btintel_pcie_resume(struct device *dev)
> > +{
> > + struct btintel_pcie_data *data;
> > + struct pci_dev *pdev = to_pci_dev(dev);
Ditto.
> > + int err;
Why's the order inconsistent compared with suspend func local vars?
> > +
> > + data = pci_get_drvdata(pdev);
> > + btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
> > + data->gp0_received = false;
> > + 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");
>
> Ditto.
>
> > + goto fail;
> > + }
> > + return 0;
> > +fail:
> > + return -EBUSY;
Quite much common code here compared with the suspend side. Perhaps a
helper function would be useful?
> > +}
> > +
> > +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))
Extra parenthesis.
--
i.
> > +
> > /* Causes for the FH register interrupts */
> > enum msix_fh_int_causes {
> > BTINTEL_PCIE_MSIX_FH_INT_CAUSES_0 = BIT(0), /* cause 0 */
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH v1] Bluetooth: btintel_pcie: Device suspend-resume support added
2024-10-23 7:18 ` [PATCH v1] Bluetooth: btintel_pcie: Device suspend-resume support added Paul Menzel
2024-10-23 19:19 ` Luiz Augusto von Dentz
2024-10-24 9:06 ` Ilpo Järvinen
@ 2024-11-04 9:18 ` Devegowda, Chandrashekar
2024-11-04 9:44 ` Paul Menzel
2 siblings, 1 reply; 5+ messages in thread
From: Devegowda, Chandrashekar @ 2024-11-04 9:18 UTC (permalink / raw)
To: Paul Menzel, K, Kiran
Cc: linux-bluetooth@vger.kernel.org, Srivatsa, Ravishankar,
Tumkur Narayan, Chethan, Bjorn Helgaas, linux-pci@vger.kernel.org
Hi Paul,
Thank you for the comments
> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Wednesday, October 23, 2024 12:49 PM
> To: Devegowda, Chandrashekar <chandrashekar.devegowda@intel.com>; 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>; Bjorn Helgaas
> <bhelgaas@google.com>; linux-pci@vger.kernel.org
> Subject: Re: [PATCH v1] Bluetooth: btintel_pcie: Device suspend-resume
> support added
>
> [Cc: +Bjorn, +linux-pci]
>
> Dear Chandra,
>
>
> Thank you for the patch.
>
> First something minor: Should there be a space in your name?
>
> ChandraShekar → Chandra Shekar
Ack will add the additional space in the v2 version of the patch
>
> `git config --global user.name "…"` can configure this for your git setup.
>
> Also for the summary/title, it’d be great if you used a statement by using a
> verb (in imperative mood):
>
> Add device suspend-resume support
>
> or shorter
>
> Support suspend-resume
>
Ack will update the summary in the v2 version of the patch
> Am 23.10.24 um 13:46 schrieb ChandraShekar:
> > This patch contains the changes in driver to support the suspend and
> > resume i.e move the controller to D3 state when the platform is
> > entering into suspend and move the controller to D0 on resume.
>
> It’d be great if you elaborated. Please start by the history, since when Intel
> Bluetooth PCIe have been there, and why until now this support was missing.
>
The initial Intel bluetooth firmware supported only the FW download and Bluetooth Tx and Rx data flows ,further now the firmware has added support for vendor specific handshake during enter of D3 and D0 exit. So corresponding to FW changes this is the incremental changes from driver.
> Then please describe, what is needed, and what documentation you used to
> implement the support.
>
> Also, please document, how you tested this, including the log messages, and
> also the time it takes to resume.
>
Ack will update the teststeps in the commit message of the v2 version of the patch ,The resume duration is around ~1ms.
> Is it also possible to use Bluetooth as a wakeup source from suspend?
>
Yes BT device initiated wake up of the platform from suspend is possible, which will be enabled in the next patch
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Signed-off-by: ChandraShekar <chandrashekar.devegowda@intel.com>
> > ---
> > drivers/bluetooth/btintel_pcie.c | 52
> ++++++++++++++++++++++++++++++++
> > drivers/bluetooth/btintel_pcie.h | 4 +++
> > 2 files changed, 56 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel_pcie.c
> > b/drivers/bluetooth/btintel_pcie.c
> > index fd4a8bd056fa..f2c44b9d7328 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 | @@ -1653,11
> > +1661,55 @@ 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);
> > + btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D3_HOT);
> > + data->gp0_received = false;
> > + 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");
>
> Please include the timeout in the message.
>
Ack will change in the next v2 version of the patch
> > + goto fail;
> > + }
> > + return 0;
> > +fail:
> > + return -EBUSY;
> > +}
> > +
> > +static int btintel_pcie_resume(struct device *dev) {
> > + struct btintel_pcie_data *data;
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + int err;
> > +
> > + data = pci_get_drvdata(pdev);
> > + btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
> > + data->gp0_received = false;
> > + 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");
>
> Ditto.
>
Ack will change in the next v2 version of the patch
> > + goto fail;
> > + }
> > + return 0;
> > +fail:
> > + return -EBUSY;
> > +}
> > +
> > +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
Thanks and Regards,
Chandru
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] Bluetooth: btintel_pcie: Device suspend-resume support added
2024-11-04 9:18 ` Devegowda, Chandrashekar
@ 2024-11-04 9:44 ` Paul Menzel
0 siblings, 0 replies; 5+ messages in thread
From: Paul Menzel @ 2024-11-04 9:44 UTC (permalink / raw)
To: Chandrashekar Devegowda, Kiran K
Cc: linux-bluetooth, Ravishankar Srivatsa, Tumkur Narayan, Chethan,
Bjorn Helgaas, linux-pci
Dear Chandrashekar,
Am 04.11.24 um 10:18 schrieb Devegowda, Chandrashekar:
> Thank you for the comments
Thank you for taking the time to address them.
>> -----Original Message-----
>> From: Paul Menzel <pmenzel@molgen.mpg.de>
>> Sent: Wednesday, October 23, 2024 12:49 PM
>> [Cc: +Bjorn, +linux-pci]
[…]
>> Am 23.10.24 um 13:46 schrieb ChandraShekar:
>>> This patch contains the changes in driver to support the suspend and
>>> resume i.e move the controller to D3 state when the platform is
>>> entering into suspend and move the controller to D0 on resume.
>>
>> It’d be great if you elaborated. Please start by the history, since when Intel
>> Bluetooth PCIe have been there, and why until now this support was missing.
>
> The initial Intel bluetooth firmware supported only the FW download
> and Bluetooth Tx and Rx data flows ,further now the firmware has
> added support for vendor specific handshake during enter of D3 and
> D0 exit. So corresponding to FW changes this is the incremental
> changes from driver.
Interesting. Please also add the firmware versions to the commit message.
[…]
>> Is it also possible to use Bluetooth as a wakeup source from
>> suspend?
>
> Yes BT device initiated wake up of the platform from suspend is
> possible, which will be enabled in the next patch
Great.
[…]
I am looking forward to the next iteration.
Kind regards,
Paul
^ permalink raw reply [flat|nested] 5+ messages in thread