First things first, I do not have access to the specs nor the schematics, so a
lot of this was done via guess work, looking at the acpi tables, and looking at
how a similar device (wcn6750) was added.
The 5th revision addresses comments from Luiz about the Bluetooth driver, as
well as Konrad's comments on the dts file.
The end result is that we do have a working device, but not entirely reliable.
Hopefully by getting this out there, people who do have access to the specs or
schematics can see where the improvements or fixes need to come.
There are a few things that I am not sure why they happen, and don't have the
knowledge level to figure out why they happen or debugging it.
Bluetooth: hci0: setting up wcn6855
Bluetooth: hci0: Frame reassembly failed (-84)
Bluetooth: hci0: QCA Product ID :0x00000013
Bluetooth: hci0: QCA SOC Version :0x400c0210
Bluetooth: hci0: QCA ROM Version :0x00000201
Bluetooth: hci0: QCA Patch Version:0x000038e6
Bluetooth: hci0: QCA controller version 0x02100201
Bluetooth: hci0: QCA Downloading qca/hpbtfw21.tlv
Bluetooth: hci0: QCA Downloading qca/hpnv21.bin
Bluetooth: hci0: QCA setup on UART is completed
I do not know why the Frame assembly failed, and modprobe -r hci_uart and then
modprobe hci_uart does not show the same Frame assembly failed.
The BD Address also seems to be incorrect, and I'm not sure what is going on
there either.
Testing was done by connecting a Razer Orochi bluetooth mouse, and using it, as
well as connecting to and using an H2GO bluetooth speaker and playing audio out
via canberra-gtk-play as well as a couple of YouTube videos in a browser.
The mouse only seems to work when < 2 ft. from the laptop, and for the speaker, only
"A2DP Sink, codec SBC" would provide audio output, and while I could see that
data was being sent to the speaker, it wasn't always outputting, and going >
4ft. away, would often disconnect.
steev@wintermute:~$ hciconfig -a
hci0: Type: Primary Bus: UART
BD Address: 00:00:00:00:5A:AD ACL MTU: 1024:8 SCO MTU: 240:4
UP RUNNING PSCAN
RX bytes:1492 acl:0 sco:0 events:126 errors:0
TX bytes:128743 acl:0 sco:0 commands:597 errors:0
Features: 0xff 0xfe 0x8f 0xfe 0xd8 0x3f 0x5b 0x87
Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3
Link policy: RSWITCH HOLD SNIFF
Link mode: PERIPHERAL ACCEPT
Name: 'wintermute'
Class: 0x0c010c
Service Classes: Rendering, Capturing
Device Class: Computer, Laptop
HCI Version: (0xc) Revision: 0x0
LMP Version: (0xc) Subversion: 0x46f7
Manufacturer: Qualcomm (29)
steev@wintermute:~$ dmesg | grep Razer
[ 3089.235440] input: Razer Orochi as /devices/virtual/misc/uhid/0005:1532:0056.0003/input/input11
[ 3089.238580] hid-generic 0005:1532:0056.0003: input,hidraw2: BLUETOOTH HID v0.01 Mouse [Razer Orochi] on 00:00:00:00:5a:ad
steev@wintermute:~$ dmesg | grep H2GO
[ 3140.959947] input: H2GO Speaker (AVRCP) as /devices/virtual/input/input12
Bjorn Andersson (1):
arm64: dts: qcom: sc8280xp: Define uart2
Steev Klimaszewski (3):
dt-bindings: net: Add WCN6855 Bluetooth
Bluetooth: hci_qca: Add support for QTI Bluetooth chip wcn6855
arm64: dts: qcom: thinkpad-x13s: Add bluetooth
.../net/bluetooth/qualcomm-bluetooth.yaml | 17 +++++
.../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 76 +++++++++++++++++++
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 14 ++++
drivers/bluetooth/btqca.c | 9 ++-
drivers/bluetooth/btqca.h | 10 +++
drivers/bluetooth/hci_qca.c | 50 +++++++++---
6 files changed, 163 insertions(+), 13 deletions(-)
base-commit: 4fafd96910add124586b549ad005dcd179de8a18
--
2.39.1
Add bindings for the QTI WCN6855 chipset.
Signed-off-by: Steev Klimaszewski <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
Changes since v4:
* None
.../net/bluetooth/qualcomm-bluetooth.yaml | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
index a6a6b0e4df7a..68f78b90d23a 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
@@ -23,6 +23,7 @@ properties:
- qcom,wcn3998-bt
- qcom,qca6390-bt
- qcom,wcn6750-bt
+ - qcom,wcn6855-bt
enable-gpios:
maxItems: 1
@@ -133,6 +134,22 @@ allOf:
- vddrfa1p7-supply
- vddrfa1p2-supply
- vddasd-supply
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,wcn6855-bt
+ then:
+ required:
+ - enable-gpios
+ - swctrl-gpios
+ - vddio-supply
+ - vddbtcxmx-supply
+ - vddrfacmn-supply
+ - vddrfa0p8-supply
+ - vddrfa1p2-supply
+ - vddrfa1p7-supply
examples:
- |
--
2.39.1
Added regulators,GPIOs and changes required to power on/off wcn6855.
Added support for firmware download for wcn6855.
Signed-off-by: Steev Klimaszewski <[email protected]>
---
Changes since v4:
* Remove unused firmware check because we don't have mbn firmware.
* Set qcadev->init_speed if it hasn't been set.
Changes since v3:
* drop unused regulators
Changes since v2:
* drop unnecessary commit info
Changes since v1:
* None
drivers/bluetooth/btqca.c | 9 ++++++-
drivers/bluetooth/btqca.h | 10 ++++++++
drivers/bluetooth/hci_qca.c | 50 ++++++++++++++++++++++++++++---------
3 files changed, 56 insertions(+), 13 deletions(-)
diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index c9064d34d830..2f9d8bd27c38 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -614,6 +614,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
config.type = ELF_TYPE_PATCH;
snprintf(config.fwname, sizeof(config.fwname),
"qca/msbtfw%02x.mbn", rom_ver);
+ } else if (soc_type == QCA_WCN6855) {
+ snprintf(config.fwname, sizeof(config.fwname),
+ "qca/hpbtfw%02x.tlv", rom_ver);
} else {
snprintf(config.fwname, sizeof(config.fwname),
"qca/rampatch_%08x.bin", soc_ver);
@@ -648,6 +651,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
else if (soc_type == QCA_WCN6750)
snprintf(config.fwname, sizeof(config.fwname),
"qca/msnv%02x.bin", rom_ver);
+ else if (soc_type == QCA_WCN6855)
+ snprintf(config.fwname, sizeof(config.fwname),
+ "qca/hpnv%02x.bin", rom_ver);
else
snprintf(config.fwname, sizeof(config.fwname),
"qca/nvm_%08x.bin", soc_ver);
@@ -672,6 +678,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
case QCA_WCN3991:
case QCA_WCN3998:
case QCA_WCN6750:
+ case QCA_WCN6855:
hci_set_msft_opcode(hdev, 0xFD70);
break;
default:
@@ -685,7 +692,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
return err;
}
- if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) {
+ if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || soc_type == QCA_WCN6855) {
/* get fw build info */
err = qca_read_fw_build_info(hdev);
if (err < 0)
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index 61e9a50e66ae..b884095bcd9d 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -147,6 +147,7 @@ enum qca_btsoc_type {
QCA_WCN3991,
QCA_QCA6390,
QCA_WCN6750,
+ QCA_WCN6855,
};
#if IS_ENABLED(CONFIG_BT_QCA)
@@ -168,6 +169,10 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type)
{
return soc_type == QCA_WCN6750;
}
+static inline bool qca_is_wcn6855(enum qca_btsoc_type soc_type)
+{
+ return soc_type == QCA_WCN6855;
+}
#else
@@ -206,6 +211,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type)
return false;
}
+static inline bool qca_is_wcn6855(enum qca_btsoc_type soc_type)
+{
+ return false;
+}
+
static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
{
return -EOPNOTSUPP;
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 3df8c3606e93..efc1c0306b4e 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -605,8 +605,7 @@ static int qca_open(struct hci_uart *hu)
if (hu->serdev) {
qcadev = serdev_device_get_drvdata(hu->serdev);
- if (qca_is_wcn399x(qcadev->btsoc_type) ||
- qca_is_wcn6750(qcadev->btsoc_type))
+ if (!(qcadev->init_speed))
hu->init_speed = qcadev->init_speed;
if (qcadev->oper_speed)
@@ -1317,7 +1316,8 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate)
/* Give the controller time to process the request */
if (qca_is_wcn399x(qca_soc_type(hu)) ||
- qca_is_wcn6750(qca_soc_type(hu)))
+ qca_is_wcn6750(qca_soc_type(hu)) ||
+ qca_is_wcn6855(qca_soc_type(hu)))
usleep_range(1000, 10000);
else
msleep(300);
@@ -1394,7 +1394,8 @@ static unsigned int qca_get_speed(struct hci_uart *hu,
static int qca_check_speeds(struct hci_uart *hu)
{
if (qca_is_wcn399x(qca_soc_type(hu)) ||
- qca_is_wcn6750(qca_soc_type(hu))) {
+ qca_is_wcn6750(qca_soc_type(hu)) ||
+ qca_is_wcn6855(qca_soc_type(hu))) {
if (!qca_get_speed(hu, QCA_INIT_SPEED) &&
!qca_get_speed(hu, QCA_OPER_SPEED))
return -EINVAL;
@@ -1682,7 +1683,8 @@ static int qca_power_on(struct hci_dev *hdev)
return 0;
if (qca_is_wcn399x(soc_type) ||
- qca_is_wcn6750(soc_type)) {
+ qca_is_wcn6750(soc_type) ||
+ qca_is_wcn6855(soc_type)) {
ret = qca_regulator_init(hu);
} else {
qcadev = serdev_device_get_drvdata(hu->serdev);
@@ -1723,7 +1725,8 @@ static int qca_setup(struct hci_uart *hu)
bt_dev_info(hdev, "setting up %s",
qca_is_wcn399x(soc_type) ? "wcn399x" :
- (soc_type == QCA_WCN6750) ? "wcn6750" : "ROME/QCA6390");
+ (soc_type == QCA_WCN6750) ? "wcn6750" :
+ (soc_type == QCA_WCN6855) ? "wcn6855" : "ROME/QCA6390");
qca->memdump_state = QCA_MEMDUMP_IDLE;
@@ -1735,7 +1738,8 @@ static int qca_setup(struct hci_uart *hu)
clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
if (qca_is_wcn399x(soc_type) ||
- qca_is_wcn6750(soc_type)) {
+ qca_is_wcn6750(soc_type) ||
+ qca_is_wcn6855(soc_type)) {
set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
hci_set_aosp_capable(hdev);
@@ -1757,7 +1761,8 @@ static int qca_setup(struct hci_uart *hu)
}
if (!(qca_is_wcn399x(soc_type) ||
- qca_is_wcn6750(soc_type))) {
+ qca_is_wcn6750(soc_type) ||
+ qca_is_wcn6855(soc_type))) {
/* Get QCA version information */
ret = qca_read_soc_version(hdev, &ver, soc_type);
if (ret)
@@ -1883,6 +1888,20 @@ static const struct qca_device_data qca_soc_data_wcn6750 = {
.capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
};
+static const struct qca_device_data qca_soc_data_wcn6855 = {
+ .soc_type = QCA_WCN6855,
+ .vregs = (struct qca_vreg []) {
+ { "vddio", 5000 },
+ { "vddbtcxmx", 126000 },
+ { "vddrfacmn", 12500 },
+ { "vddrfa0p8", 102000 },
+ { "vddrfa1p7", 302000 },
+ { "vddrfa1p2", 257000 },
+ },
+ .num_vregs = 6,
+ .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
+};
+
static void qca_power_shutdown(struct hci_uart *hu)
{
struct qca_serdev *qcadev;
@@ -2047,7 +2066,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
if (data &&
(qca_is_wcn399x(data->soc_type) ||
- qca_is_wcn6750(data->soc_type))) {
+ qca_is_wcn6750(data->soc_type) ||
+ qca_is_wcn6855(data->soc_type))) {
qcadev->btsoc_type = data->soc_type;
qcadev->bt_power = devm_kzalloc(&serdev->dev,
sizeof(struct qca_power),
@@ -2067,14 +2087,18 @@ static int qca_serdev_probe(struct serdev_device *serdev)
qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
GPIOD_OUT_LOW);
- if (IS_ERR_OR_NULL(qcadev->bt_en) && data->soc_type == QCA_WCN6750) {
+ if (IS_ERR_OR_NULL(qcadev->bt_en)
+ && (data->soc_type == QCA_WCN6750 ||
+ data->soc_type == QCA_WCN6855)) {
dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n");
power_ctrl_enabled = false;
}
qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl",
GPIOD_IN);
- if (IS_ERR_OR_NULL(qcadev->sw_ctrl) && data->soc_type == QCA_WCN6750)
+ if (IS_ERR_OR_NULL(qcadev->sw_ctrl)
+ && (data->soc_type == QCA_WCN6750 ||
+ data->soc_type == QCA_WCN6855))
dev_warn(&serdev->dev, "failed to acquire SW_CTRL gpio\n");
qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
@@ -2150,7 +2174,8 @@ static void qca_serdev_remove(struct serdev_device *serdev)
struct qca_power *power = qcadev->bt_power;
if ((qca_is_wcn399x(qcadev->btsoc_type) ||
- qca_is_wcn6750(qcadev->btsoc_type)) &&
+ qca_is_wcn6750(qcadev->btsoc_type) ||
+ qca_is_wcn6855(qcadev->btsoc_type)) &&
power->vregs_on)
qca_power_shutdown(&qcadev->serdev_hu);
else if (qcadev->susclk)
@@ -2335,6 +2360,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
{ .compatible = "qcom,wcn3991-bt", .data = &qca_soc_data_wcn3991},
{ .compatible = "qcom,wcn3998-bt", .data = &qca_soc_data_wcn3998},
{ .compatible = "qcom,wcn6750-bt", .data = &qca_soc_data_wcn6750},
+ { .compatible = "qcom,wcn6855-bt", .data = &qca_soc_data_wcn6855},
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match);
--
2.39.1
From: Bjorn Andersson <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Steev Klimaszewski <[email protected]>
Reviewed-by: Brian Masney <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
Changes since v4:
* None
Changes since v3:
* Fix commit message changelog
Changes since v2:
* No changes since v2
Changes since v1:
* change subject line, move node, and add my s-o-b
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index fa2d0d7d1367..eab54aab3b76 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1207,6 +1207,20 @@ spi2: spi@988000 {
status = "disabled";
};
+ uart2: serial@988000 {
+ compatible = "qcom,geni-uart";
+ reg = <0 0x00988000 0 0x4000>;
+ clocks = <&gcc GCC_QUPV3_WRAP0_S2_CLK>;
+ clock-names = "se";
+ interrupts = <GIC_SPI 603 IRQ_TYPE_LEVEL_HIGH>;
+ operating-points-v2 = <&qup_opp_table_100mhz>;
+ power-domains = <&rpmhpd SC8280XP_CX>;
+ interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
+ <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_0 0>;
+ interconnect-names = "qup-core", "qup-config";
+ status = "disabled";
+ };
+
i2c3: i2c@98c000 {
compatible = "qcom,geni-i2c";
reg = <0 0x0098c000 0 0x4000>;
--
2.39.1
The Lenovo Thinkpad X13s has a WCN6855 Bluetooth controller on uart2,
add this.
Signed-off-by: Steev Klimaszewski <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
Changes since v4:
* Address Konrad's review comments.
Changes since v3:
* Add vreg_s1c
* Add regulators and not dead code
* Fix commit message changelog
Changes since v2:
* Remove dead code and add TODO comment
* Make dtbs_check happy with the pin definitions
.../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 76 +++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index f936b020a71d..ad20cfb3a830 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -24,6 +24,8 @@ / {
aliases {
i2c4 = &i2c4;
i2c21 = &i2c21;
+ serial0 = &uart17;
+ serial1 = &uart2;
};
wcd938x: audio-codec {
@@ -297,6 +299,15 @@ pmc8280c-rpmh-regulators {
qcom,pmic-id = "c";
vdd-bob-supply = <&vreg_vph_pwr>;
+ vreg_s1c: smps1 {
+ regulator-name = "vreg_s1c";
+ regulator-min-microvolt = <1880000>;
+ regulator-max-microvolt = <1900000>;
+ regulator-allowed-modes = <RPMH_REGULATOR_MODE_AUTO>,
+ <RPMH_REGULATOR_MODE_RET>;
+ regulator-allow-set-load;
+ };
+
vreg_l1c: ldo1 {
regulator-name = "vreg_l1c";
regulator-min-microvolt = <1800000>;
@@ -712,6 +723,32 @@ &qup0 {
status = "okay";
};
+&uart2 {
+ pinctrl-0 = <&uart2_state>;
+ pinctrl-names = "default";
+
+ status = "okay";
+
+ bluetooth {
+ compatible = "qcom,wcn6855-bt";
+
+ vddio-supply = <&vreg_s10b>;
+ vddbtcxmx-supply = <&vreg_s12b>;
+ vddrfacmn-supply = <&vreg_s12b>;
+ vddrfa0p8-supply = <&vreg_s12b>;
+ vddrfa1p2-supply = <&vreg_s11b>;
+ vddrfa1p7-supply = <&vreg_s1c>;
+
+ max-speed = <3200000>;
+
+ enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>;
+ swctrl-gpios = <&tlmm 132 GPIO_ACTIVE_HIGH>;
+
+ pinctrl-0 = <&bt_en>;
+ pinctrl-names = "default";
+ };
+};
+
&qup1 {
status = "okay";
};
@@ -720,6 +757,11 @@ &qup2 {
status = "okay";
};
+&uart17 {
+ compatible = "qcom,geni-debug-uart";
+ status = "okay";
+};
+
&remoteproc_adsp {
firmware-name = "qcom/sc8280xp/LENOVO/21BX/qcadsp8280.mbn";
@@ -980,6 +1022,19 @@ hastings_reg_en: hastings-reg-en-state {
&tlmm {
gpio-reserved-ranges = <70 2>, <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
+ bt_en: bt-en-state {
+ hstp-sw-ctrl-pins {
+ pins = "gpio132";
+ function = "gpio";
+ };
+
+ hstp-bt-en-pins {
+ pins = "gpio133";
+ function = "gpio";
+ drive-strength = <16>;
+ };
+ };
+
edp_reg_en: edp-reg-en-state {
pins = "gpio25";
function = "gpio";
@@ -1001,6 +1056,27 @@ i2c4_default: i2c4-default-state {
bias-disable;
};
+ uart2_state: uart2-state {
+ cts-pins {
+ pins = "gpio122";
+ function = "qup2";
+ bias-disable;
+ };
+
+ rts-tx-pins {
+ pins = "gpio122", "gpio123";
+ function = "qup2";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ rx-pins {
+ pins = "gpio124";
+ function = "qup2";
+ bias-pull-up;
+ };
+ };
+
i2c21_default: i2c21-default-state {
pins = "gpio81", "gpio82";
function = "qup21";
--
2.39.1
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: arch/arm64/boot/dts/qcom/sc8280xp.dtsi:1207
error: arch/arm64/boot/dts/qcom/sc8280xp.dtsi: 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
On Wed, Feb 08, 2023 at 08:09:14PM -0600, Steev Klimaszewski wrote:
> Added regulators,GPIOs and changes required to power on/off wcn6855.
> Added support for firmware download for wcn6855.
>
> Signed-off-by: Steev Klimaszewski <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Tested-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
> ---
> Changes since v4:
> * Remove unused firmware check because we don't have mbn firmware.
> * Set qcadev->init_speed if it hasn't been set.
>
> Changes since v3:
> * drop unused regulators
>
> Changes since v2:
> * drop unnecessary commit info
>
> Changes since v1:
> * None
>
> drivers/bluetooth/btqca.c | 9 ++++++-
> drivers/bluetooth/btqca.h | 10 ++++++++
> drivers/bluetooth/hci_qca.c | 50 ++++++++++++++++++++++++++++---------
> 3 files changed, 56 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index c9064d34d830..2f9d8bd27c38 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -614,6 +614,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> config.type = ELF_TYPE_PATCH;
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/msbtfw%02x.mbn", rom_ver);
> + } else if (soc_type == QCA_WCN6855) {
> + snprintf(config.fwname, sizeof(config.fwname),
> + "qca/hpbtfw%02x.tlv", rom_ver);
> } else {
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/rampatch_%08x.bin", soc_ver);
> @@ -648,6 +651,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> else if (soc_type == QCA_WCN6750)
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/msnv%02x.bin", rom_ver);
> + else if (soc_type == QCA_WCN6855)
> + snprintf(config.fwname, sizeof(config.fwname),
> + "qca/hpnv%02x.bin", rom_ver);
> else
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/nvm_%08x.bin", soc_ver);
> @@ -672,6 +678,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> case QCA_WCN3991:
> case QCA_WCN3998:
> case QCA_WCN6750:
> + case QCA_WCN6855:
> hci_set_msft_opcode(hdev, 0xFD70);
> break;
> default:
> @@ -685,7 +692,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> return err;
> }
>
> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) {
> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || soc_type == QCA_WCN6855) {
> /* get fw build info */
> err = qca_read_fw_build_info(hdev);
> if (err < 0)
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index 61e9a50e66ae..b884095bcd9d 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -147,6 +147,7 @@ enum qca_btsoc_type {
> QCA_WCN3991,
> QCA_QCA6390,
> QCA_WCN6750,
> + QCA_WCN6855,
> };
>
> #if IS_ENABLED(CONFIG_BT_QCA)
> @@ -168,6 +169,10 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type)
> {
> return soc_type == QCA_WCN6750;
> }
> +static inline bool qca_is_wcn6855(enum qca_btsoc_type soc_type)
> +{
> + return soc_type == QCA_WCN6855;
> +}
>
> #else
>
> @@ -206,6 +211,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type)
> return false;
> }
>
> +static inline bool qca_is_wcn6855(enum qca_btsoc_type soc_type)
> +{
> + return false;
> +}
> +
> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
> {
> return -EOPNOTSUPP;
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 3df8c3606e93..efc1c0306b4e 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -605,8 +605,7 @@ static int qca_open(struct hci_uart *hu)
> if (hu->serdev) {
> qcadev = serdev_device_get_drvdata(hu->serdev);
>
> - if (qca_is_wcn399x(qcadev->btsoc_type) ||
> - qca_is_wcn6750(qcadev->btsoc_type))
> + if (!(qcadev->init_speed))
> hu->init_speed = qcadev->init_speed;
>
> if (qcadev->oper_speed)
> @@ -1317,7 +1316,8 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate)
>
> /* Give the controller time to process the request */
> if (qca_is_wcn399x(qca_soc_type(hu)) ||
> - qca_is_wcn6750(qca_soc_type(hu)))
> + qca_is_wcn6750(qca_soc_type(hu)) ||
> + qca_is_wcn6855(qca_soc_type(hu)))
> usleep_range(1000, 10000);
> else
> msleep(300);
> @@ -1394,7 +1394,8 @@ static unsigned int qca_get_speed(struct hci_uart *hu,
> static int qca_check_speeds(struct hci_uart *hu)
> {
> if (qca_is_wcn399x(qca_soc_type(hu)) ||
> - qca_is_wcn6750(qca_soc_type(hu))) {
> + qca_is_wcn6750(qca_soc_type(hu)) ||
> + qca_is_wcn6855(qca_soc_type(hu))) {
> if (!qca_get_speed(hu, QCA_INIT_SPEED) &&
> !qca_get_speed(hu, QCA_OPER_SPEED))
> return -EINVAL;
> @@ -1682,7 +1683,8 @@ static int qca_power_on(struct hci_dev *hdev)
> return 0;
>
> if (qca_is_wcn399x(soc_type) ||
> - qca_is_wcn6750(soc_type)) {
> + qca_is_wcn6750(soc_type) ||
> + qca_is_wcn6855(soc_type)) {
> ret = qca_regulator_init(hu);
> } else {
> qcadev = serdev_device_get_drvdata(hu->serdev);
> @@ -1723,7 +1725,8 @@ static int qca_setup(struct hci_uart *hu)
>
> bt_dev_info(hdev, "setting up %s",
> qca_is_wcn399x(soc_type) ? "wcn399x" :
> - (soc_type == QCA_WCN6750) ? "wcn6750" : "ROME/QCA6390");
> + (soc_type == QCA_WCN6750) ? "wcn6750" :
> + (soc_type == QCA_WCN6855) ? "wcn6855" : "ROME/QCA6390");
>
> qca->memdump_state = QCA_MEMDUMP_IDLE;
>
> @@ -1735,7 +1738,8 @@ static int qca_setup(struct hci_uart *hu)
> clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
>
> if (qca_is_wcn399x(soc_type) ||
> - qca_is_wcn6750(soc_type)) {
> + qca_is_wcn6750(soc_type) ||
> + qca_is_wcn6855(soc_type)) {
> set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
> hci_set_aosp_capable(hdev);
>
> @@ -1757,7 +1761,8 @@ static int qca_setup(struct hci_uart *hu)
> }
>
> if (!(qca_is_wcn399x(soc_type) ||
> - qca_is_wcn6750(soc_type))) {
> + qca_is_wcn6750(soc_type) ||
> + qca_is_wcn6855(soc_type))) {
> /* Get QCA version information */
> ret = qca_read_soc_version(hdev, &ver, soc_type);
> if (ret)
> @@ -1883,6 +1888,20 @@ static const struct qca_device_data qca_soc_data_wcn6750 = {
> .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> };
>
> +static const struct qca_device_data qca_soc_data_wcn6855 = {
> + .soc_type = QCA_WCN6855,
> + .vregs = (struct qca_vreg []) {
> + { "vddio", 5000 },
> + { "vddbtcxmx", 126000 },
> + { "vddrfacmn", 12500 },
> + { "vddrfa0p8", 102000 },
> + { "vddrfa1p7", 302000 },
> + { "vddrfa1p2", 257000 },
> + },
> + .num_vregs = 6,
> + .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> +};
> +
> static void qca_power_shutdown(struct hci_uart *hu)
> {
> struct qca_serdev *qcadev;
> @@ -2047,7 +2066,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>
> if (data &&
> (qca_is_wcn399x(data->soc_type) ||
> - qca_is_wcn6750(data->soc_type))) {
> + qca_is_wcn6750(data->soc_type) ||
> + qca_is_wcn6855(data->soc_type))) {
> qcadev->btsoc_type = data->soc_type;
> qcadev->bt_power = devm_kzalloc(&serdev->dev,
> sizeof(struct qca_power),
> @@ -2067,14 +2087,18 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>
> qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
> GPIOD_OUT_LOW);
> - if (IS_ERR_OR_NULL(qcadev->bt_en) && data->soc_type == QCA_WCN6750) {
> + if (IS_ERR_OR_NULL(qcadev->bt_en)
> + && (data->soc_type == QCA_WCN6750 ||
> + data->soc_type == QCA_WCN6855)) {
> dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n");
> power_ctrl_enabled = false;
> }
>
> qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl",
> GPIOD_IN);
> - if (IS_ERR_OR_NULL(qcadev->sw_ctrl) && data->soc_type == QCA_WCN6750)
> + if (IS_ERR_OR_NULL(qcadev->sw_ctrl)
> + && (data->soc_type == QCA_WCN6750 ||
> + data->soc_type == QCA_WCN6855))
> dev_warn(&serdev->dev, "failed to acquire SW_CTRL gpio\n");
>
> qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
> @@ -2150,7 +2174,8 @@ static void qca_serdev_remove(struct serdev_device *serdev)
> struct qca_power *power = qcadev->bt_power;
>
> if ((qca_is_wcn399x(qcadev->btsoc_type) ||
> - qca_is_wcn6750(qcadev->btsoc_type)) &&
> + qca_is_wcn6750(qcadev->btsoc_type) ||
> + qca_is_wcn6855(qcadev->btsoc_type)) &&
> power->vregs_on)
> qca_power_shutdown(&qcadev->serdev_hu);
> else if (qcadev->susclk)
> @@ -2335,6 +2360,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
> { .compatible = "qcom,wcn3991-bt", .data = &qca_soc_data_wcn3991},
> { .compatible = "qcom,wcn3998-bt", .data = &qca_soc_data_wcn3998},
> { .compatible = "qcom,wcn6750-bt", .data = &qca_soc_data_wcn6750},
> + { .compatible = "qcom,wcn6855-bt", .data = &qca_soc_data_wcn6855},
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match);
> --
> 2.39.1
>
On Wed, Feb 08, 2023 at 08:09:12PM -0600, Steev Klimaszewski wrote:
> First things first, I do not have access to the specs nor the schematics, so a
> lot of this was done via guess work, looking at the acpi tables, and looking at
> how a similar device (wcn6750) was added.
>
> The 5th revision addresses comments from Luiz about the Bluetooth driver, as
> well as Konrad's comments on the dts file.
>
> The end result is that we do have a working device, but not entirely reliable.
>
Except for the one warning/error about frame assembly I've not seen any
reliability issues with this series.
> Hopefully by getting this out there, people who do have access to the specs or
> schematics can see where the improvements or fixes need to come.
>
> There are a few things that I am not sure why they happen, and don't have the
> knowledge level to figure out why they happen or debugging it.
>
> Bluetooth: hci0: setting up wcn6855
> Bluetooth: hci0: Frame reassembly failed (-84)
> Bluetooth: hci0: QCA Product ID :0x00000013
> Bluetooth: hci0: QCA SOC Version :0x400c0210
> Bluetooth: hci0: QCA ROM Version :0x00000201
> Bluetooth: hci0: QCA Patch Version:0x000038e6
> Bluetooth: hci0: QCA controller version 0x02100201
> Bluetooth: hci0: QCA Downloading qca/hpbtfw21.tlv
> Bluetooth: hci0: QCA Downloading qca/hpnv21.bin
> Bluetooth: hci0: QCA setup on UART is completed
>
> I do not know why the Frame assembly failed, and modprobe -r hci_uart and then
> modprobe hci_uart does not show the same Frame assembly failed.
>
> The BD Address also seems to be incorrect, and I'm not sure what is going on
> there either.
>
Changing the public-addr after the fact works...
> Testing was done by connecting a Razer Orochi bluetooth mouse, and using it, as
> well as connecting to and using an H2GO bluetooth speaker and playing audio out
> via canberra-gtk-play as well as a couple of YouTube videos in a browser.
>
> The mouse only seems to work when < 2 ft. from the laptop, and for the speaker, only
> "A2DP Sink, codec SBC" would provide audio output, and while I could see that
> data was being sent to the speaker, it wasn't always outputting, and going >
> 4ft. away, would often disconnect.
>
With the interference from WiFi removed I have very positive results
with this, been listening to music using this for a week now without any
concerns.
Tested-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
> steev@wintermute:~$ hciconfig -a
> hci0: Type: Primary Bus: UART
> BD Address: 00:00:00:00:5A:AD ACL MTU: 1024:8 SCO MTU: 240:4
> UP RUNNING PSCAN
> RX bytes:1492 acl:0 sco:0 events:126 errors:0
> TX bytes:128743 acl:0 sco:0 commands:597 errors:0
> Features: 0xff 0xfe 0x8f 0xfe 0xd8 0x3f 0x5b 0x87
> Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3
> Link policy: RSWITCH HOLD SNIFF
> Link mode: PERIPHERAL ACCEPT
> Name: 'wintermute'
> Class: 0x0c010c
> Service Classes: Rendering, Capturing
> Device Class: Computer, Laptop
> HCI Version: (0xc) Revision: 0x0
> LMP Version: (0xc) Subversion: 0x46f7
> Manufacturer: Qualcomm (29)
>
> steev@wintermute:~$ dmesg | grep Razer
> [ 3089.235440] input: Razer Orochi as /devices/virtual/misc/uhid/0005:1532:0056.0003/input/input11
> [ 3089.238580] hid-generic 0005:1532:0056.0003: input,hidraw2: BLUETOOTH HID v0.01 Mouse [Razer Orochi] on 00:00:00:00:5a:ad
> steev@wintermute:~$ dmesg | grep H2GO
> [ 3140.959947] input: H2GO Speaker (AVRCP) as /devices/virtual/input/input12
>
> Bjorn Andersson (1):
> arm64: dts: qcom: sc8280xp: Define uart2
>
> Steev Klimaszewski (3):
> dt-bindings: net: Add WCN6855 Bluetooth
> Bluetooth: hci_qca: Add support for QTI Bluetooth chip wcn6855
> arm64: dts: qcom: thinkpad-x13s: Add bluetooth
>
> .../net/bluetooth/qualcomm-bluetooth.yaml | 17 +++++
> .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 76 +++++++++++++++++++
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 14 ++++
> drivers/bluetooth/btqca.c | 9 ++-
> drivers/bluetooth/btqca.h | 10 +++
> drivers/bluetooth/hci_qca.c | 50 +++++++++---
> 6 files changed, 163 insertions(+), 13 deletions(-)
>
>
> base-commit: 4fafd96910add124586b549ad005dcd179de8a18
> --
> 2.39.1
>
On Wed, Feb 08, 2023 at 08:09:14PM -0600, Steev Klimaszewski wrote:
> Added regulators,GPIOs and changes required to power on/off wcn6855.
> Added support for firmware download for wcn6855.
>
> Signed-off-by: Steev Klimaszewski <[email protected]>
> ---
> drivers/bluetooth/btqca.c | 9 ++++++-
> drivers/bluetooth/btqca.h | 10 ++++++++
> drivers/bluetooth/hci_qca.c | 50 ++++++++++++++++++++++++++++---------
> 3 files changed, 56 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index c9064d34d830..2f9d8bd27c38 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> +static const struct qca_device_data qca_soc_data_wcn6855 = {
> + .soc_type = QCA_WCN6855,
> + .vregs = (struct qca_vreg []) {
> + { "vddio", 5000 },
> + { "vddbtcxmx", 126000 },
> + { "vddrfacmn", 12500 },
> + { "vddrfa0p8", 102000 },
> + { "vddrfa1p7", 302000 },
> + { "vddrfa1p2", 257000 },
> + },
> + .num_vregs = 6,
> + .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> +};
> +
> static void qca_power_shutdown(struct hci_uart *hu)
> {
> struct qca_serdev *qcadev;
As I just mentioned IRC, you forgot to update qca_power_shutdown() here
so the regulators are currently never disabled (e.g. when closing the
device or on module unload).
> @@ -2047,7 +2066,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>
> if (data &&
> (qca_is_wcn399x(data->soc_type) ||
> - qca_is_wcn6750(data->soc_type))) {
> + qca_is_wcn6750(data->soc_type) ||
> + qca_is_wcn6855(data->soc_type))) {
> qcadev->btsoc_type = data->soc_type;
> qcadev->bt_power = devm_kzalloc(&serdev->dev,
> sizeof(struct qca_power),
> @@ -2150,7 +2174,8 @@ static void qca_serdev_remove(struct serdev_device *serdev)
> struct qca_power *power = qcadev->bt_power;
>
> if ((qca_is_wcn399x(qcadev->btsoc_type) ||
> - qca_is_wcn6750(qcadev->btsoc_type)) &&
> + qca_is_wcn6750(qcadev->btsoc_type) ||
> + qca_is_wcn6855(qcadev->btsoc_type)) &&
> power->vregs_on)
> qca_power_shutdown(&qcadev->serdev_hu);
> else if (qcadev->susclk)
Johan
On Wed, Feb 08, 2023 at 08:09:14PM -0600, Steev Klimaszewski wrote:
> Added regulators,GPIOs and changes required to power on/off wcn6855.
> Added support for firmware download for wcn6855.
>
> Signed-off-by: Steev Klimaszewski <[email protected]>
> ---
> Changes since v4:
> * Remove unused firmware check because we don't have mbn firmware.
> * Set qcadev->init_speed if it hasn't been set.
>
> Changes since v3:
> * drop unused regulators
>
> Changes since v2:
> * drop unnecessary commit info
>
> Changes since v1:
> * None
>
> drivers/bluetooth/btqca.c | 9 ++++++-
> drivers/bluetooth/btqca.h | 10 ++++++++
> drivers/bluetooth/hci_qca.c | 50 ++++++++++++++++++++++++++++---------
> 3 files changed, 56 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index c9064d34d830..2f9d8bd27c38 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -614,6 +614,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> config.type = ELF_TYPE_PATCH;
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/msbtfw%02x.mbn", rom_ver);
> + } else if (soc_type == QCA_WCN6855) {
> + snprintf(config.fwname, sizeof(config.fwname),
> + "qca/hpbtfw%02x.tlv", rom_ver);
> } else {
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/rampatch_%08x.bin", soc_ver);
> @@ -648,6 +651,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> else if (soc_type == QCA_WCN6750)
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/msnv%02x.bin", rom_ver);
> + else if (soc_type == QCA_WCN6855)
> + snprintf(config.fwname, sizeof(config.fwname),
> + "qca/hpnv%02x.bin", rom_ver);
> else
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/nvm_%08x.bin", soc_ver);
> @@ -672,6 +678,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> case QCA_WCN3991:
> case QCA_WCN3998:
> case QCA_WCN6750:
> + case QCA_WCN6855:
Did you actually verify the microsoft extensions need this, or you are
assuming it works as 6750?
> hci_set_msft_opcode(hdev, 0xFD70);
> break;
> default:
> @@ -685,7 +692,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> return err;
> }
>
> - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) {
> + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || soc_type == QCA_WCN6855) {
Line is now over 80 columns which is still the preferred limit.
Perhaps this should now be a switch statement instead?
> /* get fw build info */
> err = qca_read_fw_build_info(hdev);
> if (err < 0)
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index 61e9a50e66ae..b884095bcd9d 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -147,6 +147,7 @@ enum qca_btsoc_type {
> QCA_WCN3991,
> QCA_QCA6390,
> QCA_WCN6750,
> + QCA_WCN6855,
> };
>
> #if IS_ENABLED(CONFIG_BT_QCA)
> @@ -168,6 +169,10 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type)
> {
> return soc_type == QCA_WCN6750;
> }
> +static inline bool qca_is_wcn6855(enum qca_btsoc_type soc_type)
> +{
> + return soc_type == QCA_WCN6855;
> +}
>
> #else
>
> @@ -206,6 +211,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type)
> return false;
> }
>
> +static inline bool qca_is_wcn6855(enum qca_btsoc_type soc_type)
> +{
> + return false;
> +}
> +
> static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
> {
> return -EOPNOTSUPP;
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 3df8c3606e93..efc1c0306b4e 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -605,8 +605,7 @@ static int qca_open(struct hci_uart *hu)
> if (hu->serdev) {
> qcadev = serdev_device_get_drvdata(hu->serdev);
>
> - if (qca_is_wcn399x(qcadev->btsoc_type) ||
> - qca_is_wcn6750(qcadev->btsoc_type))
> + if (!(qcadev->init_speed))
> hu->init_speed = qcadev->init_speed;
This change makes no sense.
In fact, it seems the driver never sets init_speed anywhere.
Either way, it should not be needed for wcn6855.
>
> if (qcadev->oper_speed)
> @@ -1317,7 +1316,8 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate)
>
> /* Give the controller time to process the request */
> if (qca_is_wcn399x(qca_soc_type(hu)) ||
> - qca_is_wcn6750(qca_soc_type(hu)))
> + qca_is_wcn6750(qca_soc_type(hu)) ||
> + qca_is_wcn6855(qca_soc_type(hu)))
> usleep_range(1000, 10000);
> else
> msleep(300);
> @@ -1394,7 +1394,8 @@ static unsigned int qca_get_speed(struct hci_uart *hu,
> static int qca_check_speeds(struct hci_uart *hu)
> {
> if (qca_is_wcn399x(qca_soc_type(hu)) ||
> - qca_is_wcn6750(qca_soc_type(hu))) {
> + qca_is_wcn6750(qca_soc_type(hu)) ||
> + qca_is_wcn6855(qca_soc_type(hu))) {
> if (!qca_get_speed(hu, QCA_INIT_SPEED) &&
> !qca_get_speed(hu, QCA_OPER_SPEED))
> return -EINVAL;
> @@ -1682,7 +1683,8 @@ static int qca_power_on(struct hci_dev *hdev)
> return 0;
>
> if (qca_is_wcn399x(soc_type) ||
> - qca_is_wcn6750(soc_type)) {
> + qca_is_wcn6750(soc_type) ||
> + qca_is_wcn6855(soc_type)) {
> ret = qca_regulator_init(hu);
> } else {
> qcadev = serdev_device_get_drvdata(hu->serdev);
> @@ -1723,7 +1725,8 @@ static int qca_setup(struct hci_uart *hu)
>
> bt_dev_info(hdev, "setting up %s",
> qca_is_wcn399x(soc_type) ? "wcn399x" :
> - (soc_type == QCA_WCN6750) ? "wcn6750" : "ROME/QCA6390");
> + (soc_type == QCA_WCN6750) ? "wcn6750" :
> + (soc_type == QCA_WCN6855) ? "wcn6855" : "ROME/QCA6390");
This is hideous, but not your fault...
>
> qca->memdump_state = QCA_MEMDUMP_IDLE;
>
> @@ -1735,7 +1738,8 @@ static int qca_setup(struct hci_uart *hu)
> clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
>
> if (qca_is_wcn399x(soc_type) ||
> - qca_is_wcn6750(soc_type)) {
> + qca_is_wcn6750(soc_type) ||
> + qca_is_wcn6855(soc_type)) {
> set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
> hci_set_aosp_capable(hdev);
>
> @@ -1757,7 +1761,8 @@ static int qca_setup(struct hci_uart *hu)
> }
>
> if (!(qca_is_wcn399x(soc_type) ||
> - qca_is_wcn6750(soc_type))) {
> + qca_is_wcn6750(soc_type) ||
> + qca_is_wcn6855(soc_type))) {
Perhaps you can add a leading space while changing this so that the
open-parenthesis alignment makes sense.
> /* Get QCA version information */
> ret = qca_read_soc_version(hdev, &ver, soc_type);
> if (ret)
> @@ -1883,6 +1888,20 @@ static const struct qca_device_data qca_soc_data_wcn6750 = {
> .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> };
>
> +static const struct qca_device_data qca_soc_data_wcn6855 = {
> + .soc_type = QCA_WCN6855,
> + .vregs = (struct qca_vreg []) {
> + { "vddio", 5000 },
> + { "vddbtcxmx", 126000 },
> + { "vddrfacmn", 12500 },
> + { "vddrfa0p8", 102000 },
> + { "vddrfa1p7", 302000 },
> + { "vddrfa1p2", 257000 },
Hmm. More random regulator load values. I really think we should get rid
of this but that's a separate discussion.
> + },
> + .num_vregs = 6,
> + .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> +};
> +
> static void qca_power_shutdown(struct hci_uart *hu)
> {
> struct qca_serdev *qcadev;
As I mentioned elsewhere, you need to update also this function so that
wcn6855 can be powered down.
> @@ -2047,7 +2066,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>
> if (data &&
> (qca_is_wcn399x(data->soc_type) ||
> - qca_is_wcn6750(data->soc_type))) {
> + qca_is_wcn6750(data->soc_type) ||
> + qca_is_wcn6855(data->soc_type))) {
Perhaps you fix the alignment here too.
> qcadev->btsoc_type = data->soc_type;
> qcadev->bt_power = devm_kzalloc(&serdev->dev,
> sizeof(struct qca_power),
> @@ -2067,14 +2087,18 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>
> qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
> GPIOD_OUT_LOW);
> - if (IS_ERR_OR_NULL(qcadev->bt_en) && data->soc_type == QCA_WCN6750) {
> + if (IS_ERR_OR_NULL(qcadev->bt_en)
> + && (data->soc_type == QCA_WCN6750 ||
&& operator should go on the previous line before the line break.
> + data->soc_type == QCA_WCN6855)) {
> dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n");
> power_ctrl_enabled = false;
> }
>
> qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl",
> GPIOD_IN);
> - if (IS_ERR_OR_NULL(qcadev->sw_ctrl) && data->soc_type == QCA_WCN6750)
> + if (IS_ERR_OR_NULL(qcadev->sw_ctrl)
> + && (data->soc_type == QCA_WCN6750 ||
Same here.
> + data->soc_type == QCA_WCN6855))
> dev_warn(&serdev->dev, "failed to acquire SW_CTRL gpio\n");
>
> qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
> @@ -2150,7 +2174,8 @@ static void qca_serdev_remove(struct serdev_device *serdev)
> struct qca_power *power = qcadev->bt_power;
>
> if ((qca_is_wcn399x(qcadev->btsoc_type) ||
> - qca_is_wcn6750(qcadev->btsoc_type)) &&
> + qca_is_wcn6750(qcadev->btsoc_type) ||
> + qca_is_wcn6855(qcadev->btsoc_type)) &&
> power->vregs_on)
> qca_power_shutdown(&qcadev->serdev_hu);
> else if (qcadev->susclk)
> @@ -2335,6 +2360,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
> { .compatible = "qcom,wcn3991-bt", .data = &qca_soc_data_wcn3991},
> { .compatible = "qcom,wcn3998-bt", .data = &qca_soc_data_wcn3998},
> { .compatible = "qcom,wcn6750-bt", .data = &qca_soc_data_wcn6750},
> + { .compatible = "qcom,wcn6855-bt", .data = &qca_soc_data_wcn6855},
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match);
With power-off handling fixed, this seems to work as quite well on my
X13s with 6.3-rc1. Nice job!
Btw, apart from the frame reassembly error, I'm also seeing:
Bluetooth: Received HCI_IBS_WAKE_ACK in tx state 0
during probe.
Johan
On Wed, Feb 08, 2023 at 08:09:15PM -0600, Steev Klimaszewski wrote:
> From: Bjorn Andersson <[email protected]>
I assume you got it from Bjorn in this form, but you still need to a
sentence or two here.
> Signed-off-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Steev Klimaszewski <[email protected]>
> Reviewed-by: Brian Masney <[email protected]>
> Reviewed-by: Konrad Dybcio <[email protected]>
With a proper commit message you can add my:
Reviewed-by: Johan Hovold <[email protected]>
Johan
On Wed, Feb 08, 2023 at 08:09:16PM -0600, Steev Klimaszewski wrote:
> The Lenovo Thinkpad X13s has a WCN6855 Bluetooth controller on uart2,
> add this.
>
> Signed-off-by: Steev Klimaszewski <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
This link should not be needed.
Also, please update the patch Subject to use the following prefix:
arm64: dts: qcom: sc8280xp-x13s: ...
> ---
> Changes since v4:
> * Address Konrad's review comments.
>
> Changes since v3:
> * Add vreg_s1c
> * Add regulators and not dead code
> * Fix commit message changelog
>
> Changes since v2:
> * Remove dead code and add TODO comment
> * Make dtbs_check happy with the pin definitions
> .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 76 +++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> index f936b020a71d..ad20cfb3a830 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> @@ -24,6 +24,8 @@ / {
> aliases {
> i2c4 = &i2c4;
> i2c21 = &i2c21;
> + serial0 = &uart17;
This is an unrelated change that does not belong in this patch.
> + serial1 = &uart2;
> };
>
> wcd938x: audio-codec {
> @@ -297,6 +299,15 @@ pmc8280c-rpmh-regulators {
> qcom,pmic-id = "c";
> vdd-bob-supply = <&vreg_vph_pwr>;
>
> + vreg_s1c: smps1 {
> + regulator-name = "vreg_s1c";
> + regulator-min-microvolt = <1880000>;
> + regulator-max-microvolt = <1900000>;
> + regulator-allowed-modes = <RPMH_REGULATOR_MODE_AUTO>,
> + <RPMH_REGULATOR_MODE_RET>;
> + regulator-allow-set-load;
Don't you need to specify initial-mode as well?
> + };
> +
> vreg_l1c: ldo1 {
> regulator-name = "vreg_l1c";
> regulator-min-microvolt = <1800000>;
> @@ -712,6 +723,32 @@ &qup0 {
> status = "okay";
> };
>
> +&uart2 {
> + pinctrl-0 = <&uart2_state>;
> + pinctrl-names = "default";
> +
> + status = "okay";
> +
> + bluetooth {
> + compatible = "qcom,wcn6855-bt";
> +
> + vddio-supply = <&vreg_s10b>;
> + vddbtcxmx-supply = <&vreg_s12b>;
> + vddrfacmn-supply = <&vreg_s12b>;
> + vddrfa0p8-supply = <&vreg_s12b>;
> + vddrfa1p2-supply = <&vreg_s11b>;
> + vddrfa1p7-supply = <&vreg_s1c>;
> +
> + max-speed = <3200000>;
> +
> + enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>;
> + swctrl-gpios = <&tlmm 132 GPIO_ACTIVE_HIGH>;
> +
> + pinctrl-0 = <&bt_en>;
> + pinctrl-names = "default";
> + };
> +};
> +
> &qup1 {
> status = "okay";
> };
> @@ -720,6 +757,11 @@ &qup2 {
> status = "okay";
> };
>
> +&uart17 {
> + compatible = "qcom,geni-debug-uart";
> + status = "okay";
> +};
This bit does not belong here either. We don't have any means of
accessing the debug uart on the X13s so we should probably just leave it
disabled.
> +
> &remoteproc_adsp {
> firmware-name = "qcom/sc8280xp/LENOVO/21BX/qcadsp8280.mbn";
>
> @@ -980,6 +1022,19 @@ hastings_reg_en: hastings-reg-en-state {
> &tlmm {
> gpio-reserved-ranges = <70 2>, <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
>
> + bt_en: bt-en-state {
As you are configuring more than one pin, please rename this as:
bt_default: bt-default-state
> + hstp-sw-ctrl-pins {
> + pins = "gpio132";
> + function = "gpio";
You should define the bias configuration as well. I guess we need to
keep the default pull-down enabled.
> + };
> +
> + hstp-bt-en-pins {
> + pins = "gpio133";
> + function = "gpio";
> + drive-strength = <16>;
bias-disable?
> + };
> + };
> +
> edp_reg_en: edp-reg-en-state {
> pins = "gpio25";
> function = "gpio";
> @@ -1001,6 +1056,27 @@ i2c4_default: i2c4-default-state {
> bias-disable;
> };
>
> + uart2_state: uart2-state {
Rename this one too:
uart2_default: uart2-default-state
> + cts-pins {
> + pins = "gpio122";
This should be gpio121 (gpio122 is rts).
> + function = "qup2";
> + bias-disable;
Don't we need a pull-down on this one to avoid a floating input when the
module is powered down?
> + };
> +
> + rts-tx-pins {
Please split this in two nodes.
> + pins = "gpio122", "gpio123";
> + function = "qup2";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + rx-pins {
> + pins = "gpio124";
> + function = "qup2";
> + bias-pull-up;
> + };
> + };
> +
> i2c21_default: i2c21-default-state {
> pins = "gpio81", "gpio82";
> function = "qup21";
Johan
On Thu, Mar 9, 2023 at 11:24 AM Johan Hovold <[email protected]> wrote:
>
> On Wed, Feb 08, 2023 at 08:09:16PM -0600, Steev Klimaszewski wrote:
> > The Lenovo Thinkpad X13s has a WCN6855 Bluetooth controller on uart2,
> > add this.
> >
> > Signed-off-by: Steev Klimaszewski <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
>
> This link should not be needed.
>
> Also, please update the patch Subject to use the following prefix:
>
> arm64: dts: qcom: sc8280xp-x13s: ...
>
Yeah, that was me screwing up my patch, will make those changes for v6
> > ---
> > Changes since v4:
> > * Address Konrad's review comments.
> >
> > Changes since v3:
> > * Add vreg_s1c
> > * Add regulators and not dead code
> > * Fix commit message changelog
> >
> > Changes since v2:
> > * Remove dead code and add TODO comment
> > * Make dtbs_check happy with the pin definitions
> > .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 76 +++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > index f936b020a71d..ad20cfb3a830 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > @@ -24,6 +24,8 @@ / {
> > aliases {
> > i2c4 = &i2c4;
> > i2c21 = &i2c21;
> > + serial0 = &uart17;
>
> This is an unrelated change that does not belong in this patch.
>
> > + serial1 = &uart2;
> > };
> >
> > wcd938x: audio-codec {
> > @@ -297,6 +299,15 @@ pmc8280c-rpmh-regulators {
> > qcom,pmic-id = "c";
> > vdd-bob-supply = <&vreg_vph_pwr>;
> >
> > + vreg_s1c: smps1 {
> > + regulator-name = "vreg_s1c";
> > + regulator-min-microvolt = <1880000>;
> > + regulator-max-microvolt = <1900000>;
> > + regulator-allowed-modes = <RPMH_REGULATOR_MODE_AUTO>,
> > + <RPMH_REGULATOR_MODE_RET>;
> > + regulator-allow-set-load;
>
> Don't you need to specify initial-mode as well?
>
> > + };
> > +
> > vreg_l1c: ldo1 {
> > regulator-name = "vreg_l1c";
> > regulator-min-microvolt = <1800000>;
> > @@ -712,6 +723,32 @@ &qup0 {
> > status = "okay";
> > };
> >
> > +&uart2 {
> > + pinctrl-0 = <&uart2_state>;
> > + pinctrl-names = "default";
> > +
> > + status = "okay";
> > +
> > + bluetooth {
> > + compatible = "qcom,wcn6855-bt";
> > +
> > + vddio-supply = <&vreg_s10b>;
> > + vddbtcxmx-supply = <&vreg_s12b>;
> > + vddrfacmn-supply = <&vreg_s12b>;
> > + vddrfa0p8-supply = <&vreg_s12b>;
> > + vddrfa1p2-supply = <&vreg_s11b>;
> > + vddrfa1p7-supply = <&vreg_s1c>;
> > +
> > + max-speed = <3200000>;
> > +
> > + enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>;
> > + swctrl-gpios = <&tlmm 132 GPIO_ACTIVE_HIGH>;
> > +
> > + pinctrl-0 = <&bt_en>;
> > + pinctrl-names = "default";
> > + };
> > +};
> > +
> > &qup1 {
> > status = "okay";
> > };
> > @@ -720,6 +757,11 @@ &qup2 {
> > status = "okay";
> > };
> >
> > +&uart17 {
> > + compatible = "qcom,geni-debug-uart";
> > + status = "okay";
> > +};
>
> This bit does not belong here either. We don't have any means of
> accessing the debug uart on the X13s so we should probably just leave it
> disabled.
>
Will drop it (and the above one as well)
> > +
> > &remoteproc_adsp {
> > firmware-name = "qcom/sc8280xp/LENOVO/21BX/qcadsp8280.mbn";
> >
> > @@ -980,6 +1022,19 @@ hastings_reg_en: hastings-reg-en-state {
> > &tlmm {
> > gpio-reserved-ranges = <70 2>, <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
> >
> > + bt_en: bt-en-state {
>
> As you are configuring more than one pin, please rename this as:
>
> bt_default: bt-default-state
>
> > + hstp-sw-ctrl-pins {
> > + pins = "gpio132";
> > + function = "gpio";
>
> You should define the bias configuration as well. I guess we need to
> keep the default pull-down enabled.
>
> > + };
> > +
> > + hstp-bt-en-pins {
> > + pins = "gpio133";
> > + function = "gpio";
> > + drive-strength = <16>;
>
> bias-disable?
>
> > + };
> > + };
> > +
> > edp_reg_en: edp-reg-en-state {
> > pins = "gpio25";
> > function = "gpio";
> > @@ -1001,6 +1056,27 @@ i2c4_default: i2c4-default-state {
> > bias-disable;
> > };
> >
> > + uart2_state: uart2-state {
>
> Rename this one too:
>
> uart2_default: uart2-default-state
>
> > + cts-pins {
> > + pins = "gpio122";
>
> This should be gpio121 (gpio122 is rts).
>
You are right that it should be... however... if I actually set it to
be 121.... bluetooth doesn't actually come up/work?
> > + function = "qup2";
> > + bias-disable;
>
> Don't we need a pull-down on this one to avoid a floating input when the
> module is powered down?
>
Maybe? I don't have access to the schematics or anything so I was
going with the best guess based on what worked by poking and prodding.
Will try this.
> > + };
> > +
> > + rts-tx-pins {
>
> Please split this in two nodes.
>
> > + pins = "gpio122", "gpio123";
> > + function = "qup2";
> > + drive-strength = <2>;
> > + bias-disable;
> > + };
> > +
> > + rx-pins {
> > + pins = "gpio124";
> > + function = "qup2";
> > + bias-pull-up;
> > + };
> > + };
> > +
> > i2c21_default: i2c21-default-state {
> > pins = "gpio81", "gpio82";
> > function = "qup21";
>
> Johan
On Thu, Mar 9, 2023 at 11:08 AM Johan Hovold <[email protected]> wrote:
>
> On Wed, Feb 08, 2023 at 08:09:14PM -0600, Steev Klimaszewski wrote:
> > Added regulators,GPIOs and changes required to power on/off wcn6855.
> > Added support for firmware download for wcn6855.
> >
> > Signed-off-by: Steev Klimaszewski <[email protected]>
> > ---
> > Changes since v4:
> > * Remove unused firmware check because we don't have mbn firmware.
> > * Set qcadev->init_speed if it hasn't been set.
> >
> > Changes since v3:
> > * drop unused regulators
> >
> > Changes since v2:
> > * drop unnecessary commit info
> >
> > Changes since v1:
> > * None
> >
> > drivers/bluetooth/btqca.c | 9 ++++++-
> > drivers/bluetooth/btqca.h | 10 ++++++++
> > drivers/bluetooth/hci_qca.c | 50 ++++++++++++++++++++++++++++---------
> > 3 files changed, 56 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> > index c9064d34d830..2f9d8bd27c38 100644
> > --- a/drivers/bluetooth/btqca.c
> > +++ b/drivers/bluetooth/btqca.c
> > @@ -614,6 +614,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> > config.type = ELF_TYPE_PATCH;
> > snprintf(config.fwname, sizeof(config.fwname),
> > "qca/msbtfw%02x.mbn", rom_ver);
> > + } else if (soc_type == QCA_WCN6855) {
> > + snprintf(config.fwname, sizeof(config.fwname),
> > + "qca/hpbtfw%02x.tlv", rom_ver);
> > } else {
> > snprintf(config.fwname, sizeof(config.fwname),
> > "qca/rampatch_%08x.bin", soc_ver);
> > @@ -648,6 +651,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> > else if (soc_type == QCA_WCN6750)
> > snprintf(config.fwname, sizeof(config.fwname),
> > "qca/msnv%02x.bin", rom_ver);
> > + else if (soc_type == QCA_WCN6855)
> > + snprintf(config.fwname, sizeof(config.fwname),
> > + "qca/hpnv%02x.bin", rom_ver);
> > else
> > snprintf(config.fwname, sizeof(config.fwname),
> > "qca/nvm_%08x.bin", soc_ver);
> > @@ -672,6 +678,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> > case QCA_WCN3991:
> > case QCA_WCN3998:
> > case QCA_WCN6750:
> > + case QCA_WCN6855:
>
> Did you actually verify the microsoft extensions need this, or you are
> assuming it works as 6750?
>
It was 100% an assumption that since the 6750 does it, the 6855 does
too. I should know better than to assume since I used to work at a
device manufacturer but high hopes things have changed a bit in the
past 12 years ;)
> > hci_set_msft_opcode(hdev, 0xFD70);
> > break;
> > default:
> > @@ -685,7 +692,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> > return err;
> > }
> >
> > - if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) {
> > + if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750 || soc_type == QCA_WCN6855) {
>
> Line is now over 80 columns which is still the preferred limit.
>
> Perhaps this should now be a switch statement instead?
>
switch statement might work, I'll give it a shot here.
> > /* get fw build info */
> > err = qca_read_fw_build_info(hdev);
> > if (err < 0)
> > diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> > index 61e9a50e66ae..b884095bcd9d 100644
> > --- a/drivers/bluetooth/btqca.h
> > +++ b/drivers/bluetooth/btqca.h
> > @@ -147,6 +147,7 @@ enum qca_btsoc_type {
> > QCA_WCN3991,
> > QCA_QCA6390,
> > QCA_WCN6750,
> > + QCA_WCN6855,
> > };
> >
> > #if IS_ENABLED(CONFIG_BT_QCA)
> > @@ -168,6 +169,10 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type)
> > {
> > return soc_type == QCA_WCN6750;
> > }
> > +static inline bool qca_is_wcn6855(enum qca_btsoc_type soc_type)
> > +{
> > + return soc_type == QCA_WCN6855;
> > +}
> >
> > #else
> >
> > @@ -206,6 +211,11 @@ static inline bool qca_is_wcn6750(enum qca_btsoc_type soc_type)
> > return false;
> > }
> >
> > +static inline bool qca_is_wcn6855(enum qca_btsoc_type soc_type)
> > +{
> > + return false;
> > +}
> > +
> > static inline int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
> > {
> > return -EOPNOTSUPP;
> > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> > index 3df8c3606e93..efc1c0306b4e 100644
> > --- a/drivers/bluetooth/hci_qca.c
> > +++ b/drivers/bluetooth/hci_qca.c
> > @@ -605,8 +605,7 @@ static int qca_open(struct hci_uart *hu)
> > if (hu->serdev) {
> > qcadev = serdev_device_get_drvdata(hu->serdev);
> >
> > - if (qca_is_wcn399x(qcadev->btsoc_type) ||
> > - qca_is_wcn6750(qcadev->btsoc_type))
> > + if (!(qcadev->init_speed))
> > hu->init_speed = qcadev->init_speed;
>
> This change makes no sense.
>
> In fact, it seems the driver never sets init_speed anywhere.
>
> Either way, it should not be needed for wcn6855.
>
So, that was a request from an earlier review, but if it's not needed
for 6855, I'll just drop it, and then I don't need to do any of those
changes :D
> >
> > if (qcadev->oper_speed)
> > @@ -1317,7 +1316,8 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate)
> >
> > /* Give the controller time to process the request */
> > if (qca_is_wcn399x(qca_soc_type(hu)) ||
> > - qca_is_wcn6750(qca_soc_type(hu)))
> > + qca_is_wcn6750(qca_soc_type(hu)) ||
> > + qca_is_wcn6855(qca_soc_type(hu)))
> > usleep_range(1000, 10000);
> > else
> > msleep(300);
> > @@ -1394,7 +1394,8 @@ static unsigned int qca_get_speed(struct hci_uart *hu,
> > static int qca_check_speeds(struct hci_uart *hu)
> > {
> > if (qca_is_wcn399x(qca_soc_type(hu)) ||
> > - qca_is_wcn6750(qca_soc_type(hu))) {
> > + qca_is_wcn6750(qca_soc_type(hu)) ||
> > + qca_is_wcn6855(qca_soc_type(hu))) {
> > if (!qca_get_speed(hu, QCA_INIT_SPEED) &&
> > !qca_get_speed(hu, QCA_OPER_SPEED))
> > return -EINVAL;
> > @@ -1682,7 +1683,8 @@ static int qca_power_on(struct hci_dev *hdev)
> > return 0;
> >
> > if (qca_is_wcn399x(soc_type) ||
> > - qca_is_wcn6750(soc_type)) {
> > + qca_is_wcn6750(soc_type) ||
> > + qca_is_wcn6855(soc_type)) {
> > ret = qca_regulator_init(hu);
> > } else {
> > qcadev = serdev_device_get_drvdata(hu->serdev);
> > @@ -1723,7 +1725,8 @@ static int qca_setup(struct hci_uart *hu)
> >
> > bt_dev_info(hdev, "setting up %s",
> > qca_is_wcn399x(soc_type) ? "wcn399x" :
> > - (soc_type == QCA_WCN6750) ? "wcn6750" : "ROME/QCA6390");
> > + (soc_type == QCA_WCN6750) ? "wcn6750" :
> > + (soc_type == QCA_WCN6855) ? "wcn6855" : "ROME/QCA6390");
>
> This is hideous, but not your fault...
>
It is, and, I'm not entirely sure we need it? I mean, it's nice to
show that it's now starting to set up, but it isn't particularly
helpful for end users or making sure things are working?
> >
> > qca->memdump_state = QCA_MEMDUMP_IDLE;
> >
> > @@ -1735,7 +1738,8 @@ static int qca_setup(struct hci_uart *hu)
> > clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
> >
> > if (qca_is_wcn399x(soc_type) ||
> > - qca_is_wcn6750(soc_type)) {
> > + qca_is_wcn6750(soc_type) ||
> > + qca_is_wcn6855(soc_type)) {
> > set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
> > hci_set_aosp_capable(hdev);
> >
> > @@ -1757,7 +1761,8 @@ static int qca_setup(struct hci_uart *hu)
> > }
> >
> > if (!(qca_is_wcn399x(soc_type) ||
> > - qca_is_wcn6750(soc_type))) {
> > + qca_is_wcn6750(soc_type) ||
> > + qca_is_wcn6855(soc_type))) {
>
> Perhaps you can add a leading space while changing this so that the
> open-parenthesis alignment makes sense.
>
> > /* Get QCA version information */
> > ret = qca_read_soc_version(hdev, &ver, soc_type);
> > if (ret)
> > @@ -1883,6 +1888,20 @@ static const struct qca_device_data qca_soc_data_wcn6750 = {
> > .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> > };
> >
> > +static const struct qca_device_data qca_soc_data_wcn6855 = {
> > + .soc_type = QCA_WCN6855,
> > + .vregs = (struct qca_vreg []) {
> > + { "vddio", 5000 },
> > + { "vddbtcxmx", 126000 },
> > + { "vddrfacmn", 12500 },
> > + { "vddrfa0p8", 102000 },
> > + { "vddrfa1p7", 302000 },
> > + { "vddrfa1p2", 257000 },
>
> Hmm. More random regulator load values. I really think we should get rid
> of this but that's a separate discussion.
>
Bjorn specifically requested that he wanted me to leave them in. I'm
not married to them, and don't care one way or the other, I just
wanted working bluetooth since audio wasn't quite ready yet :)
> > + },
> > + .num_vregs = 6,
> > + .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> > +};
> > +
> > static void qca_power_shutdown(struct hci_uart *hu)
> > {
> > struct qca_serdev *qcadev;
>
> As I mentioned elsewhere, you need to update also this function so that
> wcn6855 can be powered down.
Sorry, I do have that locally, I just haven't pushed a v6 as I was
looking at Tim's v2 of the qca2066 and was wondering if I should or
shouldn't continue working on my version of the driver?
>
> > @@ -2047,7 +2066,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
> >
> > if (data &&
> > (qca_is_wcn399x(data->soc_type) ||
> > - qca_is_wcn6750(data->soc_type))) {
> > + qca_is_wcn6750(data->soc_type) ||
> > + qca_is_wcn6855(data->soc_type))) {
>
> Perhaps you fix the alignment here too.
>
> > qcadev->btsoc_type = data->soc_type;
> > qcadev->bt_power = devm_kzalloc(&serdev->dev,
> > sizeof(struct qca_power),
> > @@ -2067,14 +2087,18 @@ static int qca_serdev_probe(struct serdev_device *serdev)
> >
> > qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
> > GPIOD_OUT_LOW);
> > - if (IS_ERR_OR_NULL(qcadev->bt_en) && data->soc_type == QCA_WCN6750) {
> > + if (IS_ERR_OR_NULL(qcadev->bt_en)
> > + && (data->soc_type == QCA_WCN6750 ||
>
> && operator should go on the previous line before the line break.
>
> > + data->soc_type == QCA_WCN6855)) {
> > dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n");
> > power_ctrl_enabled = false;
> > }
> >
> > qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl",
> > GPIOD_IN);
> > - if (IS_ERR_OR_NULL(qcadev->sw_ctrl) && data->soc_type == QCA_WCN6750)
> > + if (IS_ERR_OR_NULL(qcadev->sw_ctrl)
> > + && (data->soc_type == QCA_WCN6750 ||
>
> Same here.
>
> > + data->soc_type == QCA_WCN6855))
> > dev_warn(&serdev->dev, "failed to acquire SW_CTRL gpio\n");
> >
> > qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
> > @@ -2150,7 +2174,8 @@ static void qca_serdev_remove(struct serdev_device *serdev)
> > struct qca_power *power = qcadev->bt_power;
> >
> > if ((qca_is_wcn399x(qcadev->btsoc_type) ||
> > - qca_is_wcn6750(qcadev->btsoc_type)) &&
> > + qca_is_wcn6750(qcadev->btsoc_type) ||
> > + qca_is_wcn6855(qcadev->btsoc_type)) &&
> > power->vregs_on)
> > qca_power_shutdown(&qcadev->serdev_hu);
> > else if (qcadev->susclk)
> > @@ -2335,6 +2360,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
> > { .compatible = "qcom,wcn3991-bt", .data = &qca_soc_data_wcn3991},
> > { .compatible = "qcom,wcn3998-bt", .data = &qca_soc_data_wcn3998},
> > { .compatible = "qcom,wcn6750-bt", .data = &qca_soc_data_wcn6750},
> > + { .compatible = "qcom,wcn6855-bt", .data = &qca_soc_data_wcn6855},
> > { /* sentinel */ }
> > };
> > MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match);
>
> With power-off handling fixed, this seems to work as quite well on my
> X13s with 6.3-rc1. Nice job!
>
> Btw, apart from the frame reassembly error, I'm also seeing:
>
> Bluetooth: Received HCI_IBS_WAKE_ACK in tx state 0
>
> during probe.
>
I'm still not sure where the frame reassembly error comes from, and I
don't know how to get more info to figure it out either, if anyone
happens to have any guidance for that, I would love some.
Additionally, it doesn't always happen. It seems to happen on the
first load of the module, however, running modprobe -r && modprobe in
a loop (with the powerdown properly modified so the log isn't full of
splats), it doesn't seem to occur every time. Likewise for the
WAKE_ACK.
> Johan
On Thu, Mar 09, 2023 at 02:07:58PM -0600, Steev Klimaszewski wrote:
> On Thu, Mar 9, 2023 at 11:24 AM Johan Hovold <[email protected]> wrote:
> > On Wed, Feb 08, 2023 at 08:09:16PM -0600, Steev Klimaszewski wrote:
> > > + uart2_state: uart2-state {
> >
> > Rename this one too:
> >
> > uart2_default: uart2-default-state
> >
> > > + cts-pins {
> > > + pins = "gpio122";
> >
> > This should be gpio121 (gpio122 is rts).
> >
>
> You are right that it should be... however... if I actually set it to
> be 121.... bluetooth doesn't actually come up/work?
I'm running with this fixed locally and it's working here.
Not muxing the cts-pin should break flow control (e.g. the host will
send data regardless of if the device signals that it's ready to receive
it).
> > > + function = "qup2";
> > > + bias-disable;
> >
> > Don't we need a pull-down on this one to avoid a floating input when the
> > module is powered down?
>
> Maybe? I don't have access to the schematics or anything so I was
> going with the best guess based on what worked by poking and prodding.
> Will try this.
There are no external resistors and most of the Qualcomm boards with
these Bluetooth modules appear to enable the internal pull-down on cts.
But there are also two boards that recently switched to bias-bus-hold:
3d0e375bae55 ("arm64: dts: qcom: sc7280-qcard: Configure CTS pin to bias-bus-hold for bluetooth")
to avoid leakage. Perhaps that's what we want here too.
Johan
On Thu, Mar 09, 2023 at 02:24:57PM -0600, Steev Klimaszewski wrote:
> On Thu, Mar 9, 2023 at 11:08 AM Johan Hovold <[email protected]> wrote:
> > On Wed, Feb 08, 2023 at 08:09:14PM -0600, Steev Klimaszewski wrote:
> > > @@ -672,6 +678,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> > > case QCA_WCN3991:
> > > case QCA_WCN3998:
> > > case QCA_WCN6750:
> > > + case QCA_WCN6855:
> >
> > Did you actually verify the microsoft extensions need this, or you are
> > assuming it works as 6750?
> >
> It was 100% an assumption that since the 6750 does it, the 6855 does
> too. I should know better than to assume since I used to work at a
> device manufacturer but high hopes things have changed a bit in the
> past 12 years ;)
Heh. Thanks for confirming. :)
> > > hci_set_msft_opcode(hdev, 0xFD70);
> > > break;
> > > default:
> > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> > > index 3df8c3606e93..efc1c0306b4e 100644
> > > --- a/drivers/bluetooth/hci_qca.c
> > > +++ b/drivers/bluetooth/hci_qca.c
> > > @@ -605,8 +605,7 @@ static int qca_open(struct hci_uart *hu)
> > > if (hu->serdev) {
> > > qcadev = serdev_device_get_drvdata(hu->serdev);
> > >
> > > - if (qca_is_wcn399x(qcadev->btsoc_type) ||
> > > - qca_is_wcn6750(qcadev->btsoc_type))
> > > + if (!(qcadev->init_speed))
> > > hu->init_speed = qcadev->init_speed;
> >
> > This change makes no sense.
> >
> > In fact, it seems the driver never sets init_speed anywhere.
> >
> > Either way, it should not be needed for wcn6855.
>
> So, that was a request from an earlier review, but if it's not needed
> for 6855, I'll just drop it, and then I don't need to do any of those
> changes :D
Note that with the above, init_speed is only is set if qcadev->init_speed
is *not* set, so if this was needed the test would need to be negated.
But as I mentioned above, this also looks broken as qcadev->init_speed
is never set anywhere.
You could investigate and clean up this code before or after adding
support for wcn6855, but the above really doesn't look right and would
at least need to go in a separate patch with a proper explanation.
> > >
> > > if (qcadev->oper_speed)
> > > @@ -1723,7 +1725,8 @@ static int qca_setup(struct hci_uart *hu)
> > >
> > > bt_dev_info(hdev, "setting up %s",
> > > qca_is_wcn399x(soc_type) ? "wcn399x" :
> > > - (soc_type == QCA_WCN6750) ? "wcn6750" : "ROME/QCA6390");
> > > + (soc_type == QCA_WCN6750) ? "wcn6750" :
> > > + (soc_type == QCA_WCN6855) ? "wcn6855" : "ROME/QCA6390");
> >
> > This is hideous, but not your fault...
> >
> It is, and, I'm not entirely sure we need it? I mean, it's nice to
> show that it's now starting to set up, but it isn't particularly
> helpful for end users or making sure things are working?
This is driver is already spamming the logs, while generally we should
only be logging when things go wrong. Those info messages can probably
be changed to dev_dbg(), but that's a separate issue and not something
that's needed to add support for wcn6855.
And same here, you can keep the above as is, but it seems like someone
will soon need to clean up the type handling as all these conditionals
are getting a bit unwieldy.
> > > @@ -1883,6 +1888,20 @@ static const struct qca_device_data qca_soc_data_wcn6750 = {
> > > .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> > > };
> > >
> > > +static const struct qca_device_data qca_soc_data_wcn6855 = {
> > > + .soc_type = QCA_WCN6855,
> > > + .vregs = (struct qca_vreg []) {
> > > + { "vddio", 5000 },
> > > + { "vddbtcxmx", 126000 },
> > > + { "vddrfacmn", 12500 },
> > > + { "vddrfa0p8", 102000 },
> > > + { "vddrfa1p7", 302000 },
> > > + { "vddrfa1p2", 257000 },
> >
> > Hmm. More random regulator load values. I really think we should get rid
> > of this but that's a separate discussion.
> >
> Bjorn specifically requested that he wanted me to leave them in. I'm
> not married to them, and don't care one way or the other, I just
> wanted working bluetooth since audio wasn't quite ready yet :)
You can them in for now, but we had a discussion last fall about whether
these made up numbers really belong in the kernel.
> > > + },
> > > + .num_vregs = 6,
> > > + .capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> > > +};
> > > +
> > > static void qca_power_shutdown(struct hci_uart *hu)
> > > {
> > > struct qca_serdev *qcadev;
> >
> > As I mentioned elsewhere, you need to update also this function so that
> > wcn6855 can be powered down.
>
> Sorry, I do have that locally, I just haven't pushed a v6 as I was
> looking at Tim's v2 of the qca2066 and was wondering if I should or
> shouldn't continue working on my version of the driver?
I only skimmed that patch a while ago, but that ones not strictly needed
for wcn6855, right? Things seems to work well here with just this series
applied.
> > With power-off handling fixed, this seems to work as quite well on my
> > X13s with 6.3-rc1. Nice job!
> >
> > Btw, apart from the frame reassembly error, I'm also seeing:
> >
> > Bluetooth: Received HCI_IBS_WAKE_ACK in tx state 0
> >
> > during probe.
> >
> I'm still not sure where the frame reassembly error comes from, and I
> don't know how to get more info to figure it out either, if anyone
> happens to have any guidance for that, I would love some.
> Additionally, it doesn't always happen. It seems to happen on the
> first load of the module, however, running modprobe -r && modprobe in
> a loop (with the powerdown properly modified so the log isn't full of
> splats), it doesn't seem to occur every time. Likewise for the
> WAKE_ACK.
Ok. Looks like the Chromium team tried to suppress these errors when
switching line speed by toggling rts, but the frame-assembly error I get
appears to happen before that.
Johan
Hi Johan,
<SNIP>
> > > As I mentioned elsewhere, you need to update also this function so that
> > > wcn6855 can be powered down.
> >
> > Sorry, I do have that locally, I just haven't pushed a v6 as I was
> > looking at Tim's v2 of the qca2066 and was wondering if I should or
> > shouldn't continue working on my version of the driver?
>
> I only skimmed that patch a while ago, but that ones not strictly needed
> for wcn6855, right? Things seems to work well here with just this series
> applied.
Works, but, not quite well, and with the nvm bits from Tim's patch, we
end up getting closer? I think that is the best way to put it. With
what we currently have, we end up loading hpnv21.bin for our nvm patch
file, however, we actually want (at least on my Thinkpad X13s) the
.b8c file from the Windows partition for our nvm patch; With the b8c
file symlinked to .bin with just my patch set, I am able to connect a
pair of Air Pods Gen1 to the ThinkPad and play back audio, as well as
use them for input. With the .bin file that comes from
linux-firmware, they will still connect, however, they will randomly
disconnect, as well as the audio output is all garbled. I think,
ideally, we get v6+ in, and then we can figure out what to do about
the bits that Tim's patch adds. I've tried them locally, but I'm not
confident enough in my knowledge to address the issues that are
brought up in the code review there.
> > > With power-off handling fixed, this seems to work as quite well on my
> > > X13s with 6.3-rc1. Nice job!
> > >
> > > Btw, apart from the frame reassembly error, I'm also seeing:
> > >
> > > Bluetooth: Received HCI_IBS_WAKE_ACK in tx state 0
> > >
> > > during probe.
> > >
> > I'm still not sure where the frame reassembly error comes from, and I
> > don't know how to get more info to figure it out either, if anyone
> > happens to have any guidance for that, I would love some.
> > Additionally, it doesn't always happen. It seems to happen on the
> > first load of the module, however, running modprobe -r && modprobe in
> > a loop (with the powerdown properly modified so the log isn't full of
> > splats), it doesn't seem to occur every time. Likewise for the
> > WAKE_ACK.
>
> Ok. Looks like the Chromium team tried to suppress these errors when
> switching line speed by toggling rts, but the frame-assembly error I get
> appears to happen before that.
I am still trying to figure it out here as well, but I want to get v6 out there.
> Johan
On Sun, Mar 12, 2023 at 10:18:48PM -0500, Steev Klimaszewski wrote:
> Hi Johan,
>
> <SNIP>
> > > > As I mentioned elsewhere, you need to update also this function so that
> > > > wcn6855 can be powered down.
> > >
> > > Sorry, I do have that locally, I just haven't pushed a v6 as I was
> > > looking at Tim's v2 of the qca2066 and was wondering if I should or
> > > shouldn't continue working on my version of the driver?
> >
> > I only skimmed that patch a while ago, but that ones not strictly needed
> > for wcn6855, right? Things seems to work well here with just this series
> > applied.
>
> Works, but, not quite well, and with the nvm bits from Tim's patch, we
> end up getting closer? I think that is the best way to put it. With
> what we currently have, we end up loading hpnv21.bin for our nvm patch
> file, however, we actually want (at least on my Thinkpad X13s) the
> .b8c file from the Windows partition for our nvm patch; With the b8c
> file symlinked to .bin with just my patch set, I am able to connect a
> pair of Air Pods Gen1 to the ThinkPad and play back audio, as well as
> use them for input. With the .bin file that comes from
> linux-firmware, they will still connect, however, they will randomly
> disconnect, as well as the audio output is all garbled.
Hmm. Ok, but then we need to ask Lenovo and Qualcomm to release the
firmware files we need for the X13s. Until then using your patch and
"hpnv21.bin" at least works to some extent.
I could connect to one bluetooth speaker without noticing any problems,
but I did indeed get some garbled output when connecting to another. I
have not tried the .b8c file yet though, so this could possibly be some
other incompatibility issue.
> I think,
> ideally, we get v6+ in, and then we can figure out what to do about
> the bits that Tim's patch adds. I've tried them locally, but I'm not
> confident enough in my knowledge to address the issues that are
> brought up in the code review there.
Yes, that seems reasonable. Your patch is more complete in that it adds
supports for managing power. Adding support for more fine grained
loading of "NVM configuration" files could be done on top.
> > > > With power-off handling fixed, this seems to work as quite well on my
> > > > X13s with 6.3-rc1. Nice job!
> > > >
> > > > Btw, apart from the frame reassembly error, I'm also seeing:
> > > >
> > > > Bluetooth: Received HCI_IBS_WAKE_ACK in tx state 0
> > > >
> > > > during probe.
> > > >
> > > I'm still not sure where the frame reassembly error comes from, and I
> > > don't know how to get more info to figure it out either, if anyone
> > > happens to have any guidance for that, I would love some.
> > > Additionally, it doesn't always happen. It seems to happen on the
> > > first load of the module, however, running modprobe -r && modprobe in
> > > a loop (with the powerdown properly modified so the log isn't full of
> > > splats), it doesn't seem to occur every time. Likewise for the
> > > WAKE_ACK.
> >
> > Ok. Looks like the Chromium team tried to suppress these errors when
> > switching line speed by toggling rts, but the frame-assembly error I get
> > appears to happen before that.
>
> I am still trying to figure it out here as well, but I want to get v6
> out there.
Yeah, I don't think that message during probe should be a show stopper
here.
Johan
On Tue, Mar 14, 2023 at 11:44:14AM +0100, Johan Hovold wrote:
> On Sun, Mar 12, 2023 at 10:18:48PM -0500, Steev Klimaszewski wrote:
> > Works, but, not quite well, and with the nvm bits from Tim's patch, we
> > end up getting closer? I think that is the best way to put it. With
> > what we currently have, we end up loading hpnv21.bin for our nvm patch
> > file, however, we actually want (at least on my Thinkpad X13s) the
> > .b8c file from the Windows partition for our nvm patch; With the b8c
> > file symlinked to .bin with just my patch set, I am able to connect a
> > pair of Air Pods Gen1 to the ThinkPad and play back audio, as well as
> > use them for input. With the .bin file that comes from
> > linux-firmware, they will still connect, however, they will randomly
> > disconnect, as well as the audio output is all garbled.
>
> Hmm. Ok, but then we need to ask Lenovo and Qualcomm to release the
> firmware files we need for the X13s. Until then using your patch and
> "hpnv21.bin" at least works to some extent.
>
> I could connect to one bluetooth speaker without noticing any problems,
> but I did indeed get some garbled output when connecting to another. I
> have not tried the .b8c file yet though, so this could possibly be some
> other incompatibility issue.
I just tried with the hpnv21.b8c from the (somewhat old) windows
installation on my x13s, but the bluetooth speaker that produced garbled
output with the firmware from linux-firmware still does so with the
windows file.
Johan
On Wed, 8 Feb 2023 20:09:12 -0600, Steev Klimaszewski wrote:
> First things first, I do not have access to the specs nor the schematics, so a
> lot of this was done via guess work, looking at the acpi tables, and looking at
> how a similar device (wcn6750) was added.
>
> The 5th revision addresses comments from Luiz about the Bluetooth driver, as
> well as Konrad's comments on the dts file.
>
> [...]
Applied, thanks!
[1/4] dt-bindings: net: Add WCN6855 Bluetooth
(no commit info)
[2/4] Bluetooth: hci_qca: Add support for QTI Bluetooth chip wcn6855
(no commit info)
[3/4] arm64: dts: qcom: sc8280xp: Define uart2
commit: 9db28f297526f17c6575ec0eefc93a8b1642cff7
[4/4] arm64: dts: qcom: thinkpad-x13s: Add bluetooth
(no commit info)
Best regards,
--
Bjorn Andersson <[email protected]>