2023-04-26 23:09:22

by Sean Wang

[permalink] [raw]
Subject: [PATCH v4 2/3] Bluetooth: btmtk: introduce btmtk reset work

From: Jing Cai <[email protected]>

Introduce btmtk_reset_work which can be called whenever the firmware abort,
HCI command timeout, other fatal error happen.

Co-developed-by: Sean Wang <[email protected]>
Signed-off-by: Sean Wang <[email protected]>
Signed-off-by: Jing Cai <[email protected]>
---
v2, v3, v4: rebase onto the latest codebase
---
drivers/bluetooth/btmtk.c | 16 ++++++
drivers/bluetooth/btmtk.h | 14 +++++
drivers/bluetooth/btusb.c | 106 ++++++++++++++++++++------------------
3 files changed, 86 insertions(+), 50 deletions(-)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 809762d64fc6..77df7b5c3ef3 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -53,6 +53,8 @@ struct btmtk_section_map {
};
} __packed;

+static struct btmtk_reset_work reset_work;
+
int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
wmt_cmd_sync_func_t wmt_cmd_sync)
{
@@ -280,6 +282,20 @@ int btmtk_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
}
EXPORT_SYMBOL_GPL(btmtk_set_bdaddr);

+void btmtk_init_reset_work(struct hci_dev *hdev, work_func_t func)
+{
+ reset_work.hdev = hdev;
+ INIT_WORK(&reset_work.work, func);
+}
+EXPORT_SYMBOL_GPL(btmtk_init_reset_work);
+
+void btmtk_reset_sync(struct hci_dev *hdev)
+{
+ schedule_work(&reset_work.work);
+ flush_work(&reset_work.work);
+}
+EXPORT_SYMBOL_GPL(btmtk_reset_sync);
+
MODULE_AUTHOR("Sean Wang <[email protected]>");
MODULE_AUTHOR("Mark Chen <[email protected]>");
MODULE_DESCRIPTION("Bluetooth support for MediaTek devices ver " VERSION);
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index 2a88ea8e475e..22d39f637652 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -119,6 +119,11 @@ struct btmtk_hci_wmt_params {
u32 *status;
};

+struct btmtk_reset_work {
+ struct hci_dev *hdev;
+ struct work_struct work;
+};
+
typedef int (*wmt_cmd_sync_func_t)(struct hci_dev *,
struct btmtk_hci_wmt_params *);

@@ -131,6 +136,8 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,

int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
wmt_cmd_sync_func_t wmt_cmd_sync);
+void btmtk_init_reset_work(struct hci_dev *hdev, work_func_t func);
+void btmtk_reset_sync(struct hci_dev *hdev);
#else

static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
@@ -151,4 +158,11 @@ static int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
return -EOPNOTSUPP;
}

+static void btmtk_init_reset_work(struct hci_dev *hdev, work_func_t func)
+{
+}
+
+static void btmtk_reset_sync(struct hci_dev *hdev)
+{
+}
#endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 275e0666503d..e84f1a8d03a3 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2946,6 +2946,59 @@ static u32 btusb_mtk_reset_done(struct hci_dev *hdev)
return val & MTK_BT_RST_DONE;
}

+static void btusb_mtk_reset_work(struct work_struct *work)
+{
+ struct btmtk_reset_work *info = container_of(work,
+ struct btmtk_reset_work,
+ work);
+ struct hci_dev *hdev = info->hdev;
+ struct btusb_data *data = hci_get_drvdata(hdev);
+ 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;
+ }
+
+ err = usb_autopm_get_interface(data->intf);
+ if (err < 0)
+ return;
+
+ btusb_stop_traffic(data);
+ usb_kill_anchored_urbs(&data->tx_anchor);
+
+ /* It's Device EndPoint Reset Option Register */
+ bt_dev_dbg(hdev, "Initiating reset mechanism via uhw");
+ btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, MTK_EP_RST_IN_OUT_OPT);
+ btusb_mtk_uhw_reg_read(data, MTK_BT_WDT_STATUS, &val);
+
+ /* Reset the bluetooth chip via USB interface. */
+ btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, 1);
+ btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT, 0x000000FF);
+ btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT, &val);
+ btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT1, 0x000000FF);
+ btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT1, &val);
+ /* MT7921 need to delay 20ms between toggle reset bit */
+ msleep(20);
+ btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, 0);
+ btusb_mtk_uhw_reg_read(data, MTK_BT_SUBSYS_RST, &val);
+
+ err = readx_poll_timeout(btusb_mtk_reset_done, hdev, val,
+ val & MTK_BT_RST_DONE, 20000, 1000000);
+ if (err < 0)
+ bt_dev_err(hdev, "Reset timeout");
+
+ btusb_mtk_id_get(data, 0x70010200, &val);
+ if (!val)
+ bt_dev_err(hdev, "Can't get device id, subsys reset fail.");
+
+ usb_queue_reset_device(data->intf);
+
+ clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
+}
+
static int btusb_mtk_setup(struct hci_dev *hdev)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -2982,6 +3035,8 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
}
}

+ btmtk_init_reset_work(hdev, btusb_mtk_reset_work);
+
switch (dev_id) {
case 0x7663:
fwname = FIRMWARE_MT7663;
@@ -3131,55 +3186,6 @@ 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;
-
- /* 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;
- }
-
- err = usb_autopm_get_interface(data->intf);
- if (err < 0)
- return;
-
- btusb_stop_traffic(data);
- usb_kill_anchored_urbs(&data->tx_anchor);
-
- /* It's Device EndPoint Reset Option Register */
- bt_dev_dbg(hdev, "Initiating reset mechanism via uhw");
- btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, MTK_EP_RST_IN_OUT_OPT);
- btusb_mtk_uhw_reg_read(data, MTK_BT_WDT_STATUS, &val);
-
- /* Reset the bluetooth chip via USB interface. */
- btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, 1);
- btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT, 0x000000FF);
- btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT, &val);
- btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT1, 0x000000FF);
- btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT1, &val);
- /* MT7921 need to delay 20ms between toggle reset bit */
- msleep(20);
- btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, 0);
- btusb_mtk_uhw_reg_read(data, MTK_BT_SUBSYS_RST, &val);
-
- err = readx_poll_timeout(btusb_mtk_reset_done, hdev, val,
- val & MTK_BT_RST_DONE, 20000, 1000000);
- if (err < 0)
- bt_dev_err(hdev, "Reset timeout");
-
- btusb_mtk_id_get(data, 0x70010200, &val);
- if (!val)
- bt_dev_err(hdev, "Can't get device id, subsys reset fail.");
-
- usb_queue_reset_device(data->intf);
-
- clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
-}
-
static int btusb_recv_acl_mtk(struct hci_dev *hdev, struct sk_buff *skb)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -4301,7 +4307,7 @@ static int btusb_probe(struct usb_interface *intf,
hdev->setup = btusb_mtk_setup;
hdev->shutdown = btusb_mtk_shutdown;
hdev->manufacturer = 70;
- hdev->cmd_timeout = btusb_mtk_cmd_timeout;
+ hdev->cmd_timeout = btmtk_reset_sync;
hdev->set_bdaddr = btmtk_set_bdaddr;
set_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN, &hdev->quirks);
set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
--
2.25.1


2023-04-27 00:19:21

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] Bluetooth: btmtk: introduce btmtk reset work

Hi Sean,

On Wed, Apr 26, 2023 at 4:08 PM <[email protected]> wrote:
>
> From: Jing Cai <[email protected]>
>
> Introduce btmtk_reset_work which can be called whenever the firmware abort,
> HCI command timeout, other fatal error happen.
>
> Co-developed-by: Sean Wang <[email protected]>
> Signed-off-by: Sean Wang <[email protected]>
> Signed-off-by: Jing Cai <[email protected]>
> ---
> v2, v3, v4: rebase onto the latest codebase
> ---
> drivers/bluetooth/btmtk.c | 16 ++++++
> drivers/bluetooth/btmtk.h | 14 +++++
> drivers/bluetooth/btusb.c | 106 ++++++++++++++++++++------------------
> 3 files changed, 86 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> index 809762d64fc6..77df7b5c3ef3 100644
> --- a/drivers/bluetooth/btmtk.c
> +++ b/drivers/bluetooth/btmtk.c
> @@ -53,6 +53,8 @@ struct btmtk_section_map {
> };
> } __packed;
>
> +static struct btmtk_reset_work reset_work;
> +
> int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
> wmt_cmd_sync_func_t wmt_cmd_sync)
> {
> @@ -280,6 +282,20 @@ int btmtk_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> }
> EXPORT_SYMBOL_GPL(btmtk_set_bdaddr);
>
> +void btmtk_init_reset_work(struct hci_dev *hdev, work_func_t func)
> +{
> + reset_work.hdev = hdev;
> + INIT_WORK(&reset_work.work, func);
> +}
> +EXPORT_SYMBOL_GPL(btmtk_init_reset_work);
> +
> +void btmtk_reset_sync(struct hci_dev *hdev)
> +{
> + schedule_work(&reset_work.work);
> + flush_work(&reset_work.work);
> +}
> +EXPORT_SYMBOL_GPL(btmtk_reset_sync);

Lets stop assuming there is only one controller in the system, so
works like reset_work need to be store on a per hdev basis, not using
a global variable, also Im not sure why you would need dedicated work
for reset? Can't you use hci_cmd_sync_queue/hci_cmd_sync_submit?

> MODULE_AUTHOR("Sean Wang <[email protected]>");
> MODULE_AUTHOR("Mark Chen <[email protected]>");
> MODULE_DESCRIPTION("Bluetooth support for MediaTek devices ver " VERSION);
> diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> index 2a88ea8e475e..22d39f637652 100644
> --- a/drivers/bluetooth/btmtk.h
> +++ b/drivers/bluetooth/btmtk.h
> @@ -119,6 +119,11 @@ struct btmtk_hci_wmt_params {
> u32 *status;
> };
>
> +struct btmtk_reset_work {
> + struct hci_dev *hdev;
> + struct work_struct work;
> +};
> +
> typedef int (*wmt_cmd_sync_func_t)(struct hci_dev *,
> struct btmtk_hci_wmt_params *);
>
> @@ -131,6 +136,8 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
>
> int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
> wmt_cmd_sync_func_t wmt_cmd_sync);
> +void btmtk_init_reset_work(struct hci_dev *hdev, work_func_t func);
> +void btmtk_reset_sync(struct hci_dev *hdev);
> #else
>
> static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
> @@ -151,4 +158,11 @@ static int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
> return -EOPNOTSUPP;
> }
>
> +static void btmtk_init_reset_work(struct hci_dev *hdev, work_func_t func)
> +{
> +}
> +
> +static void btmtk_reset_sync(struct hci_dev *hdev)
> +{
> +}
> #endif
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 275e0666503d..e84f1a8d03a3 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2946,6 +2946,59 @@ static u32 btusb_mtk_reset_done(struct hci_dev *hdev)
> return val & MTK_BT_RST_DONE;
> }
>
> +static void btusb_mtk_reset_work(struct work_struct *work)
> +{
> + struct btmtk_reset_work *info = container_of(work,
> + struct btmtk_reset_work,
> + work);
> + struct hci_dev *hdev = info->hdev;
> + struct btusb_data *data = hci_get_drvdata(hdev);
> + 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;
> + }
> +
> + err = usb_autopm_get_interface(data->intf);
> + if (err < 0)
> + return;
> +
> + btusb_stop_traffic(data);
> + usb_kill_anchored_urbs(&data->tx_anchor);
> +
> + /* It's Device EndPoint Reset Option Register */
> + bt_dev_dbg(hdev, "Initiating reset mechanism via uhw");
> + btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, MTK_EP_RST_IN_OUT_OPT);
> + btusb_mtk_uhw_reg_read(data, MTK_BT_WDT_STATUS, &val);
> +
> + /* Reset the bluetooth chip via USB interface. */
> + btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, 1);
> + btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT, 0x000000FF);
> + btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT, &val);
> + btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT1, 0x000000FF);
> + btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT1, &val);
> + /* MT7921 need to delay 20ms between toggle reset bit */
> + msleep(20);
> + btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, 0);
> + btusb_mtk_uhw_reg_read(data, MTK_BT_SUBSYS_RST, &val);
> +
> + err = readx_poll_timeout(btusb_mtk_reset_done, hdev, val,
> + val & MTK_BT_RST_DONE, 20000, 1000000);
> + if (err < 0)
> + bt_dev_err(hdev, "Reset timeout");
> +
> + btusb_mtk_id_get(data, 0x70010200, &val);
> + if (!val)
> + bt_dev_err(hdev, "Can't get device id, subsys reset fail.");
> +
> + usb_queue_reset_device(data->intf);
> +
> + clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
> +}
> +
> static int btusb_mtk_setup(struct hci_dev *hdev)
> {
> struct btusb_data *data = hci_get_drvdata(hdev);
> @@ -2982,6 +3035,8 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
> }
> }
>
> + btmtk_init_reset_work(hdev, btusb_mtk_reset_work);
> +
> switch (dev_id) {
> case 0x7663:
> fwname = FIRMWARE_MT7663;
> @@ -3131,55 +3186,6 @@ 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;
> -
> - /* 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;
> - }
> -
> - err = usb_autopm_get_interface(data->intf);
> - if (err < 0)
> - return;
> -
> - btusb_stop_traffic(data);
> - usb_kill_anchored_urbs(&data->tx_anchor);
> -
> - /* It's Device EndPoint Reset Option Register */
> - bt_dev_dbg(hdev, "Initiating reset mechanism via uhw");
> - btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, MTK_EP_RST_IN_OUT_OPT);
> - btusb_mtk_uhw_reg_read(data, MTK_BT_WDT_STATUS, &val);
> -
> - /* Reset the bluetooth chip via USB interface. */
> - btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, 1);
> - btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT, 0x000000FF);
> - btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT, &val);
> - btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT1, 0x000000FF);
> - btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT1, &val);
> - /* MT7921 need to delay 20ms between toggle reset bit */
> - msleep(20);
> - btusb_mtk_uhw_reg_write(data, MTK_BT_SUBSYS_RST, 0);
> - btusb_mtk_uhw_reg_read(data, MTK_BT_SUBSYS_RST, &val);
> -
> - err = readx_poll_timeout(btusb_mtk_reset_done, hdev, val,
> - val & MTK_BT_RST_DONE, 20000, 1000000);
> - if (err < 0)
> - bt_dev_err(hdev, "Reset timeout");
> -
> - btusb_mtk_id_get(data, 0x70010200, &val);
> - if (!val)
> - bt_dev_err(hdev, "Can't get device id, subsys reset fail.");
> -
> - usb_queue_reset_device(data->intf);
> -
> - clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
> -}
> -
> static int btusb_recv_acl_mtk(struct hci_dev *hdev, struct sk_buff *skb)
> {
> struct btusb_data *data = hci_get_drvdata(hdev);
> @@ -4301,7 +4307,7 @@ static int btusb_probe(struct usb_interface *intf,
> hdev->setup = btusb_mtk_setup;
> hdev->shutdown = btusb_mtk_shutdown;
> hdev->manufacturer = 70;
> - hdev->cmd_timeout = btusb_mtk_cmd_timeout;
> + hdev->cmd_timeout = btmtk_reset_sync;
> hdev->set_bdaddr = btmtk_set_bdaddr;
> set_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN, &hdev->quirks);
> set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
> --
> 2.25.1
>


--
Luiz Augusto von Dentz