diff mbox series

[2/2] Bluetooth: Print broken quirks

Message ID 20220325211314.425364-2-luiz.dentz@gmail.com (mailing list archive)
State Superseded
Headers show
Series [1/2] Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN quirk | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/checkpatch fail [2/2] Bluetooth: Print broken quirks\WARNING:SPLIT_STRING: quoted string split across lines #113: FILE: net/bluetooth/hci_sync.c:3842: + "HCI Delete Stored Link Key command is advertised, " + "but not supported."), WARNING:SPLIT_STRING: quoted string split across lines #116: FILE: net/bluetooth/hci_sync.c:3845: + "HCI Read Default Erroneous Data Reporting command is " + "advertised, but not supported."), WARNING:SPLIT_STRING: quoted string split across lines #119: FILE: net/bluetooth/hci_sync.c:3848: + "HCI Read Transmit Power Level command is advertised, " + "but not supported."), WARNING:SPLIT_STRING: quoted string split across lines #124: FILE: net/bluetooth/hci_sync.c:3853: + "HCI Enhanced Setup Synchronous Connection command is " + "advertised, but not supported.") total: 0 errors, 4 warnings, 0 checks, 56 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/12792078.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
tedd_an/gitlint success Gitlint PASS
tedd_an/subjectprefix success PASS

Commit Message

Luiz Augusto von Dentz March 25, 2022, 9:13 p.m. UTC 
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

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 <luiz.von.dentz@intel.com>
---
 net/bluetooth/hci_sync.c | 44 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

Comments

kernel test robot March 26, 2022, 7:30 a.m. UTC | #1 
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/202203261536.VslNAaC7-lkp@intel.com/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 <lkp@intel.com>


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
Marcel Holtmann March 29, 2022, 11:33 a.m. UTC | #2 
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 <luiz.von.dentz@intel.com>
> ---
> 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
diff mbox series

Patch

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;
 }