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?
> +
> +#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.
> +#define XYD_Y 65000
> +#define XINDIV_CLOCK 20000000
And what if input clock has different rate?
> +#define UART_BRDL_20M 0x01
> +#define UART_BRDH_20M 0x00
Blank line
> +static int clock_div_tab[] = { 10, 4, 2};
> +static int clock_div_reg[] = { 4, 2, 1};
Why not const?
Best regards,
Krzysztof
