On Mon, May 12, 2025 at 12:31:58PM +0530, Sumit Garg wrote:
> 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.
>
Can try again with the following diff added in addition to the
bias-pull-up changes from below? Pretty sure this is needed to apply the
pinctrl before the UART driver is initialized in the pre-reloc stage.
We're resetting the UART controller during initialization, so I don't
think whatever happens before U-Boot should make any difference.
Thanks,
Stephan
diff --git a/drivers/pinctrl/qcom/pinctrl-qcm2290.c
b/drivers/pinctrl/qcom/pinctrl-qcm2290.c
index 84f76b63b93..dcd499d99ae 100644
--- a/drivers/pinctrl/qcom/pinctrl-qcm2290.c
+++ b/drivers/pinctrl/qcom/pinctrl-qcm2290.c
@@ -67,4 +67,5 @@ U_BOOT_DRIVER(pinctrl_qcm2290) = {
.of_match = msm_pinctrl_ids,
.ops = &msm_pinctrl_ops,
.bind = msm_pinctrl_bind,
+ .flags = DM_FLAG_PRE_RELOC,
};
> [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
>
