From: Sean Wang <[email protected]>
First three are all minor changes and the final one adds a runtime pm support to
enter a lower power state for decreasing power dissipation when there is idle
traffic in SDIO transport for a while.
Sean Wang (4):
Bluetooth: btmtksdio: Drop newline with bt_dev logging macros
Bluetooth: btmtksdio: Add a bit definition for CHLPCR
Bluetooth: btmtksdio: Fix hdev->stat.byte_rx accumulation
Bluetooth: btmtksdio: Add runtime PM support to SDIO based Bluetooth
drivers/bluetooth/btmtksdio.c | 161 ++++++++++++++++++++++++++++++++--
1 file changed, 153 insertions(+), 8 deletions(-)
--
2.18.0
From: Sean Wang <[email protected]>
Add runtime PM support to btmtksdio. With this way, there will be the
benefit of the device entering the more power saving state once it is
been a while data traffic is idle.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 144 ++++++++++++++++++++++++++++++++++
1 file changed, 144 insertions(+)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 877c0a831775..813338288453 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -17,6 +17,7 @@
#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/pm_runtime.h>
#include <linux/skbuff.h>
#include <linux/mmc/host.h>
@@ -33,6 +34,10 @@
#define FIRMWARE_MT7663 "mediatek/mt7663pr2h.bin"
#define FIRMWARE_MT7668 "mediatek/mt7668pr2h.bin"
+#define MTKBTSDIO_AUTOSUSPEND_DELAY 8000
+
+static bool enable_autosuspend;
+
struct btmtksdio_data {
const char *fwname;
};
@@ -150,6 +155,7 @@ struct btmtk_hci_wmt_params {
struct btmtksdio_dev {
struct hci_dev *hdev;
struct sdio_func *func;
+ struct device *dev;
struct work_struct tx_work;
unsigned long tx_state;
@@ -301,6 +307,8 @@ static void btmtksdio_tx_work(struct work_struct *work)
struct sk_buff *skb;
int err;
+ pm_runtime_get_sync(bdev->dev);
+
sdio_claim_host(bdev->func);
while ((skb = skb_dequeue(&bdev->txq))) {
@@ -313,6 +321,9 @@ static void btmtksdio_tx_work(struct work_struct *work)
}
sdio_release_host(bdev->func);
+
+ pm_runtime_mark_last_busy(bdev->dev);
+ pm_runtime_put_autosuspend(bdev->dev);
}
static int btmtksdio_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
@@ -471,6 +482,18 @@ static void btmtksdio_interrupt(struct sdio_func *func)
u32 int_status;
u16 rx_size;
+ /* It is required that the host gets ownership from the device before
+ * accessing any register, however, if SDIO host is not being released,
+ * a potential deadlock probably happens in a circular wait between SDIO
+ * IRQ work and PM runtime work. So, we have to explicitly release SDIO
+ * host here and claim again after the PM runtime work is all done.
+ */
+ sdio_release_host(bdev->func);
+
+ pm_runtime_get_sync(bdev->dev);
+
+ sdio_claim_host(bdev->func);
+
/* Disable interrupt */
sdio_writel(func, C_INT_EN_CLR, MTK_REG_CHLPCR, 0);
@@ -507,6 +530,9 @@ static void btmtksdio_interrupt(struct sdio_func *func)
/* Enable interrupt */
sdio_writel(func, C_INT_EN_SET, MTK_REG_CHLPCR, 0);
+
+ pm_runtime_mark_last_busy(bdev->dev);
+ pm_runtime_put_autosuspend(bdev->dev);
}
static int btmtksdio_open(struct hci_dev *hdev)
@@ -815,6 +841,23 @@ static int btmtksdio_setup(struct hci_dev *hdev)
delta = ktime_sub(rettime, calltime);
duration = (unsigned long long)ktime_to_ns(delta) >> 10;
+ pm_runtime_set_autosuspend_delay(bdev->dev,
+ MTKBTSDIO_AUTOSUSPEND_DELAY);
+ pm_runtime_use_autosuspend(bdev->dev);
+
+ err = pm_runtime_set_active(bdev->dev);
+ if (err < 0)
+ return err;
+
+ /* Default forbid runtime auto suspend, that can be allowed by
+ * enable_autosuspend flag or the PM runtime entry under sysfs.
+ */
+ pm_runtime_forbid(bdev->dev);
+ pm_runtime_enable(bdev->dev);
+
+ if (enable_autosuspend)
+ pm_runtime_allow(bdev->dev);
+
bt_dev_info(hdev, "Device setup in %llu usecs", duration);
return 0;
@@ -822,10 +865,16 @@ static int btmtksdio_setup(struct hci_dev *hdev)
static int btmtksdio_shutdown(struct hci_dev *hdev)
{
+ struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
struct btmtk_hci_wmt_params wmt_params;
u8 param = 0x0;
int err;
+ /* Get back the state to be consistent with the state
+ * in btmtksdio_setup.
+ */
+ pm_runtime_get_sync(bdev->dev);
+
/* Disable the device */
wmt_params.op = MTK_WMT_FUNC_CTRL;
wmt_params.flag = 0;
@@ -839,6 +888,9 @@ static int btmtksdio_shutdown(struct hci_dev *hdev)
return err;
}
+ pm_runtime_put_noidle(bdev->dev);
+ pm_runtime_disable(bdev->dev);
+
return 0;
}
@@ -885,6 +937,7 @@ static int btmtksdio_probe(struct sdio_func *func,
if (!bdev->data)
return -ENODEV;
+ bdev->dev = &func->dev;
bdev->func = func;
INIT_WORK(&bdev->tx_work, btmtksdio_tx_work);
@@ -922,6 +975,25 @@ static int btmtksdio_probe(struct sdio_func *func,
sdio_set_drvdata(func, bdev);
+ /* pm_runtime_enable would be done after the firmware is being
+ * downloaded because the core layer probably already enables
+ * runtime PM for this func such as the case host->caps &
+ * MMC_CAP_POWER_OFF_CARD.
+ */
+ if (pm_runtime_enabled(bdev->dev))
+ pm_runtime_disable(bdev->dev);
+
+ /* As explaination in drivers/mmc/core/sdio_bus.c tells us:
+ * Unbound SDIO functions are always suspended.
+ * During probe, the function is set active and the usage count
+ * is incremented. If the driver supports runtime PM,
+ * it should call pm_runtime_put_noidle() in its probe routine and
+ * pm_runtime_get_noresume() in its remove routine.
+ *
+ * So, put a pm_runtime_put_noidle here !
+ */
+ pm_runtime_put_noidle(bdev->dev);
+
return 0;
}
@@ -933,6 +1005,9 @@ static void btmtksdio_remove(struct sdio_func *func)
if (!bdev)
return;
+ /* Be consistent the state in btmtksdio_probe */
+ pm_runtime_get_noresume(bdev->dev);
+
hdev = bdev->hdev;
sdio_set_drvdata(func, NULL);
@@ -940,15 +1015,84 @@ static void btmtksdio_remove(struct sdio_func *func)
hci_free_dev(hdev);
}
+#ifdef CONFIG_PM
+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);
+ if (!bdev)
+ return 0;
+
+ sdio_claim_host(bdev->func);
+
+ 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:
+ bt_dev_info(bdev->hdev, "status (%d) return ownership to device", err);
+
+ sdio_release_host(bdev->func);
+
+ return err;
+}
+
+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);
+ if (!bdev)
+ 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 = 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;
+}
+
+static UNIVERSAL_DEV_PM_OPS(btmtksdio_pm_ops, btmtksdio_runtime_suspend,
+ btmtksdio_runtime_resume, NULL);
+#define BTMTKSDIO_PM_OPS (&btmtksdio_pm_ops)
+#else /* CONFIG_PM */
+#define BTMTKSDIO_PM_OPS NULL
+#endif /* CONFIG_PM */
+
static struct sdio_driver btmtksdio_driver = {
.name = "btmtksdio",
.probe = btmtksdio_probe,
.remove = btmtksdio_remove,
.id_table = btmtksdio_table,
+ .drv = {
+ .owner = THIS_MODULE,
+ .pm = BTMTKSDIO_PM_OPS,
+ }
};
module_sdio_driver(btmtksdio_driver);
+module_param(enable_autosuspend, bool, 0644);
+MODULE_PARM_DESC(enable_autosuspend, "Enable autosuspend by default");
+
MODULE_AUTHOR("Sean Wang <[email protected]>");
MODULE_DESCRIPTION("MediaTek Bluetooth SDIO driver ver " VERSION);
MODULE_VERSION(VERSION);
--
2.18.0
From: Sean Wang <[email protected]>
Add a register bit definition about CHLPCR bit 8 because the bit is quite
different in the meaning between reading and writing that bit.
The patch adds a definition particularly for the bit read to avoid the
confusion about using write definition to read the bit.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 681e3e34977e..9c123a9de673 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -56,7 +56,8 @@ static const struct sdio_device_id btmtksdio_table[] = {
#define MTK_REG_CHLPCR 0x4 /* W1S */
#define C_INT_EN_SET BIT(0)
#define C_INT_EN_CLR BIT(1)
-#define C_FW_OWN_REQ_SET BIT(8)
+#define C_FW_OWN_REQ_SET BIT(8) /* For write */
+#define C_COM_DRV_OWN BIT(8) /* For read */
#define C_FW_OWN_REQ_CLR BIT(9)
#define MTK_REG_CSDIOCSR 0x8
@@ -526,7 +527,7 @@ static int btmtksdio_open(struct hci_dev *hdev)
goto err_disable_func;
err = readx_poll_timeout(btmtksdio_drv_own_query, bdev, status,
- status & C_FW_OWN_REQ_SET, 2000, 1000000);
+ status & C_COM_DRV_OWN, 2000, 1000000);
if (err < 0) {
bt_dev_err(bdev->hdev, "Cannot get ownership from device");
goto err_disable_func;
@@ -606,7 +607,7 @@ static int btmtksdio_close(struct hci_dev *hdev)
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_FW_OWN_REQ_SET), 2000, 1000000);
+ !(status & C_COM_DRV_OWN), 2000, 1000000);
if (err < 0)
bt_dev_err(bdev->hdev, "Cannot return ownership to device");
--
2.18.0
From: Sean Wang <[email protected]>
bt_dev logging macros already include a newline at each output
so drop these unnecessary additional newlines in the driver.
Signed-off-by: Sean Wang <[email protected]>
---
drivers/bluetooth/btmtksdio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 7d0d1cb93b0e..681e3e34977e 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -487,15 +487,15 @@ static void btmtksdio_interrupt(struct sdio_func *func)
sdio_writel(func, int_status, MTK_REG_CHISR, NULL);
if (unlikely(!int_status))
- bt_dev_err(bdev->hdev, "CHISR is 0\n");
+ bt_dev_err(bdev->hdev, "CHISR is 0");
if (int_status & FW_OWN_BACK_INT)
- bt_dev_dbg(bdev->hdev, "Get fw own back\n");
+ bt_dev_dbg(bdev->hdev, "Get fw own back");
if (int_status & TX_EMPTY)
schedule_work(&bdev->tx_work);
else if (unlikely(int_status & TX_FIFO_OVERFLOW))
- bt_dev_warn(bdev->hdev, "Tx fifo overflow\n");
+ bt_dev_warn(bdev->hdev, "Tx fifo overflow");
if (int_status & RX_DONE_INT) {
rx_size = (int_status & RX_PKT_LEN) >> 16;
--
2.18.0
From: Sean Wang <[email protected]>
Accumulate hdev->stat.byte_rx only for valid packets as btmtkuart doing.
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 9c123a9de673..877c0a831775 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -391,8 +391,6 @@ static int btmtksdio_rx_packet(struct btmtksdio_dev *bdev, u16 rx_size)
if (err < 0)
goto err_kfree_skb;
- bdev->hdev->stat.byte_rx += rx_size;
-
sdio_hdr = (void *)skb->data;
/* We assume the default error as -EILSEQ simply to make the error path
@@ -457,6 +455,8 @@ static int btmtksdio_rx_packet(struct btmtksdio_dev *bdev, u16 rx_size)
/* Complete frame */
(&pkts[i])->recv(bdev->hdev, skb);
+ bdev->hdev->stat.byte_rx += rx_size;
+
return 0;
err_kfree_skb:
--
2.18.0
Hi Sean,
> First three are all minor changes and the final one adds a runtime pm support to
> enter a lower power state for decreasing power dissipation when there is idle
> traffic in SDIO transport for a while.
>
> Sean Wang (4):
> Bluetooth: btmtksdio: Drop newline with bt_dev logging macros
> Bluetooth: btmtksdio: Add a bit definition for CHLPCR
> Bluetooth: btmtksdio: Fix hdev->stat.byte_rx accumulation
> Bluetooth: btmtksdio: Add runtime PM support to SDIO based Bluetooth
>
> drivers/bluetooth/btmtksdio.c | 161 ++++++++++++++++++++++++++++++++--
> 1 file changed, 153 insertions(+), 8 deletions(-)
all 4 patches have been applied to bluetooth-next tree.
Regards
Marcel