Hi Heiner,
On Fri, Oct 11, 2024 at 4:56 PM Heiner Kallweit <hkallweit1@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> wrote:
> >>>>
> >>>> Hi Heiner,
> >>>>
> >>>> On Fri, Oct 11, 2024 at 2:52 PM Heiner Kallweit <hkallweit1@xxxxxxxxx> 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@xxxxxxxxx> 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
