[PATCH v2 1/2] Bluetooth: btusb: mediatek: refactor btusb_mtk_reset function

[PATCH v2 1/2] Bluetooth: btusb: mediatek: refactor btusb_mtk_reset function

[Hao Qin]

From: "hao.qin" <hao.qin@mediatek.com>

Extract function btusb_mtk_subsys_reset from btusb_mtk_reset
for future handling of resetting bluetooth controller without
USB reset.

Signed-off-by: hao.qin <hao.qin@mediatek.com>

---
V1 -> V2: refactor btusb_mtk_reset function

Link:
  https://lore.kernel.org/all/20231215063714.7684-1-hao.qin@mediatek.com/
---
 drivers/bluetooth/btusb.c | 45 +++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 0926e4451802..abefcd1a089d 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2994,28 +2994,13 @@ static u32 btusb_mtk_reset_done(struct hci_dev *hdev)
 	return val & MTK_BT_RST_DONE;
 }
 
-static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
+static int btusb_mtk_subsys_reset(struct hci_dev *hdev, u32 dev_id)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
-	struct btmediatek_data *mediatek;
 	u32 val;
 	int err;
 
-	/* It's MediaTek specific bluetooth reset mechanism via USB */
-	if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
-		bt_dev_err(hdev, "last reset failed? Not resetting again");
-		return -EBUSY;
-	}
-
-	err = usb_autopm_get_interface(data->intf);
-	if (err < 0)
-		return err;
-
-	btusb_stop_traffic(data);
-	usb_kill_anchored_urbs(&data->tx_anchor);
-	mediatek = hci_get_priv(hdev);
-
-	if (mediatek->dev_id == 0x7925) {
+	if (dev_id == 0x7925) {
 		btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
 		val |= (1 << 5);
 		btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
@@ -3059,8 +3044,32 @@ static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
 	if (!val)
 		bt_dev_err(hdev, "Can't get device id, subsys reset fail.");
 
-	usb_queue_reset_device(data->intf);
+	return err;
+}
 
+static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
+{
+	struct btusb_data *data = hci_get_drvdata(hdev);
+	struct btmediatek_data *mediatek;
+	int err;
+
+	/* It's MediaTek specific bluetooth reset mechanism via USB */
+	if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
+		bt_dev_err(hdev, "last reset failed? Not resetting again");
+		return -EBUSY;
+	}
+
+	err = usb_autopm_get_interface(data->intf);
+	if (err < 0)
+		return err;
+
+	btusb_stop_traffic(data);
+	usb_kill_anchored_urbs(&data->tx_anchor);
+	mediatek = hci_get_priv(hdev);
+
+	err = btusb_mtk_subsys_reset(hdev, mediatek->dev_id);
+
+	usb_queue_reset_device(data->intf);
 	clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
 
 	return err;
-- 
2.18.0

[PATCH v2 2/2] Bluetooth: btusb: mediatek: add a recovery method for MT7922 and MT7925

[Hao Qin]

From: "hao.qin" <hao.qin@mediatek.com>

For MT7922 and MT7925, add USB reset retry to recover from patch
download failure, and perform a reset before patch download to
avoid unexpected problems caused by unkonwn status of dongle.

Signed-off-by: hao.qin <hao.qin@mediatek.com>
---
 drivers/bluetooth/btusb.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index abefcd1a089d..26ad4864d06c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3000,17 +3000,26 @@ static int btusb_mtk_subsys_reset(struct hci_dev *hdev, u32 dev_id)
 	u32 val;
 	int err;
 
-	if (dev_id == 0x7925) {
+	if (dev_id == 0x7922) {
+		btusb_mtk_uhw_reg_read(data, MTK_BT_SUBSYS_RST, &val);
+		val |= 0x00002020;
+		btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, val);
+		btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, 0x00010001);
+		btusb_mtk_uhw_reg_read(data, MTK_BT_SUBSYS_RST, &val);
+		val |= BIT(0);
+		btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, val);
+		msleep(100);
+	} else if (dev_id == 0x7925) {
 		btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
-		val |= (1 << 5);
+		val |= BIT(5);
 		btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
 		btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
 		val &= 0xFFFF00FF;
-		val |= (1 << 13);
+		val |= BIT(13);
 		btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
 		btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, 0x00010001);
 		btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
-		val |= (1 << 0);
+		val |= BIT(0);
 		btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
 		btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT, 0x000000FF);
 		btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT, &val);
@@ -3040,6 +3049,9 @@ static int btusb_mtk_subsys_reset(struct hci_dev *hdev, u32 dev_id)
 	if (err < 0)
 		bt_dev_err(hdev, "Reset timeout");
 
+	if (dev_id == 0x7922)
+		btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT, 0x000000FF);
+
 	btusb_mtk_id_get(data, 0x70010200, &val);
 	if (!val)
 		bt_dev_err(hdev, "Can't get device id, subsys reset fail.");
@@ -3128,8 +3140,10 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
 		fwname = FIRMWARE_MT7668;
 		break;
 	case 0x7922:
-	case 0x7961:
 	case 0x7925:
+		btusb_mtk_subsys_reset(hdev, dev_id);
+		fallthrough;
+	case 0x7961:
 		if (dev_id == 0x7925)
 			snprintf(fw_bin_name, sizeof(fw_bin_name),
 				 "mediatek/mt%04x/BT_RAM_CODE_MT%04x_1_%x_hdr.bin",
@@ -3143,6 +3157,11 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
 						btusb_mtk_hci_wmt_sync);
 		if (err < 0) {
 			bt_dev_err(hdev, "Failed to set up firmware (%d)", err);
+			if (dev_id == 0x7922 || dev_id == 0x7925) {
+				btusb_stop_traffic(data);
+				usb_kill_anchored_urbs(&data->tx_anchor);
+				usb_queue_reset_device(data->intf);
+			}
 			return err;
 		}
 
-- 
2.18.0

Re: [PATCH v2 1/2] Bluetooth: btusb: mediatek: refactor btusb_mtk_reset function

[Sean Wang]

On Tue, Jan 2, 2024 at 6:49 AM Hao Qin <hao.qin@mediatek.com> wrote:
>
> From: "hao.qin" <hao.qin@mediatek.com>
>
> Extract function btusb_mtk_subsys_reset from btusb_mtk_reset
> for future handling of resetting bluetooth controller without
> USB reset.
>
> Signed-off-by: hao.qin <hao.qin@mediatek.com>
>

The patch doesn't modify any logic; it simply divides the existing
logic into two parts,
each prepared to be called in a specific context. Thus,  Acked-by:
Sean Wang <sean.wang@kernel.org>

> ---
> V1 -> V2: refactor btusb_mtk_reset function
>
> Link:
>   https://lore.kernel.org/all/20231215063714.7684-1-hao.qin@mediatek.com/
> ---
>  drivers/bluetooth/btusb.c | 45 +++++++++++++++++++++++----------------
>  1 file changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 0926e4451802..abefcd1a089d 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2994,28 +2994,13 @@ static u32 btusb_mtk_reset_done(struct hci_dev *hdev)
>         return val & MTK_BT_RST_DONE;
>  }
>
> -static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
> +static int btusb_mtk_subsys_reset(struct hci_dev *hdev, u32 dev_id)
>  {
>         struct btusb_data *data = hci_get_drvdata(hdev);
> -       struct btmediatek_data *mediatek;
>         u32 val;
>         int err;
>
> -       /* It's MediaTek specific bluetooth reset mechanism via USB */
> -       if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
> -               bt_dev_err(hdev, "last reset failed? Not resetting again");
> -               return -EBUSY;
> -       }
> -
> -       err = usb_autopm_get_interface(data->intf);
> -       if (err < 0)
> -               return err;
> -
> -       btusb_stop_traffic(data);
> -       usb_kill_anchored_urbs(&data->tx_anchor);
> -       mediatek = hci_get_priv(hdev);
> -
> -       if (mediatek->dev_id == 0x7925) {
> +       if (dev_id == 0x7925) {
>                 btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
>                 val |= (1 << 5);
>                 btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
> @@ -3059,8 +3044,32 @@ static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
>         if (!val)
>                 bt_dev_err(hdev, "Can't get device id, subsys reset fail.");
>
> -       usb_queue_reset_device(data->intf);
> +       return err;
> +}
>
> +static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
> +{
> +       struct btusb_data *data = hci_get_drvdata(hdev);
> +       struct btmediatek_data *mediatek;
> +       int err;
> +
> +       /* It's MediaTek specific bluetooth reset mechanism via USB */
> +       if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
> +               bt_dev_err(hdev, "last reset failed? Not resetting again");
> +               return -EBUSY;
> +       }
> +
> +       err = usb_autopm_get_interface(data->intf);
> +       if (err < 0)
> +               return err;
> +
> +       btusb_stop_traffic(data);
> +       usb_kill_anchored_urbs(&data->tx_anchor);
> +       mediatek = hci_get_priv(hdev);
> +
> +       err = btusb_mtk_subsys_reset(hdev, mediatek->dev_id);
> +
> +       usb_queue_reset_device(data->intf);
>         clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
>
>         return err;
> --
> 2.18.0
>
>

Re: [PATCH v2 2/2] Bluetooth: btusb: mediatek: add a recovery method for MT7922 and MT7925

[Sean Wang]

On Tue, Jan 2, 2024 at 6:49 AM Hao Qin <hao.qin@mediatek.com> wrote:
>
> From: "hao.qin" <hao.qin@mediatek.com>
>
> For MT7922 and MT7925, add USB reset retry to recover from patch
> download failure, and perform a reset before patch download to
> avoid unexpected problems caused by unkonwn status of dongle.
>
> Signed-off-by: hao.qin <hao.qin@mediatek.com>
> ---
>  drivers/bluetooth/btusb.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index abefcd1a089d..26ad4864d06c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3000,17 +3000,26 @@ static int btusb_mtk_subsys_reset(struct hci_dev *hdev, u32 dev_id)
>         u32 val;
>         int err;
>
> -       if (dev_id == 0x7925) {
> +       if (dev_id == 0x7922) {
> +               btusb_mtk_uhw_reg_read(data, MTK_BT_SUBSYS_RST, &val);
> +               val |= 0x00002020;
> +               btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, val);
> +               btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, 0x00010001);
> +               btusb_mtk_uhw_reg_read(data, MTK_BT_SUBSYS_RST, &val);
> +               val |= BIT(0);
> +               btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, val);
> +               msleep(100);
> +       } else if (dev_id == 0x7925) {
>                 btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
> -               val |= (1 << 5);
> +               val |= BIT(5);
>                 btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
>                 btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
>                 val &= 0xFFFF00FF;
> -               val |= (1 << 13);
> +               val |= BIT(13);
>                 btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
>                 btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, 0x00010001);
>                 btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
> -               val |= (1 << 0);
> +               val |= BIT(0);
>                 btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
>                 btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT, 0x000000FF);
>                 btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT, &val);
> @@ -3040,6 +3049,9 @@ static int btusb_mtk_subsys_reset(struct hci_dev *hdev, u32 dev_id)
>         if (err < 0)
>                 bt_dev_err(hdev, "Reset timeout");
>
> +       if (dev_id == 0x7922)
> +               btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT, 0x000000FF);
> +
>         btusb_mtk_id_get(data, 0x70010200, &val);
>         if (!val)
>                 bt_dev_err(hdev, "Can't get device id, subsys reset fail.");
> @@ -3128,8 +3140,10 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
>                 fwname = FIRMWARE_MT7668;
>                 break;
>         case 0x7922:
> -       case 0x7961:
>         case 0x7925:
> +               btusb_mtk_subsys_reset(hdev, dev_id);

Is there any reason we cannot perform a subsystem reset for 0x7961?
I believe it would enhance robustness under any circumstance, such as
cold reboot, warm reboot, or suspension,
ensuring that the controller can be reset to its initial state

> +               fallthrough;
> +       case 0x7961:
>                 if (dev_id == 0x7925)
>                         snprintf(fw_bin_name, sizeof(fw_bin_name),
>                                  "mediatek/mt%04x/BT_RAM_CODE_MT%04x_1_%x_hdr.bin",
> @@ -3143,6 +3157,11 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
>                                                 btusb_mtk_hci_wmt_sync);
>                 if (err < 0) {
>                         bt_dev_err(hdev, "Failed to set up firmware (%d)", err);
> +                       if (dev_id == 0x7922 || dev_id == 0x7925) {

Similarly, is there any reason we cannot apply the same approach for
mt7961? Using a less specific condition would reduce maintenance
effort in the future.

> +                               btusb_stop_traffic(data);
> +                               usb_kill_anchored_urbs(&data->tx_anchor);
> +                               usb_queue_reset_device(data->intf);

If an error occurs during firmware download, I guess we can
subsequently attempt to download the firmware again through several
retries.
This helps the controller to autonomously recover from any potential
errors and ensure we can complete the initialization process.

> +                       }
>                         return err;
>                 }
>
> --
> 2.18.0
>
>