Re: [PATCH v2 1/4] serial: Add Goldfish TTY driver

Yao Zi
2025-12-27T02:08:05Z

On Fri, Dec 26, 2025 at 05:53:57PM +0000, Kuan-Wei Chiu wrote:
> Add support for the Google Goldfish TTY serial device. This virtual
> device is commonly used in QEMU virtual machines (such as the m68k
> virt machine) and Android emulators.
>
> The driver implements basic console output and input polling using the
> Goldfish MMIO interface.
>
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> ---
> Changes in v2:
> - Update SPDX license identifier to GPL-2.0-or-later.
> - Sort header inclusions alphabetically.
> - Move RX buffer into goldfish_tty_priv instead of using a static buffer.
> - Make sure getc only read a single byte at a time.
>
> MAINTAINERS | 6 ++
> drivers/serial/Kconfig | 8 +++
> drivers/serial/Makefile | 1 +
> drivers/serial/serial_goldfish.c | 104 +++++++++++++++++++++++++++++++
> include/goldfish_tty.h | 18 ++++++
> 5 files changed, 137 insertions(+)
> create mode 100644 drivers/serial/serial_goldfish.c
> create mode 100644 include/goldfish_tty.h

...

> diff --git a/drivers/serial/serial_goldfish.c b/drivers/serial/serial_goldfish.c
> new file mode 100644
> index 00000000000..ce5bff6bf4c
> --- /dev/null
> +++ b/drivers/serial/serial_goldfish.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025, Kuan-Wei Chiu <visitorckw@gmail.com>
> + * Goldfish TTY driver for U-Boot
> + */
> +
> +#include <asm/io.h>
> +#include <dm.h>
> +#include <goldfish_tty.h>
> +#include <linux/types.h>
> +#include <serial.h>
> +
> +/* Goldfish TTY Register Offsets */
> +#define GOLDFISH_TTY_PUT_CHAR 0x00
> +#define GOLDFISH_TTY_BYTES_READY 0x04
> +#define GOLDFISH_TTY_CMD 0x08
> +#define GOLDFISH_TTY_DATA_PTR 0x10
> +#define GOLDFISH_TTY_DATA_LEN 0x14
> +#define GOLDFISH_TTY_DATA_PTR_HIGH 0x18
> +#define GOLDFISH_TTY_VERSION 0x20
> +
> +/* Commands */
> +#define CMD_WRITE_BUFFER 2
> +#define CMD_READ_BUFFER 3
> +
> +struct goldfish_tty_priv {
> + void __iomem *base;
> + u8 rx_buf;
> +};
> +
> +static int goldfish_serial_getc(struct udevice *dev)
> +{
> + struct goldfish_tty_priv *priv = dev_get_priv(dev);
> + unsigned long base = (unsigned long)priv->base;
> + unsigned long paddr;
> + u32 count;
> +
> + count = __raw_readl((void *)(base + GOLDFISH_TTY_BYTES_READY));

I think it's okay to do pointer arithmetic directly against priv->base
in GNU C, right? In which case void * is treated like char *.

If I'm correct, variable base could be dropped, and we could avoid some
casts here and below.

> + if (count == 0)
> + return -EAGAIN;
> +
> + paddr = virt_to_phys((void *)&priv->rx_buf);
> +
> + __raw_writel(0, (void *)(base + GOLDFISH_TTY_DATA_PTR_HIGH));
> + __raw_writel(paddr, (void *)(base + GOLDFISH_TTY_DATA_PTR));
> + __raw_writel(1, (void *)(base + GOLDFISH_TTY_DATA_LEN));
> +
> + __raw_writel(CMD_READ_BUFFER, (void *)(base + GOLDFISH_TTY_CMD));
> +
> + return priv->rx_buf;
> +}

Regards,
Yao Zi