| Message ID | ef0603f2553d09fb84043704e4726d0ab98aa63b.1640141813.git.objelf@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/3] Bluetooth: mt7921s: Support wake on bluetooth | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
| tedd_an/buildkernel | success | Build Kernel PASS |
| tedd_an/incremental_build | success | Pass |
| tedd_an/testrunnersetup | success | Test Runner Setup PASS |
| tedd_an/testrunnerl2cap-tester | success | Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnerbnep-tester | success | Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnermgmt-tester | success | Total: 493, Passed: 493 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnerrfcomm-tester | success | Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersco-tester | success | Total: 12, Passed: 12 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersmp-tester | success | Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunneruserchan-tester | success | Total: 4, Passed: 4 (100.0%), Failed: 0, Not Run: 0 |
Hi Sean, > Enable wake on bluetooth on mt7921s that can be supported since the > firmware with version 20211129211059 was added, and the patch would > not cause any harm even when the old firmware is applied. > > The patch was tested by setting up an HID or HOGP profile to connect a > Bluetooth keyboard and mouse, then putting the system to suspend, then > trying to wake up the system by moving the Bluetooth keyboard or mouse, > and then checking if the system can wake up and be brought back to > the normal state. > > Co-developed-by: Sean Wang <sean.wang@mediatek.com> > Signed-off-by: Sean Wang <sean.wang@mediatek.com> > Signed-off-by: Mark Chen <mark-yw.chen@mediatek.com> > --- > v2: refine the git message > --- > drivers/bluetooth/btmtk.h | 8 ++++++++ > drivers/bluetooth/btmtksdio.c | 31 ++++++++++++++++++++++++++++++- > 2 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h > index 6e7b0c7567c0..2be1d2680ad8 100644 > --- a/drivers/bluetooth/btmtk.h > +++ b/drivers/bluetooth/btmtk.h > @@ -68,6 +68,14 @@ struct btmtk_tci_sleep { > u8 time_compensation; > } __packed; > > +struct btmtk_wakeon { > + u8 mode; > + u8 gpo; > + u8 active_high; > + __le16 enable_delay; > + __le16 wakeup_delay; > +} __packed; > + > struct btmtk_hci_wmt_params { > u8 op; > u8 flag; > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c > index b5ea8d3bffaa..771733ce362b 100644 > --- a/drivers/bluetooth/btmtksdio.c > +++ b/drivers/bluetooth/btmtksdio.c > @@ -958,6 +958,30 @@ static int btmtksdio_send_frame(struct hci_dev *hdev, struct sk_buff *skb) > return 0; > } > > +static bool btmtk_sdio_wakeup(struct hci_dev *hdev) > +{ > + struct btmtksdio_dev *bdev = hci_get_drvdata(hdev); > + bool may_wakeup = device_may_wakeup(bdev->dev); > + struct btmtk_wakeon bt_awake = { > + .mode = 0x1, > + .gpo = 0, > + .active_high = 0x1, > + .enable_delay = cpu_to_le16(0xc80), > + .wakeup_delay = cpu_to_le16(0x20) > + }; > + struct sk_buff *skb; > + > + if (may_wakeup && > + bdev->data->chipid == 0x7921) { this doesn’t fit in a single line? > + skb = __hci_cmd_sync(hdev, 0xfc27, sizeof(bt_awake), > + &bt_awake, HCI_CMD_TIMEOUT); > + if (IS_ERR(skb)) > + may_wakeup = false; Memory leak? Any why not move the skb variable into the scope where it is used. > + } > + > + return may_wakeup; > +} > + > static int btmtksdio_probe(struct sdio_func *func, > const struct sdio_device_id *id) > { > @@ -998,6 +1022,7 @@ static int btmtksdio_probe(struct sdio_func *func, > hdev->shutdown = btmtksdio_shutdown; > hdev->send = btmtksdio_send_frame; > hdev->set_bdaddr = btmtk_set_bdaddr; > + hdev->wakeup = btmtk_sdio_wakeup; > > SET_HCIDEV_DEV(hdev, &func->dev); > > @@ -1032,7 +1057,11 @@ static int btmtksdio_probe(struct sdio_func *func, > */ > pm_runtime_put_noidle(bdev->dev); > > - return 0; > + err = device_init_wakeup(bdev->dev, true); > + if (err) > + bt_dev_err(hdev, "%s: failed to init_wakeup", __func__); > + > + return err; > } > > static void btmtksdio_remove(struct sdio_func *func) Regards Marcel
Dear Sean, Am 22.12.21 um 04:11 schrieb sean.wang@mediatek.com: > From: Mark Chen <mark-yw.chen@mediatek.com> > > Enable wake on bluetooth on mt7921s that can be supported since the > firmware with version 20211129211059 was added, and the patch would > not cause any harm even when the old firmware is applied. Maybe print a notice level message, if the firmware is too old, and the feature cannot be supported? > The patch was tested by setting up an HID or HOGP profile to connect a > Bluetooth keyboard and mouse, then putting the system to suspend, then > trying to wake up the system by moving the Bluetooth keyboard or mouse, > and then checking if the system can wake up and be brought back to > the normal state. I’d still would like to see the datasheet name, revision, and section in the commit message, even though the datasheet is not public. > Co-developed-by: Sean Wang <sean.wang@mediatek.com> > Signed-off-by: Sean Wang <sean.wang@mediatek.com> > Signed-off-by: Mark Chen <mark-yw.chen@mediatek.com> > --- > v2: refine the git message > --- > drivers/bluetooth/btmtk.h | 8 ++++++++ > drivers/bluetooth/btmtksdio.c | 31 ++++++++++++++++++++++++++++++- > 2 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h > index 6e7b0c7567c0..2be1d2680ad8 100644 > --- a/drivers/bluetooth/btmtk.h > +++ b/drivers/bluetooth/btmtk.h > @@ -68,6 +68,14 @@ struct btmtk_tci_sleep { > u8 time_compensation; > } __packed; > > +struct btmtk_wakeon { > + u8 mode; > + u8 gpo; > + u8 active_high; > + __le16 enable_delay; > + __le16 wakeup_delay; > +} __packed; > + > struct btmtk_hci_wmt_params { > u8 op; > u8 flag; > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c > index b5ea8d3bffaa..771733ce362b 100644 > --- a/drivers/bluetooth/btmtksdio.c > +++ b/drivers/bluetooth/btmtksdio.c > @@ -958,6 +958,30 @@ static int btmtksdio_send_frame(struct hci_dev *hdev, struct sk_buff *skb) > return 0; > } > > +static bool btmtk_sdio_wakeup(struct hci_dev *hdev) > +{ > + struct btmtksdio_dev *bdev = hci_get_drvdata(hdev); > + bool may_wakeup = device_may_wakeup(bdev->dev); > + struct btmtk_wakeon bt_awake = { > + .mode = 0x1, > + .gpo = 0, > + .active_high = 0x1, > + .enable_delay = cpu_to_le16(0xc80), > + .wakeup_delay = cpu_to_le16(0x20) > + }; > + struct sk_buff *skb; > + > + if (may_wakeup && > + bdev->data->chipid == 0x7921) { > + skb = __hci_cmd_sync(hdev, 0xfc27, sizeof(bt_awake), > + &bt_awake, HCI_CMD_TIMEOUT); > + if (IS_ERR(skb)) > + may_wakeup = false; > + } > + > + return may_wakeup; > +} > + > static int btmtksdio_probe(struct sdio_func *func, > const struct sdio_device_id *id) > { > @@ -998,6 +1022,7 @@ static int btmtksdio_probe(struct sdio_func *func, > hdev->shutdown = btmtksdio_shutdown; > hdev->send = btmtksdio_send_frame; > hdev->set_bdaddr = btmtk_set_bdaddr; > + hdev->wakeup = btmtk_sdio_wakeup; Just a nit, that two and three lines above, the equal signs are aligned. > SET_HCIDEV_DEV(hdev, &func->dev); > > @@ -1032,7 +1057,11 @@ static int btmtksdio_probe(struct sdio_func *func, > */ > pm_runtime_put_noidle(bdev->dev); > > - return 0; > + err = device_init_wakeup(bdev->dev, true); > + if (err) > + bt_dev_err(hdev, "%s: failed to init_wakeup", __func__); > + > + return err; > } > > static void btmtksdio_remove(struct sdio_func *func) Kind regards, Paul
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h index 6e7b0c7567c0..2be1d2680ad8 100644 --- a/drivers/bluetooth/btmtk.h +++ b/drivers/bluetooth/btmtk.h @@ -68,6 +68,14 @@ struct btmtk_tci_sleep { u8 time_compensation; } __packed; +struct btmtk_wakeon { + u8 mode; + u8 gpo; + u8 active_high; + __le16 enable_delay; + __le16 wakeup_delay; +} __packed; + struct btmtk_hci_wmt_params { u8 op; u8 flag; diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c index b5ea8d3bffaa..771733ce362b 100644 --- a/drivers/bluetooth/btmtksdio.c +++ b/drivers/bluetooth/btmtksdio.c @@ -958,6 +958,30 @@ static int btmtksdio_send_frame(struct hci_dev *hdev, struct sk_buff *skb) return 0; } +static bool btmtk_sdio_wakeup(struct hci_dev *hdev) +{ + struct btmtksdio_dev *bdev = hci_get_drvdata(hdev); + bool may_wakeup = device_may_wakeup(bdev->dev); + struct btmtk_wakeon bt_awake = { + .mode = 0x1, + .gpo = 0, + .active_high = 0x1, + .enable_delay = cpu_to_le16(0xc80), + .wakeup_delay = cpu_to_le16(0x20) + }; + struct sk_buff *skb; + + if (may_wakeup && + bdev->data->chipid == 0x7921) { + skb = __hci_cmd_sync(hdev, 0xfc27, sizeof(bt_awake), + &bt_awake, HCI_CMD_TIMEOUT); + if (IS_ERR(skb)) + may_wakeup = false; + } + + return may_wakeup; +} + static int btmtksdio_probe(struct sdio_func *func, const struct sdio_device_id *id) { @@ -998,6 +1022,7 @@ static int btmtksdio_probe(struct sdio_func *func, hdev->shutdown = btmtksdio_shutdown; hdev->send = btmtksdio_send_frame; hdev->set_bdaddr = btmtk_set_bdaddr; + hdev->wakeup = btmtk_sdio_wakeup; SET_HCIDEV_DEV(hdev, &func->dev); @@ -1032,7 +1057,11 @@ static int btmtksdio_probe(struct sdio_func *func, */ pm_runtime_put_noidle(bdev->dev); - return 0; + err = device_init_wakeup(bdev->dev, true); + if (err) + bt_dev_err(hdev, "%s: failed to init_wakeup", __func__); + + return err; } static void btmtksdio_remove(struct sdio_func *func)