From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 61D83C433F5 for ; Sun, 19 Sep 2021 20:30:26 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 645C260F8F for ; Sun, 19 Sep 2021 20:30:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 645C260F8F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id DDF2D81FC6; Sun, 19 Sep 2021 22:30:22 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id D1ECD81BC8; Sun, 19 Sep 2021 22:30:20 +0200 (CEST) Received: from sibelius.xs4all.nl (sibelius.xs4all.nl [83.163.83.176]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id E5F9882BC3 for ; Sun, 19 Sep 2021 22:30:16 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=mark.kettenis@xs4all.nl Received: from localhost (bloch.sibelius.xs4all.nl [local]) by bloch.sibelius.xs4all.nl (OpenSMTPD) with ESMTPA id 4fc54bdc; Sun, 19 Sep 2021 22:30:16 +0200 (CEST) Date: Sun, 19 Sep 2021 22:30:16 +0200 (CEST) From: Mark Kettenis To: Bin Meng Cc: kettenis@openbsd.org, u-boot@lists.denx.de, oliver.graute@kococonnector.com, ycliang@andestech.com, kishon@ti.com, priyanka.jain@nxp.com, stephan@gerhold.net, padmarao.begari@microchip.com, tianrui-wei@outlook.com, michael@walle.cc, xypron.glpk@gmx.de, sjg@chromium.org, ashe@kivikakk.ee, bin.meng@windriver.com, michal.simek@xilinx.com, wasim.khan@nxp.com, ye.li@nxp.com, igor.opaniuk@foundries.io, sr@denx.de, takahiro.akashi@linaro.org, vabhav.sharma@nxp.com, weijie.gao@mediatek.com, p.yadav@ti.com, andriy.shevchenko@linux.intel.com In-Reply-To: (message from Bin Meng on Sun, 19 Sep 2021 09:11:06 +0800) Subject: Re: [PATCH 2/5] serial: s5p: Add Apple M1 support References: <20210918135437.36667-1-kettenis@openbsd.org> <20210918135437.36667-3-kettenis@openbsd.org> Message-ID: <56146be19c3804ba@bloch.sibelius.xs4all.nl> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean > From: Bin Meng > Date: Sun, 19 Sep 2021 09:11:06 +0800 > > Hi Mark, > > On Sat, Sep 18, 2021 at 9:55 PM Mark Kettenis 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 > > --- > > 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 > > + */ > > + > > +#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 > > + * Heungjun Kim > > + */ > > + > > +#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 >