diff mbox series

[v1,1/2] Bluetooth: btintel_pcie: Add handshake between driver and firmware

Message ID 20241001104451.626964-1-kiran.k@intel.com (mailing list archive)
State Accepted
Commit 5ea625845b0f6fdda3318842c0d24c440fdf8b8c
Headers show
Series [v1,1/2] Bluetooth: btintel_pcie: Add handshake between driver and firmware | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/SubjectPrefix success Gitlint PASS
tedd_an/BuildKernel success BuildKernel PASS
tedd_an/CheckAllWarning success CheckAllWarning PASS
tedd_an/CheckSparse success CheckSparse PASS

Commit Message

K, Kiran Oct. 1, 2024, 10:44 a.m. UTC
The following handshake mechanism needs be followed after firmware
download is completed to bring the firmware to running state.

After firmware fragments of Operational image are downloaded and
secure sends result of the image succeeds,

1. Driver sends HCI Intel reset with boot option #1 to switch FW image.
2. FW sends Alive GP[0] MSIx
3. Driver enables data path (doorbell 0x460 for RBDs, etc...)
4. Driver gets Bootup event from firmware
5. Driver performs D0 entry to device (WRITE to IPC_Sleep_Control =0x0)
6. FW sends Alive GP[0] MSIx
7. Device host interface is fully set for BT protocol stack operation.
8. Driver may optionally get debug event with ID 0x97 which can be dropped

For Intermediate loadger image, all the above steps are applicable
expcept #5 and #6.

On HCI_OP_RESET, firmware raises alive interrupt. Driver needs to wait
for it before passing control over to bluetooth stack.

Co-developed-by: Devegowda Chandrashekar <chandrashekar.devegowda@intel.com>
Signed-off-by: Devegowda Chandrashekar <chandrashekar.devegowda@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel.c      |  56 ++++++-
 drivers/bluetooth/btintel.h      |   7 +
 drivers/bluetooth/btintel_pcie.c | 262 ++++++++++++++++++++++++++++++-
 drivers/bluetooth/btintel_pcie.h |  16 +-
 4 files changed, 329 insertions(+), 12 deletions(-)

Comments

Luiz Augusto von Dentz Oct. 1, 2024, 2:32 p.m. UTC | #1
Hi Kiran,

On Tue, Oct 1, 2024 at 6:29 AM Kiran K <kiran.k@intel.com> wrote:
>
> The following handshake mechanism needs be followed after firmware
> download is completed to bring the firmware to running state.
>
> After firmware fragments of Operational image are downloaded and
> secure sends result of the image succeeds,
>
> 1. Driver sends HCI Intel reset with boot option #1 to switch FW image.
> 2. FW sends Alive GP[0] MSIx
> 3. Driver enables data path (doorbell 0x460 for RBDs, etc...)
> 4. Driver gets Bootup event from firmware
> 5. Driver performs D0 entry to device (WRITE to IPC_Sleep_Control =0x0)
> 6. FW sends Alive GP[0] MSIx
> 7. Device host interface is fully set for BT protocol stack operation.
> 8. Driver may optionally get debug event with ID 0x97 which can be dropped

We should probably start versioning the boot sequence from the very
beginning, I also wonder if we should promote the use usage of
something like the hci_init_stage_sync for such use cases so one can
clearly define each step separately as a function which makes it very
simple to reuse them in different init stage (operational vs
intermediate).

> For Intermediate loadger image, all the above steps are applicable
> expcept #5 and #6.
>
> On HCI_OP_RESET, firmware raises alive interrupt. Driver needs to wait
> for it before passing control over to bluetooth stack.
>
> Co-developed-by: Devegowda Chandrashekar <chandrashekar.devegowda@intel.com>
> Signed-off-by: Devegowda Chandrashekar <chandrashekar.devegowda@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>  drivers/bluetooth/btintel.c      |  56 ++++++-
>  drivers/bluetooth/btintel.h      |   7 +
>  drivers/bluetooth/btintel_pcie.c | 262 ++++++++++++++++++++++++++++++-
>  drivers/bluetooth/btintel_pcie.h |  16 +-
>  4 files changed, 329 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 1ccbb5157515..fed1a939aad6 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1841,6 +1841,37 @@ static int btintel_boot_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
>         return 0;
>  }
>
> +static int btintel_boot_wait_d0(struct hci_dev *hdev, ktime_t calltime,
> +                               int msec)
> +{
> +       ktime_t delta, rettime;
> +       unsigned long long duration;
> +       int err;
> +
> +       bt_dev_info(hdev, "Waiting for device transition to d0");
> +
> +       err = btintel_wait_on_flag_timeout(hdev, INTEL_WAIT_FOR_D0,
> +                                          TASK_INTERRUPTIBLE,
> +                                          msecs_to_jiffies(msec));
> +       if (err == -EINTR) {
> +               bt_dev_err(hdev, "Device d0 move interrupted");
> +               return -EINTR;
> +       }
> +
> +       if (err) {
> +               bt_dev_err(hdev, "Device d0 move timeout");
> +               return -ETIMEDOUT;
> +       }
> +
> +       rettime = ktime_get();
> +       delta = ktime_sub(rettime, calltime);
> +       duration = (unsigned long long)ktime_to_ns(delta) >> 10;
> +
> +       bt_dev_info(hdev, "Device moved to D0 in %llu usecs", duration);
> +
> +       return 0;
> +}
> +
>  static int btintel_boot(struct hci_dev *hdev, u32 boot_addr)
>  {
>         ktime_t calltime;
> @@ -1849,6 +1880,7 @@ static int btintel_boot(struct hci_dev *hdev, u32 boot_addr)
>         calltime = ktime_get();
>
>         btintel_set_flag(hdev, INTEL_BOOTING);
> +       btintel_set_flag(hdev, INTEL_WAIT_FOR_D0);
>
>         err = btintel_send_intel_reset(hdev, boot_addr);
>         if (err) {
> @@ -1861,13 +1893,28 @@ static int btintel_boot(struct hci_dev *hdev, u32 boot_addr)
>          * is done by the operational firmware sending bootup notification.
>          *
>          * Booting into operational firmware should not take longer than
> -        * 1 second. However if that happens, then just fail the setup
> +        * 5 second. However if that happens, then just fail the setup
>          * since something went wrong.
>          */
> -       err = btintel_boot_wait(hdev, calltime, 1000);
> -       if (err == -ETIMEDOUT)
> +       err = btintel_boot_wait(hdev, calltime, 5000);
> +       if (err == -ETIMEDOUT) {
>                 btintel_reset_to_bootloader(hdev);
> +               goto exit_error;
> +       }
>
> +       if (hdev->bus == HCI_PCI) {
> +               /* In case of PCIe, after receiving bootup event, driver performs
> +                * D0 entry by writing 0 to sleep control register (check
> +                * btintel_pcie_recv_event())
> +                * Firmware acks with alive interrupt indicating host is full ready to
> +                * perform BT operation. Lets wait here till INTEL_WAIT_FOR_D0
> +                * bit is cleared.
> +                */
> +               calltime = ktime_get();
> +               err = btintel_boot_wait_d0(hdev, calltime, 2000);
> +       }
> +
> +exit_error:
>         return err;
>  }
>
> @@ -3273,7 +3320,7 @@ int btintel_configure_setup(struct hci_dev *hdev, const char *driver_name)
>  }
>  EXPORT_SYMBOL_GPL(btintel_configure_setup);
>
> -static int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
> +int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>         struct intel_tlv *tlv = (void *)&skb->data[5];
>
> @@ -3302,6 +3349,7 @@ static int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
>  recv_frame:
>         return hci_recv_frame(hdev, skb);
>  }
> +EXPORT_SYMBOL_GPL(btintel_diagnostics);
>
>  int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>  {
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index aa70e4c27416..b448c67e8ed9 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -178,6 +178,7 @@ enum {
>         INTEL_ROM_LEGACY,
>         INTEL_ROM_LEGACY_NO_WBS_SUPPORT,
>         INTEL_ACPI_RESET_ACTIVE,
> +       INTEL_WAIT_FOR_D0,
>
>         __INTEL_NUM_FLAGS,
>  };
> @@ -249,6 +250,7 @@ int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
>  int btintel_shutdown_combined(struct hci_dev *hdev);
>  void btintel_hw_error(struct hci_dev *hdev, u8 code);
>  void btintel_print_fseq_info(struct hci_dev *hdev);
> +int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb);
>  #else
>
>  static inline int btintel_check_bdaddr(struct hci_dev *hdev)
> @@ -382,4 +384,9 @@ static inline void btintel_hw_error(struct hci_dev *hdev, u8 code)
>  static inline void btintel_print_fseq_info(struct hci_dev *hdev)
>  {
>  }
> +
> +static inline int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       return -EOPNOTSUPP;
> +}
>  #endif
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index fda47948c35d..cff3e7afdff9 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -48,6 +48,17 @@ MODULE_DEVICE_TABLE(pci, btintel_pcie_table);
>  #define BTINTEL_PCIE_HCI_EVT_PKT       0x00000004
>  #define BTINTEL_PCIE_HCI_ISO_PKT       0x00000005
>
> +/* Alive interrupt context */
> +enum {
> +       BTINTEL_PCIE_ROM,
> +       BTINTEL_PCIE_FW_DL,
> +       BTINTEL_PCIE_HCI_RESET,
> +       BTINTEL_PCIE_INTEL_HCI_RESET1,
> +       BTINTEL_PCIE_INTEL_HCI_RESET2,
> +       BTINTEL_PCIE_D0,
> +       BTINTEL_PCIE_D3
> +};
> +
>  static inline void ipc_print_ia_ring(struct hci_dev *hdev, struct ia *ia,
>                                      u16 queue_num)
>  {
> @@ -290,8 +301,9 @@ static int btintel_pcie_enable_bt(struct btintel_pcie_data *data)
>         /* wait for interrupt from the device after booting up to primary
>          * bootloader.
>          */
> +       data->alive_intr_ctxt = BTINTEL_PCIE_ROM;
>         err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
> -                                msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT));
> +                                msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
>         if (!err)
>                 return -ETIME;
>
> @@ -302,12 +314,78 @@ static int btintel_pcie_enable_bt(struct btintel_pcie_data *data)
>         return 0;
>  }
>
> +/* BIT(0) - ROM, BIT(1) - IML and BIT(3) - OP
> + * Sometimes during firmware image switching from ROM to IML or IML to OP image,
> + * the previous image bit is not cleared by firmware when alive interrupt is
> + * received. Driver needs to take care of these sticky bits when deciding the
> + * current image running on controller.
> + * Ex: 0x10 and 0x11 - both represents that controller is running IML
> + */
> +static inline bool btintel_pcie_in_rom(struct btintel_pcie_data *data)
> +{
> +       return data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_ROM &&
> +               !(data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_IML) &&
> +               !(data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_OPFW);
> +}
> +
> +static inline bool btintel_pcie_in_op(struct btintel_pcie_data *data)
> +{
> +       return data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_OPFW;
> +}
> +
> +static inline bool btintel_pcie_in_iml(struct btintel_pcie_data *data)
> +{
> +       return data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_IML &&
> +               !(data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_OPFW);
> +}
> +
> +static inline bool btintel_pcie_in_d3(struct btintel_pcie_data *data)
> +{
> +       return data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY;
> +}
> +
> +static inline bool btintel_pcie_in_d0(struct btintel_pcie_data *data)
> +{
> +       return !(data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY);
> +}
> +
> +static void btintel_pcie_wr_sleep_cntrl(struct btintel_pcie_data *data,
> +                                       u32 dxstate)
> +{
> +       bt_dev_dbg(data->hdev, "writing sleep_ctl_reg: 0x%8.8x", dxstate);
> +       btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_IPC_SLEEP_CTL_REG, dxstate);
> +}
> +
> +static inline char *btintel_pcie_alivectxt_state2str(u32 alive_intr_ctxt)
> +{
> +       switch (alive_intr_ctxt) {
> +       case BTINTEL_PCIE_ROM:
> +               return "rom";
> +       case BTINTEL_PCIE_FW_DL:
> +               return "fw_dl";
> +       case BTINTEL_PCIE_D0:
> +               return "d0";
> +       case BTINTEL_PCIE_D3:
> +               return "d3";
> +       case BTINTEL_PCIE_HCI_RESET:
> +               return "hci_reset";
> +       case BTINTEL_PCIE_INTEL_HCI_RESET1:
> +               return "intel_reset1";
> +       case BTINTEL_PCIE_INTEL_HCI_RESET2:
> +               return "intel_reset2";
> +       default:
> +               return "unknown";
> +       }
> +       return "null";
> +}
> +
>  /* This function handles the MSI-X interrupt for gp0 cause (bit 0 in
>   * BTINTEL_PCIE_CSR_MSIX_HW_INT_CAUSES) which is sent for boot stage and image response.
>   */
>  static void btintel_pcie_msix_gp0_handler(struct btintel_pcie_data *data)
>  {
> -       u32 reg;
> +       bool submit_rx, signal_waitq;
> +       u32 reg, old_ctxt;
>
>         /* This interrupt is for three different causes and it is not easy to
>          * know what causes the interrupt. So, it compares each register value
> @@ -317,20 +395,87 @@ static void btintel_pcie_msix_gp0_handler(struct btintel_pcie_data *data)
>         if (reg != data->boot_stage_cache)
>                 data->boot_stage_cache = reg;
>
> +       bt_dev_dbg(data->hdev, "Alive context: %s old_boot_stage: 0x%8.8x new_boot_stage: 0x%8.8x",
> +                  btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt),
> +                  data->boot_stage_cache, reg);
>         reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_IMG_RESPONSE_REG);
>         if (reg != data->img_resp_cache)
>                 data->img_resp_cache = reg;
>
>         data->gp0_received = true;
>
> -       /* If the boot stage is OP or IML, reset IA and start RX again */
> -       if (data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_OPFW ||
> -           data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_IML) {
> +       old_ctxt = data->alive_intr_ctxt;
> +       submit_rx = false;
> +       signal_waitq = false;
> +
> +       switch (data->alive_intr_ctxt) {
> +       case BTINTEL_PCIE_ROM:
> +               data->alive_intr_ctxt = BTINTEL_PCIE_FW_DL;
> +               signal_waitq = true;
> +               break;
> +       case BTINTEL_PCIE_FW_DL:
> +               /* Error case is already handled. Ideally control shall not
> +                * reach here
> +                */
> +               break;
> +       case BTINTEL_PCIE_INTEL_HCI_RESET1:
> +               if (btintel_pcie_in_op(data)) {
> +                       submit_rx = true;
> +                       break;
> +               }
> +
> +               if (btintel_pcie_in_iml(data)) {
> +                       submit_rx = true;
> +                       data->alive_intr_ctxt = BTINTEL_PCIE_FW_DL;
> +                       break;
> +               }
> +               break;
> +       case BTINTEL_PCIE_INTEL_HCI_RESET2:
> +               if (btintel_test_and_clear_flag(data->hdev, INTEL_WAIT_FOR_D0)) {
> +                       btintel_wake_up_flag(data->hdev, INTEL_WAIT_FOR_D0);
> +                       data->alive_intr_ctxt = BTINTEL_PCIE_D0;
> +               }
> +               break;
> +       case BTINTEL_PCIE_D0:
> +               if (btintel_pcie_in_d3(data)) {
> +                       data->alive_intr_ctxt = BTINTEL_PCIE_D3;
> +                       signal_waitq = true;
> +                       break;
> +               }
> +               break;
> +       case BTINTEL_PCIE_D3:
> +               if (btintel_pcie_in_d0(data)) {
> +                       data->alive_intr_ctxt = BTINTEL_PCIE_D0;
> +                       submit_rx = true;
> +                       signal_waitq = true;
> +                       break;
> +               }
> +               break;
> +       case BTINTEL_PCIE_HCI_RESET:
> +               data->alive_intr_ctxt = BTINTEL_PCIE_D0;
> +               submit_rx = true;
> +               signal_waitq = true;
> +               break;
> +       default:
> +               bt_dev_err(data->hdev, "Unknown state: 0x%2.2x",
> +                          data->alive_intr_ctxt);
> +               break;
> +       }
> +
> +       if (submit_rx) {
>                 btintel_pcie_reset_ia(data);
>                 btintel_pcie_start_rx(data);
>         }
>
> -       wake_up(&data->gp0_wait_q);
> +       if (signal_waitq) {
> +               bt_dev_dbg(data->hdev, "wake up gp0 wait_q");
> +               wake_up(&data->gp0_wait_q);
> +       }
> +
> +       if (old_ctxt != data->alive_intr_ctxt)
> +               bt_dev_dbg(data->hdev, "alive context changed: %s  ->  %s",
> +                          btintel_pcie_alivectxt_state2str(old_ctxt),
> +                          btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
>  }
>
>  /* This function handles the MSX-X interrupt for rx queue 0 which is for TX
> @@ -364,6 +509,80 @@ static void btintel_pcie_msix_tx_handle(struct btintel_pcie_data *data)
>         }
>  }
>
> +static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       struct hci_event_hdr *hdr = (void *)skb->data;
> +       const char diagnostics_hdr[] = { 0x87, 0x80, 0x03 };
> +       struct btintel_pcie_data *data = hci_get_drvdata(hdev);
> +
> +       if (skb->len > HCI_EVENT_HDR_SIZE && hdr->evt == 0xff &&
> +           hdr->plen > 0) {
> +               const void *ptr = skb->data + HCI_EVENT_HDR_SIZE + 1;
> +               unsigned int len = skb->len - HCI_EVENT_HDR_SIZE - 1;
> +
> +               if (btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
> +                       switch (skb->data[2]) {
> +                       case 0x02:
> +                               /* When switching to the operational firmware
> +                                * the device sends a vendor specific event
> +                                * indicating that the bootup completed.
> +                                */
> +                               btintel_bootup(hdev, ptr, len);
> +
> +                               /* If bootup event is from operational image,
> +                                * driver needs to write sleep control register to
> +                                * move into D0 state
> +                                */
> +                               if (btintel_pcie_in_op(data)) {
> +                                       btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
> +                                       data->alive_intr_ctxt = BTINTEL_PCIE_INTEL_HCI_RESET2;
> +                                       break;
> +                               }
> +
> +                               if (btintel_pcie_in_iml(data)) {
> +                                       /* In case of IML, there is no concept
> +                                        * of D0 transition. Just mimic as if
> +                                        * IML moved to D0 by clearing INTEL_WAIT_FOR_D0
> +                                        * bit and waking up the task waiting on
> +                                        * INTEL_WAIT_FOR_D0. This is required
> +                                        * as intel_boot() is common function for
> +                                        * both IML and OP image loading.
> +                                        */
> +                                       if (btintel_test_and_clear_flag(data->hdev,
> +                                                                       INTEL_WAIT_FOR_D0))
> +                                               btintel_wake_up_flag(data->hdev,
> +                                                                    INTEL_WAIT_FOR_D0);
> +                               }
> +                               break;
> +                       case 0x06:
> +                               /* When the firmware loading completes the
> +                                * device sends out a vendor specific event
> +                                * indicating the result of the firmware
> +                                * loading.
> +                                */
> +                               btintel_secure_send_result(hdev, ptr, len);
> +                               break;
> +                       }
> +               }
> +
> +               /* Handle all diagnostics events separately. May still call
> +                * hci_recv_frame.
> +                */
> +               if (len >= sizeof(diagnostics_hdr) &&
> +                   memcmp(&skb->data[2], diagnostics_hdr,
> +                          sizeof(diagnostics_hdr)) == 0) {
> +                       return btintel_diagnostics(hdev, skb);
> +               }
> +
> +               /* This is a debug event that comes from IML and OP image when it
> +                * starts execution. There is no need pass this event to stack.
> +                */
> +               if (skb->data[2] == 0x97)
> +                       return 0;
> +       }
> +
> +       return hci_recv_frame(hdev, skb);
> +}
>  /* Process the received rx data
>   * It check the frame header to identify the data type and create skb
>   * and calling HCI API
> @@ -465,7 +684,7 @@ static int btintel_pcie_recv_frame(struct btintel_pcie_data *data,
>         hdev->stat.byte_rx += plen;
>
>         if (pcie_pkt_type == BTINTEL_PCIE_HCI_EVT_PKT)
> -               ret = btintel_recv_event(hdev, new_skb);
> +               ret = btintel_pcie_recv_event(hdev, new_skb);
>         else
>                 ret = hci_recv_frame(hdev, new_skb);
>
> @@ -1053,8 +1272,11 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>                                        struct sk_buff *skb)
>  {
>         struct btintel_pcie_data *data = hci_get_drvdata(hdev);
> +       struct hci_command_hdr *cmd;
> +       __u16 opcode = ~0;
>         int ret;
>         u32 type;
> +       u32 old_ctxt;
>
>         /* Due to the fw limitation, the type header of the packet should be
>          * 4 bytes unlike 1 byte for UART. In UART, the firmware can read
> @@ -1073,6 +1295,8 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>         switch (hci_skb_pkt_type(skb)) {
>         case HCI_COMMAND_PKT:
>                 type = BTINTEL_PCIE_HCI_CMD_PKT;
> +               cmd = (void *)skb->data;
> +               opcode = le16_to_cpu(cmd->opcode);
>                 if (btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
>                         struct hci_command_hdr *cmd = (void *)skb->data;
>                         __u16 opcode = le16_to_cpu(cmd->opcode);
> @@ -1111,6 +1335,30 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>                 bt_dev_err(hdev, "Failed to send frame (%d)", ret);
>                 goto exit_error;
>         }
> +
> +       if (type == BTINTEL_PCIE_HCI_CMD_PKT &&
> +           (opcode == HCI_OP_RESET || opcode == 0xfc01)) {
> +               old_ctxt = data->alive_intr_ctxt;
> +               data->alive_intr_ctxt =
> +                       (opcode == 0xfc01 ? BTINTEL_PCIE_INTEL_HCI_RESET1 :
> +                               BTINTEL_PCIE_HCI_RESET);
> +               bt_dev_dbg(data->hdev, "sent cmd: 0x%4.4x alive context changed: %s  ->  %s",
> +                          opcode, btintel_pcie_alivectxt_state2str(old_ctxt),
> +                          btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
> +               if (opcode == HCI_OP_RESET) {
> +                       data->gp0_received = false;
> +                       ret = wait_event_timeout(data->gp0_wait_q,
> +                                                data->gp0_received,
> +                                                msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
> +                       if (!ret) {
> +                               hdev->stat.err_tx++;
> +                               bt_dev_err(hdev, "No alive interrupt received for %s",
> +                                          btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
> +                               ret = -ETIME;
> +                               goto exit_error;
> +                       }
> +               }
> +       }
>         hdev->stat.byte_tx += skb->len;
>         kfree_skb(skb);
>
> diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
> index baaff70420f5..8b7824ad005a 100644
> --- a/drivers/bluetooth/btintel_pcie.h
> +++ b/drivers/bluetooth/btintel_pcie.h
> @@ -12,6 +12,7 @@
>  #define BTINTEL_PCIE_CSR_HW_REV_REG            (BTINTEL_PCIE_CSR_BASE + 0x028)
>  #define BTINTEL_PCIE_CSR_RF_ID_REG             (BTINTEL_PCIE_CSR_BASE + 0x09C)
>  #define BTINTEL_PCIE_CSR_BOOT_STAGE_REG                (BTINTEL_PCIE_CSR_BASE + 0x108)
> +#define BTINTEL_PCIE_CSR_IPC_SLEEP_CTL_REG     (BTINTEL_PCIE_CSR_BASE + 0x114)
>  #define BTINTEL_PCIE_CSR_CI_ADDR_LSB_REG       (BTINTEL_PCIE_CSR_BASE + 0x118)
>  #define BTINTEL_PCIE_CSR_CI_ADDR_MSB_REG       (BTINTEL_PCIE_CSR_BASE + 0x11C)
>  #define BTINTEL_PCIE_CSR_IMG_RESPONSE_REG      (BTINTEL_PCIE_CSR_BASE + 0x12C)
> @@ -32,6 +33,7 @@
>  #define BTINTEL_PCIE_CSR_BOOT_STAGE_IML_LOCKDOWN       (BIT(11))
>  #define BTINTEL_PCIE_CSR_BOOT_STAGE_MAC_ACCESS_ON      (BIT(16))
>  #define BTINTEL_PCIE_CSR_BOOT_STAGE_ALIVE              (BIT(23))
> +#define BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY     (BIT(24))
>
>  /* Registers for MSI-X */
>  #define BTINTEL_PCIE_CSR_MSIX_BASE             (0x2000)
> @@ -55,6 +57,16 @@ enum msix_hw_int_causes {
>         BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0     = BIT(0),       /* cause 32 */
>  };
>
> +/* PCIe device states
> + * Host-Device interface is active
> + * Host-Device interface is inactive(as reflected by IPC_SLEEP_CONTROL_CSR_AD)
> + * Host-Device interface is inactive(as reflected by IPC_SLEEP_CONTROL_CSR_AD)
> + */
> +enum {
> +       BTINTEL_PCIE_STATE_D0 = 0,
> +       BTINTEL_PCIE_STATE_D3_HOT = 2,
> +       BTINTEL_PCIE_STATE_D3_COLD = 3,
> +};
>  #define BTINTEL_PCIE_MSIX_NON_AUTO_CLEAR_CAUSE BIT(7)
>
>  /* Minimum and Maximum number of MSI-X Vector
> @@ -67,7 +79,7 @@ enum msix_hw_int_causes {
>  #define BTINTEL_DEFAULT_MAC_ACCESS_TIMEOUT_US  200000
>
>  /* Default interrupt timeout in msec */
> -#define BTINTEL_DEFAULT_INTR_TIMEOUT   3000
> +#define BTINTEL_DEFAULT_INTR_TIMEOUT_MS        3000
>
>  /* The number of descriptors in TX/RX queues */
>  #define BTINTEL_DESCS_COUNT    16
> @@ -343,6 +355,7 @@ struct rxq {
>   * @ia: Index Array struct
>   * @txq: TX Queue struct
>   * @rxq: RX Queue struct
> + * @alive_intr_ctxt: Alive interrupt context
>   */
>  struct btintel_pcie_data {
>         struct pci_dev  *pdev;
> @@ -389,6 +402,7 @@ struct btintel_pcie_data {
>         struct ia       ia;
>         struct txq      txq;
>         struct rxq      rxq;
> +       u32     alive_intr_ctxt;
>  };
>
>  static inline u32 btintel_pcie_rd_reg32(struct btintel_pcie_data *data,
> --
> 2.40.1
>
>
K, Kiran Oct. 7, 2024, 1:51 p.m. UTC | #2
Hi Luiz,

Thanks for the comments.

>-----Original Message-----
>From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
>Sent: Tuesday, October 1, 2024 8:03 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>; Devegowda, Chandrashekar
><chandrashekar.devegowda@intel.com>
>Subject: Re: [PATCH v1 1/2] Bluetooth: btintel_pcie: Add handshake between
>driver and firmware
>
>Hi Kiran,
>
>On Tue, Oct 1, 2024 at 6:29 AM Kiran K <kiran.k@intel.com> wrote:
>>
>> The following handshake mechanism needs be followed after firmware
>> download is completed to bring the firmware to running state.
>>
>> After firmware fragments of Operational image are downloaded and
>> secure sends result of the image succeeds,
>>
>> 1. Driver sends HCI Intel reset with boot option #1 to switch FW image.
>> 2. FW sends Alive GP[0] MSIx
>> 3. Driver enables data path (doorbell 0x460 for RBDs, etc...) 4.
>> Driver gets Bootup event from firmware 5. Driver performs D0 entry to
>> device (WRITE to IPC_Sleep_Control =0x0) 6. FW sends Alive GP[0] MSIx
>> 7. Device host interface is fully set for BT protocol stack operation.
>> 8. Driver may optionally get debug event with ID 0x97 which can be
>> dropped
>
>We should probably start versioning the boot sequence from the very
>beginning, 

Ack. I will start adding the version information as comments in code.

I also wonder if we should promote the use usage of something like
>the hci_init_stage_sync for such use cases so one can clearly define each step
>separately as a function which makes it very simple to reuse them in different
>init stage (operational vs intermediate).
>

I wonder this might more complexity in handling both USB and PCIE transport as most of the firmware download code is shared between them.

>> For Intermediate loadger image, all the above steps are applicable
>> expcept #5 and #6.
>>
>> On HCI_OP_RESET, firmware raises alive interrupt. Driver needs to wait
>> for it before passing control over to bluetooth stack.
>>
>> Co-developed-by: Devegowda Chandrashekar
>> <chandrashekar.devegowda@intel.com>
>> Signed-off-by: Devegowda Chandrashekar
>> <chandrashekar.devegowda@intel.com>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>>  drivers/bluetooth/btintel.c      |  56 ++++++-
>>  drivers/bluetooth/btintel.h      |   7 +
>>  drivers/bluetooth/btintel_pcie.c | 262
>> ++++++++++++++++++++++++++++++-  drivers/bluetooth/btintel_pcie.h |
>> 16 +-
>>  4 files changed, 329 insertions(+), 12 deletions(-)
>>

Thanks,
Kiran
patchwork-bot+bluetooth@kernel.org Oct. 11, 2024, 6:32 p.m. UTC | #3
Hello:

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

On Tue,  1 Oct 2024 16:14:50 +0530 you wrote:
> The following handshake mechanism needs be followed after firmware
> download is completed to bring the firmware to running state.
> 
> After firmware fragments of Operational image are downloaded and
> secure sends result of the image succeeds,
> 
> 1. Driver sends HCI Intel reset with boot option #1 to switch FW image.
> 2. FW sends Alive GP[0] MSIx
> 3. Driver enables data path (doorbell 0x460 for RBDs, etc...)
> 4. Driver gets Bootup event from firmware
> 5. Driver performs D0 entry to device (WRITE to IPC_Sleep_Control =0x0)
> 6. FW sends Alive GP[0] MSIx
> 7. Device host interface is fully set for BT protocol stack operation.
> 8. Driver may optionally get debug event with ID 0x97 which can be dropped
> 
> [...]

Here is the summary with links:
  - [v1,1/2] Bluetooth: btintel_pcie: Add handshake between driver and firmware
    https://git.kernel.org/bluetooth/bluetooth-next/c/5ea625845b0f
  - [v1,2/2] Bluetooth: btintel_pcie: Add recovery mechanism
    (no matching commit)

You are awesome, thank you!
Bjorn Helgaas Nov. 11, 2024, 6:29 p.m. UTC | #4
On Tue, Oct 01, 2024 at 04:14:50PM +0530, Kiran K wrote:
> The following handshake mechanism needs be followed after firmware
> download is completed to bring the firmware to running state.
> 
> After firmware fragments of Operational image are downloaded and
> secure sends result of the image succeeds,
> 
> 1. Driver sends HCI Intel reset with boot option #1 to switch FW image.
> 2. FW sends Alive GP[0] MSIx
> 3. Driver enables data path (doorbell 0x460 for RBDs, etc...)
> 4. Driver gets Bootup event from firmware
> 5. Driver performs D0 entry to device (WRITE to IPC_Sleep_Control =0x0)
> 6. FW sends Alive GP[0] MSIx
> 7. Device host interface is fully set for BT protocol stack operation.
> 8. Driver may optionally get debug event with ID 0x97 which can be dropped
> 
> For Intermediate loadger image, all the above steps are applicable
> expcept #5 and #6.

I see this is already applied to bluetooth-next and is probably
immutable, but if not...

s/loadger/loader/
s/expcept/except/

But more importantly, the race below is still in linux-next as of
next-20241111.  I mentioned this before at
https://lore.kernel.org/all/20241023191352.GA924610@bhelgaas/#t, but
it probably got lost in the suspend/resume conversation.

> @@ -1053,8 +1272,11 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>  				       struct sk_buff *skb)
>  {
>  	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
> +	struct hci_command_hdr *cmd;
> +	__u16 opcode = ~0;
>  	int ret;
>  	u32 type;
> +	u32 old_ctxt;
>  
>  	/* Due to the fw limitation, the type header of the packet should be
>  	 * 4 bytes unlike 1 byte for UART. In UART, the firmware can read
> @@ -1073,6 +1295,8 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>  	switch (hci_skb_pkt_type(skb)) {
>  	case HCI_COMMAND_PKT:
>  		type = BTINTEL_PCIE_HCI_CMD_PKT;
> +		cmd = (void *)skb->data;
> +		opcode = le16_to_cpu(cmd->opcode);
>  		if (btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
>  			struct hci_command_hdr *cmd = (void *)skb->data;
>  			__u16 opcode = le16_to_cpu(cmd->opcode);
> @@ -1111,6 +1335,30 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>  		bt_dev_err(hdev, "Failed to send frame (%d)", ret);
>  		goto exit_error;
>  	}
> +
> +	if (type == BTINTEL_PCIE_HCI_CMD_PKT &&
> +	    (opcode == HCI_OP_RESET || opcode == 0xfc01)) {
> +		old_ctxt = data->alive_intr_ctxt;
> +		data->alive_intr_ctxt =
> +			(opcode == 0xfc01 ? BTINTEL_PCIE_INTEL_HCI_RESET1 :
> +				BTINTEL_PCIE_HCI_RESET);
> +		bt_dev_dbg(data->hdev, "sent cmd: 0x%4.4x alive context changed: %s  ->  %s",
> +			   opcode, btintel_pcie_alivectxt_state2str(old_ctxt),
> +			   btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
> +		if (opcode == HCI_OP_RESET) {
> +			data->gp0_received = false;
> +			ret = wait_event_timeout(data->gp0_wait_q,
> +						 data->gp0_received,
> +						 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));

This looks like a race.  You're apparently started something
previously that will eventually result in data->gp0_received being
set.  The ordering you expect is this:

  1) Tell device to do something and interrupt on completion
  2) Set data->gp0_received = false here
  3) Call wait_event_timeout() here
  4) Completion interrupt causes btintel_pcie_msix_gp0_handler() to be
     called
  5) btintel_pcie_msix_gp0_handler() sets data->gp0_received = true
  6) wait_event_timeout() completes

But this ordering can also happen:

  1) Tell device to do something and interrupt on completion
  2) Completion interrupt causes
     btintel_pcie_msix_gp0_handler() to be called
  3) btintel_pcie_msix_gp0_handler() sets data->gp0_received = true
  4) Set data->gp0_received = false here, overwriting the "true"
  5) Call wait_event_timeout() here
  6) wait_event_timeout() times out because completion interrupt has
     already happened
  7) We complain "No alive interrupt received" here

You should be able to force this failure by adding a sleep before
setting data->gp0_received = false in this patch.

> +			if (!ret) {
> +				hdev->stat.err_tx++;
> +				bt_dev_err(hdev, "No alive interrupt received for %s",
> +					   btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
> +				ret = -ETIME;
> +				goto exit_error;
> +			}
> +		}
> +	}
>  	hdev->stat.byte_tx += skb->len;
>  	kfree_skb(skb);
K, Kiran Nov. 12, 2024, 3:59 p.m. UTC | #5
Hi Bjorn,

>-----Original Message-----
>From: Bjorn Helgaas <helgaas@kernel.org>
>Sent: Monday, November 11, 2024 11:59 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>; Devegowda, Chandrashekar
><chandrashekar.devegowda@intel.com>
>Subject: Re: [PATCH v1 1/2] Bluetooth: btintel_pcie: Add handshake between
>driver and firmware
>
>On Tue, Oct 01, 2024 at 04:14:50PM +0530, Kiran K wrote:
>> The following handshake mechanism needs be followed after firmware
>> download is completed to bring the firmware to running state.
>>
>> After firmware fragments of Operational image are downloaded and
>> secure sends result of the image succeeds,
>>
>> 1. Driver sends HCI Intel reset with boot option #1 to switch FW image.
>> 2. FW sends Alive GP[0] MSIx
>> 3. Driver enables data path (doorbell 0x460 for RBDs, etc...) 4.
>> Driver gets Bootup event from firmware 5. Driver performs D0 entry to
>> device (WRITE to IPC_Sleep_Control =0x0) 6. FW sends Alive GP[0] MSIx
>> 7. Device host interface is fully set for BT protocol stack operation.
>> 8. Driver may optionally get debug event with ID 0x97 which can be
>> dropped
>>
>> For Intermediate loadger image, all the above steps are applicable
>> expcept #5 and #6.
>
>I see this is already applied to bluetooth-next and is probably immutable, but
>if not...
>
>s/loadger/loader/
>s/expcept/except/
>
>But more importantly, the race below is still in linux-next as of next-20241111.
>I mentioned this before at
>https://lore.kernel.org/all/20241023191352.GA924610@bhelgaas/#t, but it
>probably got lost in the suspend/resume conversation.
>
>> @@ -1053,8 +1272,11 @@ static int btintel_pcie_send_frame(struct hci_dev
>*hdev,
>>  				       struct sk_buff *skb)
>>  {
>>  	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
>> +	struct hci_command_hdr *cmd;
>> +	__u16 opcode = ~0;
>>  	int ret;
>>  	u32 type;
>> +	u32 old_ctxt;
>>
>>  	/* Due to the fw limitation, the type header of the packet should be
>>  	 * 4 bytes unlike 1 byte for UART. In UART, the firmware can read @@
>> -1073,6 +1295,8 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>>  	switch (hci_skb_pkt_type(skb)) {
>>  	case HCI_COMMAND_PKT:
>>  		type = BTINTEL_PCIE_HCI_CMD_PKT;
>> +		cmd = (void *)skb->data;
>> +		opcode = le16_to_cpu(cmd->opcode);
>>  		if (btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
>>  			struct hci_command_hdr *cmd = (void *)skb->data;
>>  			__u16 opcode = le16_to_cpu(cmd->opcode); @@ -
>1111,6 +1335,30 @@
>> static int btintel_pcie_send_frame(struct hci_dev *hdev,
>>  		bt_dev_err(hdev, "Failed to send frame (%d)", ret);
>>  		goto exit_error;
>>  	}
>> +
>> +	if (type == BTINTEL_PCIE_HCI_CMD_PKT &&
>> +	    (opcode == HCI_OP_RESET || opcode == 0xfc01)) {
>> +		old_ctxt = data->alive_intr_ctxt;
>> +		data->alive_intr_ctxt =
>> +			(opcode == 0xfc01 ? BTINTEL_PCIE_INTEL_HCI_RESET1
>:
>> +				BTINTEL_PCIE_HCI_RESET);
>> +		bt_dev_dbg(data->hdev, "sent cmd: 0x%4.4x alive context
>changed: %s  ->  %s",
>> +			   opcode, btintel_pcie_alivectxt_state2str(old_ctxt),
>> +			   btintel_pcie_alivectxt_state2str(data-
>>alive_intr_ctxt));
>> +		if (opcode == HCI_OP_RESET) {
>> +			data->gp0_received = false;
>> +			ret = wait_event_timeout(data->gp0_wait_q,
>> +						 data->gp0_received,
>> +
>msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
>
>This looks like a race.  You're apparently started something previously that will
>eventually result in data->gp0_received being set.  The ordering you expect is
>this:
>
>  1) Tell device to do something and interrupt on completion
>  2) Set data->gp0_received = false here
>  3) Call wait_event_timeout() here
>  4) Completion interrupt causes btintel_pcie_msix_gp0_handler() to be
>     called
>  5) btintel_pcie_msix_gp0_handler() sets data->gp0_received = true
>  6) wait_event_timeout() completes
>
>But this ordering can also happen:
>
>  1) Tell device to do something and interrupt on completion
>  2) Completion interrupt causes
>     btintel_pcie_msix_gp0_handler() to be called
>  3) btintel_pcie_msix_gp0_handler() sets data->gp0_received = true
>  4) Set data->gp0_received = false here, overwriting the "true"
>  5) Call wait_event_timeout() here
>  6) wait_event_timeout() times out because completion interrupt has
>     already happened
>  7) We complain "No alive interrupt received" here
>
Possible. Unfortunately, this issue did not show up in our test. I will reproduce this issue and publish the same.
>You should be able to force this failure by adding a sleep before setting data-
>>gp0_received = false in this patch.

Ack.


>
>> +			if (!ret) {
>> +				hdev->stat.err_tx++;
>> +				bt_dev_err(hdev, "No alive interrupt received
>for %s",
>> +					   btintel_pcie_alivectxt_state2str(data-
>>alive_intr_ctxt));
>> +				ret = -ETIME;
>> +				goto exit_error;
>> +			}
>> +		}
>> +	}
>>  	hdev->stat.byte_tx += skb->len;
>>  	kfree_skb(skb);

Thanks,
Kiran
diff mbox series

Patch

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 1ccbb5157515..fed1a939aad6 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1841,6 +1841,37 @@  static int btintel_boot_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
 	return 0;
 }
 
+static int btintel_boot_wait_d0(struct hci_dev *hdev, ktime_t calltime,
+				int msec)
+{
+	ktime_t delta, rettime;
+	unsigned long long duration;
+	int err;
+
+	bt_dev_info(hdev, "Waiting for device transition to d0");
+
+	err = btintel_wait_on_flag_timeout(hdev, INTEL_WAIT_FOR_D0,
+					   TASK_INTERRUPTIBLE,
+					   msecs_to_jiffies(msec));
+	if (err == -EINTR) {
+		bt_dev_err(hdev, "Device d0 move interrupted");
+		return -EINTR;
+	}
+
+	if (err) {
+		bt_dev_err(hdev, "Device d0 move timeout");
+		return -ETIMEDOUT;
+	}
+
+	rettime = ktime_get();
+	delta = ktime_sub(rettime, calltime);
+	duration = (unsigned long long)ktime_to_ns(delta) >> 10;
+
+	bt_dev_info(hdev, "Device moved to D0 in %llu usecs", duration);
+
+	return 0;
+}
+
 static int btintel_boot(struct hci_dev *hdev, u32 boot_addr)
 {
 	ktime_t calltime;
@@ -1849,6 +1880,7 @@  static int btintel_boot(struct hci_dev *hdev, u32 boot_addr)
 	calltime = ktime_get();
 
 	btintel_set_flag(hdev, INTEL_BOOTING);
+	btintel_set_flag(hdev, INTEL_WAIT_FOR_D0);
 
 	err = btintel_send_intel_reset(hdev, boot_addr);
 	if (err) {
@@ -1861,13 +1893,28 @@  static int btintel_boot(struct hci_dev *hdev, u32 boot_addr)
 	 * is done by the operational firmware sending bootup notification.
 	 *
 	 * Booting into operational firmware should not take longer than
-	 * 1 second. However if that happens, then just fail the setup
+	 * 5 second. However if that happens, then just fail the setup
 	 * since something went wrong.
 	 */
-	err = btintel_boot_wait(hdev, calltime, 1000);
-	if (err == -ETIMEDOUT)
+	err = btintel_boot_wait(hdev, calltime, 5000);
+	if (err == -ETIMEDOUT) {
 		btintel_reset_to_bootloader(hdev);
+		goto exit_error;
+	}
 
+	if (hdev->bus == HCI_PCI) {
+		/* In case of PCIe, after receiving bootup event, driver performs
+		 * D0 entry by writing 0 to sleep control register (check
+		 * btintel_pcie_recv_event())
+		 * Firmware acks with alive interrupt indicating host is full ready to
+		 * perform BT operation. Lets wait here till INTEL_WAIT_FOR_D0
+		 * bit is cleared.
+		 */
+		calltime = ktime_get();
+		err = btintel_boot_wait_d0(hdev, calltime, 2000);
+	}
+
+exit_error:
 	return err;
 }
 
@@ -3273,7 +3320,7 @@  int btintel_configure_setup(struct hci_dev *hdev, const char *driver_name)
 }
 EXPORT_SYMBOL_GPL(btintel_configure_setup);
 
-static int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
+int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct intel_tlv *tlv = (void *)&skb->data[5];
 
@@ -3302,6 +3349,7 @@  static int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
 recv_frame:
 	return hci_recv_frame(hdev, skb);
 }
+EXPORT_SYMBOL_GPL(btintel_diagnostics);
 
 int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 {
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index aa70e4c27416..b448c67e8ed9 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -178,6 +178,7 @@  enum {
 	INTEL_ROM_LEGACY,
 	INTEL_ROM_LEGACY_NO_WBS_SUPPORT,
 	INTEL_ACPI_RESET_ACTIVE,
+	INTEL_WAIT_FOR_D0,
 
 	__INTEL_NUM_FLAGS,
 };
@@ -249,6 +250,7 @@  int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
 int btintel_shutdown_combined(struct hci_dev *hdev);
 void btintel_hw_error(struct hci_dev *hdev, u8 code);
 void btintel_print_fseq_info(struct hci_dev *hdev);
+int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb);
 #else
 
 static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -382,4 +384,9 @@  static inline void btintel_hw_error(struct hci_dev *hdev, u8 code)
 static inline void btintel_print_fseq_info(struct hci_dev *hdev)
 {
 }
+
+static inline int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	return -EOPNOTSUPP;
+}
 #endif
diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index fda47948c35d..cff3e7afdff9 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -48,6 +48,17 @@  MODULE_DEVICE_TABLE(pci, btintel_pcie_table);
 #define BTINTEL_PCIE_HCI_EVT_PKT	0x00000004
 #define BTINTEL_PCIE_HCI_ISO_PKT	0x00000005
 
+/* Alive interrupt context */
+enum {
+	BTINTEL_PCIE_ROM,
+	BTINTEL_PCIE_FW_DL,
+	BTINTEL_PCIE_HCI_RESET,
+	BTINTEL_PCIE_INTEL_HCI_RESET1,
+	BTINTEL_PCIE_INTEL_HCI_RESET2,
+	BTINTEL_PCIE_D0,
+	BTINTEL_PCIE_D3
+};
+
 static inline void ipc_print_ia_ring(struct hci_dev *hdev, struct ia *ia,
 				     u16 queue_num)
 {
@@ -290,8 +301,9 @@  static int btintel_pcie_enable_bt(struct btintel_pcie_data *data)
 	/* wait for interrupt from the device after booting up to primary
 	 * bootloader.
 	 */
+	data->alive_intr_ctxt = BTINTEL_PCIE_ROM;
 	err = wait_event_timeout(data->gp0_wait_q, data->gp0_received,
-				 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT));
+				 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
 	if (!err)
 		return -ETIME;
 
@@ -302,12 +314,78 @@  static int btintel_pcie_enable_bt(struct btintel_pcie_data *data)
 	return 0;
 }
 
+/* BIT(0) - ROM, BIT(1) - IML and BIT(3) - OP
+ * Sometimes during firmware image switching from ROM to IML or IML to OP image,
+ * the previous image bit is not cleared by firmware when alive interrupt is
+ * received. Driver needs to take care of these sticky bits when deciding the
+ * current image running on controller.
+ * Ex: 0x10 and 0x11 - both represents that controller is running IML
+ */
+static inline bool btintel_pcie_in_rom(struct btintel_pcie_data *data)
+{
+	return data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_ROM &&
+		!(data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_IML) &&
+		!(data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_OPFW);
+}
+
+static inline bool btintel_pcie_in_op(struct btintel_pcie_data *data)
+{
+	return data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_OPFW;
+}
+
+static inline bool btintel_pcie_in_iml(struct btintel_pcie_data *data)
+{
+	return data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_IML &&
+		!(data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_OPFW);
+}
+
+static inline bool btintel_pcie_in_d3(struct btintel_pcie_data *data)
+{
+	return data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY;
+}
+
+static inline bool btintel_pcie_in_d0(struct btintel_pcie_data *data)
+{
+	return !(data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY);
+}
+
+static void btintel_pcie_wr_sleep_cntrl(struct btintel_pcie_data *data,
+					u32 dxstate)
+{
+	bt_dev_dbg(data->hdev, "writing sleep_ctl_reg: 0x%8.8x", dxstate);
+	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_IPC_SLEEP_CTL_REG, dxstate);
+}
+
+static inline char *btintel_pcie_alivectxt_state2str(u32 alive_intr_ctxt)
+{
+	switch (alive_intr_ctxt) {
+	case BTINTEL_PCIE_ROM:
+		return "rom";
+	case BTINTEL_PCIE_FW_DL:
+		return "fw_dl";
+	case BTINTEL_PCIE_D0:
+		return "d0";
+	case BTINTEL_PCIE_D3:
+		return "d3";
+	case BTINTEL_PCIE_HCI_RESET:
+		return "hci_reset";
+	case BTINTEL_PCIE_INTEL_HCI_RESET1:
+		return "intel_reset1";
+	case BTINTEL_PCIE_INTEL_HCI_RESET2:
+		return "intel_reset2";
+	default:
+		return "unknown";
+	}
+	return "null";
+}
+
 /* This function handles the MSI-X interrupt for gp0 cause (bit 0 in
  * BTINTEL_PCIE_CSR_MSIX_HW_INT_CAUSES) which is sent for boot stage and image response.
  */
 static void btintel_pcie_msix_gp0_handler(struct btintel_pcie_data *data)
 {
-	u32 reg;
+	bool submit_rx, signal_waitq;
+	u32 reg, old_ctxt;
 
 	/* This interrupt is for three different causes and it is not easy to
 	 * know what causes the interrupt. So, it compares each register value
@@ -317,20 +395,87 @@  static void btintel_pcie_msix_gp0_handler(struct btintel_pcie_data *data)
 	if (reg != data->boot_stage_cache)
 		data->boot_stage_cache = reg;
 
+	bt_dev_dbg(data->hdev, "Alive context: %s old_boot_stage: 0x%8.8x new_boot_stage: 0x%8.8x",
+		   btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt),
+		   data->boot_stage_cache, reg);
 	reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_IMG_RESPONSE_REG);
 	if (reg != data->img_resp_cache)
 		data->img_resp_cache = reg;
 
 	data->gp0_received = true;
 
-	/* If the boot stage is OP or IML, reset IA and start RX again */
-	if (data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_OPFW ||
-	    data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_IML) {
+	old_ctxt = data->alive_intr_ctxt;
+	submit_rx = false;
+	signal_waitq = false;
+
+	switch (data->alive_intr_ctxt) {
+	case BTINTEL_PCIE_ROM:
+		data->alive_intr_ctxt = BTINTEL_PCIE_FW_DL;
+		signal_waitq = true;
+		break;
+	case BTINTEL_PCIE_FW_DL:
+		/* Error case is already handled. Ideally control shall not
+		 * reach here
+		 */
+		break;
+	case BTINTEL_PCIE_INTEL_HCI_RESET1:
+		if (btintel_pcie_in_op(data)) {
+			submit_rx = true;
+			break;
+		}
+
+		if (btintel_pcie_in_iml(data)) {
+			submit_rx = true;
+			data->alive_intr_ctxt = BTINTEL_PCIE_FW_DL;
+			break;
+		}
+		break;
+	case BTINTEL_PCIE_INTEL_HCI_RESET2:
+		if (btintel_test_and_clear_flag(data->hdev, INTEL_WAIT_FOR_D0)) {
+			btintel_wake_up_flag(data->hdev, INTEL_WAIT_FOR_D0);
+			data->alive_intr_ctxt = BTINTEL_PCIE_D0;
+		}
+		break;
+	case BTINTEL_PCIE_D0:
+		if (btintel_pcie_in_d3(data)) {
+			data->alive_intr_ctxt = BTINTEL_PCIE_D3;
+			signal_waitq = true;
+			break;
+		}
+		break;
+	case BTINTEL_PCIE_D3:
+		if (btintel_pcie_in_d0(data)) {
+			data->alive_intr_ctxt = BTINTEL_PCIE_D0;
+			submit_rx = true;
+			signal_waitq = true;
+			break;
+		}
+		break;
+	case BTINTEL_PCIE_HCI_RESET:
+		data->alive_intr_ctxt = BTINTEL_PCIE_D0;
+		submit_rx = true;
+		signal_waitq = true;
+		break;
+	default:
+		bt_dev_err(data->hdev, "Unknown state: 0x%2.2x",
+			   data->alive_intr_ctxt);
+		break;
+	}
+
+	if (submit_rx) {
 		btintel_pcie_reset_ia(data);
 		btintel_pcie_start_rx(data);
 	}
 
-	wake_up(&data->gp0_wait_q);
+	if (signal_waitq) {
+		bt_dev_dbg(data->hdev, "wake up gp0 wait_q");
+		wake_up(&data->gp0_wait_q);
+	}
+
+	if (old_ctxt != data->alive_intr_ctxt)
+		bt_dev_dbg(data->hdev, "alive context changed: %s  ->  %s",
+			   btintel_pcie_alivectxt_state2str(old_ctxt),
+			   btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
 }
 
 /* This function handles the MSX-X interrupt for rx queue 0 which is for TX
@@ -364,6 +509,80 @@  static void btintel_pcie_msix_tx_handle(struct btintel_pcie_data *data)
 	}
 }
 
+static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_event_hdr *hdr = (void *)skb->data;
+	const char diagnostics_hdr[] = { 0x87, 0x80, 0x03 };
+	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
+
+	if (skb->len > HCI_EVENT_HDR_SIZE && hdr->evt == 0xff &&
+	    hdr->plen > 0) {
+		const void *ptr = skb->data + HCI_EVENT_HDR_SIZE + 1;
+		unsigned int len = skb->len - HCI_EVENT_HDR_SIZE - 1;
+
+		if (btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
+			switch (skb->data[2]) {
+			case 0x02:
+				/* When switching to the operational firmware
+				 * the device sends a vendor specific event
+				 * indicating that the bootup completed.
+				 */
+				btintel_bootup(hdev, ptr, len);
+
+				/* If bootup event is from operational image,
+				 * driver needs to write sleep control register to
+				 * move into D0 state
+				 */
+				if (btintel_pcie_in_op(data)) {
+					btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
+					data->alive_intr_ctxt = BTINTEL_PCIE_INTEL_HCI_RESET2;
+					break;
+				}
+
+				if (btintel_pcie_in_iml(data)) {
+					/* In case of IML, there is no concept
+					 * of D0 transition. Just mimic as if
+					 * IML moved to D0 by clearing INTEL_WAIT_FOR_D0
+					 * bit and waking up the task waiting on
+					 * INTEL_WAIT_FOR_D0. This is required
+					 * as intel_boot() is common function for
+					 * both IML and OP image loading.
+					 */
+					if (btintel_test_and_clear_flag(data->hdev,
+									INTEL_WAIT_FOR_D0))
+						btintel_wake_up_flag(data->hdev,
+								     INTEL_WAIT_FOR_D0);
+				}
+				break;
+			case 0x06:
+				/* When the firmware loading completes the
+				 * device sends out a vendor specific event
+				 * indicating the result of the firmware
+				 * loading.
+				 */
+				btintel_secure_send_result(hdev, ptr, len);
+				break;
+			}
+		}
+
+		/* Handle all diagnostics events separately. May still call
+		 * hci_recv_frame.
+		 */
+		if (len >= sizeof(diagnostics_hdr) &&
+		    memcmp(&skb->data[2], diagnostics_hdr,
+			   sizeof(diagnostics_hdr)) == 0) {
+			return btintel_diagnostics(hdev, skb);
+		}
+
+		/* This is a debug event that comes from IML and OP image when it
+		 * starts execution. There is no need pass this event to stack.
+		 */
+		if (skb->data[2] == 0x97)
+			return 0;
+	}
+
+	return hci_recv_frame(hdev, skb);
+}
 /* Process the received rx data
  * It check the frame header to identify the data type and create skb
  * and calling HCI API
@@ -465,7 +684,7 @@  static int btintel_pcie_recv_frame(struct btintel_pcie_data *data,
 	hdev->stat.byte_rx += plen;
 
 	if (pcie_pkt_type == BTINTEL_PCIE_HCI_EVT_PKT)
-		ret = btintel_recv_event(hdev, new_skb);
+		ret = btintel_pcie_recv_event(hdev, new_skb);
 	else
 		ret = hci_recv_frame(hdev, new_skb);
 
@@ -1053,8 +1272,11 @@  static int btintel_pcie_send_frame(struct hci_dev *hdev,
 				       struct sk_buff *skb)
 {
 	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
+	struct hci_command_hdr *cmd;
+	__u16 opcode = ~0;
 	int ret;
 	u32 type;
+	u32 old_ctxt;
 
 	/* Due to the fw limitation, the type header of the packet should be
 	 * 4 bytes unlike 1 byte for UART. In UART, the firmware can read
@@ -1073,6 +1295,8 @@  static int btintel_pcie_send_frame(struct hci_dev *hdev,
 	switch (hci_skb_pkt_type(skb)) {
 	case HCI_COMMAND_PKT:
 		type = BTINTEL_PCIE_HCI_CMD_PKT;
+		cmd = (void *)skb->data;
+		opcode = le16_to_cpu(cmd->opcode);
 		if (btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
 			struct hci_command_hdr *cmd = (void *)skb->data;
 			__u16 opcode = le16_to_cpu(cmd->opcode);
@@ -1111,6 +1335,30 @@  static int btintel_pcie_send_frame(struct hci_dev *hdev,
 		bt_dev_err(hdev, "Failed to send frame (%d)", ret);
 		goto exit_error;
 	}
+
+	if (type == BTINTEL_PCIE_HCI_CMD_PKT &&
+	    (opcode == HCI_OP_RESET || opcode == 0xfc01)) {
+		old_ctxt = data->alive_intr_ctxt;
+		data->alive_intr_ctxt =
+			(opcode == 0xfc01 ? BTINTEL_PCIE_INTEL_HCI_RESET1 :
+				BTINTEL_PCIE_HCI_RESET);
+		bt_dev_dbg(data->hdev, "sent cmd: 0x%4.4x alive context changed: %s  ->  %s",
+			   opcode, btintel_pcie_alivectxt_state2str(old_ctxt),
+			   btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
+		if (opcode == HCI_OP_RESET) {
+			data->gp0_received = false;
+			ret = wait_event_timeout(data->gp0_wait_q,
+						 data->gp0_received,
+						 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
+			if (!ret) {
+				hdev->stat.err_tx++;
+				bt_dev_err(hdev, "No alive interrupt received for %s",
+					   btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
+				ret = -ETIME;
+				goto exit_error;
+			}
+		}
+	}
 	hdev->stat.byte_tx += skb->len;
 	kfree_skb(skb);
 
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index baaff70420f5..8b7824ad005a 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -12,6 +12,7 @@ 
 #define BTINTEL_PCIE_CSR_HW_REV_REG		(BTINTEL_PCIE_CSR_BASE + 0x028)
 #define BTINTEL_PCIE_CSR_RF_ID_REG		(BTINTEL_PCIE_CSR_BASE + 0x09C)
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_REG		(BTINTEL_PCIE_CSR_BASE + 0x108)
+#define BTINTEL_PCIE_CSR_IPC_SLEEP_CTL_REG	(BTINTEL_PCIE_CSR_BASE + 0x114)
 #define BTINTEL_PCIE_CSR_CI_ADDR_LSB_REG	(BTINTEL_PCIE_CSR_BASE + 0x118)
 #define BTINTEL_PCIE_CSR_CI_ADDR_MSB_REG	(BTINTEL_PCIE_CSR_BASE + 0x11C)
 #define BTINTEL_PCIE_CSR_IMG_RESPONSE_REG	(BTINTEL_PCIE_CSR_BASE + 0x12C)
@@ -32,6 +33,7 @@ 
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_IML_LOCKDOWN	(BIT(11))
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_MAC_ACCESS_ON	(BIT(16))
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_ALIVE		(BIT(23))
+#define BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY	(BIT(24))
 
 /* Registers for MSI-X */
 #define BTINTEL_PCIE_CSR_MSIX_BASE		(0x2000)
@@ -55,6 +57,16 @@  enum msix_hw_int_causes {
 	BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0	= BIT(0),	/* cause 32 */
 };
 
+/* PCIe device states
+ * Host-Device interface is active
+ * Host-Device interface is inactive(as reflected by IPC_SLEEP_CONTROL_CSR_AD)
+ * Host-Device interface is inactive(as reflected by IPC_SLEEP_CONTROL_CSR_AD)
+ */
+enum {
+	BTINTEL_PCIE_STATE_D0 = 0,
+	BTINTEL_PCIE_STATE_D3_HOT = 2,
+	BTINTEL_PCIE_STATE_D3_COLD = 3,
+};
 #define BTINTEL_PCIE_MSIX_NON_AUTO_CLEAR_CAUSE	BIT(7)
 
 /* Minimum and Maximum number of MSI-X Vector
@@ -67,7 +79,7 @@  enum msix_hw_int_causes {
 #define BTINTEL_DEFAULT_MAC_ACCESS_TIMEOUT_US	200000
 
 /* Default interrupt timeout in msec */
-#define BTINTEL_DEFAULT_INTR_TIMEOUT	3000
+#define BTINTEL_DEFAULT_INTR_TIMEOUT_MS	3000
 
 /* The number of descriptors in TX/RX queues */
 #define BTINTEL_DESCS_COUNT	16
@@ -343,6 +355,7 @@  struct rxq {
  * @ia: Index Array struct
  * @txq: TX Queue struct
  * @rxq: RX Queue struct
+ * @alive_intr_ctxt: Alive interrupt context
  */
 struct btintel_pcie_data {
 	struct pci_dev	*pdev;
@@ -389,6 +402,7 @@  struct btintel_pcie_data {
 	struct ia	ia;
 	struct txq	txq;
 	struct rxq	rxq;
+	u32	alive_intr_ctxt;
 };
 
 static inline u32 btintel_pcie_rd_reg32(struct btintel_pcie_data *data,