From: Sean Wang <[email protected]>
Use the existent variable to drop the unnecessary variable created.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index ce6a6c00ff98..4f3412ad8fca 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -98,6 +98,7 @@ MODULE_DEVICE_TABLE(sdio, btmtksdio_table);
#define MTK_SDIO_BLOCK_SIZE 256
#define BTMTKSDIO_TX_WAIT_VND_EVT 1
+#define BTMTKSDIO_HW_TX_READY 2
struct mtkbtsdio_hdr {
__le16 len;
@@ -113,7 +114,6 @@ struct btmtksdio_dev {
struct work_struct txrx_work;
unsigned long tx_state;
struct sk_buff_head txq;
- bool hw_tx_ready;
struct sk_buff *evt_skb;
@@ -254,7 +254,7 @@ static int btmtksdio_tx_packet(struct btmtksdio_dev *bdev,
sdio_hdr->reserved = cpu_to_le16(0);
sdio_hdr->bt_type = hci_skb_pkt_type(skb);
- bdev->hw_tx_ready = false;
+ clear_bit(BTMTKSDIO_HW_TX_READY, &bdev->tx_state);
err = sdio_writesb(bdev->func, MTK_REG_CTDR, skb->data,
round_up(skb->len, MTK_SDIO_BLOCK_SIZE));
if (err < 0)
@@ -463,11 +463,12 @@ static void btmtksdio_txrx_work(struct work_struct *work)
bt_dev_dbg(bdev->hdev, "Get fw own back");
if (int_status & TX_EMPTY)
- bdev->hw_tx_ready = true;
+ set_bit(BTMTKSDIO_HW_TX_READY, &bdev->tx_state);
+
else if (unlikely(int_status & TX_FIFO_OVERFLOW))
bt_dev_warn(bdev->hdev, "Tx fifo overflow");
- if (bdev->hw_tx_ready) {
+ if (test_bit(BTMTKSDIO_HW_TX_READY, &bdev->tx_state)) {
skb = skb_dequeue(&bdev->txq);
if (skb) {
err = btmtksdio_tx_packet(bdev, skb);
@@ -811,7 +812,7 @@ static int btmtksdio_setup(struct hci_dev *hdev)
u32 fw_version = 0;
calltime = ktime_get();
- bdev->hw_tx_ready = true;
+ set_bit(BTMTKSDIO_HW_TX_READY, &bdev->tx_state);
switch (bdev->data->chipid) {
case 0x7921:
--
2.25.1
From: Sean Wang <[email protected]>
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 <[email protected]>
Signed-off-by: Mark-yw Chen <[email protected]>
Signed-off-by: Sean Wang <[email protected]>
---
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
From: Sean Wang <[email protected]>
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 <[email protected]>
Signed-off-by: Mark-yw Chen <[email protected]>
Signed-off-by: Sean Wang <[email protected]>
---
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
From: Sean Wang <[email protected]>
Add support of processing the firmware coredump and log for the diagnostic
purpose.
Co-developed-by: Mark-yw Chen <[email protected]>
Signed-off-by: Mark-yw Chen <[email protected]>
Signed-off-by: Sean Wang <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Mark-yw Chen <[email protected]>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> 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
Hi Sean,
> Use the existent variable to drop the unnecessary variable created.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/bluetooth/btmtksdio.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
Hi Sean,
> Add support of processing the firmware coredump and log for the diagnostic
> purpose.
>
> Co-developed-by: Mark-yw Chen <[email protected]>
> Signed-off-by: Mark-yw Chen <[email protected]>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/bluetooth/btmtksdio.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
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 <[email protected]>
> Signed-off-by: Mark-yw Chen <[email protected]>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> 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
From: Sean Wang <[email protected]>
>>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 <[email protected]>
>> Signed-off-by: Mark-yw Chen <[email protected]>
>> Signed-off-by: Sean Wang <[email protected]>
>> ---
>> 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
>