uart: Introduce uart driver for the Loongson family

include/uapi/linux/serial_core.h line 34

Quoted 7 lines · Binbin Zhou

include/uapi/linux/serial_core.h +1 -0

@@ -31,6 +31,7 @@
 #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
 #define PORT_RT2880  29      /* Ralink RT2880 internal UART */
 #define PORT_16550A_FSL64 30 /* Freescale 16550 UART with 64 FIFOs */
+#define PORT_LOONGSON        31      /* Loongson 16550 UART */

Why does userspace need to have this value exported?

thanks,

greg k-h

Hi Greg:

Thanks for your reply.

Quoted 11 lines · Greg Kroah-Hartman

include/uapi/linux/serial_core.h line 34

Quoted 7 lines · Binbin Zhou

include/uapi/linux/serial_core.h +1 -0

@@ -31,6 +31,7 @@
  #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
  #define PORT_RT2880  29      /* Ralink RT2880 internal UART */
  #define PORT_16550A_FSL64 30 /* Freescale 16550 UART with 64 FIFOs */
+#define PORT_LOONGSON        31      /* Loongson 16550 UART */

Why does userspace need to have this value exported?

Sorry, this was a cheap mistake.
It should follow the existing latest macro definition as follows:

Hi,

Quoted 18 lines · Binbin Zhou

Hi Greg:

Thanks for your reply.

Quoted 11 lines · Greg Kroah-Hartman

include/uapi/linux/serial_core.h line 34

Quoted 7 lines · Binbin Zhou

include/uapi/linux/serial_core.h +1 -0

@@ -31,6 +31,7 @@
  #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
  #define PORT_RT2880  29      /* Ralink RT2880 internal UART */
  #define PORT_16550A_FSL64 30 /* Freescale 16550 UART with 64 FIFOs */
+#define PORT_LOONGSON        31      /* Loongson 16550 UART */

Why does userspace need to have this value exported?

Quoted 2 lines

Sorry, this was a cheap mistake.
It should follow the existing latest macro definition as follows:

That was not the point. The point was why do you need that at all?

--
js
suse labs

Hi Jiri:

Thanks for your reply.

Quoted 22 lines · Jiri Slaby

Hi,

Quoted 18 lines · Binbin Zhou

Hi Greg:

Thanks for your reply.

Quoted 11 lines · Greg Kroah-Hartman

include/uapi/linux/serial_core.h line 34

Quoted 7 lines · Binbin Zhou

include/uapi/linux/serial_core.h +1 -0

@@ -31,6 +31,7 @@
  #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
  #define PORT_RT2880  29      /* Ralink RT2880 internal UART */
  #define PORT_16550A_FSL64 30 /* Freescale 16550 UART with 64 FIFOs */
+#define PORT_LOONGSON        31      /* Loongson 16550 UART */

Why does userspace need to have this value exported?

Quoted 4 lines

Quoted 2 lines

Sorry, this was a cheap mistake.
It should follow the existing latest macro definition as follows:

That was not the point. The point was why do you need that at all?

Emm...
We attempted to define Loongson UART as a new uart_config entry. Therefore, PORT_LOONGSON is referenced as follows:

drivers/tty/serial/8250/8250_port.c +8 -0

index 719faf92aa8a..53efe841656f 100644
@@ -310,6 +310,14 @@ static const struct serial8250_config uart_config[] = {
                .rxtrig_bytes   = {1, 8, 16, 30},
                .flags          = UART_CAP_FIFO | UART_CAP_AFE,
        },
+       [PORT_LOONGSON] = {
+               .name           = "Loongson",
+               .fifo_size      = 16,
+               .tx_loadsz      = 16,
+               .fcr            = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
+               .rxtrig_bytes   = {1, 4, 8, 14},
+               .flags          = UART_CAP_FIFO,
+       },
 };
 
 /* Uart divisor latch read */

Additionally, uart.port.type will also be assigned the value PORT_LOONGSON.

Add the driver for on-chip UART used on Loongson family chips.

The hardware is similar to 8250, but there are the following differences:
- Some chips (such as Loongson-2K2000) have added a fractional division register to obtain the required baud rate accurately, so the {get,set}_divisor callback is overridden.
- Due to hardware defects, quirk handling is required for UART_MCR/UART_MSR.

drivers/tty/serial/8250/8250_loongson.c new file +0 -0

@@ -0,0 +1,202 @@

drivers/tty/serial/8250/8250_loongson.c new file +5 -0

+{
+	struct loongson_uart_data *ddata = p->private_data;
+
+	if (offset == UART_MCR)
+		val ^= ddata->mcr_invert;

Quoted 9 lines · Binbin Zhou

Add the driver for on-chip UART used on Loongson family chips.

The hardware is similar to 8250, but there are the following differences:
- Some chips (such as Loongson-2K2000) have added a fractional division register to obtain the required baud rate accurately, so the {get,set}_divisor callback is overridden.
- Due to hardware defects, quirk handling is required for UART_MCR/UART_MSR.

So how hard would be to implement this in 8250 using existing (or new) callbacks+quirks?

Quoted 4 lines

drivers/tty/serial/8250/8250_loongson.c new file +0 -0

@@ -0,0 +1,202 @@

...

drivers/tty/serial/8250/8250_loongson.c

Quoted 4 lines

drivers/tty/serial/8250/8250_loongson.c new file +4 -0

+struct loongson_uart_data {
+     int line;
+     int mcr_invert;
+     int msr_invert;

These two should be unsigned. They should be u8, actually.

drivers/tty/serial/8250/8250_loongson.c

Quoted 5 lines

drivers/tty/serial/8250/8250_loongson.c new file +4 -0

+     struct reset_control *rst;
+};
+
+static unsigned int serial_fixup(struct uart_port *p, unsigned int offset, unsigned int val)

Both 'val' and ret type should be u8.

Quoted 6 lines

drivers/tty/serial/8250/8250_loongson.c new file +5 -0

+{
+     struct loongson_uart_data *ddata = p->private_data;
+
+     if (offset == UART_MCR)
+             val ^= ddata->mcr_invert;

drivers/tty/serial/8250/8250_loongson.c

Quoted 10 lines

drivers/tty/serial/8250/8250_loongson.c new file +10 -0

+
+     if (offset == UART_MSR)
+             val ^= ddata->msr_invert;
+
+     return val;
+}
+
+static u32 loongson_serial_in(struct uart_port *p, unsigned int offset)
+{
+     unsigned int val;

u8

drivers/tty/serial/8250/8250_loongson.c

Quoted 5 lines

drivers/tty/serial/8250/8250_loongson.c new file +5 -0

+
+     val = readb(p->membase + (offset << p->regshift));
+
+     return serial_fixup(p, offset, val);
+}

thanks,
--
js
suse labs

Quoted 9 lines · Binbin Zhou

Add the driver for on-chip UART used on Loongson family chips.

The hardware is similar to 8250, but there are the following differences:
- Some chips (such as Loongson-2K2000) have added a fractional division register to obtain the required baud rate accurately, so the {get,set}_divisor callback is overridden.
- Due to hardware defects, quirk handling is required for UART_MCR/UART_MSR.

So how hard would be to implement this in 8250 using existing (or new) callbacks+quirks?

Quoted 3 lines

drivers/tty/serial/8250/8250_loongson.c new file +0 -0

@@ -0,0 +1,202 @@

...

drivers/tty/serial/8250/8250_loongson.c line 30

Quoted 4 lines

drivers/tty/serial/8250/8250_loongson.c new file +4 -0

+struct loongson_uart_data {
+	int line;
+	int mcr_invert;
+	int msr_invert;

These two should be unsigned. They should be u8, actually.

Quoted 5 lines

drivers/tty/serial/8250/8250_loongson.c new file +4 -0

+	struct reset_control *rst;
+};
+
+static unsigned int serial_fixup(struct uart_port *p, unsigned int offset,

unsigned int val)

Both 'val' and ret type should be u8.

Quoted 5 lines

drivers/tty/serial/8250/8250_loongson.c new file +5 -0

+{
+	struct loongson_uart_data *ddata = p->private_data;
+
+	if (offset == UART_MCR)
+		val ^= ddata->mcr_invert;

drivers/tty/serial/8250/8250_loongson.c line 49

Quoted 10 lines

drivers/tty/serial/8250/8250_loongson.c new file +10 -0

+
+	if (offset == UART_MSR)
+		val ^= ddata->msr_invert;
+
+	return val;
+}
+
+static u32 loongson_serial_in(struct uart_port *p, unsigned int offset)
+{
+	unsigned int val;

u8

drivers/tty/serial/8250/8250_loongson.c line 54

Quoted 5 lines

drivers/tty/serial/8250/8250_loongson.c new file +5 -0

+
+	val = readb(p->membase + (offset << p->regshift));
+
+	return serial_fixup(p, offset, val);
+}

thanks,

Add the driver for on-chip UART used on Loongson family chips.

The hardware is similar to 8250, but there are the following differences:
- Some chips (such as Loongson-2K2000) have added a fractional division register to obtain the required baud rate accurately, so the {get,set}_divisor callback is overridden.
- Due to hardware defects, quirk handling is required for UART_MCR/UART_MSR.

Co-developed-by Haowei Zheng <[email protected]>

Signed-off-by Haowei Zheng <[email protected]>

Signed-off-by Binbin Zhou <[email protected]>

---
 drivers/tty/serial/8250/8250_loongson.c | 202 ++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_port.c     |   8 +
 drivers/tty/serial/8250/Kconfig         |  10 ++
 drivers/tty/serial/8250/Makefile        |   1 +
 include/uapi/linux/serial_core.h        |   1 +
 5 files changed, 222 insertions(+)
 create mode 100644 drivers/tty/serial/8250/8250_loongson.c

drivers/tty/serial/8250/8250_loongson.c new file +49 -0

new file mode 100644
index 000000000000..a114b4e6d5c3
@@ -0,0 +1,202 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Serial Port driver for Loongson family chips
+ *
+ * Copyright (C) 2020-2025 Loongson Technology Corporation Limited
+ */
+
+#include <linux/bitfield.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/mod_devicetable.h>
+#include <linux/reset.h>
+
+#include "8250.h"
+
+/* Divisor Latch Fraction Register */
+#define LOONGSON_UART_DLF		0x2
+
+/* Flags */
+#define LOONGSON_UART_HAS_FRAC		BIT(0)
+#define LOONGSON_UART_QUIRK_MCR		BIT(1)
+#define LOONGSON_UART_QUIRK_MSR		BIT(2)
+
+#define LS2K0500_UART_FLAG	(LOONGSON_UART_QUIRK_MCR | LOONGSON_UART_QUIRK_MSR)
+#define LS2K1500_UART_FLAG	(LOONGSON_UART_HAS_FRAC | LOONGSON_UART_QUIRK_MCR)
+
+struct loongson_uart_data {
+	int line;
+	int mcr_invert;
+	int msr_invert;
+	struct reset_control *rst;
+};
+
+static unsigned int serial_fixup(struct uart_port *p, unsigned int offset, unsigned int val)
+{
+	struct loongson_uart_data *ddata = p->private_data;
+
+	if (offset == UART_MCR)
+		val ^= ddata->mcr_invert;
+
+	if (offset == UART_MSR)
+		val ^= ddata->msr_invert;
+
+	return val;
+}
+
+static u32 loongson_serial_in(struct uart_port *p, unsigned int offset)
+{
+	unsigned int val;

drivers/tty/serial/8250/8250_loongson.c new file +39 -0

+}
+
+static void loongson_frac_set_divisor(struct uart_port *port, unsigned int baud,
+				      unsigned int quot, unsigned int quot_frac)
+{
+	struct uart_8250_port *up = up_to_u8250p(port);
+
+	serial_port_out(port, UART_LCR, up->lcr | UART_LCR_DLAB);
+	serial_dl_write(up, quot);
+	serial_port_out(port, LOONGSON_UART_DLF, quot_frac);
+}
+
+static int loongson_uart_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct uart_8250_port uart = {};
+	struct loongson_uart_data *ddata;
+	struct resource *res;
+	unsigned int flags;
+	int ret;
+
+	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+	if (!ddata)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	uart.port.irq = platform_get_irq(pdev, 0);
+	if (uart.port.irq < 0)
+		return -EINVAL;
+
+	device_property_read_u32(dev, "clock-frequency", &uart.port.uartclk);
+
+	spin_lock_init(&uart.port.lock);
+	uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_IOREMAP;
+	uart.port.iotype = UPIO_MEM;
+	uart.port.regshift = 0;

drivers/tty/serial/8250/8250_loongson.c new file +38 -0

+	ddata->rst = devm_reset_control_get_optional_shared(dev, NULL);
+	if (IS_ERR(ddata->rst))
+		return PTR_ERR(ddata->rst);
+
+	ret = reset_control_deassert(ddata->rst);
+	if (ret)
+		return ret;
+
+	ret = serial8250_register_8250_port(&uart);
+	if (ret < 0) {
+		reset_control_assert(ddata->rst);
+		return ret;
+	}
+
+	ddata->line = ret;
+	platform_set_drvdata(pdev, ddata);
+
+	return 0;
+}
+
+static void loongson_uart_remove(struct platform_device *pdev)
+{
+	struct loongson_uart_data *ddata = platform_get_drvdata(pdev);
+
+	serial8250_unregister_port(ddata->line);
+	reset_control_assert(ddata->rst);
+}
+
+static int loongson_uart_suspend(struct device *dev)
+{
+	struct loongson_uart_data *ddata = dev_get_drvdata(dev);
+
+	serial8250_suspend_port(ddata->line);
+
+	return 0;
+}
+
+static int loongson_uart_resume(struct device *dev)

drivers/tty/serial/8250/8250_loongson.c new file +18 -0

+};
+MODULE_DEVICE_TABLE(of, loongson_uart_of_ids);
+
+static struct platform_driver loongson_uart_driver = {
+	.probe = loongson_uart_probe,
+	.remove = loongson_uart_remove,
+	.driver = {
+		.name = "loongson-uart",
+		.pm = pm_ptr(&loongson_uart_pm_ops),
+		.of_match_table = loongson_uart_of_ids,
+	},
+};
+
+module_platform_driver(loongson_uart_driver);
+
+MODULE_DESCRIPTION("Loongson UART driver");
+MODULE_AUTHOR("Loongson Technology Corporation Limited.");
+MODULE_LICENSE("GPL");

drivers/tty/serial/8250/8250_port.c +0 -0

index 719faf92aa8a..53efe841656f 100644
@@ -310,6 +310,14 @@ static const struct serial8250_config uart_config[] = {
 		.rxtrig_bytes	= {1, 8, 16, 30},

drivers/tty/serial/8250/8250_port.c +0 -0

 };
 
 /* Uart divisor latch read */

drivers/tty/serial/8250/Kconfig +10 -0

index f64ef0819cd4..98236b3bec10 100644
@@ -468,6 +468,16 @@ config SERIAL_8250_OMAP_TTYO_FIXUP
 	  not booting kernel because the serial console remains silent in case
 	  they forgot to update the command line.
 
+config SERIAL_8250_LOONGSON
+	tristate "Loongson 8250 based serial port"
+	depends on SERIAL_8250
+	depends on LOONGARCH || COMPILE_TEST
+	help
+	  If you have a machine based on LoongArch CPU you can enable
+	  its onboard serial ports by enabling this option. The option
+	  is applicable to both devicetree and ACPI, say Y to this option.
+	  If unsure, say N.
+
 config SERIAL_8250_LPC18XX
 	tristate "NXP LPC18xx/43xx serial port support"
 	depends on SERIAL_8250 && OF && (ARCH_LPC18XX || COMPILE_TEST)

drivers/tty/serial/8250/Makefile +1 -0

index 513a0941c284..e318a3240789 100644
@@ -38,6 +38,7 @@ obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
 obj-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
 obj-$(CONFIG_SERIAL_8250_INGENIC)	+= 8250_ingenic.o
 obj-$(CONFIG_SERIAL_8250_IOC3)		+= 8250_ioc3.o
+obj-$(CONFIG_SERIAL_8250_LOONGSON)	+= 8250_loongson.o
 obj-$(CONFIG_SERIAL_8250_LPC18XX)	+= 8250_lpc18xx.o
 obj-$(CONFIG_SERIAL_8250_LPSS)		+= 8250_lpss.o
 obj-$(CONFIG_SERIAL_8250_MEN_MCB)	+= 8250_men_mcb.o

include/uapi/linux/serial_core.h +1 -0

index 9c007a106330..607cf060a72a 100644
@@ -31,6 +31,7 @@
 #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
 #define PORT_RT2880	29	/* Ralink RT2880 internal UART */
 #define PORT_16550A_FSL64 30	/* Freescale 16550 UART with 64 FIFOs */
+#define PORT_LOONGSON	31	/* Loongson 16550 UART */
 
 /*
  * ARM specific type numbers.  These are not currently guaranteed

Quoted 81 lines · Binbin Zhou

Add the driver for on-chip UART used on Loongson family chips.

The hardware is similar to 8250, but there are the following differences:
- Some chips (such as Loongson-2K2000) have added a fractional division register to obtain the required baud rate accurately, so the {get,set}_divisor callback is overridden.
- Due to hardware defects, quirk handling is required for UART_MCR/UART_MSR.

Co-developed-by Haowei Zheng <[email protected]>

Signed-off-by Haowei Zheng <[email protected]>

Signed-off-by Binbin Zhou <[email protected]>

---
 drivers/tty/serial/8250/8250_loongson.c | 202 ++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_port.c     |   8 +
 drivers/tty/serial/8250/Kconfig         |  10 ++
 drivers/tty/serial/8250/Makefile        |   1 +
 include/uapi/linux/serial_core.h        |   1 +
 5 files changed, 222 insertions(+)
 create mode 100644 drivers/tty/serial/8250/8250_loongson.c

drivers/tty/serial/8250/8250_loongson.c new file +53 -0

new file mode 100644
index 000000000000..a114b4e6d5c3
@@ -0,0 +1,202 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Serial Port driver for Loongson family chips
+ *
+ * Copyright (C) 2020-2025 Loongson Technology Corporation Limited
+ */
+
+#include <linux/bitfield.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/mod_devicetable.h>
+#include <linux/reset.h>
+
+#include "8250.h"
+
+/* Divisor Latch Fraction Register */
+#define LOONGSON_UART_DLF            0x2
+
+/* Flags */
+#define LOONGSON_UART_HAS_FRAC               BIT(0)
+#define LOONGSON_UART_QUIRK_MCR              BIT(1)
+#define LOONGSON_UART_QUIRK_MSR              BIT(2)
+
+#define LS2K0500_UART_FLAG   (LOONGSON_UART_QUIRK_MCR | LOONGSON_UART_QUIRK_MSR)
+#define LS2K1500_UART_FLAG   (LOONGSON_UART_HAS_FRAC | LOONGSON_UART_QUIRK_MCR)
+
+struct loongson_uart_data {
+     int line;
+     int mcr_invert;
+     int msr_invert;
+     struct reset_control *rst;
+};
+
+static unsigned int serial_fixup(struct uart_port *p, unsigned int offset, unsigned int val)
+{
+     struct loongson_uart_data *ddata = p->private_data;
+
+     if (offset == UART_MCR)
+             val ^= ddata->mcr_invert;
+
+     if (offset == UART_MSR)
+             val ^= ddata->msr_invert;
+
+     return val;
+}
+
+static u32 loongson_serial_in(struct uart_port *p, unsigned int offset)
+{
+     unsigned int val;
+
+     val = readb(p->membase + (offset << p->regshift));
+
+     return serial_fixup(p, offset, val);

drivers/tty/serial/8250/8250_loongson.c line 59

Quoted 6 lines

drivers/tty/serial/8250/8250_loongson.c new file +6 -0

+}
+
+static void loongson_serial_out(struct uart_port *p, unsigned int offset, unsigned int value)
+{
+     offset <<= p->regshift;
+     writeb(serial_fixup(p, offset, value), p->membase + offset);

This would be cleaner if you do that serial_fixup() on a preceeding line.

Include for writeb()?

drivers/tty/serial/8250/8250_loongson.c line 67

Quoted 9 lines

drivers/tty/serial/8250/8250_loongson.c new file +8 -0

+}
+
+static unsigned int loongson_frac_get_divisor(struct uart_port *port, unsigned int baud,
+                                           unsigned int *frac)
+{
+     unsigned int quot;
+
+     quot = DIV_ROUND_CLOSEST((port->uartclk << 4), baud);

Missing include.

drivers/tty/serial/8250/8250_loongson.c line 70

Quoted 4 lines

drivers/tty/serial/8250/8250_loongson.c new file +3 -0

+     *frac = FIELD_GET(GENMASK(7, 0), quot);
+
+     return FIELD_GET(GENMASK(15, 8), quot);

Please name the fields properly with defines.

You're also missing #include for GENMASK().

Quoted 40 lines

drivers/tty/serial/8250/8250_loongson.c new file +39 -0

+}
+
+static void loongson_frac_set_divisor(struct uart_port *port, unsigned int baud,
+                                   unsigned int quot, unsigned int quot_frac)
+{
+     struct uart_8250_port *up = up_to_u8250p(port);
+
+     serial_port_out(port, UART_LCR, up->lcr | UART_LCR_DLAB);
+     serial_dl_write(up, quot);
+     serial_port_out(port, LOONGSON_UART_DLF, quot_frac);
+}
+
+static int loongson_uart_probe(struct platform_device *pdev)
+{
+     struct device *dev = &pdev->dev;
+     struct uart_8250_port uart = {};
+     struct loongson_uart_data *ddata;
+     struct resource *res;
+     unsigned int flags;
+     int ret;
+
+     ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+     if (!ddata)
+             return -ENOMEM;
+
+     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+     if (!res)
+             return -ENODEV;
+
+     uart.port.irq = platform_get_irq(pdev, 0);
+     if (uart.port.irq < 0)
+             return -EINVAL;
+
+     device_property_read_u32(dev, "clock-frequency", &uart.port.uartclk);
+
+     spin_lock_init(&uart.port.lock);
+     uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_IOREMAP;
+     uart.port.iotype = UPIO_MEM;
+     uart.port.regshift = 0;

drivers/tty/serial/8250/8250_loongson.c line 119

Quoted 10 lines

drivers/tty/serial/8250/8250_loongson.c new file +10 -0

+     uart.port.dev = dev;
+     uart.port.type = PORT_LOONGSON;
+     uart.port.private_data = ddata;
+
+     uart.port.mapbase = res->start;
+     uart.port.mapsize = resource_size(res);
+     uart.port.serial_in = loongson_serial_in;
+     uart.port.serial_out = loongson_serial_out;
+
+     flags = (uintptr_t)device_get_match_data(dev);

Perhaps flags should be unsigned long?

drivers/tty/serial/8250/8250_loongson.c line 130

Quoted 10 lines

drivers/tty/serial/8250/8250_loongson.c new file +10 -0

+     if (flags & LOONGSON_UART_HAS_FRAC) {
+             uart.port.get_divisor = loongson_frac_get_divisor;
+             uart.port.set_divisor = loongson_frac_set_divisor;
+     }
+
+     if (flags & LOONGSON_UART_QUIRK_MCR)
+             ddata->mcr_invert |= (UART_MCR_RTS | UART_MCR_DTR);
+
+     if (flags & LOONGSON_UART_QUIRK_MSR)
+             ddata->msr_invert |= (UART_MSR_CTS | UART_MSR_DSR);

I think it would be better to put these invert masks directly into a struct which is then put into .data. LOONGSON_UART_HAS_FRAC can be bool in that struct.

Quoted 44 lines

drivers/tty/serial/8250/8250_loongson.c new file +43 -0

+     ddata->rst = devm_reset_control_get_optional_shared(dev, NULL);
+     if (IS_ERR(ddata->rst))
+             return PTR_ERR(ddata->rst);
+
+     ret = reset_control_deassert(ddata->rst);
+     if (ret)
+             return ret;
+
+     ret = serial8250_register_8250_port(&uart);
+     if (ret < 0) {
+             reset_control_assert(ddata->rst);
+             return ret;
+     }
+
+     ddata->line = ret;
+     platform_set_drvdata(pdev, ddata);
+
+     return 0;
+}
+
+static void loongson_uart_remove(struct platform_device *pdev)
+{
+     struct loongson_uart_data *ddata = platform_get_drvdata(pdev);
+
+     serial8250_unregister_port(ddata->line);
+     reset_control_assert(ddata->rst);
+}
+
+static int loongson_uart_suspend(struct device *dev)
+{
+     struct loongson_uart_data *ddata = dev_get_drvdata(dev);
+
+     serial8250_suspend_port(ddata->line);
+
+     return 0;
+}
+
+static int loongson_uart_resume(struct device *dev)
+{
+     struct loongson_uart_data *data = dev_get_drvdata(dev);
+
+     serial8250_resume_port(data->line);
+

drivers/tty/serial/8250/8250_loongson.c line 179

Quoted 5 lines

drivers/tty/serial/8250/8250_loongson.c new file +5 -0

+     return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(loongson_uart_pm_ops, loongson_uart_suspend,
+                             loongson_uart_resume);

Include? Please check if you miss even more includes beyond those I've highlighted here.

drivers/tty/serial/8250/8250_loongson.c line 184

Quoted 5 lines

drivers/tty/serial/8250/8250_loongson.c new file +4 -0

+static const struct of_device_id loongson_uart_of_ids[] = {
+     { .compatible = "loongson,ls2k0500-uart", .data = (void *)LS2K0500_UART_FLAG },
+     { .compatible = "loongson,ls2k1500-uart", .data = (void *)LS2K1500_UART_FLAG },
+     { /* sentinel */ },

Nit, no need for comma in the terminator entry.

Quoted 30 lines

drivers/tty/serial/8250/8250_loongson.c new file +18 -0

+};
+MODULE_DEVICE_TABLE(of, loongson_uart_of_ids);
+
+static struct platform_driver loongson_uart_driver = {
+     .probe = loongson_uart_probe,
+     .remove = loongson_uart_remove,
+     .driver = {
+             .name = "loongson-uart",
+             .pm = pm_ptr(&loongson_uart_pm_ops),
+             .of_match_table = loongson_uart_of_ids,
+     },
+};
+
+module_platform_driver(loongson_uart_driver);
+
+MODULE_DESCRIPTION("Loongson UART driver");
+MODULE_AUTHOR("Loongson Technology Corporation Limited.");
+MODULE_LICENSE("GPL");

drivers/tty/serial/8250/8250_port.c +3 -0

index 719faf92aa8a..53efe841656f 100644
@@ -310,6 +310,14 @@ static const struct serial8250_config uart_config[] = {
              .rxtrig_bytes   = {1, 8, 16, 30},
              .flags          = UART_CAP_FIFO | UART_CAP_AFE,
      },
+     [PORT_LOONGSON] = {
+             .name           = "Loongson",
+             .fifo_size      = 16,

drivers/tty/serial/8250/8250_port.c static const struct serial8250_config uart_config[] = { · line 320

Quoted 5 lines

drivers/tty/serial/8250/8250_port.c +5 -0

+             .tx_loadsz      = 16,
+             .fcr            = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
+             .rxtrig_bytes   = {1, 4, 8, 14},
+             .flags          = UART_CAP_FIFO,
+     },

This is exactly identical to PORT_16550A. Just use PORT_16550A instead of adding a new one unnecessarily.

--
i.

Quoted 49 lines

drivers/tty/serial/8250/8250_port.c +0 -0

 };
 
 /* Uart divisor latch read */

drivers/tty/serial/8250/Kconfig +10 -0

index f64ef0819cd4..98236b3bec10 100644
@@ -468,6 +468,16 @@ config SERIAL_8250_OMAP_TTYO_FIXUP
        not booting kernel because the serial console remains silent in case
        they forgot to update the command line.
 
+config SERIAL_8250_LOONGSON
+     tristate "Loongson 8250 based serial port"
+     depends on SERIAL_8250
+     depends on LOONGARCH || COMPILE_TEST
+     help
+       If you have a machine based on LoongArch CPU you can enable
+       its onboard serial ports by enabling this option. The option
+       is applicable to both devicetree and ACPI, say Y to this option.
+       If unsure, say N.
+
 config SERIAL_8250_LPC18XX
      tristate "NXP LPC18xx/43xx serial port support"
      depends on SERIAL_8250 && OF && (ARCH_LPC18XX || COMPILE_TEST)

drivers/tty/serial/8250/Makefile +1 -0

index 513a0941c284..e318a3240789 100644
@@ -38,6 +38,7 @@ obj-$(CONFIG_SERIAL_8250_HP300)             += 8250_hp300.o
 obj-$(CONFIG_SERIAL_8250_HUB6)               += 8250_hub6.o
 obj-$(CONFIG_SERIAL_8250_INGENIC)    += 8250_ingenic.o
 obj-$(CONFIG_SERIAL_8250_IOC3)               += 8250_ioc3.o
+obj-$(CONFIG_SERIAL_8250_LOONGSON)   += 8250_loongson.o
 obj-$(CONFIG_SERIAL_8250_LPC18XX)    += 8250_lpc18xx.o
 obj-$(CONFIG_SERIAL_8250_LPSS)               += 8250_lpss.o
 obj-$(CONFIG_SERIAL_8250_MEN_MCB)    += 8250_men_mcb.o

include/uapi/linux/serial_core.h +1 -0

index 9c007a106330..607cf060a72a 100644
@@ -31,6 +31,7 @@
 #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
 #define PORT_RT2880  29      /* Ralink RT2880 internal UART */
 #define PORT_16550A_FSL64 30 /* Freescale 16550 UART with 64 FIFOs */
+#define PORT_LOONGSON        31      /* Loongson 16550 UART */
 
 /*
  * ARM specific type numbers.  These are not currently guaranteed

Hi Ilpo:

Sorry for the late reply and thanks for your detailed review.

Quoted 17 lines · Ilpo Järvinen

Quoted 14 lines · Binbin Zhou

Add the driver for on-chip UART used on Loongson family chips.

The hardware is similar to 8250, but there are the following differences:
- Some chips (such as Loongson-2K2000) have added a fractional division register to obtain the required baud rate accurately, so the {get,set}_divisor callback is overridden.
- Due to hardware defects, quirk handling is required for UART_MCR/UART_MSR.

Co-developed-by Haowei Zheng <[email protected]>

Signed-off-by Haowei Zheng <[email protected]>

Signed-off-by Binbin Zhou <[email protected]>

---

Quoted 9 lines

drivers/tty/serial/8250/8250_loongson.c line 42

Quoted 9 lines

drivers/tty/serial/8250/8250_loongson.c new file +9 -0

+static unsigned int serial_fixup(struct uart_port *p, unsigned int offset, unsigned int val)
+{
+     struct loongson_uart_data *ddata = p->private_data;
+
+     if (offset == UART_MCR)
+             val ^= ddata->mcr_invert;
+
+     if (offset == UART_MSR)
+             val ^= ddata->msr_invert;

Quoted 6 lines

Quoted 6 lines

drivers/tty/serial/8250/8250_loongson.c new file +6 -0

+     if (flags & LOONGSON_UART_HAS_FRAC) {
+             uart.port.get_divisor = loongson_frac_get_divisor;
+             uart.port.set_divisor = loongson_frac_set_divisor;
+     }
+
+     if (flags & LOONGSON_UART_QUIRK_MCR)

Quoted 8 lines

drivers/tty/serial/8250/8250_loongson.c line 130

Quoted 4 lines

drivers/tty/serial/8250/8250_loongson.c new file +4 -0

+             ddata->mcr_invert |= (UART_MCR_RTS | UART_MCR_DTR);
+
+     if (flags & LOONGSON_UART_QUIRK_MSR)
+             ddata->msr_invert |= (UART_MSR_CTS | UART_MSR_DSR);

I think it would be better to put these invert masks directly into a struct which is then put into .data. LOONGSON_UART_HAS_FRAC can be bool in that struct.

I attempted the following refactoring:

struct loongson_uart_ddata {
        bool has_frac;
        u8 mcr_invert;
        u8 msr_invert;
};

static const struct loongson_uart_ddata ls2k0500_uart_data {
.has_frac = false,
.mcr_invert = UART_MCR_RTS | UART_MCR_DTR, .msr_invert = UART_MSR_CTS | UART_MSR_DSR,
};

static const struct loongson_uart_ddata ls2k1500_uart_data {
.has_frac = true,
.mcr_invert = UART_MCR_RTS | UART_MCR_DTR, .msr_invert = 0,
};

struct loongson_uart_priv {
        int line;
        struct clk *clk;
        struct resource *res;
        struct reset_control *rst;
        struct loongson_uart_ddata *ddata;

};

.............
In loongson_uart_probe():

       priv->ddata = device_get_match_data(dev);

        if (priv->ddata->has_frac) {
                port->get_divisor = loongson_frac_get_divisor;
                port->set_divisor = loongson_frac_set_divisor;
        }

.............

static const struct of_device_id loongson_uart_of_ids[] = {
        { .compatible = "loongson,ls2k0500-uart", .data = ls2k0500_uart_data },
        { .compatible = "loongson,ls2k1500-uart", .data = ls2k1500_uart_data },
        { },
};