Pinebook uses RTL8723CS for WiFi and bluetooth. Unfortunately, RTL8723CS
has broken BT-4.1 support, so it requires a quirk.
Add a quirk and wire up 8723CS support in btrtl.
I was asked for a btmon output without the quirk;
however, using the chip without the quirk ends up in a bad state with
"Opcode 0x c77 failed: -56" (HCI_OP_READ_SYNC_TRAIN_PARAMS) on training.
A btmon output with the quirk active was already sent by Vasily.
v1 of this series was sent in July 2020 by Vasily Khoruzhick.
I have tested it to work on the Pinebook.
Changelog:
v2:
* Rebase
* Add uart-has-rtscts to device tree as requested by reviewer
v3:
* Drop the device tree as it was split out and is already integrated.
* Rename the quirk as requested by reviewer Marcel Holtmann
v4:
* Use skb_pull_data as requested by reviewer Luiz Augusto von Dentz
v5:
* Make use of skb_pull_data's length check
v6:
* Warn on active quirk
Vasily Khoruzhick (2):
Bluetooth: Add new quirk for broken local ext features page 2
Bluetooth: btrtl: add support for the RTL8723CS
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 | 9 ++-
5 files changed, 139 insertions(+), 6 deletions(-)
--
2.39.2
From: Vasily Khoruzhick <[email protected]>
The Realtek RTL8723CS is a 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 <[email protected]>
Signed-off-by: Bastian Germann <[email protected]>
---
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 69c3fe649ca7..44b672cca69e 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, \
@@ -59,6 +64,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;
@@ -99,6 +105,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,
@@ -208,7 +247,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;
@@ -225,6 +265,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;
}
@@ -307,6 +350,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 */
@@ -587,6 +631,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);
+ }
+
+ chip_type = skb_pull_data(skb, sizeof(*chip_type));
+ if (!chip_type) {
+ rtl_dev_err(hdev, "RTL chip type event length mismatch");
+ kfree_skb(skb);
+ return -EIO;
+ }
+
+ 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);
@@ -603,7 +689,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];
@@ -629,8 +715,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;
@@ -673,7 +765,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);
@@ -755,6 +847,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");
@@ -795,6 +888,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_FEATURES_PAGE_2,
+ &hdev->quirks);
+ break;
+ default:
+ break;
+ }
}
EXPORT_SYMBOL_GPL(btrtl_set_quirks);
@@ -953,6 +1059,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 ebf0101c959b..349d72ee571b 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 6455bc4fb5bb..e90670955df2 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.39.2
From: Vasily Khoruzhick <[email protected]>
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 <[email protected]>
Signed-off-by: Bastian Germann <[email protected]>
---
include/net/bluetooth/hci.h | 7 +++++++
net/bluetooth/hci_event.c | 9 +++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 400f8a7d0c3f..997107bfc0b1 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -294,6 +294,13 @@ enum {
* during the hdev->setup vendor callback.
*/
HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG,
+
+ /* 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_FEATURES_PAGE_2,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ad92a4be5851..8d8547fa9032 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -886,8 +886,13 @@ 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)
- hdev->max_page = rp->max_page;
+ if (hdev->max_page < rp->max_page) {
+ if (test_bit(HCI_QUIRK_BROKEN_LOCAL_EXT_FEATURES_PAGE_2,
+ &hdev->quirks))
+ bt_dev_warn(hdev, "broken local ext features page 2");
+ else
+ hdev->max_page = rp->max_page;
+ }
if (rp->page < HCI_MAX_PAGES)
memcpy(hdev->features[rp->page], rp->features, 8);
--
2.39.2
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=727648
---Test result---
Test Summary:
CheckPatch PASS 2.00 seconds
GitLint PASS 0.68 seconds
SubjectPrefix PASS 0.25 seconds
BuildKernel PASS 31.28 seconds
CheckAllWarning PASS 34.68 seconds
CheckSparse WARNING 39.08 seconds
CheckSmatch WARNING 106.29 seconds
BuildKernel32 PASS 30.42 seconds
TestRunnerSetup PASS 442.73 seconds
TestRunner_l2cap-tester PASS 16.49 seconds
TestRunner_iso-tester PASS 17.64 seconds
TestRunner_bnep-tester PASS 5.72 seconds
TestRunner_mgmt-tester PASS 112.73 seconds
TestRunner_rfcomm-tester PASS 9.10 seconds
TestRunner_sco-tester PASS 8.38 seconds
TestRunner_ioctl-tester PASS 9.78 seconds
TestRunner_mesh-tester PASS 7.24 seconds
TestRunner_smp-tester PASS 8.22 seconds
TestRunner_userchan-tester PASS 6.12 seconds
IncrementalBuild PASS 34.31 seconds
Details
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):drivers/bluetooth/btrtl.c: note: in included file:drivers/bluetooth/btrtl.h:52:45: warning: array of flexible structures
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):drivers/bluetooth/btrtl.c: note: in included file:drivers/bluetooth/btrtl.h:52:45: warning: array of flexible structures
---
Regards,
Linux Bluetooth
Hello:
This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Tue, 7 Mar 2023 23:17:29 +0100 you wrote:
> Pinebook uses RTL8723CS for WiFi and bluetooth. Unfortunately, RTL8723CS
> has broken BT-4.1 support, so it requires a quirk.
>
> Add a quirk and wire up 8723CS support in btrtl.
> I was asked for a btmon output without the quirk;
> however, using the chip without the quirk ends up in a bad state with
> "Opcode 0x c77 failed: -56" (HCI_OP_READ_SYNC_TRAIN_PARAMS) on training.
> A btmon output with the quirk active was already sent by Vasily.
>
> [...]
Here is the summary with links:
- [v6,1/2] Bluetooth: Add new quirk for broken local ext features page 2
https://git.kernel.org/bluetooth/bluetooth-next/c/000b57d5c009
- [v6,2/2] Bluetooth: btrtl: add support for the RTL8723CS
https://git.kernel.org/bluetooth/bluetooth-next/c/b8e482d02513
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html