Re: [PATCH] Bluetooth: Revert vendor-specific ISO classification for

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Luiz

>  are you not using btmon to capture these traces?

I used `btmon -r` - is that sufficient?

> maybe it is the wrong trace?

It's the right trace. Happy to run again if you still have doubts?

Thanks

On Mon, 7 Jul 2025 at 17:05, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Sean,
>
> On Mon, Jun 30, 2025 at 4:08 AM Sean Rhodes <sean@starlabs.systems> wrote:
> >
> > Hi
> >
> > Btmon outputs attached, one with 6.8 which works fine connecting to an LG monitor and Logitech Mouse. The other with 6.11, which fails to pair with the LG monitor, and can't see the Logtech Mouse.
>
> I don't see a single ISO packet in these traces, if the reclassify is
> being the problem we would see ACL packets as ISO instead, which is
> not the case, or are you not using btmon to capture these traces?
> Anyway, I don't see the pairing error, or anything really that would
> indicate there is any errors with C4:30:18:62:E2:01 which seems to be
> the LG monitor (A2DP/AVRCP/etc), or maybe it is the wrong trace?
>
> > Thanks
> >
> > Sean
> >
> > On Fri, 20 Jun 2025 at 17:00, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote:
> >>
> >> Hi Kiran,
> >>
> >> On Thu, Jun 19, 2025 at 4:33 PM Sean Rhodes <sean@starlabs.systems> wrote:
> >> >
> >> > Multiple users confirmed the revert fixed the issue (https://github.com/StarLabsLtd/firmware/issues/180#issuecomment-2784770614) and that 6.9 works as it doesn't have this patch and we've got a DKMS module with that patch reverted that makes things works.
> >> >
> >> > I'll grab a trace on Monday
> >> >
> >> > On Thu, 19 Jun 2025 at 21:13, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote:
> >> >>
> >> >> Hi Sean,
> >> >>
> >> >> On Thu, Jun 19, 2025 at 4:03 PM Sean Rhodes <sean@starlabs.systems> wrote:
> >> >> >
> >> >> > Hi Luiz
> >> >> >
> >> >> > It breaks pairing - some details can be found here - https://bugzilla.kernel.org/show_bug.cgi?id=219553
> >> >>
> >> >> Yeah, and Ive commented:
> >> >>
> >> >> https://bugzilla.kernel.org/show_bug.cgi?id=219553#c4
> >> >>
> >> >> There seems to be a mixup of issues, something with pairing, which has
> >> >> absolutely nothing to do with reclassifying packets, it is very likely
> >> >> the following bug:
> >> >>
> >> >> https://github.com/bluez/bluez/issues/1138
> >> >>
> >> >> If you have evidence that there is something that these changes
> >> >> actually cause a problem I'd like to see the HCI trace of that (use
> >> >> btmon to collect it).
> >> >>
> >> >> > Thanks
> >> >> >
> >> >> >
> >> >> > On Thu, 19 Jun 2025 at 21:00, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote:
> >> >> >>
> >> >> >> Hi Sean,
> >> >> >>
> >> >> >> On Thu, Jun 19, 2025 at 3:12 PM Sean Rhodes <sean@starlabs.systems> wrote:
> >> >> >> >
> >> >> >> > From 3b5497d0154a58d948ee95900e4c62704399de0a Mon Sep 17 00:00:00 2001
> >> >> >> > From: Sean Rhodes <sean@starlabs.systems>
> >> >> >> > Date: Wed, 2 Apr 2025 09:05:17 +0100
> >> >> >> > Subject: [PATCH] Bluetooth: Revert vendor-specific ISO classification for
> >> >> >> >  non-offload cards
> >> >> >> >
> >> >> >> > This reverts commit f25b7fd36cc3a850e006aed686f5bbecd200de1b.
> >> >> >> >
> >> >> >> > The commit introduces vendor-specific classification of ISO data,
> >> >> >> > but breaks Bluetooth functionality on certain Intel cards that do
> >> >> >> > not support audio offload, such as the 9462. Affected devices are
> >> >> >> > unable to discover new Bluetooth peripherals, and previously paired
> >> >> >> > devices fail to reconnect.
> >> >> >>
> >> >> >> How it breaks? It doesn't seem there is anything regarding the
> >> >> >> reclassification of the packets that could affect something that
> >> >> >> doesn't support ISO packets, well except if it happens that older
> >> >> >> controllers use the handle range of ISO but btintel_classify_pkt_type
> >> >> >> shouldn't be set for them.
> >> >> >>
> >> >> >> > This issue does not affect newer cards (e.g., AX201+) that support
> >> >> >> > audio offload. A conditional check using AOLD() could be used in
> >> >> >> > the future to reintroduce this behavior only on supported hardware.
> >> >> >> >
> >> >> >> > Cc: Marcel Holtmann <marcel@xxxxxxxxxxxx>
> >> >> >> > Cc: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> >> >> >> > Cc: Ying Hsu <yinghsu@xxxxxxxxxxxx>
> >> >> >> > Cc: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> >> >> >> > Cc: Marcel Holtmann <marcel@xxxxxxxxxxxx>
> >> >> >> > Cc: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> >> >> >> > Cc: Johan Hedberg <johan.hedberg@xxxxxxxxx>
> >> >> >> > Cc: linux-bluetooth@xxxxxxxxxxxxxxx
> >> >> >> > Signed-off-by: Sean Rhodes <sean@starlabs.systems>
> >> >> >> > ---
> >> >> >> >  drivers/bluetooth/btintel.c      |  7 ++-----
> >> >> >> >  include/net/bluetooth/hci_core.h |  1 -
> >> >> >> >  net/bluetooth/hci_core.c         | 16 ----------------
> >> >> >> >  3 files changed, 2 insertions(+), 22 deletions(-)
> >> >> >> >
> >> >> >> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> >> >> >> > index 55cc1652bfe4..1a5108cf6517 100644
> >> >> >> > --- a/drivers/bluetooth/btintel.c
> >> >> >> > +++ b/drivers/bluetooth/btintel.c
> >> >> >> > @@ -3582,15 +3582,12 @@ static int btintel_setup_combined(struct hci_dev *hdev)
> >> >> >> >                 err = btintel_bootloader_setup(hdev, &ver);
> >> >> >> >                 btintel_register_devcoredump_support(hdev);
> >> >> >> >                 break;
> >> >> >> > -       case 0x18: /* GfP2 */
> >> >> >> > -       case 0x1c: /* GaP */
> >> >> >> > -               /* Re-classify packet type for controllers with LE audio */
> >> >> >> > -               hdev->classify_pkt_type = btintel_classify_pkt_type;
> >> >> >> > -               fallthrough;
> >> >> >>
> >> >> >> 9462 seem to be JfP (0x11), so the above code shouldn't even be used for it.
> >>
> >> I'm starting to suspect that JfP may not be responding with the expect
> >> id here, which means we will need to use another field to confirm what
> >> is the real model.
> >>
> >> >> >>
> >> >> >> >         case 0x17:
> >> >> >> > +       case 0x18:
> >> >> >> >         case 0x19:
> >> >> >> >         case 0x1b:
> >> >> >> >         case 0x1d:
> >> >> >> > +       case 0x1c:
> >> >> >> >         case 0x1e:
> >> >> >> >         case 0x1f:
> >> >> >> >                 /* Display version information of TLV type */
> >> >> >> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> >> >> > index 2b261e74e2c4..648ee7e2403f 100644
> >> >> >> > --- a/include/net/bluetooth/hci_core.h
> >> >> >> > +++ b/include/net/bluetooth/hci_core.h
> >> >> >> > @@ -649,7 +649,6 @@ struct hci_dev {
> >> >> >> >         int (*get_codec_config_data)(struct hci_dev *hdev, __u8 type,
> >> >> >> >                                      struct bt_codec *codec, __u8 *vnd_len,
> >> >> >> >                                      __u8 **vnd_data);
> >> >> >> > -       u8 (*classify_pkt_type)(struct hci_dev *hdev, struct sk_buff *skb);
> >> >> >> >  };
> >> >> >> >
> >> >> >> >  #define HCI_PHY_HANDLE(handle) (handle & 0xff)
> >> >> >> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >> >> >> > index 3b49828160b7..64ab7702be81 100644
> >> >> >> > --- a/net/bluetooth/hci_core.c
> >> >> >> > +++ b/net/bluetooth/hci_core.c
> >> >> >> > @@ -2868,31 +2868,15 @@ int hci_reset_dev(struct hci_dev *hdev)
> >> >> >> >  }
> >> >> >> >  EXPORT_SYMBOL(hci_reset_dev);
> >> >> >> >
> >> >> >> > -static u8 hci_dev_classify_pkt_type(struct hci_dev *hdev, struct sk_buff *skb)
> >> >> >> > -{
> >> >> >> > -       if (hdev->classify_pkt_type)
> >> >> >> > -               return hdev->classify_pkt_type(hdev, skb);
> >> >> >> > -
> >> >> >> > -       return hci_skb_pkt_type(skb);
> >> >> >> > -}
> >> >> >> > -
> >> >> >> >  /* Receive frame from HCI drivers */
> >> >> >> >  int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
> >> >> >> >  {
> >> >> >> > -       u8 dev_pkt_type;
> >> >> >> > -
> >> >> >> >         if (!hdev || (!test_bit(HCI_UP, &hdev->flags)
> >> >> >> >                       && !test_bit(HCI_INIT, &hdev->flags))) {
> >> >> >> >                 kfree_skb(skb);
> >> >> >> >                 return -ENXIO;
> >> >> >> >         }
> >> >> >> >
> >> >> >> > -       /* Check if the driver agree with packet type classification */
> >> >> >> > -       dev_pkt_type = hci_dev_classify_pkt_type(hdev, skb);
> >> >> >> > -       if (hci_skb_pkt_type(skb) != dev_pkt_type) {
> >> >> >> > -               hci_skb_pkt_type(skb) = dev_pkt_type;
> >> >> >> > -       }
> >> >> >>
> >> >> >> This will affect all cards, not just the one you are claiming.
> >> >> >>
> >> >> >>
> >> >> >> >         switch (hci_skb_pkt_type(skb)) {
> >> >> >> >         case HCI_EVENT_PKT:
> >> >> >> >                 break;
> >> >> >> > --
> >> >> >> > 2.48.1
> >> >> >> >
> >> >> >>
> >> >> >>
> >> >> >> --
> >> >> >> Luiz Augusto von Dentz
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> Luiz Augusto von Dentz
> >>
> >>
> >>
> >> --
> >> Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux