All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: Bin Meng <bmeng.cn@gmail.com>
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
Subject: Re: [PATCH 2/5] serial: s5p: Add Apple M1 support
Date: Sun, 19 Sep 2021 22:30:16 +0200 (CEST)	[thread overview]
Message-ID: <56146be19c3804ba@bloch.sibelius.xs4all.nl> (raw)
In-Reply-To: <CAEUhbmVjuqK_ix6D22t_hHbuDNjLimpcnQCOw1mJnJQQq9_LGA@mail.gmail.com> (message from Bin Meng on Sun, 19 Sep 2021 09:11:06 +0800)

> From: Bin Meng <bmeng.cn@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@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@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@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@samsung.com>
> > + * Heungjun Kim <riverful.kim@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
> 

  reply	other threads:[~2021-09-19 20:30 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-18 13:54 [PATCH 0/5] Apple M1 Support Mark Kettenis
2021-09-18 13:54 ` [PATCH 1/5] arm: apple: Add initial support for Apple's M1 SoC Mark Kettenis
2021-09-19  1:04   ` Bin Meng
2021-09-19  1:17     ` Bin Meng
2021-09-19 20:33       ` Mark Kettenis
2021-09-21 12:42         ` Tom Rini
2021-09-21 15:53           ` Bin Meng
2021-09-21 16:04             ` Tom Rini
2021-09-21 16:08             ` Mark Kettenis
2021-09-25 13:27               ` Simon Glass
2021-09-19 20:05     ` Mark Kettenis
2021-09-20  3:15   ` Simon Glass
2021-09-20  8:49     ` Mark Kettenis
2021-09-21  1:11       ` Simon Glass
2021-09-18 13:54 ` [PATCH 2/5] serial: s5p: Add Apple M1 support Mark Kettenis
2021-09-19  1:11   ` Bin Meng
2021-09-19 20:30     ` Mark Kettenis [this message]
2021-09-20  3:15   ` Simon Glass
2021-09-25 13:27     ` Simon Glass
2021-10-02 22:15     ` Mark Kettenis
2021-10-03  2:01       ` Simon Glass
2021-09-18 13:54 ` [PATCH 3/5] misc: Add Apple DART driver Mark Kettenis
2021-09-20  3:16   ` Simon Glass
2021-09-20  8:33     ` Mark Kettenis
2021-09-21  1:11       ` Simon Glass
2021-09-25 13:27         ` Simon Glass
2021-09-26 20:53         ` Mark Kettenis
2021-09-27 20:14           ` Simon Glass
2021-09-18 13:54 ` [PATCH 4/5] arm: dts: apple: Add preliminary device trees Mark Kettenis
2021-09-20  3:16   ` Simon Glass
2021-09-25 13:27     ` Simon Glass
2021-09-18 13:54 ` [PATCH 5/5] doc: board: apple: Add Apple M1 documentation Mark Kettenis
2021-09-19  1:22   ` Bin Meng
2021-09-20  3:16   ` Simon Glass
2021-09-25 13:27     ` Simon Glass
2021-09-20  8:45   ` Igor Opaniuk
2021-09-25  1:20 ` [PATCH 0/5] Apple M1 Support Simon Glass
2021-09-25  8:11   ` Mark Kettenis
2021-09-25 13:27     ` Simon Glass
2021-09-25 13:52       ` Mark Kettenis
2021-09-25 14:42         ` Simon Glass
2021-09-25 16:45           ` Mark Kettenis
2021-09-26 15:53             ` Simon Glass
2021-09-28  3:46               ` Simon Glass
2021-09-28  7:36                 ` Mark Kettenis
2021-09-28 12:07                   ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56146be19c3804ba@bloch.sibelius.xs4all.nl \
    --to=mark.kettenis@xs4all.nl \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=ashe@kivikakk.ee \
    --cc=bin.meng@windriver.com \
    --cc=bmeng.cn@gmail.com \
    --cc=igor.opaniuk@foundries.io \
    --cc=kettenis@openbsd.org \
    --cc=kishon@ti.com \
    --cc=michael@walle.cc \
    --cc=michal.simek@xilinx.com \
    --cc=oliver.graute@kococonnector.com \
    --cc=p.yadav@ti.com \
    --cc=padmarao.begari@microchip.com \
    --cc=priyanka.jain@nxp.com \
    --cc=sjg@chromium.org \
    --cc=sr@denx.de \
    --cc=stephan@gerhold.net \
    --cc=takahiro.akashi@linaro.org \
    --cc=tianrui-wei@outlook.com \
    --cc=u-boot@lists.denx.de \
    --cc=vabhav.sharma@nxp.com \
    --cc=wasim.khan@nxp.com \
    --cc=weijie.gao@mediatek.com \
    --cc=xypron.glpk@gmx.de \
    --cc=ycliang@andestech.com \
    --cc=ye.li@nxp.com \
    --subject='Re: [PATCH 2/5] serial: s5p: Add Apple M1 support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.