linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] arm64: allwinner: a64: add bluetooth support for Pinebook
@ 2022-05-24 21:21 Bastian Germann
  2022-05-24 21:21 ` [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page Bastian Germann
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Bastian Germann @ 2022-05-24 21:21 UTC (permalink / raw)
  To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Marcel Holtmann,
	Johan Hedberg, David S . Miller, Jakub Kicinski, devicetree,
	linux-arm-kernel, linux-kernel, linux-bluetooth, netdev
  Cc: Bastian Germann, Vasily Khoruzhick

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page
  2022-05-24 21:21 [PATCH v2 0/3] arm64: allwinner: a64: add bluetooth support for Pinebook Bastian Germann
@ 2022-05-24 21:21 ` Bastian Germann
  2022-06-02 16:10   ` Marcel Holtmann
  2022-05-24 21:21 ` [PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS Bastian Germann
  2022-05-24 21:21 ` [PATCH v2 3/3] arm64: allwinner: a64: enable Bluetooth On Pinebook Bastian Germann
  2 siblings, 1 reply; 7+ messages in thread
From: Bastian Germann @ 2022-05-24 21:21 UTC (permalink / raw)
  To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Marcel Holtmann,
	Johan Hedberg, David S . Miller, Jakub Kicinski, devicetree,
	linux-arm-kernel, linux-kernel, linux-bluetooth, netdev
  Cc: Vasily Khoruzhick, 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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS
  2022-05-24 21:21 [PATCH v2 0/3] arm64: allwinner: a64: add bluetooth support for Pinebook Bastian Germann
  2022-05-24 21:21 ` [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page Bastian Germann
@ 2022-05-24 21:21 ` Bastian Germann
  2022-06-02 16:14   ` Marcel Holtmann
  2022-05-24 21:21 ` [PATCH v2 3/3] arm64: allwinner: a64: enable Bluetooth On Pinebook Bastian Germann
  2 siblings, 1 reply; 7+ messages in thread
From: Bastian Germann @ 2022-05-24 21:21 UTC (permalink / raw)
  To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Marcel Holtmann,
	Johan Hedberg, David S . Miller, Jakub Kicinski, devicetree,
	linux-arm-kernel, linux-kernel, linux-bluetooth, netdev
  Cc: Vasily Khoruzhick, 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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/3] arm64: allwinner: a64: enable Bluetooth On Pinebook
  2022-05-24 21:21 [PATCH v2 0/3] arm64: allwinner: a64: add bluetooth support for Pinebook Bastian Germann
  2022-05-24 21:21 ` [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page Bastian Germann
  2022-05-24 21:21 ` [PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS Bastian Germann
@ 2022-05-24 21:21 ` Bastian Germann
  2 siblings, 0 replies; 7+ messages in thread
From: Bastian Germann @ 2022-05-24 21:21 UTC (permalink / raw)
  To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Marcel Holtmann,
	Johan Hedberg, David S . Miller, Jakub Kicinski, devicetree,
	linux-arm-kernel, linux-kernel, linux-bluetooth, netdev
  Cc: Vasily Khoruzhick, 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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page
  2022-05-24 21:21 ` [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page Bastian Germann
@ 2022-06-02 16:10   ` Marcel Holtmann
  2022-11-05  7:13     ` Vasily Khoruzhick
  0 siblings, 1 reply; 7+ messages in thread
From: Marcel Holtmann @ 2022-06-02 16:10 UTC (permalink / raw)
  To: Bastian Germann
  Cc: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Johan Hedberg,
	David S. Miller, Jakub Kicinski, devicetree, linux-arm-kernel,
	linux-kernel, linux-bluetooth, netdev, Vasily Khoruzhick

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS
  2022-05-24 21:21 ` [PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS Bastian Germann
@ 2022-06-02 16:14   ` Marcel Holtmann
  0 siblings, 0 replies; 7+ messages in thread
From: Marcel Holtmann @ 2022-06-02 16:14 UTC (permalink / raw)
  To: Bastian Germann
  Cc: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Johan Hedberg,
	David S. Miller, Jakub Kicinski, devicetree, linux-arm-kernel,
	linux-kernel, linux-bluetooth, netdev, Vasily Khoruzhick

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page
  2022-06-02 16:10   ` Marcel Holtmann
@ 2022-11-05  7:13     ` Vasily Khoruzhick
  0 siblings, 0 replies; 7+ messages in thread
From: Vasily Khoruzhick @ 2022-11-05  7:13 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Bastian Germann, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Johan Hedberg, David S. Miller, Jakub Kicinski, devicetree,
	linux-arm-kernel, linux-kernel, linux-bluetooth, netdev

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-11-05  7:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-24 21:21 [PATCH v2 0/3] arm64: allwinner: a64: add bluetooth support for Pinebook Bastian Germann
2022-05-24 21:21 ` [PATCH v2 1/3] Bluetooth: Add new quirk for broken local ext features max_page Bastian Germann
2022-06-02 16:10   ` Marcel Holtmann
2022-11-05  7:13     ` Vasily Khoruzhick
2022-05-24 21:21 ` [PATCH v2 2/3] Bluetooth: btrtl: add support for the RTL8723CS Bastian Germann
2022-06-02 16:14   ` Marcel Holtmann
2022-05-24 21:21 ` [PATCH v2 3/3] arm64: allwinner: a64: enable Bluetooth On Pinebook Bastian Germann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).