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 6th revision addresses comments from Johan to the driver itself, which
include adding poweroff support so we no longer splat when we modprobe -r or
power off the device.
The 5th revision can be found at
https://lore.kernel.org/all/[email protected]/
The end result is that we do have a working device, but not entirely reliable.
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: sc8280xp-x13s: Add bluetooth
.../net/bluetooth/qualcomm-bluetooth.yaml | 17 ++++
.../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 80 +++++++++++++++++++
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 14 ++++
drivers/bluetooth/btqca.c | 14 +++-
drivers/bluetooth/btqca.h | 10 +++
drivers/bluetooth/hci_qca.c | 57 +++++++++----
6 files changed, 177 insertions(+), 15 deletions(-)
--
2.39.2
Add bindings for the QTI WCN6855 chipset.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Steev Klimaszewski <[email protected]>
---
Changes since v5:
* 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.2
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]>
---
Changes since v5:
* Revert Set qcadev->initspeed since 6855 doesn't use it, don't touch.
* Convert get_fw_build_info to a switch statement
* Add poweroff handling
* Fix up line alignments
* Drop from microsoft extensions check since I don't actually know if we need
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 | 14 ++++++++-
drivers/bluetooth/btqca.h | 10 +++++++
drivers/bluetooth/hci_qca.c | 57 ++++++++++++++++++++++++++++---------
3 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index c9064d34d830..fd0941fe8608 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);
@@ -685,11 +691,17 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
return err;
}
- if (soc_type == QCA_WCN3991 || soc_type == QCA_WCN6750) {
+ switch (soc_type) {
+ case QCA_WCN3991:
+ case QCA_WCN6750:
+ case QCA_WCN6855:
/* get fw build info */
err = qca_read_fw_build_info(hdev);
if (err < 0)
return err;
+ break;
+ default:
+ break;
}
bt_dev_info(hdev, "QCA setup on UART is completed");
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..38ff962662ff 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1317,7 +1317,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 +1395,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;
@@ -1428,7 +1430,8 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)
* changing the baudrate of chip and host.
*/
if (qca_is_wcn399x(soc_type) ||
- qca_is_wcn6750(soc_type))
+ qca_is_wcn6750(soc_type) ||
+ qca_is_wcn6855(soc_type))
hci_uart_set_flow_control(hu, true);
if (soc_type == QCA_WCN3990) {
@@ -1446,7 +1449,8 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)
error:
if (qca_is_wcn399x(soc_type) ||
- qca_is_wcn6750(soc_type))
+ qca_is_wcn6750(soc_type) ||
+ qca_is_wcn6855(soc_type))
hci_uart_set_flow_control(hu, false);
if (soc_type == QCA_WCN3990) {
@@ -1682,7 +1686,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 +1728,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 +1741,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 +1764,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 +1891,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;
@@ -1912,7 +1934,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
host_set_baudrate(hu, 2400);
qca_send_power_pulse(hu, false);
qca_regulator_disable(qcadev);
- } else if (soc_type == QCA_WCN6750) {
+ } else if (soc_type == QCA_WCN6750 || soc_type == QCA_WCN6855) {
gpiod_set_value_cansleep(qcadev->bt_en, 0);
msleep(100);
qca_regulator_disable(qcadev);
@@ -2047,7 +2069,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 +2090,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,8 +2177,9 @@ 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)) &&
- power->vregs_on)
+ 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)
clk_disable_unprepare(qcadev->susclk);
@@ -2335,6 +2363,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.2
From: Bjorn Andersson <[email protected]>
Add the definition for uart2 for sc8280xp devices.
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]>
Reviewed-by: Johan Hovold <[email protected]>
---
Changes since v5:
* Add sentence to git commit description.
* Add Johan's R-b
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 86e5ecd6a7b8..37ea05df66a5 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1209,6 +1209,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.2
The Lenovo Thinkpad X13s has a WCN6855 Bluetooth controller on uart2,
add this.
Signed-off-by: Steev Klimaszewski <[email protected]>
---
Changes since v5:
* Update patch subject
* Specify initial mode (via guess) for vreg_s1c
* Drop uart17 definition
* Rename bt_en to bt_default because configuring more than one pin
* Correct (maybe) bias configurations
* Correct cts gpio
* Split rts-tx into two nodes
* Drop incorrect link in the commit message
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 | 80 +++++++++++++++++++
1 file changed, 80 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 53ae75fb52ed..b3221c27903a 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,7 @@ / {
aliases {
i2c4 = &i2c4;
i2c21 = &i2c21;
+ serial1 = &uart2;
};
wcd938x: audio-codec {
@@ -431,6 +432,16 @@ regulators-1 {
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-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ 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>;
@@ -901,6 +912,32 @@ &qup0 {
status = "okay";
};
+&uart2 {
+ pinctrl-0 = <&uart2_default>;
+ 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_default>;
+ pinctrl-names = "default";
+ };
+};
+
&qup1 {
status = "okay";
};
@@ -1175,6 +1212,21 @@ hastings_reg_en: hastings-reg-en-state {
&tlmm {
gpio-reserved-ranges = <70 2>, <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
+ bt_default: bt-default-state {
+ hstp-sw-ctrl-pins {
+ pins = "gpio132";
+ function = "gpio";
+ bias-pull-down;
+ };
+
+ hstp-bt-en-pins {
+ pins = "gpio133";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ };
+ };
+
edp_reg_en: edp-reg-en-state {
pins = "gpio25";
function = "gpio";
@@ -1196,6 +1248,34 @@ i2c4_default: i2c4-default-state {
bias-disable;
};
+ uart2_default: uart2-default-state {
+ cts-pins {
+ pins = "gpio121";
+ function = "qup2";
+ bias-pull-down;
+ };
+
+ rts-pins {
+ pins = "gpio122";
+ function = "qup2";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ tx-pins {
+ pins = "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.2
On Wed, Mar 15, 2023 at 10:47:56PM -0500, 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]>
> ---
> Changes since v5:
> * Revert Set qcadev->initspeed since 6855 doesn't use it, don't touch.
> * Convert get_fw_build_info to a switch statement
> * Add poweroff handling
> * Fix up line alignments
> * Drop from microsoft extensions check since I don't actually know if we need
Thanks for the update.
Reviewed-by: Johan Hovold <[email protected]>
Tested-by: Johan Hovold <[email protected]>
Johan
Dear Steev,
Thank you for your patch. Some nits.
Am 16.03.23 um 04:47 schrieb Steev Klimaszewski:
> Added regulators,GPIOs and changes required to power on/off wcn6855.
Please add a space after the comma.
> Added support for firmware download for wcn6855.
You might want to use imperative mood (Add …).
How did you test this? What firmware files did you use?
Maybe mention, that the assumption is, that it’s identical to WCN6750?
> Signed-off-by: Steev Klimaszewski <[email protected]>
> Reviewed-by: Bjorn Andersson <[email protected]>
> Tested-by: Bjorn Andersson <[email protected]>
> ---
[…]
Kind regards,
Paul
On Wed, Mar 15, 2023 at 10:47:58PM -0500, Steev Klimaszewski wrote:
> The Lenovo Thinkpad X13s has a WCN6855 Bluetooth controller on uart2,
> add this.
>
> Signed-off-by: Steev Klimaszewski <[email protected]>
> ---
> Changes since v5:
> * Update patch subject
> * Specify initial mode (via guess) for vreg_s1c
> * Drop uart17 definition
> * Rename bt_en to bt_default because configuring more than one pin
> * Correct (maybe) bias configurations
> * Correct cts gpio
> * Split rts-tx into two nodes
> * Drop incorrect link in the commit message
>
> 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 | 80 +++++++++++++++++++
> 1 file changed, 80 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 53ae75fb52ed..b3221c27903a 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,7 @@ / {
> aliases {
> i2c4 = &i2c4;
> i2c21 = &i2c21;
> + serial1 = &uart2;
> };
>
> wcd938x: audio-codec {
> @@ -431,6 +432,16 @@ regulators-1 {
> 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-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> + regulator-allowed-modes = <RPMH_REGULATOR_MODE_AUTO>,
> + <RPMH_REGULATOR_MODE_RET>;
> + regulator-allow-set-load;
So this does not look quite right still as you're specifying an initial
mode which is not listed as allowed.
Also there are no other in-tree users of RPMH_REGULATOR_MODE_RET and
AUTO is used to switch mode automatically which seems odd to use with
allow-set-load.
This regulator is in fact also used by the wifi part of the chip and as
that driver does not set any loads so we may end up with a regulator in
retention mode while wifi is in use.
Perhaps Bjorn can enlighten us, but my guess is that this should just be
"intial-mode = AUTO" (or even HPM, but I have no idea where this came
from originally).
> + };
> +
> vreg_l1c: ldo1 {
> regulator-name = "vreg_l1c";
> regulator-min-microvolt = <1800000>;
> @@ -901,6 +912,32 @@ &qup0 {
> status = "okay";
> };
>
> +&uart2 {
> + pinctrl-0 = <&uart2_default>;
> + 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_default>;
> + pinctrl-names = "default";
> + };
> +};
> +
> &qup1 {
> status = "okay";
> };
> @@ -1175,6 +1212,21 @@ hastings_reg_en: hastings-reg-en-state {
> &tlmm {
> gpio-reserved-ranges = <70 2>, <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
>
> + bt_default: bt-default-state {
> + hstp-sw-ctrl-pins {
> + pins = "gpio132";
> + function = "gpio";
> + bias-pull-down;
> + };
> +
> + hstp-bt-en-pins {
> + pins = "gpio133";
> + function = "gpio";
> + drive-strength = <16>;
> + bias-disable;
> + };
> + };
> +
> edp_reg_en: edp-reg-en-state {
> pins = "gpio25";
> function = "gpio";
> @@ -1196,6 +1248,34 @@ i2c4_default: i2c4-default-state {
> bias-disable;
> };
>
> + uart2_default: uart2-default-state {
> + cts-pins {
> + pins = "gpio121";
> + function = "qup2";
> + bias-pull-down;
So I believe this should be 'bias-bus-hold' even if the pinctrl binding
may need to be updated to suppress the corresponding dtb check warning.
I'll send a patch for that.
> + };
> +
> + rts-pins {
> + pins = "gpio122";
> + function = "qup2";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + tx-pins {
nit: tx should go after rx for alphabetical sorting.
> + pins = "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 16, 2023 at 11:26:12AM +0100, Johan Hovold wrote:
> On Wed, Mar 15, 2023 at 10:47:58PM -0500, Steev Klimaszewski wrote:
> > The Lenovo Thinkpad X13s has a WCN6855 Bluetooth controller on uart2,
> > add this.
> >
> > Signed-off-by: Steev Klimaszewski <[email protected]>
> > ---
> > + vreg_s1c: smps1 {
> > + regulator-name = "vreg_s1c";
> > + regulator-min-microvolt = <1880000>;
> > + regulator-max-microvolt = <1900000>;
> > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > + regulator-allowed-modes = <RPMH_REGULATOR_MODE_AUTO>,
> > + <RPMH_REGULATOR_MODE_RET>;
> > + regulator-allow-set-load;
>
> So this does not look quite right still as you're specifying an initial
> mode which is not listed as allowed.
>
> Also there are no other in-tree users of RPMH_REGULATOR_MODE_RET and
> AUTO is used to switch mode automatically which seems odd to use with
> allow-set-load.
>
> This regulator is in fact also used by the wifi part of the chip and as
> that driver does not set any loads so we may end up with a regulator in
> retention mode while wifi is in use.
>
> Perhaps Bjorn can enlighten us, but my guess is that this should just be
> "intial-mode = AUTO" (or even HPM, but I have no idea where this came
> from originally).
This one probably also needs to be marked as always-on as we don't
currently describe the fact that the wifi part also uses s1c.
Johan
On Thu, Mar 16, 2023 at 4:58 AM Paul Menzel <[email protected]> wrote:
>
> Dear Steev,
>
>
> Thank you for your patch. Some nits.
>
> Am 16.03.23 um 04:47 schrieb Steev Klimaszewski:
> > Added regulators,GPIOs and changes required to power on/off wcn6855.
>
> Please add a space after the comma.
>
Good catch, sorry about that, will do in v7!
> > Added support for firmware download for wcn6855.
>
> You might want to use imperative mood (Add …).
>
Bah, I'd seen others mention this, and still did it in mine :/
> How did you test this? What firmware files did you use?
>
> Maybe mention, that the assumption is, that it’s identical to WCN6750?
>
Are you wanting the firmware used in the commit message? I think I've
seen similar in e.g. ath11k patches like:
"Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1"
something like that? The firmware itself is already in
linux-firmware. I figured it wasn't imperative to the patch and it's
in the cover letter but I can definitely throw it in the commit
message!
> > Signed-off-by: Steev Klimaszewski <[email protected]>
> > Reviewed-by: Bjorn Andersson <[email protected]>
> > Tested-by: Bjorn Andersson <[email protected]>
> > ---
>
> […]
>
>
> Kind regards,
>
> Paul
Hi Johan,
On Thu, Mar 16, 2023 at 6:05 AM Johan Hovold <[email protected]> wrote:
>
> On Thu, Mar 16, 2023 at 11:26:12AM +0100, Johan Hovold wrote:
> > On Wed, Mar 15, 2023 at 10:47:58PM -0500, Steev Klimaszewski wrote:
> > > The Lenovo Thinkpad X13s has a WCN6855 Bluetooth controller on uart2,
> > > add this.
> > >
> > > Signed-off-by: Steev Klimaszewski <[email protected]>
> > > ---
>
> > > + vreg_s1c: smps1 {
> > > + regulator-name = "vreg_s1c";
> > > + regulator-min-microvolt = <1880000>;
> > > + regulator-max-microvolt = <1900000>;
> > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > > + regulator-allowed-modes = <RPMH_REGULATOR_MODE_AUTO>,
> > > + <RPMH_REGULATOR_MODE_RET>;
> > > + regulator-allow-set-load;
> >
> > So this does not look quite right still as you're specifying an initial
> > mode which is not listed as allowed.
> >
> > Also there are no other in-tree users of RPMH_REGULATOR_MODE_RET and
> > AUTO is used to switch mode automatically which seems odd to use with
> > allow-set-load.
> >
> > This regulator is in fact also used by the wifi part of the chip and as
> > that driver does not set any loads so we may end up with a regulator in
> > retention mode while wifi is in use.
> >
> > Perhaps Bjorn can enlighten us, but my guess is that this should just be
> > "intial-mode = AUTO" (or even HPM, but I have no idea where this came
> > from originally).
>
> This one probably also needs to be marked as always-on as we don't
> currently describe the fact that the wifi part also uses s1c.
>
> Johan
I couldn't remember exactly why I chose HPM, and so I recreated what
I'd done. I looked to see what modes were available by git grepping
the kernel sources and since they are in
include/dt-bindings/regulator/qcom,rpmh-regulator.h with a comment
explaining what each mode is, I picked HPM since it starts it at the
full rated current. As to why I chose the others... it was based on
SMPS being mentioned in that comment block. Since I wasn't sure what
PFM is (and admittedly, did not look it up) I skipped it.
And you are right, we probably don't want to yank that regulator out
from under the wifi... will add that in v7, so I guess for v7 we want
HPM, LPM, AUTO with AUTO being initial. I guess I was trying to think
that RET would allow as little power usage as possible when bluetooth
isn't in use.
--steev
On Thu, Mar 16, 2023 at 02:17:38PM -0500, Steev Klimaszewski wrote:
> On Thu, Mar 16, 2023 at 6:05 AM Johan Hovold <[email protected]> wrote:
> > On Thu, Mar 16, 2023 at 11:26:12AM +0100, Johan Hovold wrote:
> > > On Wed, Mar 15, 2023 at 10:47:58PM -0500, Steev Klimaszewski wrote:
> > > > + vreg_s1c: smps1 {
> > > > + regulator-name = "vreg_s1c";
> > > > + regulator-min-microvolt = <1880000>;
> > > > + regulator-max-microvolt = <1900000>;
> > > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > > > + regulator-allowed-modes = <RPMH_REGULATOR_MODE_AUTO>,
> > > > + <RPMH_REGULATOR_MODE_RET>;
> > > > + regulator-allow-set-load;
> > >
> > > So this does not look quite right still as you're specifying an initial
> > > mode which is not listed as allowed.
> > >
> > > Also there are no other in-tree users of RPMH_REGULATOR_MODE_RET and
> > > AUTO is used to switch mode automatically which seems odd to use with
> > > allow-set-load.
> > >
> > > This regulator is in fact also used by the wifi part of the chip and as
> > > that driver does not set any loads so we may end up with a regulator in
> > > retention mode while wifi is in use.
> > >
> > > Perhaps Bjorn can enlighten us, but my guess is that this should just be
> > > "intial-mode = AUTO" (or even HPM, but I have no idea where this came
> > > from originally).
> >
> > This one probably also needs to be marked as always-on as we don't
> > currently describe the fact that the wifi part also uses s1c.
> I couldn't remember exactly why I chose HPM, and so I recreated what
> I'd done. I looked to see what modes were available by git grepping
> the kernel sources and since they are in
> include/dt-bindings/regulator/qcom,rpmh-regulator.h with a comment
> explaining what each mode is, I picked HPM since it starts it at the
> full rated current. As to why I chose the others... it was based on
> SMPS being mentioned in that comment block. Since I wasn't sure what
> PFM is (and admittedly, did not look it up) I skipped it.
>
> And you are right, we probably don't want to yank that regulator out
> from under the wifi... will add that in v7, so I guess for v7 we want
> HPM, LPM, AUTO with AUTO being initial. I guess I was trying to think
> that RET would allow as little power usage as possible when bluetooth
> isn't in use.
No, I think you need to stick with HPM and disallow setting the load
since doing so could impact other consumers that are not yet described.
Johan