[PATCH v2 0/3] arm64: allwinner: a64: add bluetooth support for Pinebook

[PATCH v2 0/3] arm64: allwinner: a64: add bluetooth support for Pinebook

[Bastian Germann]

Pinebook uses RTL8723CS for WiFi and bluetooth. Unfortunately RTL8723CS
has broken BT-4.1 support, so it requires a quirk.

Add a quirk, wire up 8723CS support in btrtl and enable bluetooth
in Pinebook dts.

This series was sent in July 2020 by Vasily Khoruzhick.
This is a rebase on the current tree.
I have tested it to work on the Pinebook.

Changelog:
v2:
   * Rebase
   * Add uart-has-rtscts to device tree as requested by reviewer

Vasily Khoruzhick (3):
  Bluetooth: Add new quirk for broken local ext features max_page
  Bluetooth: btrtl: add support for the RTL8723CS
  arm64: allwinner: a64: enable Bluetooth On Pinebook

 .../dts/allwinner/sun50i-a64-pinebook.dts     |  13 ++
 drivers/bluetooth/btrtl.c                     | 120 +++++++++++++++++-
 drivers/bluetooth/btrtl.h                     |   5 +
 drivers/bluetooth/hci_h5.c                    |   4 +
 include/net/bluetooth/hci.h                   |   7 +
 net/bluetooth/hci_event.c                     |   4 +-
 6 files changed, 148 insertions(+), 5 deletions(-)

-- 
2.36.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 0/3] arm64: allwinner: a64: add bluetooth support for Pinebook

[Bastian Germann]

Pinebook uses RTL8723CS for WiFi and bluetooth. Unfortunately RTL8723CS
has broken BT-4.1 support, so it requires a quirk.

Add a quirk, wire up 8723CS support in btrtl and enable bluetooth
in Pinebook dts.

This series was sent in July 2020 by Vasily Khoruzhick.
This is a rebase on the current tree.
I have tested it to work on the Pinebook.

Changelog:
v2:
   * Rebase
   * Add uart-has-rtscts to device tree as requested by reviewer

Vasily Khoruzhick (3):
  Bluetooth: Add new quirk for broken local ext features max_page
  Bluetooth: btrtl: add support for the RTL8723CS
  arm64: allwinner: a64: enable Bluetooth On Pinebook

 .../dts/allwinner/sun50i-a64-pinebook.dts     |  13 ++
 drivers/bluetooth/btrtl.c                     | 120 +++++++++++++++++-
 drivers/bluetooth/btrtl.h                     |   5 +
 drivers/bluetooth/hci_h5.c                    |   4 +
 include/net/bluetooth/hci.h                   |   7 +
 net/bluetooth/hci_event.c                     |   4 +-
 6 files changed, 148 insertions(+), 5 deletions(-)

-- 
2.36.1

[PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page

[Bastian Germann]

From: Vasily Khoruzhick <anarsoul@gmail.com>

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 <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,
 };
 
 /* 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)
-- 
2.36.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page

[Bastian Germann]

From: Vasily Khoruzhick <anarsoul@gmail.com>

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 <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,
 };
 
 /* 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)
-- 
2.36.1

[PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS

[Bastian Germann]

From: Vasily Khoruzhick <anarsoul@gmail.com>

The Realtek RTL8723CS is SDIO WiFi chip. It also contains a Bluetooth
module which is connected via UART to the host.

It shares lmp subversion with 8703B, so Realtek's userspace
initialization tool (rtk_hciattach) differentiates varieties of RTL8723CS
(CG, VF, XX) with RTL8703B using vendor's command to read chip type.

Also this chip declares support for some features it doesn't support
so add a quirk to indicate that these features are broken.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
[move former btrtl_apply_quirks to btrtl_set_quirks]
[rebase on current tree]
Signed-off-by: Bastian Germann <bage@debian.org>
---
 drivers/bluetooth/btrtl.c  | 120 +++++++++++++++++++++++++++++++++++--
 drivers/bluetooth/btrtl.h  |   5 ++
 drivers/bluetooth/hci_h5.c |   4 ++
 3 files changed, 125 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 481d488bca0f..4b7378a65895 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -17,7 +17,11 @@
 
 #define VERSION "0.1"
 
+#define RTL_CHIP_8723CS_CG	3
+#define RTL_CHIP_8723CS_VF	4
+#define RTL_CHIP_8723CS_XX	5
 #define RTL_EPATCH_SIGNATURE	"Realtech"
+#define RTL_ROM_LMP_8703B	0x8703
 #define RTL_ROM_LMP_8723A	0x1200
 #define RTL_ROM_LMP_8723B	0x8723
 #define RTL_ROM_LMP_8821A	0x8821
@@ -30,6 +34,7 @@
 #define IC_MATCH_FL_HCIREV	(1 << 1)
 #define IC_MATCH_FL_HCIVER	(1 << 2)
 #define IC_MATCH_FL_HCIBUS	(1 << 3)
+#define IC_MATCH_FL_CHIP_TYPE	(1 << 4)
 #define IC_INFO(lmps, hcir, hciv, bus) \
 	.match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_HCIREV | \
 		       IC_MATCH_FL_HCIVER | IC_MATCH_FL_HCIBUS, \
@@ -58,6 +63,7 @@ struct id_table {
 	__u16 hci_rev;
 	__u8 hci_ver;
 	__u8 hci_bus;
+	__u8 chip_type;
 	bool config_needed;
 	bool has_rom_version;
 	bool has_msft_ext;
@@ -98,6 +104,39 @@ static const struct id_table ic_id_table[] = {
 	  .fw_name  = "rtl_bt/rtl8723b_fw.bin",
 	  .cfg_name = "rtl_bt/rtl8723b_config" },
 
+	/* 8723CS-CG */
+	{ .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_CHIP_TYPE |
+			 IC_MATCH_FL_HCIBUS,
+	  .lmp_subver = RTL_ROM_LMP_8703B,
+	  .chip_type = RTL_CHIP_8723CS_CG,
+	  .hci_bus = HCI_UART,
+	  .config_needed = true,
+	  .has_rom_version = true,
+	  .fw_name  = "rtl_bt/rtl8723cs_cg_fw.bin",
+	  .cfg_name = "rtl_bt/rtl8723cs_cg_config" },
+
+	/* 8723CS-VF */
+	{ .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_CHIP_TYPE |
+			 IC_MATCH_FL_HCIBUS,
+	  .lmp_subver = RTL_ROM_LMP_8703B,
+	  .chip_type = RTL_CHIP_8723CS_VF,
+	  .hci_bus = HCI_UART,
+	  .config_needed = true,
+	  .has_rom_version = true,
+	  .fw_name  = "rtl_bt/rtl8723cs_vf_fw.bin",
+	  .cfg_name = "rtl_bt/rtl8723cs_vf_config" },
+
+	/* 8723CS-XX */
+	{ .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_CHIP_TYPE |
+			 IC_MATCH_FL_HCIBUS,
+	  .lmp_subver = RTL_ROM_LMP_8703B,
+	  .chip_type = RTL_CHIP_8723CS_XX,
+	  .hci_bus = HCI_UART,
+	  .config_needed = true,
+	  .has_rom_version = true,
+	  .fw_name  = "rtl_bt/rtl8723cs_xx_fw.bin",
+	  .cfg_name = "rtl_bt/rtl8723cs_xx_config" },
+
 	/* 8723D */
 	{ IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_USB),
 	  .config_needed = true,
@@ -199,7 +238,8 @@ static const struct id_table ic_id_table[] = {
 	};
 
 static const struct id_table *btrtl_match_ic(u16 lmp_subver, u16 hci_rev,
-					     u8 hci_ver, u8 hci_bus)
+					     u8 hci_ver, u8 hci_bus,
+					     u8 chip_type)
 {
 	int i;
 
@@ -216,6 +256,9 @@ static const struct id_table *btrtl_match_ic(u16 lmp_subver, u16 hci_rev,
 		if ((ic_id_table[i].match_flags & IC_MATCH_FL_HCIBUS) &&
 		    (ic_id_table[i].hci_bus != hci_bus))
 			continue;
+		if ((ic_id_table[i].match_flags & IC_MATCH_FL_CHIP_TYPE) &&
+		    (ic_id_table[i].chip_type != chip_type))
+			continue;
 
 		break;
 	}
@@ -298,6 +341,7 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
 		{ RTL_ROM_LMP_8723B, 1 },
 		{ RTL_ROM_LMP_8821A, 2 },
 		{ RTL_ROM_LMP_8761A, 3 },
+		{ RTL_ROM_LMP_8703B, 7 },
 		{ RTL_ROM_LMP_8822B, 8 },
 		{ RTL_ROM_LMP_8723B, 9 },	/* 8723D */
 		{ RTL_ROM_LMP_8821A, 10 },	/* 8821C */
@@ -577,6 +621,48 @@ static int btrtl_setup_rtl8723b(struct hci_dev *hdev,
 	return ret;
 }
 
+static bool rtl_has_chip_type(u16 lmp_subver)
+{
+	switch (lmp_subver) {
+	case RTL_ROM_LMP_8703B:
+		return true;
+	default:
+		break;
+	}
+
+	return  false;
+}
+
+static int rtl_read_chip_type(struct hci_dev *hdev, u8 *type)
+{
+	struct rtl_chip_type_evt *chip_type;
+	struct sk_buff *skb;
+	const unsigned char cmd_buf[] = {0x00, 0x94, 0xa0, 0x00, 0xb0};
+
+	/* Read RTL chip type command */
+	skb = __hci_cmd_sync(hdev, 0xfc61, 5, cmd_buf, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		rtl_dev_err(hdev, "Read chip type failed (%ld)",
+			    PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+
+	if (skb->len != sizeof(*chip_type)) {
+		rtl_dev_err(hdev, "RTL chip type event length mismatch");
+		kfree_skb(skb);
+		return -EIO;
+	}
+
+	chip_type = (struct rtl_chip_type_evt *)skb->data;
+	rtl_dev_info(hdev, "chip_type status=%x type=%x",
+		     chip_type->status, chip_type->type);
+
+	*type = chip_type->type & 0x0f;
+
+	kfree_skb(skb);
+	return 0;
+}
+
 void btrtl_free(struct btrtl_device_info *btrtl_dev)
 {
 	kvfree(btrtl_dev->fw_data);
@@ -593,7 +679,7 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 	struct hci_rp_read_local_version *resp;
 	char cfg_name[40];
 	u16 hci_rev, lmp_subver;
-	u8 hci_ver;
+	u8 hci_ver, chip_type = 0;
 	int ret;
 	u16 opcode;
 	u8 cmd[2];
@@ -619,8 +705,14 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 	hci_rev = le16_to_cpu(resp->hci_rev);
 	lmp_subver = le16_to_cpu(resp->lmp_subver);
 
+	if (rtl_has_chip_type(lmp_subver)) {
+		ret = rtl_read_chip_type(hdev, &chip_type);
+		if (ret)
+			goto err_free;
+	}
+
 	btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
-					    hdev->bus);
+					    hdev->bus, chip_type);
 
 	if (!btrtl_dev->ic_info)
 		btrtl_dev->drop_fw = true;
@@ -663,7 +755,7 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 		lmp_subver = le16_to_cpu(resp->lmp_subver);
 
 		btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
-						    hdev->bus);
+						    hdev->bus, chip_type);
 	}
 out_free:
 	kfree_skb(skb);
@@ -745,6 +837,7 @@ int btrtl_download_firmware(struct hci_dev *hdev,
 	case RTL_ROM_LMP_8761A:
 	case RTL_ROM_LMP_8822B:
 	case RTL_ROM_LMP_8852A:
+	case RTL_ROM_LMP_8703B:
 		return btrtl_setup_rtl8723b(hdev, btrtl_dev);
 	default:
 		rtl_dev_info(hdev, "assuming no firmware upload needed");
@@ -777,6 +870,19 @@ void btrtl_set_quirks(struct hci_dev *hdev, struct btrtl_device_info *btrtl_dev)
 		rtl_dev_dbg(hdev, "WBS supported not enabled.");
 		break;
 	}
+
+	switch (btrtl_dev->ic_info->lmp_subver) {
+	case RTL_ROM_LMP_8703B:
+		/* 8723CS reports two pages for local ext features,
+		 * but it doesn't support any features from page 2 -
+		 * it either responds with garbage or with error status
+		 */
+		set_bit(HCI_QUIRK_BROKEN_LOCAL_EXT_FTR_MAX_PAGE,
+			&hdev->quirks);
+		break;
+	default:
+		break;
+	}
 }
 EXPORT_SYMBOL_GPL(btrtl_set_quirks);
 
@@ -935,6 +1041,12 @@ MODULE_FIRMWARE("rtl_bt/rtl8723b_fw.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723b_config.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723bs_fw.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723bs_config.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_cg_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_cg_config.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_vf_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_vf_config.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_xx_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_xx_config.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723ds_fw.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723ds_config.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8761a_fw.bin");
diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h
index 2c441bda390a..1c6282241d2d 100644
--- a/drivers/bluetooth/btrtl.h
+++ b/drivers/bluetooth/btrtl.h
@@ -14,6 +14,11 @@
 
 struct btrtl_device_info;
 
+struct rtl_chip_type_evt {
+	__u8 status;
+	__u8 type;
+} __packed;
+
 struct rtl_download_cmd {
 	__u8 index;
 	__u8 data[RTL_FRAG_LEN];
diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index c5a0409ef84f..048fefd03233 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -936,6 +936,8 @@ static int h5_btrtl_setup(struct h5 *h5)
 	err = btrtl_download_firmware(h5->hu->hdev, btrtl_dev);
 	/* Give the device some time before the hci-core sends it a reset */
 	usleep_range(10000, 20000);
+	if (err)
+		goto out_free;
 
 	btrtl_set_quirks(h5->hu->hdev, btrtl_dev);
 
@@ -1100,6 +1102,8 @@ static const struct of_device_id rtl_bluetooth_of_match[] = {
 	  .data = (const void *)&h5_data_rtl8822cs },
 	{ .compatible = "realtek,rtl8723bs-bt",
 	  .data = (const void *)&h5_data_rtl8723bs },
+	{ .compatible = "realtek,rtl8723cs-bt",
+	  .data = (const void *)&h5_data_rtl8723bs },
 	{ .compatible = "realtek,rtl8723ds-bt",
 	  .data = (const void *)&h5_data_rtl8723bs },
 #endif
-- 
2.36.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS

[Bastian Germann]

From: Vasily Khoruzhick <anarsoul@gmail.com>

The Realtek RTL8723CS is SDIO WiFi chip. It also contains a Bluetooth
module which is connected via UART to the host.

It shares lmp subversion with 8703B, so Realtek's userspace
initialization tool (rtk_hciattach) differentiates varieties of RTL8723CS
(CG, VF, XX) with RTL8703B using vendor's command to read chip type.

Also this chip declares support for some features it doesn't support
so add a quirk to indicate that these features are broken.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
[move former btrtl_apply_quirks to btrtl_set_quirks]
[rebase on current tree]
Signed-off-by: Bastian Germann <bage@debian.org>
---
 drivers/bluetooth/btrtl.c  | 120 +++++++++++++++++++++++++++++++++++--
 drivers/bluetooth/btrtl.h  |   5 ++
 drivers/bluetooth/hci_h5.c |   4 ++
 3 files changed, 125 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 481d488bca0f..4b7378a65895 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -17,7 +17,11 @@
 
 #define VERSION "0.1"
 
+#define RTL_CHIP_8723CS_CG	3
+#define RTL_CHIP_8723CS_VF	4
+#define RTL_CHIP_8723CS_XX	5
 #define RTL_EPATCH_SIGNATURE	"Realtech"
+#define RTL_ROM_LMP_8703B	0x8703
 #define RTL_ROM_LMP_8723A	0x1200
 #define RTL_ROM_LMP_8723B	0x8723
 #define RTL_ROM_LMP_8821A	0x8821
@@ -30,6 +34,7 @@
 #define IC_MATCH_FL_HCIREV	(1 << 1)
 #define IC_MATCH_FL_HCIVER	(1 << 2)
 #define IC_MATCH_FL_HCIBUS	(1 << 3)
+#define IC_MATCH_FL_CHIP_TYPE	(1 << 4)
 #define IC_INFO(lmps, hcir, hciv, bus) \
 	.match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_HCIREV | \
 		       IC_MATCH_FL_HCIVER | IC_MATCH_FL_HCIBUS, \
@@ -58,6 +63,7 @@ struct id_table {
 	__u16 hci_rev;
 	__u8 hci_ver;
 	__u8 hci_bus;
+	__u8 chip_type;
 	bool config_needed;
 	bool has_rom_version;
 	bool has_msft_ext;
@@ -98,6 +104,39 @@ static const struct id_table ic_id_table[] = {
 	  .fw_name  = "rtl_bt/rtl8723b_fw.bin",
 	  .cfg_name = "rtl_bt/rtl8723b_config" },
 
+	/* 8723CS-CG */
+	{ .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_CHIP_TYPE |
+			 IC_MATCH_FL_HCIBUS,
+	  .lmp_subver = RTL_ROM_LMP_8703B,
+	  .chip_type = RTL_CHIP_8723CS_CG,
+	  .hci_bus = HCI_UART,
+	  .config_needed = true,
+	  .has_rom_version = true,
+	  .fw_name  = "rtl_bt/rtl8723cs_cg_fw.bin",
+	  .cfg_name = "rtl_bt/rtl8723cs_cg_config" },
+
+	/* 8723CS-VF */
+	{ .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_CHIP_TYPE |
+			 IC_MATCH_FL_HCIBUS,
+	  .lmp_subver = RTL_ROM_LMP_8703B,
+	  .chip_type = RTL_CHIP_8723CS_VF,
+	  .hci_bus = HCI_UART,
+	  .config_needed = true,
+	  .has_rom_version = true,
+	  .fw_name  = "rtl_bt/rtl8723cs_vf_fw.bin",
+	  .cfg_name = "rtl_bt/rtl8723cs_vf_config" },
+
+	/* 8723CS-XX */
+	{ .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_CHIP_TYPE |
+			 IC_MATCH_FL_HCIBUS,
+	  .lmp_subver = RTL_ROM_LMP_8703B,
+	  .chip_type = RTL_CHIP_8723CS_XX,
+	  .hci_bus = HCI_UART,
+	  .config_needed = true,
+	  .has_rom_version = true,
+	  .fw_name  = "rtl_bt/rtl8723cs_xx_fw.bin",
+	  .cfg_name = "rtl_bt/rtl8723cs_xx_config" },
+
 	/* 8723D */
 	{ IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_USB),
 	  .config_needed = true,
@@ -199,7 +238,8 @@ static const struct id_table ic_id_table[] = {
 	};
 
 static const struct id_table *btrtl_match_ic(u16 lmp_subver, u16 hci_rev,
-					     u8 hci_ver, u8 hci_bus)
+					     u8 hci_ver, u8 hci_bus,
+					     u8 chip_type)
 {
 	int i;
 
@@ -216,6 +256,9 @@ static const struct id_table *btrtl_match_ic(u16 lmp_subver, u16 hci_rev,
 		if ((ic_id_table[i].match_flags & IC_MATCH_FL_HCIBUS) &&
 		    (ic_id_table[i].hci_bus != hci_bus))
 			continue;
+		if ((ic_id_table[i].match_flags & IC_MATCH_FL_CHIP_TYPE) &&
+		    (ic_id_table[i].chip_type != chip_type))
+			continue;
 
 		break;
 	}
@@ -298,6 +341,7 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
 		{ RTL_ROM_LMP_8723B, 1 },
 		{ RTL_ROM_LMP_8821A, 2 },
 		{ RTL_ROM_LMP_8761A, 3 },
+		{ RTL_ROM_LMP_8703B, 7 },
 		{ RTL_ROM_LMP_8822B, 8 },
 		{ RTL_ROM_LMP_8723B, 9 },	/* 8723D */
 		{ RTL_ROM_LMP_8821A, 10 },	/* 8821C */
@@ -577,6 +621,48 @@ static int btrtl_setup_rtl8723b(struct hci_dev *hdev,
 	return ret;
 }
 
+static bool rtl_has_chip_type(u16 lmp_subver)
+{
+	switch (lmp_subver) {
+	case RTL_ROM_LMP_8703B:
+		return true;
+	default:
+		break;
+	}
+
+	return  false;
+}
+
+static int rtl_read_chip_type(struct hci_dev *hdev, u8 *type)
+{
+	struct rtl_chip_type_evt *chip_type;
+	struct sk_buff *skb;
+	const unsigned char cmd_buf[] = {0x00, 0x94, 0xa0, 0x00, 0xb0};
+
+	/* Read RTL chip type command */
+	skb = __hci_cmd_sync(hdev, 0xfc61, 5, cmd_buf, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		rtl_dev_err(hdev, "Read chip type failed (%ld)",
+			    PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+
+	if (skb->len != sizeof(*chip_type)) {
+		rtl_dev_err(hdev, "RTL chip type event length mismatch");
+		kfree_skb(skb);
+		return -EIO;
+	}
+
+	chip_type = (struct rtl_chip_type_evt *)skb->data;
+	rtl_dev_info(hdev, "chip_type status=%x type=%x",
+		     chip_type->status, chip_type->type);
+
+	*type = chip_type->type & 0x0f;
+
+	kfree_skb(skb);
+	return 0;
+}
+
 void btrtl_free(struct btrtl_device_info *btrtl_dev)
 {
 	kvfree(btrtl_dev->fw_data);
@@ -593,7 +679,7 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 	struct hci_rp_read_local_version *resp;
 	char cfg_name[40];
 	u16 hci_rev, lmp_subver;
-	u8 hci_ver;
+	u8 hci_ver, chip_type = 0;
 	int ret;
 	u16 opcode;
 	u8 cmd[2];
@@ -619,8 +705,14 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 	hci_rev = le16_to_cpu(resp->hci_rev);
 	lmp_subver = le16_to_cpu(resp->lmp_subver);
 
+	if (rtl_has_chip_type(lmp_subver)) {
+		ret = rtl_read_chip_type(hdev, &chip_type);
+		if (ret)
+			goto err_free;
+	}
+
 	btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
-					    hdev->bus);
+					    hdev->bus, chip_type);
 
 	if (!btrtl_dev->ic_info)
 		btrtl_dev->drop_fw = true;
@@ -663,7 +755,7 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 		lmp_subver = le16_to_cpu(resp->lmp_subver);
 
 		btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
-						    hdev->bus);
+						    hdev->bus, chip_type);
 	}
 out_free:
 	kfree_skb(skb);
@@ -745,6 +837,7 @@ int btrtl_download_firmware(struct hci_dev *hdev,
 	case RTL_ROM_LMP_8761A:
 	case RTL_ROM_LMP_8822B:
 	case RTL_ROM_LMP_8852A:
+	case RTL_ROM_LMP_8703B:
 		return btrtl_setup_rtl8723b(hdev, btrtl_dev);
 	default:
 		rtl_dev_info(hdev, "assuming no firmware upload needed");
@@ -777,6 +870,19 @@ void btrtl_set_quirks(struct hci_dev *hdev, struct btrtl_device_info *btrtl_dev)
 		rtl_dev_dbg(hdev, "WBS supported not enabled.");
 		break;
 	}
+
+	switch (btrtl_dev->ic_info->lmp_subver) {
+	case RTL_ROM_LMP_8703B:
+		/* 8723CS reports two pages for local ext features,
+		 * but it doesn't support any features from page 2 -
+		 * it either responds with garbage or with error status
+		 */
+		set_bit(HCI_QUIRK_BROKEN_LOCAL_EXT_FTR_MAX_PAGE,
+			&hdev->quirks);
+		break;
+	default:
+		break;
+	}
 }
 EXPORT_SYMBOL_GPL(btrtl_set_quirks);
 
@@ -935,6 +1041,12 @@ MODULE_FIRMWARE("rtl_bt/rtl8723b_fw.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723b_config.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723bs_fw.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723bs_config.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_cg_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_cg_config.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_vf_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_vf_config.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_xx_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_xx_config.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723ds_fw.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8723ds_config.bin");
 MODULE_FIRMWARE("rtl_bt/rtl8761a_fw.bin");
diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h
index 2c441bda390a..1c6282241d2d 100644
--- a/drivers/bluetooth/btrtl.h
+++ b/drivers/bluetooth/btrtl.h
@@ -14,6 +14,11 @@
 
 struct btrtl_device_info;
 
+struct rtl_chip_type_evt {
+	__u8 status;
+	__u8 type;
+} __packed;
+
 struct rtl_download_cmd {
 	__u8 index;
 	__u8 data[RTL_FRAG_LEN];
diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index c5a0409ef84f..048fefd03233 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -936,6 +936,8 @@ static int h5_btrtl_setup(struct h5 *h5)
 	err = btrtl_download_firmware(h5->hu->hdev, btrtl_dev);
 	/* Give the device some time before the hci-core sends it a reset */
 	usleep_range(10000, 20000);
+	if (err)
+		goto out_free;
 
 	btrtl_set_quirks(h5->hu->hdev, btrtl_dev);
 
@@ -1100,6 +1102,8 @@ static const struct of_device_id rtl_bluetooth_of_match[] = {
 	  .data = (const void *)&h5_data_rtl8822cs },
 	{ .compatible = "realtek,rtl8723bs-bt",
 	  .data = (const void *)&h5_data_rtl8723bs },
+	{ .compatible = "realtek,rtl8723cs-bt",
+	  .data = (const void *)&h5_data_rtl8723bs },
 	{ .compatible = "realtek,rtl8723ds-bt",
 	  .data = (const void *)&h5_data_rtl8723bs },
 #endif
-- 
2.36.1

[PATCH v2 3/3] arm64: allwinner: a64: enable Bluetooth On Pinebook

[Bastian Germann]

From: Vasily Khoruzhick <anarsoul@gmail.com>

Pinebook has an RTL8723CS WiFi + BT chip, BT is connected to UART1
and uses PL5 as device wake GPIO, PL6 as host wake GPIO the I2C
controlling signals are connected to R_I2C bus.

Enable it in the device tree.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
[add uart-has-rtscts]
Signed-off-by: Bastian Germann <bage@debian.org>
---
 .../boot/dts/allwinner/sun50i-a64-pinebook.dts      | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index 63571df24da4..70d823f8c837 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -406,6 +406,19 @@ &uart0 {
 	status = "okay";
 };
 
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+	uart-has-rtscts;
+	status = "okay";
+
+	bluetooth {
+		compatible = "realtek,rtl8723cs-bt";
+		device-wake-gpios = <&r_pio 0 5 GPIO_ACTIVE_LOW>; /* PL5 */
+		host-wake-gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+	};
+};
+
 &usb_otg {
 	dr_mode = "host";
 };
-- 
2.36.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/3] arm64: allwinner: a64: enable Bluetooth On Pinebook

[Bastian Germann]

From: Vasily Khoruzhick <anarsoul@gmail.com>

Pinebook has an RTL8723CS WiFi + BT chip, BT is connected to UART1
and uses PL5 as device wake GPIO, PL6 as host wake GPIO the I2C
controlling signals are connected to R_I2C bus.

Enable it in the device tree.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
[add uart-has-rtscts]
Signed-off-by: Bastian Germann <bage@debian.org>
---
 .../boot/dts/allwinner/sun50i-a64-pinebook.dts      | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index 63571df24da4..70d823f8c837 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -406,6 +406,19 @@ &uart0 {
 	status = "okay";
 };
 
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+	uart-has-rtscts;
+	status = "okay";
+
+	bluetooth {
+		compatible = "realtek,rtl8723cs-bt";
+		device-wake-gpios = <&r_pio 0 5 GPIO_ACTIVE_LOW>; /* PL5 */
+		host-wake-gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+	};
+};
+
 &usb_otg {
 	dr_mode = "host";
 };
-- 
2.36.1

RE: arm64: allwinner: a64: add bluetooth support for Pinebook

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

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

Re: [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page

[Marcel Holtmann]

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

Re: [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page

[Marcel Holtmann]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS

[Marcel Holtmann]

Hi Bastien,

> The Realtek RTL8723CS is SDIO WiFi chip. It also contains a Bluetooth
> module which is connected via UART to the host.
> 
> It shares lmp subversion with 8703B, so Realtek's userspace
> initialization tool (rtk_hciattach) differentiates varieties of RTL8723CS
> (CG, VF, XX) with RTL8703B using vendor's command to read chip type.
> 
> Also this chip declares support for some features it doesn't support
> so add a quirk to indicate that these features are broken.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> [move former btrtl_apply_quirks to btrtl_set_quirks]
> [rebase on current tree]

I don’t know what these mean. Can you just remove them.

> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
> drivers/bluetooth/btrtl.c  | 120 +++++++++++++++++++++++++++++++++++--
> drivers/bluetooth/btrtl.h  |   5 ++
> drivers/bluetooth/hci_h5.c |   4 ++
> 3 files changed, 125 insertions(+), 4 deletions(-)

Regards

Marcel

Re: [PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS

[Marcel Holtmann]

Hi Bastien,

> The Realtek RTL8723CS is SDIO WiFi chip. It also contains a Bluetooth
> module which is connected via UART to the host.
> 
> It shares lmp subversion with 8703B, so Realtek's userspace
> initialization tool (rtk_hciattach) differentiates varieties of RTL8723CS
> (CG, VF, XX) with RTL8703B using vendor's command to read chip type.
> 
> Also this chip declares support for some features it doesn't support
> so add a quirk to indicate that these features are broken.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> [move former btrtl_apply_quirks to btrtl_set_quirks]
> [rebase on current tree]

I don’t know what these mean. Can you just remove them.

> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
> drivers/bluetooth/btrtl.c  | 120 +++++++++++++++++++++++++++++++++++--
> drivers/bluetooth/btrtl.h  |   5 ++
> drivers/bluetooth/hci_h5.c |   4 ++
> 3 files changed, 125 insertions(+), 4 deletions(-)

Regards

Marcel


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page

[Vasily Khoruzhick]

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
>

Re: [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page

[Vasily Khoruzhick]

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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel