* [PATCH v2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280
@ 2021-10-12 13:01 Balakrishna Godavarthi
2021-10-12 17:24 ` Stephen Boyd
0 siblings, 1 reply; 4+ messages in thread
From: Balakrishna Godavarthi @ 2021-10-12 13:01 UTC (permalink / raw)
To: marcel, johan.hedberg, bjorn.andersson
Cc: mka, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm,
pharish, rjliao, hbandi, saluvala, abhishekpandit,
Balakrishna Godavarthi
Add bluetooth SoC WCN6750 node for SC7280 IDP boards.
Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
---
v2:
* merged two patches into one
* Removed unused comments
* Removed pinmux & pin conf.
* Addressed reviewers comments
v1: initial patch
---
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 6 ++++++
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 25 +++++++++++++++++++++++++
arch/arm64/boot/dts/qcom/sc7280-idp2.dts | 6 ++++++
3 files changed, 37 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 64fc22a..d8b9262 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -16,7 +16,9 @@
compatible = "qcom,sc7280-idp", "google,senor", "qcom,sc7280";
aliases {
+ bluetooth0 = &bluetooth;
serial0 = &uart5;
+ serial1 = &uart7;
};
chosen {
@@ -68,3 +70,7 @@
qcom,pre-scaling = <1 1>;
};
};
+
+&bluetooth {
+ vddio-supply = <&vreg_l19b_1p8>;
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 272d5ca..09adc802 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -393,6 +393,23 @@
<&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
pinctrl-names = "default", "sleep";
pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>;
+
+ bluetooth: wcn6750-bt {
+ compatible = "qcom,wcn6750-bt";
+ pinctrl-names = "default";
+ pinctrl-0 = <&bt_en_default>;
+ enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>;
+ swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>;
+ vddaon-supply = <&vreg_s7b_0p9>;
+ vddbtcxmx-supply = <&vreg_s7b_0p9>;
+ vddrfacmn-supply = <&vreg_s7b_0p9>;
+ vddrfa0p8-supply = <&vreg_s7b_0p9>;
+ vddrfa1p7-supply = <&vreg_s1b_1p8>;
+ vddrfa1p2-supply = <&vreg_s8b_1p2>;
+ vddrfa2p2-supply = <&vreg_s1c_2p2>;
+ vddasd-supply = <&vreg_l11c_2p8>;
+ max-speed = <3200000>;
+ };
};
/* PINCTRL - additions to nodes defined in sc7280.dtsi */
@@ -504,6 +521,14 @@
*/
bias-pull-up;
};
+
+ bt_en_default: bt_en_default {
+ pins = "gpio85";
+ function = "gpio";
+ drive-strength = <2>;
+ output-low;
+ bias-pull-down;
+ };
};
&sdc1_on {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
index 1fc2add..e60fa88 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
@@ -14,10 +14,16 @@
compatible = "qcom,sc7280-idp2", "google,piglin", "qcom,sc7280";
aliases {
+ bluetooth0 = &bluetooth;
serial0 = &uart5;
+ serial1 = &uart7;
};
chosen {
stdout-path = "serial0:115200n8";
};
};
+
+&bluetooth {
+ vddio-supply = <&vreg_l18b_1p8>;
+};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280
2021-10-12 13:01 [PATCH v2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280 Balakrishna Godavarthi
@ 2021-10-12 17:24 ` Stephen Boyd
2021-10-13 5:30 ` bgodavar
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Boyd @ 2021-10-12 17:24 UTC (permalink / raw)
To: Balakrishna Godavarthi, bjorn.andersson, johan.hedberg, marcel
Cc: mka, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm,
pharish, rjliao, hbandi, saluvala, abhishekpandit
Quoting Balakrishna Godavarthi (2021-10-12 06:01:38)
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index 272d5ca..09adc802 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -393,6 +393,23 @@
> <&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
> pinctrl-names = "default", "sleep";
> pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>;
> +
> + bluetooth: wcn6750-bt {
bluetooth: bluetooth {
Node names should be generic.
> + compatible = "qcom,wcn6750-bt";
> + pinctrl-names = "default";
> + pinctrl-0 = <&bt_en_default>;
> + enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>;
> + swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>;
Is there any pinctrl config for gpio 86?
> + vddaon-supply = <&vreg_s7b_0p9>;
> + vddbtcxmx-supply = <&vreg_s7b_0p9>;
> + vddrfacmn-supply = <&vreg_s7b_0p9>;
> + vddrfa0p8-supply = <&vreg_s7b_0p9>;
> + vddrfa1p7-supply = <&vreg_s1b_1p8>;
> + vddrfa1p2-supply = <&vreg_s8b_1p2>;
> + vddrfa2p2-supply = <&vreg_s1c_2p2>;
> + vddasd-supply = <&vreg_l11c_2p8>;
> + max-speed = <3200000>;
> + };
> };
>
> /* PINCTRL - additions to nodes defined in sc7280.dtsi */
> @@ -504,6 +521,14 @@
> */
> bias-pull-up;
> };
> +
> + bt_en_default: bt_en_default {
bt_en: bt-en {
Node names shouldn't have underscores and 'default' is redundant.
> + pins = "gpio85";
> + function = "gpio";
> + drive-strength = <2>;
> + output-low;
> + bias-pull-down;
Why is there a pull down on an output gpio? Shouldn't this be
bias-disable?
> + };
> };
>
> &sdc1_on {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280
2021-10-12 17:24 ` Stephen Boyd
@ 2021-10-13 5:30 ` bgodavar
2021-10-13 20:14 ` Stephen Boyd
0 siblings, 1 reply; 4+ messages in thread
From: bgodavar @ 2021-10-13 5:30 UTC (permalink / raw)
To: Stephen Boyd
Cc: bjorn.andersson, johan.hedberg, marcel, mka, linux-kernel,
linux-bluetooth, hemantg, linux-arm-msm, pharish, rjliao, hbandi,
saluvala, abhishekpandit
Hi Stephen Boyd,
On 2021-10-12 22:54, Stephen Boyd wrote:
> Quoting Balakrishna Godavarthi (2021-10-12 06:01:38)
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index 272d5ca..09adc802 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -393,6 +393,23 @@
>> <&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
>> pinctrl-names = "default", "sleep";
>> pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>,
>> <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>;
>> +
>> + bluetooth: wcn6750-bt {
>
> bluetooth: bluetooth {
>
> Node names should be generic.
>
[Bala]: will update in next patch.
>> + compatible = "qcom,wcn6750-bt";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&bt_en_default>;
>> + enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>;
>> + swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>;
>
> Is there any pinctrl config for gpio 86?
>
[Bala]: This is input GPIO to apps, BT SOC will handle configurations.
>> + vddaon-supply = <&vreg_s7b_0p9>;
>> + vddbtcxmx-supply = <&vreg_s7b_0p9>;
>> + vddrfacmn-supply = <&vreg_s7b_0p9>;
>> + vddrfa0p8-supply = <&vreg_s7b_0p9>;
>> + vddrfa1p7-supply = <&vreg_s1b_1p8>;
>> + vddrfa1p2-supply = <&vreg_s8b_1p2>;
>> + vddrfa2p2-supply = <&vreg_s1c_2p2>;
>> + vddasd-supply = <&vreg_l11c_2p8>;
>> + max-speed = <3200000>;
>> + };
>> };
>>
>> /* PINCTRL - additions to nodes defined in sc7280.dtsi */
>> @@ -504,6 +521,14 @@
>> */
>> bias-pull-up;
>> };
>> +
>> + bt_en_default: bt_en_default {
>
> bt_en: bt-en {
>
> Node names shouldn't have underscores and 'default' is redundant.
>
[Bala]: will update in next patch.
>> + pins = "gpio85";
>> + function = "gpio";
>> + drive-strength = <2>;
>> + output-low;
>> + bias-pull-down;
>
> Why is there a pull down on an output gpio? Shouldn't this be
> bias-disable?
>
[Bala]: BT_EN pin is OP of apps and input to BT SoC.
by default we want the state of BT_EN to be low. so used pull down
instead of bias-disable
as AFAIK bias-disable may trigger a tristate on BT_EN pin, which may
trigger BT SoC enable
if it is not actually triggered.
>> + };
>> };
>>
>> &sdc1_on {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280
2021-10-13 5:30 ` bgodavar
@ 2021-10-13 20:14 ` Stephen Boyd
0 siblings, 0 replies; 4+ messages in thread
From: Stephen Boyd @ 2021-10-13 20:14 UTC (permalink / raw)
To: bgodavar
Cc: bjorn.andersson, johan.hedberg, marcel, mka, linux-kernel,
linux-bluetooth, hemantg, linux-arm-msm, pharish, rjliao, hbandi,
saluvala, abhishekpandit
Quoting bgodavar@codeaurora.org (2021-10-12 22:30:50)
> On 2021-10-12 22:54, Stephen Boyd wrote:
> > Quoting Balakrishna Godavarthi (2021-10-12 06:01:38)
> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> >> b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> >> index 272d5ca..09adc802 100644
> >> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> >> + compatible = "qcom,wcn6750-bt";
> >> + pinctrl-names = "default";
> >> + pinctrl-0 = <&bt_en_default>;
> >> + enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>;
> >> + swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>;
> >
> > Is there any pinctrl config for gpio 86?
> >
> [Bala]: This is input GPIO to apps, BT SOC will handle configurations.
Ok. So there should be some pinctrl stating that the function is "gpio"
and the biasing is probably bias-disable. The BT SOC will handle setting
any pull, either up or down?
>
> >> + vddaon-supply = <&vreg_s7b_0p9>;
> >> + vddbtcxmx-supply = <&vreg_s7b_0p9>;
> >> + vddrfacmn-supply = <&vreg_s7b_0p9>;
> >> + vddrfa0p8-supply = <&vreg_s7b_0p9>;
> >> + vddrfa1p7-supply = <&vreg_s1b_1p8>;
> >> + vddrfa1p2-supply = <&vreg_s8b_1p2>;
> >> + vddrfa2p2-supply = <&vreg_s1c_2p2>;
> >> + vddasd-supply = <&vreg_l11c_2p8>;
> >> + max-speed = <3200000>;
> >> + };
> >> };
> >>
> >> /* PINCTRL - additions to nodes defined in sc7280.dtsi */
> >> @@ -504,6 +521,14 @@
> >> */
> >> bias-pull-up;
> >> };
> >> +
> >> + bt_en_default: bt_en_default {
> >
> > bt_en: bt-en {
> >
> > Node names shouldn't have underscores and 'default' is redundant.
> >
> [Bala]: will update in next patch.
>
> >> + pins = "gpio85";
> >> + function = "gpio";
> >> + drive-strength = <2>;
> >> + output-low;
> >> + bias-pull-down;
> >
> > Why is there a pull down on an output gpio? Shouldn't this be
> > bias-disable?
> >
>
> [Bala]: BT_EN pin is OP of apps and input to BT SoC.
> by default we want the state of BT_EN to be low. so used pull down
> instead of bias-disable
The pin state will be low because of the 'output-low' property.
> as AFAIK bias-disable may trigger a tristate on BT_EN pin, which may
> trigger BT SoC enable
> if it is not actually triggered.
Is the pin ever "turned around" and made an input? If not then it will
be output and be driving low until the enable-gpios is set to output
active. The pull down is probably wasting power when the pin is being
driven either high or low.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-13 20:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12 13:01 [PATCH v2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280 Balakrishna Godavarthi
2021-10-12 17:24 ` Stephen Boyd
2021-10-13 5:30 ` bgodavar
2021-10-13 20:14 ` Stephen Boyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).