[PATCH 1/2] Bluetooth: btusb: Record debug log for Mediatek Chip.

[PATCH 1/2] Bluetooth: btusb: Record debug log for Mediatek Chip.

[mark-yw.chen]

From: "mark-yw.chen" <mark-yw.chen@mediatek.com>

1. Add btusb_recv_bulk_mtk to receive debug log.
2. Send the debug log via hci_recv_diag channel.
3. The upper layerd use hci_channel_minitor to receive
   these packets.

Signed-off-by: mark-yw.chen <mark-yw.chen@mediatek.com>
---
 drivers/bluetooth/btusb.c | 67 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index b015dcecfb13..2a55ae0a5f8c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3851,6 +3851,72 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
 	return 0;
 }
 
+static int btusb_recv_bulk_mtk(struct btusb_data *data, void *buffer, int count)
+{
+	struct sk_buff *skb;
+	unsigned long flags;
+	int err = 0;
+
+	spin_lock_irqsave(&data->rxlock, flags);
+	skb = data->acl_skb;
+
+	while (count) {
+		int len;
+
+		if (!skb) {
+			skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
+			if (!skb) {
+				err = -ENOMEM;
+				break;
+			}
+
+			hci_skb_pkt_type(skb) = HCI_ACLDATA_PKT;
+			hci_skb_expect(skb) = HCI_ACL_HDR_SIZE;
+		}
+
+		len = min_t(uint, hci_skb_expect(skb), count);
+		skb_put_data(skb, buffer, len);
+
+		count -= len;
+		buffer += len;
+		hci_skb_expect(skb) -= len;
+
+		if (skb->len == HCI_ACL_HDR_SIZE) {
+			__le16 dlen = hci_acl_hdr(skb)->dlen;
+
+			/* Complete ACL header */
+			hci_skb_expect(skb) = __le16_to_cpu(dlen);
+
+			if (skb_tailroom(skb) < hci_skb_expect(skb)) {
+				kfree_skb(skb);
+				skb = NULL;
+
+				err = -EILSEQ;
+				break;
+			}
+		}
+
+		if (!hci_skb_expect(skb)) {
+			/* Complete frame */
+			if (skb->data[0] == 0x6f && skb->data[1] == 0xfc) {
+				usb_disable_autosuspend(data->udev);
+				hci_recv_diag(data->hdev, skb);
+			} else if ((skb->data[0] == 0xfe || skb->data[0] == 0xff) &&
+				   skb->data[1] == 0x05) {
+				hci_recv_diag(data->hdev, skb);
+			} else {
+				hci_recv_frame(data->hdev, skb);
+			}
+			skb = NULL;
+		}
+	}
+
+	data->acl_skb = skb;
+	spin_unlock_irqrestore(&data->rxlock, flags);
+
+	return err;
+}
+
 MODULE_FIRMWARE(FIRMWARE_MT7663);
 MODULE_FIRMWARE(FIRMWARE_MT7668);
 
@@ -4669,6 +4735,7 @@ static int btusb_probe(struct usb_interface *intf,
 		hdev->setup = btusb_mtk_setup;
 		hdev->shutdown = btusb_mtk_shutdown;
 		hdev->manufacturer = 70;
+		data->recv_bulk = btusb_recv_bulk_mtk;
 		set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
 	}
 
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 2/2] Bluetooth: btusb: Support Bluetooth Reset for Mediatek Chip.

[mark-yw.chen]

From: "mark-yw.chen" <mark-yw.chen@mediatek.com>

This change enable Mediatek reset mechanism via USB, it's a specific
reset mechanism with Mediatek chips. To handle error recovery, it use
the cmd_timoeut to reset Mediatek Bluetooth.

Signed-off-by: mark-yw.chen <mark-yw.chen@mediatek.com>
---
 drivers/bluetooth/btusb.c | 145 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 2a55ae0a5f8c..acc23afbfe0f 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3075,6 +3075,18 @@ static int btusb_shutdown_intel_new(struct hci_dev *hdev)
 	return 0;
 }
 
+/* UHW CR mapping */
+#define BT_MISC			0x70002510
+#define BT_SUBSYS_RST		0x70002610
+#define UDMA_INT_STA_BT		0x74000024
+#define UDMA_INT_STA_BT1	0x74000308
+#define BT_WDT_STATUS		0x740003A0
+#define EP_RST_OPT		0x74011890
+#define EP_RST_IN_OUT_OPT	0x00010001
+#define BT_RST_DONE		0x00000100
+#define BT_RESET_WAIT_MS	100
+#define BT_RESET_NUM_TRIES	10
+
 #define FIRMWARE_MT7663		"mediatek/mt7663pr2h.bin"
 #define FIRMWARE_MT7668		"mediatek/mt7668pr2h.bin"
 
@@ -3649,6 +3661,59 @@ static int btusb_mtk_func_query(struct hci_dev *hdev)
 	return status;
 }
 
+static int btusb_mtk_uhw_reg_write(struct btusb_data *data, u32 reg, u32 val)
+{
+	int pipe, err, size = sizeof(u32);
+	u8 buf[4];
+
+	buf[0] = (val & 0x00ff);
+	buf[1] = ((val >> 8) & 0x00ff);
+	buf[2] = ((val >> 16) & 0x00ff);
+	buf[3] = ((val >> 24) & 0x00ff);
+
+	pipe = usb_sndctrlpipe(data->udev, 0);
+	err = usb_control_msg(data->udev, pipe, 0x02,
+			      0x5E,
+			      reg >> 16, reg & 0xffff,
+			      buf, size, USB_CTRL_SET_TIMEOUT);
+	if (err < 0) {
+		BT_ERR("Failed to write uhw reg(%d)", err);
+		goto err_free_buf;
+	}
+
+err_free_buf:
+
+	return err;
+}
+
+static int btusb_mtk_uhw_reg_read(struct btusb_data *data, u32 reg, u32 *val)
+{
+	int pipe, err, size = sizeof(u32);
+	void *buf;
+
+	buf = kzalloc(size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	pipe = usb_rcvctrlpipe(data->udev, 0);
+	err = usb_control_msg(data->udev, pipe, 0x01,
+			      0xDE,
+			      reg >> 16, reg & 0xffff,
+			      buf, size, USB_CTRL_SET_TIMEOUT);
+	if (err < 0) {
+		BT_ERR("Failed to read uhw reg(%d)", err);
+		goto err_free_buf;
+	}
+
+	*val = get_unaligned_le32(buf);
+	BT_INFO("%s: reg=%x, value=0x%08x", __func__, reg, *val);
+
+err_free_buf:
+	kfree(buf);
+
+	return err;
+}
+
 static int btusb_mtk_reg_read(struct btusb_data *data, u32 reg, u32 *val)
 {
 	int pipe, err, size = sizeof(u32);
@@ -3728,6 +3793,7 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
 			 dev_id & 0xffff, (fw_version & 0xff) + 1);
 		err = btusb_mtk_setup_firmware_79xx(hdev, fw_bin_name);
 
+		btusb_mtk_uhw_reg_write(data, EP_RST_OPT, EP_RST_IN_OUT_OPT);
 		/* Enable Bluetooth protocol */
 		param = 1;
 		wmt_params.op = BTMTK_WMT_FUNC_CTRL;
@@ -3851,6 +3917,84 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
 	return 0;
 }
 
+static void btusb_mtk_cmd_timeout(struct hci_dev *hdev)
+{
+	struct btusb_data *data = hci_get_drvdata(hdev);
+	u32 val;
+	int err, retry = 0;
+
+	/*
+	 * Toggle the bluetooth reset via usb. It's mediatek spicific reset
+	 * mechanism, and the reset flow won't replug the USB. Thus, it must
+	 * handle the urb transaction, such as stop_traffic and submit_urb.
+	 * After bluetooth reset completed, we will re-download the firmware
+	 * and send hw_err to host.
+	 */
+	if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
+		bt_dev_err(hdev, "last reset failed? Not resetting again");
+		return;
+	}
+
+	if (enable_autosuspend)
+		usb_disable_autosuspend(data->udev);
+
+	data->sco_num = 0;
+
+	btusb_stop_traffic(data);
+	usb_kill_anchored_urbs(&data->tx_anchor);
+
+	/* For reset */
+	bt_dev_info(hdev, "Initiating reset mechanism via uhw");
+	/*
+	 * Todo - add error handling for usb no response case and uhw
+	 * something worng.
+	 */
+	btusb_mtk_uhw_reg_write(data, EP_RST_OPT, EP_RST_IN_OUT_OPT);
+	btusb_mtk_uhw_reg_read(data, BT_WDT_STATUS, &val);
+
+	/* Write Reset CR to 1 */
+	btusb_mtk_uhw_reg_write(data, BT_SUBSYS_RST, 1);
+
+	btusb_mtk_uhw_reg_write(data, UDMA_INT_STA_BT, 0x000000FF);
+	btusb_mtk_uhw_reg_read(data, UDMA_INT_STA_BT, &val);
+	btusb_mtk_uhw_reg_write(data, UDMA_INT_STA_BT1, 0x000000FF);
+	btusb_mtk_uhw_reg_read(data, UDMA_INT_STA_BT1, &val);
+	msleep(1);
+
+	/* Write Reset CR to 0 */
+	btusb_mtk_uhw_reg_write(data, BT_SUBSYS_RST, 0);
+
+	/* Read reset CR */
+	btusb_mtk_uhw_reg_read(data, BT_SUBSYS_RST, &val);
+
+	do {
+		btusb_mtk_uhw_reg_read(data, BT_MISC, &val);
+		if ((val & 0x00000100) == BT_RST_DONE) {
+			bt_dev_info(hdev, "Bluetooth Reset Sucessfully");
+			break;
+		} else
+			bt_dev_dbg(hdev, "Polling Bluetooth Reset CR");
+
+		retry++;
+		msleep(BT_RESET_WAIT_MS);
+	} while (retry < BT_RESET_NUM_TRIES);
+
+	btusb_mtk_id_get(data, 0x70010200, &val);
+	bt_dev_info(hdev, "device id (%x)", val);
+
+	err = btusb_mtk_setup(hdev);
+	if (err < 0) {
+		bt_dev_err(hdev, "mtk_setup failed, err = %d", err);
+		return;
+	}
+	hci_reset_dev(hdev);
+
+	if (enable_autosuspend)
+		usb_enable_autosuspend(data->udev);
+
+	clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
+}
+
 static int btusb_recv_bulk_mtk(struct btusb_data *data, void *buffer, int count)
 {
 	struct sk_buff *skb;
@@ -4736,6 +4880,7 @@ static int btusb_probe(struct usb_interface *intf,
 		hdev->shutdown = btusb_mtk_shutdown;
 		hdev->manufacturer = 70;
 		data->recv_bulk = btusb_recv_bulk_mtk;
+		hdev->cmd_timeout = btusb_mtk_cmd_timeout;
 		set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
 	}
 
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/2] Bluetooth: btusb: Record debug log for Mediatek Chip.

[Marcel Holtmann]

Hi Mark,

> 1. Add btusb_recv_bulk_mtk to receive debug log.
> 2. Send the debug log via hci_recv_diag channel.
> 3. The upper layerd use hci_channel_minitor to receive
>   these packets.
> 
> Signed-off-by: mark-yw.chen <mark-yw.chen@mediatek.com>
> ---
> drivers/bluetooth/btusb.c | 67 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index b015dcecfb13..2a55ae0a5f8c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3851,6 +3851,72 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
> 	return 0;
> }
> 
> +static int btusb_recv_bulk_mtk(struct btusb_data *data, void *buffer, int count)
> +{
> +	struct sk_buff *skb;
> +	unsigned long flags;
> +	int err = 0;
> +
> +	spin_lock_irqsave(&data->rxlock, flags);
> +	skb = data->acl_skb;
> +
> +	while (count) {
> +		int len;
> +
> +		if (!skb) {
> +			skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
> +			if (!skb) {
> +				err = -ENOMEM;
> +				break;
> +			}
> +
> +			hci_skb_pkt_type(skb) = HCI_ACLDATA_PKT;
> +			hci_skb_expect(skb) = HCI_ACL_HDR_SIZE;
> +		}
> +
> +		len = min_t(uint, hci_skb_expect(skb), count);
> +		skb_put_data(skb, buffer, len);
> +
> +		count -= len;
> +		buffer += len;
> +		hci_skb_expect(skb) -= len;
> +
> +		if (skb->len == HCI_ACL_HDR_SIZE) {
> +			__le16 dlen = hci_acl_hdr(skb)->dlen;
> +
> +			/* Complete ACL header */
> +			hci_skb_expect(skb) = __le16_to_cpu(dlen);
> +
> +			if (skb_tailroom(skb) < hci_skb_expect(skb)) {
> +				kfree_skb(skb);
> +				skb = NULL;
> +
> +				err = -EILSEQ;
> +				break;
> +			}
> +		}
> +
> +		if (!hci_skb_expect(skb)) {
> +			/* Complete frame */
> +			if (skb->data[0] == 0x6f && skb->data[1] == 0xfc) {
> +				usb_disable_autosuspend(data->udev);
> +				hci_recv_diag(data->hdev, skb);
> +			} else if ((skb->data[0] == 0xfe || skb->data[0] == 0xff) &&
> +				   skb->data[1] == 0x05) {
> +				hci_recv_diag(data->hdev, skb);
> +			} else {
> +				hci_recv_frame(data->hdev, skb);
> +			}
> +			skb = NULL;
> +		}
> +	}
> +
> +	data->acl_skb = skb;
> +	spin_unlock_irqrestore(&data->rxlock, flags);
> +
> +	return err;
> +}
> +
> MODULE_FIRMWARE(FIRMWARE_MT7663);
> MODULE_FIRMWARE(FIRMWARE_MT7668);
> 
> @@ -4669,6 +4735,7 @@ static int btusb_probe(struct usb_interface *intf,
> 		hdev->setup = btusb_mtk_setup;
> 		hdev->shutdown = btusb_mtk_shutdown;
> 		hdev->manufacturer = 70;
> +		data->recv_bulk = btusb_recv_bulk_mtk;
> 		set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);

NO. The recv_bulk was meant for cases when bulk was faster and needed to redirected as interrupt/event data. Doing a complete copy of btusb_recv_bulk is not acceptable. You need to introduce data->recv_acl.

Regards

Marcel

Re: [PATCH 1/2] Bluetooth: btusb: Record debug log for Mediatek Chip.

[Marcel Holtmann]

Hi Mark,

> 1. Add btusb_recv_bulk_mtk to receive debug log.
> 2. Send the debug log via hci_recv_diag channel.
> 3. The upper layerd use hci_channel_minitor to receive
>   these packets.
> 
> Signed-off-by: mark-yw.chen <mark-yw.chen@mediatek.com>
> ---
> drivers/bluetooth/btusb.c | 67 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index b015dcecfb13..2a55ae0a5f8c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3851,6 +3851,72 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
> 	return 0;
> }
> 
> +static int btusb_recv_bulk_mtk(struct btusb_data *data, void *buffer, int count)
> +{
> +	struct sk_buff *skb;
> +	unsigned long flags;
> +	int err = 0;
> +
> +	spin_lock_irqsave(&data->rxlock, flags);
> +	skb = data->acl_skb;
> +
> +	while (count) {
> +		int len;
> +
> +		if (!skb) {
> +			skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
> +			if (!skb) {
> +				err = -ENOMEM;
> +				break;
> +			}
> +
> +			hci_skb_pkt_type(skb) = HCI_ACLDATA_PKT;
> +			hci_skb_expect(skb) = HCI_ACL_HDR_SIZE;
> +		}
> +
> +		len = min_t(uint, hci_skb_expect(skb), count);
> +		skb_put_data(skb, buffer, len);
> +
> +		count -= len;
> +		buffer += len;
> +		hci_skb_expect(skb) -= len;
> +
> +		if (skb->len == HCI_ACL_HDR_SIZE) {
> +			__le16 dlen = hci_acl_hdr(skb)->dlen;
> +
> +			/* Complete ACL header */
> +			hci_skb_expect(skb) = __le16_to_cpu(dlen);
> +
> +			if (skb_tailroom(skb) < hci_skb_expect(skb)) {
> +				kfree_skb(skb);
> +				skb = NULL;
> +
> +				err = -EILSEQ;
> +				break;
> +			}
> +		}
> +
> +		if (!hci_skb_expect(skb)) {
> +			/* Complete frame */
> +			if (skb->data[0] == 0x6f && skb->data[1] == 0xfc) {
> +				usb_disable_autosuspend(data->udev);
> +				hci_recv_diag(data->hdev, skb);
> +			} else if ((skb->data[0] == 0xfe || skb->data[0] == 0xff) &&
> +				   skb->data[1] == 0x05) {
> +				hci_recv_diag(data->hdev, skb);
> +			} else {
> +				hci_recv_frame(data->hdev, skb);
> +			}
> +			skb = NULL;
> +		}
> +	}
> +
> +	data->acl_skb = skb;
> +	spin_unlock_irqrestore(&data->rxlock, flags);
> +
> +	return err;
> +}
> +
> MODULE_FIRMWARE(FIRMWARE_MT7663);
> MODULE_FIRMWARE(FIRMWARE_MT7668);
> 
> @@ -4669,6 +4735,7 @@ static int btusb_probe(struct usb_interface *intf,
> 		hdev->setup = btusb_mtk_setup;
> 		hdev->shutdown = btusb_mtk_shutdown;
> 		hdev->manufacturer = 70;
> +		data->recv_bulk = btusb_recv_bulk_mtk;
> 		set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);

NO. The recv_bulk was meant for cases when bulk was faster and needed to redirected as interrupt/event data. Doing a complete copy of btusb_recv_bulk is not acceptable. You need to introduce data->recv_acl.

Regards

Marcel


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 2/2] Bluetooth: btusb: Support Bluetooth Reset for Mediatek Chip.

[Marcel Holtmann]

Hi Mark,

> This change enable Mediatek reset mechanism via USB, it's a specific
> reset mechanism with Mediatek chips. To handle error recovery, it use
> the cmd_timoeut to reset Mediatek Bluetooth.
> 
> Signed-off-by: mark-yw.chen <mark-yw.chen@mediatek.com>
> ---
> drivers/bluetooth/btusb.c | 145 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 145 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 2a55ae0a5f8c..acc23afbfe0f 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3075,6 +3075,18 @@ static int btusb_shutdown_intel_new(struct hci_dev *hdev)
> 	return 0;
> }
> 
> +/* UHW CR mapping */
> +#define BT_MISC			0x70002510
> +#define BT_SUBSYS_RST		0x70002610
> +#define UDMA_INT_STA_BT		0x74000024
> +#define UDMA_INT_STA_BT1	0x74000308
> +#define BT_WDT_STATUS		0x740003A0
> +#define EP_RST_OPT		0x74011890
> +#define EP_RST_IN_OUT_OPT	0x00010001
> +#define BT_RST_DONE		0x00000100
> +#define BT_RESET_WAIT_MS	100
> +#define BT_RESET_NUM_TRIES	10
> +
> #define FIRMWARE_MT7663		"mediatek/mt7663pr2h.bin"
> #define FIRMWARE_MT7668		"mediatek/mt7668pr2h.bin"
> 
> @@ -3649,6 +3661,59 @@ static int btusb_mtk_func_query(struct hci_dev *hdev)
> 	return status;
> }
> 
> +static int btusb_mtk_uhw_reg_write(struct btusb_data *data, u32 reg, u32 val)
> +{
> +	int pipe, err, size = sizeof(u32);
> +	u8 buf[4];
> +
> +	buf[0] = (val & 0x00ff);
> +	buf[1] = ((val >> 8) & 0x00ff);
> +	buf[2] = ((val >> 16) & 0x00ff);
> +	buf[3] = ((val >> 24) & 0x00ff);

can we not just just put_unalined_le32 in these cases.

> +
> +	pipe = usb_sndctrlpipe(data->udev, 0);
> +	err = usb_control_msg(data->udev, pipe, 0x02,
> +			      0x5E,
> +			      reg >> 16, reg & 0xffff,
> +			      buf, size, USB_CTRL_SET_TIMEOUT);

The extra size variable is pointless.

> +	if (err < 0) {
> +		BT_ERR("Failed to write uhw reg(%d)", err);

Please use bt_dev_err.

> +		goto err_free_buf;
> +	}
> +
> +err_free_buf:
> +
> +	return err;
> +}
> +
> +static int btusb_mtk_uhw_reg_read(struct btusb_data *data, u32 reg, u32 *val)
> +{
> +	int pipe, err, size = sizeof(u32);
> +	void *buf;
> +
> +	buf = kzalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;

Pointless size variable. Just use the value 4.

> +
> +	pipe = usb_rcvctrlpipe(data->udev, 0);
> +	err = usb_control_msg(data->udev, pipe, 0x01,
> +			      0xDE,
> +			      reg >> 16, reg & 0xffff,
> +			      buf, size, USB_CTRL_SET_TIMEOUT);
> +	if (err < 0) {
> +		BT_ERR("Failed to read uhw reg(%d)", err);
> +		goto err_free_buf;
> +	}
> +
> +	*val = get_unaligned_le32(buf);
> +	BT_INFO("%s: reg=%x, value=0x%08x", __func__, reg, *val);
> +
> +err_free_buf:
> +	kfree(buf);
> +
> +	return err;
> +}
> +
> static int btusb_mtk_reg_read(struct btusb_data *data, u32 reg, u32 *val)
> {
> 	int pipe, err, size = sizeof(u32);
> @@ -3728,6 +3793,7 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
> 			 dev_id & 0xffff, (fw_version & 0xff) + 1);
> 		err = btusb_mtk_setup_firmware_79xx(hdev, fw_bin_name);
> 
> +		btusb_mtk_uhw_reg_write(data, EP_RST_OPT, EP_RST_IN_OUT_OPT);

Extra line below and comment above on what this is doing.

> 		/* Enable Bluetooth protocol */
> 		param = 1;
> 		wmt_params.op = BTMTK_WMT_FUNC_CTRL;
> @@ -3851,6 +3917,84 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
> 	return 0;
> }
> 
> +static void btusb_mtk_cmd_timeout(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = hci_get_drvdata(hdev);
> +	u32 val;
> +	int err, retry = 0;
> +
> +	/*
> +	 * Toggle the bluetooth reset via usb. It's mediatek spicific reset
> +	 * mechanism, and the reset flow won't replug the USB. Thus, it must
> +	 * handle the urb transaction, such as stop_traffic and submit_urb.
> +	 * After bluetooth reset completed, we will re-download the firmware
> +	 * and send hw_err to host.
> +	 */

Wrong comment style.

> +	if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
> +		bt_dev_err(hdev, "last reset failed? Not resetting again");
> +		return;
> +	}
> +
> +	if (enable_autosuspend)
> +		usb_disable_autosuspend(data->udev);
> +
> +	data->sco_num = 0;
> +
> +	btusb_stop_traffic(data);
> +	usb_kill_anchored_urbs(&data->tx_anchor);
> +
> +	/* For reset */
> +	bt_dev_info(hdev, "Initiating reset mechanism via uhw");

Extra empty line.

> +	/*
> +	 * Todo - add error handling for usb no response case and uhw
> +	 * something worng.
> +	 */

Wrong comment style.

> +	btusb_mtk_uhw_reg_write(data, EP_RST_OPT, EP_RST_IN_OUT_OPT);
> +	btusb_mtk_uhw_reg_read(data, BT_WDT_STATUS, &val);
> +
> +	/* Write Reset CR to 1 */
> +	btusb_mtk_uhw_reg_write(data, BT_SUBSYS_RST, 1);
> +
> +	btusb_mtk_uhw_reg_write(data, UDMA_INT_STA_BT, 0x000000FF);
> +	btusb_mtk_uhw_reg_read(data, UDMA_INT_STA_BT, &val);
> +	btusb_mtk_uhw_reg_write(data, UDMA_INT_STA_BT1, 0x000000FF);
> +	btusb_mtk_uhw_reg_read(data, UDMA_INT_STA_BT1, &val);
> +	msleep(1);
> +
> +	/* Write Reset CR to 0 */
> +	btusb_mtk_uhw_reg_write(data, BT_SUBSYS_RST, 0);
> +
> +	/* Read reset CR */
> +	btusb_mtk_uhw_reg_read(data, BT_SUBSYS_RST, &val);
> +
> +	do {
> +		btusb_mtk_uhw_reg_read(data, BT_MISC, &val);
> +		if ((val & 0x00000100) == BT_RST_DONE) {
> +			bt_dev_info(hdev, "Bluetooth Reset Sucessfully");
> +			break;
> +		} else
> +			bt_dev_dbg(hdev, "Polling Bluetooth Reset CR");

Close here as well with { }.

> +
> +		retry++;
> +		msleep(BT_RESET_WAIT_MS);
> +	} while (retry < BT_RESET_NUM_TRIES);
> +
> +	btusb_mtk_id_get(data, 0x70010200, &val);
> +	bt_dev_info(hdev, "device id (%x)", val);
> +
> +	err = btusb_mtk_setup(hdev);
> +	if (err < 0) {
> +		bt_dev_err(hdev, "mtk_setup failed, err = %d", err);
> +		return;
> +	}
> +	hci_reset_dev(hdev);
> +
> +	if (enable_autosuspend)
> +		usb_enable_autosuspend(data->udev);
> +
> +	clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
> +}
> +
> static int btusb_recv_bulk_mtk(struct btusb_data *data, void *buffer, int count)
> {
> 	struct sk_buff *skb;
> @@ -4736,6 +4880,7 @@ static int btusb_probe(struct usb_interface *intf,
> 		hdev->shutdown = btusb_mtk_shutdown;
> 		hdev->manufacturer = 70;
> 		data->recv_bulk = btusb_recv_bulk_mtk;
> +		hdev->cmd_timeout = btusb_mtk_cmd_timeout;
> 		set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
> 	}

Regards

Marcel

Re: [PATCH 2/2] Bluetooth: btusb: Support Bluetooth Reset for Mediatek Chip.

[Marcel Holtmann]

Hi Mark,

> This change enable Mediatek reset mechanism via USB, it's a specific
> reset mechanism with Mediatek chips. To handle error recovery, it use
> the cmd_timoeut to reset Mediatek Bluetooth.
> 
> Signed-off-by: mark-yw.chen <mark-yw.chen@mediatek.com>
> ---
> drivers/bluetooth/btusb.c | 145 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 145 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 2a55ae0a5f8c..acc23afbfe0f 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3075,6 +3075,18 @@ static int btusb_shutdown_intel_new(struct hci_dev *hdev)
> 	return 0;
> }
> 
> +/* UHW CR mapping */
> +#define BT_MISC			0x70002510
> +#define BT_SUBSYS_RST		0x70002610
> +#define UDMA_INT_STA_BT		0x74000024
> +#define UDMA_INT_STA_BT1	0x74000308
> +#define BT_WDT_STATUS		0x740003A0
> +#define EP_RST_OPT		0x74011890
> +#define EP_RST_IN_OUT_OPT	0x00010001
> +#define BT_RST_DONE		0x00000100
> +#define BT_RESET_WAIT_MS	100
> +#define BT_RESET_NUM_TRIES	10
> +
> #define FIRMWARE_MT7663		"mediatek/mt7663pr2h.bin"
> #define FIRMWARE_MT7668		"mediatek/mt7668pr2h.bin"
> 
> @@ -3649,6 +3661,59 @@ static int btusb_mtk_func_query(struct hci_dev *hdev)
> 	return status;
> }
> 
> +static int btusb_mtk_uhw_reg_write(struct btusb_data *data, u32 reg, u32 val)
> +{
> +	int pipe, err, size = sizeof(u32);
> +	u8 buf[4];
> +
> +	buf[0] = (val & 0x00ff);
> +	buf[1] = ((val >> 8) & 0x00ff);
> +	buf[2] = ((val >> 16) & 0x00ff);
> +	buf[3] = ((val >> 24) & 0x00ff);

can we not just just put_unalined_le32 in these cases.

> +
> +	pipe = usb_sndctrlpipe(data->udev, 0);
> +	err = usb_control_msg(data->udev, pipe, 0x02,
> +			      0x5E,
> +			      reg >> 16, reg & 0xffff,
> +			      buf, size, USB_CTRL_SET_TIMEOUT);

The extra size variable is pointless.

> +	if (err < 0) {
> +		BT_ERR("Failed to write uhw reg(%d)", err);

Please use bt_dev_err.

> +		goto err_free_buf;
> +	}
> +
> +err_free_buf:
> +
> +	return err;
> +}
> +
> +static int btusb_mtk_uhw_reg_read(struct btusb_data *data, u32 reg, u32 *val)
> +{
> +	int pipe, err, size = sizeof(u32);
> +	void *buf;
> +
> +	buf = kzalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;

Pointless size variable. Just use the value 4.

> +
> +	pipe = usb_rcvctrlpipe(data->udev, 0);
> +	err = usb_control_msg(data->udev, pipe, 0x01,
> +			      0xDE,
> +			      reg >> 16, reg & 0xffff,
> +			      buf, size, USB_CTRL_SET_TIMEOUT);
> +	if (err < 0) {
> +		BT_ERR("Failed to read uhw reg(%d)", err);
> +		goto err_free_buf;
> +	}
> +
> +	*val = get_unaligned_le32(buf);
> +	BT_INFO("%s: reg=%x, value=0x%08x", __func__, reg, *val);
> +
> +err_free_buf:
> +	kfree(buf);
> +
> +	return err;
> +}
> +
> static int btusb_mtk_reg_read(struct btusb_data *data, u32 reg, u32 *val)
> {
> 	int pipe, err, size = sizeof(u32);
> @@ -3728,6 +3793,7 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
> 			 dev_id & 0xffff, (fw_version & 0xff) + 1);
> 		err = btusb_mtk_setup_firmware_79xx(hdev, fw_bin_name);
> 
> +		btusb_mtk_uhw_reg_write(data, EP_RST_OPT, EP_RST_IN_OUT_OPT);

Extra line below and comment above on what this is doing.

> 		/* Enable Bluetooth protocol */
> 		param = 1;
> 		wmt_params.op = BTMTK_WMT_FUNC_CTRL;
> @@ -3851,6 +3917,84 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
> 	return 0;
> }
> 
> +static void btusb_mtk_cmd_timeout(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = hci_get_drvdata(hdev);
> +	u32 val;
> +	int err, retry = 0;
> +
> +	/*
> +	 * Toggle the bluetooth reset via usb. It's mediatek spicific reset
> +	 * mechanism, and the reset flow won't replug the USB. Thus, it must
> +	 * handle the urb transaction, such as stop_traffic and submit_urb.
> +	 * After bluetooth reset completed, we will re-download the firmware
> +	 * and send hw_err to host.
> +	 */

Wrong comment style.

> +	if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
> +		bt_dev_err(hdev, "last reset failed? Not resetting again");
> +		return;
> +	}
> +
> +	if (enable_autosuspend)
> +		usb_disable_autosuspend(data->udev);
> +
> +	data->sco_num = 0;
> +
> +	btusb_stop_traffic(data);
> +	usb_kill_anchored_urbs(&data->tx_anchor);
> +
> +	/* For reset */
> +	bt_dev_info(hdev, "Initiating reset mechanism via uhw");

Extra empty line.

> +	/*
> +	 * Todo - add error handling for usb no response case and uhw
> +	 * something worng.
> +	 */

Wrong comment style.

> +	btusb_mtk_uhw_reg_write(data, EP_RST_OPT, EP_RST_IN_OUT_OPT);
> +	btusb_mtk_uhw_reg_read(data, BT_WDT_STATUS, &val);
> +
> +	/* Write Reset CR to 1 */
> +	btusb_mtk_uhw_reg_write(data, BT_SUBSYS_RST, 1);
> +
> +	btusb_mtk_uhw_reg_write(data, UDMA_INT_STA_BT, 0x000000FF);
> +	btusb_mtk_uhw_reg_read(data, UDMA_INT_STA_BT, &val);
> +	btusb_mtk_uhw_reg_write(data, UDMA_INT_STA_BT1, 0x000000FF);
> +	btusb_mtk_uhw_reg_read(data, UDMA_INT_STA_BT1, &val);
> +	msleep(1);
> +
> +	/* Write Reset CR to 0 */
> +	btusb_mtk_uhw_reg_write(data, BT_SUBSYS_RST, 0);
> +
> +	/* Read reset CR */
> +	btusb_mtk_uhw_reg_read(data, BT_SUBSYS_RST, &val);
> +
> +	do {
> +		btusb_mtk_uhw_reg_read(data, BT_MISC, &val);
> +		if ((val & 0x00000100) == BT_RST_DONE) {
> +			bt_dev_info(hdev, "Bluetooth Reset Sucessfully");
> +			break;
> +		} else
> +			bt_dev_dbg(hdev, "Polling Bluetooth Reset CR");

Close here as well with { }.

> +
> +		retry++;
> +		msleep(BT_RESET_WAIT_MS);
> +	} while (retry < BT_RESET_NUM_TRIES);
> +
> +	btusb_mtk_id_get(data, 0x70010200, &val);
> +	bt_dev_info(hdev, "device id (%x)", val);
> +
> +	err = btusb_mtk_setup(hdev);
> +	if (err < 0) {
> +		bt_dev_err(hdev, "mtk_setup failed, err = %d", err);
> +		return;
> +	}
> +	hci_reset_dev(hdev);
> +
> +	if (enable_autosuspend)
> +		usb_enable_autosuspend(data->udev);
> +
> +	clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
> +}
> +
> static int btusb_recv_bulk_mtk(struct btusb_data *data, void *buffer, int count)
> {
> 	struct sk_buff *skb;
> @@ -4736,6 +4880,7 @@ static int btusb_probe(struct usb_interface *intf,
> 		hdev->shutdown = btusb_mtk_shutdown;
> 		hdev->manufacturer = 70;
> 		data->recv_bulk = btusb_recv_bulk_mtk;
> +		hdev->cmd_timeout = btusb_mtk_cmd_timeout;
> 		set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
> 	}

Regards

Marcel


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek