[PATCH 2/5] serial: s5p: Add Apple M1 support
Mark Kettenis
mark.kettenis at xs4all.nl
Sun Sep 19 22:30:16 CEST 2021
> From: Bin Meng <bmeng.cn at gmail.com>
> Date: Sun, 19 Sep 2021 09:11:06 +0800
>
> Hi Mark,
>
> On Sat, Sep 18, 2021 at 9:55 PM Mark Kettenis <kettenis at 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 <kettenis at 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 <kettenis at 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.kang at samsung.com>
> > + * Heungjun Kim <riverful.kim at 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
Well, this is the status quo for this driver. There are several
variations of the hardware with slightly different register layouts
and the header file trick is how that is handled.
> > +
> > +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 ...
Yeah, the Apple hardware is based on an older variant of the s5p that
had a smaller FIFO. The size of the FIFO was increased somewhere
along the way and the bits were moved...
> > +#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?
There actually is such a property in the binding called
"reg-io-width". So I suppose I should use that.
> > +#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
>
More information about the U-Boot
mailing list