Re: [PATCH v5 1/7] serial: Add Goldfish TTY driver

[Kuan-Wei Chiu <visitorckw@gmail.com>]

Hi Simon,

On Mon, Jan 05, 2026 at 04:30:28PM -0700, Simon Glass wrote:
> Hi Kuan-Wei,
> 
> On Mon, 5 Jan 2026 at 11:21, Kuan-Wei Chiu <visitorckw@gmail.com> 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>
> > Reviewed-by: Yao Zi <me@ziyao.cc>
> > Tested-by: Daniel Palmer <daniel@0x0f.com>
> > Acked-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > ---
> > Changes in v5:
> > - Rebase on u-boot/next branch.
> >
> >  MAINTAINERS                      |   6 ++
> >  drivers/serial/Kconfig           |   8 +++
> >  drivers/serial/Makefile          |   1 +
> >  drivers/serial/serial_goldfish.c | 109 +++++++++++++++++++++++++++++++
> >  include/goldfish_tty.h           |  18 +++++
> >  5 files changed, 142 insertions(+)
> >  create mode 100644 drivers/serial/serial_goldfish.c
> >  create mode 100644 include/goldfish_tty.h
> >
> 
> Reviewed-by: Simon Glass <simon.glass@canonical.com>

Thanks for your review!

> 
> One thought below.
> 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 27ce73d83f4..8f884ff495a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1260,6 +1260,12 @@ S:       Maintained
> >  F:     drivers/misc/gsc.c
> >  F:     include/gsc.h
> >
> > +GOLDFISH SERIAL DRIVER
> > +M:     Kuan-Wei Chiu <visitorckw@gmail.com>
> > +S:     Maintained
> > +F:     drivers/serial/serial_goldfish.c
> > +F:     include/goldfish_tty.h
> > +
> >  INTERCONNECT:
> >  M:     Neil Armstrong <neil.armstrong@linaro.org>
> >  S:     Maintained
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > index 371d7aa5bba..b84cb9ec781 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -1193,4 +1193,12 @@ config SYS_SDMR
> >         depends on MPC8XX_CONS
> >         default 0x0
> >
> > +config SERIAL_GOLDFISH
> > +       bool "Goldfish TTY support"
> > +       depends on DM_SERIAL
> > +       help
> > +         Select this to enable support for the Goldfish TTY serial port.
> > +         This virtual device is commonly used by QEMU virtual machines
> > +         (e.g. m68k virt) for console output.
> > +
> >  endif
> > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> > index 8eaae62b0fc..fe8d23be512 100644
> > --- a/drivers/serial/Makefile
> > +++ b/drivers/serial/Makefile
> > @@ -63,3 +63,4 @@ obj-$(CONFIG_XTENSA_SEMIHOSTING_SERIAL) += serial_xtensa_semihosting.o
> >  obj-$(CONFIG_S5P4418_PL011_SERIAL) += serial_s5p4418_pl011.o
> >
> >  obj-$(CONFIG_UART4_SERIAL) += serial_adi_uart4.o
> > +obj-$(CONFIG_SERIAL_GOLDFISH) += serial_goldfish.o
> > diff --git a/drivers/serial/serial_goldfish.c b/drivers/serial/serial_goldfish.c
> > new file mode 100644
> > index 00000000000..278d0cf3e0d
> > --- /dev/null
> > +++ b/drivers/serial/serial_goldfish.c
> > @@ -0,0 +1,109 @@
> > +// 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 <mapmem.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 paddr;
> > +       u32 count;
> > +
> > +       count = __raw_readl(priv->base + GOLDFISH_TTY_BYTES_READY);
> > +       if (count == 0)
> > +               return -EAGAIN;
> > +
> > +       paddr = virt_to_phys((void *)&priv->rx_buf);
> > +
> > +       __raw_writel(0, priv->base + GOLDFISH_TTY_DATA_PTR_HIGH);
> > +       __raw_writel(paddr, priv->base + GOLDFISH_TTY_DATA_PTR);
> > +       __raw_writel(1, priv->base + GOLDFISH_TTY_DATA_LEN);
> > +       __raw_writel(CMD_READ_BUFFER, priv->base + GOLDFISH_TTY_CMD);
> > +
> > +       return priv->rx_buf;
> > +}
> > +
> > +static int goldfish_serial_putc(struct udevice *dev, const char ch)
> > +{
> > +       struct goldfish_tty_priv *priv = dev_get_priv(dev);
> > +
> > +       __raw_writel(ch, priv->base + GOLDFISH_TTY_PUT_CHAR);
> > +
> > +       return 0;
> > +}
> > +
> > +static int goldfish_serial_pending(struct udevice *dev, bool input)
> > +{
> > +       struct goldfish_tty_priv *priv = dev_get_priv(dev);
> > +
> > +       if (input)
> > +               return __raw_readl(priv->base + GOLDFISH_TTY_BYTES_READY);
> > +
> > +       return 0;
> > +}
> > +
> > +static int goldfish_serial_probe(struct udevice *dev)
> > +{
> > +       struct goldfish_tty_priv *priv = dev_get_priv(dev);
> > +       struct goldfish_tty_plat *plat = dev_get_plat(dev);
> > +       fdt_addr_t addr;
> > +
> > +       addr = dev_read_addr(dev);
> > +       if (addr != FDT_ADDR_T_NONE) {
> > +               priv->base = map_sysmem(addr, 0x1000);
> > +       } else {
> > +               plat = dev_get_plat(dev);
> > +               if (!plat)
> > +                       return -EINVAL;
> > +               priv->base = plat->base;
> > +       }
> > +
> > +       return 0;
> > +}
> 
> Strictly speaking, reading the devicetree should be done in the
> of_to_plat() method. So you would read the base address there.
> 
> Then probe() would convert that to a pointer (I suggest the same 'reg'
> for registers, but it isn't that important).

I will move the device tree reading to of_to_plat() and rename the
platform data member to reg (using phys_addr_t). The probe() function
will then handle the map_sysmem() conversion. I will also apply this
same pattern to the timer and rtc drivers.

Regards,
Kuan-Wei

> 
> > +
> > +static const struct dm_serial_ops goldfish_serial_ops = {
> > +       .putc = goldfish_serial_putc,
> > +       .pending = goldfish_serial_pending,
> > +       .getc = goldfish_serial_getc,
> > +};
> > +
> > +static const struct udevice_id goldfish_serial_ids[] = {
> > +       { .compatible = "google,goldfish-tty" },
> > +       { }
> > +};
> > +
> > +U_BOOT_DRIVER(serial_goldfish) = {
> > +       .name   = "serial_goldfish",
> > +       .id     = UCLASS_SERIAL,
> > +       .of_match = goldfish_serial_ids,
> > +       .probe  = goldfish_serial_probe,
> > +       .ops    = &goldfish_serial_ops,
> > +       .priv_auto = sizeof(struct goldfish_tty_priv),
> > +       .flags  = DM_FLAG_PRE_RELOC,
> > +};
> > diff --git a/include/goldfish_tty.h b/include/goldfish_tty.h
> > new file mode 100644
> > index 00000000000..8777da4942d
> > --- /dev/null
> > +++ b/include/goldfish_tty.h
> > @@ -0,0 +1,18 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2025, Kuan-Wei Chiu <visitorckw@gmail.com>
> > + */
> > +
> > +#ifndef _GOLDFISH_TTY_H_
> > +#define _GOLDFISH_TTY_H_
> > +
> > +#include <linux/types.h>
> > +
> > +/* Platform data for the Goldfish TTY driver
> > + * Used to pass hardware base address from Board to Driver
> > + */
> > +struct goldfish_tty_plat {
> > +       void __iomem *base;
> > +};
> > +
> > +#endif /* _GOLDFISH_TTY_H_ */
> > --
> > 2.52.0.358.g0dd7633a29-goog
> >
> 
> Regards,
> Simon