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

From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: clancy_shang@163.com
Cc: marcel@holtmann.org, johan.hedberg@gmail.com,
	 linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
	 zhongjun.yu@quectel.com, Clancy Shang <clancy.shang@quectel.com>
Subject: Re: [PATCH] Bluetooth: hci_sync: fix hogp device suspend bug
Date: Tue, 26 Dec 2023 10:40:07 -0500	[thread overview]
Message-ID: <CABBYNZJQFD_=VcFZxK=AugixVMgc=Zw4WPgQn+ax28cNGWBGUQ@mail.gmail.com> (raw)
In-Reply-To: <20231226060818.2446327-1-clancy_shang@163.com>

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