Re: [PATCH] serial: meson: make the current driver compatible with S4

From: Neil Armstrong <hidden>
Date: 2021-12-06 10:50:48
Also in: linux-amlogic, linux-arm-kernel, lkml

Hi,

On 06/12/2021 11:02, xianwei.zhao wrote:

quoted hunk

Because S4 UART use a different clock source, the baud rate calculation need to be updated.
Reset the UART during initialization to clear previous status.

Signed-off-by: xianwei.zhao <xianwei.zhao@amlogic.com>
---
 drivers/tty/serial/meson_uart.c | 50 +++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index efee3935917f..15a992ee6a28 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c

@@ -65,9 +65,11 @@
 #define AML_UART_RECV_IRQ(c)		((c) & 0xff)
 
 /* AML_UART_REG5 bits */
-#define AML_UART_BAUD_MASK		0x7fffff
+#define AML_UART_BAUD_MASK		GENMASK(22, 0)
 #define AML_UART_BAUD_USE		BIT(23)
 #define AML_UART_BAUD_XTAL		BIT(24)
+#define AML_UART_BAUD_XTAL_TICK		BIT(26)
+#define AML_UART_BAUD_XTAL_DIV2		BIT(27)
 
 #define AML_UART_PORT_NUM		12
 #define AML_UART_PORT_OFFSET		6

@@ -80,6 +82,14 @@ static struct uart_driver meson_uart_driver;
 
 static struct uart_port *meson_ports[AML_UART_PORT_NUM];
 
+/*
+ * struct aml_uart - device data
+ * @xtal_tick_en: A clock source that calculates baud rates
+ */
+struct aml_uart {

please change to a better name like meson_uart_data and pass it as data of meson_uart_dt_match

quoted hunk

+	unsigned int xtal_tick_en;
+};
+
 static void meson_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
 }

@@ -103,7 +113,7 @@ static void meson_uart_stop_tx(struct uart_port *port)
 	u32 val;
 
 	val = readl(port->membase + AML_UART_CONTROL);
-	val &= ~AML_UART_TX_INT_EN;
+	val &= ~AML_UART_TX_EN;

Seems to be a cosmetic change, please split this in a separate patch.

quoted hunk

 	writel(val, port->membase + AML_UART_CONTROL);
 }

@@ -121,12 +131,12 @@ static void meson_uart_shutdown(struct uart_port *port)
 	unsigned long flags;
 	u32 val;
 
-	free_irq(port->irq, port);
+	devm_free_irq(port->dev, port->irq, port);

Please split this in a separate patch

 
 	spin_lock_irqsave(&port->lock, flags);
 
 	val = readl(port->membase + AML_UART_CONTROL);
-	val &= ~AML_UART_RX_EN;
+	val &= ~(AML_UART_RX_EN | AML_UART_TX_EN);

Seems it fixes a bug, split in a separate patch.

quoted hunk

 	val &= ~(AML_UART_RX_INT_EN | AML_UART_TX_INT_EN);
 	writel(val, port->membase + AML_UART_CONTROL);

@@ -270,6 +280,7 @@ static int meson_uart_startup(struct uart_port *port)
 	u32 val;
 	int ret = 0;
 
+	meson_uart_reset(port);

Same, split in a separate patch

quoted hunk

 	val = readl(port->membase + AML_UART_CONTROL);
 	val |= AML_UART_CLEAR_ERR;
 	writel(val, port->membase + AML_UART_CONTROL);

@@ -285,24 +296,37 @@ static int meson_uart_startup(struct uart_port *port)
 	val = (AML_UART_RECV_IRQ(1) | AML_UART_XMIT_IRQ(port->fifosize / 2));
 	writel(val, port->membase + AML_UART_MISC);
 
-	ret = request_irq(port->irq, meson_uart_interrupt, 0,
-			  port->name, port);
+	ret = devm_request_irq(port->dev, port->irq, meson_uart_interrupt,
+			       IRQF_SHARED, port->name, port);

Same, split in a separate patch

 
 	return ret;
 }
 
 static void meson_uart_change_speed(struct uart_port *port, unsigned long baud)
 {
+	struct aml_uart *aml_uart_data = port->private_data;
 	u32 val;
 
 	while (!meson_uart_tx_empty(port))
 		cpu_relax();
 
+	val = readl_relaxed(port->membase + AML_UART_REG5);
+	val &= ~AML_UART_BAUD_MASK;
+
 	if (port->uartclk == 24000000) {
-		val = ((port->uartclk / 3) / baud) - 1;
-		val |= AML_UART_BAUD_XTAL;
+		if (aml_uart_data->xtal_tick_en) {
+			val = (port->uartclk / 2 + baud / 2) / baud  - 1;
+			val |= (AML_UART_BAUD_XTAL | AML_UART_BAUD_XTAL_DIV2);

This should be triggered by a value in match data with a new compatible.

quoted hunk

+		} else {
+			val = ((port->uartclk / 3) + baud / 2) / baud  - 1;
+			val &= (~(AML_UART_BAUD_XTAL_TICK |
+				AML_UART_BAUD_XTAL_DIV2));
+			val |= AML_UART_BAUD_XTAL;
+		}
 	} else {
 		val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1;
+		val &= (~(AML_UART_BAUD_XTAL | AML_UART_BAUD_XTAL_TICK |
+			AML_UART_BAUD_XTAL_DIV2));
 	}
 	val |= AML_UART_BAUD_USE;
 	writel(val, port->membase + AML_UART_REG5);

@@ -715,6 +739,7 @@ static int meson_uart_probe(struct platform_device *pdev)
 {
 	struct resource *res_mem, *res_irq;
 	struct uart_port *port;
+	struct aml_uart *aml_uart_data;
 	u32 fifosize = 64; /* Default is 64, 128 for EE UART_0 */
 	int ret = 0;
 
@@ -754,6 +779,14 @@ static int meson_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	aml_uart_data = devm_kzalloc(&pdev->dev, sizeof(struct aml_uart),
+				     GFP_KERNEL);
+	if (!aml_uart_data)
+		return -ENOMEM;
+
+	of_property_read_u32(pdev->dev.of_node, "xtal_tick_en",
+			     &aml_uart_data->xtal_tick_en);

This property is not documented in the bindings, instead introduce
a new compatible for the S4 and pass the "aml_uart" as the meson_uart_dt_match
data pointer.

And don't forget to submit the bindings change in another separate patch.

quoted hunk

+
 	/* Use legacy way until all platforms switch to new bindings */
 	if (of_device_is_compatible(pdev->dev.of_node, "amlogic,meson-uart"))
 		ret = meson_uart_probe_clocks_legacy(pdev, port);

@@ -775,6 +808,7 @@ static int meson_uart_probe(struct platform_device *pdev)
 	port->x_char = 0;
 	port->ops = &meson_uart_ops;
 	port->fifosize = fifosize;
+	port->private_data = aml_uart_data;
 
 	meson_ports[pdev->id] = port;
 	platform_set_drvdata(pdev, port);

base-commit: 3f19fed8d0daed6e0e04b130d203d4333b757901

Thanks,
Neil

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help