* [PATCH 2/4] Bluetooth: btmtksdio: handle runtime pm only when sdio_func is available
2021-11-19 22:25 [PATCH 1/4] Bluetooth: btmtksdio: drop the unnecessary variable created sean.wang
@ 2021-11-19 22:25 ` sean.wang
2021-11-24 14:53 ` Marcel Holtmann
2021-11-19 22:25 ` [PATCH 3/4] Bluetooth: btmtksdio: fix resume failure sean.wang
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: sean.wang @ 2021-11-19 22:25 UTC (permalink / raw)
To: marcel, johan.hedberg
Cc: Mark-YW.Chen, sean.wang, Soul.Huang, YN.Chen, Leon.Yen,
Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh,
posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
steve.lee, jsiuda, frankgor, jemele, abhishekpandit, michaelfsun,
mcchou, shawnku, linux-bluetooth, linux-mediatek, linux-kernel,
Mark-yw Chen
From: Sean Wang <sean.wang@mediatek.com>
Runtime pm ops is not aware the sdio_func status that is probably
being disabled by btmtksdio_close. Thus, we are only able to access the
sdio_func for the runtime pm operations only when the sdio_func is
available.
Fixes: 7f3c563c575e7 ("Bluetooth: btmtksdio: Add runtime PM support to SDIO based Bluetooth")
Co-developed-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
Signed-off-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/bluetooth/btmtksdio.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 4f3412ad8fca..4c46c62e4623 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -1037,6 +1037,9 @@ static int btmtksdio_runtime_suspend(struct device *dev)
if (!bdev)
return 0;
+ if (!test_bit(HCI_RUNNING, &bdev->hdev->flags))
+ return 0;
+
sdio_claim_host(bdev->func);
sdio_writel(bdev->func, C_FW_OWN_REQ_SET, MTK_REG_CHLPCR, &err);
@@ -1064,6 +1067,9 @@ static int btmtksdio_runtime_resume(struct device *dev)
if (!bdev)
return 0;
+ if (!test_bit(HCI_RUNNING, &bdev->hdev->flags))
+ return 0;
+
sdio_claim_host(bdev->func);
sdio_writel(bdev->func, C_FW_OWN_REQ_CLR, MTK_REG_CHLPCR, &err);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] Bluetooth: btmtksdio: handle runtime pm only when sdio_func is available
2021-11-19 22:25 ` [PATCH 2/4] Bluetooth: btmtksdio: handle runtime pm only when sdio_func is available sean.wang
@ 2021-11-24 14:53 ` Marcel Holtmann
0 siblings, 0 replies; 9+ messages in thread
From: Marcel Holtmann @ 2021-11-24 14:53 UTC (permalink / raw)
To: Sean Wang
Cc: Johan Hedberg, "Mark-YW Chen (陳揚文)",
Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang, Deren.Wu, km.lin,
robin.chiu, Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
abhishekpandit, michaelfsun, mcchou, shawnku, linux-bluetooth,
linux-mediatek, linux-kernel
Hi Sean,
> Runtime pm ops is not aware the sdio_func status that is probably
> being disabled by btmtksdio_close. Thus, we are only able to access the
> sdio_func for the runtime pm operations only when the sdio_func is
> available.
>
> Fixes: 7f3c563c575e7 ("Bluetooth: btmtksdio: Add runtime PM support to SDIO based Bluetooth")
> Co-developed-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
> Signed-off-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
> drivers/bluetooth/btmtksdio.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
> index 4f3412ad8fca..4c46c62e4623 100644
> --- a/drivers/bluetooth/btmtksdio.c
> +++ b/drivers/bluetooth/btmtksdio.c
> @@ -1037,6 +1037,9 @@ static int btmtksdio_runtime_suspend(struct device *dev)
> if (!bdev)
> return 0;
>
> + if (!test_bit(HCI_RUNNING, &bdev->hdev->flags))
> + return 0;
> +
> sdio_claim_host(bdev->func);
>
> sdio_writel(bdev->func, C_FW_OWN_REQ_SET, MTK_REG_CHLPCR, &err);
> @@ -1064,6 +1067,9 @@ static int btmtksdio_runtime_resume(struct device *dev)
> if (!bdev)
> return 0;
>
> + if (!test_bit(HCI_RUNNING, &bdev->hdev->flags))
> + return 0;
> +
> sdio_claim_host(bdev->func);
>
> sdio_writel(bdev->func, C_FW_OWN_REQ_CLR, MTK_REG_CHLPCR, &err);
I dislike looking at HCI_RUNNING since that check should be removed from a driver. Do you really need it? I mean, a driver should now if it is running or not.
Regards
Marcel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] Bluetooth: btmtksdio: fix resume failure
2021-11-19 22:25 [PATCH 1/4] Bluetooth: btmtksdio: drop the unnecessary variable created sean.wang
2021-11-19 22:25 ` [PATCH 2/4] Bluetooth: btmtksdio: handle runtime pm only when sdio_func is available sean.wang
@ 2021-11-19 22:25 ` sean.wang
2021-11-24 15:21 ` Marcel Holtmann
2021-11-19 22:25 ` [PATCH 4/4] Bluetooth: btmtksdio: add support of processing firmware coredump and log sean.wang
2021-11-24 15:19 ` [PATCH 1/4] Bluetooth: btmtksdio: drop the unnecessary variable created Marcel Holtmann
3 siblings, 1 reply; 9+ messages in thread
From: sean.wang @ 2021-11-19 22:25 UTC (permalink / raw)
To: marcel, johan.hedberg
Cc: Mark-YW.Chen, sean.wang, Soul.Huang, YN.Chen, Leon.Yen,
Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh,
posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
steve.lee, jsiuda, frankgor, jemele, abhishekpandit, michaelfsun,
mcchou, shawnku, linux-bluetooth, linux-mediatek, linux-kernel,
Mark-yw Chen
From: Sean Wang <sean.wang@mediatek.com>
btmtksdio have to rely on MMC_PM_KEEP_POWER in pm_flags to avoid that
SDIO power is being shut off during the device is in suspend. That fixes
the SDIO command fails to access the bus after the device is resumed.
Fixes: 7f3c563c575e7 ("Bluetooth: btmtksdio: Add runtime PM support to SDIO based Bluetooth")
Co-developed-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
Signed-off-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/bluetooth/btmtksdio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 4c46c62e4623..cae1fcd15512 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -1040,6 +1040,8 @@ static int btmtksdio_runtime_suspend(struct device *dev)
if (!test_bit(HCI_RUNNING, &bdev->hdev->flags))
return 0;
+ sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
+
sdio_claim_host(bdev->func);
sdio_writel(bdev->func, C_FW_OWN_REQ_SET, MTK_REG_CHLPCR, &err);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] Bluetooth: btmtksdio: fix resume failure
2021-11-19 22:25 ` [PATCH 3/4] Bluetooth: btmtksdio: fix resume failure sean.wang
@ 2021-11-24 15:21 ` Marcel Holtmann
0 siblings, 0 replies; 9+ messages in thread
From: Marcel Holtmann @ 2021-11-24 15:21 UTC (permalink / raw)
To: Sean Wang
Cc: Johan Hedberg, "Mark-YW Chen (陳揚文)",
Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang, Deren.Wu, km.lin,
robin.chiu, Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
abhishekpandit, michaelfsun, mcchou, shawnku, linux-bluetooth,
linux-mediatek, linux-kernel
Hi Sean,
> btmtksdio have to rely on MMC_PM_KEEP_POWER in pm_flags to avoid that
> SDIO power is being shut off during the device is in suspend. That fixes
> the SDIO command fails to access the bus after the device is resumed.
>
> Fixes: 7f3c563c575e7 ("Bluetooth: btmtksdio: Add runtime PM support to SDIO based Bluetooth")
> Co-developed-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
> Signed-off-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
> drivers/bluetooth/btmtksdio.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
> index 4c46c62e4623..cae1fcd15512 100644
> --- a/drivers/bluetooth/btmtksdio.c
> +++ b/drivers/bluetooth/btmtksdio.c
> @@ -1040,6 +1040,8 @@ static int btmtksdio_runtime_suspend(struct device *dev)
> if (!test_bit(HCI_RUNNING, &bdev->hdev->flags))
> return 0;
>
> + sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
> +
> sdio_claim_host(bdev->func);
>
> sdio_writel(bdev->func, C_FW_OWN_REQ_SET, MTK_REG_CHLPCR, &err);
if this makes sense without 2/4 patch, then please re-send.
Regards
Marcel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] Bluetooth: btmtksdio: add support of processing firmware coredump and log
2021-11-19 22:25 [PATCH 1/4] Bluetooth: btmtksdio: drop the unnecessary variable created sean.wang
2021-11-19 22:25 ` [PATCH 2/4] Bluetooth: btmtksdio: handle runtime pm only when sdio_func is available sean.wang
2021-11-19 22:25 ` [PATCH 3/4] Bluetooth: btmtksdio: fix resume failure sean.wang
@ 2021-11-19 22:25 ` sean.wang
2021-11-24 15:20 ` Marcel Holtmann
2021-11-24 15:19 ` [PATCH 1/4] Bluetooth: btmtksdio: drop the unnecessary variable created Marcel Holtmann
3 siblings, 1 reply; 9+ messages in thread
From: sean.wang @ 2021-11-19 22:25 UTC (permalink / raw)
To: marcel, johan.hedberg
Cc: Mark-YW.Chen, sean.wang, Soul.Huang, YN.Chen, Leon.Yen,
Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh,
posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
steve.lee, jsiuda, frankgor, jemele, abhishekpandit, michaelfsun,
mcchou, shawnku, linux-bluetooth, linux-mediatek, linux-kernel,
Mark-yw Chen
From: Sean Wang <sean.wang@mediatek.com>
Add support of processing the firmware coredump and log for the diagnostic
purpose.
Co-developed-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
Signed-off-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/bluetooth/btmtksdio.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index cae1fcd15512..adf9c89648cc 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -324,8 +324,29 @@ static int btmtksdio_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
return err;
}
+static int btmtksdio_recv_acl(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
+ u16 handle = le16_to_cpu(hci_acl_hdr(skb)->handle);
+
+ switch (handle) {
+ case 0xfc6f:
+ /* Firmware dump from device: when the firmware hangs, the
+ * device can no longer suspend and thus disable auto-suspend.
+ */
+ pm_runtime_forbid(bdev->dev);
+ fallthrough;
+ case 0x05ff:
+ case 0x05fe:
+ /* Firmware debug logging */
+ return hci_recv_diag(hdev, skb);
+ }
+
+ return hci_recv_frame(hdev, skb);
+}
+
static const struct h4_recv_pkt mtk_recv_pkts[] = {
- { H4_RECV_ACL, .recv = hci_recv_frame },
+ { H4_RECV_ACL, .recv = btmtksdio_recv_acl },
{ H4_RECV_SCO, .recv = hci_recv_frame },
{ H4_RECV_EVENT, .recv = btmtksdio_recv_event },
};
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] Bluetooth: btmtksdio: add support of processing firmware coredump and log
2021-11-19 22:25 ` [PATCH 4/4] Bluetooth: btmtksdio: add support of processing firmware coredump and log sean.wang
@ 2021-11-24 15:20 ` Marcel Holtmann
0 siblings, 0 replies; 9+ messages in thread
From: Marcel Holtmann @ 2021-11-24 15:20 UTC (permalink / raw)
To: Sean Wang
Cc: Johan Hedberg, "Mark-YW Chen (陳揚文)",
Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang, Deren.Wu, km.lin,
robin.chiu, Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
abhishekpandit, Michael Sun, Miao-chen Chou, shawnku,
linux-bluetooth, linux-mediatek, linux-kernel
Hi Sean,
> Add support of processing the firmware coredump and log for the diagnostic
> purpose.
>
> Co-developed-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
> Signed-off-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
> drivers/bluetooth/btmtksdio.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] Bluetooth: btmtksdio: drop the unnecessary variable created
2021-11-19 22:25 [PATCH 1/4] Bluetooth: btmtksdio: drop the unnecessary variable created sean.wang
` (2 preceding siblings ...)
2021-11-19 22:25 ` [PATCH 4/4] Bluetooth: btmtksdio: add support of processing firmware coredump and log sean.wang
@ 2021-11-24 15:19 ` Marcel Holtmann
3 siblings, 0 replies; 9+ messages in thread
From: Marcel Holtmann @ 2021-11-24 15:19 UTC (permalink / raw)
To: Sean Wang
Cc: Johan Hedberg, "Mark-YW Chen (陳揚文)",
Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang, Deren.Wu, km.lin,
robin.chiu, Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
abhishekpandit, michaelfsun, mcchou, shawnku, linux-bluetooth,
linux-mediatek, linux-kernel
Hi Sean,
> Use the existent variable to drop the unnecessary variable created.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
> drivers/bluetooth/btmtksdio.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] Bluetooth: btmtksdio: handle runtime pm only when sdio_func is available
[not found] <74A2D0D6-6A65-4832-BAFC-BCBA68F8DE78@holtmann.org--annotate>
@ 2021-11-25 8:22 ` sean.wang
0 siblings, 0 replies; 9+ messages in thread
From: sean.wang @ 2021-11-25 8:22 UTC (permalink / raw)
To: marcel, johan.hedberg
Cc: Mark-YW.Chen, sean.wang, Soul.Huang, YN.Chen, Leon.Yen,
Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh,
posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
steve.lee, jsiuda, frankgor, jemele, abhishekpandit, michaelfsun,
mcchou, shawnku, linux-bluetooth, linux-mediatek, linux-kernel
From: Sean Wang <sean.wang@mediatek.com>
>>Hi Sean,
>
>> Runtime pm ops is not aware the sdio_func status that is probably
>> being disabled by btmtksdio_close. Thus, we are only able to access
>> the sdio_func for the runtime pm operations only when the sdio_func is
>> available.
>>
>> Fixes: 7f3c563c575e7 ("Bluetooth: btmtksdio: Add runtime PM support to
>> SDIO based Bluetooth")
>> Co-developed-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
>> Signed-off-by: Mark-yw Chen <mark-yw.chen@mediatek.com>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>> drivers/bluetooth/btmtksdio.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btmtksdio.c
>> b/drivers/bluetooth/btmtksdio.c index 4f3412ad8fca..4c46c62e4623
>> 100644
>> --- a/drivers/bluetooth/btmtksdio.c
>> +++ b/drivers/bluetooth/btmtksdio.c
>> @@ -1037,6 +1037,9 @@ static int btmtksdio_runtime_suspend(struct device *dev)
>> if (!bdev)
>> return 0;
>>
>> + if (!test_bit(HCI_RUNNING, &bdev->hdev->flags))
>> + return 0;
>> +
>> sdio_claim_host(bdev->func);
>>
>> sdio_writel(bdev->func, C_FW_OWN_REQ_SET, MTK_REG_CHLPCR, &err); @@
>> -1064,6 +1067,9 @@ static int btmtksdio_runtime_resume(struct device *dev)
>> if (!bdev)
>> return 0;
>>
>> + if (!test_bit(HCI_RUNNING, &bdev->hdev->flags))
>> + return 0;
>> +
>> sdio_claim_host(bdev->func);
>>
>> sdio_writel(bdev->func, C_FW_OWN_REQ_CLR, MTK_REG_CHLPCR, &err);
>
>I dislike looking at HCI_RUNNING since that check should be removed from a driver. Do you really need it? I mean, a driver should now if it is running or not.
We don't really need it, instead we can use internal flags in the driver to know the status. I will do this in v2.
Sean
>
>Regards
>
>Marcel
>
^ permalink raw reply [flat|nested] 9+ messages in thread