Re: [PATCH] arm64: dts: qcom: msm8998: Fixup uart3 gpio config for bluetooth

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Sat 19 Oct 15:12 PDT 2019, Jeffrey Hugo wrote:

> It turns out that the wcn3990 can float the gpio lines during bootup, etc
> which will result in the uart core thinking there is incoming data.  This
> results in the bluetooth stack getting garbage.  By applying a bias to
> match what wcn3990 would drive, the issue is corrected.
> 
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  .../boot/dts/qcom/msm8998-clamshell.dtsi      | 31 +++++++++++++++++++
>  arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi     | 31 +++++++++++++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi b/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
> index ab24d415acc0..7e02cb6c8e07 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
> @@ -74,6 +74,37 @@
>  	};
>  };
>  
> +&blsp1_uart3_on {
> +	/delete-node/ config;

So it would make much more sense then to split &blsp1_uart3_on in
sections defining its pins, than the "logical" config and mux split.


Please send a patch that changes msm8998-pins.dtsi to be:

blsp_uart3_on: blsp1-uart3-on {
	ctx {
	};

	rts {
	};

	rx [
	};

	tx {
	};
};


There's no need for the "pinconf" part and for pinctrl states with a
single pin/group we can simply omit the subnode (as this is now
possible).


Apart from that, I think this looks good!

Regards,
Bjorn

> +
> +	pinconf-cts {
> +		/*
> +		 * Configure a pull-down on 47 (CTS) to match the pull
> +		 * of the Bluetooth module.
> +		 */
> +		pins = "gpio47";
> +		bias-pull-down;
> +	};
> +
> +	pinconf-rts-tx {
> +		/* We'll drive 48 (RFR) and 45 (TX), so no pull */
> +		pins = "gpio45", "gpio48";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	pinconf-rx {
> +		/*
> +		 * Configure a pull-up on 45 (RX). This is needed to
> +		 * avoid garbage data when the TX pin of the Bluetooth
> +		 * module is in tri-state (module powered off or not
> +		 * driving the signal yet).
> +		 */
> +		pins = "gpio45";
> +		bias-pull-up;
> +	};
> +};
> +
>  &dsi0 {
>  	status = "okay";
>  
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> index 1a1836ed1052..17f51af5e999 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> @@ -37,6 +37,37 @@
>  	};
>  };
>  
> +&blsp1_uart3_on {
> +	/delete-node/ config;
> +
> +	pinconf-cts {
> +		/*
> +		 * Configure a pull-down on 47 (CTS) to match the pull
> +		 * of the Bluetooth module.
> +		 */
> +		pins = "gpio47";
> +		bias-pull-down;
> +	};
> +
> +	pinconf-rts-tx {
> +		/* We'll drive 48 (RFR) and 45 (TX), so no pull */
> +		pins = "gpio45", "gpio48";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	pinconf-rx {
> +		/*
> +		 * Configure a pull-up on 45 (RX). This is needed to
> +		 * avoid garbage data when the TX pin of the Bluetooth
> +		 * module is in tri-state (module powered off or not
> +		 * driving the signal yet).
> +		 */
> +		pins = "gpio45";
> +		bias-pull-up;
> +	};
> +};
> +
>  &blsp2_uart1 {
>  	status = "okay";
>  };
> -- 
> 2.17.1
>