Re: [PATCH v3] arm64: dts: qcom: qcs6490-rb3gen2: enable Bluetooth

[Janaki Ramaiah Thota <quic_janathot@quicinc.com>]

Hi Krzysztof,
Thanks for the valuable review comments.

On 10/22/2024 5:57 PM, Krzysztof Kozlowski wrote:
> On 22/10/2024 12:46, Janaki Ramaiah Thota wrote:
>> Add Bluetooth and UART7 support for qcs6490-rb3gen2.
>>
>> Signed-off-by: Janaki Ramaiah Thota <quic_janathot@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 122 ++++++++++++++++++-
>>   1 file changed, 121 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> index 0d45662b8028..b774d89172ea 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: BSD-3-Clause
>>   /*
>> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>   
>>   /dts-v1/;
>> @@ -31,7 +31,9 @@
>>   	chassis-type = "embedded";
>>   
>>   	aliases {
>> +		bluetooth0 = &bluetooth;
> 
> Which driver uses this alias?
> 
>>   		serial0 = &uart5;
>> +		serial1 = &uart7;
>>   	};
> 
> ...
> 
> 
>>   };
>>   
>>   &uart5 {
>>   	status = "okay";
>>   };
>>   
>> +&uart7 {
>> +	status = "okay";
>> +	/delete-property/interrupts;
>> +	interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>,
>> +				<&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>;
> 
> This could be just one phandle to state node with multiple pins subnode.
> 

Since we have different configuration for 3 GPIOs, it’s not good to 
combine all 4 GPIOs into one common sleep configuration. Each GPIO 
configuration is having separate requirements based on the Bluetooth SOC.
This is as per hardware recommendations.


>> +
>> +	bluetooth: bluetooth {
>> +		compatible = "qcom,wcn6750-bt";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&bt_en>, <&sw_ctrl>;
>> +		enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>;
>> +		swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>;
>> +		vddaon-supply = <&vreg_s7b_0p972>;
>> +		vddbtcxmx-supply = <&vreg_s7b_0p972>;
>> +		vddrfacmn-supply = <&vreg_s7b_0p972>;
>> +		vddrfa0p8-supply = <&vreg_s7b_0p972>;
>> +		vddrfa1p7-supply = <&vreg_s1b_1p872>;
>> +		vddrfa1p2-supply = <&vreg_s8b_1p272>;
>> +		vddrfa2p2-supply = <&vreg_s1c_2p19>;
>> +		vddasd-supply = <&vreg_l11c_2p8>;
>> +		max-speed = <3200000>;
> 
> There were warnings in the past. Please confirm that you addressed them
> and this is warning free.
> 

Sure.

> Best regards,
> Krzysztof
> 

Thanks,
Janakiram