Re: [PATCH v2 4/7] serial: s5p: Add Apple M1 support

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> From: Simon Glass <sjg@chromium.org>
> Date: Mon, 11 Oct 2021 11:00:39 -0600
> 
> Hi Mark,
> 
> On Sun, 3 Oct 2021 at 12:33, 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/Kconfig                    |   1 +
> >  arch/arm/include/asm/arch-m1/uart.h |  41 +++++++++++
> >  configs/apple_m1_defconfig          |   4 ++
> >  drivers/serial/Kconfig              |   4 +-
> >  drivers/serial/serial_s5p.c         | 104 ++++++++++++++++++++++------
> >  5 files changed, 130 insertions(+), 24 deletions(-)
> >  create mode 100644 arch/arm/include/asm/arch-m1/uart.h
> >
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> [..]
> 
> > diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c
> > index 6d09952a5d..53a7b0bd1b 100644
> > --- a/drivers/serial/serial_s5p.c
> > +++ b/drivers/serial/serial_s5p.c
> [..]
> > @@ -93,7 +116,7 @@ int s5p_serial_setbrg(struct udevice *dev, int baudrate)
> >         struct s5p_uart *const uart = plat->reg;
> >         u32 uclk;
> >
> > -#ifdef CONFIG_CLK_EXYNOS
> > +#if CONFIG_IS_ENABLED(CLK_EXYNOS) || CONFIG_IS_ENABLED(ARCH_APPLE)
> 
> I really don't like making this any worse.
> 
> Does this work?
> 
> ret = clk_get_by_index(...)
> if (ret && ret != -ENOSYS)
>    return ret

The problem really is that I'm trying to avoid having to define
get_uart_clock() for the new platform.  So I need an #if of some sorts
to avoid that code.  I really think the current diff is the cleanest I
can come up.
 
> >         struct clk clk;
> >         u32 ret;
> >
> > @@ -105,7 +128,7 @@ int s5p_serial_setbrg(struct udevice *dev, int baudrate)
> >         uclk = get_uart_clk(plat->port_id);
> >  #endif
> >
> > -       s5p_serial_baud(uart, uclk, baudrate);
> > +       s5p_serial_baud(uart, plat->reg_width, uclk, baudrate);
> >
> >         return 0;
> >  }
> [..]
> 
> Regards,
> Simon
>