The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth
@ 2021-12-22  3:11 sean.wang
  2021-12-22  3:11 ` [PATCH v2 2/3] Bluetooth: mt7921s: Enable SCO over I2S sean.wang
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: sean.wang @ 2021-12-22  3:11 UTC (permalink / raw)
  To: marcel, johan.hedberg
  Cc: Mark-YW.Chen, sean.wang, Soul.Huang, YN.Chen, Leon.Yen,
	Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh,
	posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
	steve.lee, jsiuda, frankgor, jemele, abhishekpandit, michaelfsun,
	mcchou, shawnku, linux-bluetooth, linux-mediatek, linux-kernel,
	Mark Chen

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.

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) {
+		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)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/3] Bluetooth: mt7921s: Enable SCO over I2S
  2021-12-22  3:11 [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth sean.wang
@ 2021-12-22  3:11 ` sean.wang
  2021-12-22  8:38   ` Marcel Holtmann
  2021-12-22  3:11 ` [PATCH v2 3/3] Bluetooth: btmtksdio: move struct reg_read_cmd to common file sean.wang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: sean.wang @ 2021-12-22  3:11 UTC (permalink / raw)
  To: marcel, johan.hedberg
  Cc: Mark-YW.Chen, sean.wang, Soul.Huang, YN.Chen, Leon.Yen,
	Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh,
	posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
	steve.lee, jsiuda, frankgor, jemele, abhishekpandit, michaelfsun,
	mcchou, shawnku, linux-bluetooth, linux-mediatek, linux-kernel,
	Mark Chen

From: Mark Chen <mark-yw.chen@mediatek.com>

The driver has to issue the specific command to enable Bluetooth SCO over
the I2S/PCM interface on mt7921s, that is supported since the firmware
with version 20211215151112 was added, and the patch would not cause any
harm even when the old firmware is applied.

The SCO profile with the patch was tested by setting up a VOIP application,
connected to HFP device, checked telephony function can work normally.

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 git message and fix typo
---
 drivers/bluetooth/btmtk.h     | 20 +++++++++
 drivers/bluetooth/btmtksdio.c | 76 +++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index 2be1d2680ad8..fc57ef09d132 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -7,8 +7,12 @@
 
 #define HCI_WMT_MAX_EVENT_SIZE		64
 
+#define BTMTK_WMT_REG_WRITE 0x1
 #define BTMTK_WMT_REG_READ 0x2
 
+#define MT7921_PINMUX_0 0x70005050
+#define MT7921_PINMUX_1 0x70005054
+
 enum {
 	BTMTK_WMT_PATCH_DWNLD = 0x1,
 	BTMTK_WMT_TEST = 0x2,
@@ -76,6 +80,22 @@ struct btmtk_wakeon {
 	__le16 wakeup_delay;
 } __packed;
 
+struct btmtk_sco {
+	u8 clock_config;
+	u8 transmit_format_config;
+	u8 channel_format_config;
+	u8 channel_select_config;
+} __packed;
+
+struct reg_write_cmd {
+	u8 type;
+	u8 rsv;
+	u8 num;
+	__le32 addr;
+	__le32 data;
+	__le32 mask;
+} __packed;
+
 struct btmtk_hci_wmt_params {
 	u8 op;
 	u8 flag;
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 771733ce362b..c9120bd755d3 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -830,6 +830,74 @@ static int btsdio_mtk_reg_read(struct hci_dev *hdev, u32 reg, u32 *val)
 	return err;
 }
 
+static int btsdio_mtk_reg_write(struct hci_dev *hdev, u32 reg, u32 val, u32 mask)
+{
+	struct btmtk_hci_wmt_params wmt_params;
+	struct reg_write_cmd reg_write = {
+		.type = 1,
+		.num = 1,
+		.addr = cpu_to_le32(reg),
+		.data = cpu_to_le32(val),
+		.mask = cpu_to_le32(mask),
+	};
+	int err, status;
+
+	wmt_params.op = BTMTK_WMT_REGISTER;
+	wmt_params.flag = BTMTK_WMT_REG_WRITE;
+	wmt_params.dlen = sizeof(reg_write);
+	wmt_params.data = &reg_write;
+	wmt_params.status = &status;
+
+	err = mtk_hci_wmt_sync(hdev, &wmt_params);
+	if (err < 0)
+		bt_dev_err(hdev, "Failed to write reg(%d)", err);
+
+	return err;
+}
+
+static int btsdio_mtk_sco_setting(struct hci_dev *hdev)
+{
+	struct btmtk_sco sco_setting = {
+		.clock_config = 0x49,
+		.channel_format_config = 0x80,
+	};
+	struct sk_buff *skb;
+	u32 val;
+	int err;
+
+	/* Enable SCO over I2S/PCM for MediaTek chipset */
+	skb =  __hci_cmd_sync(hdev, 0xfc72, sizeof(sco_setting),
+			      &sco_setting, HCI_CMD_TIMEOUT);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	err = btsdio_mtk_reg_read(hdev, MT7921_PINMUX_0, &val);
+	if (err < 0) {
+		bt_dev_err(hdev, "Failed to read register (%d)", err);
+		return err;
+	}
+
+	val |= 0x11000000;
+	err = btsdio_mtk_reg_write(hdev, MT7921_PINMUX_0, val, ~0);
+	if (err < 0) {
+		bt_dev_err(hdev, "Failed to write register (%d)", err);
+		return err;
+	}
+
+	err = btsdio_mtk_reg_read(hdev, MT7921_PINMUX_1, &val);
+	if (err < 0) {
+		bt_dev_err(hdev, "Failed to read register (%d)", err);
+		return err;
+	}
+
+	val |= 0x00000101;
+	err = btsdio_mtk_reg_write(hdev, MT7921_PINMUX_1, val, ~0);
+	if (err < 0)
+		bt_dev_err(hdev, "Failed to write register (%d)", err);
+
+	return err;
+}
+
 static int btmtksdio_setup(struct hci_dev *hdev)
 {
 	struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
@@ -862,6 +930,14 @@ static int btmtksdio_setup(struct hci_dev *hdev)
 		err = mt79xx_setup(hdev, fwname);
 		if (err < 0)
 			return err;
+
+		/* Enable SCO over I2S/PCM */
+		err = btsdio_mtk_sco_setting(hdev);
+		if (err < 0) {
+			bt_dev_err(hdev, "Failed to enable SCO setting (%d)", err);
+			return err;
+		}
+
 		break;
 	case 0x7663:
 	case 0x7668:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/3] Bluetooth: btmtksdio: move struct reg_read_cmd to common file
  2021-12-22  3:11 [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth sean.wang
  2021-12-22  3:11 ` [PATCH v2 2/3] Bluetooth: mt7921s: Enable SCO over I2S sean.wang
@ 2021-12-22  3:11 ` sean.wang
  2021-12-22  8:35 ` [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth Marcel Holtmann
  2021-12-22 14:24 ` Paul Menzel
  3 siblings, 0 replies; 7+ messages in thread
From: sean.wang @ 2021-12-22  3:11 UTC (permalink / raw)
  To: marcel, johan.hedberg
  Cc: Mark-YW.Chen, sean.wang, Soul.Huang, YN.Chen, Leon.Yen,
	Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh,
	posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
	steve.lee, jsiuda, frankgor, jemele, abhishekpandit, michaelfsun,
	mcchou, shawnku, linux-bluetooth, linux-mediatek, linux-kernel

From: Sean Wang <sean.wang@mediatek.com>

move struct reg_read_cmd to btmtk.h to allow other mtk drivers refer to.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
v2: no change
---
 drivers/bluetooth/btmtk.h     | 7 +++++++
 drivers/bluetooth/btmtksdio.c | 7 +------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index fc57ef09d132..fb76d9765ce0 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -87,6 +87,13 @@ struct btmtk_sco {
 	u8 channel_select_config;
 } __packed;
 
+struct reg_read_cmd {
+	u8 type;
+	u8 rsv;
+	u8 num;
+	__le32 addr;
+} __packed;
+
 struct reg_write_cmd {
 	u8 type;
 	u8 rsv;
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index c9120bd755d3..3f6b3f84077a 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -800,12 +800,7 @@ static int mt79xx_setup(struct hci_dev *hdev, const char *fwname)
 static int btsdio_mtk_reg_read(struct hci_dev *hdev, u32 reg, u32 *val)
 {
 	struct btmtk_hci_wmt_params wmt_params;
-	struct reg_read_cmd {
-		u8 type;
-		u8 rsv;
-		u8 num;
-		__le32 addr;
-	} __packed reg_read = {
+	struct reg_read_cmd reg_read = {
 		.type = 1,
 		.num = 1,
 	};
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth
  2021-12-22  3:11 [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth sean.wang
  2021-12-22  3:11 ` [PATCH v2 2/3] Bluetooth: mt7921s: Enable SCO over I2S sean.wang
  2021-12-22  3:11 ` [PATCH v2 3/3] Bluetooth: btmtksdio: move struct reg_read_cmd to common file sean.wang
@ 2021-12-22  8:35 ` Marcel Holtmann
  2021-12-22 14:24 ` Paul Menzel
  3 siblings, 0 replies; 7+ messages in thread
From: Marcel Holtmann @ 2021-12-22  8:35 UTC (permalink / raw)
  To: Sean Wang
  Cc: Johan Hedberg, "Mark-YW Chen (陳揚文)",
	Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang, Deren.Wu, km.lin,
	robin.chiu, Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
	Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
	abhishekpandit, Michael Sun, Miao-chen Chou, shawnku,
	linux-bluetooth, linux-mediatek, linux-kernel

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/3] Bluetooth: mt7921s: Enable SCO over I2S
  2021-12-22  3:11 ` [PATCH v2 2/3] Bluetooth: mt7921s: Enable SCO over I2S sean.wang
@ 2021-12-22  8:38   ` Marcel Holtmann
  0 siblings, 0 replies; 7+ messages in thread
From: Marcel Holtmann @ 2021-12-22  8:38 UTC (permalink / raw)
  To: Sean Wang
  Cc: Johan Hedberg, "Mark-YW Chen (陳揚文)",
	Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang, Deren.Wu, km.lin,
	robin.chiu, Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang,
	Stella.Chang, Tom.Chou, steve.lee, jsiuda, frankgor, jemele,
	abhishekpandit, michaelfsun, mcchou, shawnku, linux-bluetooth,
	linux-mediatek, linux-kernel

Hi Sean,

> The driver has to issue the specific command to enable Bluetooth SCO over
> the I2S/PCM interface on mt7921s, that is supported since the firmware
> with version 20211215151112 was added, and the patch would not cause any
> harm even when the old firmware is applied.
> 
> The SCO profile with the patch was tested by setting up a VOIP application,
> connected to HFP device, checked telephony function can work normally.
> 
> 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 git message and fix typo
> ---
> drivers/bluetooth/btmtk.h     | 20 +++++++++
> drivers/bluetooth/btmtksdio.c | 76 +++++++++++++++++++++++++++++++++++
> 2 files changed, 96 insertions(+)
> 
> diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> index 2be1d2680ad8..fc57ef09d132 100644
> --- a/drivers/bluetooth/btmtk.h
> +++ b/drivers/bluetooth/btmtk.h
> @@ -7,8 +7,12 @@
> 
> #define HCI_WMT_MAX_EVENT_SIZE		64
> 
> +#define BTMTK_WMT_REG_WRITE 0x1
> #define BTMTK_WMT_REG_READ 0x2
> 
> +#define MT7921_PINMUX_0 0x70005050
> +#define MT7921_PINMUX_1 0x70005054
> +
> enum {
> 	BTMTK_WMT_PATCH_DWNLD = 0x1,
> 	BTMTK_WMT_TEST = 0x2,
> @@ -76,6 +80,22 @@ struct btmtk_wakeon {
> 	__le16 wakeup_delay;
> } __packed;
> 
> +struct btmtk_sco {
> +	u8 clock_config;
> +	u8 transmit_format_config;
> +	u8 channel_format_config;
> +	u8 channel_select_config;
> +} __packed;
> +
> +struct reg_write_cmd {
> +	u8 type;
> +	u8 rsv;
> +	u8 num;
> +	__le32 addr;
> +	__le32 data;
> +	__le32 mask;
> +} __packed;
> +
> struct btmtk_hci_wmt_params {
> 	u8 op;
> 	u8 flag;
> diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
> index 771733ce362b..c9120bd755d3 100644
> --- a/drivers/bluetooth/btmtksdio.c
> +++ b/drivers/bluetooth/btmtksdio.c
> @@ -830,6 +830,74 @@ static int btsdio_mtk_reg_read(struct hci_dev *hdev, u32 reg, u32 *val)
> 	return err;
> }
> 
> +static int btsdio_mtk_reg_write(struct hci_dev *hdev, u32 reg, u32 val, u32 mask)
> +{
> +	struct btmtk_hci_wmt_params wmt_params;
> +	struct reg_write_cmd reg_write = {
> +		.type = 1,
> +		.num = 1,
> +		.addr = cpu_to_le32(reg),
> +		.data = cpu_to_le32(val),
> +		.mask = cpu_to_le32(mask),
> +	};
> +	int err, status;
> +
> +	wmt_params.op = BTMTK_WMT_REGISTER;
> +	wmt_params.flag = BTMTK_WMT_REG_WRITE;
> +	wmt_params.dlen = sizeof(reg_write);
> +	wmt_params.data = &reg_write;
> +	wmt_params.status = &status;
> +
> +	err = mtk_hci_wmt_sync(hdev, &wmt_params);
> +	if (err < 0)
> +		bt_dev_err(hdev, "Failed to write reg(%d)", err);
> +
> +	return err;
> +}
> +
> +static int btsdio_mtk_sco_setting(struct hci_dev *hdev)
> +{
> +	struct btmtk_sco sco_setting = {
> +		.clock_config = 0x49,
> +		.channel_format_config = 0x80,
> +	};

const?

> +	struct sk_buff *skb;
> +	u32 val;
> +	int err;
> +
> +	/* Enable SCO over I2S/PCM for MediaTek chipset */
> +	skb =  __hci_cmd_sync(hdev, 0xfc72, sizeof(sco_setting),
> +			      &sco_setting, HCI_CMD_TIMEOUT);
> +	if (IS_ERR(skb))
> +		return PTR_ERR(skb);

This is also leaking the skb.

> +
> +	err = btsdio_mtk_reg_read(hdev, MT7921_PINMUX_0, &val);
> +	if (err < 0) {
> +		bt_dev_err(hdev, "Failed to read register (%d)", err);
> +		return err;
> +	}
> +
> +	val |= 0x11000000;
> +	err = btsdio_mtk_reg_write(hdev, MT7921_PINMUX_0, val, ~0);
> +	if (err < 0) {
> +		bt_dev_err(hdev, "Failed to write register (%d)", err);
> +		return err;
> +	}
> +
> +	err = btsdio_mtk_reg_read(hdev, MT7921_PINMUX_1, &val);
> +	if (err < 0) {
> +		bt_dev_err(hdev, "Failed to read register (%d)", err);
> +		return err;
> +	}
> +
> +	val |= 0x00000101;
> +	err = btsdio_mtk_reg_write(hdev, MT7921_PINMUX_1, val, ~0);
> +	if (err < 0)
> +		bt_dev_err(hdev, "Failed to write register (%d)", err);
> +
> +	return err;
> +}
> +
> static int btmtksdio_setup(struct hci_dev *hdev)
> {
> 	struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
> @@ -862,6 +930,14 @@ static int btmtksdio_setup(struct hci_dev *hdev)
> 		err = mt79xx_setup(hdev, fwname);
> 		if (err < 0)
> 			return err;
> +
> +		/* Enable SCO over I2S/PCM */
> +		err = btsdio_mtk_sco_setting(hdev);
> +		if (err < 0) {
> +			bt_dev_err(hdev, "Failed to enable SCO setting (%d)", err);
> +			return err;
> +		}
> +
> 		break;
> 	case 0x7663:
> 	case 0x7668:

Regards

Marcel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth
  2021-12-22  3:11 [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth sean.wang
                   ` (2 preceding siblings ...)
  2021-12-22  8:35 ` [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth Marcel Holtmann
@ 2021-12-22 14:24 ` Paul Menzel
  3 siblings, 0 replies; 7+ messages in thread
From: Paul Menzel @ 2021-12-22 14:24 UTC (permalink / raw)
  To: Sean Wang, Marcel Holtmann, Johan Hedberg
  Cc: Mark-YW.Chen, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
	Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh, posh.sun,
	ted.huang, Eric.Liang, Stella.Chang, Tom.Chou, steve.lee, jsiuda,
	frankgor, jemele, abhishekpandit, michaelfsun, mcchou, shawnku,
	linux-bluetooth, linux-mediatek, linux-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth
       [not found] <f6baa415-b078-4308-e31d-6b44e748c1c5@molgen.mpg.de--annotate>
@ 2021-12-22 19:54 ` sean.wang
  0 siblings, 0 replies; 7+ messages in thread
From: sean.wang @ 2021-12-22 19:54 UTC (permalink / raw)
  To: pmenzel
  Cc: marcel, johan.hedberg, Mark-YW.Chen, sean.wang, Soul.Huang,
	YN.Chen, Leon.Yen, Eric-SY.Chang, Deren.Wu, km.lin, robin.chiu,
	Eddie.Chen, ch.yeh, posh.sun, ted.huang, Eric.Liang, Stella.Chang,
	Tom.Chou, steve.lee, jsiuda, frankgor, jemele, abhishekpandit,
	michaelfsun, mcchou, shawnku, linux-bluetooth, linux-mediatek,
	linux-kernel

From: Sean Wang <sean.wang@mediatek.com>

>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?
>

mt7921 firmware will be updated regularly to add features or fix bugs, so I would try to avoid adding such
message related to firmware to make the driver look messy.

>> 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.
>

actually the hardware datasheet doesn't mention the things regarding wake on bluetooth because that is a totally software feature
extended from the previous firmware.

>> 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.

thanks, I will do it with another patch

>
>>	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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-12-22 19:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-22  3:11 [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth sean.wang
2021-12-22  3:11 ` [PATCH v2 2/3] Bluetooth: mt7921s: Enable SCO over I2S sean.wang
2021-12-22  8:38   ` Marcel Holtmann
2021-12-22  3:11 ` [PATCH v2 3/3] Bluetooth: btmtksdio: move struct reg_read_cmd to common file sean.wang
2021-12-22  8:35 ` [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth Marcel Holtmann
2021-12-22 14:24 ` Paul Menzel
     [not found] <f6baa415-b078-4308-e31d-6b44e748c1c5@molgen.mpg.de--annotate>
2021-12-22 19:54 ` sean.wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox