[PATCH RFC 4/9] serial: serial_meson: add minimal non-DM driver

[Simon Glass]

Hi Ferass,

On Thu, 11 Sept 2025 at 09:37, Ferass El Hafidi
<funderscore at postmarketos.org> wrote:
>
> On Wed Sep 10, 2025 at 8:30 PM UTC, Heinrich Schuchardt wrote:
> > On 9/10/25 22:14, Ferass El Hafidi wrote:
> >> On Wed Sep 10, 2025 at 7:53 PM UTC, Heinrich Schuchardt wrote:
> >>> <...>
> >>>> +static int meson_serial_getc(void)
> >>>> +{
> >>>> +  struct meson_uart *const uart = (struct meson_uart *)CONFIG_VAL(DEBUG_UART_BASE);
> >>>> +  uint32_t status = readl(&uart->status);
> >>>> +
> >>>> +  if (status & AML_UART_RX_EMPTY)
> >>>> +          return -EAGAIN;
> >>>> +
> >>>> +  if (status & AML_UART_ERR) {
> >>>> +          u32 val = readl(&uart->control);
> >>>> +
> >>>> +          /* Clear error */
> >>>> +          val |= AML_UART_CLR_ERR;
> >>>> +          writel(val, &uart->control);
> >>>> +          val &= ~AML_UART_CLR_ERR;
> >>>> +          writel(val, &uart->control);
> >>>> +
> >>>> +          /* Remove spurious byte from fifo */
> >>>> +          readl(&uart->rfifo);
> >>>> +          return -EIO;
> >>>> +  }
> >>>> +
> >>>> +  return readl(&uart->rfifo) & 0xff;
> >>>> +}
> >>>> +
> >>>> +static int meson_serial_tstc(void)
> >>>> +{
> >>>> +  struct meson_uart *const uart = (struct meson_uart *)CONFIG_VAL(DEBUG_UART_BASE);
> >>>
> >>> CONFIG_SPL_DEBUG_UART_BASE only exists for CONFIG_DEBUG_UART=y and
> >>> CONFIG_SPL=y. Shall all configurations depend on CONFIG_DEBUG_UART=y?
> >>>
> >>> Creating this dependency does not look right to me.
> >>>
> >>
> >> All Amlogic defconfigs have CONFIG_DEBUG_UART enabled, but I can remove
> >> the dependency on CONFIG_DEBUG_UART_BASE if you think it doesn't look
> >> right.
> >
> > We should avoid the possibility of build failures for changed configuration.
> >
> > Will CONFIG_MESON_SERIAL depend on CONFIG_DEBUG_UART
> > or will CONFIG_ARCH_MESON select CONFIG_DEBUG_UART?
>
> Then it might be best to not use DEBUG_UART_BASE.  I'll fix that for
> next revision.  Thanks!

I've been thinking about doing something about a minimal UART (and
i2c) for a while.

Before driver model we just called serial_putc() and *the* serial
driver would handle it. Can we do something similar here?

My suggestion for an API:

CONFIG_SERIAL_ONE to indicate that driver model is not used for serial

Then in serial-uclass.c update serial_putc() to use that config and
call serial_one_putc(int ch), with that function being defined in the
only serial driver that is compiled in. Update serial_init() to call
serial_one_init().

That way we keep things compatible with driver model and avoid
building on the old serial.c

This is similar to how the debug UART, but the debug UART is special.

Regards,
Simon