| Message ID | 1634043698-20256-1-git-send-email-bgodavar@codeaurora.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280 | expand |
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 {
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 {
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.
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>; +};
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(+)