Re: [PATCH v3 1/6] serial: Add Goldfish TTY driver

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

Hi Heinrich,

On Wed, Dec 31, 2025 at 01:25:10PM +0100, Heinrich Schuchardt wrote:
> On 12/30/25 17:01, 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 v3:
> > - Remove unnecessary type casts for MMIO register access.
> > 
> >   MAINTAINERS                      |   6 ++
> >   drivers/serial/Kconfig           |   8 +++
> >   drivers/serial/Makefile          |   1 +
> >   drivers/serial/serial_goldfish.c | 102 +++++++++++++++++++++++++++++++
> >   include/goldfish_tty.h           |  18 ++++++
> >   5 files changed, 135 insertions(+)
> >   create mode 100644 drivers/serial/serial_goldfish.c
> >   create mode 100644 include/goldfish_tty.h
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 6ce0bbce13d..da4a6e4d518 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1259,6 +1259,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
> > +
> >   I2C
> >   M:	Heiko Schocher <hs@nabladev.com>
> >   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..1d2977d809d
> > --- /dev/null
> > +++ b/drivers/serial/serial_goldfish.c
> > @@ -0,0 +1,102 @@
> > +// 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 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);
> > +
> > +	if (!plat)
> > +		return -EINVAL;
> > +
> > +	priv->base = plat->base;
> 
> The Linux binding documentation in
> Documentation/devicetree/bindings/goldfish/tty.txt has this example:
> 
>         goldfish_tty@1f004000 {
>                 compatible = "google,goldfish-tty";
>                 reg = <0x1f004000 0x1000>;
>                 interrupts = <0xc>;
>         };
> 
> The driver should take the reg value as address and use plat->base only as
> fallback for devices that don't provide a proper device-tree.
> 
> We already do this for the RTC driver. Please, apply the same logic to all
> Goldfish drivers.

Agreed.
Will do this for both tty and timer drivers in the next version.

Regards,
Kuan-Wei