The Lenovo Thinkpad X13s has a WCN6855 Bluetooth controller on uart2,
add this.
Signed-off-by: Steev Klimaszewski <steev@kali.org>
---
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.2On 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 <steev@kali.org>
> ---
> 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 <steev@kali.org>
> > ---
> > + 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
Hi Johan,
On Thu, Mar 16, 2023 at 6:05 AM Johan Hovold <johan@kernel.org> 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 <steev@kali.org>
> > > ---
>
> > > + 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 <johan@kernel.org> 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
© 2016 - 2026 Red Hat, Inc.