2023-02-06 23:39:40

by Bastian Germann

[permalink] [raw]
Subject: [PATCH v3 1/2] Bluetooth: Add new quirk for broken local ext features page 2

From: Vasily Khoruzhick <[email protected]>

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.

Signed-off-by: Vasily Khoruzhick <[email protected]>
Signed-off-by: Bastian Germann <[email protected]>
---
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 8d773b042c85..7127313140cf 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -294,6 +294,13 @@ enum {
* during the hdev->setup vendor callback.
*/
HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG,
+
+ /* 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_FEATURES_PAGE_2,
};

/* HCI device flags */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ad92a4be5851..83ebc8e65b42 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -886,7 +886,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_FEATURES_PAGE_2,
+ &hdev->quirks) &&
+ hdev->max_page < rp->max_page)
hdev->max_page = rp->max_page;

if (rp->page < HCI_MAX_PAGES)
--
2.39.1



2023-02-07 01:15:57

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: btrtl: add support for the RTL8723CS

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=719291

---Test result---

Test Summary:
CheckPatch PASS 2.18 seconds
GitLint PASS 0.73 seconds
SubjectPrefix PASS 0.25 seconds
BuildKernel PASS 31.21 seconds
CheckAllWarning PASS 34.24 seconds
CheckSparse WARNING 38.42 seconds
CheckSmatch WARNING 107.48 seconds
BuildKernel32 PASS 29.86 seconds
TestRunnerSetup PASS 430.17 seconds
TestRunner_l2cap-tester PASS 16.19 seconds
TestRunner_iso-tester PASS 16.39 seconds
TestRunner_bnep-tester PASS 5.42 seconds
TestRunner_mgmt-tester PASS 108.87 seconds
TestRunner_rfcomm-tester PASS 8.57 seconds
TestRunner_sco-tester PASS 7.96 seconds
TestRunner_ioctl-tester PASS 9.31 seconds
TestRunner_mesh-tester PASS 6.93 seconds
TestRunner_smp-tester PASS 7.82 seconds
TestRunner_userchan-tester PASS 5.75 seconds
IncrementalBuild PASS 33.40 seconds

Details
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):drivers/bluetooth/btrtl.c: note: in included file:drivers/bluetooth/btrtl.h:52:45: warning: array of flexible structures
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):drivers/bluetooth/btrtl.c: note: in included file:drivers/bluetooth/btrtl.h:52:45: warning: array of flexible structures


---
Regards,
Linux Bluetooth

2023-03-04 00:21:27

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Bluetooth: Add new quirk for broken local ext features page 2

Hi Bastien,

On Mon, Feb 6, 2023 at 3:39 PM Bastian Germann <[email protected]> wrote:
>
> From: Vasily Khoruzhick <[email protected]>
>
> 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.
>
> Signed-off-by: Vasily Khoruzhick <[email protected]>
> Signed-off-by: Bastian Germann <[email protected]>
> ---
> 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 8d773b042c85..7127313140cf 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -294,6 +294,13 @@ enum {
> * during the hdev->setup vendor callback.
> */
> HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG,
> +
> + /* 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_FEATURES_PAGE_2,
> };
>
> /* HCI device flags */
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index ad92a4be5851..83ebc8e65b42 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -886,7 +886,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_FEATURES_PAGE_2,
> + &hdev->quirks) &&
> + hdev->max_page < rp->max_page)
> hdev->max_page = rp->max_page;
>
> if (rp->page < HCI_MAX_PAGES)
> --
> 2.39.1

Looks like I never replied to this one, we might want to add a warning
when the controller requires such quirks since the manufacturer is
supposed to fix this in their firmware.



--
Luiz Augusto von Dentz