[PATCH 0/7] Introduce PRU UART driver

[PATCH 0/7] Introduce PRU UART driver

[Judith Mendez]

The PRU_ICSSG subsystems in am64x SoC, the PRU subsystem in am62 SoC, and
PRU_ICSS subsystem in am335x SoC include a UART sub-module. This patch
series introduces the driver and the corresponding binding documentation
for this UART sub-module.

The DTS patches for adding PRU UART nodes and enabling PRU UART is added
in this v1 version, but marked as DONOTMERGE since the patches only add
context to this series.

This driver version has been tested on the following boards: am64x SK and
am62x SK.

The RFC version of this driver has been previously tested on am335x SK as
well. DTS patches for enabling PRU UART for am335x SK will be sent as a
separate series once this series is merged.

Changes since RFC:
- Add DTS patches 3-6
- Fix include list
- Switch to platform_get_resource & uart_read_port_properties
- Remove custom speed hack in pruss8250_get_divisor
- Use port->serial_out functions provided by core driver instead of
  local writel() functions
- Switch to UPIO_MEM32 since largest UART register is 18 bits in length
- Cleanup whitspace, comments, variable/structure names, error paths
  and GPL licensing

Link to RFC:
https://lore.kernel.org/all/20250501003113.1609342-1-jm@ti.com/

Bin Liu (2):
  dt-bindings: serial: add binding documentation for TI PRUSS UART
  serial: 8250: Add PRUSS UART driver

Judith Mendez (5):
  dt-bindings: soc: ti: pruss: Add documentation for PRU UART support
  DONOTMERGE: arm64: dts: ti: k3-am64-main: Add PRU UART nodes
  DONOTMERGE: arm64: dts: ti: k3-am642-sk: Enable PRU UART
  DONOTMERGE: arm64: dts: ti: k3-am62-main: Add PRU UART node
  DONOTMERGE: arm64: dts: ti: k3-am62x-sk: Enable PRU UART

 .../bindings/serial/ti,pruss-uart.yaml        |  54 ++++++
 .../devicetree/bindings/soc/ti/ti,pruss.yaml  |   7 +
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi      |   9 +
 .../arm64/boot/dts/ti/k3-am62x-sk-common.dtsi |  14 ++
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi      |  18 ++
 arch/arm64/boot/dts/ti/k3-am642-sk.dts        |  16 ++
 drivers/tty/serial/8250/8250_pruss.c          | 178 ++++++++++++++++++
 drivers/tty/serial/8250/Kconfig               |  11 ++
 drivers/tty/serial/8250/Makefile              |   1 +
 9 files changed, 308 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/ti,pruss-uart.yaml
 create mode 100644 drivers/tty/serial/8250/8250_pruss.c


base-commit: edef457004774e598fc4c1b7d1d4f0bcd9d0bb30
-- 
2.49.0

[PATCH 1/7] dt-bindings: serial: add binding documentation for TI PRUSS UART

[Judith Mendez]

From: Bin Liu <b-liu@ti.com>

This adds the YAML DT binding for PRUSS UART on TI SoCs.

Signed-off-by: Bin Liu <b-liu@ti.com>
Signed-off-by: Judith Mendez <jm@ti.com>
---
 .../bindings/serial/ti,pruss-uart.yaml        | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/ti,pruss-uart.yaml

diff --git a/Documentation/devicetree/bindings/serial/ti,pruss-uart.yaml b/Documentation/devicetree/bindings/serial/ti,pruss-uart.yaml
new file mode 100644
index 000000000000..34a03d572333
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/ti,pruss-uart.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/ti,pruss-uart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI PRUSS serial UART
+
+maintainers:
+  - Bin Liu <b-liu@ti.com>
+
+description: |
+  The PRU subsystem has a serial UART peripheral based on the industry
+  standard TL16C550, with 16-byte TX/RX FIFOs.
+
+allOf:
+  - $ref: /schemas/serial.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: ti,pruss-uart
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description: |
+      PRU UART interrupt mappings, containing an entry of 3 cell-values.
+      The first is the PRU System Event ID for PRU UART Interrupt Request.
+      The second is the PRU interrupt channel ID.
+      The third is the PRU host interrupt ID.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    pruss_uart: serial@28000 {
+        compatible = "ti,pruss-uart";
+        reg = <0x28000 0x40>;
+        clocks = <&k3_clks 81 13>;
+        interrupt-parent = <&pruss_intc>;
+        interrupts = <6 4 4>;
+    };
-- 
2.49.0

[PATCH 2/7] dt-bindings: soc: ti: pruss: Add documentation for PRU UART support

[Judith Mendez]

Add documentation for PRU UART node which is for PRU serial UART
based-off the industry standard TL16C550 asynchronous communications
element.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
index 927b3200e29e..54397297cbf5 100644
--- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
+++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
@@ -324,6 +324,13 @@ patternProperties:
     $ref: /schemas/net/ti,davinci-mdio.yaml#
     type: object
 
+  serial@[a-f0-9]+$:
+    description: |
+      Serial UART sub-module. A PRUSS can have a serial UART sub-module based
+      on the industry standard TL16C550 asynchronous communications element.
+    $ref: /schemas/serial/ti,pruss-uart.yaml#
+    type: object
+
   "^(pru|rtu|txpru)@[0-9a-f]+$":
     description: |
       PRU Node. Each PRUSS has dual PRU cores, each represented as a RemoteProc
-- 
2.49.0

[PATCH 3/7] serial: 8250: Add PRUSS UART driver

[Judith Mendez]

From: Bin Liu <b-liu@ti.com>

This adds a new serial 8250 driver that supports the UART in PRUSS or
PRU_ICSS*.

The UART sub-module is based on the industry standard TL16C550 UART
controller, which has 16-bytes FIFO and supports 16x and 13x over
samplings.

Signed-off-by: Bin Liu <b-liu@ti.com>
Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/tty/serial/8250/8250_pruss.c | 178 +++++++++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig      |  11 ++
 drivers/tty/serial/8250/Makefile     |   1 +
 3 files changed, 190 insertions(+)
 create mode 100644 drivers/tty/serial/8250/8250_pruss.c

diff --git a/drivers/tty/serial/8250/8250_pruss.c b/drivers/tty/serial/8250/8250_pruss.c
new file mode 100644
index 000000000000..04a455580022
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_pruss.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  Serial Port driver for PRUSS UART on TI platforms
+ *
+ *  Copyright (C) 2025 by Texas Instruments Incorporated - http://www.ti.com/
+ *  Author: Bin Liu <b-liu@ti.com>
+ */
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/serial_reg.h>
+#include <linux/serial_core.h>
+
+#include "8250.h"
+
+/* PowerManagement and Emulation */
+#define PRUSS_UART_PEREMU_MGMT	12
+#define PRUSS_UART_TX_EN	BIT(14)
+#define PRUSS_UART_RX_EN	BIT(13)
+#define PRUSS_UART_FREE_RUN	BIT(0)
+
+/* Oversampling Mode Select */
+#define PRUSS_UART_MDR			13
+#define PRUSS_UART_MDR_OSM_SEL_MASK	BIT(0)
+#define PRUSS_UART_MDR_16X_MODE		0
+#define PRUSS_UART_MDR_13X_MODE		1
+
+struct pruss8250_data {
+	struct clk *clk;
+	int line;
+};
+
+static int pruss8250_startup(struct uart_port *port)
+{
+	int ret;
+
+	port->serial_out(port, PRUSS_UART_PEREMU_MGMT, 0);
+
+	ret = serial8250_do_startup(port);
+	if (!ret)
+		port->serial_out(port, PRUSS_UART_PEREMU_MGMT, PRUSS_UART_TX_EN |
+							       PRUSS_UART_RX_EN |
+							       PRUSS_UART_FREE_RUN);
+
+	return ret;
+}
+
+static unsigned int pruss8250_get_divisor(struct uart_port *port,
+					  unsigned int baud,
+					  unsigned int *frac)
+{
+	unsigned int uartclk = port->uartclk;
+	unsigned int div_13, div_16;
+	unsigned int abs_d13, abs_d16;
+	u16 quot;
+
+	div_13 = DIV_ROUND_CLOSEST(uartclk, 13 * baud);
+	div_16 = DIV_ROUND_CLOSEST(uartclk, 16 * baud);
+	div_13 = div_13 ? : 1;
+	div_16 = div_16 ? : 1;
+
+	abs_d13 = abs(baud - uartclk / 13 / div_13);
+	abs_d16 = abs(baud - uartclk / 16 / div_16);
+
+	if (abs_d13 >= abs_d16) {
+		*frac = PRUSS_UART_MDR_16X_MODE;
+		quot = div_16;
+	} else {
+		*frac = PRUSS_UART_MDR_13X_MODE;
+		quot = div_13;
+	}
+
+	return quot;
+}
+
+static void pruss8250_set_divisor(struct uart_port *port, unsigned int baud,
+				  unsigned int quot, unsigned int quot_frac)
+{
+	serial8250_do_set_divisor(port, baud, quot);
+
+	/*
+	 * quot_frac holds the MDR over-sampling mode
+	 * which is set in pruss8250_get_divisor()
+	 */
+	quot_frac &= PRUSS_UART_MDR_OSM_SEL_MASK;
+	port->serial_out(port, PRUSS_UART_MDR, quot_frac);
+}
+
+static int pruss8250_probe(struct platform_device *pdev)
+{
+	struct uart_8250_port port8250;
+	struct uart_port *port = &port8250.port;
+	struct device *dev = &pdev->dev;
+	struct pruss8250_data *data;
+	struct resource *res;
+	int ret;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	memset(&port8250, 0, sizeof(port8250));
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Failed to get resource");
+		return -EINVAL;
+	}
+
+	if (!port->uartclk) {
+		data->clk = devm_clk_get(dev, NULL);
+		if (IS_ERR(data->clk)) {
+			dev_err(dev, "Failed to get clock!\n");
+			return -ENODEV;
+		} else {
+			port->uartclk = clk_get_rate(data->clk);
+			devm_clk_put(dev, data->clk);
+		}
+	}
+
+	port->dev = dev;
+	port->mapbase = res->start;
+	port->mapsize = resource_size(res);
+	port->type = PORT_16550A;
+	port->flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE |
+		      UPF_IOREMAP;
+	port->startup = pruss8250_startup;
+	port->rs485_config = serial8250_em485_config;
+	port->get_divisor = pruss8250_get_divisor;
+	port->set_divisor = pruss8250_set_divisor;
+
+	ret = uart_read_port_properties(port);
+	if (ret)
+		return ret;
+
+	port->iotype = UPIO_MEM32;
+	port->regshift = 2;
+
+	spin_lock_init(&port8250.port.lock);
+	port8250.capabilities = UART_CAP_FIFO | UART_CAP_AFE;
+
+	ret = serial8250_register_8250_port(&port8250);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Unable to register 8250 port.\n");
+
+	data->line = ret;
+	platform_set_drvdata(pdev, data);
+	return 0;
+}
+
+static void pruss8250_remove(struct platform_device *pdev)
+{
+	struct pruss8250_data *data = platform_get_drvdata(pdev);
+
+	serial8250_unregister_port(data->line);
+}
+
+static const struct of_device_id pruss8250_of_match[] = {
+	{ .compatible = "ti,pruss-uart", .data = (void *)PORT_16550A },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, pruss8250_of_match);
+
+static struct platform_driver pruss8250_driver = {
+	.driver = {
+		.name = "pruss8250",
+		.of_match_table = pruss8250_of_match,
+	},
+	.probe = pruss8250_probe,
+	.remove = pruss8250_remove,
+};
+
+module_platform_driver(pruss8250_driver);
+
+MODULE_AUTHOR("Bin Liu <b-liu@ti.com");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Serial Port driver for PRUSS UART on TI platforms");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index f64ef0819cd4..5fdb502b7eae 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -582,6 +582,17 @@ config SERIAL_8250_NI
 	  To compile this driver as a module, choose M here: the module
 	  will be called 8250_ni.
 
+config SERIAL_8250_PRUSS
+	tristate "TI PRU-ICSS UART support"
+	depends on SERIAL_8250
+	depends on (PRU_REMOTEPROC && TI_PRUSS_INTC) || COMPILE_TEST
+	help
+	  This driver is to support the UART module in PRU-ICSS which is
+	  available in some TI platforms.
+
+	  To compile this driver as a module, chose M here: the
+	  module will be called 8250_pruss.
+
 config SERIAL_OF_PLATFORM
 	tristate "Devicetree based probing for 8250 ports"
 	depends on SERIAL_8250 && OF
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index b04eeda03b23..3132b4f40a34 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_SERIAL_8250_PARISC)	+= 8250_parisc.o
 obj-$(CONFIG_SERIAL_8250_PCI)		+= 8250_pci.o
 obj-$(CONFIG_SERIAL_8250_PCI1XXXX)	+= 8250_pci1xxxx.o
 obj-$(CONFIG_SERIAL_8250_PERICOM)	+= 8250_pericom.o
+obj-$(CONFIG_SERIAL_8250_PRUSS)		+= 8250_pruss.o
 obj-$(CONFIG_SERIAL_8250_PXA)		+= 8250_pxa.o
 obj-$(CONFIG_SERIAL_8250_RT288X)	+= 8250_rt288x.o
 obj-$(CONFIG_SERIAL_8250_CS)		+= serial_cs.o
-- 
2.49.0

[PATCH 4/7] DONOTMERGE: arm64: dts: ti: k3-am64-main: Add PRU UART nodes

[Judith Mendez]

There is one PRU UART module in each PRU_ICSSG for am64 SoC.

Add a PRU UART child node in each ICSSG node in am64 main voltage
domain .dtsi file for am64x device.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
index c7e5da37486a..5ba7b8a8dbcf 100644
--- a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
@@ -1280,6 +1280,15 @@ icssg0_iepclk_mux: iepclk-mux@30 {
 			};
 		};
 
+		icssg0_uart: serial@28000 {
+			compatible = "ti,pruss-uart";
+			reg = <0x28000 0x40>;
+			clocks = <&k3_clks 81 19>;
+			interrupt-parent = <&icssg0_intc>;
+			interrupts = <6 7 7>;
+			status = "disabled";
+		};
+
 		icssg0_iep0: iep@2e000 {
 			compatible = "ti,am654-icss-iep";
 			reg = <0x2e000 0x1000>;
@@ -1459,6 +1468,15 @@ icssg1_iepclk_mux: iepclk-mux@30 {
 			};
 		};
 
+		icssg1_uart: serial@28000 {
+			compatible = "ti,pruss-uart";
+			reg = <0x28000 0x40>;
+			clocks = <&k3_clks 82 19>;
+			interrupt-parent = <&icssg1_intc>;
+			interrupts = <6 7 7>;
+			status = "disabled";
+		};
+
 		icssg1_iep0: iep@2e000 {
 			compatible = "ti,am654-icss-iep";
 			reg = <0x2e000 0x1000>;
-- 
2.49.0

[PATCH 5/7] DONOTMERGE: arm64: dts: ti: k3-am642-sk: Enable PRU UART

[Judith Mendez]

There is one PRU UART module in each ICSSG for am64 SoC.

UART RX/TX signals for PRU UART in ICSSG0 can be routed from/to the
PRU Connector J10 (pins 45/44) on am64x SK, so enable icssg0_uart by
default and add pinmux node.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am642-sk.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am642-sk.dts b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
index 1deaa0be0085..9065fc8a7569 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-sk.dts
@@ -25,6 +25,7 @@ aliases {
 		serial0 = &mcu_uart0;
 		serial1 = &main_uart1;
 		serial2 = &main_uart0;
+		serial3 = &icssg0_uart;
 		i2c0 = &main_i2c0;
 		i2c1 = &main_i2c1;
 		mmc0 = &sdhci0;
@@ -284,6 +285,15 @@ AM64X_IOPAD(0x0244, PIN_OUTPUT, 0) /* (E14) UART1_TXD */
 		>;
 	};
 
+	icssg0_uart_pins_default: icssg0-uart-default-pins {
+		pinctrl-single,pins = <
+			AM64X_IOPAD(0x0184, PIN_INPUT, 2) /* (W6) PRG0_PRU0_GPO9.PRG0_UART0_CTSn */
+			AM64X_IOPAD(0x0188, PIN_OUTPUT, 2) /* (AA5) PRG0_PRU0_GPO10.PRG0_UART0_RTSn */
+			AM64X_IOPAD(0x01d4, PIN_INPUT, 2) /* (Y5) PRG0_PRU1_GPO9.PRG0_UART0_RXD */
+			AM64X_IOPAD(0x01d8, PIN_OUTPUT, 2) /* (V6) PRG0_PRU1_GPO10.PRG0_UART0_TXD */
+		>;
+	};
+
 	main_usb0_pins_default: main-usb0-default-pins {
 		bootph-all;
 		pinctrl-single,pins = <
@@ -413,6 +423,12 @@ &main_uart1 {
 	pinctrl-0 = <&main_uart1_pins_default>;
 };
 
+&icssg0_uart {
+	pinctrl-names = "default";
+	pinctrl-0 = <&icssg0_uart_pins_default>;
+	status = "okay";
+};
+
 &main_i2c0 {
 	bootph-all;
 	status = "okay";
-- 
2.49.0

[PATCH 6/7] DONOTMERGE: arm64: dts: ti: k3-am62-main: Add PRU UART node

[Judith Mendez]

There is one PRU UART module in the PRU subsystem for am62 SoC.

Add a PRU UART child node in PRUSS node in am62 main voltage
domain .dtsi file for am62x device.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index 9e0b6eee9ac7..c012f33f4609 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -1144,6 +1144,15 @@ pruss_intc: interrupt-controller@20000 {
 					  "host_intr6", "host_intr7";
 		};
 
+		pruss_uart: serial@28000 {
+			compatible = "ti,pruss-uart";
+			reg = <0x28000 0x40>;
+			clocks = <&k3_clks 81 13>; /* pruss_uart_clk */
+			interrupt-parent = <&pruss_intc>;
+			interrupts = <6 4 4>;
+			status = "disabled";
+		};
+
 		pru0: pru@34000 {
 			compatible = "ti,am625-pru";
 			reg = <0x34000 0x3000>,
-- 
2.49.0

[PATCH 7/7] DONOTMERGE: arm64: dts: ti: k3-am62x-sk: Enable PRU UART

[Judith Mendez]

There is one PRU UART module in the PRU subsystem for am62 SoC.

UART RX/TX signals for PRU UART in PRU subsystem can be routed from/to
the user expansion header J3 (pins 10/8) on am62x SK, so enable
pruss_uart by default and add pinmux node.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
index ee8337bfbbfd..c474e1d1a74d 100644
--- a/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi
@@ -15,6 +15,7 @@ aliases {
 		serial0 = &wkup_uart0;
 		serial1 = &mcu_uart0;
 		serial2 = &main_uart0;
+		serial3 = &pruss_uart;
 		mmc0 = &sdhci0;
 		mmc1 = &sdhci1;
 		mmc2 = &sdhci2;
@@ -181,6 +182,13 @@ AM62X_IOPAD(0x1b0, PIN_OUTPUT, 2) /* (A20/D16) MCASP0_ACLKR.UART1_TXD */
 		>;
 	};
 
+	pruss_uart_pins: pruss-uart-pins {
+		pinctrl-single,pins = <
+			AM62X_IOPAD(0x01d8, PIN_INPUT, 6) /* (C15) MCAN0_TX.PR0_UART0_RXD */
+			AM62X_IOPAD(0x01dc, PIN_OUTPUT, 6) /* (E15) MCAN0_RX.PR0_UART0_TXD */
+		>;
+	};
+
 	main_i2c0_pins_default: main-i2c0-default-pins {
 		pinctrl-single,pins = <
 			AM62X_IOPAD(0x1e0, PIN_INPUT_PULLUP, 0) /* (B16/E12) I2C0_SCL */
@@ -370,6 +378,12 @@ &main_uart1 {
 	pinctrl-0 = <&main_uart1_pins_default>;
 };
 
+&pruss_uart {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pruss_uart_pins>;
+	status = "okay";
+};
+
 &main_i2c0 {
 	status = "okay";
 	pinctrl-names = "default";
-- 
2.49.0

Re: [PATCH 0/7] Introduce PRU UART driver

[Greg Kroah-Hartman]

On Tue, May 13, 2025 at 04:59:27PM -0500, Judith Mendez wrote:
> The PRU_ICSSG subsystems in am64x SoC, the PRU subsystem in am62 SoC, and
> PRU_ICSS subsystem in am335x SoC include a UART sub-module. This patch
> series introduces the driver and the corresponding binding documentation
> for this UART sub-module.
> 
> The DTS patches for adding PRU UART nodes and enabling PRU UART is added
> in this v1 version, but marked as DONOTMERGE since the patches only add
> context to this series.

This prevents the series from being merged as our tools want to take the
whole series :(

So please, submit this in a format that we can handle.  As-is, this just
makes me want to ignore it totally and focus on patch series that can be
applied.  In other words, what would you do if you were in the position
of attempting to review this?

thanks,

greg k-h

Re: [PATCH 3/7] serial: 8250: Add PRUSS UART driver

[Greg Kroah-Hartman]

On Tue, May 13, 2025 at 04:59:30PM -0500, Judith Mendez wrote:
> From: Bin Liu <b-liu@ti.com>
> 
> This adds a new serial 8250 driver that supports the UART in PRUSS or
> PRU_ICSS*.
> 
> The UART sub-module is based on the industry standard TL16C550 UART
> controller, which has 16-bytes FIFO and supports 16x and 13x over
> samplings.

If it is based on an existing controller, why do we need a new driver
for this?  Please explain in detail why this code is needed at all, and
not just a new "quirk" for the existing driver?

thanks,

greg k-h

Re: [PATCH 3/7] serial: 8250: Add PRUSS UART driver

[Andy Shevchenko]

On Tue, May 13, 2025 at 04:59:30PM -0500, Judith Mendez wrote:
> From: Bin Liu <b-liu@ti.com>
> 
> This adds a new serial 8250 driver that supports the UART in PRUSS or
> PRU_ICSS*.
> 
> The UART sub-module is based on the industry standard TL16C550 UART
> controller, which has 16-bytes FIFO and supports 16x and 13x over
> samplings.

...

+ bits.h

> +#include <linux/clk.h>

+ math.h

> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/serial_reg.h>
> +#include <linux/serial_core.h>

Can you keep them sorted?



...

> +static int pruss8250_startup(struct uart_port *port)
> +{
> +	int ret;
> +
> +	port->serial_out(port, PRUSS_UART_PEREMU_MGMT, 0);
> +
> +	ret = serial8250_do_startup(port);

Please, use standard pattern, i.e.

	if (ret)
		return ret;
	...
	return 0;

I believe I have told this previously. Can you double check that you read and
addressed all of the comments?

> +	if (!ret)
> +		port->serial_out(port, PRUSS_UART_PEREMU_MGMT, PRUSS_UART_TX_EN |
> +							       PRUSS_UART_RX_EN |
> +							       PRUSS_UART_FREE_RUN);
> +
> +	return ret;
> +}

...

> +static unsigned int pruss8250_get_divisor(struct uart_port *port,
> +					  unsigned int baud,
> +					  unsigned int *frac)
> +{
> +	unsigned int uartclk = port->uartclk;
> +	unsigned int div_13, div_16;
> +	unsigned int abs_d13, abs_d16;
> +	u16 quot;
> +
> +	div_13 = DIV_ROUND_CLOSEST(uartclk, 13 * baud);
> +	div_16 = DIV_ROUND_CLOSEST(uartclk, 16 * baud);
> +	div_13 = div_13 ? : 1;
> +	div_16 = div_16 ? : 1;
> +
> +	abs_d13 = abs(baud - uartclk / 13 / div_13);
> +	abs_d16 = abs(baud - uartclk / 16 / div_16);
> +
> +	if (abs_d13 >= abs_d16) {

Isn't this something like abs_diff() ?

> +		*frac = PRUSS_UART_MDR_16X_MODE;
> +		quot = div_16;
> +	} else {
> +		*frac = PRUSS_UART_MDR_13X_MODE;
> +		quot = div_13;
> +	}
> +
> +	return quot;
> +}

> +static int pruss8250_probe(struct platform_device *pdev)
> +{
> +	struct uart_8250_port port8250;
> +	struct uart_port *port = &port8250.port;
> +	struct device *dev = &pdev->dev;
> +	struct pruss8250_data *data;
> +	struct resource *res;
> +	int ret;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);

Needs device/devres.h.

> +	if (!data)
> +		return -ENOMEM;

Needs err.h (actually errno.h, but that's not enough for the following IS_ERR()
et al.)

> +	memset(&port8250, 0, sizeof(port8250));

Instead of having dependency on string.h (which is missed) just assign it to {}
in the definition.

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "Failed to get resource");
> +		return -EINVAL;

	return dev_err_probe();

> +	}

> +	if (!port->uartclk) {
> +		data->clk = devm_clk_get(dev, NULL);
> +		if (IS_ERR(data->clk)) {
> +			dev_err(dev, "Failed to get clock!\n");
> +			return -ENODEV;
> +		} else {
> +			port->uartclk = clk_get_rate(data->clk);
> +			devm_clk_put(dev, data->clk);

I think you completely ignored my review I have done in previous version...

> +		}
> +	}

Should be done after uart_read_properties().

> +	port->dev = dev;
> +	port->mapbase = res->start;
> +	port->mapsize = resource_size(res);
> +	port->type = PORT_16550A;
> +	port->flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE |
> +		      UPF_IOREMAP;
> +	port->startup = pruss8250_startup;
> +	port->rs485_config = serial8250_em485_config;
> +	port->get_divisor = pruss8250_get_divisor;
> +	port->set_divisor = pruss8250_set_divisor;
> +
> +	ret = uart_read_port_properties(port);
> +	if (ret)
> +		return ret;
> +
> +	port->iotype = UPIO_MEM32;
> +	port->regshift = 2;
> +
> +	spin_lock_init(&port8250.port.lock);
> +	port8250.capabilities = UART_CAP_FIFO | UART_CAP_AFE;
> +
> +	ret = serial8250_register_8250_port(&port8250);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Unable to register 8250 port.\n");
> +
> +	data->line = ret;
> +	platform_set_drvdata(pdev, data);
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/7] Introduce PRU UART driver

[Krzysztof Kozlowski]

On 13/05/2025 23:59, Judith Mendez wrote:
> The PRU_ICSSG subsystems in am64x SoC, the PRU subsystem in am62 SoC, and
> PRU_ICSS subsystem in am335x SoC include a UART sub-module. This patch
> series introduces the driver and the corresponding binding documentation
> for this UART sub-module.
> 
> The DTS patches for adding PRU UART nodes and enabling PRU UART is added
> in this v1 version, but marked as DONOTMERGE since the patches only add
> context to this series.
> 
> This driver version has been tested on the following boards: am64x SK and
> am62x SK.
> 
> The RFC version of this driver has been previously tested on am335x SK as
> well. DTS patches for enabling PRU UART for am335x SK will be sent as a
> separate series once this series is merged.
> 
> Changes since RFC:
So this is v2 or v3. If this is confusing, just use b4 which does it for
you.

Otherwise how are we supposed to compare it with b4 diff?

Best regards,
Krzysztof

Re: [PATCH 1/7] dt-bindings: serial: add binding documentation for TI PRUSS UART

[Krzysztof Kozlowski]

On 13/05/2025 23:59, Judith Mendez wrote:
> From: Bin Liu <b-liu@ti.com>
> 
> This adds the YAML DT binding for PRUSS UART on TI SoCs.

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

A nit, subject: drop second/last, redundant "binding documentation for".
The "dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18


> 
> Signed-off-by: Bin Liu <b-liu@ti.com>
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>  .../bindings/serial/ti,pruss-uart.yaml        | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/ti,pruss-uart.yaml
> 
> diff --git a/Documentation/devicetree/bindings/serial/ti,pruss-uart.yaml b/Documentation/devicetree/bindings/serial/ti,pruss-uart.yaml
> new file mode 100644
> index 000000000000..34a03d572333
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/ti,pruss-uart.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/ti,pruss-uart.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI PRUSS serial UART
> +
> +maintainers:
> +  - Bin Liu <b-liu@ti.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The PRU subsystem has a serial UART peripheral based on the industry
> +  standard TL16C550, with 16-byte TX/RX FIFOs.
> +
> +allOf:
> +  - $ref: /schemas/serial.yaml#

Wrong schema. From which file did you take such syntax?

> +
> +properties:
> +  compatible:
> +    items:

You can drop items here.

> +      - const: ti,pruss-uart

Why this isn't SoC compatible?

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    description: |
> +      PRU UART interrupt mappings, containing an entry of 3 cell-values.
> +      The first is the PRU System Event ID for PRU UART Interrupt Request.
> +      The second is the PRU interrupt channel ID.
> +      The third is the PRU host interrupt ID.

I do not understand how this description is related to the property.
What if I use cells=1 or cells=4? Binding becomes wrong? Drop.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pruss_uart: serial@28000 {

Drop unused label


Best regards,
Krzysztof

Re: [PATCH 2/7] dt-bindings: soc: ti: pruss: Add documentation for PRU UART support

[Krzysztof Kozlowski]

On 13/05/2025 23:59, Judith Mendez wrote:
> Add documentation for PRU UART node which is for PRU serial UART
> based-off the industry standard TL16C550 asynchronous communications
> element.
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Nothing in cover letter explains dependency and this the most important
part of cover letter. Otherwise how maintainers are supposed to guess
what they can take and what cannot?

Squash the patch with previous in such case.

> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
> index 927b3200e29e..54397297cbf5 100644
> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
> @@ -324,6 +324,13 @@ patternProperties:
>      $ref: /schemas/net/ti,davinci-mdio.yaml#
>      type: object
>  
> +  serial@[a-f0-9]+$:
> +    description: |

Do not need '|' unless you need to preserve formatting.

Best regards,
Krzysztof

Re: [PATCH 2/7] dt-bindings: soc: ti: pruss: Add documentation for PRU UART support

[Judith Mendez]

On 5/14/25 7:35 AM, Krzysztof Kozlowski wrote:
> On 13/05/2025 23:59, Judith Mendez wrote:
>> Add documentation for PRU UART node which is for PRU serial UART
>> based-off the industry standard TL16C550 asynchronous communications
>> element.
>>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
>>   Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
> 
> Nothing in cover letter explains dependency and this the most important
> part of cover letter. Otherwise how maintainers are supposed to guess
> what they can take and what cannot?
> 
> Squash the patch with previous in such case.

Sure can do.

> 
>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>> index 927b3200e29e..54397297cbf5 100644
>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>> @@ -324,6 +324,13 @@ patternProperties:
>>       $ref: /schemas/net/ti,davinci-mdio.yaml#
>>       type: object
>>   
>> +  serial@[a-f0-9]+$:
>> +    description: |
> 
> Do not need '|' unless you need to preserve formatting.

Will fix this and first patch as well.

Thanks

~ Judith

Re: [PATCH 3/7] serial: 8250: Add PRUSS UART driver

[Judith Mendez]

Hi Greg,

On 5/14/25 2:36 AM, Greg Kroah-Hartman wrote:
> On Tue, May 13, 2025 at 04:59:30PM -0500, Judith Mendez wrote:
>> From: Bin Liu <b-liu@ti.com>
>>
>> This adds a new serial 8250 driver that supports the UART in PRUSS or
>> PRU_ICSS*.
>>
>> The UART sub-module is based on the industry standard TL16C550 UART
>> controller, which has 16-bytes FIFO and supports 16x and 13x over
>> samplings.
> 
> If it is based on an existing controller, why do we need a new driver
> for this?  Please explain in detail why this code is needed at all, and
> not just a new "quirk" for the existing driver?

This was explained in RFC [0] but the patch description can be improved
so will fix for v2.

[0] https://lore.kernel.org/all/ba88a5c0-a8b8-4e48-9752-76881fa8e94e@ti.com/

Thanks
~ Judith