2022-01-12 22:31:20

by Sean Wang

[permalink] [raw]
Subject: [PATCH 1/7] Bluetooth: mt7921s: fix firmware coredump retrieve

From: Mark Chen <[email protected]>

According to the MCU firmware behavior, as the driver is aware of the
notification of the interrupt source FW_MAILBOX_INT that shows the MCU
completed delivered a core dump piece to the host, the driver must
acknowledge the MCU with the register PH2DSM0R bit PH2DSM0R_DRIVER_OWN
to notify the MCU to handle the next core dump piece.

Fixes: db57b625912a ("Bluetooth: btmtksdio: add support of processing firmware coredump and log")
Co-developed-by: Sean Wang <[email protected]>
Signed-off-by: Sean Wang <[email protected]>
Signed-off-by: Mark Chen <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 25fb9c79b1f4..784e65c3fddd 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -87,8 +87,12 @@ MODULE_DEVICE_TABLE(sdio, btmtksdio_table);
#define RX_DONE_INT BIT(1)
#define TX_EMPTY BIT(2)
#define TX_FIFO_OVERFLOW BIT(8)
+#define FW_MAILBOX_INT BIT(15)
#define RX_PKT_LEN GENMASK(31, 16)

+#define MTK_REG_PH2DSM0R 0xc4
+#define PH2DSM0R_DRIVER_OWN BIT(0)
+
#define MTK_REG_CTDR 0x18

#define MTK_REG_CRDR 0x1c
@@ -481,6 +485,12 @@ static void btmtksdio_txrx_work(struct work_struct *work)
*/
sdio_writel(bdev->func, int_status, MTK_REG_CHISR, NULL);

+ if ((int_status & FW_MAILBOX_INT) &&
+ bdev->data->chipid == 0x7921) {
+ sdio_writel(bdev->func, PH2DSM0R_DRIVER_OWN,
+ MTK_REG_PH2DSM0R, 0);
+ }
+
if (int_status & FW_OWN_BACK_INT)
bt_dev_dbg(bdev->hdev, "Get fw own back");

--
2.25.1


2022-01-12 22:31:22

by Sean Wang

[permalink] [raw]
Subject: [PATCH 3/7] Bluetooth: mt7921s: fix bus hang with wrong privilege

From: Mark Chen <[email protected]>

According to chip hw flow, mt7921s need to re-acquire privilege
again before normal running. Otherwise, the bus may be stuck in
an abnormal status.

Fixes: c603bf1f94d0 ("Bluetooth: btmtksdio: add MT7921s Bluetooth support")
Co-developed-by: Sean Wang <[email protected]>
Signed-off-by: Sean Wang <[email protected]>
Signed-off-by: Mark Chen <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 8cbd49013d86..0951bb10d117 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -960,6 +960,14 @@ static int btmtksdio_setup(struct hci_dev *hdev)
if (err < 0)
return err;

+ err = btmtksdio_fw_pmctrl(bdev);
+ if (err < 0)
+ return err;
+
+ err = btmtksdio_drv_pmctrl(bdev);
+ if (err < 0)
+ return err;
+
/* Enable SCO over I2S/PCM */
err = btmtksdio_sco_setting(hdev);
if (err < 0) {
--
2.25.1

2022-01-12 22:31:23

by Sean Wang

[permalink] [raw]
Subject: [PATCH 4/7] Bluetooth: mt7921s: fix btmtksdio_[drv|fw]_pmctrl()

From: Mark Chen <[email protected]>

According to the firmware behavior (even the oldest one in linux-firmware)

If the firmware is downloaded, MT7921S must rely on the additional mailbox
mechanism that resides in firmware to check if the device is the right
state for btmtksdio_mcu_[drv|fw]_pmctrl(). Otherwise, we still apply the
old way for that.

That is a necessary patch before we enable runtime pm for mt7921s as
default.

Fixes: c603bf1f94d0 ("Bluetooth: btmtksdio: add MT7921s Bluetooth support")
Co-developed-by: Sean Wang <[email protected]>
Signed-off-by: Sean Wang <[email protected]>
Signed-off-by: Mark Chen <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 38 +++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 0951bb10d117..b0aae4143037 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -38,21 +38,25 @@ static bool enable_autosuspend;
struct btmtksdio_data {
const char *fwname;
u16 chipid;
+ bool lp_mbox_supported;
};

static const struct btmtksdio_data mt7663_data = {
.fwname = FIRMWARE_MT7663,
.chipid = 0x7663,
+ .lp_mbox_supported = false,
};

static const struct btmtksdio_data mt7668_data = {
.fwname = FIRMWARE_MT7668,
.chipid = 0x7668,
+ .lp_mbox_supported = false,
};

static const struct btmtksdio_data mt7921_data = {
.fwname = FIRMWARE_MT7961,
.chipid = 0x7921,
+ .lp_mbox_supported = true,
};

static const struct sdio_device_id btmtksdio_table[] = {
@@ -90,8 +94,12 @@ MODULE_DEVICE_TABLE(sdio, btmtksdio_table);
#define FW_MAILBOX_INT BIT(15)
#define RX_PKT_LEN GENMASK(31, 16)

+#define MTK_REG_CSICR 0xc0
+#define CSICR_CLR_MBOX_ACK BIT(0)
#define MTK_REG_PH2DSM0R 0xc4
#define PH2DSM0R_DRIVER_OWN BIT(0)
+#define MTK_REG_PD2HRM0R 0xdc
+#define PD2HRM0R_DRV_OWN BIT(0)

#define MTK_REG_CTDR 0x18

@@ -104,6 +112,7 @@ MODULE_DEVICE_TABLE(sdio, btmtksdio_table);
#define BTMTKSDIO_TX_WAIT_VND_EVT 1
#define BTMTKSDIO_HW_TX_READY 2
#define BTMTKSDIO_FUNC_ENABLED 3
+#define BTMTKSDIO_PATCH_ENABLED 4

struct mtkbtsdio_hdr {
__le16 len;
@@ -282,6 +291,11 @@ static u32 btmtksdio_drv_own_query(struct btmtksdio_dev *bdev)
return sdio_readl(bdev->func, MTK_REG_CHLPCR, NULL);
}

+static u32 btmtksdio_drv_own_query_79xx(struct btmtksdio_dev *bdev)
+{
+ return sdio_readl(bdev->func, MTK_REG_PD2HRM0R, NULL);
+}
+
static int btmtksdio_fw_pmctrl(struct btmtksdio_dev *bdev)
{
u32 status;
@@ -289,6 +303,19 @@ static int btmtksdio_fw_pmctrl(struct btmtksdio_dev *bdev)

sdio_claim_host(bdev->func);

+ if (bdev->data->lp_mbox_supported &&
+ test_bit(BTMTKSDIO_PATCH_ENABLED, &bdev->tx_state)) {
+ sdio_writel(bdev->func, CSICR_CLR_MBOX_ACK, MTK_REG_CSICR,
+ &err);
+ err = readx_poll_timeout(btmtksdio_drv_own_query_79xx, bdev,
+ status, !(status & PD2HRM0R_DRV_OWN),
+ 2000, 1000000);
+ if (err < 0) {
+ bt_dev_err(bdev->hdev, "mailbox ACK not cleared");
+ goto out;
+ }
+ }
+
/* Return ownership to the device */
sdio_writel(bdev->func, C_FW_OWN_REQ_SET, MTK_REG_CHLPCR, &err);
if (err < 0)
@@ -321,6 +348,12 @@ static int btmtksdio_drv_pmctrl(struct btmtksdio_dev *bdev)
err = readx_poll_timeout(btmtksdio_drv_own_query, bdev, status,
status & C_COM_DRV_OWN, 2000, 1000000);

+ if (!err && bdev->data->lp_mbox_supported &&
+ test_bit(BTMTKSDIO_PATCH_ENABLED, &bdev->tx_state))
+ err = readx_poll_timeout(btmtksdio_drv_own_query_79xx, bdev,
+ status, status & PD2HRM0R_DRV_OWN,
+ 2000, 1000000);
+
out:
sdio_release_host(bdev->func);

@@ -728,6 +761,7 @@ static int btmtksdio_func_query(struct hci_dev *hdev)

static int mt76xx_setup(struct hci_dev *hdev, const char *fwname)
{
+ struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
struct btmtk_hci_wmt_params wmt_params;
struct btmtk_tci_sleep tci_sleep;
struct sk_buff *skb;
@@ -788,6 +822,8 @@ static int mt76xx_setup(struct hci_dev *hdev, const char *fwname)
return err;
}

+ set_bit(BTMTKSDIO_PATCH_ENABLED, &bdev->tx_state);
+
ignore_func_on:
/* Apply the low power environment setup */
tci_sleep.mode = 0x5;
@@ -810,6 +846,7 @@ static int mt76xx_setup(struct hci_dev *hdev, const char *fwname)

static int mt79xx_setup(struct hci_dev *hdev, const char *fwname)
{
+ struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
struct btmtk_hci_wmt_params wmt_params;
u8 param = 0x1;
int err;
@@ -835,6 +872,7 @@ static int mt79xx_setup(struct hci_dev *hdev, const char *fwname)

hci_set_msft_opcode(hdev, 0xFD30);
hci_set_aosp_capable(hdev);
+ set_bit(BTMTKSDIO_PATCH_ENABLED, &bdev->tx_state);

return err;
}
--
2.25.1

2022-01-12 22:31:25

by Sean Wang

[permalink] [raw]
Subject: [PATCH 2/7] Bluetooth: btmtksdio: refactor btmtksdio_runtime_[suspend|resume]()

From: Mark Chen <[email protected]>

Refactor btmtksdio_runtime_[suspend|resume]() to create the common
funcitons btmtksdio_[fw|drv]_pmctrl() shared with btmtksdio_[open|close]()
to avoid the redundant code as well.

This is also a prerequisite patch for the incoming patches.

Co-developed-by: Sean Wang <[email protected]>
Signed-off-by: Sean Wang <[email protected]>
Signed-off-by: Mark Chen <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 94 ++++++++++++++++++++---------------
1 file changed, 53 insertions(+), 41 deletions(-)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 784e65c3fddd..8cbd49013d86 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -282,6 +282,54 @@ static u32 btmtksdio_drv_own_query(struct btmtksdio_dev *bdev)
return sdio_readl(bdev->func, MTK_REG_CHLPCR, NULL);
}

+static int btmtksdio_fw_pmctrl(struct btmtksdio_dev *bdev)
+{
+ u32 status;
+ int err;
+
+ sdio_claim_host(bdev->func);
+
+ /* Return ownership to the device */
+ sdio_writel(bdev->func, C_FW_OWN_REQ_SET, MTK_REG_CHLPCR, &err);
+ if (err < 0)
+ goto out;
+
+ err = readx_poll_timeout(btmtksdio_drv_own_query, bdev, status,
+ !(status & C_COM_DRV_OWN), 2000, 1000000);
+
+out:
+ sdio_release_host(bdev->func);
+
+ if (err < 0)
+ bt_dev_err(bdev->hdev, "Cannot return ownership to device");
+
+ return err;
+}
+
+static int btmtksdio_drv_pmctrl(struct btmtksdio_dev *bdev)
+{
+ u32 status;
+ int err;
+
+ sdio_claim_host(bdev->func);
+
+ /* Get ownership from the device */
+ sdio_writel(bdev->func, C_FW_OWN_REQ_CLR, MTK_REG_CHLPCR, &err);
+ if (err < 0)
+ goto out;
+
+ err = readx_poll_timeout(btmtksdio_drv_own_query, bdev, status,
+ status & C_COM_DRV_OWN, 2000, 1000000);
+
+out:
+ sdio_release_host(bdev->func);
+
+ if (err < 0)
+ bt_dev_err(bdev->hdev, "Cannot get ownership from device");
+
+ return err;
+}
+
static int btmtksdio_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
{
struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
@@ -541,7 +589,7 @@ static void btmtksdio_interrupt(struct sdio_func *func)
static int btmtksdio_open(struct hci_dev *hdev)
{
struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
- u32 status, val;
+ u32 val;
int err;

sdio_claim_host(bdev->func);
@@ -552,18 +600,10 @@ static int btmtksdio_open(struct hci_dev *hdev)

set_bit(BTMTKSDIO_FUNC_ENABLED, &bdev->tx_state);

- /* Get ownership from the device */
- sdio_writel(bdev->func, C_FW_OWN_REQ_CLR, MTK_REG_CHLPCR, &err);
+ err = btmtksdio_drv_pmctrl(bdev);
if (err < 0)
goto err_disable_func;

- err = readx_poll_timeout(btmtksdio_drv_own_query, bdev, status,
- status & C_COM_DRV_OWN, 2000, 1000000);
- if (err < 0) {
- bt_dev_err(bdev->hdev, "Cannot get ownership from device");
- goto err_disable_func;
- }
-
/* Disable interrupt & mask out all interrupt sources */
sdio_writel(bdev->func, C_INT_EN_CLR, MTK_REG_CHLPCR, &err);
if (err < 0)
@@ -633,8 +673,6 @@ static int btmtksdio_open(struct hci_dev *hdev)
static int btmtksdio_close(struct hci_dev *hdev)
{
struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
- u32 status;
- int err;

sdio_claim_host(bdev->func);

@@ -645,13 +683,7 @@ static int btmtksdio_close(struct hci_dev *hdev)

cancel_work_sync(&bdev->txrx_work);

- /* Return ownership to the device */
- sdio_writel(bdev->func, C_FW_OWN_REQ_SET, MTK_REG_CHLPCR, NULL);
-
- err = readx_poll_timeout(btmtksdio_drv_own_query, bdev, status,
- !(status & C_COM_DRV_OWN), 2000, 1000000);
- if (err < 0)
- bt_dev_err(bdev->hdev, "Cannot return ownership to device");
+ btmtksdio_fw_pmctrl(bdev);

clear_bit(BTMTKSDIO_FUNC_ENABLED, &bdev->tx_state);
sdio_disable_func(bdev->func);
@@ -1162,7 +1194,6 @@ static int btmtksdio_runtime_suspend(struct device *dev)
{
struct sdio_func *func = dev_to_sdio_func(dev);
struct btmtksdio_dev *bdev;
- u32 status;
int err;

bdev = sdio_get_drvdata(func);
@@ -1174,19 +1205,10 @@ static int btmtksdio_runtime_suspend(struct device *dev)

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 (err < 0)
- goto out;
+ err = btmtksdio_fw_pmctrl(bdev);

- err = readx_poll_timeout(btmtksdio_drv_own_query, bdev, status,
- !(status & C_COM_DRV_OWN), 2000, 1000000);
-out:
bt_dev_info(bdev->hdev, "status (%d) return ownership to device", err);

- sdio_release_host(bdev->func);
-
return err;
}

@@ -1194,7 +1216,6 @@ static int btmtksdio_runtime_resume(struct device *dev)
{
struct sdio_func *func = dev_to_sdio_func(dev);
struct btmtksdio_dev *bdev;
- u32 status;
int err;

bdev = sdio_get_drvdata(func);
@@ -1204,19 +1225,10 @@ static int btmtksdio_runtime_resume(struct device *dev)
if (!test_bit(BTMTKSDIO_FUNC_ENABLED, &bdev->tx_state))
return 0;

- sdio_claim_host(bdev->func);
-
- sdio_writel(bdev->func, C_FW_OWN_REQ_CLR, MTK_REG_CHLPCR, &err);
- if (err < 0)
- goto out;
+ err = btmtksdio_drv_pmctrl(bdev);

- err = readx_poll_timeout(btmtksdio_drv_own_query, bdev, status,
- status & C_COM_DRV_OWN, 2000, 1000000);
-out:
bt_dev_info(bdev->hdev, "status (%d) get ownership from device", err);

- sdio_release_host(bdev->func);
-
return err;
}

--
2.25.1

2022-01-12 22:31:44

by Sean Wang

[permalink] [raw]
Subject: [PATCH 7/7] Bluetooth: btmtksdio: mask out interrupt status

From: Sean Wang <[email protected]>

Currently, there is a loop in btmtksdio_txrx_work() which iteratively
executes until the variable int_status is zero.

But the variable int_status should be masked out with the actual interrupt
sources (MTK_REG_CHISR bit 0-15) before we check the loop condition.
Otherwise, RX_PKT_LEN (MTK_REG_CHISR bit 16-31) which is read-only and
unclearable would cause the loop to get stuck on some chipsets like
MT7663s.

Fixes: 26270bc189ea ("Bluetooth: btmtksdio: move interrupt service to work")
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 491f029452ca..8be763ab3bf4 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -92,6 +92,7 @@ MODULE_DEVICE_TABLE(sdio, btmtksdio_table);
#define TX_EMPTY BIT(2)
#define TX_FIFO_OVERFLOW BIT(8)
#define FW_MAILBOX_INT BIT(15)
+#define INT_MASK GENMASK(15, 0)
#define RX_PKT_LEN GENMASK(31, 16)

#define MTK_REG_CSICR 0xc0
@@ -565,6 +566,7 @@ static void btmtksdio_txrx_work(struct work_struct *work)
* FIFO.
*/
sdio_writel(bdev->func, int_status, MTK_REG_CHISR, NULL);
+ int_status &= INT_MASK;

if ((int_status & FW_MAILBOX_INT) &&
bdev->data->chipid == 0x7921) {
--
2.25.1

2022-01-12 22:31:47

by Sean Wang

[permalink] [raw]
Subject: [PATCH 6/7] Bluetooth: btmtksdio: run sleep mode by default

From: Sean Wang <[email protected]>

Apply sleep mode by default and a smaller idle time to reduce power
consumption further.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 3e8f19556e2c..491f029452ca 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -31,9 +31,9 @@

#define VERSION "0.1"

-#define MTKBTSDIO_AUTOSUSPEND_DELAY 8000
+#define MTKBTSDIO_AUTOSUSPEND_DELAY 1000

-static bool enable_autosuspend;
+static bool enable_autosuspend = true;

struct btmtksdio_data {
const char *fwname;
--
2.25.1

2022-01-12 22:33:01

by Sean Wang

[permalink] [raw]
Subject: [PATCH 5/7] Bluetooth: btmtksdio: lower log level in btmtksdio_runtime_[resume|suspend]()

From: Sean Wang <[email protected]>

Lower its log level from INFO to DEBUG prior to we enable runtime pm for
mt7921s with the smaller idle time as default.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index b0aae4143037..3e8f19556e2c 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -1253,7 +1253,7 @@ static int btmtksdio_runtime_suspend(struct device *dev)

err = btmtksdio_fw_pmctrl(bdev);

- bt_dev_info(bdev->hdev, "status (%d) return ownership to device", err);
+ bt_dev_dbg(bdev->hdev, "status (%d) return ownership to device", err);

return err;
}
@@ -1273,7 +1273,7 @@ static int btmtksdio_runtime_resume(struct device *dev)

err = btmtksdio_drv_pmctrl(bdev);

- bt_dev_info(bdev->hdev, "status (%d) get ownership from device", err);
+ bt_dev_dbg(bdev->hdev, "status (%d) get ownership from device", err);

return err;
}
--
2.25.1

2022-01-22 17:09:44

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/7] Bluetooth: mt7921s: fix firmware coredump retrieve

Hi Sean,

> According to the MCU firmware behavior, as the driver is aware of the
> notification of the interrupt source FW_MAILBOX_INT that shows the MCU
> completed delivered a core dump piece to the host, the driver must
> acknowledge the MCU with the register PH2DSM0R bit PH2DSM0R_DRIVER_OWN
> to notify the MCU to handle the next core dump piece.
>
> Fixes: db57b625912a ("Bluetooth: btmtksdio: add support of processing firmware coredump and log")
> Co-developed-by: Sean Wang <[email protected]>
> Signed-off-by: Sean Wang <[email protected]>
> Signed-off-by: Mark Chen <[email protected]>
> ---
> drivers/bluetooth/btmtksdio.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)

all 7 patches have been applied to bluetooth-next tree.

Regards

Marcel