[PATCH v1 1/2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280 IDP board

[PATCH v1 1/2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280 IDP board

[Balakrishna Godavarthi]

Add bluetooth SoC WCN6750 node for SC7280 IDP board.

Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280-idp.dts  |  2 ++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 64fc22a..d3f5393 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -17,6 +17,8 @@
 
 	aliases {
 		serial0 = &uart5;
+		bluetooth0 = &bluetooth;
+		hsuart0 = &uart7;
 	};
 
 	chosen {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 272d5ca..05aa729 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -393,6 +393,24 @@
 				<&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>; /* BT_EN */
+		swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>; /* SW_CTRL */
+		vddio-supply = <&vreg_l19b_1p8>;
+		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 +522,19 @@
 		 */
 		bias-pull-up;
 	};
+
+	bt_en_default: bt_en_default {
+		pinmux {
+			pins = "gpio85";
+			function = "gpio";
+		};
+		pinconf {
+			pins = "gpio85";
+			drive-strength = <2>;
+			output-low;
+			bias-pull-down;
+		};
+	};
 };
 
 &sdc1_on {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 2/2] arm64: dts: qcom: sc7280: update bluetooth node in SC7280 IDP2 board

[Balakrishna Godavarthi]

This patch updates bluetooth node in SC7280 IDP2 board.

Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280-idp2.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
index 1fc2add..5c8d54b 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
@@ -15,9 +15,15 @@
 
 	aliases {
 		serial0 = &uart5;
+		bluetooth0 = &bluetooth;
+		hsuart0 = &uart7;
 	};
 
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
 };
+
+&bluetooth: wcn6750-bt {
+	vddio-supply = <&vreg_l18b_1p8>;
+};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v1 2/2] arm64: dts: qcom: sc7280: update bluetooth node in SC7280 IDP2 board

[Dmitry Baryshkov]

On 06/10/2021 15:30, Balakrishna Godavarthi wrote:
> This patch updates bluetooth node in SC7280 IDP2 board.
> 
> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
> ---
>   arch/arm64/boot/dts/qcom/sc7280-idp2.dts | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
> index 1fc2add..5c8d54b 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
> @@ -15,9 +15,15 @@
>   
>   	aliases {
>   		serial0 = &uart5;
> +		bluetooth0 = &bluetooth;
> +		hsuart0 = &uart7;

hsuartX does not follow existing aliases definition, so it was frowned 
upon by Rob Herring in the past.

>   	};
>   
>   	chosen {
>   		stdout-path = "serial0:115200n8";
>   	};
>   };
> +
> +&bluetooth: wcn6750-bt {
> +	vddio-supply = <&vreg_l18b_1p8>;
> +};
> 


-- 
With best wishes
Dmitry

Re: [PATCH v1 2/2] arm64: dts: qcom: sc7280: update bluetooth node in SC7280 IDP2 board

[bgodavar]

Hi Dmitry,

On 2021-10-06 22:18, Dmitry Baryshkov wrote:
> On 06/10/2021 15:30, Balakrishna Godavarthi wrote:
>> This patch updates bluetooth node in SC7280 IDP2 board.
>> 
>> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7280-idp2.dts | 6 ++++++
>>   1 file changed, 6 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts 
>> b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
>> index 1fc2add..5c8d54b 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
>> @@ -15,9 +15,15 @@
>>     	aliases {
>>   		serial0 = &uart5;
>> +		bluetooth0 = &bluetooth;
>> +		hsuart0 = &uart7;
> 
> hsuartX does not follow existing aliases definition, so it was frowned
> upon by Rob Herring in the past.
> 
[Bala]: Thanks for pointing out will do in similar way.

>>   	};
>>     	chosen {
>>   		stdout-path = "serial0:115200n8";
>>   	};
>>   };
>> +
>> +&bluetooth: wcn6750-bt {
>> +	vddio-supply = <&vreg_l18b_1p8>;
>> +};
>> 

Re: [PATCH v1 2/2] arm64: dts: qcom: sc7280: update bluetooth node in SC7280 IDP2 board

[Matthias Kaehlcke]

On Wed, Oct 06, 2021 at 06:00:03PM +0530, Balakrishna Godavarthi wrote:
> Subject: arm64: dts: qcom: sc7280: update bluetooth node in SC7280 IDP2 board

Not super helpful, what does 'update' mean?

It might be easier to have a single patch for both IDP boards, since
the Bluetooth node is added in the common sc7280-idp.dtsi board,
rather than explaining what this patch does :)

> This patch updates bluetooth node in SC7280 IDP2 board.
> 
> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-idp2.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
> index 1fc2add..5c8d54b 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
> @@ -15,9 +15,15 @@
>  
>  	aliases {
>  		serial0 = &uart5;
> +		bluetooth0 = &bluetooth;
> +		hsuart0 = &uart7;
>  	};

Sort aliases alphabetically

>  
>  	chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
>  };
> +
> +&bluetooth: wcn6750-bt {

&bluetooth {

> +	vddio-supply = <&vreg_l18b_1p8>;

nit: if it's not really common across IDP boards or a default, you could
leave it unconfigured in sc7280-idp.dtsi, and set in both board files.
Just an idea, with only two boards it doesn't really matter too much.

Re: [PATCH v1 2/2] arm64: dts: qcom: sc7280: update bluetooth node in SC7280 IDP2 board

[bgodavar]

Hi Matthias,

On 2021-10-06 23:09, Matthias Kaehlcke wrote:
> On Wed, Oct 06, 2021 at 06:00:03PM +0530, Balakrishna Godavarthi wrote:
>> Subject: arm64: dts: qcom: sc7280: update bluetooth node in SC7280 
>> IDP2 board
> 
> Not super helpful, what does 'update' mean?
> 
> It might be easier to have a single patch for both IDP boards, since
> the Bluetooth node is added in the common sc7280-idp.dtsi board,
> rather than explaining what this patch does :)
[Bala]: Sure will have one patch.

> 
>> This patch updates bluetooth node in SC7280 IDP2 board.
>> 
>> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
>> ---
>>  arch/arm64/boot/dts/qcom/sc7280-idp2.dts | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts 
>> b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
>> index 1fc2add..5c8d54b 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
>> @@ -15,9 +15,15 @@
>> 
>>  	aliases {
>>  		serial0 = &uart5;
>> +		bluetooth0 = &bluetooth;
>> +		hsuart0 = &uart7;
>>  	};
> 
> Sort aliases alphabetically
> 
>> 
>>  	chosen {
>>  		stdout-path = "serial0:115200n8";
>>  	};
>>  };
>> +
>> +&bluetooth: wcn6750-bt {
> 
> &bluetooth {
> 
>> +	vddio-supply = <&vreg_l18b_1p8>;
> 
> nit: if it's not really common across IDP boards or a default, you 
> could
> leave it unconfigured in sc7280-idp.dtsi, and set in both board files.
> Just an idea, with only two boards it doesn't really matter too much.

[Bala]: Sure will update in similar way.

Re: [PATCH v1 1/2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280 IDP board

[Matthias Kaehlcke]

On Wed, Oct 06, 2021 at 06:00:02PM +0530, Balakrishna Godavarthi wrote:
> Add bluetooth SoC WCN6750 node for SC7280 IDP board.
> 
> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-idp.dts  |  2 ++
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> index 64fc22a..d3f5393 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> @@ -17,6 +17,8 @@
>  
>  	aliases {
>  		serial0 = &uart5;
> +		bluetooth0 = &bluetooth;
> +		hsuart0 = &uart7;

Sort aliases alphabetically.

Also 'hsuart' should not be used, as Dmitry already pointed out on
patch 2/2. I suppose it should be 'serial1', as in 'second serial
port of the board'.

>  	};
>  
>  	chosen {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index 272d5ca..05aa729 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -393,6 +393,24 @@
>  				<&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>;

Do we also need a pinctrl entry for 'swctrl' ?

> +		enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>; /* BT_EN */
> +		swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>; /* SW_CTRL */

The comments aren't useful, the property names say the same.

> +		vddio-supply = <&vreg_l19b_1p8>;
> +		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 +522,19 @@
>  		 */
>  		bias-pull-up;
>  	};
> +
> +	bt_en_default: bt_en_default {

	bt_en: bt-en {

> +		pinmux {
> +			pins = "gpio85";
> +			function = "gpio";
> +		};
> +		pinconf {
> +			pins = "gpio85";
> +			drive-strength = <2>;
> +			output-low;
> +			bias-pull-down;
> +		};

No pinmux & pinconf nodes, see configuration for other pins.

Re: [PATCH v1 1/2] arm64: dts: qcom: sc7280: Add bluetooth node on SC7280 IDP board

[bgodavar]

Hi Matthias,

On 2021-10-06 22:56, Matthias Kaehlcke wrote:
> On Wed, Oct 06, 2021 at 06:00:02PM +0530, Balakrishna Godavarthi wrote:
>> Add bluetooth SoC WCN6750 node for SC7280 IDP board.
>> 
>> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
>> ---
>>  arch/arm64/boot/dts/qcom/sc7280-idp.dts  |  2 ++
>>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 31 
>> +++++++++++++++++++++++++++++++
>>  2 files changed, 33 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts 
>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> index 64fc22a..d3f5393 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> @@ -17,6 +17,8 @@
>> 
>>  	aliases {
>>  		serial0 = &uart5;
>> +		bluetooth0 = &bluetooth;
>> +		hsuart0 = &uart7;
> 
> Sort aliases alphabetically.
> 
> Also 'hsuart' should not be used, as Dmitry already pointed out on
> patch 2/2. I suppose it should be 'serial1', as in 'second serial
> port of the board'.
> 
[Bala]: will update it.
>>  	};
>> 
>>  	chosen {
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index 272d5ca..05aa729 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -393,6 +393,24 @@
>>  				<&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>;
> 
> Do we also need a pinctrl entry for 'swctrl' ?
[Bala]: It is in input to APPS and op of BT SoC.
I don't think to set any configuration as BT SOC will take care of it.

> 
>> +		enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>; /* BT_EN */
>> +		swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>; /* SW_CTRL */
> 
> The comments aren't useful, the property names say the same.
> 
[Bala]: will remove them

>> +		vddio-supply = <&vreg_l19b_1p8>;
>> +		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 +522,19 @@
>>  		 */
>>  		bias-pull-up;
>>  	};
>> +
>> +	bt_en_default: bt_en_default {
> 
> 	bt_en: bt-en {
> 
>> +		pinmux {
>> +			pins = "gpio85";
>> +			function = "gpio";
>> +		};
>> +		pinconf {
>> +			pins = "gpio85";
>> +			drive-strength = <2>;
>> +			output-low;
>> +			bias-pull-down;
>> +		};
> 
> No pinmux & pinconf nodes, see configuration for other pins.
[Bala]: Thanks for pointing will update in next version