Hi.
On 06/01/2025 08:19, Krzysztof Kozlowski wrote:
On Sun, Jan 05, 2025 at 02:11:47PM +0100, Benjamin Larsson wrote:
Support for Airoha AN7581 SoC UART and HSUART baud rate
calculation routine.
Signed-off-by: Benjamin Larsson <benjamin.larsson@xxxxxxxxxx>
---
drivers/tty/serial/8250/8250_airoha.c | 85 +++++++++++++++++++++++++++
drivers/tty/serial/8250/8250_of.c | 2 +
drivers/tty/serial/8250/8250_port.c | 26 ++++++++
drivers/tty/serial/8250/Kconfig | 10 ++++
drivers/tty/serial/8250/Makefile | 1 +
include/linux/serial_8250.h | 1 +
include/uapi/linux/serial_core.h | 6 ++
include/uapi/linux/serial_reg.h | 9 +++
8 files changed, 140 insertions(+)
create mode 100644 drivers/tty/serial/8250/8250_airoha.c
diff --git a/drivers/tty/serial/8250/8250_airoha.c b/drivers/tty/serial/8250/8250_airoha.c
new file mode 100644
index 000000000000..c57789dcc174
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_airoha.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Airoha UART driver.
+ *
+ * Copyright (c) 2025 Genexis Sweden AB
+ * Author: Benjamin Larsson <benjamin.larsson@xxxxxxxxxx>
+ */
+
+#include <linux/clk.h>
Where is it used?
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
Where is it used?
+#include <linux/of_platform.h>
Where is it used?
+#include <linux/pinctrl/consumer.h>
I have impression that none of these are used. You include some huge
amount of unused headers.
+#include <linux/platform_device.h>
???
+#include <linux/pm_runtime.h>
I really cannot find it.
+#include <linux/serial_8250.h>
+#include <linux/serial_reg.h>
+#include <linux/console.h>
+#include <linux/dma-mapping.h>
Where do you use DMA?
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
Any of these?
My bad forgot to remove most of them, they/some where needed in the
initial development. Will address in v2.
+
+#include "8250.h"
+
+/* The Airoha UART is 16550-compatible except for the baud rate calculation.
+ *
+ * crystal_clock = 20 MHz
+ * xindiv_clock = crystal_clock / clock_div
+ * (x/y) = XYD, 32 bit register with 16 bits of x and then 16 bits of y
+ * clock_div = XINCLK_DIVCNT (default set to 10 (0x4)),
+ * - 3 bit register [ 1, 2, 4, 8, 10, 12, 16, 20 ]
+ *
+ * baud_rate = ((xindiv_clock) * (x/y)) / ([BRDH,BRDL] * 16)
+ *
+ * XYD_y seems to need to be larger then XYD_x for proper waveform generation.
+ * Setting [BRDH,BRDL] to [0,1] and XYD_y to 65000 give even values
+ * for usual baud rates.
+ *
+ * Selecting divider needs to fulfill
+ * 1.8432 MHz <= xindiv_clk <= APB clock / 2
+ * The clocks are unknown but a divider of value 1 did not result in a valid
+ * waveform.
+ *
+ */
+
+#define CLOCK_DIV_TAB_ELEMS 3
No, use ARRAY_SIZE in your code.
Will address in v2.
+#define XYD_Y 65000
+#define XINDIV_CLOCK 20000000
And what if input clock has different rate?
On this family of SoCs the clock is fixed. I will add a note about that
in v2.
+#define UART_BRDL_20M 0x01
+#define UART_BRDH_20M 0x00
Blank line
Will address in v2.
+static int clock_div_tab[] = { 10, 4, 2};
+static int clock_div_reg[] = { 4, 2, 1};
Why not const?
Will address in v2.
Best regards,
Krzysztof
MvH
Benjamin Larsson
