Re: [PATCH v7 2/3] Bluetooth: btmtk: introduce btmtk reset work

From: Luiz Augusto von Dentz
Date: Mon Jun 26 2023 - 16:49:23 EST


Hi Sean,

On Mon, Jun 26, 2023 at 1:41 PM <sean.wang@xxxxxxxxxxxx> wrote:
>
> From: Jing Cai <jing.cai@xxxxxxxxxxxx>
>
> Introduce btmtk_reset_work which can be called whenever the firmware abort,
> HCI command timeout, other fatal error happen.
>
> Co-developed-by: Chris Lu <chris.lu@xxxxxxxxxxxx>
> Signed-off-by: Chris Lu <chris.lu@xxxxxxxxxxxx>
> Co-developed-by: Sean Wang <sean.wang@xxxxxxxxxxxx>
> Signed-off-by: Sean Wang <sean.wang@xxxxxxxxxxxx>
> Signed-off-by: Jing Cai <jing.cai@xxxxxxxxxxxx>
> ---
> v2, v3, v4: rebase onto the latest codebase
> v5: reset the device on hdev basis and use hci_cmd_sync_queue to
> schedule reset work
> v6 and v7: rebase onto the latest codebase
> ---
> drivers/bluetooth/btmtk.c | 15 ++++
> drivers/bluetooth/btmtk.h | 8 +++
> drivers/bluetooth/btusb.c | 145 +++++++++++++++++++-------------------
> 3 files changed, 97 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> index 9482401d97fa..b8678b75276e 100644
> --- a/drivers/bluetooth/btmtk.c
> +++ b/drivers/bluetooth/btmtk.c
> @@ -280,6 +280,21 @@ int btmtk_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> }
> EXPORT_SYMBOL_GPL(btmtk_set_bdaddr);
>
> +void btmtk_reset_sync(struct hci_dev *hdev)
> +{
> + struct btmediatek_data *reset_work = hci_get_priv(hdev);
> + int err;
> +
> + hci_dev_lock(hdev);
> +
> + err = hci_cmd_sync_queue(hdev, reset_work->reset_sync, NULL, NULL);
> + if (err)
> + bt_dev_err(hdev, "failed to reset (%d)", err);
> +
> + hci_dev_unlock(hdev);
> +}
> +EXPORT_SYMBOL_GPL(btmtk_reset_sync);
> +
> MODULE_AUTHOR("Sean Wang <sean.wang@xxxxxxxxxxxx>");
> MODULE_AUTHOR("Mark Chen <mark-yw.chen@xxxxxxxxxxxx>");
> MODULE_DESCRIPTION("Bluetooth support for MediaTek devices ver " VERSION);
> diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> index fadc1a520652..75d8e71efcd3 100644
> --- a/drivers/bluetooth/btmtk.h
> +++ b/drivers/bluetooth/btmtk.h
> @@ -120,8 +120,11 @@ struct btmtk_hci_wmt_params {
> u32 *status;
> };
>
> +typedef int (*btmtk_reset_sync_func_t)(struct hci_dev *, void *);
> +
> struct btmediatek_data {
> u32 dev_id;
> + btmtk_reset_sync_func_t reset_sync;
> };
>
> typedef int (*wmt_cmd_sync_func_t)(struct hci_dev *,
> @@ -136,6 +139,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_reset_sync(struct hci_dev *hdev);
> #else
>
> static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
> @@ -156,4 +161,7 @@ static int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
> return -EOPNOTSUPP;
> }
>
> +static void btmtk_reset_sync(struct hci_dev *hdev)
> +{
> +}
> #endif
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 71b119bcff29..4ebab61c288c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3037,6 +3037,78 @@ static u32 btusb_mtk_reset_done(struct hci_dev *hdev)
> return val & MTK_BT_RST_DONE;
> }
>
> +static int btusb_mtk_reset_work(struct hci_dev *hdev, void *rst_data)

You can probably drop _work suffix since it no longer runs on its own
dedicated work.

> +{
> + 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) {
> + 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);
> + btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
> + val &= 0xFFFF00FF;
> + val |= (1 << 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);
> + 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);
> + btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT1, 0x000000FF);
> + btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT1, &val);
> + msleep(100);
> + } else {
> + /* 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);
> +
> + return err;
> +}
> +
> static int btusb_mtk_setup(struct hci_dev *hdev)
> {
> struct btusb_data *data = hci_get_drvdata(hdev);
> @@ -3076,6 +3148,7 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
>
> mediatek = hci_get_priv(hdev);
> mediatek->dev_id = dev_id;
> + mediatek->reset_sync = btusb_mtk_reset_work;
>
> switch (dev_id) {
> case 0x7663:
> @@ -3233,76 +3306,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;
> - struct btmediatek_data *mediatek;
> -
> - /* 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);
> - mediatek = hci_get_priv(hdev);
> -
> - if (mediatek->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);
> - btusb_mtk_uhw_reg_read(data, MTK_BT_RESET_REG_CONNV3, &val);
> - val &= 0xFFFF00FF;
> - val |= (1 << 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);
> - 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);
> - btusb_mtk_uhw_reg_write(data, MTK_UDMA_INT_STA_BT1, 0x000000FF);
> - btusb_mtk_uhw_reg_read(data, MTK_UDMA_INT_STA_BT1, &val);
> - msleep(100);
> - } else {
> - /* 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);
> @@ -4429,7 +4432,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