[PATCH] Bluetooth: hci_sync: fix hogp device suspend bug

[PATCH] Bluetooth: hci_sync: fix hogp device suspend bug

[15013537245]

From: clancy shang <clancy.shang@quectel.com>

when Bluetooth enter suspend, and disconnect everything with the diconnect
reason code of 0x15,the hogp device could not into sleep and continued
advertizing. when use the diconnect reason code of 0x13, the hogp device
into sleep succeeded.

Signed-off-by: clancy shang <clancy.shang@quectel.com>
---
 net/bluetooth/hci_sync.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index d85a7091a116..16b5420c32d0 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5927,7 +5927,7 @@ int hci_suspend_sync(struct hci_dev *hdev)
 
 	if (hci_conn_count(hdev)) {
 		/* Soft disconnect everything (power off) */
-		err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_POWER_OFF);
+		err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_USER_TERM);
 		if (err) {
 			/* Set state to BT_RUNNING so resume doesn't notify */
 			hdev->suspend_state = BT_RUNNING;
-- 
2.25.1

Re: [PATCH] Bluetooth: hci_sync: fix hogp device suspend bug

[Paul Menzel]

Dear Clancy,


Thank you for your patch.


Am 21.12.23 um 08:16 schrieb 15013537245@163.com:
> From: clancy shang <clancy.shang@quectel.com>

Not important, but you might want to start your name with a capital letter.

> when Bluetooth enter suspend, and disconnect everything with the diconnect

1.  enter*s*
2.  disconnect*s*
3.  … the di*s*connect …

> reason code of 0x15,the hogp device could not into sleep and continued

Please add a space after the comma.

> advertizing. when use the diconnect reason code of 0x13, the hogp device

1.  adverti*s*ing
2.  di*s*connect

> into sleep succeeded.

going into

What device is it exactly?

> Signed-off-by: clancy shang <clancy.shang@quectel.com>
> ---
>   net/bluetooth/hci_sync.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index d85a7091a116..16b5420c32d0 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5927,7 +5927,7 @@ int hci_suspend_sync(struct hci_dev *hdev)
>   
>   	if (hci_conn_count(hdev)) {
>   		/* Soft disconnect everything (power off) */
> -		err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_POWER_OFF);
> +		err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_USER_TERM);
>   		if (err) {
>   			/* Set state to BT_RUNNING so resume doesn't notify */
>   			hdev->suspend_state = BT_RUNNING;


Kind regards,

Paul

Re: [PATCH] Bluetooth: hci_sync: fix hogp device suspend bug

[Luiz Augusto von Dentz]

Hi Clancy,

On Wed, Jan 3, 2024 at 5:12 AM <clancy_shang@163.com> wrote:
>
> From: Clancy Shang <clancy.shang@quectel.com>
>
> when Bluetooth enters suspend, and disconnects everything with the
> disconnect reason code of 0x15, the hogp device could not into sleep
> and continued advertising. when use the disconnect reason code of 0x13,
> the hogp device going into sleep succeeded.
>
> Signed-off-by: Clancy Shang <clancy.shang@quectel.com>
> ---
>  net/bluetooth/hci_sync.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index d85a7091a116..16b5420c32d0 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5927,7 +5927,7 @@ int hci_suspend_sync(struct hci_dev *hdev)
>
>         if (hci_conn_count(hdev)) {
>                 /* Soft disconnect everything (power off) */
> -               err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_POWER_OFF);
> +               err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_USER_TERM);

Are you actually paying attention to my responses? I did nak this
change before, so we need to find some way to detect the device is
handling HCI_ERROR_REMOTE_POWER_OFF as link loss, which is wrong, and
only then use HCI_ERROR_REMOTE_USER_TERM, so please don't keep sending
the same patch over and over until you make these changes.

>                 if (err) {
>                         /* Set state to BT_RUNNING so resume doesn't notify */
>                         hdev->suspend_state = BT_RUNNING;
> --
> 2.25.1
>


-- 
Luiz Augusto von Dentz

[PATCH] Bluetooth: hci_sync: fix hogp device suspend bug

[clancy_shang]

From: Clancy Shang <clancy.shang@quectel.com>

when Bluetooth enters suspend, and disconnects everything with the
disconnect reason code of 0x15, the hogp device could not into sleep
and continued advertising. when use the disconnect reason code of 0x13,
the hogp device going into sleep succeeded.

Signed-off-by: Clancy Shang <clancy.shang@quectel.com>
---
 net/bluetooth/hci_sync.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index d85a7091a116..16b5420c32d0 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5927,7 +5927,7 @@ int hci_suspend_sync(struct hci_dev *hdev)
 
 	if (hci_conn_count(hdev)) {
 		/* Soft disconnect everything (power off) */
-		err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_POWER_OFF);
+		err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_USER_TERM);
 		if (err) {
 			/* Set state to BT_RUNNING so resume doesn't notify */
 			hdev->suspend_state = BT_RUNNING;
-- 
2.25.1

Re: [PATCH] Bluetooth: hci_sync: fix hogp device suspend bug

[Luiz Augusto von Dentz]

Hi Clancy,

On Tue, Dec 26, 2023 at 1:09 AM <clancy_shang@163.com> wrote:
>
> From: Clancy Shang <clancy.shang@quectel.com>
>
> when Bluetooth enters suspend, and disconnects everything with the
> disconnect reason code of 0x15, the hogp device could not into sleep
> and continued advertising. when use the disconnect reason code of 0x13,
> the hogp device going into sleep succeeded.
>
> Signed-off-by: Clancy Shang <clancy.shang@quectel.com>
> ---
>  net/bluetooth/hci_sync.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index d85a7091a116..16b5420c32d0 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5927,7 +5927,7 @@ int hci_suspend_sync(struct hci_dev *hdev)
>
>         if (hci_conn_count(hdev)) {
>                 /* Soft disconnect everything (power off) */
> -               err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_POWER_OFF);
> +               err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_USER_TERM);

So the device interpretes that it can reconnect on power off but not
on user termination error? And you think changing the error would make
this interoperate any better? Afaik user termination can be generated
by various reasons, including the non-clean termination ones, while
power off error is quite clear on the reason, so instead of using a
more generic error you probably should contact the device manufacturer
and have it fix this problem and until then Id probably disable wakeup
support since it doesn't behave properly when receiving
HCI_ERROR_REMOTE_POWER_OFF.

>                 if (err) {
>                         /* Set state to BT_RUNNING so resume doesn't notify */
>                         hdev->suspend_state = BT_RUNNING;
> --
> 2.25.1
>
>


-- 
Luiz Augusto von Dentz

[PATCH] Bluetooth: hci_sync: fix hogp device suspend bug

[clancy_shang]

From: Clancy Shang <clancy.shang@quectel.com>

when Bluetooth enters suspend, and disconnects everything with the
disconnect reason code of 0x15, the hogp device could not into sleep
and continued advertising. when use the disconnect reason code of 0x13,
the hogp device going into sleep succeeded.

Signed-off-by: Clancy Shang <clancy.shang@quectel.com>
---
 net/bluetooth/hci_sync.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index d85a7091a116..16b5420c32d0 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5927,7 +5927,7 @@ int hci_suspend_sync(struct hci_dev *hdev)
 
 	if (hci_conn_count(hdev)) {
 		/* Soft disconnect everything (power off) */
-		err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_POWER_OFF);
+		err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_USER_TERM);
 		if (err) {
 			/* Set state to BT_RUNNING so resume doesn't notify */
 			hdev->suspend_state = BT_RUNNING;
-- 
2.25.1