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

[Janaki Ramaiah Thota]

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@xxxxxxxxxxx>
> > ---
> >   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