This patch seems to introduce a regression in the btmtksdio driver.
With this patch applied, I can't enable the hci0 interface anymore on mt8183-pumpkin:
i500-pumpkin login: root
root@i500-pumpkin:~# uname -a
Linux i500-pumpkin 5.14.0-rc3 #94 SMP PREEMPT Wed Jul 28 11:58:20 CEST 2021 aarch64 aarch64 aarch64 GNU/Linux
root@i500-pumpkin:~# hciconfig hci0 up
Can't init device hci0: Connection timed out (110)
root@i500-pumpkin:~# hciconfig hci0 down
root@i500-pumpkin:~# hciconfig hci0 up
Can't init device hci0: Input/output error (5)
Reverting it fixes the above issue.
Any suggestion on how to fix this without touching hci_core ?
Maybe the btmtksdio driver needs some rework. As I'm not familiar with the code, I would appreciate any tips.
Thanks,
Mattijs Korpershoek
This patch seems to introduce a regression in the btmtksdio driver.
With this patch applied, I can't enable the hci0 interface anymore on mt8183-pumpkin:
i500-pumpkin login: root
root@i500-pumpkin:~# uname -a
Linux i500-pumpkin 5.14.0-rc3 #94 SMP PREEMPT Wed Jul 28 11:58:20 CEST 2021 aarch64 aarch64 aarch64 GNU/Linux
root@i500-pumpkin:~# hciconfig hci0 up
Can't init device hci0: Connection timed out (110)
root@i500-pumpkin:~# hciconfig hci0 down
root@i500-pumpkin:~# hciconfig hci0 up
Can't init device hci0: Input/output error (5)
Reverting it fixes the above issue.
Any suggestion on how to fix this without touching hci_core ?
Maybe the btmtksdio driver needs some rework. As I'm not familiar with the code, I would appreciate any tips.
Can you please attach dmesg? Also, full ftrace log on btmtksdio can
also be helpful.
Kai-Heng
This patch seems to introduce a regression in the btmtksdio driver.
With this patch applied, I can't enable the hci0 interface anymore on mt8183-pumpkin:
i500-pumpkin login: root
root@i500-pumpkin:~# uname -a
Linux i500-pumpkin 5.14.0-rc3 #94 SMP PREEMPT Wed Jul 28 11:58:20 CEST 2021 aarch64 aarch64 aarch64 GNU/Linux
root@i500-pumpkin:~# hciconfig hci0 up
Can't init device hci0: Connection timed out (110)
root@i500-pumpkin:~# hciconfig hci0 down
root@i500-pumpkin:~# hciconfig hci0 up
Can't init device hci0: Input/output error (5)
Reverting it fixes the above issue.
Any suggestion on how to fix this without touching hci_core ?
Maybe the btmtksdio driver needs some rework. As I'm not familiar with the code, I would appreciate any tips.
Can you please attach dmesg? Also, full ftrace log on btmtksdio can
also be helpful.
Seems like btmtksdio needs shudown() to be called before flush().
Since the order was there for a very long time, changing the calling
order indeed can break what driver expects.
Can you please test the following patch:
Seems like btmtksdio needs shudown() to be called before flush().
Since the order was there for a very long time, changing the calling
order indeed can break what driver expects.
Can you please test the following patch:
@@ -1785,6 +1785,14 @@ int hci_dev_do_close(struct hci_dev *hdev)aosp_do_close(hdev);msft_do_close(hdev);+if(!hci_dev_test_flag(hdev,HCI_UNREGISTER)&&+!hci_dev_test_flag(hdev,HCI_USER_CHANNEL)&&+test_bit(HCI_UP,&hdev->flags)){+/* Execute vendor specific shutdown routine */+if(hdev->shutdown)+hdev->shutdown(hdev);+}+if(hdev->flush)hdev->flush(hdev);
@@ -1798,14 +1806,6 @@ int hci_dev_do_close(struct hci_dev *hdev)clear_bit(HCI_INIT,&hdev->flags);}-if(!hci_dev_test_flag(hdev,HCI_UNREGISTER)&&-!hci_dev_test_flag(hdev,HCI_USER_CHANNEL)&&-test_bit(HCI_UP,&hdev->flags)){-/* Execute vendor specific shutdown routine */-if(hdev->shutdown)-hdev->shutdown(hdev);-}-/* flush cmd work */flush_work(&hdev->cmd_work);
Thanks for the patch and your help.
I've tried it, but it seems that it does not improve for me.
I'm still observing:
i500-pumpkin login: root
root@i500-pumpkin:~# hciconfig hci0 up
Can't init device hci0: Connection timed out (110)
Logs for this session:
dmesg: https://pastebin.com/iAFk5Tzi
ftrace: https://pastebin.com/kEMWSYrE
Seems like btmtksdio needs shudown() to be called before flush().
Since the order was there for a very long time, changing the calling
order indeed can break what driver expects.
Can you please test the following patch:
@@ -1785,6 +1785,14 @@ int hci_dev_do_close(struct hci_dev *hdev)aosp_do_close(hdev);msft_do_close(hdev);+if(!hci_dev_test_flag(hdev,HCI_UNREGISTER)&&+!hci_dev_test_flag(hdev,HCI_USER_CHANNEL)&&+test_bit(HCI_UP,&hdev->flags)){+/* Execute vendor specific shutdown routine */+if(hdev->shutdown)+hdev->shutdown(hdev);+}+if(hdev->flush)hdev->flush(hdev);
@@ -1798,14 +1806,6 @@ int hci_dev_do_close(struct hci_dev *hdev)clear_bit(HCI_INIT,&hdev->flags);}-if(!hci_dev_test_flag(hdev,HCI_UNREGISTER)&&-!hci_dev_test_flag(hdev,HCI_USER_CHANNEL)&&-test_bit(HCI_UP,&hdev->flags)){-/* Execute vendor specific shutdown routine */-if(hdev->shutdown)-hdev->shutdown(hdev);-}-/* flush cmd work */flush_work(&hdev->cmd_work);
Thanks for the patch and your help.
I've tried it, but it seems that it does not improve for me.
I'm still observing:
i500-pumpkin login: root
root@i500-pumpkin:~# hciconfig hci0 up
Can't init device hci0: Connection timed out (110)
Logs for this session:
dmesg: https://pastebin.com/iAFk5Tzi
ftrace: https://pastebin.com/kEMWSYrE
Thanks for the testing!
What about moving the shutdown() part right after hci_req_sync_lock()
so tx/rx can still work:
Seems like btmtksdio needs shudown() to be called before flush().
Since the order was there for a very long time, changing the calling
order indeed can break what driver expects.
Can you please test the following patch:
@@ -1785,6 +1785,14 @@ int hci_dev_do_close(struct hci_dev *hdev)aosp_do_close(hdev);msft_do_close(hdev);+if(!hci_dev_test_flag(hdev,HCI_UNREGISTER)&&+!hci_dev_test_flag(hdev,HCI_USER_CHANNEL)&&+test_bit(HCI_UP,&hdev->flags)){+/* Execute vendor specific shutdown routine */+if(hdev->shutdown)+hdev->shutdown(hdev);+}+if(hdev->flush)hdev->flush(hdev);
@@ -1798,14 +1806,6 @@ int hci_dev_do_close(struct hci_dev *hdev)clear_bit(HCI_INIT,&hdev->flags);}-if(!hci_dev_test_flag(hdev,HCI_UNREGISTER)&&-!hci_dev_test_flag(hdev,HCI_USER_CHANNEL)&&-test_bit(HCI_UP,&hdev->flags)){-/* Execute vendor specific shutdown routine */-if(hdev->shutdown)-hdev->shutdown(hdev);-}-/* flush cmd work */flush_work(&hdev->cmd_work);
Thanks for the patch and your help.
I've tried it, but it seems that it does not improve for me.
I'm still observing:
i500-pumpkin login: root
root@i500-pumpkin:~# hciconfig hci0 up
Can't init device hci0: Connection timed out (110)
Logs for this session:
dmesg: https://pastebin.com/iAFk5Tzi
ftrace: https://pastebin.com/kEMWSYrE
Thanks for the testing!
What about moving the shutdown() part right after hci_req_sync_lock()
so tx/rx can still work:
Hi Mattijs,
On Thu, Aug 5, 2021 at 2:55 PM Mattijs Korpershoek
[off-list ref] wrote:
Hi Kai-Heng,
Thanks for your patch,
Kai-Heng Feng [off-list ref] writes:
[snipped]
I confirm this diff works for me:
root@i500-pumpkin:~# hciconfig hci0 up
root@i500-pumpkin:~# hciconfig hci0 down
root@i500-pumpkin:~# hciconfig hci0 up
root@i500-pumpkin:~# hciconfig hci0
hci0: Type: Primary Bus: SDIO
BD Address: 00:0C:E7:55:FF:12 ACL MTU: 1021:8 SCO MTU: 244:4
UP RUNNING
RX bytes:11268 acl:0 sco:0 events:829 errors:0
TX bytes:182569 acl:0 sco:0 commands:829 errors:0
root@i500-pumpkin:~# hcitool scan
Scanning ...
<redacted> Pixel 3 XL
Tested-by: Mattijs Korpershoek <redacted>
I found that btmtksdio_flush() only cancels the work instead of doing
flush_work(). That probably explains why putting ->shutdown right
before ->flush doesn't work.
So can you please test the following again:
Hi Mattijs,
On Thu, Aug 5, 2021 at 2:55 PM Mattijs Korpershoek
[off-list ref] wrote:
quoted
Hi Kai-Heng,
Thanks for your patch,
Kai-Heng Feng [off-list ref] writes:
[snipped]
quoted
I confirm this diff works for me:
root@i500-pumpkin:~# hciconfig hci0 up
root@i500-pumpkin:~# hciconfig hci0 down
root@i500-pumpkin:~# hciconfig hci0 up
root@i500-pumpkin:~# hciconfig hci0
hci0: Type: Primary Bus: SDIO
BD Address: 00:0C:E7:55:FF:12 ACL MTU: 1021:8 SCO MTU: 244:4
UP RUNNING
RX bytes:11268 acl:0 sco:0 events:829 errors:0
TX bytes:182569 acl:0 sco:0 commands:829 errors:0
root@i500-pumpkin:~# hcitool scan
Scanning ...
<redacted> Pixel 3 XL
Tested-by: Mattijs Korpershoek <redacted>
I found that btmtksdio_flush() only cancels the work instead of doing
flush_work(). That probably explains why putting ->shutdown right
before ->flush doesn't work.
So can you please test the following again:
Hi Mattijs,
On Fri, Aug 6, 2021 at 4:51 PM Mattijs Korpershoek
[off-list ref] wrote:
Hi Kai-Heng,
Kai-Heng Feng [off-list ref] writes:
quoted
Hi Mattijs,
On Thu, Aug 5, 2021 at 2:55 PM Mattijs Korpershoek
[off-list ref] wrote:
quoted
Hi Kai-Heng,
Thanks for your patch,
Kai-Heng Feng [off-list ref] writes:
[snipped]
quoted
I confirm this diff works for me:
root@i500-pumpkin:~# hciconfig hci0 up
root@i500-pumpkin:~# hciconfig hci0 down
root@i500-pumpkin:~# hciconfig hci0 up
root@i500-pumpkin:~# hciconfig hci0
hci0: Type: Primary Bus: SDIO
BD Address: 00:0C:E7:55:FF:12 ACL MTU: 1021:8 SCO MTU: 244:4
UP RUNNING
RX bytes:11268 acl:0 sco:0 events:829 errors:0
TX bytes:182569 acl:0 sco:0 commands:829 errors:0
root@i500-pumpkin:~# hcitool scan
Scanning ...
<redacted> Pixel 3 XL
Tested-by: Mattijs Korpershoek <redacted>
I found that btmtksdio_flush() only cancels the work instead of doing
flush_work(). That probably explains why putting ->shutdown right
before ->flush doesn't work.
So can you please test the following again:
Thanks for your testing. I think I finally got it:
btmtksdio_shutdown()
-> mtk_hci_wmt_sync()
-> __hci_cmd_send()
then waiting for BTMTKSDIO_TX_WAIT_VND_EVT, which is cleared in
btmtksdio_recv_event():
btmtksdio_recv_event()
-> hci_recv_frame()
-> queue_work(hdev->workqueue, &hdev->rx_work);
That means it has to be done before the following drain_workqueue() call.
Can you please see if moving the ->shutdown() part right before
drain_workqueue() can fix the issue?
Kai-Heng
Thanks for your testing. I think I finally got it:
btmtksdio_shutdown()
-> mtk_hci_wmt_sync()
-> __hci_cmd_send()
then waiting for BTMTKSDIO_TX_WAIT_VND_EVT, which is cleared in
btmtksdio_recv_event():
btmtksdio_recv_event()
-> hci_recv_frame()
-> queue_work(hdev->workqueue, &hdev->rx_work);
That means it has to be done before the following drain_workqueue() call.
Can you please see if moving the ->shutdown() part right before
drain_workqueue() can fix the issue?
@@ -1757,6 +1757,14 @@ int hci_dev_do_close(struct hci_dev *hdev)cancel_delayed_work_sync(&adv_instance->rpa_expired_cb);}+if(!hci_dev_test_flag(hdev,HCI_UNREGISTER)&&+!hci_dev_test_flag(hdev,HCI_USER_CHANNEL)&&+test_bit(HCI_UP,&hdev->flags)){+/* Execute vendor specific shutdown routine */+if(hdev->shutdown)+hdev->shutdown(hdev);+}+/* Avoid potential lockdep warnings from the *_flush() calls by*ensuringtheworkqueueisemptyupfront.*/
@@ -1798,14 +1806,6 @@ int hci_dev_do_close(struct hci_dev *hdev)clear_bit(HCI_INIT,&hdev->flags);}-if(!hci_dev_test_flag(hdev,HCI_UNREGISTER)&&-!hci_dev_test_flag(hdev,HCI_USER_CHANNEL)&&-test_bit(HCI_UP,&hdev->flags)){-/* Execute vendor specific shutdown routine */-if(hdev->shutdown)-hdev->shutdown(hdev);-}-/* flush cmd work */flush_work(&hdev->cmd_work);
It does not seem to fix the issue.
Adding the bits in btmtksdio_flush() does not change the result of the
above patch.
Here are the logs. These are just with the above patch (not with the
btmtksdio_flush() changes.
dmesg: https://pastebin.com/FZZBkqGC
btmtksdio_ftrace: https://pastebin.com/JQ0UWenY
Mattijs