[PATCH v5 0/3] uart: Introduce uart driver for the Loongson family

[PATCH v5 0/3] uart: Introduce uart driver for the Loongson family

[Binbin Zhou]

Hi all:

For various reasons, I will be taking over from Haowei and continuing to
push forward with this patch set. Thanks to Haowei for his efforts so
far.

This patchset introduce a generic UART framework driver for Loongson family.
It can be found on Loongson3 series cpus, Loongson-2K series cpus and Loongson 
LS7A bridge chips.

Thanks.

------
V5:
Patch-1:
 - Since the Loongson UART is NS8250A-compatible, simply add ls2k* compatible
   to 8250.yaml.

Sorry for the DTS{i} check error that appeared in the V3 patchset. I've fixed
this issue. Thanks to Krzysztof.

Link to V4:
https://lore.kernel.org/all/cover.1757318368.git.zhoubinbin@loongson.cn/

V4:
Patch-1:
  - Rename binding name from loongson,uart.yaml to
    loongson,ls2k0500-uart.yaml;
  - Drop ls7a compatible;
  - According to the manual, ls3a and ls2k uart are the same, so merge their
    compatible.

Patch-2:
  - Format code;
  - Add the LOONGSON_UART_DLF macro definition to avoid magic numbers;
  - Simplify the code, merge flags and quirks, and remove struct
    loongson_uart_config;
  - Use DEFINE_SIMPLE_DEV_PM_OPS;
  - Drop loongson,ls7a-uart compatible.

Patch-3:
  - Add ls2k* compatible string, and ns16550a as the fallback
    compatible.

Link to V3:
https://lore.kernel.org/all/20240826024705.55474-1-zhenghaowei@loongson.cn/

Binbin Zhou (3):
  dt-bindings: serial: 8250: Add Loongson uart compatible
  serial: 8250: Add Loongson uart driver support
  LoongArch: dts: Add uart new compatible string

 .../devicetree/bindings/serial/8250.yaml      |  14 ++
 arch/loongarch/boot/dts/loongson-2k0500.dtsi  |   2 +-
 arch/loongarch/boot/dts/loongson-2k1000.dtsi  |   2 +-
 arch/loongarch/boot/dts/loongson-2k2000.dtsi  |   2 +-
 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 +
 9 files changed, 239 insertions(+), 3 deletions(-)
 create mode 100644 drivers/tty/serial/8250/8250_loongson.c


base-commit: f4abab350840d58d69814c6993736f03ac27df83
-- 
2.47.3

[PATCH v5 1/3] dt-bindings: serial: 8250: Add Loongson uart compatible

[Binbin Zhou]

The Loongson family have a mostly NS16550A-compatible UART and
High-Speed UART hardware with the exception of custom frequency divider
latch settings register.

Co-developed-by: Haowei Zheng <zhenghaowei@loongson.cn>
Signed-off-by: Haowei Zheng <zhenghaowei@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 Documentation/devicetree/bindings/serial/8250.yaml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index b243afa69a1a..167ddcbd8800 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -125,6 +125,8 @@ properties:
               - nxp,lpc1850-uart
               - opencores,uart16550-rtlsvn105
               - ti,da830-uart
+              - loongson,ls2k0500-uart
+              - loongson,ls2k1500-uart
           - const: ns16550a
       - items:
           - enum:
@@ -169,6 +171,18 @@ properties:
               - nvidia,tegra194-uart
               - nvidia,tegra234-uart
           - const: nvidia,tegra20-uart
+      - items:
+          - enum:
+              - loongson,ls2k1000-uart
+          - const: loongson,ls2k0500-uart
+          - const: ns16550a
+      - items:
+          - enum:
+              - loongson,ls3a5000-uart
+              - loongson,ls3a6000-uart
+              - loongson,ls2k2000-uart
+          - const: loongson,ls2k1500-uart
+          - const: ns16550a
 
   reg:
     maxItems: 1
-- 
2.47.3

[PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[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 <zhenghaowei@loongson.cn>
Signed-off-by: Haowei Zheng <zhenghaowei@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 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

diff --git a/drivers/tty/serial/8250/8250_loongson.c b/drivers/tty/serial/8250/8250_loongson.c
new file mode 100644
index 000000000000..a114b4e6d5c3
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_loongson.c
@@ -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);
+}
+
+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);
+}
+
+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);
+	*frac = FIELD_GET(GENMASK(7, 0), quot);
+
+	return FIELD_GET(GENMASK(15, 8), quot);
+}
+
+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;
+	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);
+
+	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);
+
+	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);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(loongson_uart_pm_ops, loongson_uart_suspend,
+				loongson_uart_resume);
+
+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 */ },
+};
+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");
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 719faf92aa8a..53efe841656f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -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 */
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index f64ef0819cd4..98236b3bec10 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -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)
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 513a0941c284..e318a3240789 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -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
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 9c007a106330..607cf060a72a 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -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
-- 
2.47.3

[PATCH v5 3/3] LoongArch: dts: Add uart new compatible string

[Binbin Zhou]

Add loongson,ls2k*-uart compatible string on uarts.

Co-developed-by: Haowei Zheng <zhenghaowei@loongson.cn>
Signed-off-by: Haowei Zheng <zhenghaowei@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 arch/loongarch/boot/dts/loongson-2k0500.dtsi | 2 +-
 arch/loongarch/boot/dts/loongson-2k1000.dtsi | 2 +-
 arch/loongarch/boot/dts/loongson-2k2000.dtsi | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/loongarch/boot/dts/loongson-2k0500.dtsi b/arch/loongarch/boot/dts/loongson-2k0500.dtsi
index 588ebc3bded4..357de4ca7555 100644
--- a/arch/loongarch/boot/dts/loongson-2k0500.dtsi
+++ b/arch/loongarch/boot/dts/loongson-2k0500.dtsi
@@ -380,7 +380,7 @@ tsensor: thermal-sensor@1fe11500 {
 		};
 
 		uart0: serial@1ff40800 {
-			compatible = "ns16550a";
+			compatible = "loongson,ls2k0500-uart", "ns16550a";
 			reg = <0x0 0x1ff40800 0x0 0x10>;
 			clock-frequency = <100000000>;
 			interrupt-parent = <&eiointc>;
diff --git a/arch/loongarch/boot/dts/loongson-2k1000.dtsi b/arch/loongarch/boot/dts/loongson-2k1000.dtsi
index d8e01e2534dd..60ab425f793f 100644
--- a/arch/loongarch/boot/dts/loongson-2k1000.dtsi
+++ b/arch/loongarch/boot/dts/loongson-2k1000.dtsi
@@ -297,7 +297,7 @@ dma-controller@1fe00c40 {
 		};
 
 		uart0: serial@1fe20000 {
-			compatible = "ns16550a";
+			compatible = "loongson,ls2k1000-uart", "loongson,ls2k0500-uart", "ns16550a";
 			reg = <0x0 0x1fe20000 0x0 0x10>;
 			clock-frequency = <125000000>;
 			interrupt-parent = <&liointc0>;
diff --git a/arch/loongarch/boot/dts/loongson-2k2000.dtsi b/arch/loongarch/boot/dts/loongson-2k2000.dtsi
index 00cc485b753b..6c77b86ee06c 100644
--- a/arch/loongarch/boot/dts/loongson-2k2000.dtsi
+++ b/arch/loongarch/boot/dts/loongson-2k2000.dtsi
@@ -250,7 +250,7 @@ i2c@1fe00130 {
 		};
 
 		uart0: serial@1fe001e0 {
-			compatible = "ns16550a";
+			compatible = "loongson,ls2k2000-uart", "loongson,ls2k1500-uart", "ns16550a";
 			reg = <0x0 0x1fe001e0 0x0 0x10>;
 			clock-frequency = <100000000>;
 			interrupt-parent = <&liointc>;
-- 
2.47.3

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Greg Kroah-Hartman]

On Wed, Sep 24, 2025 at 02:29:37PM +0800, Binbin Zhou wrote:
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -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

Re: [PATCH v5 1/3] dt-bindings: serial: 8250: Add Loongson uart compatible

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 52 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Binbin Zhou]

Hi Greg:

Thanks for your reply.

On Wed, Sep 24, 2025 at 6:22 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 24, 2025 at 02:29:37PM +0800, Binbin Zhou wrote:
> > --- a/include/uapi/linux/serial_core.h
> > +++ b/include/uapi/linux/serial_core.h
> > @@ -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:

diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 9c007a106330..5221d86d592a 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -231,6 +231,9 @@
 /* Sunplus UART */
 #define PORT_SUNPLUS   123

+/* Loongson UART */
+#define PORT_LOONGSON  124

I'll double-check and test it again in the next version.

>
> thanks,
>
> greg k-h

--
Thanks.
Binbin

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Jiri Slaby]

Hi,

On 28. 09. 25, 4:48, Binbin Zhou wrote:
> Hi Greg:
> 
> Thanks for your reply.
> 
> On Wed, Sep 24, 2025 at 6:22 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>>
>> On Wed, Sep 24, 2025 at 02:29:37PM +0800, Binbin Zhou wrote:
>>> --- a/include/uapi/linux/serial_core.h
>>> +++ b/include/uapi/linux/serial_core.h
>>> @@ -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:

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

-- 
js
suse labs

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Jiri Slaby]

On 24. 09. 25, 8:29, Binbin Zhou wrote:
> 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?

> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_loongson.c
> @@ -0,0 +1,202 @@
...
> +struct loongson_uart_data {
> +	int line;
> +	int mcr_invert;
> +	int msr_invert;

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

> +	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.

> +{
> +	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;

u8

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

thanks,
-- 
js
suse labs

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Binbin Zhou]

Hi Jiri:

Thanks for your reply.

On Mon, Sep 29, 2025 at 2:19 PM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> Hi,
>
> On 28. 09. 25, 4:48, Binbin Zhou wrote:
> > Hi Greg:
> >
> > Thanks for your reply.
> >
> > On Wed, Sep 24, 2025 at 6:22 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> >>
> >> On Wed, Sep 24, 2025 at 02:29:37PM +0800, Binbin Zhou wrote:
> >>> --- a/include/uapi/linux/serial_core.h
> >>> +++ b/include/uapi/linux/serial_core.h
> >>> @@ -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:
>
> 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:

diff --git a/drivers/tty/serial/8250/8250_port.c
b/drivers/tty/serial/8250/8250_port.c
index 719faf92aa8a..53efe841656f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -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`.
>
> --
> js
> suse labs

-- 
Thanks.
Binbin

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Binbin Zhou]

Hi Jiri:

Thanks for your review.

On Mon, Sep 29, 2025 at 2:27 PM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 24. 09. 25, 8:29, Binbin Zhou wrote:
> > 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?

Initially, we attempted to directly add Loongson UART support to the
existing 8250 driver. The critical section is as follows:

diff --git a/drivers/tty/serial/8250/8250_port.c
b/drivers/tty/serial/8250/8250_port.c
index aa4de6907f77..6754f496a95c 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -395,14 +404,36 @@ static void hub6_serial_out(struct uart_port *p,
int offset, int value)
        outb(value, p->iobase + 1);
 }

 static unsigned int mem_serial_in(struct uart_port *p, int offset)
 {
+       unsigned int val, offset0 = offset;
        offset = offset << p->regshift;
-       return readb(p->membase + offset);
+       val = readb(p->membase + offset);
+       if (p->type == PORT_LOONGSON)
+               val = serial_in_fixup(p, offset0, val);
+       return val;
 }

 static void mem_serial_out(struct uart_port *p, int offset, int value)
 {
+       if (p->type == PORT_LOONGSON)
+               value = serial_out_fixup(p, offset, value);
        offset = offset << p->regshift;
        writeb(value, p->membase + offset);
 }

This made the code difficult to read, so I concluded it was not a good
solution. Therefore, I separated it into a standalone driver.

>
> > --- /dev/null
> > +++ b/drivers/tty/serial/8250/8250_loongson.c
> > @@ -0,0 +1,202 @@
> ...
> > +struct loongson_uart_data {
> > +     int line;
> > +     int mcr_invert;
> > +     int msr_invert;
>
> These two should be unsigned. They should be u8, actually.

OK....
>
> > +     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.

OK... I will check twice.
>
> > +{
> > +     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;
>
> u8
>
> > +
> > +     val = readb(p->membase + (offset << p->regshift));
> > +
> > +     return serial_fixup(p, offset, val);
> > +}
>
> thanks,
> --
> js
> suse labs


--
Thanks.
Binbin

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Ilpo Järvinen]

On Wed, 24 Sep 2025, Binbin Zhou wrote:

> 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 <zhenghaowei@loongson.cn>
> Signed-off-by: Haowei Zheng <zhenghaowei@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  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
> 
> diff --git a/drivers/tty/serial/8250/8250_loongson.c b/drivers/tty/serial/8250/8250_loongson.c
> new file mode 100644
> index 000000000000..a114b4e6d5c3
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_loongson.c
> @@ -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);
> +}
> +
> +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()?

> +}
> +
> +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.

> +	*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().

> +}
> +
> +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;
> +	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?

> +	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.

> +	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);
> +
> +	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.

> +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.

> +};
> +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");
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 719faf92aa8a..53efe841656f 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -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,
> +	},

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

--
 i.

>  };
>  
>  /* Uart divisor latch read */
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index f64ef0819cd4..98236b3bec10 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -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)
> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 513a0941c284..e318a3240789 100644
> --- a/drivers/tty/serial/8250/Makefile
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -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
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index 9c007a106330..607cf060a72a 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -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
> 

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Ilpo Järvinen]

On Mon, 29 Sep 2025, Jiri Slaby wrote:

> On 24. 09. 25, 8:29, Binbin Zhou wrote:
> > 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?

??

Isn't it exactly what this submission did? It implements custom in/out 
functions which handle the xor for the relevant bits?

-- 
 i.

> > --- /dev/null
> > +++ b/drivers/tty/serial/8250/8250_loongson.c
> > @@ -0,0 +1,202 @@
> ...
> > +struct loongson_uart_data {
> > +	int line;
> > +	int mcr_invert;
> > +	int msr_invert;
> 
> These two should be unsigned. They should be u8, actually.
> 
> > +	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.
> 
> > +{
> > +	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;
> 
> u8
> 
> > +
> > +	val = readb(p->membase + (offset << p->regshift));
> > +
> > +	return serial_fixup(p, offset, val);
> > +}
> 
> thanks,
> 

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Greg Kroah-Hartman]

On Tue, Sep 30, 2025 at 11:03:29AM +0800, Binbin Zhou wrote:
> Hi Jiri:
> 
> Thanks for your reply.
> 
> On Mon, Sep 29, 2025 at 2:19 PM Jiri Slaby <jirislaby@kernel.org> wrote:
> >
> > Hi,
> >
> > On 28. 09. 25, 4:48, Binbin Zhou wrote:
> > > Hi Greg:
> > >
> > > Thanks for your reply.
> > >
> > > On Wed, Sep 24, 2025 at 6:22 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > >>
> > >> On Wed, Sep 24, 2025 at 02:29:37PM +0800, Binbin Zhou wrote:
> > >>> --- a/include/uapi/linux/serial_core.h
> > >>> +++ b/include/uapi/linux/serial_core.h
> > >>> @@ -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:
> >
> > 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:
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 719faf92aa8a..53efe841656f 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -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`.

Yes, but that is the same exact value as PORT_PXA, which means this code
will not really work properly, right?  You can not just redefine an
already used user/kernel value.

thanks,

greg k-h

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Binbin Zhou]

Hi Ilpo:

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

On Tue, Sep 30, 2025 at 7:58 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Wed, 24 Sep 2025, Binbin Zhou wrote:
>
> > 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 <zhenghaowei@loongson.cn>
> > Signed-off-by: Haowei Zheng <zhenghaowei@loongson.cn>
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  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
> >
> > diff --git a/drivers/tty/serial/8250/8250_loongson.c b/drivers/tty/serial/8250/8250_loongson.c
> > new file mode 100644
> > index 000000000000..a114b4e6d5c3
> > --- /dev/null
> > +++ b/drivers/tty/serial/8250/8250_loongson.c
> > @@ -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);
> > +}
> > +
> > +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.

ok. I will split serial_fixup() on a preceding line.
>
> Include for writeb()?

#include <linux/io.h>
>
> > +}
> > +
> > +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.

#include <linux/math.h>
>
> > +     *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().

#include <linux/bits.h>
>
> > +}
> > +
> > +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;
> > +     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?
>
> > +     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.

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 },
        { },
};
>
> > +     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);
> > +
> > +     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.

#include <linux/pm.h>

I will check all missed include files.
>
> > +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.

ok..
>
> > +};
> > +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");
> > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > index 719faf92aa8a..53efe841656f 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -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,
> > +     },
>
> This is exactly identical to PORT_16550A. Just use PORT_16550A instead of
> adding a new one unnecessarily.

OK. I will drop this part and use PORT_16550A in 8250_loongson driver.
>
> --
>  i.
>
> >  };
> >
> >  /* Uart divisor latch read */
> > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > index f64ef0819cd4..98236b3bec10 100644
> > --- a/drivers/tty/serial/8250/Kconfig
> > +++ b/drivers/tty/serial/8250/Kconfig
> > @@ -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)
> > diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> > index 513a0941c284..e318a3240789 100644
> > --- a/drivers/tty/serial/8250/Makefile
> > +++ b/drivers/tty/serial/8250/Makefile
> > @@ -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
> > diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> > index 9c007a106330..607cf060a72a 100644
> > --- a/include/uapi/linux/serial_core.h
> > +++ b/include/uapi/linux/serial_core.h
> > @@ -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
> >


-- 
Thanks.
Binbin

Re: [PATCH v5 2/3] serial: 8250: Add Loongson uart driver support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3608 bytes --]

On Thu, 9 Oct 2025, Binbin Zhou wrote:

> Hi Ilpo:
> 
> Sorry for the late reply and thanks for your detailed review.
> 
> On Tue, Sep 30, 2025 at 7:58 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Wed, 24 Sep 2025, Binbin Zhou wrote:
> >
> > > 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 <zhenghaowei@loongson.cn>
> > > Signed-off-by: Haowei Zheng <zhenghaowei@loongson.cn>
> > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > ---

> > > +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;

One additional thing, this could use switch/case.

I'd just do:

	...
	case UART_MSR:
		return val ^ ddata->msr_invert;
	default:
		return val;
	}

That way you don't need to use breaks.

> > > +     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.
> 
> 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;

This can and should be const struct as well as those underlying data 
structs are const too.

> };
> 
> .............
> 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 },
>         { },
> };


-- 
 i.