Re: [PATCH v2 1/3] Bluetooth: mt7921s: Support wake on bluetooth

[<sean.wang@mediatek.com>]

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