[PATCH v3 0/3] MIPS: SiByte: Fix serial device regressions

[PATCH v3 0/3] MIPS: SiByte: Fix serial device regressions

[Maciej W. Rozycki]

Hi,

 In this v3 of the series compared to v2 changes 3/6 through 6/6 have been 
dropped as not meant for backporting, along with the relevant submission 
cc recipients.  Additionally the delay loop introduced with 1/6 has been 
fixed to work correctly early in boot.  The relevant parts of the original 
cover letter follow.

 Starting from commit 84a9582fd203 ("serial: core: Start managing serial 
controllers to enable runtime PM") the driver for the SiByte onchip DUART 
has stopped working due to a null pointer dereference in the serial core, 
which means a kernel crash at bootstrap if the driver has been enabled, as 
is usually the case for the system console.

 This patch series addresses the issue by switching the driver away from 
legacy probing to using platform devices.  A notable consequence of this 
is the serial console is only switched from the bootconsole handler that 
uses firmware calls over to our serial driver at the time the driver is 
being properly brought up.  This causes messages to be produced to the 
console between the device reset and console setup, which in turn causes 
the firmware still being called via the bootconsole handler to loop 
forever owing to the transmitter having been disabled.

 Therefore introductory change 2/3 is included to fix the issue by doing a 
rudimentary device setup right after reset, using parameters compatible 
with those used by the firmware (115200n8).  There is auxiliary change 1/3 
included as well that prevents message corruption from happening at reset, 
which becomes more prominent due to the change in timing, with the driver 
switch to the platform device, of messages produced to the console.

 See individual change descriptions for details.  Verified with a SWARM
(BCM91250A) system.

 Please apply.

 Previous iterations:

- v2 at: <https://lore.kernel.org/r/alpine.DEB.2.21.2605241602220.1450@angie.orcam.me.uk/>.

- v1 at: <https://lore.kernel.org/r/alpine.DEB.2.21.2604130239560.29980@angie.orcam.me.uk/>.

  Maciej

[PATCH v3 1/3] serial: sb1250-duart: Fix console message clobbering at channel resets

[Maciej W. Rozycki]

Ensure any characters outstanding have been sent before issuing channel 
resets so as to prevent messages issued to the bootconsole from getting 
clobbered.

Contrary to device documentation at the time the transmitter empty bit 
is set only the transmit FIFO has been drained and there is still data 
outstanding in the transmitter shift register, so wait an extra amount 
of time for that register to drain too.  This also prevents subsequent 
messages produced to the console from getting clobbered, owing to what
seems a transmitter synchronisation issue.

When called from sbd_serial_console_init() it is too early for fsleep() 
to work and even before lpj has been calculated, and therefore neither 
udelay() works.  Therefore delay by hand, observing that roughly 170 
iterations over an Input Port Register read are sufficient for the 
transmitter to drain with the BCM1250 SoC clocked at 800MHz, so using 
2048 iterations should give enough margin including in particular for 
the faster BCM1480 SoC clocked at 1GHz.

Fixes: b45d52797432 ("sb1250-duart.c: SB1250 DUART serial support")
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Cc: stable@vger.kernel.org # v2.6.23+
---
Changes from v2 (1/6), 
<https://lore.kernel.org/r/alpine.DEB.2.21.2605241620470.1450@angie.orcam.me.uk/>:

- In the early-boot case delay by hand rather than via udelay() and switch 
  to fsleep() for the late execution case.

- Update the Fixes: tag now that the delay has been sanitised and is more 
  than just a placeholder.

Changes from v1 (1/4),
<https://lore.kernel.org/r/alpine.DEB.2.21.2604130321540.29980@angie.orcam.me.uk/>:

- Sanitise the change heading.
---
 drivers/tty/serial/sb1250-duart.c |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

linux-serial-sb1250-duart-reset-drain.diff
Index: linux-swarm64/drivers/tty/serial/sb1250-duart.c
===================================================================
--- linux-swarm64.orig/drivers/tty/serial/sb1250-duart.c
+++ linux-swarm64/drivers/tty/serial/sb1250-duart.c
@@ -516,6 +516,33 @@ static void sbd_init_port(struct sbd_por
 
 	if (sport->initialised)
 		return;
+	/*
+	 * Contrary to documentation, which says that the transmitter
+	 * empty bit is set when "there are no characters to send and
+	 * the transmitter is idle," the bit is already set by hardware
+	 * once the transmit FIFO has been drained only and while the
+	 * transmitter shift register still holds data being supplied
+	 * to the line.  Consequently issuing a transmitter reset at
+	 * this point causes the final character outstanding to be lost.
+	 *
+	 * Moreover, resetting the transmitter while transmission is
+	 * in progress appears to make the transmitter go out of sync
+	 * and subsequent characters issued after the transmitter has
+	 * been reprogrammed and re-enabled are sent corrupted or with
+	 * their bit patterns shifted.
+	 *
+	 * So once the transmitter empty bit has been set wait an extra
+	 * amount of time, sufficient for the transmitter shift register
+	 * to drain at 115200bps, which is the baud rate setting used by
+	 * a standard CFE firmware compilation.
+	 */
+	sbd_line_drain(sport);
+	if (IS_ENABLED(CONFIG_SERIAL_SB1250_DUART_CONSOLE) &&
+	    system_state < SYSTEM_SCHEDULING)
+		for (int i = 0; i < 2048; i++)
+			read_sbdshr(sport, R_DUART_IN_PORT);
+	else
+		fsleep(100);
 
 	/* There is no DUART reset feature, so just set some sane defaults.  */
 	write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_TX);

[PATCH v3 2/3] serial: sb1250-duart: Fix bootconsole handover lockup

[Maciej W. Rozycki]

Calling sbd_init_port() in the course of setting up the serial device 
causes line parameters to be messed up and the transmitter disabled.  
We've been lucky in that no message is usually produced to the kernel 
log between this call and the later call to uart_set_options() in the 
course of console setup done by sbd_serial_console_init(), or the system 
would hang as the console output handler in CFE tried to access a port 
whose transmitter has been disabled and line parameters messed up.

It'll change with the next change to the driver, so fix sbd_init_port() 
such that line parameters are set for 115200n8 console operation as with 
the CFE firmware and the transmitter re-enabled after reset.

Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Cc: stable@vger.kernel.org # v6.5+
---
No change from v2 (2/3),
<https://lore.kernel.org/r/alpine.DEB.2.21.2605241623110.1450@angie.orcam.me.uk/>.

Changes from v1 (2/4),
<https://lore.kernel.org/r/alpine.DEB.2.21.2604130338210.29980@angie.orcam.me.uk/>:

- Sanitise the change heading.
---
 drivers/tty/serial/sb1250-duart.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

linux-serial-sb1250-duart-prom-console.diff
Index: linux-macro/drivers/tty/serial/sb1250-duart.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/sb1250-duart.c
+++ linux-macro/drivers/tty/serial/sb1250-duart.c
@@ -542,14 +542,19 @@ static void sbd_init_port(struct sbd_por
 	/* There is no DUART reset feature, so just set some sane defaults.  */
 	write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_TX);
 	write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_RX);
-	write_sbdchn(sport, R_DUART_MODE_REG_1, V_DUART_BITS_PER_CHAR_8);
+	write_sbdchn(sport, R_DUART_MODE_REG_1,
+		     V_DUART_PARITY_MODE_NONE | V_DUART_BITS_PER_CHAR_8);
 	write_sbdchn(sport, R_DUART_MODE_REG_2, 0);
+	write_sbdchn(sport, R_DUART_CLK_SEL, V_DUART_BAUD_RATE(115200));
 	write_sbdchn(sport, R_DUART_FULL_CTL,
 		     V_DUART_INT_TIME(0) | V_DUART_SIG_FULL(15));
 	write_sbdchn(sport, R_DUART_OPCR_X, 0);
 	write_sbdchn(sport, R_DUART_AUXCTL_X, 0);
 	write_sbdshr(sport, R_DUART_IMRREG((uport->line) % 2), 0);
 
+	/* Re-enable transmission for the initial PROM-based console.  */
+	write_sbdchn(sport, R_DUART_CMD, M_DUART_TX_EN);
+
 	sport->initialised = 1;
 }
 

[PATCH v3 3/3] serial: sb1250-duart: Convert to use a platform device

[Maciej W. Rozycki]

Prevent a crash from happening as the first serial port is initialised:

  pata-swarm: PATA interface at GenBus slot 4
  workingset: timestamp_bits=62 max_order=18 bucket_order=0
  Block layer SCSI generic (bsg) driver version 0.4 loaded (major 253)
  CPU 1 Unable to handle kernel paging request at virtual address 0000000000000208, epc == ffffffff8067f8f8, ra == ffffffff80666330
  Oops[#1]:
  CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.19.0-dirty #27 NONE 
  $ 0   : 0000000000000000 0000000014001fe0 0000000000000020 ffffffff80666130
  $ 4   : 0000000000000000 a800000100e6f118 ffffffff8112cbc0 0000000000000000
  $ 8   : 0000000000000002 0000000000000000 0000000000000000 0000000001a80000
  $12   : 0000000000000000 ffffffff809fd488 000000004ddf14dd ffffffff00000000
  $16   : a800000100e6f000 0000000000000000 ffffffff8112c1d0 a800000100e6f000
  $20   : 0000000000000000 00000000000004d0 0000000000000004 ffffffff8112c1d0
  $24   : 0000000000000001 0000000000000003                                  
  $28   : a80000010007c000 a80000010007fcb0 00000000000000ef ffffffff80666330
  Hi    : fffffffffffffdb9
  Lo    : 0000000000000035
  epc   : ffffffff8067f8f8 __dev_fwnode+0x0/0x8
  ra    : ffffffff80666330 serial_base_ctrl_add+0xb8/0x180
  Status: 14001fe3	KX SX UX KERNEL EXL IE 
  Cause : 80800008 (ExcCode 02)
  BadVA : 0000000000000208
  PrId  : 03040102 (SiByte SB1)
  Process swapper/0 (pid: 1, threadinfo=(____ptrval____), task=(____ptrval____), tls=0000000000000000)
  Stack : 0000000000000000 ffffffff80cd5178 ffffffff80cd0000 ffffffff8112c1c8
          0000000000000260 ffffffff806655c4 a800000100275bc0 ffffffff8064ac88
          004000408112c000 0000000000000002 0000000000000000 ffffffff801965d0
          a800000100786ba0 ffffffff80cd5178 a800000100786ba0 0000000000000004
          a800000100275bc0 0000000000000000 0000000000000000 ffffffff80cd5178
          0000000000000000 ffffffff8112c1c8 0000000000000260 00000000000004d0
          0000000000000004 ffffffff80bf0000 00000000000000ef ffffffff80d171dc
          ffffffff80d17120 ffffffff80d25658 0000000000000000 ffffffff80d50000
          ffffffff80d2f928 ffffffff80d50000 ffffffff80d25698 ffffffff80cfcecc
          00ffffff80b84428 0000000000000000 0000000000000006 0000000000000006
          ...
  Call Trace:
  [<ffffffff8067f8f8>] __dev_fwnode+0x0/0x8
  [<ffffffff80666330>] serial_base_ctrl_add+0xb8/0x180
  [<ffffffff806655c4>] serial_core_register_port+0x174/0x8e0
  [<ffffffff80d171dc>] sbd_init+0xbc/0xf4
  [<ffffffff80cfcecc>] do_one_initcall+0x64/0x150
  [<ffffffff80cfd284>] kernel_init_freeable+0x25c/0x30c
  [<ffffffff809ff6e4>] kernel_init+0x24/0x118
  [<ffffffff80112d20>] ret_from_kernel_thread+0x14/0x1c
  
  Code: 67bd0010  03e00008  2402ffea <03e00008> dc820208  67bdffa0  ffbe0050  ffb70048  ffb60040 
  
  ---[ end trace 0000000000000000 ]---

-- where a pointer is dereferenced that has been derived from a null
pointer to the port's parent device.

Since no device is available with legacy probing and it's not anymore a
preferable way to discover devices anyway, switch the driver to using a
platform device and use it as the port's parent device.

Use platform_driver_probe() because SB1250 DUART devices are embedded 
onchip the SoC and therefore not that straightforward to remove.

An unfortunate consequence of the switch to a platform device is we now 
hand the console over from the bootconsole much later in the bootstrap. 
The CFE console handler appears good enough though to work so late and 
in particular with interrupts enabled.

Conversely, only starting the console device so late lets the port 
initialisation code always use fsleep() for transmitter draining, so 
drop the other variant since it is now dead code.

Since there is one way only remaining to reach sbd_init_port() now, drop 
the port initialisation marker as no longer needed and go through the 
channel resets unconditionally.

Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Cc: stable@vger.kernel.org # needs to use .remove_new for <= 6.10
---
Changes from v2 (3/6),
<https://lore.kernel.org/r/alpine.DEB.2.21.2605241625180.1450@angie.orcam.me.uk/>:

- Update port initialisation code according to transmitter drain delay 
  improvements in 1/3.

Changes from v1 (3/4),
<https://lore.kernel.org/r/alpine.DEB.2.21.2604130357420.29980@angie.orcam.me.uk/>:

- Sanitise the change heading.
---
 arch/mips/sibyte/swarm/platform.c |   97 ++++++++++++++++++++++--
 drivers/tty/serial/sb1250-duart.c |  152 +++++++++++++-------------------------
 2 files changed, 145 insertions(+), 104 deletions(-)

linux-serial-sb1250-duart-platform.diff
Index: linux-swarm64/arch/mips/sibyte/swarm/platform.c
===================================================================
--- linux-swarm64.orig/arch/mips/sibyte/swarm/platform.c
+++ linux-swarm64/arch/mips/sibyte/swarm/platform.c
@@ -8,7 +8,13 @@
 
 #include <asm/sibyte/board.h>
 #include <asm/sibyte/sb1250_genbus.h>
+#if defined(CONFIG_SIBYTE_BCM1x80)
+#include <asm/sibyte/bcm1480_regs.h>
+#include <asm/sibyte/bcm1480_int.h>
+#else
 #include <asm/sibyte/sb1250_regs.h>
+#include <asm/sibyte/sb1250_int.h>
+#endif
 
 #if defined(CONFIG_SIBYTE_SWARM) || defined(CONFIG_SIBYTE_LITTLESUR)
 
@@ -85,6 +91,82 @@ device_initcall(swarm_pata_init);
 
 #endif /* defined(CONFIG_SIBYTE_SWARM) || defined(CONFIG_SIBYTE_LITTLESUR) */
 
+#if defined(CONFIG_SIBYTE_BCM1x80)
+static struct resource sb1250_duart_resources[][2] = {
+	{
+		{
+			.name = "sb1250-duart0",
+			.flags = IORESOURCE_MEM,
+			.start = A_BCM1480_DUART0,
+			.end = (A_BCM1480_DUART0 +
+				4 * BCM1480_DUART_CHANREG_SPACING - 1),
+		},
+		{
+			.name = "sb1250-duart0",
+			.flags = IORESOURCE_IRQ,
+			.start = K_BCM1480_INT_UART_0,
+			.end = K_BCM1480_INT_UART_1,
+		},
+	},
+	{
+		{
+			.name = "sb1250-duart1",
+			.flags = IORESOURCE_MEM,
+			.start = A_BCM1480_DUART1,
+			.end = (A_BCM1480_DUART1 +
+				4 * BCM1480_DUART_CHANREG_SPACING - 1),
+		},
+		{
+			.name = "sb1250-duart1",
+			.flags = IORESOURCE_IRQ,
+			.start = K_BCM1480_INT_UART_2,
+			.end = K_BCM1480_INT_UART_3,
+		},
+	},
+};
+#else /* !defined(CONFIG_SIBYTE_BCM1x80) */
+static struct resource sb1250_duart_resources[][2] = {
+	{
+		{
+			.name = "sb1250-duart0",
+			.flags = IORESOURCE_MEM,
+			.start = A_DUART,
+			.end = A_DUART + 4 * DUART_CHANREG_SPACING - 1,
+		},
+		{
+			.name = "sb1250-duart0",
+			.flags = IORESOURCE_IRQ,
+			.start = K_INT_UART_0,
+			.end = K_INT_UART_1,
+		},
+	},
+};
+#endif /* !defined(CONFIG_SIBYTE_BCM1x80) */
+
+static struct platform_device sb1250_duart_device[] = {
+	{
+		.name = "sb1250-duart",
+		.id = 0,
+		.resource = sb1250_duart_resources[0],
+		.num_resources = ARRAY_SIZE(sb1250_duart_resources[0]),
+	},
+#if defined(CONFIG_SIBYTE_BCM1x80)
+	{
+		.name = "sb1250-duart",
+		.id = 1,
+		.resource = sb1250_duart_resources[1],
+		.num_resources = ARRAY_SIZE(sb1250_duart_resources[1]),
+	},
+#endif
+};
+
+static struct platform_device *sb1250_duart_devices[] __initdata = {
+	&sb1250_duart_device[0],
+#if defined(CONFIG_SIBYTE_BCM1x80)
+	&sb1250_duart_device[1],
+#endif
+};
+
 #define sb1250_dev_struct(num) \
 	static struct resource sb1250_res##num = {		\
 		.name = "SB1250 MAC " __stringify(num),		\
@@ -113,28 +195,31 @@ static struct platform_device *sb1250_de
 
 static int __init sb1250_device_init(void)
 {
-	int ret;
+	int ret1, ret2;
+
+	ret1 = platform_add_devices(sb1250_duart_devices,
+				    ARRAY_SIZE(sb1250_duart_devices));
 
 	/* Set the number of available units based on the SOC type.  */
 	switch (soc_type) {
 	case K_SYS_SOC_TYPE_BCM1250:
 	case K_SYS_SOC_TYPE_BCM1250_ALT:
-		ret = platform_add_devices(sb1250_devs, 3);
+		ret2 = platform_add_devices(sb1250_devs, 3);
 		break;
 	case K_SYS_SOC_TYPE_BCM1120:
 	case K_SYS_SOC_TYPE_BCM1125:
 	case K_SYS_SOC_TYPE_BCM1125H:
 	case K_SYS_SOC_TYPE_BCM1250_ALT2:	/* Hybrid */
-		ret = platform_add_devices(sb1250_devs, 2);
+		ret2 = platform_add_devices(sb1250_devs, 2);
 		break;
 	case K_SYS_SOC_TYPE_BCM1x55:
 	case K_SYS_SOC_TYPE_BCM1x80:
-		ret = platform_add_devices(sb1250_devs, 4);
+		ret2 = platform_add_devices(sb1250_devs, 4);
 		break;
 	default:
-		ret = -ENODEV;
+		ret2 = 0;
 		break;
 	}
-	return ret;
+	return ret1 ? ret1 : ret2;
 }
 device_initcall(sb1250_device_init);
Index: linux-swarm64/drivers/tty/serial/sb1250-duart.c
===================================================================
--- linux-swarm64.orig/drivers/tty/serial/sb1250-duart.c
+++ linux-swarm64/drivers/tty/serial/sb1250-duart.c
@@ -3,7 +3,7 @@
  *	Support for the asynchronous serial interface (DUART) included
  *	in the BCM1250 and derived System-On-a-Chip (SOC) devices.
  *
- *	Copyright (c) 2007  Maciej W. Rozycki
+ *	Copyright (c) 2007, 2026  Maciej W. Rozycki
  *
  *	Derived from drivers/char/sb1250_duart.c for which the following
  *	copyright applies:
@@ -25,6 +25,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/major.h>
+#include <linux/platform_device.h>
 #include <linux/serial.h>
 #include <linux/serial_core.h>
 #include <linux/spinlock.h>
@@ -45,10 +46,6 @@
 #include <asm/sibyte/bcm1480_regs.h>
 #include <asm/sibyte/bcm1480_int.h>
 
-#define SBD_CHANREGS(line)	A_BCM1480_DUART_CHANREG((line), 0)
-#define SBD_CTRLREGS(line)	A_BCM1480_DUART_CTRLREG((line), 0)
-#define SBD_INT(line)		(K_BCM1480_INT_UART_0 + (line))
-
 #define DUART_CHANREG_SPACING	BCM1480_DUART_CHANREG_SPACING
 
 #define R_DUART_IMRREG(line)	R_BCM1480_DUART_IMRREG(line)
@@ -59,10 +56,6 @@
 #include <asm/sibyte/sb1250_regs.h>
 #include <asm/sibyte/sb1250_int.h>
 
-#define SBD_CHANREGS(line)	A_DUART_CHANREG((line), 0)
-#define SBD_CTRLREGS(line)	A_DUART_CTRLREG(0)
-#define SBD_INT(line)		(K_INT_UART_0 + (line))
-
 #else
 #error invalid SB1250 UART configuration
 
@@ -85,7 +78,6 @@ struct sbd_port {
 	struct uart_port	port;
 	unsigned char __iomem	*memctrl;
 	int			tx_stopped;
-	int			initialised;
 };
 
 /*
@@ -100,6 +92,7 @@ struct sbd_duart {
 #define to_sport(uport) container_of(uport, struct sbd_port, port)
 
 static struct sbd_duart sbd_duarts[DUART_MAX_CHIP];
+static struct uart_driver sbd_reg;
 
 
 /*
@@ -514,8 +507,6 @@ static void sbd_init_port(struct sbd_por
 {
 	struct uart_port *uport = &sport->port;
 
-	if (sport->initialised)
-		return;
 	/*
 	 * Contrary to documentation, which says that the transmitter
 	 * empty bit is set when "there are no characters to send and
@@ -537,12 +528,7 @@ static void sbd_init_port(struct sbd_por
 	 * a standard CFE firmware compilation.
 	 */
 	sbd_line_drain(sport);
-	if (IS_ENABLED(CONFIG_SERIAL_SB1250_DUART_CONSOLE) &&
-	    system_state < SYSTEM_SCHEDULING)
-		for (int i = 0; i < 2048; i++)
-			read_sbdshr(sport, R_DUART_IN_PORT);
-	else
-		fsleep(100);
+	fsleep(100);
 
 	/* There is no DUART reset feature, so just set some sane defaults.  */
 	write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_TX);
@@ -559,8 +545,6 @@ static void sbd_init_port(struct sbd_por
 
 	/* Re-enable transmission for the initial PROM-based console.  */
 	write_sbdchn(sport, R_DUART_CMD, M_DUART_TX_EN);
-
-	sport->initialised = 1;
 }
 
 static void sbd_set_termios(struct uart_port *uport, struct ktermios *termios,
@@ -799,50 +783,54 @@ static const struct uart_ops sbd_ops = {
 };
 
 /* Initialize SB1250 DUART port structures.  */
-static void __init sbd_probe_duarts(void)
+static int __init sbd_probe(struct platform_device *pdev)
 {
-	static int probed;
+	struct resource *mem_resource, *irq_resource;
 	int chip, side;
-	int max_lines, line;
 
-	if (probed)
-		return;
+	mem_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	irq_resource = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!mem_resource || !irq_resource)
+		return -ENODEV;
 
-	/* Set the number of available units based on the SOC type.  */
-	switch (soc_type) {
-	case K_SYS_SOC_TYPE_BCM1x55:
-	case K_SYS_SOC_TYPE_BCM1x80:
-		max_lines = 4;
-		break;
-	default:
-		/* Assume at least two serial ports at the normal address.  */
-		max_lines = 2;
-		break;
-	}
+	chip = pdev->id;
+	sbd_duarts[chip].mapctrl = mem_resource->start +
+				   DUART_CHANREG_SPACING * 3;
+	for (side = 0; side < DUART_MAX_SIDE; side++) {
+		struct sbd_port *sport = &sbd_duarts[chip].sport[side];
+		struct uart_port *uport = &sport->port;
 
-	probed = 1;
+		sport->duart	= &sbd_duarts[chip];
 
-	for (chip = 0, line = 0; chip < DUART_MAX_CHIP && line < max_lines;
-	     chip++) {
-		sbd_duarts[chip].mapctrl = SBD_CTRLREGS(line);
+		uport->dev	= &pdev->dev;
+		uport->irq	= irq_resource->start + side;
+		uport->uartclk	= 100000000 / 20 * 16;
+		uport->fifosize	= 16;
+		uport->iotype	= UPIO_MEM;
+		uport->flags	= UPF_BOOT_AUTOCONF;
+		uport->ops	= &sbd_ops;
+		uport->line	= chip * DUART_MAX_SIDE + side;
+		uport->mapbase	= mem_resource->start +
+				  DUART_CHANREG_SPACING * (side + 1);
+		uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_SB1250_DUART_CONSOLE);
+		if (uart_add_one_port(&sbd_reg, uport))
+			uport->dev = NULL;
+	}
 
-		for (side = 0; side < DUART_MAX_SIDE && line < max_lines;
-		     side++, line++) {
-			struct sbd_port *sport = &sbd_duarts[chip].sport[side];
-			struct uart_port *uport = &sport->port;
+	return 0;
+}
 
-			sport->duart	= &sbd_duarts[chip];
+static void __exit sbd_remove(struct platform_device *pdev)
+{
+	int chip, side;
 
-			uport->irq	= SBD_INT(line);
-			uport->uartclk	= 100000000 / 20 * 16;
-			uport->fifosize	= 16;
-			uport->iotype	= UPIO_MEM;
-			uport->flags	= UPF_BOOT_AUTOCONF;
-			uport->ops	= &sbd_ops;
-			uport->line	= line;
-			uport->mapbase	= SBD_CHANREGS(line);
-			uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_SB1250_DUART_CONSOLE);
-		}
+	chip = pdev->id;
+	for (side = DUART_MAX_SIDE - 1; side >= 0; side--) {
+		struct sbd_port *sport = &sbd_duarts[chip].sport[side];
+		struct uart_port *uport = &sport->port;
+
+		if (uport->dev)
+			uart_remove_one_port(&sbd_reg, uport);
 	}
 }
 
@@ -900,23 +888,14 @@ static int __init sbd_console_setup(stru
 	int bits = 8;
 	int parity = 'n';
 	int flow = 'n';
-	int ret;
 
 	if (!sport->duart)
 		return -ENXIO;
-
-	ret = sbd_map_port(uport);
-	if (ret)
-		return ret;
-
-	sbd_init_port(sport);
-
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
 	return uart_set_options(uport, co, baud, parity, bits, flow);
 }
 
-static struct uart_driver sbd_reg;
 static struct console sbd_console = {
 	.name	= "duart",
 	.write	= sbd_console_write,
@@ -927,16 +906,6 @@ static struct console sbd_console = {
 	.data	= &sbd_reg
 };
 
-static int __init sbd_serial_console_init(void)
-{
-	sbd_probe_duarts();
-	register_console(&sbd_console);
-
-	return 0;
-}
-
-console_initcall(sbd_serial_console_init);
-
 #define SERIAL_SB1250_DUART_CONSOLE	&sbd_console
 #else
 #define SERIAL_SB1250_DUART_CONSOLE	NULL
@@ -953,43 +922,30 @@ static struct uart_driver sbd_reg = {
 	.cons		= SERIAL_SB1250_DUART_CONSOLE,
 };
 
+static struct platform_driver sbd_driver = {
+	.remove = __exit_p(sbd_remove),
+	.driver = { .name = "sb1250-duart" },
+};
+
 /* Set up the driver and register it.  */
 static int __init sbd_init(void)
 {
-	int i, ret;
-
-	sbd_probe_duarts();
+	int ret;
 
 	ret = uart_register_driver(&sbd_reg);
 	if (ret)
 		return ret;
+	ret = platform_driver_probe(&sbd_driver, sbd_probe);
+	if (ret)
+		uart_unregister_driver(&sbd_reg);
 
-	for (i = 0; i < DUART_MAX_CHIP * DUART_MAX_SIDE; i++) {
-		struct sbd_duart *duart = &sbd_duarts[i / DUART_MAX_SIDE];
-		struct sbd_port *sport = &duart->sport[i % DUART_MAX_SIDE];
-		struct uart_port *uport = &sport->port;
-
-		if (sport->duart)
-			uart_add_one_port(&sbd_reg, uport);
-	}
-
-	return 0;
+	return ret;
 }
 
 /* Unload the driver.  Unregister stuff, get ready to go away.  */
 static void __exit sbd_exit(void)
 {
-	int i;
-
-	for (i = DUART_MAX_CHIP * DUART_MAX_SIDE - 1; i >= 0; i--) {
-		struct sbd_duart *duart = &sbd_duarts[i / DUART_MAX_SIDE];
-		struct sbd_port *sport = &duart->sport[i % DUART_MAX_SIDE];
-		struct uart_port *uport = &sport->port;
-
-		if (sport->duart)
-			uart_remove_one_port(&sbd_reg, uport);
-	}
-
+	platform_driver_unregister(&sbd_driver);
 	uart_unregister_driver(&sbd_reg);
 }
 

Re: [PATCH v3 2/3] serial: sb1250-duart: Fix bootconsole handover lockup

[Philippe Mathieu-Daudé]

On 6/8/26 12:54, Maciej W. Rozycki wrote:
> Calling sbd_init_port() in the course of setting up the serial device
> causes line parameters to be messed up and the transmitter disabled.
> We've been lucky in that no message is usually produced to the kernel
> log between this call and the later call to uart_set_options() in the
> course of console setup done by sbd_serial_console_init(), or the system
> would hang as the console output handler in CFE tried to access a port
> whose transmitter has been disabled and line parameters messed up.
> 
> It'll change with the next change to the driver, so fix sbd_init_port()
> such that line parameters are set for 115200n8 console operation as with
> the CFE firmware and the transmitter re-enabled after reset.
> 
> Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Cc: stable@vger.kernel.org # v6.5+
> ---
> No change from v2 (2/3),
> <https://lore.kernel.org/r/alpine.DEB.2.21.2605241623110.1450@angie.orcam.me.uk/>.
> 
> Changes from v1 (2/4),
> <https://lore.kernel.org/r/alpine.DEB.2.21.2604130338210.29980@angie.orcam.me.uk/>:
> 
> - Sanitise the change heading.
> ---
>   drivers/tty/serial/sb1250-duart.c |    7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> linux-serial-sb1250-duart-prom-console.diff
> Index: linux-macro/drivers/tty/serial/sb1250-duart.c
> ===================================================================
> --- linux-macro.orig/drivers/tty/serial/sb1250-duart.c
> +++ linux-macro/drivers/tty/serial/sb1250-duart.c
> @@ -542,14 +542,19 @@ static void sbd_init_port(struct sbd_por
>   	/* There is no DUART reset feature, so just set some sane defaults.  */
>   	write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_TX);
>   	write_sbdchn(sport, R_DUART_CMD, V_DUART_MISC_CMD_RESET_RX);
> -	write_sbdchn(sport, R_DUART_MODE_REG_1, V_DUART_BITS_PER_CHAR_8);
> +	write_sbdchn(sport, R_DUART_MODE_REG_1,
> +		     V_DUART_PARITY_MODE_NONE | V_DUART_BITS_PER_CHAR_8);
>   	write_sbdchn(sport, R_DUART_MODE_REG_2, 0);
> +	write_sbdchn(sport, R_DUART_CLK_SEL, V_DUART_BAUD_RATE(115200));
>   	write_sbdchn(sport, R_DUART_FULL_CTL,
>   		     V_DUART_INT_TIME(0) | V_DUART_SIG_FULL(15));
>   	write_sbdchn(sport, R_DUART_OPCR_X, 0);
>   	write_sbdchn(sport, R_DUART_AUXCTL_X, 0);
>   	write_sbdshr(sport, R_DUART_IMRREG((uport->line) % 2), 0);
>   
> +	/* Re-enable transmission for the initial PROM-based console.  */
> +	write_sbdchn(sport, R_DUART_CMD, M_DUART_TX_EN);
> +
>   	sport->initialised = 1;
>   }
>   
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH v3 3/3] serial: sb1250-duart: Convert to use a platform device

[Florian Fainelli]

On 8/6/26 03:54, Maciej W. Rozycki wrote:
> Prevent a crash from happening as the first serial port is initialised:
> 
>    pata-swarm: PATA interface at GenBus slot 4
>    workingset: timestamp_bits=62 max_order=18 bucket_order=0
>    Block layer SCSI generic (bsg) driver version 0.4 loaded (major 253)
>    CPU 1 Unable to handle kernel paging request at virtual address 0000000000000208, epc == ffffffff8067f8f8, ra == ffffffff80666330
>    Oops[#1]:
>    CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.19.0-dirty #27 NONE
>    $ 0   : 0000000000000000 0000000014001fe0 0000000000000020 ffffffff80666130
>    $ 4   : 0000000000000000 a800000100e6f118 ffffffff8112cbc0 0000000000000000
>    $ 8   : 0000000000000002 0000000000000000 0000000000000000 0000000001a80000
>    $12   : 0000000000000000 ffffffff809fd488 000000004ddf14dd ffffffff00000000
>    $16   : a800000100e6f000 0000000000000000 ffffffff8112c1d0 a800000100e6f000
>    $20   : 0000000000000000 00000000000004d0 0000000000000004 ffffffff8112c1d0
>    $24   : 0000000000000001 0000000000000003
>    $28   : a80000010007c000 a80000010007fcb0 00000000000000ef ffffffff80666330
>    Hi    : fffffffffffffdb9
>    Lo    : 0000000000000035
>    epc   : ffffffff8067f8f8 __dev_fwnode+0x0/0x8
>    ra    : ffffffff80666330 serial_base_ctrl_add+0xb8/0x180
>    Status: 14001fe3	KX SX UX KERNEL EXL IE
>    Cause : 80800008 (ExcCode 02)
>    BadVA : 0000000000000208
>    PrId  : 03040102 (SiByte SB1)
>    Process swapper/0 (pid: 1, threadinfo=(____ptrval____), task=(____ptrval____), tls=0000000000000000)
>    Stack : 0000000000000000 ffffffff80cd5178 ffffffff80cd0000 ffffffff8112c1c8
>            0000000000000260 ffffffff806655c4 a800000100275bc0 ffffffff8064ac88
>            004000408112c000 0000000000000002 0000000000000000 ffffffff801965d0
>            a800000100786ba0 ffffffff80cd5178 a800000100786ba0 0000000000000004
>            a800000100275bc0 0000000000000000 0000000000000000 ffffffff80cd5178
>            0000000000000000 ffffffff8112c1c8 0000000000000260 00000000000004d0
>            0000000000000004 ffffffff80bf0000 00000000000000ef ffffffff80d171dc
>            ffffffff80d17120 ffffffff80d25658 0000000000000000 ffffffff80d50000
>            ffffffff80d2f928 ffffffff80d50000 ffffffff80d25698 ffffffff80cfcecc
>            00ffffff80b84428 0000000000000000 0000000000000006 0000000000000006
>            ...
>    Call Trace:
>    [<ffffffff8067f8f8>] __dev_fwnode+0x0/0x8
>    [<ffffffff80666330>] serial_base_ctrl_add+0xb8/0x180
>    [<ffffffff806655c4>] serial_core_register_port+0x174/0x8e0
>    [<ffffffff80d171dc>] sbd_init+0xbc/0xf4
>    [<ffffffff80cfcecc>] do_one_initcall+0x64/0x150
>    [<ffffffff80cfd284>] kernel_init_freeable+0x25c/0x30c
>    [<ffffffff809ff6e4>] kernel_init+0x24/0x118
>    [<ffffffff80112d20>] ret_from_kernel_thread+0x14/0x1c
>    
>    Code: 67bd0010  03e00008  2402ffea <03e00008> dc820208  67bdffa0  ffbe0050  ffb70048  ffb60040
>    
>    ---[ end trace 0000000000000000 ]---
> 
> -- where a pointer is dereferenced that has been derived from a null
> pointer to the port's parent device.
> 
> Since no device is available with legacy probing and it's not anymore a
> preferable way to discover devices anyway, switch the driver to using a
> platform device and use it as the port's parent device.
> 
> Use platform_driver_probe() because SB1250 DUART devices are embedded
> onchip the SoC and therefore not that straightforward to remove.
> 
> An unfortunate consequence of the switch to a platform device is we now
> hand the console over from the bootconsole much later in the bootstrap.
> The CFE console handler appears good enough though to work so late and
> in particular with interrupts enabled.
> 
> Conversely, only starting the console device so late lets the port
> initialisation code always use fsleep() for transmitter draining, so
> drop the other variant since it is now dead code.
> 
> Since there is one way only remaining to reach sbd_init_port() now, drop
> the port initialisation marker as no longer needed and go through the
> channel resets unconditionally.
> 
> Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Cc: stable@vger.kernel.org # needs to use .remove_new for <= 6.10

On BCM91250E (Sentosa):

Tested-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v3 1/3] serial: sb1250-duart: Fix console message clobbering at channel resets

[Florian Fainelli]

On 8/6/26 03:54, Maciej W. Rozycki wrote:
> Ensure any characters outstanding have been sent before issuing channel
> resets so as to prevent messages issued to the bootconsole from getting
> clobbered.
> 
> Contrary to device documentation at the time the transmitter empty bit
> is set only the transmit FIFO has been drained and there is still data
> outstanding in the transmitter shift register, so wait an extra amount
> of time for that register to drain too.  This also prevents subsequent
> messages produced to the console from getting clobbered, owing to what
> seems a transmitter synchronisation issue.
> 
> When called from sbd_serial_console_init() it is too early for fsleep()
> to work and even before lpj has been calculated, and therefore neither
> udelay() works.  Therefore delay by hand, observing that roughly 170
> iterations over an Input Port Register read are sufficient for the
> transmitter to drain with the BCM1250 SoC clocked at 800MHz, so using
> 2048 iterations should give enough margin including in particular for
> the faster BCM1480 SoC clocked at 1GHz.
> 
> Fixes: b45d52797432 ("sb1250-duart.c: SB1250 DUART serial support")
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Cc: stable@vger.kernel.org # v2.6.23+

Tested-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v3 2/3] serial: sb1250-duart: Fix bootconsole handover lockup

[Florian Fainelli]

On 8/6/26 03:54, Maciej W. Rozycki wrote:
> Calling sbd_init_port() in the course of setting up the serial device
> causes line parameters to be messed up and the transmitter disabled.
> We've been lucky in that no message is usually produced to the kernel
> log between this call and the later call to uart_set_options() in the
> course of console setup done by sbd_serial_console_init(), or the system
> would hang as the console output handler in CFE tried to access a port
> whose transmitter has been disabled and line parameters messed up.
> 
> It'll change with the next change to the driver, so fix sbd_init_port()
> such that line parameters are set for 115200n8 console operation as with
> the CFE firmware and the transmitter re-enabled after reset.
> 
> Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Cc: stable@vger.kernel.org # v6.5+

Tested-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian