All of lore.kernel.org
 help / color / mirror / Atom feed
* [kernel PATCH v1] Bluetooth: btmtksdio: clear BTMTKSDIO_BT_WAKE_ENABLED after resume
@ 2023-12-08 21:07 ` Zhengping Jiang
  0 siblings, 0 replies; 5+ messages in thread
From: Zhengping Jiang @ 2023-12-08 21:07 UTC (permalink / raw)
  To: linux-bluetooth, marcel, luiz.dentz
  Cc: chromeos-bluetooth-upstreaming, Zhengping Jiang,
	AngeloGioacchino Del Regno, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Johan Hedberg, Matthias Brugger, Paolo Abeni,
	linux-arm-kernel, linux-kernel, linux-mediatek, netdev

Always clear BTMTKSDIO_BT_WAKE_ENABLED bit after resume. When Bluetooth
does not generate interrupts, the bit will not be cleared and causes
premature wakeup.

Fixes: 4ed924fc122f ("Bluetooth: btmtksdio: enable bluetooth wakeup in system suspend")
Signed-off-by: Zhengping Jiang <jiangzp@google.com>
---

Changes in v1:
- Clear BTMTKSDIO_BT_WAKE_ENABLED flag on resume

 drivers/bluetooth/btmtksdio.c    | 10 ++++++++++
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_sync.c         |  2 ++
 3 files changed, 13 insertions(+)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index ff4868c83cd8..8f00b71573c8 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -1296,6 +1296,15 @@ static bool btmtksdio_sdio_inband_wakeup(struct hci_dev *hdev)
 	return device_may_wakeup(bdev->dev);
 }
 
+static void btmtksdio_disable_bt_wakeup(struct hci_dev *hdev)
+{
+	struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
+
+	if (!bdev)
+		return;
+	clear_bit(BTMTKSDIO_BT_WAKE_ENABLED, &bdev->tx_state);
+}
+
 static bool btmtksdio_sdio_wakeup(struct hci_dev *hdev)
 {
 	struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
@@ -1363,6 +1372,7 @@ static int btmtksdio_probe(struct sdio_func *func,
 	hdev->shutdown = btmtksdio_shutdown;
 	hdev->send     = btmtksdio_send_frame;
 	hdev->wakeup   = btmtksdio_sdio_wakeup;
+	hdev->clear_wakeup = btmtksdio_disable_bt_wakeup;
 	/*
 	 * If SDIO controller supports wake on Bluetooth, sending a wakeon
 	 * command is not necessary.
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0c1754f416bd..4bbd55335269 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -672,6 +672,7 @@ struct hci_dev {
 	int (*get_codec_config_data)(struct hci_dev *hdev, __u8 type,
 				     struct bt_codec *codec, __u8 *vnd_len,
 				     __u8 **vnd_data);
+	void (*clear_wakeup)(struct hci_dev *hdev);
 };
 
 #define HCI_PHY_HANDLE(handle)	(handle & 0xff)
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 3563a90ed2ac..6c4d5ce40524 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5947,6 +5947,8 @@ int hci_resume_sync(struct hci_dev *hdev)
 		return 0;
 
 	hdev->suspended = false;
+	if (hdev->clear_wakeup)
+		hdev->clear_wakeup(hdev);
 
 	/* Restore event mask */
 	hci_set_event_mask_sync(hdev);
-- 
2.43.0.472.g3155946c3a-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [kernel PATCH v1] Bluetooth: btmtksdio: clear BTMTKSDIO_BT_WAKE_ENABLED after resume
@ 2023-12-08 21:07 ` Zhengping Jiang
  0 siblings, 0 replies; 5+ messages in thread
From: Zhengping Jiang @ 2023-12-08 21:07 UTC (permalink / raw)
  To: linux-bluetooth, marcel, luiz.dentz
  Cc: chromeos-bluetooth-upstreaming, Zhengping Jiang,
	AngeloGioacchino Del Regno, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Johan Hedberg, Matthias Brugger, Paolo Abeni,
	linux-arm-kernel, linux-kernel, linux-mediatek, netdev

Always clear BTMTKSDIO_BT_WAKE_ENABLED bit after resume. When Bluetooth
does not generate interrupts, the bit will not be cleared and causes
premature wakeup.

Fixes: 4ed924fc122f ("Bluetooth: btmtksdio: enable bluetooth wakeup in system suspend")
Signed-off-by: Zhengping Jiang <jiangzp@google.com>
---

Changes in v1:
- Clear BTMTKSDIO_BT_WAKE_ENABLED flag on resume

 drivers/bluetooth/btmtksdio.c    | 10 ++++++++++
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_sync.c         |  2 ++
 3 files changed, 13 insertions(+)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index ff4868c83cd8..8f00b71573c8 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -1296,6 +1296,15 @@ static bool btmtksdio_sdio_inband_wakeup(struct hci_dev *hdev)
 	return device_may_wakeup(bdev->dev);
 }
 
+static void btmtksdio_disable_bt_wakeup(struct hci_dev *hdev)
+{
+	struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
+
+	if (!bdev)
+		return;
+	clear_bit(BTMTKSDIO_BT_WAKE_ENABLED, &bdev->tx_state);
+}
+
 static bool btmtksdio_sdio_wakeup(struct hci_dev *hdev)
 {
 	struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
@@ -1363,6 +1372,7 @@ static int btmtksdio_probe(struct sdio_func *func,
 	hdev->shutdown = btmtksdio_shutdown;
 	hdev->send     = btmtksdio_send_frame;
 	hdev->wakeup   = btmtksdio_sdio_wakeup;
+	hdev->clear_wakeup = btmtksdio_disable_bt_wakeup;
 	/*
 	 * If SDIO controller supports wake on Bluetooth, sending a wakeon
 	 * command is not necessary.
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0c1754f416bd..4bbd55335269 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -672,6 +672,7 @@ struct hci_dev {
 	int (*get_codec_config_data)(struct hci_dev *hdev, __u8 type,
 				     struct bt_codec *codec, __u8 *vnd_len,
 				     __u8 **vnd_data);
+	void (*clear_wakeup)(struct hci_dev *hdev);
 };
 
 #define HCI_PHY_HANDLE(handle)	(handle & 0xff)
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 3563a90ed2ac..6c4d5ce40524 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5947,6 +5947,8 @@ int hci_resume_sync(struct hci_dev *hdev)
 		return 0;
 
 	hdev->suspended = false;
+	if (hdev->clear_wakeup)
+		hdev->clear_wakeup(hdev);
 
 	/* Restore event mask */
 	hci_set_event_mask_sync(hdev);
-- 
2.43.0.472.g3155946c3a-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* RE: [kernel,v1] Bluetooth: btmtksdio: clear BTMTKSDIO_BT_WAKE_ENABLED after resume
  2023-12-08 21:07 ` Zhengping Jiang
  (?)
@ 2023-12-08 21:58 ` bluez.test.bot
  -1 siblings, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2023-12-08 21:58 UTC (permalink / raw)
  To: linux-bluetooth, jiangzp

[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=808392

---Test result---

Test Summary:
CheckPatch                    PASS      1.05 seconds
GitLint                       PASS      0.36 seconds
SubjectPrefix                 PASS      0.16 seconds
BuildKernel                   PASS      31.33 seconds
CheckAllWarning               PASS      34.58 seconds
CheckSparse                   PASS      41.02 seconds
CheckSmatch                   PASS      112.13 seconds
BuildKernel32                 PASS      30.70 seconds
TestRunnerSetup               PASS      473.45 seconds
TestRunner_l2cap-tester       PASS      24.56 seconds
TestRunner_iso-tester         PASS      47.06 seconds
TestRunner_bnep-tester        PASS      7.56 seconds
TestRunner_mgmt-tester        PASS      171.85 seconds
TestRunner_rfcomm-tester      PASS      11.66 seconds
TestRunner_sco-tester         PASS      15.22 seconds
TestRunner_ioctl-tester       PASS      13.18 seconds
TestRunner_mesh-tester        PASS      10.50 seconds
TestRunner_smp-tester         PASS      11.30 seconds
TestRunner_userchan-tester    PASS      8.04 seconds
IncrementalBuild              PASS      29.36 seconds



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [kernel PATCH v1] Bluetooth: btmtksdio: clear BTMTKSDIO_BT_WAKE_ENABLED after resume
  2023-12-08 21:07 ` Zhengping Jiang
@ 2023-12-19 18:06   ` Luiz Augusto von Dentz
  -1 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2023-12-19 18:06 UTC (permalink / raw)
  To: Zhengping Jiang
  Cc: linux-bluetooth, marcel, chromeos-bluetooth-upstreaming,
	AngeloGioacchino Del Regno, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Johan Hedberg, Matthias Brugger, Paolo Abeni,
	linux-arm-kernel, linux-kernel, linux-mediatek, netdev

Hi Zhengping,

On Fri, Dec 8, 2023 at 4:07 PM Zhengping Jiang <jiangzp@google.com> wrote:
>
> Always clear BTMTKSDIO_BT_WAKE_ENABLED bit after resume. When Bluetooth
> does not generate interrupts, the bit will not be cleared and causes
> premature wakeup.
>
> Fixes: 4ed924fc122f ("Bluetooth: btmtksdio: enable bluetooth wakeup in system suspend")
> Signed-off-by: Zhengping Jiang <jiangzp@google.com>
> ---
>
> Changes in v1:
> - Clear BTMTKSDIO_BT_WAKE_ENABLED flag on resume
>
>  drivers/bluetooth/btmtksdio.c    | 10 ++++++++++
>  include/net/bluetooth/hci_core.h |  1 +
>  net/bluetooth/hci_sync.c         |  2 ++
>  3 files changed, 13 insertions(+)
>
> diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
> index ff4868c83cd8..8f00b71573c8 100644
> --- a/drivers/bluetooth/btmtksdio.c
> +++ b/drivers/bluetooth/btmtksdio.c
> @@ -1296,6 +1296,15 @@ static bool btmtksdio_sdio_inband_wakeup(struct hci_dev *hdev)
>         return device_may_wakeup(bdev->dev);
>  }
>
> +static void btmtksdio_disable_bt_wakeup(struct hci_dev *hdev)
> +{
> +       struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
> +
> +       if (!bdev)
> +               return;
> +       clear_bit(BTMTKSDIO_BT_WAKE_ENABLED, &bdev->tx_state);
> +}
> +
>  static bool btmtksdio_sdio_wakeup(struct hci_dev *hdev)
>  {
>         struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
> @@ -1363,6 +1372,7 @@ static int btmtksdio_probe(struct sdio_func *func,
>         hdev->shutdown = btmtksdio_shutdown;
>         hdev->send     = btmtksdio_send_frame;
>         hdev->wakeup   = btmtksdio_sdio_wakeup;
> +       hdev->clear_wakeup = btmtksdio_disable_bt_wakeup;
>         /*
>          * If SDIO controller supports wake on Bluetooth, sending a wakeon
>          * command is not necessary.
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 0c1754f416bd..4bbd55335269 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -672,6 +672,7 @@ struct hci_dev {
>         int (*get_codec_config_data)(struct hci_dev *hdev, __u8 type,
>                                      struct bt_codec *codec, __u8 *vnd_len,
>                                      __u8 **vnd_data);
> +       void (*clear_wakeup)(struct hci_dev *hdev);

I wonder if it wouldn't be a better idea to add something like suspend
and resume callbacks to notify the about these hdev states, that way
we can synchronize the states better and avoid having to clear the
wakeup state when it shouldn't be active to begin with since the hdev
is not suspended.

>  };
>
>  #define HCI_PHY_HANDLE(handle) (handle & 0xff)
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 3563a90ed2ac..6c4d5ce40524 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5947,6 +5947,8 @@ int hci_resume_sync(struct hci_dev *hdev)
>                 return 0;
>
>         hdev->suspended = false;
> +       if (hdev->clear_wakeup)
> +               hdev->clear_wakeup(hdev);
>
>         /* Restore event mask */
>         hci_set_event_mask_sync(hdev);
> --
> 2.43.0.472.g3155946c3a-goog
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [kernel PATCH v1] Bluetooth: btmtksdio: clear BTMTKSDIO_BT_WAKE_ENABLED after resume
@ 2023-12-19 18:06   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2023-12-19 18:06 UTC (permalink / raw)
  To: Zhengping Jiang
  Cc: linux-bluetooth, marcel, chromeos-bluetooth-upstreaming,
	AngeloGioacchino Del Regno, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Johan Hedberg, Matthias Brugger, Paolo Abeni,
	linux-arm-kernel, linux-kernel, linux-mediatek, netdev

Hi Zhengping,

On Fri, Dec 8, 2023 at 4:07 PM Zhengping Jiang <jiangzp@google.com> wrote:
>
> Always clear BTMTKSDIO_BT_WAKE_ENABLED bit after resume. When Bluetooth
> does not generate interrupts, the bit will not be cleared and causes
> premature wakeup.
>
> Fixes: 4ed924fc122f ("Bluetooth: btmtksdio: enable bluetooth wakeup in system suspend")
> Signed-off-by: Zhengping Jiang <jiangzp@google.com>
> ---
>
> Changes in v1:
> - Clear BTMTKSDIO_BT_WAKE_ENABLED flag on resume
>
>  drivers/bluetooth/btmtksdio.c    | 10 ++++++++++
>  include/net/bluetooth/hci_core.h |  1 +
>  net/bluetooth/hci_sync.c         |  2 ++
>  3 files changed, 13 insertions(+)
>
> diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
> index ff4868c83cd8..8f00b71573c8 100644
> --- a/drivers/bluetooth/btmtksdio.c
> +++ b/drivers/bluetooth/btmtksdio.c
> @@ -1296,6 +1296,15 @@ static bool btmtksdio_sdio_inband_wakeup(struct hci_dev *hdev)
>         return device_may_wakeup(bdev->dev);
>  }
>
> +static void btmtksdio_disable_bt_wakeup(struct hci_dev *hdev)
> +{
> +       struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
> +
> +       if (!bdev)
> +               return;
> +       clear_bit(BTMTKSDIO_BT_WAKE_ENABLED, &bdev->tx_state);
> +}
> +
>  static bool btmtksdio_sdio_wakeup(struct hci_dev *hdev)
>  {
>         struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
> @@ -1363,6 +1372,7 @@ static int btmtksdio_probe(struct sdio_func *func,
>         hdev->shutdown = btmtksdio_shutdown;
>         hdev->send     = btmtksdio_send_frame;
>         hdev->wakeup   = btmtksdio_sdio_wakeup;
> +       hdev->clear_wakeup = btmtksdio_disable_bt_wakeup;
>         /*
>          * If SDIO controller supports wake on Bluetooth, sending a wakeon
>          * command is not necessary.
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 0c1754f416bd..4bbd55335269 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -672,6 +672,7 @@ struct hci_dev {
>         int (*get_codec_config_data)(struct hci_dev *hdev, __u8 type,
>                                      struct bt_codec *codec, __u8 *vnd_len,
>                                      __u8 **vnd_data);
> +       void (*clear_wakeup)(struct hci_dev *hdev);

I wonder if it wouldn't be a better idea to add something like suspend
and resume callbacks to notify the about these hdev states, that way
we can synchronize the states better and avoid having to clear the
wakeup state when it shouldn't be active to begin with since the hdev
is not suspended.

>  };
>
>  #define HCI_PHY_HANDLE(handle) (handle & 0xff)
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 3563a90ed2ac..6c4d5ce40524 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5947,6 +5947,8 @@ int hci_resume_sync(struct hci_dev *hdev)
>                 return 0;
>
>         hdev->suspended = false;
> +       if (hdev->clear_wakeup)
> +               hdev->clear_wakeup(hdev);
>
>         /* Restore event mask */
>         hci_set_event_mask_sync(hdev);
> --
> 2.43.0.472.g3155946c3a-goog
>


-- 
Luiz Augusto von Dentz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-19 18:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-08 21:07 [kernel PATCH v1] Bluetooth: btmtksdio: clear BTMTKSDIO_BT_WAKE_ENABLED after resume Zhengping Jiang
2023-12-08 21:07 ` Zhengping Jiang
2023-12-08 21:58 ` [kernel,v1] " bluez.test.bot
2023-12-19 18:06 ` [kernel PATCH v1] " Luiz Augusto von Dentz
2023-12-19 18:06   ` Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.