Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Message-ID: <d8cd02e7-7e85-4b75-b48c-e9544d03c197@linaro.org>
Date:   Sat, 25 Nov 2023 15:04:30 +0000
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/2] arm64: dts: qcom: qrb4210-rb2: Enable bluetooth
Content-Language: en-US
To:     Konrad Dybcio <konrad.dybcio@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>
Cc:     Marijn Suijten <marijn.suijten@somainline.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20231120-topic-rb2_bt-v1-0-509db2e28e70@linaro.org>
 <20231120-topic-rb2_bt-v1-2-509db2e28e70@linaro.org>
From:   Bryan O'Donoghue <bryan.odonoghue@linaro.org>
In-Reply-To: <20231120-topic-rb2_bt-v1-2-509db2e28e70@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 25/11/2023 13:04, Konrad Dybcio wrote:
> Enable the QCA bluetooth on RB2. It identifies like the following:
> 
> Bluetooth: hci0: QCA Product ID   :0x0000000a
> Bluetooth: hci0: QCA SOC Version  :0x40020150
> Bluetooth: hci0: QCA ROM Version  :0x00000201
> Bluetooth: hci0: QCA Patch Version:0x00000001
> Bluetooth: hci0: QCA controller version 0x01500201
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 81 +++++++++++++++++++++++++++++++-
>   1 file changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts
> index 9738c0dacd58..bd751236f983 100644
> --- a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts
> +++ b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts
> @@ -15,6 +15,7 @@ / {
>   
>   	aliases {
>   		serial0 = &uart4;
> +		serial1 = &uart3;
>   	};
>   
>   	chosen {
> @@ -352,7 +353,8 @@ vreg_l8a_0p664: l8 {
>   
>   		vreg_l9a_1p8: l9 {
>   			regulator-min-microvolt = <1800000>;
> -			regulator-max-microvolt = <2000000>;
> +			regulator-max-microvolt = <1800000>;
> +			regulator-allow-set-load;
>   		};
>   
>   		vreg_l10a_1p8: l10 {
> @@ -389,11 +391,13 @@ vreg_l15a_3p128: l15 {
>   		vreg_l16a_1p3: l16 {
>   			regulator-min-microvolt = <1704000>;
>   			regulator-max-microvolt = <1904000>;
> +			regulator-allow-set-load;
>   		};
>   
>   		vreg_l17a_1p3: l17 {
>   			regulator-min-microvolt = <1152000>;
>   			regulator-max-microvolt = <1384000>;
> +			regulator-allow-set-load;
>   		};
>   
>   		vreg_l18a_1p232: l18 {
> @@ -426,6 +430,7 @@ vreg_l22a_2p96: l22 {
>   		vreg_l23a_3p3: l23 {
>   			regulator-min-microvolt = <3200000>;
>   			regulator-max-microvolt = <3400000>;
> +			regulator-allow-set-load;
>   		};
>   
>   		vreg_l24a_2p96: l24 {
> @@ -487,6 +492,60 @@ &tlmm {
>   			       <56 3>, <61 2>, <64 1>,
>   			       <68 1>, <72 8>, <96 1>;
>   
> +	uart3_default: uart3-default-state {
> +		cts-pins {
> +			pins = "gpio8";
> +			function = "qup3";
> +			bias-bus-hold;
> +		};
> +
> +		rts-pins {
> +			pins = "gpio9";
> +			function = "qup3";
> +			drive-strength = <2>;
> +			bias-disable;
> +		};
> +
> +		tx-pins {
> +			pins = "gpio10";
> +			function = "qup3";
> +			drive-strength = <2>;
> +			bias-disable;
> +		};
> +
> +		rx-pins {
> +			pins = "gpio11";
> +			function = "qup3";
> +			bias-pull-up;
> +		};
> +	};
> +
> +	uart3_sleep: uart3-sleep-state {
> +		cts-pins {
> +			pins = "gpio8";
> +			function = "gpio";
> +			bias-bus-hold;
> +		};
> +
> +		rts-pins {
> +			pins = "gpio9";
> +			function = "gpio";
> +			bias-pull-down;
> +		};
> +
> +		tx-pins {
> +			pins = "gpio10";
> +			function = "gpio";
> +			bias-pull-up;
> +		};
> +
> +		rx-pins {
> +			pins = "gpio11";
> +			function = "gpio";
> +			bias-pull-up;
> +		};
> +	};
> +

My reading of downstream has all of these pins drive-strength = <2>.


>   	lt9611_rst_pin: lt9611-rst-state {
>   		pins = "gpio41";
>   		function = "gpio";
> @@ -508,6 +567,26 @@ sdc2_card_det_n: sd-card-det-n-state {
>   	};
>   };
>   
> +&uart3 {
> +	interrupts-extended = <&intc GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>,
> +			      <&tlmm 11 IRQ_TYPE_LEVEL_HIGH>;
> +	pinctrl-0 = <&uart3_default>;
> +	pinctrl-1 = <&uart3_sleep>;
> +	pinctrl-names = "default", "sleep";
> +	status = "okay";
> +
> +	bluetooth {
> +		compatible = "qcom,wcn3988-bt";
> +
> +		vddio-supply = <&vreg_l9a_1p8>;
> +		vddxo-supply = <&vreg_l16a_1p3>;
> +		vddrf-supply = <&vreg_l17a_1p3>;
> +		vddch0-supply = <&vreg_l23a_3p3>;
> +		enable-gpios = <&tlmm 87 GPIO_ACTIVE_HIGH>;
> +		max-speed = <3200000>;
> +	};
> +};

Does firmware name not matter here ?

bluetooth {
     compatible = "qcom,wcn3990-bt";
     vddio-supply = <&vreg_s4a_1p8>;
     vddxo-supply = <&vreg_l7a_1p8>;
     vddrf-supply = <&vreg_l17a_1p3>;
     vddch0-supply = <&vreg_l25a_3p3>;
     max-speed = <3200000>;
     firmware-name = "crnv21.bin";
};

> +
>   &uart4 {
>   	status = "okay";
>   };

---
bod