| Message ID | 20220524212155.16944-2-bage@debian.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: allwinner: a64: add bluetooth support for Pinebook | expand |
This is an automated email and please do not reply to this email. Dear Submitter, Thank you for submitting the patches to the linux bluetooth mailing list. While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository. ----- Output ----- error: patch failed: include/net/bluetooth/hci.h:265 error: include/net/bluetooth/hci.h: patch does not apply hint: Use 'git am --show-current-patch' to see the failed patch Please resolve the issue and submit the patches again. --- Regards, Linux Bluetooth
Hi Bastian, > Some adapters (e.g. RTL8723CS) advertise that they have more than > 2 pages for local ext features, but they don't support any features > declared in these pages. RTL8723CS reports max_page = 2 and declares > support for sync train and secure connection, but it responds with > either garbage or with error in status on corresponding commands. please include btmon output for the garbage and/or error. > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > [rebase on current tree] > Signed-off-by: Bastian Germann <bage@debian.org> > --- > include/net/bluetooth/hci.h | 7 +++++++ > net/bluetooth/hci_event.c | 4 +++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index 69ef31cea582..af26e8051905 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -265,6 +265,13 @@ enum { > * runtime suspend, because event filtering takes place there. > */ > HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL, > + > + /* When this quirk is set, max_page for local extended features > + * is set to 1, even if controller reports higher number. Some > + * controllers (e.g. RTL8723CS) report more pages, but they > + * don't actually support features declared there. > + */ > + HCI_QUIRK_BROKEN_LOCAL_EXT_FTR_MAX_PAGE, > }; Can we just call it _BROKEN_LOCAL_EXT_FEATURES_PAGE_2. Now with that said, is Secure Connections really broken? We need that bit to indicate support for this. Regards Marcel
On Thu, Jun 2, 2022 at 9:10 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Bastian, Hi Marcel, > > Some adapters (e.g. RTL8723CS) advertise that they have more than > > 2 pages for local ext features, but they don't support any features > > declared in these pages. RTL8723CS reports max_page = 2 and declares > > support for sync train and secure connection, but it responds with > > either garbage or with error in status on corresponding commands. > > > please include btmon output for the garbage and/or error. We had it in v1 thread, here is relevant part: < HCI Command: Read Local Extend.. (0x04|0x0004) plen 1 #228 [hci0] 6.889869 Page: 2 > HCI Event: Command Complete (0x0e) plen 14 #229 [hci0] 6.890487 Read Local Extended Features (0x04|0x0004) ncmd 2 Status: Success (0x00) Page: 2/2 Features: 0x5f 0x03 0x00 0x00 0x00 0x00 0x00 0x00 Connectionless Slave Broadcast - Master Connectionless Slave Broadcast - Slave Synchronization Train Synchronization Scan Inquiry Response Notification Event Coarse Clock Adjustment Secure Connections (Controller Support) Ping < HCI Command: Delete Stored Lin.. (0x03|0x0012) plen 7 #230 [hci0] 6.890559 Address: 00:00:00:00:00:00 (OUI 00-00-00) Delete all: 0x01 > HCI Event: Command Complete (0x0e) plen 6 #231 [hci0] 6.891170 Delete Stored Link Key (0x03|0x0012) ncmd 2 Status: Success (0x00) Num keys: 0 < HCI Command: Read Synchronizat.. (0x03|0x0077) plen 0 #232 [hci0] 6.891199 > HCI Event: Command Complete (0x0e) plen 9 #233 [hci0] 6.891788 Read Synchronization Train Parameters (0x03|0x0077) ncmd 2 invalid packet size 01 ac bd 11 80 80 ...... = Close Index: 00:E0:4C:23:99:87 [hci0] 6.891832 > > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > [rebase on current tree] > > Signed-off-by: Bastian Germann <bage@debian.org> > > --- > > include/net/bluetooth/hci.h | 7 +++++++ > > net/bluetooth/hci_event.c | 4 +++- > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > > index 69ef31cea582..af26e8051905 100644 > > --- a/include/net/bluetooth/hci.h > > +++ b/include/net/bluetooth/hci.h > > @@ -265,6 +265,13 @@ enum { > > * runtime suspend, because event filtering takes place there. > > */ > > HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL, > > + > > + /* When this quirk is set, max_page for local extended features > > + * is set to 1, even if controller reports higher number. Some > > + * controllers (e.g. RTL8723CS) report more pages, but they > > + * don't actually support features declared there. > > + */ > > + HCI_QUIRK_BROKEN_LOCAL_EXT_FTR_MAX_PAGE, > > }; > > Can we just call it _BROKEN_LOCAL_EXT_FEATURES_PAGE_2. > > Now with that said, is Secure Connections really broken? We need that bit to indicate support for this. I don't really see the point in testing any 4.1 features if the chip vendor claims that they are broken. I understand your intention to get the max out of the hardware, but it doesn't look like a good idea to me to use something that the vendor claims to be broken. Regards, Vasily > Regards > > Marcel >
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 69ef31cea582..af26e8051905 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -265,6 +265,13 @@ enum { * runtime suspend, because event filtering takes place there. */ HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL, + + /* When this quirk is set, max_page for local extended features + * is set to 1, even if controller reports higher number. Some + * controllers (e.g. RTL8723CS) report more pages, but they + * don't actually support features declared there. + */ + HCI_QUIRK_BROKEN_LOCAL_EXT_FTR_MAX_PAGE, }; /* HCI device flags */ diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 66451661283c..52b358c33344 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -837,7 +837,9 @@ static u8 hci_cc_read_local_ext_features(struct hci_dev *hdev, void *data, if (rp->status) return rp->status; - if (hdev->max_page < rp->max_page) + if (!test_bit(HCI_QUIRK_BROKEN_LOCAL_EXT_FTR_MAX_PAGE, + &hdev->quirks) && + hdev->max_page < rp->max_page) hdev->max_page = rp->max_page; if (rp->page < HCI_MAX_PAGES)