All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c
@ 2025-09-08 11:31 Stephan Gerhold
  2025-09-08 11:31 ` [PATCH v2 1/6] board: dragonboard410c: Drop now unneeded bootph-all for console Stephan Gerhold
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Stephan Gerhold @ 2025-09-08 11:31 UTC (permalink / raw)
  To: Casey Connolly
  Cc: Sumit Garg, Neil Armstrong, Tom Rini, Sam Day, u-boot-qcom,
	u-boot

There have been issues with autoboot on DB410c for years, where autoboot
gets interrupted by spurious input on the UART console. I've tried fixing
this back in 2021, but it turned out this fix was not enough. Looking into
it further, there are two separate issues to fix:

 - Spurious input with UART disconnected: The RX pin is floating in this
   case, resulting in garbage or a UART break event (0x00 byte). This is
   interpreted as input in the autoboot prompt. Fix this by adding
   bias-pull-up for the RX pin. I've made this change upstream in Linux
   6.16 for all MSM8916/MSM8939 devices and it has already landed in U-Boot
   through the recent subtree update of dts/upstream.

 - Spurious input when querying console size with UART connected: It's
   unclear why this happens, but it seems to be some issue in the overly
   complex implementation of 4-character packing mode in the serial_msm
   driver. Fix this by switching the driver to use the more simple
   single-character mode, which doesn't have this issue.

With these patches, autoboot is finally working reliably on DragonBoard
410c (and hopefully will remain so for the future :-)).

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
Changes in v2:
- Rebase on top of U-Boot master and fix conflicts
- Drop backported upstream DT changes (already landed through recent
  subtree merge of DT changes in Linux 6.16)
- Link to v1: https://lore.kernel.org/r/20250624-db410c-autoboot-fixes-v1-0-d96c97ae41d1@linaro.org

---
Stephan Gerhold (6):
      board: dragonboard410c: Drop now unneeded bootph-all for console
      Revert "serial: serial_msm: Delay initialization to let pins stabilize"
      serial: msm: Cleanup register naming
      serial: msm: Reset after writing to DMEN
      serial: msm: Re-enable after resetting
      serial: msm: Use single character mode

 arch/arm/dts/apq8016-sbc-u-boot.dtsi |   9 ---
 drivers/serial/serial_msm.c          | 127 ++++++++---------------------------
 2 files changed, 29 insertions(+), 107 deletions(-)
---
base-commit: 328747974a0620d0996113fe2b3906b06e96de00
change-id: 20250419-db410c-autoboot-fixes-ad7f4f5973a8

Best regards,
-- 
Stephan Gerhold <stephan.gerhold@linaro.org>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/6] board: dragonboard410c: Drop now unneeded bootph-all for console
  2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
@ 2025-09-08 11:31 ` Stephan Gerhold
  2025-09-08 11:31 ` [PATCH v2 2/6] Revert "serial: serial_msm: Delay initialization to let pins stabilize" Stephan Gerhold
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stephan Gerhold @ 2025-09-08 11:31 UTC (permalink / raw)
  To: Casey Connolly
  Cc: Sumit Garg, Neil Armstrong, Tom Rini, Sam Day, u-boot-qcom,
	u-boot

This is applied for all devices upstream in Linux now (when using the
console-specific pinctrl templates).

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 arch/arm/dts/apq8016-sbc-u-boot.dtsi | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/arch/arm/dts/apq8016-sbc-u-boot.dtsi b/arch/arm/dts/apq8016-sbc-u-boot.dtsi
index 26d4506815ea28cc667d3fbb211bfef33b43d013..de77a31cf1152e869c7c2bcc6433772ffb01591c 100644
--- a/arch/arm/dts/apq8016-sbc-u-boot.dtsi
+++ b/arch/arm/dts/apq8016-sbc-u-boot.dtsi
@@ -9,12 +9,3 @@
 		reg = <0 0x80000000 0 0x40000000>;
 	};
 };
-
-/*
- * When running as a first-stage bootloader, we need to re-configure the UART pins
- * because SBL de-initialises them. Indicate that the UART pins should be configured
- * during all boot stages.
- */
-&blsp_uart2_console_default {
-	bootph-all;
-};

-- 
2.50.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/6] Revert "serial: serial_msm: Delay initialization to let pins stabilize"
  2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
  2025-09-08 11:31 ` [PATCH v2 1/6] board: dragonboard410c: Drop now unneeded bootph-all for console Stephan Gerhold
@ 2025-09-08 11:31 ` Stephan Gerhold
  2025-09-08 11:31 ` [PATCH v2 3/6] serial: msm: Cleanup register naming Stephan Gerhold
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stephan Gerhold @ 2025-09-08 11:31 UTC (permalink / raw)
  To: Casey Connolly
  Cc: Sumit Garg, Neil Armstrong, Tom Rini, Sam Day, u-boot-qcom,
	u-boot

There have been issues with autoboot on DB410c for years, where autoboot
gets interrupted by spurious input on the UART console. Back in 2021, I've
tried to fix this by inserting a delay before UART initialization, but it
has turned out this is not working reliably either.

It looks like the root cause has always been the lack of bias-pull-up,
which was causing the RX line to be floating when UART is disconnected.
The delay does not seem to be needed anymore when applying bias-pull-up,
so drop it again in favor of the proper fix.

This reverts commit ad7e967738a9c639e07cf50b83ffccdf9a8537b0.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 drivers/serial/serial_msm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
index 757e5eaf974fc171c7a56006a218554be1425612..c8df91fca589433e57cd4c25a18d09548715c840 100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -17,7 +17,6 @@
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <linux/compiler.h>
-#include <linux/delay.h>
 #include <dm/pinctrl.h>
 
 /* Serial registers - this driver works in uartdm mode*/
@@ -207,8 +206,6 @@ static int calc_csr_bitrate(struct msm_serial_data *priv)
 
 static void uart_dm_init(struct msm_serial_data *priv)
 {
-	/* Delay initialization for a bit to let pins stabilize if necessary */
-	mdelay(5);
 	int bitrate = calc_csr_bitrate(priv);
 	if (bitrate < 0) {
 		log_warning("Couldn't calculate bit clock divider! Using default\n");

-- 
2.50.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/6] serial: msm: Cleanup register naming
  2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
  2025-09-08 11:31 ` [PATCH v2 1/6] board: dragonboard410c: Drop now unneeded bootph-all for console Stephan Gerhold
  2025-09-08 11:31 ` [PATCH v2 2/6] Revert "serial: serial_msm: Delay initialization to let pins stabilize" Stephan Gerhold
@ 2025-09-08 11:31 ` Stephan Gerhold
  2025-09-08 11:31 ` [PATCH v2 4/6] serial: msm: Reset after writing to DMEN Stephan Gerhold
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stephan Gerhold @ 2025-09-08 11:31 UTC (permalink / raw)
  To: Casey Connolly
  Cc: Sumit Garg, Neil Armstrong, Tom Rini, Sam Day, u-boot-qcom,
	u-boot

Some of the register definitions are inconsistently named (likely copied
as-is from Qualcomm's Little Kernel/LK bootloader, which uses the
MSM_BOOT_UART naming scheme). Rename them to be in line with the other
register definitions and move them up to be next to the related register.

No functional change.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 drivers/serial/serial_msm.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
index c8df91fca589433e57cd4c25a18d09548715c840..aa4d10e7c5138f919978272e4967ece8293073b3 100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -29,7 +29,9 @@
 #define UARTDM_RXFS_BUF_SHIFT   0x7  /* Number of bytes in the packing buffer */
 #define UARTDM_RXFS_BUF_MASK    0x7
 #define UARTDM_MR1				 0x00
+#define UARTDM_MR1_RX_RDY_CTL			 BIT(7)
 #define UARTDM_MR2				 0x04
+#define UARTDM_MR2_8_N_1_MODE			 0x34
 /*
  * This is documented on page 1817 of the apq8016e technical reference manual.
  * section 6.2.5.3.26
@@ -48,6 +50,8 @@
 #define UARTDM_SR_UART_OVERRUN   (1 << 4) /* Receive overrun */
 
 #define UARTDM_CR                         0xA8 /* Command register */
+#define UARTDM_CR_CMD_RESET_RX            (1 << 4) /* Reset receiver */
+#define UARTDM_CR_CMD_RESET_TX            (2 << 4) /* Reset transmitter */
 #define UARTDM_CR_CMD_RESET_ERR           (3 << 4) /* Clear overrun error */
 #define UARTDM_CR_CMD_RESET_STALE_INT     (8 << 4) /* Clears stale irq */
 #define UARTDM_CR_CMD_RESET_TX_READY      (3 << 8) /* Clears TX Ready irq*/
@@ -61,11 +65,6 @@
 #define UARTDM_TF               0x100 /* UART Transmit FIFO register */
 #define UARTDM_RF               0x140 /* UART Receive FIFO register */
 
-#define MSM_BOOT_UART_DM_8_N_1_MODE	0x34
-#define MSM_BOOT_UART_DM_CMD_RESET_RX	0x10
-#define MSM_BOOT_UART_DM_CMD_RESET_TX	0x20
-#define MSM_UART_MR1_RX_RDY_CTL		BIT(7)
-
 DECLARE_GLOBAL_DATA_PTR;
 
 struct msm_serial_data {
@@ -218,10 +217,10 @@ static void uart_dm_init(struct msm_serial_data *priv)
 
 	writel(bitrate, priv->base + UARTDM_CSR);
 	/* Enable RS232 flow control to support RS232 db9 connector */
-	writel(MSM_UART_MR1_RX_RDY_CTL, priv->base + UARTDM_MR1);
-	writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
-	writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
-	writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
+	writel(UARTDM_MR1_RX_RDY_CTL, priv->base + UARTDM_MR1);
+	writel(UARTDM_MR2_8_N_1_MODE, priv->base + UARTDM_MR2);
+	writel(UARTDM_CR_CMD_RESET_RX, priv->base + UARTDM_CR);
+	writel(UARTDM_CR_CMD_RESET_TX, priv->base + UARTDM_CR);
 
 	/* Make sure BAM/single character mode is disabled */
 	writel(0x0, priv->base + UARTDM_DMEN);

-- 
2.50.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/6] serial: msm: Reset after writing to DMEN
  2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
                   ` (2 preceding siblings ...)
  2025-09-08 11:31 ` [PATCH v2 3/6] serial: msm: Cleanup register naming Stephan Gerhold
@ 2025-09-08 11:31 ` Stephan Gerhold
  2025-09-08 11:31 ` [PATCH v2 5/6] serial: msm: Re-enable after resetting Stephan Gerhold
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stephan Gerhold @ 2025-09-08 11:31 UTC (permalink / raw)
  To: Casey Connolly
  Cc: Sumit Garg, Neil Armstrong, Tom Rini, Sam Day, u-boot-qcom,
	u-boot

According to the documentation of the UART controller in the APQ8016E TRM,
clearing bits inside UARTDM_DMEN requires resetting the transmitter and/or
receiver. We do reset inside uart_dm_init(), but before writing to
UARTDM_DMEN. This doesn't seem to cause problems in practice, but let's
move the reset to the end of uart_dm_init() to better match the
recommendations in the documentation.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 drivers/serial/serial_msm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
index aa4d10e7c5138f919978272e4967ece8293073b3..5523ec4afe17f242d61bc2ec4b4534b5b974434c 100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -219,11 +219,12 @@ static void uart_dm_init(struct msm_serial_data *priv)
 	/* Enable RS232 flow control to support RS232 db9 connector */
 	writel(UARTDM_MR1_RX_RDY_CTL, priv->base + UARTDM_MR1);
 	writel(UARTDM_MR2_8_N_1_MODE, priv->base + UARTDM_MR2);
-	writel(UARTDM_CR_CMD_RESET_RX, priv->base + UARTDM_CR);
-	writel(UARTDM_CR_CMD_RESET_TX, priv->base + UARTDM_CR);
 
 	/* Make sure BAM/single character mode is disabled */
 	writel(0x0, priv->base + UARTDM_DMEN);
+
+	writel(UARTDM_CR_CMD_RESET_RX, priv->base + UARTDM_CR);
+	writel(UARTDM_CR_CMD_RESET_TX, priv->base + UARTDM_CR);
 }
 static int msm_serial_probe(struct udevice *dev)
 {

-- 
2.50.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 5/6] serial: msm: Re-enable after resetting
  2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
                   ` (3 preceding siblings ...)
  2025-09-08 11:31 ` [PATCH v2 4/6] serial: msm: Reset after writing to DMEN Stephan Gerhold
@ 2025-09-08 11:31 ` Stephan Gerhold
  2025-09-08 11:31 ` [PATCH v2 6/6] serial: msm: Use single character mode Stephan Gerhold
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stephan Gerhold @ 2025-09-08 11:31 UTC (permalink / raw)
  To: Casey Connolly
  Cc: Sumit Garg, Neil Armstrong, Tom Rini, Sam Day, u-boot-qcom,
	u-boot

The documentation for the UART controller in the APQ8016E specifies that
both RESET and ENABLE commands must be issued to set up the receiver and
transmitter, but at the moment we only issue RESET. This doesn't seem to
cause issues in practice (looks like the reset already re-enables the
receiver/transmitter), but let's add the two writes to RX_ENABLE/TX_ENABLE
to better match the recommendations in the documentation.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 drivers/serial/serial_msm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
index 5523ec4afe17f242d61bc2ec4b4534b5b974434c..2c08a84b02773663f3c3a11eddaaa7a8bab2b4a1 100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -50,6 +50,8 @@
 #define UARTDM_SR_UART_OVERRUN   (1 << 4) /* Receive overrun */
 
 #define UARTDM_CR                         0xA8 /* Command register */
+#define UARTDM_CR_RX_ENABLE               (1 << 0) /* Enable receiver */
+#define UARTDM_CR_TX_ENABLE               (1 << 2) /* Enable transmitter */
 #define UARTDM_CR_CMD_RESET_RX            (1 << 4) /* Reset receiver */
 #define UARTDM_CR_CMD_RESET_TX            (2 << 4) /* Reset transmitter */
 #define UARTDM_CR_CMD_RESET_ERR           (3 << 4) /* Clear overrun error */
@@ -225,6 +227,8 @@ static void uart_dm_init(struct msm_serial_data *priv)
 
 	writel(UARTDM_CR_CMD_RESET_RX, priv->base + UARTDM_CR);
 	writel(UARTDM_CR_CMD_RESET_TX, priv->base + UARTDM_CR);
+	writel(UARTDM_CR_RX_ENABLE, priv->base + UARTDM_CR);
+	writel(UARTDM_CR_TX_ENABLE, priv->base + UARTDM_CR);
 }
 static int msm_serial_probe(struct udevice *dev)
 {

-- 
2.50.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 6/6] serial: msm: Use single character mode
  2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
                   ` (4 preceding siblings ...)
  2025-09-08 11:31 ` [PATCH v2 5/6] serial: msm: Re-enable after resetting Stephan Gerhold
@ 2025-09-08 11:31 ` Stephan Gerhold
  2025-09-09  4:23 ` [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Sumit Garg
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stephan Gerhold @ 2025-09-08 11:31 UTC (permalink / raw)
  To: Casey Connolly
  Cc: Sumit Garg, Neil Armstrong, Tom Rini, Sam Day, u-boot-qcom,
	u-boot

The UART DM controller supports different channel data packing modes,
either the 4-character packing mode (where 32-bit are read/written at once)
or the single-character mode (where only a single character is read/written
at a time). The 4-character mode can be more efficient, but the
single-character mode is much easier to implement.

At the moment, serial_msm uses the 4-character mode. Since the
dm_serial_ops operate on one character at the time, the code goes through
quite some hoops in order to break this down to single characters. This
code is prone to race conditions (e.g. priv->chars_cnt is read from the
registers, then a command is issued, what if another char came in
inbetween?). It also seems to cause another subtle issue with autoboot:

Unlike the previous autoboot failures that happened when UART was
disconnected, this problem occurs when UART is connected and open in a
terminal: For EFI boot, the console size is queried in efi_console.c
query_console_serial() by sending an ANSI escape code via UART. For some
reason, with the current driver we get yet another 0x00 byte (UART break
event?) when reading the reply from serial input. Because of that, reading
the console size fails in efi_console.c, the actual reply remains in the
UART buffer, and later the boot flow aborts because it detects input after
printing a prompt.

Rather than trying to fix the issue in the current complicated approach,
switch the driver to use the single-character mode. This is simple and
straightforward to implement without race conditions:

 - We write one character at a time to UARTDM_TF, as long as the TX FIFO
   has space available (TX_READY). To flush the console before starting
   Linux, we wait for TX_EMPTY.

 - We read one character at a time from UARTDM_RF and strip off the
   additional error information (assuming there is something in the
   RX FIFO, as indicated by RX_READY).

In this mode, querying the serial console size works and autoboot is no
longer interrupted. The overall code is also much shorter.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 drivers/serial/serial_msm.c | 102 +++++++-------------------------------------
 1 file changed, 16 insertions(+), 86 deletions(-)

diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
index 2c08a84b02773663f3c3a11eddaaa7a8bab2b4a1..18d15b7b15b3aeb2e8fd1da1aa49967d55c943ab 100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -21,13 +21,9 @@
 
 /* Serial registers - this driver works in uartdm mode*/
 
-#define UARTDM_DMRX             0x34 /* Max RX transfer length */
-#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
-#define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
+#define UARTDM_DMEN			0x3C /* DMA/data-packing mode */
+#define UARTDM_DMEN_TXRX_SC_ENABLE	(BIT(4) | BIT(5))
 
-#define UARTDM_RXFS             0x50 /* RX channel status register */
-#define UARTDM_RXFS_BUF_SHIFT   0x7  /* Number of bytes in the packing buffer */
-#define UARTDM_RXFS_BUF_MASK    0x7
 #define UARTDM_MR1				 0x00
 #define UARTDM_MR1_RX_RDY_CTL			 BIT(7)
 #define UARTDM_MR2				 0x04
@@ -45,118 +41,56 @@
 #define UARTDM_CSR				 0xA0
 
 #define UARTDM_SR                0xA4 /* Status register */
-#define UARTDM_SR_RX_READY       (1 << 0) /* Word is the receiver FIFO */
+#define UARTDM_SR_RX_READY       (1 << 0) /* Receiver FIFO has data */
+#define UARTDM_SR_TX_READY       (1 << 2) /* Transmitter FIFO has space */
 #define UARTDM_SR_TX_EMPTY       (1 << 3) /* Transmitter underrun */
-#define UARTDM_SR_UART_OVERRUN   (1 << 4) /* Receive overrun */
 
 #define UARTDM_CR                         0xA8 /* Command register */
 #define UARTDM_CR_RX_ENABLE               (1 << 0) /* Enable receiver */
 #define UARTDM_CR_TX_ENABLE               (1 << 2) /* Enable transmitter */
 #define UARTDM_CR_CMD_RESET_RX            (1 << 4) /* Reset receiver */
 #define UARTDM_CR_CMD_RESET_TX            (2 << 4) /* Reset transmitter */
-#define UARTDM_CR_CMD_RESET_ERR           (3 << 4) /* Clear overrun error */
-#define UARTDM_CR_CMD_RESET_STALE_INT     (8 << 4) /* Clears stale irq */
-#define UARTDM_CR_CMD_RESET_TX_READY      (3 << 8) /* Clears TX Ready irq*/
-#define UARTDM_CR_CMD_FORCE_STALE         (4 << 8) /* Causes stale event */
-#define UARTDM_CR_CMD_STALE_EVENT_DISABLE (6 << 8) /* Disable stale event */
-
-#define UARTDM_IMR                0xB0 /* Interrupt mask register */
-#define UARTDM_ISR                0xB4 /* Interrupt status register */
-#define UARTDM_ISR_TX_READY       0x80 /* TX FIFO empty */
 
 #define UARTDM_TF               0x100 /* UART Transmit FIFO register */
 #define UARTDM_RF               0x140 /* UART Receive FIFO register */
+#define UARTDM_RF_CHAR          0xff /* higher bits contain error information */
 
 DECLARE_GLOBAL_DATA_PTR;
 
 struct msm_serial_data {
 	phys_addr_t base;
-	unsigned chars_cnt; /* number of buffered chars */
-	uint32_t chars_buf; /* buffered chars */
 	uint32_t clk_rate; /* core clock rate */
 };
 
-static int msm_serial_fetch(struct udevice *dev)
-{
-	struct msm_serial_data *priv = dev_get_priv(dev);
-	unsigned sr;
-
-	if (priv->chars_cnt)
-		return priv->chars_cnt;
-
-	/* Clear error in case of buffer overrun */
-	if (readl(priv->base + UARTDM_SR) & UARTDM_SR_UART_OVERRUN)
-		writel(UARTDM_CR_CMD_RESET_ERR, priv->base + UARTDM_CR);
-
-	/* We need to fetch new character */
-	sr = readl(priv->base + UARTDM_SR);
-
-	if (sr & UARTDM_SR_RX_READY) {
-		/* There are at least 4 bytes in fifo */
-		priv->chars_buf = readl(priv->base + UARTDM_RF);
-		priv->chars_cnt = 4;
-	} else {
-		/* Check if there is anything in fifo */
-		priv->chars_cnt = readl(priv->base + UARTDM_RXFS);
-		/* Extract number of characters in UART packing buffer*/
-		priv->chars_cnt = (priv->chars_cnt >>
-				   UARTDM_RXFS_BUF_SHIFT) &
-				  UARTDM_RXFS_BUF_MASK;
-		if (!priv->chars_cnt)
-			return 0;
-
-		/* There is at least one charcter, move it to fifo */
-		writel(UARTDM_CR_CMD_FORCE_STALE,
-		       priv->base + UARTDM_CR);
-
-		priv->chars_buf = readl(priv->base + UARTDM_RF);
-		writel(UARTDM_CR_CMD_RESET_STALE_INT,
-		       priv->base + UARTDM_CR);
-		writel(0x7, priv->base + UARTDM_DMRX);
-	}
-
-	return priv->chars_cnt;
-}
-
 static int msm_serial_getc(struct udevice *dev)
 {
 	struct msm_serial_data *priv = dev_get_priv(dev);
-	char c;
 
-	if (!msm_serial_fetch(dev))
+	if (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_RX_READY))
 		return -EAGAIN;
 
-	c = priv->chars_buf & 0xFF;
-	priv->chars_buf >>= 8;
-	priv->chars_cnt--;
-
-	return c;
+	return readl(priv->base + UARTDM_RF) & UARTDM_RF_CHAR;
 }
 
 static int msm_serial_putc(struct udevice *dev, const char ch)
 {
 	struct msm_serial_data *priv = dev_get_priv(dev);
 
-	if (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_EMPTY) &&
-	    !(readl(priv->base + UARTDM_ISR) & UARTDM_ISR_TX_READY))
+	if (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_READY))
 		return -EAGAIN;
 
-	writel(UARTDM_CR_CMD_RESET_TX_READY, priv->base + UARTDM_CR);
-
-	writel(1, priv->base + UARTDM_NCF_TX);
 	writel(ch, priv->base + UARTDM_TF);
-
 	return 0;
 }
 
 static int msm_serial_pending(struct udevice *dev, bool input)
 {
-	if (input) {
-		if (msm_serial_fetch(dev))
-			return 1;
-	}
+	struct msm_serial_data *priv = dev_get_priv(dev);
 
-	return 0;
+	if (input)
+		return !!(readl(priv->base + UARTDM_SR) & UARTDM_SR_RX_READY);
+	else
+		return !(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_EMPTY);
 }
 
 static const struct dm_serial_ops msm_serial_ops = {
@@ -222,8 +156,8 @@ static void uart_dm_init(struct msm_serial_data *priv)
 	writel(UARTDM_MR1_RX_RDY_CTL, priv->base + UARTDM_MR1);
 	writel(UARTDM_MR2_8_N_1_MODE, priv->base + UARTDM_MR2);
 
-	/* Make sure BAM/single character mode is disabled */
-	writel(0x0, priv->base + UARTDM_DMEN);
+	/* Enable single character mode */
+	writel(UARTDM_DMEN_TXRX_SC_ENABLE, priv->base + UARTDM_DMEN);
 
 	writel(UARTDM_CR_CMD_RESET_RX, priv->base + UARTDM_CR);
 	writel(UARTDM_CR_CMD_RESET_TX, priv->base + UARTDM_CR);
@@ -320,13 +254,9 @@ static inline void _debug_uart_putc(int ch)
 {
 	struct msm_serial_data *priv = &init_serial_data;
 
-	while (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_EMPTY) &&
-	       !(readl(priv->base + UARTDM_ISR) & UARTDM_ISR_TX_READY))
+	while (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_READY))
 		;
 
-	writel(UARTDM_CR_CMD_RESET_TX_READY, priv->base + UARTDM_CR);
-
-	writel(1, priv->base + UARTDM_NCF_TX);
 	writel(ch, priv->base + UARTDM_TF);
 }
 

-- 
2.50.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c
  2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
                   ` (5 preceding siblings ...)
  2025-09-08 11:31 ` [PATCH v2 6/6] serial: msm: Use single character mode Stephan Gerhold
@ 2025-09-09  4:23 ` Sumit Garg
  2025-09-11 14:24 ` Alexey Minnekhanov
  2025-10-30 12:16 ` Casey Connolly
  8 siblings, 0 replies; 10+ messages in thread
From: Sumit Garg @ 2025-09-09  4:23 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Casey Connolly, Neil Armstrong, Tom Rini, Sam Day, u-boot-qcom,
	u-boot

On Mon, Sep 08, 2025 at 01:31:20PM +0200, Stephan Gerhold wrote:
> There have been issues with autoboot on DB410c for years, where autoboot
> gets interrupted by spurious input on the UART console. I've tried fixing
> this back in 2021, but it turned out this fix was not enough. Looking into
> it further, there are two separate issues to fix:
> 
>  - Spurious input with UART disconnected: The RX pin is floating in this
>    case, resulting in garbage or a UART break event (0x00 byte). This is
>    interpreted as input in the autoboot prompt. Fix this by adding
>    bias-pull-up for the RX pin. I've made this change upstream in Linux
>    6.16 for all MSM8916/MSM8939 devices and it has already landed in U-Boot
>    through the recent subtree update of dts/upstream.
> 
>  - Spurious input when querying console size with UART connected: It's
>    unclear why this happens, but it seems to be some issue in the overly
>    complex implementation of 4-character packing mode in the serial_msm
>    driver. Fix this by switching the driver to use the more simple
>    single-character mode, which doesn't have this issue.
> 
> With these patches, autoboot is finally working reliably on DragonBoard
> 410c (and hopefully will remain so for the future :-)).
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
> Changes in v2:
> - Rebase on top of U-Boot master and fix conflicts
> - Drop backported upstream DT changes (already landed through recent
>   subtree merge of DT changes in Linux 6.16)
> - Link to v1: https://lore.kernel.org/r/20250624-db410c-autoboot-fixes-v1-0-d96c97ae41d1@linaro.org

Nice fixes and cleanup, FWIW:

Acked-by: Sumit Garg <sumit.garg@oss.qualcomm.com>

-Sumit

> 
> ---
> Stephan Gerhold (6):
>       board: dragonboard410c: Drop now unneeded bootph-all for console
>       Revert "serial: serial_msm: Delay initialization to let pins stabilize"
>       serial: msm: Cleanup register naming
>       serial: msm: Reset after writing to DMEN
>       serial: msm: Re-enable after resetting
>       serial: msm: Use single character mode
> 
>  arch/arm/dts/apq8016-sbc-u-boot.dtsi |   9 ---
>  drivers/serial/serial_msm.c          | 127 ++++++++---------------------------
>  2 files changed, 29 insertions(+), 107 deletions(-)
> ---
> base-commit: 328747974a0620d0996113fe2b3906b06e96de00
> change-id: 20250419-db410c-autoboot-fixes-ad7f4f5973a8
> 
> Best regards,
> -- 
> Stephan Gerhold <stephan.gerhold@linaro.org>
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c
  2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
                   ` (6 preceding siblings ...)
  2025-09-09  4:23 ` [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Sumit Garg
@ 2025-09-11 14:24 ` Alexey Minnekhanov
  2025-10-30 12:16 ` Casey Connolly
  8 siblings, 0 replies; 10+ messages in thread
From: Alexey Minnekhanov @ 2025-09-11 14:24 UTC (permalink / raw)
  To: u-boot-qcom, Casey Connolly
  Cc: Sumit Garg, Neil Armstrong, Tom Rini, Sam Day, u-boot

On 08.09.2025 14:31, Stephan Gerhold via groups.io wrote:
> There have been issues with autoboot on DB410c for years, where autoboot
> gets interrupted by spurious input on the UART console. I've tried fixing
> this back in 2021, but it turned out this fix was not enough. Looking into
> it further, there are two separate issues to fix:
> 
>   - Spurious input with UART disconnected: The RX pin is floating in this
>     case, resulting in garbage or a UART break event (0x00 byte). This is
>     interpreted as input in the autoboot prompt. Fix this by adding
>     bias-pull-up for the RX pin. I've made this change upstream in Linux
>     6.16 for all MSM8916/MSM8939 devices and it has already landed in U-Boot
>     through the recent subtree update of dts/upstream.
> 
>   - Spurious input when querying console size with UART connected: It's
>     unclear why this happens, but it seems to be some issue in the overly
>     complex implementation of 4-character packing mode in the serial_msm
>     driver. Fix this by switching the driver to use the more simple
>     single-character mode, which doesn't have this issue.
> 
> With these patches, autoboot is finally working reliably on DragonBoard
> 410c (and hopefully will remain so for the future :-)).
> 
FWIW I've tested this series using serial UART DM interface on SDM660 
device. It does not fix *all* the issues needed for it to function 
there, but I can confirm it does not break anything and makes things 
much better than before, now with proper resetting and single-character 
mode it's possible to not rely on configuration from previous boot 
loader, for example.

Tested-by: Alexey Minnekhanov <alexeymin@minlexx.ru>

--
Regards,
Alexey Minnekhanov

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c
  2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
                   ` (7 preceding siblings ...)
  2025-09-11 14:24 ` Alexey Minnekhanov
@ 2025-10-30 12:16 ` Casey Connolly
  8 siblings, 0 replies; 10+ messages in thread
From: Casey Connolly @ 2025-10-30 12:16 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Sumit Garg, Neil Armstrong, Tom Rini, Sam Day, u-boot-qcom,
	u-boot


On Mon, 08 Sep 2025 13:31:20 +0200, Stephan Gerhold wrote:
> There have been issues with autoboot on DB410c for years, where autoboot
> gets interrupted by spurious input on the UART console. I've tried fixing
> this back in 2021, but it turned out this fix was not enough. Looking into
> it further, there are two separate issues to fix:
> 
>  - Spurious input with UART disconnected: The RX pin is floating in this
>    case, resulting in garbage or a UART break event (0x00 byte). This is
>    interpreted as input in the autoboot prompt. Fix this by adding
>    bias-pull-up for the RX pin. I've made this change upstream in Linux
>    6.16 for all MSM8916/MSM8939 devices and it has already landed in U-Boot
>    through the recent subtree update of dts/upstream.
> 
> [...]

Applied, thanks!

[1/6] board: dragonboard410c: Drop now unneeded bootph-all for console
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/4e26c1e2fa6a
[2/6] Revert "serial: serial_msm: Delay initialization to let pins stabilize"
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/080d19b6e64e
[3/6] serial: msm: Cleanup register naming
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/a8f5b34ec578
[4/6] serial: msm: Reset after writing to DMEN
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/eac35b902715
[5/6] serial: msm: Re-enable after resetting
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/5b83a081a762
[6/6] serial: msm: Use single character mode
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/ff701837850e

Best regards,
-- 
// Caleb (they/them)



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-10-30 12:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-08 11:31 [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Stephan Gerhold
2025-09-08 11:31 ` [PATCH v2 1/6] board: dragonboard410c: Drop now unneeded bootph-all for console Stephan Gerhold
2025-09-08 11:31 ` [PATCH v2 2/6] Revert "serial: serial_msm: Delay initialization to let pins stabilize" Stephan Gerhold
2025-09-08 11:31 ` [PATCH v2 3/6] serial: msm: Cleanup register naming Stephan Gerhold
2025-09-08 11:31 ` [PATCH v2 4/6] serial: msm: Reset after writing to DMEN Stephan Gerhold
2025-09-08 11:31 ` [PATCH v2 5/6] serial: msm: Re-enable after resetting Stephan Gerhold
2025-09-08 11:31 ` [PATCH v2 6/6] serial: msm: Use single character mode Stephan Gerhold
2025-09-09  4:23 ` [PATCH v2 0/6] Fix UART autoboot issues on DragonBoard 410c Sumit Garg
2025-09-11 14:24 ` Alexey Minnekhanov
2025-10-30 12:16 ` Casey Connolly

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.