Hi Mark,
On Sat, Sep 18, 2021 at 9:55 PM Mark Kettenis <kette...@openbsd.org> wrote:
>
> Apple M1 SoCs include an S5L UART which is a variant of the S5P
> UART. Add support for this variant and enable it by default
> on Apple SoCs.
>
> Signed-off-by: Mark Kettenis <kette...@openbsd.org>
> ---
> arch/arm/include/asm/arch-m1/clk.h | 11 ++++++++
> arch/arm/include/asm/arch-m1/uart.h | 41 +++++++++++++++++++++++++++++
> arch/arm/mach-apple/board.c | 5 ++++
> drivers/serial/Kconfig | 2 +-
> drivers/serial/serial_s5p.c | 22 ++++++++++++++++
> 5 files changed, 80 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/include/asm/arch-m1/clk.h
> create mode 100644 arch/arm/include/asm/arch-m1/uart.h
>
> diff --git a/arch/arm/include/asm/arch-m1/clk.h
> b/arch/arm/include/asm/arch-m1/clk.h
> new file mode 100644
> index 0000000000..f4326d0c0f
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-m1/clk.h
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2021 Mark Kettenis <kette...@openbsd.org>
> + */
> +
> +#ifndef __ASM_ARM_ARCH_CLK_H_
> +#define __ASM_ARM_ARCH_CLK_H_
> +
> +unsigned long get_uart_clk(int dev_index);
> +
> +#endif
> diff --git a/arch/arm/include/asm/arch-m1/uart.h
> b/arch/arm/include/asm/arch-m1/uart.h
> new file mode 100644
> index 0000000000..d2a17a221e
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-m1/uart.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * (C) Copyright 2009 Samsung Electronics
> + * Minkyu Kang <mk7.k...@samsung.com>
> + * Heungjun Kim <riverful....@samsung.com>
> + */
> +
> +#ifndef __ASM_ARCH_UART_H_
> +#define __ASM_ARCH_UART_H_
> +
> +#ifndef __ASSEMBLY__
> +/* baudrate rest value */
> +union br_rest {
> + unsigned short slot; /* udivslot */
> + unsigned char value; /* ufracval */
> +};
> +
> +struct s5p_uart {
> + unsigned int ulcon;
> + unsigned int ucon;
> + unsigned int ufcon;
> + unsigned int umcon;
> + unsigned int utrstat;
> + unsigned int uerstat;
> + unsigned int ufstat;
> + unsigned int umstat;
> + unsigned int utxh;
> + unsigned int urxh;
> + unsigned int ubrdiv;
> + union br_rest rest;
> + unsigned char res3[0x3fd0];
> +};
This does not look correct. This should be declared in the s5p UART
driver header instead of SoC's
> +
> +static inline int s5p_uart_divslot(void)
> +{
> + return 0;
> +}
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif
> diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c
> index 0c8b35292e..8bc5c2f69e 100644
> --- a/arch/arm/mach-apple/board.c
> +++ b/arch/arm/mach-apple/board.c
> @@ -156,3 +156,8 @@ ulong board_get_usable_ram_top(ulong total_size)
> */
> return 0x980000000;
> }
> +
> +unsigned long get_uart_clk(int dev_index)
> +{
> + return 24000000;
> +}
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 93348c0929..c3ac773929 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -719,7 +719,7 @@ config ROCKCHIP_SERIAL
>
> config S5P_SERIAL
> bool "Support for Samsung S5P UART"
> - depends on ARCH_EXYNOS || ARCH_S5PC1XX
> + depends on ARCH_APPLE || ARCH_EXYNOS || ARCH_S5PC1XX
> default y
> help
> Select this to enable Samsung S5P UART support.
> diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c
> index 6d09952a5d..eb770d9b62 100644
> --- a/drivers/serial/serial_s5p.c
> +++ b/drivers/serial/serial_s5p.c
> @@ -21,12 +21,21 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> +#ifdef CONFIG_ARCH_APPLE
> +#define RX_FIFO_COUNT_SHIFT 0
> +#define RX_FIFO_COUNT_MASK (0xf << RX_FIFO_COUNT_SHIFT)
> +#define RX_FIFO_FULL (1 << 8)
> +#define TX_FIFO_COUNT_SHIFT 4
> +#define TX_FIFO_COUNT_MASK (0xf << TX_FIFO_COUNT_SHIFT)
> +#define TX_FIFO_FULL (1 << 9)
So different bit positions for RX/TX FIFIO register but still it is
s5p compatible, strange ...
> +#else
> #define RX_FIFO_COUNT_SHIFT 0
> #define RX_FIFO_COUNT_MASK (0xff << RX_FIFO_COUNT_SHIFT)
> #define RX_FIFO_FULL (1 << 8)
> #define TX_FIFO_COUNT_SHIFT 16
> #define TX_FIFO_COUNT_MASK (0xff << TX_FIFO_COUNT_SHIFT)
> #define TX_FIFO_FULL (1 << 24)
> +#endif
>
> /* Information about a serial port */
> struct s5p_serial_plat {
> @@ -83,7 +92,11 @@ static void __maybe_unused s5p_serial_baud(struct s5p_uart
> *uart, uint uclk,
> if (s5p_uart_divslot())
> writew(udivslot[val % 16], &uart->rest.slot);
> else
> +#ifdef CONFIG_ARCH_APPLE
> + writel(val % 16, &uart->rest.value);
This looks like we should add a property in DT like "reg-width" to
control such behavior?
> +#else
> writeb(val % 16, &uart->rest.value);
> +#endif
> }
>
> #ifndef CONFIG_SPL_BUILD
> @@ -148,7 +161,11 @@ static int s5p_serial_getc(struct udevice *dev)
> return -EAGAIN;
>
> serial_err_check(uart, 0);
> +#ifdef CONFIG_ARCH_APPLE
> + return (int)(readl(&uart->urxh) & 0xff);
> +#else
> return (int)(readb(&uart->urxh) & 0xff);
> +#endif
> }
>
> static int s5p_serial_putc(struct udevice *dev, const char ch)
> @@ -159,7 +176,11 @@ static int s5p_serial_putc(struct udevice *dev, const
> char ch)
> if (readl(&uart->ufstat) & TX_FIFO_FULL)
> return -EAGAIN;
>
> +#ifdef CONFIG_ARCH_APPLE
> + writel(ch, &uart->utxh);
> +#else
> writeb(ch, &uart->utxh);
> +#endif
> serial_err_check(uart, 1);
>
> return 0;
> @@ -201,6 +222,7 @@ static const struct dm_serial_ops s5p_serial_ops = {
>
> static const struct udevice_id s5p_serial_ids[] = {
> { .compatible = "samsung,exynos4210-uart" },
> + { .compatible = "apple,s5l-uart" },
> { }
> };
Regards,
Bin
