Add a couple of enhancements to btmtksdio
Subsystems (3)
cover Add a couple of enhancements to btmtksdio
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(-)
Signature · 1 line
2.18.0
PATCH 1/4 Bluetooth: btmtksdio: Drop newline with bt_dev logging macros
From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/[email protected]>
bt_dev logging macros already include a newline at each output so drop these unnecessary additional newlines in the driver.
--- drivers/bluetooth/btmtksdio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/bluetooth/btmtksdio.c +3 -3
@@ -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 ;
Signature · 1 line
2.18.0
PATCH 2/4 Bluetooth: btmtksdio: Add a bit definition for CHLPCR
In reply to [email protected]From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/[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.
--- drivers/bluetooth/btmtksdio.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/bluetooth/btmtksdio.c +4 -3
@@ -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" );
Signature · 1 line
2.18.0
PATCH 3/4 Bluetooth: btmtksdio: Fix hdev->stat.byte_rx accumulation
In reply to [email protected]From: Sean Wang <[email protected]>
Accumulate hdev->stat.byte_rx only for valid packets as btmtkuart doing.
--- drivers/bluetooth/btmtksdio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/bluetooth/btmtksdio.c +2 -2
@@ -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:
Signature · 1 line
2.18.0
PATCH 4/4 Bluetooth: btmtksdio: Add runtime PM support to SDIO based Bluetooth
In reply to [email protected]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.
--- drivers/bluetooth/btmtksdio.c | 144 ++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+)
drivers/bluetooth/btmtksdio.c +144 -0
@@ -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 );
Signature · 1 line
2.18.0
Marcel Holtmann
maintainerHi Sean,
Quoted 4 lines
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.
Quoted 8 lines
Sean Wang (4):
Bluetooth: btmtksdio: Drop newline withbt_devlogging macros
Bluetooth: btmtksdio: Add a bit definition for CHLPCR
Bluetooth: btmtksdio: Fixhdev->stat.byte_rxaccumulation
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