All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces
@ 2025-02-18 12:26 Kiran K
  2025-02-18 12:26 ` [PATCH v1 2/4] Bluetooth: btintel_pcie: Read hardware exception data Kiran K
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Kiran K @ 2025-02-18 12:26 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: ravishankar.srivatsa, chethan.tumkur.narayan,
	chandrashekar.devegowda, vijay.satija, Kiran K

This patch allocates the host memory which is used by controller to dump
the firmware traces. The memory needs to be shared with controller via
context information.

Co-developed-by: Vijay Satija <vijay.satija@intel.com>
Signed-off-by: Vijay Satija <vijay.satija@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel_pcie.c | 89 ++++++++++++++++++++++++++++++++
 drivers/bluetooth/btintel_pcie.h | 32 ++++++++++++
 2 files changed, 121 insertions(+)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index b8b241a92bf9..11e2b805c7cc 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -49,6 +49,8 @@ MODULE_DEVICE_TABLE(pci, btintel_pcie_table);
 #define BTINTEL_PCIE_HCI_EVT_PKT	0x00000004
 #define BTINTEL_PCIE_HCI_ISO_PKT	0x00000005
 
+ #define BTINTEL_PCIE_MAGIC_NUM    0xA5A5A5A5
+
 /* Alive interrupt context */
 enum {
 	BTINTEL_PCIE_ROM,
@@ -60,6 +62,83 @@ enum {
 	BTINTEL_PCIE_D3
 };
 
+/* Structure for dbgc fragment buffer
+ * @buf_addr_lsb: LSB of the buffer's physical address
+ * @buf_addr_msb: MSB of the buffer's physical address
+ * @buf_size: Total size of the buffer
+ */
+struct btintel_pcie_dbgc_ctxt_buf {
+	u32	buf_addr_lsb;
+	u32	buf_addr_msb;
+	u32	buf_size;
+};
+
+/* Structure for dbgc fragment
+ * @magic_num: 0XA5A5A5A5
+ * @ver: For Driver-FW compatibility
+ * @total_size: Total size of the payload debug info
+ * @num_buf: Num of allocated debug bufs
+ * @bufs: All buffer's addresses and sizes
+ */
+struct btintel_pcie_dbgc_ctxt {
+	u32	magic_num;
+	u32     ver;
+	u32     total_size;
+	u32     num_buf;
+	struct btintel_pcie_dbgc_ctxt_buf bufs[BTINTEL_PCIE_DBGC_BUFFER_COUNT];
+};
+
+/* This function initializes the memory for DBGC buffers and formats the
+ * DBGC fragment which consists header info and DBGC buffer's LSB, MSB and
+ * size as the payload
+ */
+static int btintel_pcie_setup_dbgc(struct btintel_pcie_data *data)
+{
+	struct btintel_pcie_dbgc_ctxt db_frag;
+	struct data_buf *buf;
+	int i;
+
+	data->dbgc.count = BTINTEL_PCIE_DBGC_BUFFER_COUNT;
+	data->dbgc.bufs = devm_kcalloc(&data->pdev->dev, data->dbgc.count,
+				       sizeof(*buf), GFP_KERNEL);
+	if (!data->dbgc.bufs)
+		return -ENOMEM;
+
+	data->dbgc.buf_v_addr = dmam_alloc_coherent(&data->pdev->dev,
+						    data->dbgc.count *
+						    BTINTEL_PCIE_DBGC_BUFFER_SIZE,
+						    &data->dbgc.buf_p_addr,
+						    GFP_KERNEL | __GFP_NOWARN);
+	if (!data->dbgc.buf_v_addr)
+		return -ENOMEM;
+
+	data->dbgc.frag_v_addr = dmam_alloc_coherent(&data->pdev->dev,
+						     sizeof(struct btintel_pcie_dbgc_ctxt),
+						     &data->dbgc.frag_p_addr,
+						     GFP_KERNEL | __GFP_NOWARN);
+	if (!data->dbgc.frag_v_addr)
+		return -ENOMEM;
+
+	data->dbgc.frag_size = sizeof(struct btintel_pcie_dbgc_ctxt);
+
+	db_frag.magic_num = BTINTEL_PCIE_MAGIC_NUM;
+	db_frag.ver = BTINTEL_PCIE_DBGC_FRAG_VERSION;
+	db_frag.total_size = BTINTEL_PCIE_DBGC_FRAG_PAYLOAD_SIZE;
+	db_frag.num_buf = BTINTEL_PCIE_DBGC_FRAG_BUFFER_COUNT;
+
+	for (i = 0; i < data->dbgc.count; i++) {
+		buf = &data->dbgc.bufs[i];
+		buf->data_p_addr = data->dbgc.buf_p_addr + i * BTINTEL_PCIE_DBGC_BUFFER_SIZE;
+		buf->data = data->dbgc.buf_v_addr + i * BTINTEL_PCIE_DBGC_BUFFER_SIZE;
+		db_frag.bufs[i].buf_addr_lsb = (u32)(buf->data_p_addr & 0xffffffff);
+		db_frag.bufs[i].buf_addr_msb = (u32)((buf->data_p_addr >> 32) & 0xffffffff);
+		db_frag.bufs[i].buf_size = BTINTEL_PCIE_DBGC_BUFFER_SIZE;
+	}
+
+	memcpy(data->dbgc.frag_v_addr, &db_frag, sizeof(db_frag));
+	return 0;
+}
+
 static inline void ipc_print_ia_ring(struct hci_dev *hdev, struct ia *ia,
 				     u16 queue_num)
 {
@@ -1008,6 +1087,11 @@ static void btintel_pcie_init_ci(struct btintel_pcie_data *data,
 	ci->addr_urbdq1 = data->rxq.urbd1s_p_addr;
 	ci->num_urbdq1 = data->rxq.count;
 	ci->urbdq_db_vec = BTINTEL_PCIE_RXQ_NUM;
+
+	ci->dbg_output_mode = 0x01;
+	ci->dbgc_addr = data->dbgc.frag_p_addr;
+	ci->dbgc_size = data->dbgc.frag_size;
+	ci->dbg_preset = 0x00;
 }
 
 static void btintel_pcie_free_txq_bufs(struct btintel_pcie_data *data,
@@ -1220,6 +1304,11 @@ static int btintel_pcie_alloc(struct btintel_pcie_data *data)
 	/* Setup Index Array */
 	btintel_pcie_setup_ia(data, p_addr, v_addr, &data->ia);
 
+	/* Setup data buffers for dbgc */
+	err = btintel_pcie_setup_dbgc(data);
+	if (err)
+		goto exit_error_txq;
+
 	/* Setup Context Information */
 	p_addr += sizeof(u16) * BTINTEL_PCIE_NUM_QUEUES * 4;
 	v_addr += sizeof(u16) * BTINTEL_PCIE_NUM_QUEUES * 4;
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index f9aada0543c4..b9d32393002b 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -48,6 +48,21 @@
 #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))
 
+/* The DRAM buffer count, each buffer size, and
+ * fragment buffer size
+ */
+#define BTINTEL_PCIE_DBGC_BUFFER_COUNT		16
+#define BTINTEL_PCIE_DBGC_BUFFER_SIZE		(256 * 1024) /* 256 KB */
+
+#define BTINTEL_PCIE_DBGC_FRAG_VERSION		1
+#define BTINTEL_PCIE_DBGC_FRAG_BUFFER_COUNT	BTINTEL_PCIE_DBGC_BUFFER_COUNT
+
+/* Magic number(4), version(4), size of payload length(4) */
+#define BTINTEL_PCIE_DBGC_FRAG_HEADER_SIZE	12
+
+/* Num of alloc Dbg buff (4) + (LSB(4), MSB(4), Size(4)) for each buffer */
+#define BTINTEL_PCIE_DBGC_FRAG_PAYLOAD_SIZE	196
+
 /* Causes for the FH register interrupts */
 enum msix_fh_int_causes {
 	BTINTEL_PCIE_MSIX_FH_INT_CAUSES_0	= BIT(0),	/* cause 0 */
@@ -325,6 +340,22 @@ struct rxq {
 	struct data_buf	*bufs;
 };
 
+/* Structure for DRAM Buffer
+ * @count: Number of descriptors
+ * @buf: Array of data_buf structure
+ */
+struct btintel_pcie_dbgc {
+	u16		count;
+
+	void		*frag_v_addr;
+	dma_addr_t	frag_p_addr;
+	u16		frag_size;
+
+	dma_addr_t	buf_p_addr;
+	void		*buf_v_addr;
+	struct data_buf *bufs;
+};
+
 /* struct btintel_pcie_data
  * @pdev: pci device
  * @hdev: hdev device
@@ -405,6 +436,7 @@ struct btintel_pcie_data {
 	struct txq	txq;
 	struct rxq	rxq;
 	u32	alive_intr_ctxt;
+	struct btintel_pcie_dbgc	dbgc;
 };
 
 static inline u32 btintel_pcie_rd_reg32(struct btintel_pcie_data *data,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1 2/4] Bluetooth: btintel_pcie: Read hardware exception data
  2025-02-18 12:26 [PATCH v1 1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces Kiran K
@ 2025-02-18 12:26 ` Kiran K
  2025-02-18 17:28   ` Luiz Augusto von Dentz
  2025-02-18 12:26 ` [PATCH v1 3/4] Bluetooth: btintel_pcie: Add support for device coredump Kiran K
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Kiran K @ 2025-02-18 12:26 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: ravishankar.srivatsa, chethan.tumkur.narayan,
	chandrashekar.devegowda, vijay.satija, Kiran K

On hardware error, controller writes hardware error event and optional
vendor specific hci events in device memory in TLV format and raises
MSIX interrupt. Driver reads the device memory and passes the events to
the stack for further processing.

Co-developed-by: Vijay Satija <vijay.satija@intel.com>
Signed-off-by: Vijay Satija <vijay.satija@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel.h      |   1 +
 drivers/bluetooth/btintel_pcie.c | 212 ++++++++++++++++++++++++++++++-
 drivers/bluetooth/btintel_pcie.h |  23 ++++
 3 files changed, 235 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 19530ea14905..4c21e69887a3 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -190,6 +190,7 @@ enum {
 struct btintel_data {
 	DECLARE_BITMAP(flags, __INTEL_NUM_FLAGS);
 	int (*acpi_reset_method)(struct hci_dev *hdev);
+	u32	cnvi_top;
 };
 
 #define btintel_set_flag(hdev, nr)					\
diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 11e2b805c7cc..6c78472f9dad 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -51,6 +51,14 @@ MODULE_DEVICE_TABLE(pci, btintel_pcie_table);
 
  #define BTINTEL_PCIE_MAGIC_NUM    0xA5A5A5A5
 
+#define BTINTEL_PCIE_BLZR_HWEXP_SIZE		1024
+#define BTINTEL_PCIE_BLZR_HWEXP_DMP_ADDR	0xB00A7C00
+
+#define BTINTEL_PCIE_SCP_HWEXP_SIZE		4096
+#define BTINTEL_PCIE_SCP_HWEXP_DMP_ADDR		0xB030F800
+
+#define BTINTEL_PCIE_MAGIC_NUM	0xA5A5A5A5
+
 /* Alive interrupt context */
 enum {
 	BTINTEL_PCIE_ROM,
@@ -353,6 +361,63 @@ static int btintel_pcie_reset_bt(struct btintel_pcie_data *data)
 	return reg == 0 ? 0 : -ENODEV;
 }
 
+static void btintel_pcie_mac_init(struct btintel_pcie_data *data)
+{
+	u32 reg;
+
+	/* 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 |
+			BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_DISCON |
+			BTINTEL_PCIE_CSR_FUNC_CTRL_SW_RESET);
+	reg |= (BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_ENA |
+			BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_INIT);
+	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
+}
+
+static int btintel_pcie_get_mac_access(struct btintel_pcie_data *data)
+{
+	u32 reg;
+	int retry = 15;
+
+	reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
+
+	reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
+	reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
+	if ((reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS) == 0)
+		reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
+
+	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
+
+	do {
+		reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
+		if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS)
+			return 0;
+		usleep_range(1000, 1200);
+
+	} while (--retry > 0);
+
+	return -ETIME;
+}
+
+static void btintel_pcie_release_mac_access(struct btintel_pcie_data *data)
+{
+	u32 reg;
+
+	reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
+
+	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ)
+		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
+
+	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS)
+		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
+
+	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ)
+		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
+
+	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
+}
+
 /* 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.
@@ -473,6 +538,133 @@ static inline char *btintel_pcie_alivectxt_state2str(u32 alive_intr_ctxt)
 	}
 }
 
+static int btintel_pcie_read_device_mem(struct btintel_pcie_data *data,
+					void *buf, u32 dev_addr, int len)
+{
+	int err;
+	u32 *val = buf;
+
+	/* Get device mac access */
+	err = btintel_pcie_get_mac_access(data);
+	if (err) {
+		bt_dev_err(data->hdev, "Failed to get mac access %d", err);
+		return err;
+	}
+
+	for (; len > 0; len -= 4, dev_addr += 4, val++)
+		*val = btintel_pcie_rd_dev_mem(data, dev_addr);
+
+	btintel_pcie_release_mac_access(data);
+
+	return 0;
+}
+
+static void btintel_pcie_dump_hwexp(struct btintel_pcie_data *data)
+{
+	struct btintel_data *intel_data = hci_get_priv(data->hdev);
+	int len, err, offset, pending;
+	struct sk_buff *skb;
+	u32 addr, val;
+	u8 *buf;
+
+	struct tlv {
+		u8	type;
+		u16	len;
+		u8	val[];
+	} __packed;
+
+	struct tlv *tlv;
+
+	switch (intel_data->cnvi_top & 0xfff) {
+	case BTINTEL_CNVI_BLAZARI:
+	case BTINTEL_CNVI_BLAZARIW:
+		/* only from step B0 onwards */
+		if (INTEL_CNVX_TOP_STEP(intel_data->cnvi_top) != 0x01)
+			return;
+		len = BTINTEL_PCIE_BLZR_HWEXP_SIZE; /* exception data length */
+		addr = BTINTEL_PCIE_BLZR_HWEXP_DMP_ADDR;
+	break;
+	case BTINTEL_CNVI_SCP:
+		len = BTINTEL_PCIE_SCP_HWEXP_SIZE;
+		addr = BTINTEL_PCIE_SCP_HWEXP_DMP_ADDR;
+	break;
+	default:
+		bt_dev_err(data->hdev, "Unsupported cnvi 0x%8x", intel_data->cnvi_top);
+		return;
+	}
+
+	buf = kzalloc(len, GFP_KERNEL);
+	if (!buf)
+		goto exit_on_error;
+
+	btintel_pcie_mac_init(data);
+
+	err = btintel_pcie_read_device_mem(data, buf, addr, len);
+	if (err)
+		goto exit_on_error;
+
+	val = get_unaligned_le32(buf);
+	if (val != BTINTEL_PCIE_MAGIC_NUM) {
+		bt_dev_err(data->hdev, "Invalid exception dump signature: 0x%8.8x",
+			   val);
+		goto exit_on_error;
+	}
+
+	offset = 4;
+	do {
+		pending = len - offset;
+		if (pending < sizeof(*tlv))
+			break;
+		tlv = (struct tlv *)(buf + offset);
+		if (!tlv->type) {
+			bt_dev_dbg(data->hdev, "Invalid TLV type 0");
+			break;
+		}
+		tlv->len = le16_to_cpu((__force __le16)tlv->len);
+		offset += sizeof(*tlv);
+		pending = len - offset;
+		if (tlv->len > pending)
+			break;
+
+		offset += tlv->len;
+		 /* Only TLV of type = 1 are vendor HCI events */
+		if (tlv->type != 1)
+			continue;
+
+		bt_dev_dbg(data->hdev, "Exception pkt len: %u", tlv->len);
+		if (tlv->len > HCI_MAX_EVENT_SIZE)
+			break;
+		skb = bt_skb_alloc(tlv->len, GFP_KERNEL);
+		if (!skb)
+			goto exit_on_error;
+		hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
+		skb_put_data(skb, tlv->val, tlv->len);
+
+		/* copy Intel specific pcie packet type */
+		val = BTINTEL_PCIE_HCI_EVT_PKT;
+		memcpy(skb_push(skb, BTINTEL_PCIE_HCI_TYPE_LEN), &val,
+		       BTINTEL_PCIE_HCI_TYPE_LEN);
+
+		print_hex_dump(KERN_DEBUG, "Bluetooth: ", DUMP_PREFIX_OFFSET, 16,
+			       1, tlv->val, tlv->len, false);
+
+		skb_queue_tail(&data->rx_skb_q, skb);
+		queue_work(data->workqueue, &data->rx_work);
+	} while (offset < len);
+
+exit_on_error:
+	kfree(buf);
+}
+
+static void btintel_pcie_hwexp_work(struct work_struct *work)
+{
+	struct btintel_pcie_data *data = container_of(work,
+					struct btintel_pcie_data, hwexp_work);
+	btintel_pcie_dump_hwexp(data);
+
+	clear_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags);
+}
+
 /* 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.
  */
@@ -794,6 +986,14 @@ static int btintel_pcie_recv_frame(struct btintel_pcie_data *data,
 	return ret;
 }
 
+static void btintel_pcie_msix_hw_exp_handler(struct btintel_pcie_data *data)
+{
+	bt_dev_err(data->hdev, "Received hw exception interrupt");
+	if (test_and_set_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags))
+		return;
+	queue_work(data->workqueue, &data->hwexp_work);
+}
+
 static void btintel_pcie_rx_work(struct work_struct *work)
 {
 	struct btintel_pcie_data *data = container_of(work,
@@ -920,6 +1120,10 @@ static irqreturn_t btintel_pcie_irq_msix_handler(int irq, void *dev_id)
 		return IRQ_NONE;
 	}
 
+	/* This interrupt is raised when there is an hardware exception */
+	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_HWEXP)
+		btintel_pcie_msix_hw_exp_handler(data);
+
 	/* This interrupt is triggered by the firmware after updating
 	 * boot_stage register and image_response register
 	 */
@@ -1000,7 +1204,8 @@ struct btintel_pcie_causes_list {
 static struct btintel_pcie_causes_list causes_list[] = {
 	{ BTINTEL_PCIE_MSIX_FH_INT_CAUSES_0,	BTINTEL_PCIE_CSR_MSIX_FH_INT_MASK,	0x00 },
 	{ BTINTEL_PCIE_MSIX_FH_INT_CAUSES_1,	BTINTEL_PCIE_CSR_MSIX_FH_INT_MASK,	0x01 },
-	{ BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0, BTINTEL_PCIE_CSR_MSIX_HW_INT_MASK,	0x20 },
+	{ BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0,  BTINTEL_PCIE_CSR_MSIX_HW_INT_MASK,	0x20 },
+	{ BTINTEL_PCIE_MSIX_HW_INT_CAUSES_HWEXP, BTINTEL_PCIE_CSR_MSIX_HW_INT_MASK,	0x23 },
 };
 
 /* This function configures the interrupt masks for both HW_INT_CAUSES and
@@ -1482,6 +1687,7 @@ static void btintel_pcie_release_hdev(struct btintel_pcie_data *data)
 
 static int btintel_pcie_setup_internal(struct hci_dev *hdev)
 {
+	struct btintel_data *data = hci_get_priv(hdev);
 	const u8 param[1] = { 0xFF };
 	struct intel_version_tlv ver_tlv;
 	struct sk_buff *skb;
@@ -1520,6 +1726,7 @@ static int btintel_pcie_setup_internal(struct hci_dev *hdev)
 		goto exit_error;
 	}
 
+	data->cnvi_top = ver_tlv.cnvi_top;
 	switch (INTEL_HW_PLATFORM(ver_tlv.cnvi_bt)) {
 	case 0x37:
 		break;
@@ -1667,6 +1874,8 @@ static int btintel_pcie_probe(struct pci_dev *pdev,
 	skb_queue_head_init(&data->rx_skb_q);
 	INIT_WORK(&data->rx_work, btintel_pcie_rx_work);
 
+	INIT_WORK(&data->hwexp_work, btintel_pcie_hwexp_work);
+
 	data->boot_stage_cache = 0x00;
 	data->img_resp_cache = 0x00;
 
@@ -1731,6 +1940,7 @@ static void btintel_pcie_remove(struct pci_dev *pdev)
 	btintel_pcie_release_hdev(data);
 
 	flush_work(&data->rx_work);
+	flush_work(&data->hwexp_work);
 
 	destroy_workqueue(data->workqueue);
 
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index b9d32393002b..98902fd4fc96 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -16,6 +16,8 @@
 #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)
+#define BTINTEL_PCIE_PRPH_DEV_ADDR_REG		(BTINTEL_PCIE_CSR_BASE + 0x440)
+#define BTINTEL_PCIE_PRPH_DEV_RD_REG		(BTINTEL_PCIE_CSR_BASE + 0x458)
 #define BTINTEL_PCIE_CSR_HBUS_TARG_WRPTR	(BTINTEL_PCIE_CSR_BASE + 0x460)
 
 /* BTINTEL_PCIE_CSR Function Control Register */
@@ -23,6 +25,12 @@
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_INIT		(BIT(6))
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_INIT		(BIT(7))
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS	(BIT(20))
+
+#define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ	(BIT(21))
+/* Stop MAC Access disconnection request */
+#define BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS	(BIT(22))
+#define BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ		(BIT(23))
+
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_STS	(BIT(28))
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_DISCON	(BIT(29))
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_SW_RESET		(BIT(31))
@@ -72,6 +80,7 @@ enum msix_fh_int_causes {
 /* Causes for the HW register interrupts */
 enum msix_hw_int_causes {
 	BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0	= BIT(0),	/* cause 32 */
+	BTINTEL_PCIE_MSIX_HW_INT_CAUSES_HWEXP	= BIT(3),	/* cause 35 */
 };
 
 /* PCIe device states
@@ -84,6 +93,11 @@ enum {
 	BTINTEL_PCIE_STATE_D3_HOT = 2,
 	BTINTEL_PCIE_STATE_D3_COLD = 3,
 };
+
+enum {
+	BTINTEL_PCIE_HWEXP_INPROGRESS,
+};
+
 #define BTINTEL_PCIE_MSIX_NON_AUTO_CLEAR_CAUSE	BIT(7)
 
 /* Minimum and Maximum number of MSI-X Vector
@@ -437,6 +451,7 @@ struct btintel_pcie_data {
 	struct rxq	rxq;
 	u32	alive_intr_ctxt;
 	struct btintel_pcie_dbgc	dbgc;
+	struct work_struct	hwexp_work;
 };
 
 static inline u32 btintel_pcie_rd_reg32(struct btintel_pcie_data *data,
@@ -476,3 +491,11 @@ static inline void btintel_pcie_clr_reg_bits(struct btintel_pcie_data *data,
 	r &= ~bits;
 	iowrite32(r, data->base_addr + offset);
 }
+
+static inline u32 btintel_pcie_rd_dev_mem(struct btintel_pcie_data *data,
+					  u32 addr)
+{
+	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_PRPH_DEV_ADDR_REG, addr);
+	return btintel_pcie_rd_reg32(data, BTINTEL_PCIE_PRPH_DEV_RD_REG);
+}
+
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1 3/4] Bluetooth: btintel_pcie: Add support for device coredump
  2025-02-18 12:26 [PATCH v1 1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces Kiran K
  2025-02-18 12:26 ` [PATCH v1 2/4] Bluetooth: btintel_pcie: Read hardware exception data Kiran K
@ 2025-02-18 12:26 ` Kiran K
  2025-02-18 12:26 ` [PATCH v1 4/4] Bluetooth: btintel_pcie: Trigger device coredump on exception Kiran K
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Kiran K @ 2025-02-18 12:26 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: ravishankar.srivatsa, chethan.tumkur.narayan,
	chandrashekar.devegowda, vijay.satija, Kiran K

1. Driver registers device coredump callback
2. Dumps firmware traces as part of coredump

Co-developed-by: Vijay Satija <vijay.satija@intel.com>
Signed-off-by: Vijay Satija <vijay.satija@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel.h      |   1 -
 drivers/bluetooth/btintel_pcie.c | 272 ++++++++++++++++++++++++++++++-
 drivers/bluetooth/btintel_pcie.h |  39 +++++
 3 files changed, 305 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 4c21e69887a3..19530ea14905 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -190,7 +190,6 @@ enum {
 struct btintel_data {
 	DECLARE_BITMAP(flags, __INTEL_NUM_FLAGS);
 	int (*acpi_reset_method)(struct hci_dev *hdev);
-	u32	cnvi_top;
 };
 
 #define btintel_set_flag(hdev, nr)					\
diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 6c78472f9dad..0f04603105a3 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -59,6 +59,8 @@ MODULE_DEVICE_TABLE(pci, btintel_pcie_table);
 
 #define BTINTEL_PCIE_MAGIC_NUM	0xA5A5A5A5
 
+#define BTINTEL_PCIE_TRIGGER_REASON_USER_TRIGGER	0x17A2
+
 /* Alive interrupt context */
 enum {
 	BTINTEL_PCIE_ROM,
@@ -375,6 +377,25 @@ static void btintel_pcie_mac_init(struct btintel_pcie_data *data)
 	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
 }
 
+static int btintel_pcie_add_dmp_data(struct hci_dev *hdev, const void *data, int size)
+{
+	struct sk_buff *skb;
+	int err;
+
+	skb = alloc_skb(size, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	skb_put_data(skb, data, size);
+	err = hci_devcd_append(hdev, skb);
+	if (err) {
+		bt_dev_err(hdev, "Failed to append data in the coredump");
+		return err;
+	}
+
+	return 0;
+}
+
 static int btintel_pcie_get_mac_access(struct btintel_pcie_data *data)
 {
 	u32 reg;
@@ -418,6 +439,194 @@ static void btintel_pcie_release_mac_access(struct btintel_pcie_data *data)
 	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
 }
 
+static void btintel_pcie_copy_tlv(struct sk_buff *skb, enum btintel_pcie_tlv_type type,
+				  void *data, int size)
+{
+	struct intel_tlv *tlv;
+
+	tlv = skb_put(skb, sizeof(*tlv) + size);
+	tlv->type = type;
+	tlv->len = size;
+	memcpy(tlv->val, data, tlv->len);
+}
+
+static int btintel_pcie_read_dram_buffers(struct btintel_pcie_data *data)
+{
+	u32 offset, prev_size, wr_ptr_status, dump_size, i;
+	struct btintel_pcie_dbgc *dbgc = &data->dbgc;
+	u8 buf_idx, dump_time_len, fw_build;
+	struct hci_dev *hdev = data->hdev;
+	struct intel_tlv *tlv;
+	struct timespec64 now;
+	struct sk_buff *skb;
+	struct tm tm_now;
+	char buf[100];
+	u16 hdr_len;
+	int ret;
+
+	wr_ptr_status = btintel_pcie_rd_dev_mem(data, BTINTEL_PCIE_DBGC_CUR_DBGBUFF_STATUS);
+	offset = wr_ptr_status & BTINTEL_PCIE_DBG_OFFSET_BIT_MASK;
+
+	buf_idx = BTINTEL_PCIE_DBGC_DBG_BUF_IDX(wr_ptr_status);
+	if (buf_idx > dbgc->count) {
+		bt_dev_warn(hdev, "Buffer index is invalid");
+		return -EINVAL;
+	}
+
+	prev_size = buf_idx * BTINTEL_PCIE_DBGC_BUFFER_SIZE;
+	if (prev_size + offset >= prev_size)
+		data->dmp_hdr.write_ptr = prev_size + offset;
+	else
+		return -EINVAL;
+
+	ktime_get_real_ts64(&now);
+	time64_to_tm(now.tv_sec, 0, &tm_now);
+	dump_time_len = snprintf(buf, sizeof(buf), "Dump Time: %02d-%02d-%04ld %02d:%02d:%02d",
+				 tm_now.tm_mday, tm_now.tm_mon + 1, tm_now.tm_year + 1900,
+				 tm_now.tm_hour, tm_now.tm_min, tm_now.tm_sec);
+
+	fw_build = snprintf(buf + dump_time_len, sizeof(buf),
+			    "Firmware Timestamp: Year %u WW %02u buildtype %u build %u",
+			    2000 + (data->dmp_hdr.fw_timestamp >> 8),
+			    data->dmp_hdr.fw_timestamp & 0xff, data->dmp_hdr.fw_build_type,
+			    data->dmp_hdr.fw_build_num);
+
+	hdr_len = sizeof(*tlv) + sizeof(data->dmp_hdr.cnvi_bt) +
+		  sizeof(*tlv) + sizeof(data->dmp_hdr.write_ptr) +
+		  sizeof(*tlv) + sizeof(data->dmp_hdr.wrap_ctr) +
+		  sizeof(*tlv) + sizeof(data->dmp_hdr.trigger_reason) +
+		  sizeof(*tlv) + sizeof(data->dmp_hdr.fw_git_sha1) +
+		  sizeof(*tlv) + sizeof(data->dmp_hdr.cnvr_top) +
+		  sizeof(*tlv) + sizeof(data->dmp_hdr.cnvi_top) +
+		  sizeof(*tlv) + dump_time_len +
+		  sizeof(*tlv) + fw_build;
+
+	dump_size = hdr_len + sizeof(hdr_len);
+
+	skb = alloc_skb(dump_size, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	/* Add debug buffers data length to dump size */
+	dump_size += BTINTEL_PCIE_DBGC_BUFFER_SIZE * dbgc->count;
+
+	ret = hci_devcd_init(hdev, dump_size);
+	if (ret) {
+		bt_dev_err(hdev, "Failed to init devcoredump, err %d", ret);
+		kfree_skb(skb);
+		return ret;
+	}
+
+	skb_put_data(skb, &hdr_len, sizeof(hdr_len));
+
+	btintel_pcie_copy_tlv(skb, BTINTEL_CNVI_BT, &data->dmp_hdr.cnvi_bt,
+			      sizeof(data->dmp_hdr.cnvi_bt));
+
+	btintel_pcie_copy_tlv(skb, BTINTEL_WRITE_PTR, &data->dmp_hdr.write_ptr,
+			      sizeof(data->dmp_hdr.write_ptr));
+
+	data->dmp_hdr.wrap_ctr = btintel_pcie_rd_dev_mem(data,
+							 BTINTEL_PCIE_DBGC_DBGBUFF_WRAP_ARND);
+
+	btintel_pcie_copy_tlv(skb, BTINTEL_WRAP_CTR, &data->dmp_hdr.wrap_ctr,
+			      sizeof(data->dmp_hdr.wrap_ctr));
+
+	btintel_pcie_copy_tlv(skb, BTINTEL_TRIGGER_REASON, &data->dmp_hdr.trigger_reason,
+			      sizeof(data->dmp_hdr.trigger_reason));
+
+	btintel_pcie_copy_tlv(skb, BTINTEL_FW_SHA, &data->dmp_hdr.fw_git_sha1,
+			      sizeof(data->dmp_hdr.fw_git_sha1));
+
+	btintel_pcie_copy_tlv(skb, BTINTEL_CNVR_TOP, &data->dmp_hdr.cnvr_top,
+			      sizeof(data->dmp_hdr.cnvr_top));
+
+	btintel_pcie_copy_tlv(skb, BTINTEL_CNVI_TOP, &data->dmp_hdr.cnvi_top,
+			      sizeof(data->dmp_hdr.cnvi_top));
+
+	btintel_pcie_copy_tlv(skb, BTINTEL_DUMP_TIME, buf, dump_time_len);
+
+	btintel_pcie_copy_tlv(skb, BTINTEL_FW_BUILD, buf + dump_time_len, fw_build);
+
+	ret = hci_devcd_append(hdev, skb);
+	if (ret)
+		goto exit_err;
+
+	for (i = 0; i < dbgc->count; i++) {
+		ret = btintel_pcie_add_dmp_data(hdev, dbgc->bufs[i].data,
+						BTINTEL_PCIE_DBGC_BUFFER_SIZE);
+		if (ret)
+			break;
+	}
+
+exit_err:
+	hci_devcd_complete(hdev);
+	return ret;
+}
+
+static void btintel_pcie_dump_traces(struct hci_dev *hdev)
+{
+	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
+	int ret = 0;
+
+	ret = btintel_pcie_get_mac_access(data);
+	if (ret) {
+		bt_dev_err(hdev, "Failed to get mac access: (%d)", ret);
+		return;
+	}
+
+	ret = btintel_pcie_read_dram_buffers(data);
+
+	btintel_pcie_release_mac_access(data);
+
+	if (ret)
+		bt_dev_err(hdev, "Failed to dump traces: (%d)", ret);
+}
+
+static void btintel_pcie_dump_hdr(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
+	u16 len = skb->len;
+	u16 *hdrlen_ptr;
+	char buf[80];
+
+	hdrlen_ptr = skb_put_zero(skb, sizeof(len));
+
+	snprintf(buf, sizeof(buf), "Controller Name: 0x%X\n",
+		 INTEL_HW_VARIANT(data->dmp_hdr.cnvi_bt));
+	skb_put_data(skb, buf, strlen(buf));
+
+	snprintf(buf, sizeof(buf), "Firmware Build Number: %u\n",
+		 data->dmp_hdr.fw_build_num);
+	skb_put_data(skb, buf, strlen(buf));
+
+	snprintf(buf, sizeof(buf), "Driver: %s\n", data->dmp_hdr.driver_name);
+	skb_put_data(skb, buf, strlen(buf));
+
+	snprintf(buf, sizeof(buf), "Vendor: Intel\n");
+	skb_put_data(skb, buf, strlen(buf));
+
+	*hdrlen_ptr = skb->len - len;
+}
+
+static void btintel_pcie_dump_notify(struct hci_dev *hdev, int state)
+{
+	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
+
+	switch (state) {
+	case HCI_DEVCOREDUMP_IDLE:
+		data->dmp_hdr.state = HCI_DEVCOREDUMP_IDLE;
+		break;
+	case HCI_DEVCOREDUMP_ACTIVE:
+		data->dmp_hdr.state = HCI_DEVCOREDUMP_ACTIVE;
+		break;
+	case HCI_DEVCOREDUMP_TIMEOUT:
+	case HCI_DEVCOREDUMP_ABORT:
+	case HCI_DEVCOREDUMP_DONE:
+		data->dmp_hdr.state = HCI_DEVCOREDUMP_IDLE;
+		break;
+	}
+}
+
 /* 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.
@@ -561,7 +770,6 @@ static int btintel_pcie_read_device_mem(struct btintel_pcie_data *data,
 
 static void btintel_pcie_dump_hwexp(struct btintel_pcie_data *data)
 {
-	struct btintel_data *intel_data = hci_get_priv(data->hdev);
 	int len, err, offset, pending;
 	struct sk_buff *skb;
 	u32 addr, val;
@@ -575,11 +783,11 @@ static void btintel_pcie_dump_hwexp(struct btintel_pcie_data *data)
 
 	struct tlv *tlv;
 
-	switch (intel_data->cnvi_top & 0xfff) {
+	switch (data->dmp_hdr.cnvi_top & 0xfff) {
 	case BTINTEL_CNVI_BLAZARI:
 	case BTINTEL_CNVI_BLAZARIW:
 		/* only from step B0 onwards */
-		if (INTEL_CNVX_TOP_STEP(intel_data->cnvi_top) != 0x01)
+		if (INTEL_CNVX_TOP_STEP(data->dmp_hdr.cnvi_top) != 0x01)
 			return;
 		len = BTINTEL_PCIE_BLZR_HWEXP_SIZE; /* exception data length */
 		addr = BTINTEL_PCIE_BLZR_HWEXP_DMP_ADDR;
@@ -589,7 +797,7 @@ static void btintel_pcie_dump_hwexp(struct btintel_pcie_data *data)
 		addr = BTINTEL_PCIE_SCP_HWEXP_DMP_ADDR;
 	break;
 	default:
-		bt_dev_err(data->hdev, "Unsupported cnvi 0x%8x", intel_data->cnvi_top);
+		bt_dev_err(data->hdev, "Unsupported cnvi 0x%8.8x", data->dmp_hdr.cnvi_top);
 		return;
 	}
 
@@ -665,6 +873,17 @@ static void btintel_pcie_hwexp_work(struct work_struct *work)
 	clear_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags);
 }
 
+static void btintel_pcie_coredump_work(struct work_struct *work)
+{
+	struct btintel_pcie_data *data = container_of(work,
+					struct btintel_pcie_data, coredump_work);
+	struct hci_dev *hdev = data->hdev;
+
+	btintel_pcie_dump_traces(hdev);
+
+	clear_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags);
+}
+
 /* 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.
  */
@@ -986,6 +1205,15 @@ static int btintel_pcie_recv_frame(struct btintel_pcie_data *data,
 	return ret;
 }
 
+static void btintel_pcie_do_firmware_dump(struct btintel_pcie_data *data, u16 trigger_reason)
+{
+	if (test_and_set_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags))
+		return;
+
+	data->dmp_hdr.trigger_reason  = trigger_reason;
+	queue_work(data->workqueue, &data->coredump_work);
+}
+
 static void btintel_pcie_msix_hw_exp_handler(struct btintel_pcie_data *data)
 {
 	bt_dev_err(data->hdev, "Received hw exception interrupt");
@@ -1687,7 +1915,7 @@ static void btintel_pcie_release_hdev(struct btintel_pcie_data *data)
 
 static int btintel_pcie_setup_internal(struct hci_dev *hdev)
 {
-	struct btintel_data *data = hci_get_priv(hdev);
+	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
 	const u8 param[1] = { 0xFF };
 	struct intel_version_tlv ver_tlv;
 	struct sk_buff *skb;
@@ -1726,7 +1954,6 @@ static int btintel_pcie_setup_internal(struct hci_dev *hdev)
 		goto exit_error;
 	}
 
-	data->cnvi_top = ver_tlv.cnvi_top;
 	switch (INTEL_HW_PLATFORM(ver_tlv.cnvi_bt)) {
 	case 0x37:
 		break;
@@ -1772,6 +1999,23 @@ static int btintel_pcie_setup_internal(struct hci_dev *hdev)
 		break;
 	}
 
+	data->dmp_hdr.cnvi_top = ver_tlv.cnvi_top;
+	data->dmp_hdr.cnvr_top = ver_tlv.cnvr_top;
+	data->dmp_hdr.fw_timestamp = ver_tlv.timestamp;
+	data->dmp_hdr.fw_build_type = ver_tlv.build_type;
+	data->dmp_hdr.fw_build_num = ver_tlv.build_num;
+	data->dmp_hdr.cnvi_bt = ver_tlv.cnvi_bt;
+
+	if (ver_tlv.img_type == 0x02 || ver_tlv.img_type == 0x03)
+		data->dmp_hdr.fw_git_sha1 = ver_tlv.git_sha1;
+
+	err = hci_devcd_register(hdev, btintel_pcie_dump_traces, btintel_pcie_dump_hdr,
+				 btintel_pcie_dump_notify);
+	if (err) {
+		bt_dev_err(hdev, "Failed to register coredump (%d)", err);
+		goto exit_error;
+	}
+
 	btintel_print_fseq_info(hdev);
 exit_error:
 	kfree_skb(skb);
@@ -1836,6 +2080,7 @@ static int btintel_pcie_setup_hdev(struct btintel_pcie_data *data)
 		goto exit_error;
 	}
 
+	data->dmp_hdr.driver_name = KBUILD_MODNAME;
 	return 0;
 
 exit_error:
@@ -1875,6 +2120,7 @@ static int btintel_pcie_probe(struct pci_dev *pdev,
 	INIT_WORK(&data->rx_work, btintel_pcie_rx_work);
 
 	INIT_WORK(&data->hwexp_work, btintel_pcie_hwexp_work);
+	INIT_WORK(&data->coredump_work, btintel_pcie_coredump_work);
 
 	data->boot_stage_cache = 0x00;
 	data->img_resp_cache = 0x00;
@@ -1941,6 +2187,7 @@ static void btintel_pcie_remove(struct pci_dev *pdev)
 
 	flush_work(&data->rx_work);
 	flush_work(&data->hwexp_work);
+	flush_work(&data->coredump_work);
 
 	destroy_workqueue(data->workqueue);
 
@@ -1951,11 +2198,24 @@ static void btintel_pcie_remove(struct pci_dev *pdev)
 	pci_set_drvdata(pdev, NULL);
 }
 
+#ifdef CONFIG_DEV_COREDUMP
+static void btintel_pcie_coredump(struct device *dev)
+{
+	struct  pci_dev *pdev = to_pci_dev(dev);
+	struct btintel_pcie_data *data = pci_get_drvdata(pdev);
+
+	btintel_pcie_do_firmware_dump(data, BTINTEL_PCIE_TRIGGER_REASON_USER_TRIGGER);
+}
+#endif
+
 static struct pci_driver btintel_pcie_driver = {
 	.name = KBUILD_MODNAME,
 	.id_table = btintel_pcie_table,
 	.probe = btintel_pcie_probe,
 	.remove = btintel_pcie_remove,
+#ifdef CONFIG_DEV_COREDUMP
+	.driver.coredump = btintel_pcie_coredump
+#endif
 };
 module_pci_driver(btintel_pcie_driver);
 
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index 98902fd4fc96..c13f17100a74 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -56,6 +56,15 @@
 #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))
 
+/* IOSF Debug Register */
+#define BTINTEL_PCIE_DBGC_BASE_ADDR			(0xf3800300)
+#define BTINTEL_PCIE_DBGC_CUR_DBGBUFF_STATUS		(BTINTEL_PCIE_DBGC_BASE_ADDR + 0x1C)
+#define BTINTEL_PCIE_DBGC_DBGBUFF_WRAP_ARND		(BTINTEL_PCIE_DBGC_BASE_ADDR + 0x2C)
+
+#define BTINTEL_PCIE_DBG_IDX_BIT_MASK		0x0F
+#define BTINTEL_PCIE_DBGC_DBG_BUF_IDX(data)	(((data) >> 24) & BTINTEL_PCIE_DBG_IDX_BIT_MASK)
+#define BTINTEL_PCIE_DBG_OFFSET_BIT_MASK	0xFFFFFF
+
 /* The DRAM buffer count, each buffer size, and
  * fragment buffer size
  */
@@ -96,6 +105,19 @@ enum {
 
 enum {
 	BTINTEL_PCIE_HWEXP_INPROGRESS,
+	BTINTEL_PCIE_COREDUMP_INPROGRESS
+};
+
+enum btintel_pcie_tlv_type {
+	BTINTEL_CNVI_BT,
+	BTINTEL_WRITE_PTR,
+	BTINTEL_WRAP_CTR,
+	BTINTEL_TRIGGER_REASON,
+	BTINTEL_FW_SHA,
+	BTINTEL_CNVR_TOP,
+	BTINTEL_CNVI_TOP,
+	BTINTEL_DUMP_TIME,
+	BTINTEL_FW_BUILD,
 };
 
 #define BTINTEL_PCIE_MSIX_NON_AUTO_CLEAR_CAUSE	BIT(7)
@@ -370,6 +392,21 @@ struct btintel_pcie_dbgc {
 	struct data_buf *bufs;
 };
 
+struct btintel_pcie_dump_header {
+	const char	*driver_name;
+	u32		cnvi_top;
+	u32		cnvr_top;
+	u16		fw_timestamp;
+	u8		fw_build_type;
+	u32		fw_build_num;
+	u32		fw_git_sha1;
+	u32		cnvi_bt;
+	u32		write_ptr;
+	u32		wrap_ctr;
+	u16		trigger_reason;
+	int		state;
+};
+
 /* struct btintel_pcie_data
  * @pdev: pci device
  * @hdev: hdev device
@@ -452,6 +489,8 @@ struct btintel_pcie_data {
 	u32	alive_intr_ctxt;
 	struct btintel_pcie_dbgc	dbgc;
 	struct work_struct	hwexp_work;
+	struct work_struct	coredump_work;
+	struct btintel_pcie_dump_header dmp_hdr;
 };
 
 static inline u32 btintel_pcie_rd_reg32(struct btintel_pcie_data *data,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1 4/4] Bluetooth: btintel_pcie: Trigger device coredump on exception
  2025-02-18 12:26 [PATCH v1 1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces Kiran K
  2025-02-18 12:26 ` [PATCH v1 2/4] Bluetooth: btintel_pcie: Read hardware exception data Kiran K
  2025-02-18 12:26 ` [PATCH v1 3/4] Bluetooth: btintel_pcie: Add support for device coredump Kiran K
@ 2025-02-18 12:26 ` Kiran K
  2025-02-18 13:17 ` [v1,1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces bluez.test.bot
  2025-02-18 17:54 ` [PATCH v1 1/4] " kernel test robot
  4 siblings, 0 replies; 8+ messages in thread
From: Kiran K @ 2025-02-18 12:26 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: ravishankar.srivatsa, chethan.tumkur.narayan,
	chandrashekar.devegowda, vijay.satija, Kiran K

Driver dumps device core dump on firmware exception.

Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel_pcie.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 0f04603105a3..761811de56f3 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -60,6 +60,7 @@ MODULE_DEVICE_TABLE(pci, btintel_pcie_table);
 #define BTINTEL_PCIE_MAGIC_NUM	0xA5A5A5A5
 
 #define BTINTEL_PCIE_TRIGGER_REASON_USER_TRIGGER	0x17A2
+#define BTINTEL_PCIE_TRIGGER_REASON_FW_ASSERT		0x1E61
 
 /* Alive interrupt context */
 enum {
@@ -1217,6 +1218,7 @@ static void btintel_pcie_do_firmware_dump(struct btintel_pcie_data *data, u16 tr
 static void btintel_pcie_msix_hw_exp_handler(struct btintel_pcie_data *data)
 {
 	bt_dev_err(data->hdev, "Received hw exception interrupt");
+	btintel_pcie_do_firmware_dump(data, BTINTEL_PCIE_TRIGGER_REASON_FW_ASSERT);
 	if (test_and_set_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags))
 		return;
 	queue_work(data->workqueue, &data->hwexp_work);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [v1,1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces
  2025-02-18 12:26 [PATCH v1 1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces Kiran K
                   ` (2 preceding siblings ...)
  2025-02-18 12:26 ` [PATCH v1 4/4] Bluetooth: btintel_pcie: Trigger device coredump on exception Kiran K
@ 2025-02-18 13:17 ` bluez.test.bot
  2025-02-18 17:54 ` [PATCH v1 1/4] " kernel test robot
  4 siblings, 0 replies; 8+ messages in thread
From: bluez.test.bot @ 2025-02-18 13:17 UTC (permalink / raw)
  To: linux-bluetooth, kiran.k

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

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=935089

---Test result---

Test Summary:
CheckPatch                    PENDING   0.40 seconds
GitLint                       PENDING   0.26 seconds
SubjectPrefix                 PASS      0.31 seconds
BuildKernel                   PASS      24.71 seconds
CheckAllWarning               PASS      26.95 seconds
CheckSparse                   PASS      30.76 seconds
BuildKernel32                 PASS      24.33 seconds
TestRunnerSetup               PASS      431.41 seconds
TestRunner_l2cap-tester       PASS      20.91 seconds
TestRunner_iso-tester         FAIL      148.82 seconds
TestRunner_bnep-tester        PASS      4.80 seconds
TestRunner_mgmt-tester        FAIL      129.13 seconds
TestRunner_rfcomm-tester      PASS      8.32 seconds
TestRunner_sco-tester         PASS      9.58 seconds
TestRunner_ioctl-tester       PASS      8.39 seconds
TestRunner_mesh-tester        PASS      6.09 seconds
TestRunner_smp-tester         PASS      7.19 seconds
TestRunner_userchan-tester    PASS      4.96 seconds
IncrementalBuild              PENDING   0.56 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Total: 125, Passed: 108 (86.4%), Failed: 13, Not Run: 4

Failed Test Cases
ISO Connect2 CIG 0x01 - Success                      Timed out    2.621 seconds
ISO Defer Connect2 CIG 0x01 - Success                Timed out    2.270 seconds
ISO Connected2 Suspend - Success                     Timed out    2.713 seconds
ISO AC 6(ii) - Success                               Timed out    1.882 seconds
ISO AC 7(ii) - Success                               Timed out    2.514 seconds
ISO AC 8(ii) - Success                               Timed out    2.500 seconds
ISO AC 9(ii) - Success                               Timed out    2.507 seconds
ISO AC 11(ii) - Success                              Timed out    2.515 seconds
ISO AC 1 + 2 - Success                               Timed out    1.968 seconds
ISO AC 1 + 2 CIG 0x01/0x02 - Success                 Timed out    1.999 seconds
ISO Reconnect AC 6(i) - Success                      Timed out    2.023 seconds
ISO Reconnect AC 6(ii) - Success                     Timed out    2.000 seconds
ISO AC 6(ii) CIS 0xEF/auto - Success                 Timed out    1.999 seconds
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 482 (98.4%), Failed: 4, Not Run: 4

Failed Test Cases
LL Privacy - Add Device 3 (AL is full)               Failed       0.190 seconds
LL Privacy - Set Flags 3 (2 Devices to RL)           Failed       0.174 seconds
LL Privacy - Unpair 1                                Timed out    1.894 seconds
LL Privacy - Unpair 2 (Remove from AL)               Failed       2.548 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 2/4] Bluetooth: btintel_pcie: Read hardware exception data
  2025-02-18 12:26 ` [PATCH v1 2/4] Bluetooth: btintel_pcie: Read hardware exception data Kiran K
@ 2025-02-18 17:28   ` Luiz Augusto von Dentz
  2025-02-20  8:11     ` K, Kiran
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2025-02-18 17:28 UTC (permalink / raw)
  To: Kiran K
  Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan,
	chandrashekar.devegowda, vijay.satija

Hi Kiran,

On Tue, Feb 18, 2025 at 7:27 AM Kiran K <kiran.k@intel.com> wrote:
>
> On hardware error, controller writes hardware error event and optional
> vendor specific hci events in device memory in TLV format and raises
> MSIX interrupt. Driver reads the device memory and passes the events to
> the stack for further processing.
>
> Co-developed-by: Vijay Satija <vijay.satija@intel.com>
> Signed-off-by: Vijay Satija <vijay.satija@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>  drivers/bluetooth/btintel.h      |   1 +
>  drivers/bluetooth/btintel_pcie.c | 212 ++++++++++++++++++++++++++++++-
>  drivers/bluetooth/btintel_pcie.h |  23 ++++
>  3 files changed, 235 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index 19530ea14905..4c21e69887a3 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -190,6 +190,7 @@ enum {
>  struct btintel_data {
>         DECLARE_BITMAP(flags, __INTEL_NUM_FLAGS);
>         int (*acpi_reset_method)(struct hci_dev *hdev);
> +       u32     cnvi_top;
>  };
>
>  #define btintel_set_flag(hdev, nr)                                     \
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 11e2b805c7cc..6c78472f9dad 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -51,6 +51,14 @@ MODULE_DEVICE_TABLE(pci, btintel_pcie_table);
>
>   #define BTINTEL_PCIE_MAGIC_NUM    0xA5A5A5A5
>
> +#define BTINTEL_PCIE_BLZR_HWEXP_SIZE           1024
> +#define BTINTEL_PCIE_BLZR_HWEXP_DMP_ADDR       0xB00A7C00
> +
> +#define BTINTEL_PCIE_SCP_HWEXP_SIZE            4096
> +#define BTINTEL_PCIE_SCP_HWEXP_DMP_ADDR                0xB030F800
> +
> +#define BTINTEL_PCIE_MAGIC_NUM 0xA5A5A5A5
> +
>  /* Alive interrupt context */
>  enum {
>         BTINTEL_PCIE_ROM,
> @@ -353,6 +361,63 @@ static int btintel_pcie_reset_bt(struct btintel_pcie_data *data)
>         return reg == 0 ? 0 : -ENODEV;
>  }
>
> +static void btintel_pcie_mac_init(struct btintel_pcie_data *data)
> +{
> +       u32 reg;
> +
> +       /* 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 |
> +                       BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_DISCON |
> +                       BTINTEL_PCIE_CSR_FUNC_CTRL_SW_RESET);
> +       reg |= (BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_ENA |
> +                       BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_INIT);
> +       btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
> +}
> +
> +static int btintel_pcie_get_mac_access(struct btintel_pcie_data *data)
> +{
> +       u32 reg;
> +       int retry = 15;
> +
> +       reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
> +
> +       reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
> +       reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
> +       if ((reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS) == 0)
> +               reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
> +
> +       btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
> +
> +       do {
> +               reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
> +               if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS)
> +                       return 0;
> +               usleep_range(1000, 1200);
> +
> +       } while (--retry > 0);

There seems to be a similar call to btintel_pcie_wr_reg32(data,
BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg); in btintel_pcie_mac_init but
that doesn't retry for some reason, maybe leave a comment if that is
intentional, that said this write, read and wait is sort of weird to
me, if we know it takes time there should probably be an event coming
back, rather than polling multiple times in quick succession.

> +
> +       return -ETIME;
> +}
> +
> +static void btintel_pcie_release_mac_access(struct btintel_pcie_data *data)
> +{
> +       u32 reg;
> +
> +       reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
> +
> +       if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ)
> +               reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
> +
> +       if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS)
> +               reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
> +
> +       if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ)
> +               reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
> +
> +       btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
> +}
> +
>  /* 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.
> @@ -473,6 +538,133 @@ static inline char *btintel_pcie_alivectxt_state2str(u32 alive_intr_ctxt)
>         }
>  }
>
> +static int btintel_pcie_read_device_mem(struct btintel_pcie_data *data,
> +                                       void *buf, u32 dev_addr, int len)
> +{
> +       int err;
> +       u32 *val = buf;
> +
> +       /* Get device mac access */
> +       err = btintel_pcie_get_mac_access(data);
> +       if (err) {
> +               bt_dev_err(data->hdev, "Failed to get mac access %d", err);
> +               return err;
> +       }
> +
> +       for (; len > 0; len -= 4, dev_addr += 4, val++)
> +               *val = btintel_pcie_rd_dev_mem(data, dev_addr);
> +
> +       btintel_pcie_release_mac_access(data);
> +
> +       return 0;
> +}
> +
> +static void btintel_pcie_dump_hwexp(struct btintel_pcie_data *data)
> +{
> +       struct btintel_data *intel_data = hci_get_priv(data->hdev);
> +       int len, err, offset, pending;
> +       struct sk_buff *skb;
> +       u32 addr, val;
> +       u8 *buf;
> +
> +       struct tlv {
> +               u8      type;
> +               u16     len;
> +               u8      val[];
> +       } __packed;
> +
> +       struct tlv *tlv;
> +
> +       switch (intel_data->cnvi_top & 0xfff) {
> +       case BTINTEL_CNVI_BLAZARI:
> +       case BTINTEL_CNVI_BLAZARIW:
> +               /* only from step B0 onwards */
> +               if (INTEL_CNVX_TOP_STEP(intel_data->cnvi_top) != 0x01)
> +                       return;
> +               len = BTINTEL_PCIE_BLZR_HWEXP_SIZE; /* exception data length */
> +               addr = BTINTEL_PCIE_BLZR_HWEXP_DMP_ADDR;
> +       break;
> +       case BTINTEL_CNVI_SCP:
> +               len = BTINTEL_PCIE_SCP_HWEXP_SIZE;
> +               addr = BTINTEL_PCIE_SCP_HWEXP_DMP_ADDR;
> +       break;
> +       default:
> +               bt_dev_err(data->hdev, "Unsupported cnvi 0x%8x", intel_data->cnvi_top);
> +               return;
> +       }
> +
> +       buf = kzalloc(len, GFP_KERNEL);
> +       if (!buf)
> +               goto exit_on_error;
> +
> +       btintel_pcie_mac_init(data);
> +
> +       err = btintel_pcie_read_device_mem(data, buf, addr, len);
> +       if (err)
> +               goto exit_on_error;
> +
> +       val = get_unaligned_le32(buf);
> +       if (val != BTINTEL_PCIE_MAGIC_NUM) {
> +               bt_dev_err(data->hdev, "Invalid exception dump signature: 0x%8.8x",
> +                          val);
> +               goto exit_on_error;
> +       }
> +
> +       offset = 4;
> +       do {
> +               pending = len - offset;
> +               if (pending < sizeof(*tlv))
> +                       break;
> +               tlv = (struct tlv *)(buf + offset);
> +               if (!tlv->type) {
> +                       bt_dev_dbg(data->hdev, "Invalid TLV type 0");
> +                       break;
> +               }
> +               tlv->len = le16_to_cpu((__force __le16)tlv->len);
> +               offset += sizeof(*tlv);
> +               pending = len - offset;
> +               if (tlv->len > pending)
> +                       break;
> +
> +               offset += tlv->len;
> +                /* Only TLV of type = 1 are vendor HCI events */
> +               if (tlv->type != 1)
> +                       continue;
> +
> +               bt_dev_dbg(data->hdev, "Exception pkt len: %u", tlv->len);
> +               if (tlv->len > HCI_MAX_EVENT_SIZE)
> +                       break;
> +               skb = bt_skb_alloc(tlv->len, GFP_KERNEL);
> +               if (!skb)
> +                       goto exit_on_error;
> +               hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
> +               skb_put_data(skb, tlv->val, tlv->len);
> +
> +               /* copy Intel specific pcie packet type */
> +               val = BTINTEL_PCIE_HCI_EVT_PKT;
> +               memcpy(skb_push(skb, BTINTEL_PCIE_HCI_TYPE_LEN), &val,
> +                      BTINTEL_PCIE_HCI_TYPE_LEN);
> +
> +               print_hex_dump(KERN_DEBUG, "Bluetooth: ", DUMP_PREFIX_OFFSET, 16,
> +                              1, tlv->val, tlv->len, false);
> +
> +               skb_queue_tail(&data->rx_skb_q, skb);
> +               queue_work(data->workqueue, &data->rx_work);
> +       } while (offset < len);

Could there be multiple exceptions? If there is only one the perhaps
you could use eir_get_data instead of recreating the code for parsing
LTV entries, although we could possibly rename that to make it more
generic.

> +
> +exit_on_error:
> +       kfree(buf);
> +}
> +
> +static void btintel_pcie_hwexp_work(struct work_struct *work)
> +{
> +       struct btintel_pcie_data *data = container_of(work,
> +                                       struct btintel_pcie_data, hwexp_work);
> +       btintel_pcie_dump_hwexp(data);
> +
> +       clear_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags);
> +}
> +
>  /* 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.
>   */
> @@ -794,6 +986,14 @@ static int btintel_pcie_recv_frame(struct btintel_pcie_data *data,
>         return ret;
>  }
>
> +static void btintel_pcie_msix_hw_exp_handler(struct btintel_pcie_data *data)
> +{
> +       bt_dev_err(data->hdev, "Received hw exception interrupt");
> +       if (test_and_set_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags))
> +               return;
> +       queue_work(data->workqueue, &data->hwexp_work);

Any particular reason why we can process the exception in the rx_work?
That shall probably be made into a comment either way.

> +}
> +
>  static void btintel_pcie_rx_work(struct work_struct *work)
>  {
>         struct btintel_pcie_data *data = container_of(work,
> @@ -920,6 +1120,10 @@ static irqreturn_t btintel_pcie_irq_msix_handler(int irq, void *dev_id)
>                 return IRQ_NONE;
>         }
>
> +       /* This interrupt is raised when there is an hardware exception */
> +       if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_HWEXP)
> +               btintel_pcie_msix_hw_exp_handler(data);
> +
>         /* This interrupt is triggered by the firmware after updating
>          * boot_stage register and image_response register
>          */
> @@ -1000,7 +1204,8 @@ struct btintel_pcie_causes_list {
>  static struct btintel_pcie_causes_list causes_list[] = {
>         { BTINTEL_PCIE_MSIX_FH_INT_CAUSES_0,    BTINTEL_PCIE_CSR_MSIX_FH_INT_MASK,      0x00 },
>         { BTINTEL_PCIE_MSIX_FH_INT_CAUSES_1,    BTINTEL_PCIE_CSR_MSIX_FH_INT_MASK,      0x01 },
> -       { BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0, BTINTEL_PCIE_CSR_MSIX_HW_INT_MASK,       0x20 },
> +       { BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0,  BTINTEL_PCIE_CSR_MSIX_HW_INT_MASK,      0x20 },
> +       { BTINTEL_PCIE_MSIX_HW_INT_CAUSES_HWEXP, BTINTEL_PCIE_CSR_MSIX_HW_INT_MASK,     0x23 },
>  };
>
>  /* This function configures the interrupt masks for both HW_INT_CAUSES and
> @@ -1482,6 +1687,7 @@ static void btintel_pcie_release_hdev(struct btintel_pcie_data *data)
>
>  static int btintel_pcie_setup_internal(struct hci_dev *hdev)
>  {
> +       struct btintel_data *data = hci_get_priv(hdev);
>         const u8 param[1] = { 0xFF };
>         struct intel_version_tlv ver_tlv;
>         struct sk_buff *skb;
> @@ -1520,6 +1726,7 @@ static int btintel_pcie_setup_internal(struct hci_dev *hdev)
>                 goto exit_error;
>         }
>
> +       data->cnvi_top = ver_tlv.cnvi_top;
>         switch (INTEL_HW_PLATFORM(ver_tlv.cnvi_bt)) {
>         case 0x37:
>                 break;
> @@ -1667,6 +1874,8 @@ static int btintel_pcie_probe(struct pci_dev *pdev,
>         skb_queue_head_init(&data->rx_skb_q);
>         INIT_WORK(&data->rx_work, btintel_pcie_rx_work);
>
> +       INIT_WORK(&data->hwexp_work, btintel_pcie_hwexp_work);
> +
>         data->boot_stage_cache = 0x00;
>         data->img_resp_cache = 0x00;
>
> @@ -1731,6 +1940,7 @@ static void btintel_pcie_remove(struct pci_dev *pdev)
>         btintel_pcie_release_hdev(data);
>
>         flush_work(&data->rx_work);
> +       flush_work(&data->hwexp_work);
>
>         destroy_workqueue(data->workqueue);
>
> diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
> index b9d32393002b..98902fd4fc96 100644
> --- a/drivers/bluetooth/btintel_pcie.h
> +++ b/drivers/bluetooth/btintel_pcie.h
> @@ -16,6 +16,8 @@
>  #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)
> +#define BTINTEL_PCIE_PRPH_DEV_ADDR_REG         (BTINTEL_PCIE_CSR_BASE + 0x440)
> +#define BTINTEL_PCIE_PRPH_DEV_RD_REG           (BTINTEL_PCIE_CSR_BASE + 0x458)
>  #define BTINTEL_PCIE_CSR_HBUS_TARG_WRPTR       (BTINTEL_PCIE_CSR_BASE + 0x460)
>
>  /* BTINTEL_PCIE_CSR Function Control Register */
> @@ -23,6 +25,12 @@
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_INIT            (BIT(6))
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_INIT           (BIT(7))
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS      (BIT(20))
> +
> +#define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ      (BIT(21))
> +/* Stop MAC Access disconnection request */
> +#define BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS (BIT(22))
> +#define BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ                (BIT(23))
> +
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_STS      (BIT(28))
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_DISCON   (BIT(29))
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_SW_RESET            (BIT(31))
> @@ -72,6 +80,7 @@ enum msix_fh_int_causes {
>  /* Causes for the HW register interrupts */
>  enum msix_hw_int_causes {
>         BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0     = BIT(0),       /* cause 32 */
> +       BTINTEL_PCIE_MSIX_HW_INT_CAUSES_HWEXP   = BIT(3),       /* cause 35 */
>  };
>
>  /* PCIe device states
> @@ -84,6 +93,11 @@ enum {
>         BTINTEL_PCIE_STATE_D3_HOT = 2,
>         BTINTEL_PCIE_STATE_D3_COLD = 3,
>  };
> +
> +enum {
> +       BTINTEL_PCIE_HWEXP_INPROGRESS,
> +};
> +
>  #define BTINTEL_PCIE_MSIX_NON_AUTO_CLEAR_CAUSE BIT(7)
>
>  /* Minimum and Maximum number of MSI-X Vector
> @@ -437,6 +451,7 @@ struct btintel_pcie_data {
>         struct rxq      rxq;
>         u32     alive_intr_ctxt;
>         struct btintel_pcie_dbgc        dbgc;
> +       struct work_struct      hwexp_work;
>  };
>
>  static inline u32 btintel_pcie_rd_reg32(struct btintel_pcie_data *data,
> @@ -476,3 +491,11 @@ static inline void btintel_pcie_clr_reg_bits(struct btintel_pcie_data *data,
>         r &= ~bits;
>         iowrite32(r, data->base_addr + offset);
>  }
> +
> +static inline u32 btintel_pcie_rd_dev_mem(struct btintel_pcie_data *data,
> +                                         u32 addr)
> +{
> +       btintel_pcie_wr_reg32(data, BTINTEL_PCIE_PRPH_DEV_ADDR_REG, addr);
> +       return btintel_pcie_rd_reg32(data, BTINTEL_PCIE_PRPH_DEV_RD_REG);
> +}
> +
> --
> 2.43.0
>
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces
  2025-02-18 12:26 [PATCH v1 1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces Kiran K
                   ` (3 preceding siblings ...)
  2025-02-18 13:17 ` [v1,1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces bluez.test.bot
@ 2025-02-18 17:54 ` kernel test robot
  4 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2025-02-18 17:54 UTC (permalink / raw)
  To: Kiran K, linux-bluetooth
  Cc: oe-kbuild-all, ravishankar.srivatsa, chethan.tumkur.narayan,
	chandrashekar.devegowda, vijay.satija, Kiran K

Hi Kiran,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on bluetooth/master linus/master v6.14-rc3 next-20250218]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kiran-K/Bluetooth-btintel_pcie-Read-hardware-exception-data/20250218-202903
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
patch link:    https://lore.kernel.org/r/20250218122620.762523-1-kiran.k%40intel.com
patch subject: [PATCH v1 1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces
config: sparc-randconfig-002-20250218 (https://download.01.org/0day-ci/archive/20250219/202502190143.0LmI162j-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250219/202502190143.0LmI162j-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502190143.0LmI162j-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/bluetooth/btintel_pcie.c: In function 'btintel_pcie_setup_dbgc':
>> drivers/bluetooth/btintel_pcie.c:134:72: warning: right shift count >= width of type [-Wshift-count-overflow]
     134 |                 db_frag.bufs[i].buf_addr_msb = (u32)((buf->data_p_addr >> 32) & 0xffffffff);
         |                                                                        ^~


vim +134 drivers/bluetooth/btintel_pcie.c

    90	
    91	/* This function initializes the memory for DBGC buffers and formats the
    92	 * DBGC fragment which consists header info and DBGC buffer's LSB, MSB and
    93	 * size as the payload
    94	 */
    95	static int btintel_pcie_setup_dbgc(struct btintel_pcie_data *data)
    96	{
    97		struct btintel_pcie_dbgc_ctxt db_frag;
    98		struct data_buf *buf;
    99		int i;
   100	
   101		data->dbgc.count = BTINTEL_PCIE_DBGC_BUFFER_COUNT;
   102		data->dbgc.bufs = devm_kcalloc(&data->pdev->dev, data->dbgc.count,
   103					       sizeof(*buf), GFP_KERNEL);
   104		if (!data->dbgc.bufs)
   105			return -ENOMEM;
   106	
   107		data->dbgc.buf_v_addr = dmam_alloc_coherent(&data->pdev->dev,
   108							    data->dbgc.count *
   109							    BTINTEL_PCIE_DBGC_BUFFER_SIZE,
   110							    &data->dbgc.buf_p_addr,
   111							    GFP_KERNEL | __GFP_NOWARN);
   112		if (!data->dbgc.buf_v_addr)
   113			return -ENOMEM;
   114	
   115		data->dbgc.frag_v_addr = dmam_alloc_coherent(&data->pdev->dev,
   116							     sizeof(struct btintel_pcie_dbgc_ctxt),
   117							     &data->dbgc.frag_p_addr,
   118							     GFP_KERNEL | __GFP_NOWARN);
   119		if (!data->dbgc.frag_v_addr)
   120			return -ENOMEM;
   121	
   122		data->dbgc.frag_size = sizeof(struct btintel_pcie_dbgc_ctxt);
   123	
   124		db_frag.magic_num = BTINTEL_PCIE_MAGIC_NUM;
   125		db_frag.ver = BTINTEL_PCIE_DBGC_FRAG_VERSION;
   126		db_frag.total_size = BTINTEL_PCIE_DBGC_FRAG_PAYLOAD_SIZE;
   127		db_frag.num_buf = BTINTEL_PCIE_DBGC_FRAG_BUFFER_COUNT;
   128	
   129		for (i = 0; i < data->dbgc.count; i++) {
   130			buf = &data->dbgc.bufs[i];
   131			buf->data_p_addr = data->dbgc.buf_p_addr + i * BTINTEL_PCIE_DBGC_BUFFER_SIZE;
   132			buf->data = data->dbgc.buf_v_addr + i * BTINTEL_PCIE_DBGC_BUFFER_SIZE;
   133			db_frag.bufs[i].buf_addr_lsb = (u32)(buf->data_p_addr & 0xffffffff);
 > 134			db_frag.bufs[i].buf_addr_msb = (u32)((buf->data_p_addr >> 32) & 0xffffffff);
   135			db_frag.bufs[i].buf_size = BTINTEL_PCIE_DBGC_BUFFER_SIZE;
   136		}
   137	
   138		memcpy(data->dbgc.frag_v_addr, &db_frag, sizeof(db_frag));
   139		return 0;
   140	}
   141	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v1 2/4] Bluetooth: btintel_pcie: Read hardware exception data
  2025-02-18 17:28   ` Luiz Augusto von Dentz
@ 2025-02-20  8:11     ` K, Kiran
  0 siblings, 0 replies; 8+ messages in thread
From: K, Kiran @ 2025-02-20  8:11 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: linux-bluetooth@vger.kernel.org, Srivatsa, Ravishankar,
	Tumkur Narayan, Chethan, Devegowda, Chandrashekar, Satija, Vijay

Hi Luiz,

Thanks for your comments.

>Hi Kiran,
>
>On Tue, Feb 18, 2025 at 7:27 AM Kiran K <kiran.k@intel.com> wrote:
>>
>> On hardware error, controller writes hardware error event and optional
>> vendor specific hci events in device memory in TLV format and raises
>> MSIX interrupt. Driver reads the device memory and passes the events
>> to the stack for further processing.
>>
>> Co-developed-by: Vijay Satija <vijay.satija@intel.com>
>> Signed-off-by: Vijay Satija <vijay.satija@intel.com>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>>  drivers/bluetooth/btintel.h      |   1 +
>>  drivers/bluetooth/btintel_pcie.c | 212
>> ++++++++++++++++++++++++++++++-  drivers/bluetooth/btintel_pcie.h |
>> 23 ++++
>>  3 files changed, 235 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
>> index 19530ea14905..4c21e69887a3 100644
>> --- a/drivers/bluetooth/btintel.h
>> +++ b/drivers/bluetooth/btintel.h
>> @@ -190,6 +190,7 @@ enum {
>>  struct btintel_data {
>>         DECLARE_BITMAP(flags, __INTEL_NUM_FLAGS);
>>         int (*acpi_reset_method)(struct hci_dev *hdev);
>> +       u32     cnvi_top;
>>  };
>>
>>  #define btintel_set_flag(hdev, nr)                                     \
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index 11e2b805c7cc..6c78472f9dad 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -51,6 +51,14 @@ MODULE_DEVICE_TABLE(pci, btintel_pcie_table);
>>
>>   #define BTINTEL_PCIE_MAGIC_NUM    0xA5A5A5A5
>>
>> +#define BTINTEL_PCIE_BLZR_HWEXP_SIZE           1024
>> +#define BTINTEL_PCIE_BLZR_HWEXP_DMP_ADDR       0xB00A7C00
>> +
>> +#define BTINTEL_PCIE_SCP_HWEXP_SIZE            4096
>> +#define BTINTEL_PCIE_SCP_HWEXP_DMP_ADDR                0xB030F800
>> +
>> +#define BTINTEL_PCIE_MAGIC_NUM 0xA5A5A5A5
>> +
>>  /* Alive interrupt context */
>>  enum {
>>         BTINTEL_PCIE_ROM,
>> @@ -353,6 +361,63 @@ static int btintel_pcie_reset_bt(struct
>btintel_pcie_data *data)
>>         return reg == 0 ? 0 : -ENODEV;  }
>>
>> +static void btintel_pcie_mac_init(struct btintel_pcie_data *data) {
>> +       u32 reg;
>> +
>> +       /* 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 |
>> +                       BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_DISCON |
>> +                       BTINTEL_PCIE_CSR_FUNC_CTRL_SW_RESET);
>> +       reg |= (BTINTEL_PCIE_CSR_FUNC_CTRL_FUNC_ENA |
>> +                       BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_INIT);
>> +       btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG,
>> +reg); }
>> +
>> +static int btintel_pcie_get_mac_access(struct btintel_pcie_data
>> +*data) {
>> +       u32 reg;
>> +       int retry = 15;
>> +
>> +       reg = btintel_pcie_rd_reg32(data,
>> + BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>> +
>> +       reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
>> +       reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
>> +       if ((reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS) == 0)
>> +               reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
>> +
>> +       btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG,
>> + reg);
>> +
>> +       do {
>> +               reg = btintel_pcie_rd_reg32(data,
>BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>> +               if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS)
>> +                       return 0;
>> +               usleep_range(1000, 1200);
>> +
>> +       } while (--retry > 0);
>
>There seems to be a similar call to btintel_pcie_wr_reg32(data,
>BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg); in btintel_pcie_mac_init but that
>doesn't retry for some reason, maybe leave a comment if that is intentional,
>that said this write, read and wait is sort of weird to me, if we know it takes
>time there should probably be an event coming back, rather than polling
>multiple times in quick succession.
>
In  btintel_pcie_mac_init(), only initialization of mac block is done. When Target Access is done via mac a delay is required for the hardware to settle down. I would add comment about this in the v2 version of the patch.
 
>> +
>> +       return -ETIME;
>> +}
>> +
>> +static void btintel_pcie_release_mac_access(struct btintel_pcie_data
>> +*data) {
>> +       u32 reg;
>> +
>> +       reg = btintel_pcie_rd_reg32(data,
>> + BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>> +
>> +       if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ)
>> +               reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
>> +
>> +       if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS)
>> +               reg &=
>> + ~BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
>> +
>> +       if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ)
>> +               reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
>> +
>> +       btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG,
>> +reg); }
>> +
>>  /* 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.
>> @@ -473,6 +538,133 @@ static inline char
>*btintel_pcie_alivectxt_state2str(u32 alive_intr_ctxt)
>>         }
>>  }
>>
>> +static int btintel_pcie_read_device_mem(struct btintel_pcie_data *data,
>> +                                       void *buf, u32 dev_addr, int
>> +len) {
>> +       int err;
>> +       u32 *val = buf;
>> +
>> +       /* Get device mac access */
>> +       err = btintel_pcie_get_mac_access(data);
>> +       if (err) {
>> +               bt_dev_err(data->hdev, "Failed to get mac access %d", err);
>> +               return err;
>> +       }
>> +
>> +       for (; len > 0; len -= 4, dev_addr += 4, val++)
>> +               *val = btintel_pcie_rd_dev_mem(data, dev_addr);
>> +
>> +       btintel_pcie_release_mac_access(data);
>> +
>> +       return 0;
>> +}
>> +
>> +static void btintel_pcie_dump_hwexp(struct btintel_pcie_data *data) {
>> +       struct btintel_data *intel_data = hci_get_priv(data->hdev);
>> +       int len, err, offset, pending;
>> +       struct sk_buff *skb;
>> +       u32 addr, val;
>> +       u8 *buf;
>> +
>> +       struct tlv {
>> +               u8      type;
>> +               u16     len;
>> +               u8      val[];
>> +       } __packed;
>> +
>> +       struct tlv *tlv;
>> +
>> +       switch (intel_data->cnvi_top & 0xfff) {
>> +       case BTINTEL_CNVI_BLAZARI:
>> +       case BTINTEL_CNVI_BLAZARIW:
>> +               /* only from step B0 onwards */
>> +               if (INTEL_CNVX_TOP_STEP(intel_data->cnvi_top) != 0x01)
>> +                       return;
>> +               len = BTINTEL_PCIE_BLZR_HWEXP_SIZE; /* exception data length */
>> +               addr = BTINTEL_PCIE_BLZR_HWEXP_DMP_ADDR;
>> +       break;
>> +       case BTINTEL_CNVI_SCP:
>> +               len = BTINTEL_PCIE_SCP_HWEXP_SIZE;
>> +               addr = BTINTEL_PCIE_SCP_HWEXP_DMP_ADDR;
>> +       break;
>> +       default:
>> +               bt_dev_err(data->hdev, "Unsupported cnvi 0x%8x", intel_data-
>>cnvi_top);
>> +               return;
>> +       }
>> +
>> +       buf = kzalloc(len, GFP_KERNEL);
>> +       if (!buf)
>> +               goto exit_on_error;
>> +
>> +       btintel_pcie_mac_init(data);
>> +
>> +       err = btintel_pcie_read_device_mem(data, buf, addr, len);
>> +       if (err)
>> +               goto exit_on_error;
>> +
>> +       val = get_unaligned_le32(buf);
>> +       if (val != BTINTEL_PCIE_MAGIC_NUM) {
>> +               bt_dev_err(data->hdev, "Invalid exception dump signature:
>0x%8.8x",
>> +                          val);
>> +               goto exit_on_error;
>> +       }
>> +
>> +       offset = 4;
>> +       do {
>> +               pending = len - offset;
>> +               if (pending < sizeof(*tlv))
>> +                       break;
>> +               tlv = (struct tlv *)(buf + offset);
>> +               if (!tlv->type) {
>> +                       bt_dev_dbg(data->hdev, "Invalid TLV type 0");
>> +                       break;
>> +               }
>> +               tlv->len = le16_to_cpu((__force __le16)tlv->len);
>> +               offset += sizeof(*tlv);
>> +               pending = len - offset;
>> +               if (tlv->len > pending)
>> +                       break;
>> +
>> +               offset += tlv->len;
>> +                /* Only TLV of type = 1 are vendor HCI events */
>> +               if (tlv->type != 1)
>> +                       continue;
>> +
>> +               bt_dev_dbg(data->hdev, "Exception pkt len: %u", tlv->len);
>> +               if (tlv->len > HCI_MAX_EVENT_SIZE)
>> +                       break;
>> +               skb = bt_skb_alloc(tlv->len, GFP_KERNEL);
>> +               if (!skb)
>> +                       goto exit_on_error;
>> +               hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
>> +               skb_put_data(skb, tlv->val, tlv->len);
>> +
>> +               /* copy Intel specific pcie packet type */
>> +               val = BTINTEL_PCIE_HCI_EVT_PKT;
>> +               memcpy(skb_push(skb, BTINTEL_PCIE_HCI_TYPE_LEN), &val,
>> +                      BTINTEL_PCIE_HCI_TYPE_LEN);
>> +
>> +               print_hex_dump(KERN_DEBUG, "Bluetooth: ",
>DUMP_PREFIX_OFFSET, 16,
>> +                              1, tlv->val, tlv->len, false);
>> +
>> +               skb_queue_tail(&data->rx_skb_q, skb);
>> +               queue_work(data->workqueue, &data->rx_work);
>> +       } while (offset < len);
>
>Could there be multiple exceptions? If there is only one the perhaps you could
As of now there will be 2 TLVs - Hardware Error event and Telemetry event. Later there can be vendor specific event related to debug.

>use eir_get_data instead of recreating the code for parsing LTV entries,
It looks we have conflict in type field used in eir_get_data() and TLVs used here.  Right now, TLVs with type == 1 is considered as HCI event and is processed.

>although we could possibly rename that to make it more generic.
>
>> +
>> +exit_on_error:
>> +       kfree(buf);
>> +}
>> +
>> +static void btintel_pcie_hwexp_work(struct work_struct *work) {
>> +       struct btintel_pcie_data *data = container_of(work,
>> +                                       struct btintel_pcie_data, hwexp_work);
>> +       btintel_pcie_dump_hwexp(data);
>> +
>> +       clear_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags); }
>> +
>>  /* 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.
>>   */
>> @@ -794,6 +986,14 @@ static int btintel_pcie_recv_frame(struct
>btintel_pcie_data *data,
>>         return ret;
>>  }
>>
>> +static void btintel_pcie_msix_hw_exp_handler(struct btintel_pcie_data
>> +*data) {
>> +       bt_dev_err(data->hdev, "Received hw exception interrupt");
>> +       if (test_and_set_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags))
>> +               return;
>> +       queue_work(data->workqueue, &data->hwexp_work);
>
>Any particular reason why we can process the exception in the rx_work?
>That shall probably be made into a comment either way.

Unlike in USB products, controller doesn’t send HW Error event when exception happens. Controller writes hardware error event into device memory, raises MSIX and halts.
Driver reads HW error from device memory, constructs skb and pushes to rx_work.  I thought its better to do all this in separate work item instead of cluttering rx_work.  

>
>> +}
>> +
>>  static void btintel_pcie_rx_work(struct work_struct *work)  {
>>         struct btintel_pcie_data *data = container_of(work, @@ -920,6
>> +1120,10 @@ static irqreturn_t btintel_pcie_irq_msix_handler(int irq, void
>*dev_id)
>>                 return IRQ_NONE;
>>         }
>>

Thanks,
Kiran


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-02-20  8:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-18 12:26 [PATCH v1 1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces Kiran K
2025-02-18 12:26 ` [PATCH v1 2/4] Bluetooth: btintel_pcie: Read hardware exception data Kiran K
2025-02-18 17:28   ` Luiz Augusto von Dentz
2025-02-20  8:11     ` K, Kiran
2025-02-18 12:26 ` [PATCH v1 3/4] Bluetooth: btintel_pcie: Add support for device coredump Kiran K
2025-02-18 12:26 ` [PATCH v1 4/4] Bluetooth: btintel_pcie: Trigger device coredump on exception Kiran K
2025-02-18 13:17 ` [v1,1/4] Bluetooth: btintel_pcie: Setup buffers for firmware traces bluez.test.bot
2025-02-18 17:54 ` [PATCH v1 1/4] " kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.