[PATCH v2 0/3] serial: 8250_mxpcie: probe and console fixes

[PATCH v2 0/3] serial: 8250_mxpcie: probe and console fixes

[Linmao Li]

Three fixes for the Moxa PCIe serial driver, found by reading the code
that was split out of 8250_pci.c and extended during the 7.3 merge
window.  That code is in mainline now but has not been in a released
kernel, so the series is against tty-linus.

Patch 1 fixes a NULL pointer dereference during probe: the serial core
calls ->rs485_config() from within serial8250_register_8250_port(),
before pci_set_drvdata() has published the board.  It takes a board
without RS232 support and SER_RS485_ENABLED still set once
uart_get_rs485_mode() has run - on Crescent's ACPI machine the firmware
node clears that flag, which is why it does not crash there.

Patch 2 makes removal unregister only the ports that were registered.
v1 failed the probe instead; as Andy pointed out that is a behavioural
change, and keeping the ports that do work is what pciserial_init_ports()
in 8250_pci.c does, so v2 keeps it and restores only the missing count.

Patch 3 stops set_termios() from dereferencing port->state->port.tty,
which is NULL when the serial core sets the line up for a console or for
kgdboc.

I have no Moxa board here.  Crescent Hsieh tested patch 3 with kgdboc on
a CP-168EL-A, and looked for patch 1's crash on a CP-134EL-A; the rest is
compile-tested only, W=1 allmodconfig build of drivers/tty/serial/8250/
and checkpatch --strict, both clean.

v2:
- 1/3: no code change; the commit message now says the crash needs
  SER_RS485_ENABLED to survive uart_get_rs485_mode() (Crescent Hsieh).
- 2/3: keep the ports that registered successfully, record how many and
  unregister only those, instead of failing the probe (Andy Shevchenko).
  Retitled accordingly.
- 3/3: drop the cflag local and read new->c_cflag directly, and pick up
  Crescent Hsieh's Tested-by.

Link to v1:
https://lore.kernel.org/all/20260818093918.3190686-1-lilinmao@kylinos.cn/

Linmao Li (3):
  serial: 8250_mxpcie: set the driver data before registering ports
  serial: 8250_mxpcie: only unregister the ports that were registered
  serial: 8250_mxpcie: take the line settings from the new termios

 drivers/tty/serial/8250/8250_mxpcie.c | 30 +++++++++++++++------------
 1 file changed, 17 insertions(+), 13 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.25.1

[PATCH v2 1/3] serial: 8250_mxpcie: set the driver data before registering ports

[Linmao Li]

mxpcie8250_rs485_config() looks the board up with dev_get_drvdata() on
the PCI device, but pci_set_drvdata() only runs after the registration
loop.  Where mxpcie8250_setup_port() presets rs485.flags to
SER_RS485_ENABLED and the flag survives uart_get_rs485_mode() - the
device has no firmware node, or its node sets
linux,rs485-enabled-at-boot-time - uart_configure_port() calls
->rs485_config() from inside serial8250_register_8250_port(), and the
callback dereferences a NULL board pointer.

Publish the driver data before the first port is registered.

Fixes: d21a1509c623 ("serial: 8250_mxpcie: support serial interface mode switching")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
v2: no code change; the commit message now says the crash needs
    SER_RS485_ENABLED to survive uart_get_rs485_mode() (Crescent
    Hsieh).

 drivers/tty/serial/8250/8250_mxpcie.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_mxpcie.c b/drivers/tty/serial/8250/8250_mxpcie.c
index ce873fbd62e9..c0e3517d4e4e 100644
--- a/drivers/tty/serial/8250/8250_mxpcie.c
+++ b/drivers/tty/serial/8250/8250_mxpcie.c
@@ -542,6 +542,8 @@ static int mxpcie8250_probe(struct pci_dev *pdev, const struct pci_device_id *id
 
 	mxpcie8250_init_board(pdev, priv);
 
+	pci_set_drvdata(pdev, priv);
+
 	up.port.dev = dev;
 	up.port.irq = pdev->irq;
 	up.port.uartclk = MOXA_PUART_BASE_BAUD * 16;
@@ -578,7 +580,6 @@ static int mxpcie8250_probe(struct pci_dev *pdev, const struct pci_device_id *id
 		}
 		priv->port[i].rx_trig_level = MOXA_PUART_RX_TRIG_DEFAULT;
 	}
-	pci_set_drvdata(pdev, priv);
 
 	return 0;
 }
-- 
2.25.1

[PATCH v2 2/3] serial: 8250_mxpcie: only unregister the ports that were registered

[Linmao Li]

When serial8250_register_8250_port() fails the loop stops and the probe
keeps the ports registered so far, like pciserial_init_ports() in
8250_pci.c this driver was split from.  What the split lost is that
function's priv->nr: mxpcie8250_remove() walks all num_ports entries,
but the ones the loop never reached keep the zero devm_kzalloc() left
there, and the one that failed keeps a negative error code.

serial8250_unregister_port() checks neither, so removal unregisters
line 0 - a port this driver does not own - and indexes
serial8250_ports[] with a negative line number.

Record how many ports were registered and unregister only those.

Fixes: 0481a041e956 ("serial: 8250: split Moxa PCIe serial board support out of 8250_pci")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
v2: keep the ports that registered successfully and restore only the
    missing count; v1 failed the probe instead (Andy Shevchenko).

 drivers/tty/serial/8250/8250_mxpcie.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mxpcie.c b/drivers/tty/serial/8250/8250_mxpcie.c
index c0e3517d4e4e..ef2516ec16da 100644
--- a/drivers/tty/serial/8250/8250_mxpcie.c
+++ b/drivers/tty/serial/8250/8250_mxpcie.c
@@ -109,6 +109,7 @@ struct mxpcie8250_port {
 struct mxpcie8250 {
 	unsigned int supp_rs;
 	unsigned int num_ports;
+	unsigned int nr;	/* ports actually registered */
 	void __iomem *bar1_base; /* UART registers (MMIO) */
 	void __iomem *bar2_base; /* UIR / GPIO / CPLD (IO) */
 	struct mxpcie8250_port port[] __counted_by(num_ports);
@@ -517,6 +518,7 @@ static int mxpcie8250_probe(struct pci_dev *pdev, const struct pci_device_id *id
 	struct mxpcie8250 *priv;
 	unsigned short device = pdev->device;
 	unsigned int num_ports;
+	unsigned int i;
 	int ret;
 
 	ret = pcim_enable_device(pdev);
@@ -564,7 +566,7 @@ static int mxpcie8250_probe(struct pci_dev *pdev, const struct pci_device_id *id
 	up.port.handle_irq = mxpcie8250_handle_irq;
 	up.port.break_ctl = mxpcie8250_break_ctl;
 
-	for (unsigned int i = 0; i < num_ports; i++) {
+	for (i = 0; i < num_ports; i++) {
 		mxpcie8250_setup_port(pdev, priv, &up, i);
 
 		dev_dbg(dev, "Setup PCI port: port %lx, irq %d, type %d\n",
@@ -580,6 +582,7 @@ static int mxpcie8250_probe(struct pci_dev *pdev, const struct pci_device_id *id
 		}
 		priv->port[i].rx_trig_level = MOXA_PUART_RX_TRIG_DEFAULT;
 	}
+	priv->nr = i;
 
 	return 0;
 }
@@ -588,7 +591,7 @@ static void mxpcie8250_remove(struct pci_dev *pdev)
 {
 	struct mxpcie8250 *priv = pci_get_drvdata(pdev);
 
-	for (unsigned int i = 0; i < priv->num_ports; i++)
+	for (unsigned int i = 0; i < priv->nr; i++)
 		serial8250_unregister_port(priv->port[i].line);
 }
 
-- 
2.25.1

Re: [PATCH v2 2/3] serial: 8250_mxpcie: only unregister the ports that were registered

[Andy Shevchenko]

On Tue, Sep 01, 2026 at 12:15:18PM +0800, Linmao Li wrote:
> When serial8250_register_8250_port() fails the loop stops and the probe
> keeps the ports registered so far, like pciserial_init_ports() in
> 8250_pci.c this driver was split from.  What the split lost is that
> function's priv->nr: mxpcie8250_remove() walks all num_ports entries,
> but the ones the loop never reached keep the zero devm_kzalloc() left
> there, and the one that failed keeps a negative error code.
> 
> serial8250_unregister_port() checks neither, so removal unregisters
> line 0 - a port this driver does not own - and indexes
> serial8250_ports[] with a negative line number.
> 
> Record how many ports were registered and unregister only those.

...

>  struct mxpcie8250 {

>  	unsigned int supp_rs;
>  	unsigned int num_ports;
> +	unsigned int nr;	/* ports actually registered */

I would make it indented with the below comments.
But no need to resend for this. It's not critical at all.

>  	void __iomem *bar1_base; /* UART registers (MMIO) */
>  	void __iomem *bar2_base; /* UIR / GPIO / CPLD (IO) */
>  	struct mxpcie8250_port port[] __counted_by(num_ports);

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 3/3] serial: 8250_mxpcie: take the line settings from the new termios

[Linmao Li]

mxpcie8250_set_termios() reads the line settings out of
port->state->port.tty, which is only set once the port has been opened.

uart_set_options() builds a termios of its own and calls ->set_termios()
with no tty behind it, so using such a board as the console
(console=ttyS<n>) dereferences a NULL tty during console setup, as does
attaching kgdboc to it and resuming a suspended console from
uart_resume_port().

Read the settings from the termios the serial core passes in instead.
It holds the same values on the normal path - uart_change_line_settings()
passes &tty->termios - and it is what serial8250_do_set_termios() right
above already uses.

Fixes: 55edf8511f47 ("serial: 8250_mxpcie: enable automatic RTS/CTS flow control")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
Tested-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
---
v2: drop the cflag local, read new->c_cflag directly (Crescent Hsieh).

 drivers/tty/serial/8250/8250_mxpcie.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mxpcie.c b/drivers/tty/serial/8250/8250_mxpcie.c
index ef2516ec16da..19f1f51eec47 100644
--- a/drivers/tty/serial/8250/8250_mxpcie.c
+++ b/drivers/tty/serial/8250/8250_mxpcie.c
@@ -218,8 +218,6 @@ static void mxpcie8250_set_termios(struct uart_port *port,
 				   const struct ktermios *old)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
-	struct tty_struct *tty = port->state->port.tty;
-	unsigned int cflag = tty->termios.c_cflag;
 	u8 efr, val;
 
 	serial8250_do_set_termios(port, new, old);
@@ -229,23 +227,25 @@ static void mxpcie8250_set_termios(struct uart_port *port,
 	efr = serial_in(up, MOXA_PUART_EFR);
 	efr &= ~(MOXA_PUART_EFR_AUTO_RTS | MOXA_PUART_EFR_AUTO_CTS);
 
-	if (cflag & CRTSCTS) {
+	if (new->c_cflag & CRTSCTS) {
 		efr |= (MOXA_PUART_EFR_AUTO_RTS | MOXA_PUART_EFR_AUTO_CTS);
 		up->port.status |= (UPSTAT_AUTORTS | UPSTAT_AUTOCTS);
 	}
 	/* Set on-chip software flow control character */
-	serial_out(up, MOXA_PUART_XON1, START_CHAR(tty));
-	serial_out(up, MOXA_PUART_XON2, START_CHAR(tty));
-	serial_out(up, MOXA_PUART_XOFF1, STOP_CHAR(tty));
-	serial_out(up, MOXA_PUART_XOFF2, STOP_CHAR(tty));
+	serial_out(up, MOXA_PUART_XON1, new->c_cc[VSTART]);
+	serial_out(up, MOXA_PUART_XON2, new->c_cc[VSTART]);
+	serial_out(up, MOXA_PUART_XOFF1, new->c_cc[VSTOP]);
+	serial_out(up, MOXA_PUART_XOFF2, new->c_cc[VSTOP]);
 
-	val = I_IXON(tty) ? MOXA_PUART_EFR_RX_FLOW_XON1_XOFF1 : MOXA_PUART_EFR_RX_FLOW_DISABLED;
+	val = (new->c_iflag & IXON) ? MOXA_PUART_EFR_RX_FLOW_XON1_XOFF1 :
+				      MOXA_PUART_EFR_RX_FLOW_DISABLED;
 	FIELD_MODIFY(MOXA_PUART_EFR_RX_FLOW_MASK, &efr, val);
 
-	val = I_IXOFF(tty) ? MOXA_PUART_EFR_TX_FLOW_XON1_XOFF1 : MOXA_PUART_EFR_TX_FLOW_DISABLED;
+	val = (new->c_iflag & IXOFF) ? MOXA_PUART_EFR_TX_FLOW_XON1_XOFF1 :
+				       MOXA_PUART_EFR_TX_FLOW_DISABLED;
 	FIELD_MODIFY(MOXA_PUART_EFR_TX_FLOW_MASK, &efr, val);
 
-	if (I_IXOFF(tty))
+	if (new->c_iflag & IXOFF)
 		up->port.status |= UPSTAT_AUTOXOFF;
 
 	serial_out(up, MOXA_PUART_EFR, efr);
-- 
2.25.1

Re: [PATCH v2 0/3] serial: 8250_mxpcie: probe and console fixes

[Andy Shevchenko]

On Tue, Sep 01, 2026 at 12:15:16PM +0800, Linmao Li wrote:
> Three fixes for the Moxa PCIe serial driver, found by reading the code
> that was split out of 8250_pci.c and extended during the 7.3 merge
> window.  That code is in mainline now but has not been in a released
> kernel, so the series is against tty-linus.
> 
> Patch 1 fixes a NULL pointer dereference during probe: the serial core
> calls ->rs485_config() from within serial8250_register_8250_port(),
> before pci_set_drvdata() has published the board.  It takes a board
> without RS232 support and SER_RS485_ENABLED still set once
> uart_get_rs485_mode() has run - on Crescent's ACPI machine the firmware
> node clears that flag, which is why it does not crash there.
> 
> Patch 2 makes removal unregister only the ports that were registered.
> v1 failed the probe instead; as Andy pointed out that is a behavioural
> change, and keeping the ports that do work is what pciserial_init_ports()
> in 8250_pci.c does, so v2 keeps it and restores only the missing count.
> 
> Patch 3 stops set_termios() from dereferencing port->state->port.tty,
> which is NULL when the serial core sets the line up for a console or for
> kgdboc.
> 
> I have no Moxa board here.  Crescent Hsieh tested patch 3 with kgdboc on
> a CP-168EL-A, and looked for patch 1's crash on a CP-134EL-A; the rest is
> compile-tested only, W=1 allmodconfig build of drivers/tty/serial/8250/
> and checkpatch --strict, both clean.

LGTM
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] serial: 8250_mxpcie: probe and console fixes

[Crescent Hsieh]

On Tue, Sep 01, 2026 at 12:15:16PM +0800, Linmao Li wrote:
> Three fixes for the Moxa PCIe serial driver, found by reading the code
> that was split out of 8250_pci.c and extended during the 7.3 merge
> window.  That code is in mainline now but has not been in a released
> kernel, so the series is against tty-linus.
> 
> Patch 1 fixes a NULL pointer dereference during probe: the serial core
> calls ->rs485_config() from within serial8250_register_8250_port(),
> before pci_set_drvdata() has published the board.  It takes a board
> without RS232 support and SER_RS485_ENABLED still set once
> uart_get_rs485_mode() has run - on Crescent's ACPI machine the firmware
> node clears that flag, which is why it does not crash there.
> 
> Patch 2 makes removal unregister only the ports that were registered.
> v1 failed the probe instead; as Andy pointed out that is a behavioural
> change, and keeping the ports that do work is what pciserial_init_ports()
> in 8250_pci.c does, so v2 keeps it and restores only the missing count.
> 
> Patch 3 stops set_termios() from dereferencing port->state->port.tty,
> which is NULL when the serial core sets the line up for a console or for
> kgdboc.
> 
> I have no Moxa board here.  Crescent Hsieh tested patch 3 with kgdboc on
> a CP-168EL-A, and looked for patch 1's crash on a CP-134EL-A; the rest is
> compile-tested only, W=1 allmodconfig build of drivers/tty/serial/8250/
> and checkpatch --strict, both clean.

The series looks good to me.

For the series:

Reviewed-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>

The additional pre-existing issues identified by Sashiko are independent
of this series. I will investigate them further and determine the
appropriate follow-up fixes.

--
Thanks,
Crescent Hsieh