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

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

[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.

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     |  12 ++
 drivers/bluetooth/btrtl.c                     | 128 +++++++++++++++++-
 drivers/bluetooth/btrtl.h                     |  12 ++
 drivers/bluetooth/hci_h5.c                    |   6 +
 include/net/bluetooth/hci.h                   |   7 +
 net/bluetooth/hci_event.c                     |   4 +-
 6 files changed, 165 insertions(+), 4 deletions(-)

-- 
2.27.0

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

[Vasily Khoruzhick]

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>
---
 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 16ab6ce87883..8e4c16210d18 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -227,6 +227,13 @@ enum {
 	 * supported.
 	 */
 	HCI_QUIRK_VALID_LE_STATES,
+
+	/* 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 cfeaee347db3..df3232828978 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -700,7 +700,9 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
 	if (rp->status)
 		return;
 
-	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.27.0

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

[Marcel Holtmann]

Hi Vasily,

> 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 send the btmon for this so I can see what the controller is responding.

Regards

Marcel

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

[Vasily Khoruzhick]

On Tue, Jul 7, 2020 at 9:03 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Vasily,
>
> > 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 send the btmon for this so I can see what the controller is responding.

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

hci0 registration stops here and bluetoothctl doesn't even see the controller.

> Regards
>
> Marcel
>

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

[Marcel Holtmann]

Hi Vasily,

>>> 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 send the btmon for this so I can see what the controller is responding.
> 
> 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
> 
> hci0 registration stops here and bluetoothctl doesn't even see the controller.

maybe just the read sync train params command is broken? Can you change the init code and not send it and see if the rest of the init phase proceeds. I would rather have the secure connections actually tested before dismissing it altogether.

Mind you, there were broken Broadcom implementation of connectionless slave broadcast as well. Maybe this is similar.

Regards

Marcel

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

[Vasily Khoruzhick]

On Sun, Jul 12, 2020 at 11:28 PM Marcel Holtmann <marcel@holtmann.org> wrote:

Hi Marcel,

> maybe just the read sync train params command is broken? Can you change the init code and not send it and see if the rest of the init phase proceeds. I would rather have the secure connections actually tested before dismissing it altogether.

I don't think that I have any devices that support secure connections
to test, I've got only a bluetooth mouse and headphones, both are from
the 2.0 era.

FWIW unofficial recommendation from Realtek to Pine64 was to avoid
using any 4.1+ features on this chip. Unfortunately I don't have any
contacts with Realtek, so I can't confirm that.

> Mind you, there were broken Broadcom implementation of connectionless slave broadcast as well. Maybe this is similar.

I'd prefer to stick to what works unless there's some comprehensive
test that can figure out what's broken.

Regards,
Vasily

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

[Marcel Holtmann]

Hi Vasily,

>> maybe just the read sync train params command is broken? Can you change the init code and not send it and see if the rest of the init phase proceeds. I would rather have the secure connections actually tested before dismissing it altogether.
> 
> I don't think that I have any devices that support secure connections
> to test, I've got only a bluetooth mouse and headphones, both are from
> the 2.0 era.
> 
> FWIW unofficial recommendation from Realtek to Pine64 was to avoid
> using any 4.1+ features on this chip. Unfortunately I don't have any
> contacts with Realtek, so I can't confirm that.
> 
>> Mind you, there were broken Broadcom implementation of connectionless slave broadcast as well. Maybe this is similar.
> 
> I'd prefer to stick to what works unless there's some comprehensive
> test that can figure out what's broken.

check if removing the read sync trains params command makes the controller initialize and usable. Then we see about the rest.

Regards

Marcel

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

[Vasily Khoruzhick]

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>
---
 drivers/bluetooth/btrtl.c  | 128 ++++++++++++++++++++++++++++++++++++-
 drivers/bluetooth/btrtl.h  |  12 ++++
 drivers/bluetooth/hci_h5.c |   6 ++
 3 files changed, 143 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 3a9afc905f24..8c7b35abe492 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -17,8 +17,12 @@
 
 #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_3499	0x3499
+#define RTL_ROM_LMP_8703B	0x8703
 #define RTL_ROM_LMP_8723A	0x1200
 #define RTL_ROM_LMP_8723B	0x8723
 #define RTL_ROM_LMP_8723D	0x8873
@@ -31,6 +35,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) \
 	.match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_HCIREV, \
 	.lmp_subver = (lmps), \
@@ -42,6 +47,7 @@ struct id_table {
 	__u16 hci_rev;
 	__u8 hci_ver;
 	__u8 hci_bus;
+	__u8 chip_type;
 	bool config_needed;
 	bool has_rom_version;
 	char *fw_name;
@@ -89,6 +95,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),
 	  .config_needed = true,
@@ -171,7 +210,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;
 
@@ -188,6 +228,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;
 	}
@@ -270,6 +313,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 */
@@ -545,6 +589,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);
@@ -561,7 +647,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;
 
 	btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
@@ -586,8 +672,14 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 	lmp_subver = le16_to_cpu(resp->lmp_subver);
 	kfree_skb(skb);
 
+	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) {
 		rtl_dev_info(hdev, "unknown IC info, lmp subver %04x, hci rev %04x, hci ver %04x",
@@ -660,6 +752,7 @@ int btrtl_download_firmware(struct hci_dev *hdev,
 	case RTL_ROM_LMP_8821A:
 	case RTL_ROM_LMP_8761A:
 	case RTL_ROM_LMP_8822B:
+	case RTL_ROM_LMP_8703B:
 		return btrtl_setup_rtl8723b(hdev, btrtl_dev);
 	default:
 		rtl_dev_info(hdev, "assuming no firmware upload needed");
@@ -678,7 +771,12 @@ int btrtl_setup_realtek(struct hci_dev *hdev)
 		return PTR_ERR(btrtl_dev);
 
 	ret = btrtl_download_firmware(hdev, btrtl_dev);
+	if (ret)
+		goto out_free;
 
+	btrtl_apply_quirks(hdev, btrtl_dev);
+
+out_free:
 	btrtl_free(btrtl_dev);
 
 	/* Enable controller to do both LE scan and BR/EDR inquiry
@@ -818,6 +916,24 @@ int btrtl_get_uart_settings(struct hci_dev *hdev,
 }
 EXPORT_SYMBOL_GPL(btrtl_get_uart_settings);
 
+void btrtl_apply_quirks(struct hci_dev *hdev,
+			struct btrtl_device_info *btrtl_dev)
+{
+	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_apply_quirks);
+
 MODULE_AUTHOR("Daniel Drake <drake@endlessm.com>");
 MODULE_DESCRIPTION("Bluetooth support for Realtek devices ver " VERSION);
 MODULE_VERSION(VERSION);
@@ -827,6 +943,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 2a582682136d..152ed2ece4c2 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];
@@ -60,6 +65,8 @@ int btrtl_get_uart_settings(struct hci_dev *hdev,
 			    struct btrtl_device_info *btrtl_dev,
 			    unsigned int *controller_baudrate,
 			    u32 *device_baudrate, bool *flow_control);
+void btrtl_apply_quirks(struct hci_dev *hdev,
+			struct btrtl_device_info *btrtl_dev);
 
 #else
 
@@ -96,6 +103,11 @@ static inline int btrtl_get_uart_settings(struct hci_dev *hdev,
 					  bool *flow_control)
 {
 	return -ENOENT;
+
+static inline void btrtl_apply_quirks(struct hci_dev *hdev,
+			struct btrtl_device_info *btrtl_dev)
+{
+}
 }
 
 #endif
diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index e60b2e0773db..c34acab9e8ac 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -904,6 +904,10 @@ 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_apply_quirks(h5->hu->hdev, btrtl_dev);
 
 out_free:
 	btrtl_free(btrtl_dev);
@@ -1020,6 +1024,8 @@ static const struct of_device_id rtl_bluetooth_of_match[] = {
 	  .data = (const void *)&rtl_vnd },
 	{ .compatible = "realtek,rtl8723bs-bt",
 	  .data = (const void *)&rtl_vnd },
+	{ .compatible = "realtek,rtl8723cs-bt",
+	  .data = (const void *)&rtl_vnd },
 #endif
 	{ },
 };
-- 
2.27.0

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

[Marcel Holtmann]

Hi Vasily,

> 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>
> ---
> drivers/bluetooth/btrtl.c  | 128 ++++++++++++++++++++++++++++++++++++-
> drivers/bluetooth/btrtl.h  |  12 ++++
> drivers/bluetooth/hci_h5.c |   6 ++
> 3 files changed, 143 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index 3a9afc905f24..8c7b35abe492 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -17,8 +17,12 @@
> 
> #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_3499	0x3499
> +#define RTL_ROM_LMP_8703B	0x8703
> #define RTL_ROM_LMP_8723A	0x1200
> #define RTL_ROM_LMP_8723B	0x8723
> #define RTL_ROM_LMP_8723D	0x8873
> @@ -31,6 +35,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) \
> 	.match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_HCIREV, \
> 	.lmp_subver = (lmps), \
> @@ -42,6 +47,7 @@ struct id_table {
> 	__u16 hci_rev;
> 	__u8 hci_ver;
> 	__u8 hci_bus;
> +	__u8 chip_type;
> 	bool config_needed;
> 	bool has_rom_version;
> 	char *fw_name;
> @@ -89,6 +95,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),
> 	  .config_needed = true,
> @@ -171,7 +210,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;
> 
> @@ -188,6 +228,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;
> 	}
> @@ -270,6 +313,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 */
> @@ -545,6 +589,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);
> @@ -561,7 +647,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;
> 
> 	btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
> @@ -586,8 +672,14 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> 	lmp_subver = le16_to_cpu(resp->lmp_subver);
> 	kfree_skb(skb);
> 
> +	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) {
> 		rtl_dev_info(hdev, "unknown IC info, lmp subver %04x, hci rev %04x, hci ver %04x",
> @@ -660,6 +752,7 @@ int btrtl_download_firmware(struct hci_dev *hdev,
> 	case RTL_ROM_LMP_8821A:
> 	case RTL_ROM_LMP_8761A:
> 	case RTL_ROM_LMP_8822B:
> +	case RTL_ROM_LMP_8703B:
> 		return btrtl_setup_rtl8723b(hdev, btrtl_dev);
> 	default:
> 		rtl_dev_info(hdev, "assuming no firmware upload needed");
> @@ -678,7 +771,12 @@ int btrtl_setup_realtek(struct hci_dev *hdev)
> 		return PTR_ERR(btrtl_dev);
> 
> 	ret = btrtl_download_firmware(hdev, btrtl_dev);
> +	if (ret)
> +		goto out_free;
> 
> +	btrtl_apply_quirks(hdev, btrtl_dev);
> +
> +out_free:
> 	btrtl_free(btrtl_dev);
> 
> 	/* Enable controller to do both LE scan and BR/EDR inquiry
> @@ -818,6 +916,24 @@ int btrtl_get_uart_settings(struct hci_dev *hdev,
> }
> EXPORT_SYMBOL_GPL(btrtl_get_uart_settings);
> 
> +void btrtl_apply_quirks(struct hci_dev *hdev,
> +			struct btrtl_device_info *btrtl_dev)
> +{
> +	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_apply_quirks);
> +
> MODULE_AUTHOR("Daniel Drake <drake@endlessm.com>");
> MODULE_DESCRIPTION("Bluetooth support for Realtek devices ver " VERSION);
> MODULE_VERSION(VERSION);
> @@ -827,6 +943,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 2a582682136d..152ed2ece4c2 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];
> @@ -60,6 +65,8 @@ int btrtl_get_uart_settings(struct hci_dev *hdev,
> 			    struct btrtl_device_info *btrtl_dev,
> 			    unsigned int *controller_baudrate,
> 			    u32 *device_baudrate, bool *flow_control);
> +void btrtl_apply_quirks(struct hci_dev *hdev,
> +			struct btrtl_device_info *btrtl_dev);
> 
> #else
> 
> @@ -96,6 +103,11 @@ static inline int btrtl_get_uart_settings(struct hci_dev *hdev,
> 					  bool *flow_control)
> {
> 	return -ENOENT;
> +
> +static inline void btrtl_apply_quirks(struct hci_dev *hdev,
> +			struct btrtl_device_info *btrtl_dev)
> +{
> +}
> }

this hunk is fundamentally broken.

Regards

Marcel

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

[Vasily Khoruzhick]

On Tue, Jul 7, 2020 at 9:01 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Vasily,
>
> > 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>
> > ---
> > drivers/bluetooth/btrtl.c  | 128 ++++++++++++++++++++++++++++++++++++-
> > drivers/bluetooth/btrtl.h  |  12 ++++
> > drivers/bluetooth/hci_h5.c |   6 ++
> > 3 files changed, 143 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> > index 3a9afc905f24..8c7b35abe492 100644
> > --- a/drivers/bluetooth/btrtl.c
> > +++ b/drivers/bluetooth/btrtl.c
> > @@ -17,8 +17,12 @@
> >
> > #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_3499      0x3499
> > +#define RTL_ROM_LMP_8703B    0x8703
> > #define RTL_ROM_LMP_8723A     0x1200
> > #define RTL_ROM_LMP_8723B     0x8723
> > #define RTL_ROM_LMP_8723D     0x8873
> > @@ -31,6 +35,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) \
> >       .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_HCIREV, \
> >       .lmp_subver = (lmps), \
> > @@ -42,6 +47,7 @@ struct id_table {
> >       __u16 hci_rev;
> >       __u8 hci_ver;
> >       __u8 hci_bus;
> > +     __u8 chip_type;
> >       bool config_needed;
> >       bool has_rom_version;
> >       char *fw_name;
> > @@ -89,6 +95,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),
> >         .config_needed = true,
> > @@ -171,7 +210,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;
> >
> > @@ -188,6 +228,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;
> >       }
> > @@ -270,6 +313,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 */
> > @@ -545,6 +589,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);
> > @@ -561,7 +647,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;
> >
> >       btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
> > @@ -586,8 +672,14 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> >       lmp_subver = le16_to_cpu(resp->lmp_subver);
> >       kfree_skb(skb);
> >
> > +     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) {
> >               rtl_dev_info(hdev, "unknown IC info, lmp subver %04x, hci rev %04x, hci ver %04x",
> > @@ -660,6 +752,7 @@ int btrtl_download_firmware(struct hci_dev *hdev,
> >       case RTL_ROM_LMP_8821A:
> >       case RTL_ROM_LMP_8761A:
> >       case RTL_ROM_LMP_8822B:
> > +     case RTL_ROM_LMP_8703B:
> >               return btrtl_setup_rtl8723b(hdev, btrtl_dev);
> >       default:
> >               rtl_dev_info(hdev, "assuming no firmware upload needed");
> > @@ -678,7 +771,12 @@ int btrtl_setup_realtek(struct hci_dev *hdev)
> >               return PTR_ERR(btrtl_dev);
> >
> >       ret = btrtl_download_firmware(hdev, btrtl_dev);
> > +     if (ret)
> > +             goto out_free;
> >
> > +     btrtl_apply_quirks(hdev, btrtl_dev);
> > +
> > +out_free:
> >       btrtl_free(btrtl_dev);
> >
> >       /* Enable controller to do both LE scan and BR/EDR inquiry
> > @@ -818,6 +916,24 @@ int btrtl_get_uart_settings(struct hci_dev *hdev,
> > }
> > EXPORT_SYMBOL_GPL(btrtl_get_uart_settings);
> >
> > +void btrtl_apply_quirks(struct hci_dev *hdev,
> > +                     struct btrtl_device_info *btrtl_dev)
> > +{
> > +     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_apply_quirks);
> > +
> > MODULE_AUTHOR("Daniel Drake <drake@endlessm.com>");
> > MODULE_DESCRIPTION("Bluetooth support for Realtek devices ver " VERSION);
> > MODULE_VERSION(VERSION);
> > @@ -827,6 +943,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 2a582682136d..152ed2ece4c2 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];
> > @@ -60,6 +65,8 @@ int btrtl_get_uart_settings(struct hci_dev *hdev,
> >                           struct btrtl_device_info *btrtl_dev,
> >                           unsigned int *controller_baudrate,
> >                           u32 *device_baudrate, bool *flow_control);
> > +void btrtl_apply_quirks(struct hci_dev *hdev,
> > +                     struct btrtl_device_info *btrtl_dev);
> >
> > #else
> >
> > @@ -96,6 +103,11 @@ static inline int btrtl_get_uart_settings(struct hci_dev *hdev,
> >                                         bool *flow_control)
> > {
> >       return -ENOENT;
> > +
> > +static inline void btrtl_apply_quirks(struct hci_dev *hdev,
> > +                     struct btrtl_device_info *btrtl_dev)
> > +{
> > +}
> > }
>
> this hunk is fundamentally broken.

Will fix in v2

> Regards
>
> Marcel
>

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

[Vasily Khoruzhick]

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>
---
 .../arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index 64b1c54f87c0..e63ff271be4e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -408,6 +408,18 @@ &uart0 {
 	status = "okay";
 };
 
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+	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.27.0

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

[Maxime Ripard]

Hi,

On Sun, Jul 05, 2020 at 12:51:10PM -0700, Vasily Khoruzhick wrote:
> 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>
> ---
>  .../arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> index 64b1c54f87c0..e63ff271be4e 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> @@ -408,6 +408,18 @@ &uart0 {
>  	status = "okay";
>  };
>  
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
> +	status = "okay";

You probably need uart-has-rtscts here

> +
> +	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 */
> +	};

And max-speed I guess?

Maxime
>

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

[Vasily Khoruzhick]

On Mon, Jul 6, 2020 at 4:47 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> On Sun, Jul 05, 2020 at 12:51:10PM -0700, Vasily Khoruzhick wrote:
> > 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>
> > ---
> >  .../arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > index 64b1c54f87c0..e63ff271be4e 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
> > @@ -408,6 +408,18 @@ &uart0 {
> >       status = "okay";
> >  };
> >
> > +&uart1 {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
> > +     status = "okay";
>
> You probably need uart-has-rtscts here

Will add in v2

> > +
> > +     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 */
> > +     };
>
> And max-speed I guess?

There's no max-speed in the schema for this bluetooth controller.
Moreover it reads uart settings from firmware config. See
btrtl_get_uart_settings() in drivers/bluetooth/btrtl.c

> Maxime
> >