Re: Connection issue since 81b3e33bb054 ("Bluetooth: btusb: Don't fail external suspend requests")

[Heiner Kallweit <hkallweit1@gmail.com>]

On 11.10.2024 21:26, Luiz Augusto von Dentz wrote:
> Hi Heiner,
> 
> On Fri, Oct 11, 2024 at 3:05 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>>
>> Hi Heiner,
>>
>> On Fri, Oct 11, 2024 at 2:52 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>
>>> On 11.10.2024 18:36, Luiz Augusto von Dentz wrote:
>>>> Hi Heiner,
>>>>
>>>> On Fri, Oct 11, 2024 at 6:49 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>>
>>>>> Since linux-next from Oct 4th my bt speaker fails to connect if I switch it on.
>>>>> It just hangs trying. Manually connecting it via bluetoothctl works though.
>>>>> With this patch reverted it auto-connects again.
>>>>> If you need additional details, please let me know.
>>>>
>>>> I suspect something is trying to suspend the controller then, it
>>>> shouldn't be USB auto-suspend since that should behave as it
>>>> previously but if there is something externally (aka. userspace)
>>>> trying to suspend then it will force it to suspend.
>>>>
>>> On the host side it's a combined WiFi/BT PCIe adapter (RTL8822CE).
>>> Runtime PM is enabled, so this may kick in. I'm not aware of any
>>> userspace tool which may try to suspend the WiFi/BT adapter.
>>> Disabling Runtime PM may be a workaround, but I don't think that's
>>> the actual solution.
>>>
>>
>> Well I assume it still using USB as transport, not PCIe, otherwise it
>> wouldn't be using btusb. Regarding runtime PM, I assume it still means
>> PMSG_IS_AUTO Documentation/driver-api/usb/power-management.rst:
>>
>>      'External suspend calls should never be allowed to fail in this way,
>>      only autosuspend calls.  The driver can tell them apart by applying
>>      the :c:func:`PMSG_IS_AUTO` macro to the message argument to the
>>      ``suspend`` method; it will return True for internal PM events'
>>
>> --
>> Luiz Augusto von Dentz
> 
> Perhaps there is a double call to the likes of hci_suspend_dev due to
> system suspend and device suspend acting together, so maybe we need
> something like the following:
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 779c4aeaef22..c257759ae2f4 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2812,7 +2812,7 @@ int hci_suspend_dev(struct hci_dev *hdev)
> 
>         /* Suspend should only act on when powered. */
>         if (!hdev_is_powered(hdev) ||
> -           hci_dev_test_flag(hdev, HCI_UNREGISTER))
> +           hci_dev_test_flag(hdev, HCI_UNREGISTER) || hdev->suspended)
>                 return 0;
> 
>         /* If powering down don't attempt to suspend */
> @@ -2843,7 +2843,7 @@ int hci_resume_dev(struct hci_dev *hdev)
> 
>         /* Resume should only act on when powered. */
>         if (!hdev_is_powered(hdev) ||
> -           hci_dev_test_flag(hdev, HCI_UNREGISTER))
> +           hci_dev_test_flag(hdev, HCI_UNREGISTER) || !hdev->suspended)
>                 return 0;
> 
>         /* If powering down don't attempt to resume */
> 
> 
No change in behavior with this change.