Re: [PATCH 2/5] serial: s5p: Add Apple M1 support

2021-10-02 Thread Simon Glass 
Hi Mark,

On Sat, 2 Oct 2021 at 16:16, Mark Kettenis  wrote:
>
> > From: Simon Glass 
> > Date: Sun, 19 Sep 2021 21:15:59 -0600
> >
> > Hi Mark,
> >
> > On Sat, 18 Sept 2021 at 07:55, 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 00..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);
> >
> > comment? But this should be in a clock driver, unless you are trying
> > to get a debug UART up ASAP with minimal code?
> >
> > > +
> > > +#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 00..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 intulcon;
> > > +   unsigned intucon;
> > > +   unsigned intufcon;
> > > +   unsigned intumcon;
> > > +   unsigned intutrstat;
> > > +   unsigned intuerstat;
> > > +   unsigned intufstat;
> > > +   unsigned intumstat;
> > > +   unsigned intutxh;
> > > +   unsigned inturxh;
> > > +   unsigned intubrdiv;
> > > +   union br_rest   rest;
> > > +   unsigned char   res3[0x3fd0];
> > > +};
> > > +
> > > +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 0x98000;
> > >  }
> > > +
> > > +unsigned long get_uart_clk(int dev_index)
> > > +{
> > > +   return 2400;
> >
> > Should add to devicetree for the driver
> >
> > > +}
> > > 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
> >
> > This should use the compatible string to decide which variant it is,
> > then the checks happen at runtime. We should never have arch-specific
> > #ifdefs in drivers.
> >
> > > +#define RX_FIFO_COUNT_SHIFT0
> > > +#define RX_FIFO_COUNT_MASK (0xf << RX_FIFO_COUNT_SHIFT)
> > > +#define RX_FIFO_FULL   (1 << 8)
> > > +#define TX_FIFO_COUNT_SHIFT4
> > > +#define TX_FIFO_COUNT_MASK (0xf << TX_FIFO_COUNT_SHIFT)
> > > +#define TX_FIFO_FULL   (1 << 9)
> > > +#else
> > >  #define RX_FIFO_COUNT_SHIFT0
> > >  #define RX_FIFO_COUNT_MASK (0xff << RX_FIFO_COUNT_SHIFT)
> > >  #define RX_FIFO_FULL   (1 << 8)
> > >  #define TX_FIFO_COUNT_SHIFT

Re: [PATCH 2/5] serial: s5p: Add Apple M1 support

2021-10-02 Thread Mark Kettenis 
> From: Simon Glass 
> Date: Sun, 19 Sep 2021 21:15:59 -0600
> 
> Hi Mark,
> 
> On Sat, 18 Sept 2021 at 07:55, 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 00..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);
> 
> comment? But this should be in a clock driver, unless you are trying
> to get a debug UART up ASAP with minimal code?
> 
> > +
> > +#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 00..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 intulcon;
> > +   unsigned intucon;
> > +   unsigned intufcon;
> > +   unsigned intumcon;
> > +   unsigned intutrstat;
> > +   unsigned intuerstat;
> > +   unsigned intufstat;
> > +   unsigned intumstat;
> > +   unsigned intutxh;
> > +   unsigned inturxh;
> > +   unsigned intubrdiv;
> > +   union br_rest   rest;
> > +   unsigned char   res3[0x3fd0];
> > +};
> > +
> > +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 0x98000;
> >  }
> > +
> > +unsigned long get_uart_clk(int dev_index)
> > +{
> > +   return 2400;
> 
> Should add to devicetree for the driver
> 
> > +}
> > 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
> 
> This should use the compatible string to decide which variant it is,
> then the checks happen at runtime. We should never have arch-specific
> #ifdefs in drivers.
> 
> > +#define RX_FIFO_COUNT_SHIFT0
> > +#define RX_FIFO_COUNT_MASK (0xf << RX_FIFO_COUNT_SHIFT)
> > +#define RX_FIFO_FULL   (1 << 8)
> > +#define TX_FIFO_COUNT_SHIFT4
> > +#define TX_FIFO_COUNT_MASK (0xf << TX_FIFO_COUNT_SHIFT)
> > +#define TX_FIFO_FULL   (1 << 9)
> > +#else
> >  #define RX_FIFO_COUNT_SHIFT0
> >  #define RX_FIFO_COUNT_MASK (0xff << RX_FIFO_COUNT_SHIFT)
> >  #define RX_FIFO_FULL   (1 << 8)
> >  #define TX_FIFO_COUNT_SHIFT16
> >  #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())
>

Re: [PATCH 2/5] serial: s5p: Add Apple M1 support

2021-09-25 Thread Simon Glass 
On Sun, 19 Sept 2021 at 21:15, Simon Glass  wrote:
>
> Hi Mark,
>
> On Sat, 18 Sept 2021 at 07:55, 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
> >

Tested on: Macbook Air M1
Tested-by: Simon Glass

Re: [PATCH 2/5] serial: s5p: Add Apple M1 support

2021-09-19 Thread Simon Glass 
Hi Mark,

On Sat, 18 Sept 2021 at 07:55, 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 00..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);

comment? But this should be in a clock driver, unless you are trying
to get a debug UART up ASAP with minimal code?

> +
> +#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 00..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 intulcon;
> +   unsigned intucon;
> +   unsigned intufcon;
> +   unsigned intumcon;
> +   unsigned intutrstat;
> +   unsigned intuerstat;
> +   unsigned intufstat;
> +   unsigned intumstat;
> +   unsigned intutxh;
> +   unsigned inturxh;
> +   unsigned intubrdiv;
> +   union br_rest   rest;
> +   unsigned char   res3[0x3fd0];
> +};
> +
> +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 0x98000;
>  }
> +
> +unsigned long get_uart_clk(int dev_index)
> +{
> +   return 2400;

Should add to devicetree for the driver

> +}
> 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

This should use the compatible string to decide which variant it is,
then the checks happen at runtime. We should never have arch-specific
#ifdefs in drivers.

> +#define RX_FIFO_COUNT_SHIFT0
> +#define RX_FIFO_COUNT_MASK (0xf << RX_FIFO_COUNT_SHIFT)
> +#define RX_FIFO_FULL   (1 << 8)
> +#define TX_FIFO_COUNT_SHIFT4
> +#define TX_FIFO_COUNT_MASK (0xf << TX_FIFO_COUNT_SHIFT)
> +#define TX_FIFO_FULL   (1 << 9)
> +#else
>  #define RX_FIFO_COUNT_SHIFT0
>  #define RX_FIFO_COUNT_MASK (0xff << RX_FIFO_COUNT_SHIFT)
>  #define RX_FIFO_FULL   (1 << 8)
>  #define TX_FIFO_COUNT_SHIFT16
>  #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);
> +#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 -EA

Re: [PATCH 2/5] serial: s5p: Add Apple M1 support

2021-09-19 Thread Mark Kettenis 
> 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 00..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 00..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 intulcon;
> > +   unsigned intucon;
> > +   unsigned intufcon;
> > +   unsigned intumcon;
> > +   unsigned intutrstat;
> > +   unsigned intuerstat;
> > +   unsigned intufstat;
> > +   unsigned intumstat;
> > +   unsigned intutxh;
> > +   unsigned inturxh;
> > +   unsigned intubrdiv;
> > +   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 0x98000;
> >  }
> > +
> > +unsigned long get_uart_clk(int dev_index)
> > +{
> > +   return 2400;
> > +}
> > 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_SHIFT0
> > +#define RX_FIFO_COUNT_MASK (0xf << RX_FIFO_COUNT_SHIFT)
> > +#define RX_FIFO_FULL   (1 << 8)
> > +#define TX_FIFO_COUNT_SHIFT4
> > +#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_SHIFT0
> >  #define RX_FIFO_COUNT_MASK (0xff << RX_FIFO_COUNT_SHIFT)
> >  #define RX_FIFO_FULL   (1 << 8)
> >  #define TX_FIFO_COUNT_SHIFT16
> >  #define TX_FIFO_COUNT_MASK (0xff << TX_FIFO_COUNT_SHIFT)
> >  #define TX_FIFO_FULL   (1 << 24)
> > +#

Re: [PATCH 2/5] serial: s5p: Add Apple M1 support

2021-09-18 Thread Bin Meng 
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 00..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 00..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 intulcon;
> +   unsigned intucon;
> +   unsigned intufcon;
> +   unsigned intumcon;
> +   unsigned intutrstat;
> +   unsigned intuerstat;
> +   unsigned intufstat;
> +   unsigned intumstat;
> +   unsigned intutxh;
> +   unsigned inturxh;
> +   unsigned intubrdiv;
> +   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 0x98000;
>  }
> +
> +unsigned long get_uart_clk(int dev_index)
> +{
> +   return 2400;
> +}
> 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_SHIFT0
> +#define RX_FIFO_COUNT_MASK (0xf << RX_FIFO_COUNT_SHIFT)
> +#define RX_FIFO_FULL   (1 << 8)
> +#define TX_FIFO_COUNT_SHIFT4
> +#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_SHIFT0
>  #define RX_FIFO_COUNT_MASK (0xff << RX_FIFO_COUNT_SHIFT)
>  #define RX_FIFO_FULL   (1 << 8)
>  #define TX_FIFO_COUNT_SHIFT16
>  #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_

[PATCH 2/5] serial: s5p: Add Apple M1 support

2021-09-18 Thread Mark Kettenis 
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 00..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 00..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 intulcon;
+   unsigned intucon;
+   unsigned intufcon;
+   unsigned intumcon;
+   unsigned intutrstat;
+   unsigned intuerstat;
+   unsigned intufstat;
+   unsigned intumstat;
+   unsigned intutxh;
+   unsigned inturxh;
+   unsigned intubrdiv;
+   union br_rest   rest;
+   unsigned char   res3[0x3fd0];
+};
+
+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 0x98000;
 }
+
+unsigned long get_uart_clk(int dev_index)
+{
+   return 2400;
+}
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_SHIFT0
+#define RX_FIFO_COUNT_MASK (0xf << RX_FIFO_COUNT_SHIFT)
+#define RX_FIFO_FULL   (1 << 8)
+#define TX_FIFO_COUNT_SHIFT4
+#define TX_FIFO_COUNT_MASK (0xf << TX_FIFO_COUNT_SHIFT)
+#define TX_FIFO_FULL   (1 << 9)
+#else
 #define RX_FIFO_COUNT_SHIFT0
 #define RX_FIFO_COUNT_MASK (0xff << RX_FIFO_COUNT_SHIFT)
 #define RX_FIFO_FULL   (1 << 8)
 #define TX_FIFO_COUNT_SHIFT16
 #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);
+#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