From: "hao.qin" <[email protected]>
For MT7922, perform a reset before patch download to avoid
unexpected problems caused by inconsistency between upper layer
status and dongle status. Add USB reset retry to recover from
unexpected patch download failure.
Signed-off-by: hao.qin <[email protected]>
---
drivers/bluetooth/btusb.c | 66 +++++++++++++++++++++++++++------------
1 file changed, 46 insertions(+), 20 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 0926e4451802..778e1a0d9cae 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2812,7 +2812,7 @@ static int btusb_mtk_hci_wmt_sync(struct hci_dev *hdev,
* WMT command.
*/
err = wait_on_bit_timeout(&data->flags, BTUSB_TX_WAIT_VND_EVT,
- TASK_INTERRUPTIBLE, HCI_INIT_TIMEOUT);
+ TASK_INTERRUPTIBLE, HCI_CMD_TIMEOUT);
if (err == -EINTR) {
bt_dev_err(hdev, "Execution of wmt command interrupted");
clear_bit(BTUSB_TX_WAIT_VND_EVT, &data->flags);
@@ -2994,28 +2994,22 @@ 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 == 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 |= (1 << 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);
btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
@@ -3053,14 +3047,41 @@ static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
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");
+ bt_dev_err(hdev, "Subsys 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.");
- 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;
@@ -3119,6 +3140,8 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
fwname = FIRMWARE_MT7668;
break;
case 0x7922:
+ btusb_mtk_subsys_reset(hdev, dev_id);
+ fallthrough;
case 0x7961:
case 0x7925:
if (dev_id == 0x7925)
@@ -3134,6 +3157,9 @@ 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);
+ btusb_stop_traffic(data);
+ usb_kill_anchored_urbs(&data->tx_anchor);
+ usb_queue_reset_device(data->intf);
return err;
}
--
2.18.0
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=810339
---Test result---
Test Summary:
CheckPatch PASS 0.68 seconds
GitLint PASS 0.68 seconds
SubjectPrefix PASS 0.13 seconds
BuildKernel PASS 27.89 seconds
CheckAllWarning PASS 30.45 seconds
CheckSparse PASS 35.75 seconds
CheckSmatch PASS 99.39 seconds
BuildKernel32 PASS 27.14 seconds
TestRunnerSetup PASS 424.70 seconds
TestRunner_l2cap-tester PASS 23.16 seconds
TestRunner_iso-tester PASS 51.26 seconds
TestRunner_bnep-tester PASS 6.98 seconds
TestRunner_mgmt-tester FAIL 164.58 seconds
TestRunner_rfcomm-tester PASS 10.89 seconds
TestRunner_sco-tester PASS 14.40 seconds
TestRunner_ioctl-tester PASS 11.99 seconds
TestRunner_mesh-tester PASS 8.82 seconds
TestRunner_smp-tester PASS 9.79 seconds
TestRunner_userchan-tester PASS 7.40 seconds
IncrementalBuild PASS 25.50 seconds
Details
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 497, Passed: 495 (99.6%), Failed: 1, Not Run: 1
Failed Test Cases
LL Privacy - Remove Device 4 (Disable Adv) Timed out 2.189 seconds
---
Regards,
Linux Bluetooth
Hi Hao,
On Fri, Dec 15, 2023 at 12:40 AM Hao Qin <[email protected]> wrote:
>
> From: "hao.qin" <[email protected]>
>
> For MT7922, perform a reset before patch download to avoid
> unexpected problems caused by inconsistency between upper layer
> status and dongle status. Add USB reset retry to recover from
> unexpected patch download failure.
After looking through the patch, I guess the patch providing the reset
mechanism for
1) the core layer to call when cmd is timeout
2) resetting the device before the patch download
3) handling situations where something goes wrong during the patch
download phase
right ? if so, the description should be revised to make the patch
close to what it does
>
> Signed-off-by: hao.qin <[email protected]>
> ---
> drivers/bluetooth/btusb.c | 66 +++++++++++++++++++++++++++------------
> 1 file changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 0926e4451802..778e1a0d9cae 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2812,7 +2812,7 @@ static int btusb_mtk_hci_wmt_sync(struct hci_dev *hdev,
> * WMT command.
> */
> err = wait_on_bit_timeout(&data->flags, BTUSB_TX_WAIT_VND_EVT,
> - TASK_INTERRUPTIBLE, HCI_INIT_TIMEOUT);
> + TASK_INTERRUPTIBLE, HCI_CMD_TIMEOUT);
The changes are not specific to MT7922 and do not align with the
commit description. Kindly consider moving them to another patch and
provide clear reasons why they are necessary for MT7922 or any other
chipsets.
> if (err == -EINTR) {
> bt_dev_err(hdev, "Execution of wmt command interrupted");
> clear_bit(BTUSB_TX_WAIT_VND_EVT, &data->flags);
> @@ -2994,28 +2994,22 @@ 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);
> -
It seems that you refactored the existing btusb_mtk_reset in the
patch. Could you please create another patch specifically for the
refactoring without introducing any functional changes of MT7922 prior
to adding the patch? This way would be good for ease of review and
tracking in the future. Additionally, it ensures that those working on
MT7921 are aware of the patch and its modifications.
> - if (mediatek->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 |= (1 << 0);
We can utilize BIT(0) instead of 1 << 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);
We can utilize BIT(5) instead of 1 << 5
> btusb_mtk_uhw_reg_write(data, MTK_BT_RESET_REG_CONNV3, val);
> @@ -3053,14 +3047,41 @@ static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
> 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");
> + bt_dev_err(hdev, "Subsys reset timeout");
I guess it would be better to avoid the unnecessary change unrelated
to the patch.
> +
> + 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.");
>
> - 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;
> @@ -3119,6 +3140,8 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
> fwname = FIRMWARE_MT7668;
> break;
> case 0x7922:
> + btusb_mtk_subsys_reset(hdev, dev_id);
> + fallthrough;
> case 0x7961:
> case 0x7925:
> if (dev_id == 0x7925)
> @@ -3134,6 +3157,9 @@ 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);
> + btusb_stop_traffic(data);
> + usb_kill_anchored_urbs(&data->tx_anchor);
> + usb_queue_reset_device(data->intf);
The change doesn't appear to be specific to MT7922, as the commit
description suggests. Please consider splitting it into another patch
to address any existing chipset issues, if necessary.
> return err;
> }
>
> --
> 2.18.0
>
>