Re: [PATCH] qcom_defconfig: Enable AUTOBOOT_KEYED support

[Sumit Garg <sumit.garg@kernel.org>]

On Fri, May 09, 2025 at 04:54:43PM +0200, Stephan Gerhold wrote:
> On Fri, May 09, 2025 at 12:45:20PM +0200, Casey Connolly wrote:
> > On 5/8/25 12:32, Sumit Garg wrote:
> > > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > 
> > > When debug serial port isn't connected, it is at least reported on RB1
> > > that autoboot gets interrupted. It is probably due to random characters
> > > on the UART RX line when disconnected. Lets try to fix this inconsistent
> > > behaviour via enabling AUTOBOOT_KEYED such that the autoboot gets
> > > interrupted only when the <SPACE> key gets pressed.
> > 
> > Honestly I'd much rather fix the bug in the UART driver, maybe it's broken
> > pinctrl?
> > 
> > The older UART driver (for msm8916) had a similar but that Stephen was able
> > to fix with better configuration.
> > 
> > Anyhow, this is a sensible enough workaround if nobody wants to look into
> > GENI...
> > 
> 
> Well, the main fix I did for that isn't related to the UART driver at
> all, it's simply adding bias-pull-up for the RX pin so that it isn't
> left floating when UART is not connected.
> 
> For RB1, qcm2290.dtsi &qup_uart4_default currently has bias-disable for
> both RX and TX pins. So please try applying bias-pull-up + bootph-all
> for the GPIO that belongs to RX.
> 
> For details, please see the following patch series for MSM8916:
> https://lore.kernel.org/linux-arm-msm/20250422-msm8916-console-pinctrl-v2-0-f345b7a53c91@linaro.org/T/

Thanks for this reference but unfortunately this doesn't fix the issue
on RB1. I have tried the diff below [1]. It seems most likely due the
chainloading via ABL on RB1 as compared to U-Boot running as primary
bootloader on db410c. The RX line seems to be floating even before the
U-Boot starts which is causing the autoboot to stop. So for the
chainloading configuration, we need to enable AUTOBOOT_KEYED. I think
it should be the default sane configuration on Qcom platforms rather
than allowing autoboot stopped via any random character on RX line.

[1]

diff --git a/dts/upstream/src/arm64/qcom/qcm2290.dtsi b/dts/upstream/src/arm64/qcom/qcm2290.dtsi
index f0746123e59..9392bd6f3e0 100644
--- a/dts/upstream/src/arm64/qcom/qcm2290.dtsi
+++ b/dts/upstream/src/arm64/qcom/qcm2290.dtsi
@@ -551,10 +551,28 @@
                        };

                        qup_uart4_default: qup-uart4-default-state {
+                               tx-pins {
+                                       pins = "gpio12";
+                                       function = "qup4";
+                                       drive-strength = <16>;
+                                       bias-disable;
+                                       bootph-all;
+                               };
+
+                               rx-pins {
+                                       pins = "gpio13";
+                                       function = "qup4";
+                                       drive-strength = <16>;
+                                       bias-pull-up;
+                                       bootph-all;
+                               };
+                       };
+
+                       qup_uart4_sleep: qup-uart4-sleep-state {
                                pins = "gpio12", "gpio13";
-                               function = "qup4";
+                               function = "gpio";
                                drive-strength = <2>;
-                               bias-disable;
+                               bias-pull-down;
                        };

                        sdc1_state_on: sdc1-on-state {
@@ -1293,7 +1311,8 @@
                                clocks = <&gcc GCC_QUPV3_WRAP0_S4_CLK>;
                                clock-names = "se";
                                pinctrl-0 = <&qup_uart4_default>;
-                               pinctrl-names = "default";
+                               pinctrl-1 = <&qup_uart4_sleep>;
+                               pinctrl-names = "default", "sleep";
                                interconnects = <&qup_virt MASTER_QUP_CORE_0 RPM_ALWAYS_TAG
                                                 &qup_virt SLAVE_QUP_CORE_0 RPM_ALWAYS_TAG>,
                                                <&bimc MASTER_APPSS_PROC RPM_ALWAYS_TAG


-Sumit