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.g...@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-f345b7a53...@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
