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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 20 Oct 2019 19:11:23 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Jeffrey Hugo <jeffrey.l.hugo@...il.com>
Cc:     agross@...nel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: dts: qcom: msm8998: Fixup uart3 gpio config for
 bluetooth

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@...il.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
>