2022-03-25 21:41:09

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH 2/2] Bluetooth: Print broken quirks

From: Luiz Augusto von Dentz <[email protected]>

This prints warnings for controllers setting broken quirks to increase
their visibility and warn about broken controllers firmware that
probably needs updates to behave properly.

Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
net/bluetooth/hci_sync.c | 44 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 8f4c5698913d..089500136096 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -3825,6 +3825,48 @@ static int hci_init_sync(struct hci_dev *hdev)
return 0;
}

+#define HCI_BROKEN(_quirk, _desc) \
+{ \
+ .quirk = _quirk, \
+ .desc = _desc, \
+}
+
+struct hci_broken {
+ unsigned long quirk;
+ const char *desc;
+} hci_broken_table[] = {
+ HCI_BROKEN(HCI_QUIRK_BROKEN_LOCAL_COMMANDS,
+ "HCI Read Local Supported Commands not supported"),
+ HCI_BROKEN(HCI_QUIRK_BROKEN_STORED_LINK_KEY,
+ "HCI Delete Stored Link Key command is advertised, "
+ "but not supported."),
+ HCI_BROKEN(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING,
+ "HCI Read Default Erroneous Data Reporting command is "
+ "advertised, but not supported."),
+ HCI_BROKEN(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
+ "HCI Read Transmit Power Level command is advertised, "
+ "but not supported."),
+ HCI_BROKEN(HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL,
+ "HCI Set Event Filter command not supported."),
+ HCI_BROKEN(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN,
+ "HCI Enhanced Setup Synchronous Connection command is "
+ "advertised, but not supported.")
+};
+
+static void hci_dev_print_broken(struct hci_dev *hdev)
+{
+ int i;
+
+ bt_dev_dbg(hdev, "");
+
+ for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) {
+ struct hci_broken *broken = &hci_broken_table[i];
+
+ if (test_bit(broken->quirk, &hdev->quirks))
+ bt_dev_warn(hdev, "%s", broken->desc);
+ }
+}
+
int hci_dev_open_sync(struct hci_dev *hdev)
{
int ret = 0;
@@ -4031,6 +4073,8 @@ int hci_dev_open_sync(struct hci_dev *hdev)
}

done:
+ hci_dev_print_broken(hdev);
+
return ret;
}

--
2.35.1


2022-03-26 13:59:39

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] Bluetooth: Print broken quirks

Hi Luiz,

I love your patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on next-20220325]
[cannot apply to bluetooth/master v5.17]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-HCI-Add-HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN-quirk/20220326-051551
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: x86_64-randconfig-s021 (https://download.01.org/0day-ci/archive/20220326/[email protected]/config)
compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/5ca7f4a3fcd3584b663e15b4c3d8230918b8971d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Augusto-von-Dentz/Bluetooth-HCI-Add-HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN-quirk/20220326-051551
git checkout 5ca7f4a3fcd3584b663e15b4c3d8230918b8971d
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>


sparse warnings: (new ones prefixed by >>)
>> net/bluetooth/hci_sync.c:3837:3: sparse: sparse: symbol 'hci_broken_table' was not declared. Should it be static?

vim +/hci_broken_table +3837 net/bluetooth/hci_sync.c

3833
3834 struct hci_broken {
3835 unsigned long quirk;
3836 const char *desc;
> 3837 } hci_broken_table[] = {
3838 HCI_BROKEN(HCI_QUIRK_BROKEN_LOCAL_COMMANDS,
3839 "HCI Read Local Supported Commands not supported"),
3840 HCI_BROKEN(HCI_QUIRK_BROKEN_STORED_LINK_KEY,
3841 "HCI Delete Stored Link Key command is advertised, "
3842 "but not supported."),
3843 HCI_BROKEN(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING,
3844 "HCI Read Default Erroneous Data Reporting command is "
3845 "advertised, but not supported."),
3846 HCI_BROKEN(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
3847 "HCI Read Transmit Power Level command is advertised, "
3848 "but not supported."),
3849 HCI_BROKEN(HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL,
3850 "HCI Set Event Filter command not supported."),
3851 HCI_BROKEN(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN,
3852 "HCI Enhanced Setup Synchronous Connection command is "
3853 "advertised, but not supported.")
3854 };
3855

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-03-29 16:48:49

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] Bluetooth: Print broken quirks

Hi Luiz,

> This prints warnings for controllers setting broken quirks to increase
> their visibility and warn about broken controllers firmware that
> probably needs updates to behave properly.
>
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> ---
> net/bluetooth/hci_sync.c | 44 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 8f4c5698913d..089500136096 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -3825,6 +3825,48 @@ static int hci_init_sync(struct hci_dev *hdev)
> return 0;
> }
>
> +#define HCI_BROKEN(_quirk, _desc) \
> +{ \
> + .quirk = _quirk, \
> + .desc = _desc, \
> +}
> +
> +struct hci_broken {
> + unsigned long quirk;
> + const char *desc;
> +} hci_broken_table[] = {
> + HCI_BROKEN(HCI_QUIRK_BROKEN_LOCAL_COMMANDS,
> + "HCI Read Local Supported Commands not supported"),
> + HCI_BROKEN(HCI_QUIRK_BROKEN_STORED_LINK_KEY,
> + "HCI Delete Stored Link Key command is advertised, "
> + "but not supported."),
> + HCI_BROKEN(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING,
> + "HCI Read Default Erroneous Data Reporting command is "
> + "advertised, but not supported."),
> + HCI_BROKEN(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> + "HCI Read Transmit Power Level command is advertised, "
> + "but not supported."),
> + HCI_BROKEN(HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL,
> + "HCI Set Event Filter command not supported."),
> + HCI_BROKEN(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN,
> + "HCI Enhanced Setup Synchronous Connection command is "
> + "advertised, but not supported.")
> +};

static const hci_quirk_broken

And then HCI_QUIRK_BROKEN(LOCAL_COMMANDS, “text”),

> +
> +static void hci_dev_print_broken(struct hci_dev *hdev)
> +{
> + int i;
> +
> + bt_dev_dbg(hdev, "");
> +
> + for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) {
> + struct hci_broken *broken = &hci_broken_table[i];
> +
> + if (test_bit(broken->quirk, &hdev->quirks))
> + bt_dev_warn(hdev, "%s", broken->desc);
> + }
> +}
> +
> int hci_dev_open_sync(struct hci_dev *hdev)
> {
> int ret = 0;
> @@ -4031,6 +4073,8 @@ int hci_dev_open_sync(struct hci_dev *hdev)
> }
>
> done:
> + hci_dev_print_broken(hdev);
> +

This is wrong. It gets printed all the time and no matter if success or failure. This needs to go after hdev->setup.

Regards

Marcel