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

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

[Heiner Kallweit]

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.

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

[Luiz Augusto von Dentz]

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.

-- 
Luiz Augusto von Dentz

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

[Heiner Kallweit]

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.

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

[Luiz Augusto von Dentz]

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

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

[Luiz Augusto von Dentz]

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 */


-- 
Luiz Augusto von Dentz

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

[Heiner Kallweit]

On 11.10.2024 21:05, Luiz Augusto von Dentz 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

Right, it's a M.2 adapter which utilizes PCIe and USB.

> 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'
> 

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

[Heiner Kallweit]

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.

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

[Luiz Augusto von Dentz]

Hi Heiner,

On Fri, Oct 11, 2024 at 3:44 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> 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.

Ok, I guess we can rule out system suspend then, does commenting out
the call to hci_suspend_dev makes it works again? If it does it means
the device is suspending and it appears the likes of
HCI_FLT_CONN_SETUP either doesn't work or is not being set to the
controller to allow incoming connections, since it is an Audio device
I think the problem is that we don't set HCI_CONN_FLAG_REMOTE_WAKEUP,
only input device do set it, so we may need to treat device suspend
and system suspend differently.

-- 
Luiz Augusto von Dentz

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

[Heiner Kallweit]

On 11.10.2024 22:06, Luiz Augusto von Dentz wrote:
> Hi Heiner,
> 
> On Fri, Oct 11, 2024 at 3:44 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> 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.
> 
> Ok, I guess we can rule out system suspend then, does commenting out
> the call to hci_suspend_dev makes it works again? If it does it means
> the device is suspending and it appears the likes of
> HCI_FLT_CONN_SETUP either doesn't work or is not being set to the
> controller to allow incoming connections, since it is an Audio device
> I think the problem is that we don't set HCI_CONN_FLAG_REMOTE_WAKEUP,
> only input device do set it, so we may need to treat device suspend
> and system suspend differently.
> 

The following makes it work again.

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7860750ec..33fd65d49 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2808,6 +2808,8 @@ int hci_suspend_dev(struct hci_dev *hdev)
 {
 	int ret;
 
+	return 0;
+
 	bt_dev_dbg(hdev, "");
 
 	/* Suspend should only act on when powered. */

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

[Luiz Augusto von Dentz]

Hi Heiner,

On Fri, Oct 11, 2024 at 4:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 11.10.2024 22:06, Luiz Augusto von Dentz wrote:
> > Hi Heiner,
> >
> > On Fri, Oct 11, 2024 at 3:44 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>
> >> 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.
> >
> > Ok, I guess we can rule out system suspend then, does commenting out
> > the call to hci_suspend_dev makes it works again? If it does it means
> > the device is suspending and it appears the likes of
> > HCI_FLT_CONN_SETUP either doesn't work or is not being set to the
> > controller to allow incoming connections, since it is an Audio device
> > I think the problem is that we don't set HCI_CONN_FLAG_REMOTE_WAKEUP,
> > only input device do set it, so we may need to treat device suspend
> > and system suspend differently.
> >
>
> The following makes it work again.
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 7860750ec..33fd65d49 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2808,6 +2808,8 @@ int hci_suspend_dev(struct hci_dev *hdev)
>  {
>         int ret;
>
> +       return 0;
> +
>         bt_dev_dbg(hdev, "");
>
>         /* Suspend should only act on when powered. */
>

Can you try with the following changes:

https://gist.github.com/Vudentz/52c0cf8b9472a03b8eb447951be13811

-- 
Luiz Augusto von Dentz

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

[Heiner Kallweit]

On 12.10.2024 00:19, Luiz Augusto von Dentz wrote:
> Hi Heiner,
> 
> On Fri, Oct 11, 2024 at 4:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> On 11.10.2024 22:06, Luiz Augusto von Dentz wrote:
>>> Hi Heiner,
>>>
>>> On Fri, Oct 11, 2024 at 3:44 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>
>>>> 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.
>>>
>>> Ok, I guess we can rule out system suspend then, does commenting out
>>> the call to hci_suspend_dev makes it works again? If it does it means
>>> the device is suspending and it appears the likes of
>>> HCI_FLT_CONN_SETUP either doesn't work or is not being set to the
>>> controller to allow incoming connections, since it is an Audio device
>>> I think the problem is that we don't set HCI_CONN_FLAG_REMOTE_WAKEUP,
>>> only input device do set it, so we may need to treat device suspend
>>> and system suspend differently.
>>>
>>
>> The following makes it work again.
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 7860750ec..33fd65d49 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -2808,6 +2808,8 @@ int hci_suspend_dev(struct hci_dev *hdev)
>>  {
>>         int ret;
>>
>> +       return 0;
>> +
>>         bt_dev_dbg(hdev, "");
>>
>>         /* Suspend should only act on when powered. */
>>
> 
> Can you try with the following changes:
> 
> https://gist.github.com/Vudentz/52c0cf8b9472a03b8eb447951be13811
> 
In this patch all tabs have been replaced with spaces, therefore it
didn't apply and I had to apply the changes manually.
Unfortunately the changes don't fix the issue for me.