On Thu, 27 Aug 2026 at 07:17, Daniel Henrique Barboza <[email protected]> wrote: > > Add a common uart FDT helper to be used by 'virt' and 'tt-atlantis'. > > To accomodate both boards the helper is doing the following: > - an 'uses_32_bit_spacing' flag controls whether we need to set a > different uart reg spacing for the 'tt-atlantis' board;
Once the designware uart is merged, Atlantis will switch to using that as it matches the real hardware. As part of that we will switch the compatible too, and at that point it might be best leaving it in the board file? > - an 'is_serial0' flag is added to control whether we need to set > additional properties related to the first serial. This is required > because the 'virt' board adds two uarts in the FDT. > > No FDT changes intended. > > Signed-off-by: Daniel Henrique Barboza <[email protected]> > Reviewed-by: Anirudh Srinivasan <[email protected]> > Reviewed-by: Philippe Mathieu-Daudé <[email protected]> > --- > hw/riscv/fdt-common.c | 39 +++++++++++++++++++++++++++++++ > hw/riscv/tt_atlantis.c | 23 ++---------------- > hw/riscv/virt.c | 44 +++++++---------------------------- > include/hw/riscv/fdt-common.h | 4 ++++ > 4 files changed, 53 insertions(+), 57 deletions(-) > > diff --git a/hw/riscv/fdt-common.c b/hw/riscv/fdt-common.c > index e6bb107309..fb7098f213 100644 > --- a/hw/riscv/fdt-common.c > +++ b/hw/riscv/fdt-common.c > @@ -795,3 +795,42 @@ void riscv_create_fdt_socket_aclint(void *fdt, > ACLINTFdtProps *props, > g_free(name); > } > } > + > +void riscv_create_fdt_uart(void *fdt, const MemMapEntry *uart_mem, > + int uart_irq, int aia_type, > + bool uses_32_bit_spacing, bool is_serial0, > + uint32_t irq_mmio_phandle) What does 'mmio' mean here? Perhaps irq_phandle? > +{ > + g_autofree char *name = NULL; > + > + name = g_strdup_printf("/soc/serial@%"HWADDR_PRIx, uart_mem->base); > + qemu_fdt_add_subnode(fdt, name); > + qemu_fdt_setprop_string(fdt, name, "compatible", "ns16550a"); > + qemu_fdt_setprop_sized_cells(fdt, name, "reg", > + 2, uart_mem->base, > + 2, uart_mem->size); > + > + /* > + * The tt-atlantis board uses an uart that has 32 bit > + * register spacing (0x0, 0x4, 0x8, 0xc....) rather than > + * 8 bit spacing like 'virt'. >From a survey of the linux source tree, most machines use reg-shift=2. If we adopted this for the virt machine then you wouldn't need to special case it, with the side effect that you'll match the layout more likely to be in hardware. > + */ > + if (uses_32_bit_spacing) { > + qemu_fdt_setprop_cell(fdt, name, "reg-shift", 2); > + qemu_fdt_setprop_cell(fdt, name, "reg-io-width", 4); > + } > + > + qemu_fdt_setprop_cell(fdt, name, "clock-frequency", 3686400); Hmmm. > + qemu_fdt_setprop_cell(fdt, name, "interrupt-parent", irq_mmio_phandle); > + > + if (aia_type == AIA_TYPE_NONE) { > + qemu_fdt_setprop_cell(fdt, name, "interrupts", uart_irq); > + } else { > + qemu_fdt_setprop_cells(fdt, name, "interrupts", uart_irq, 0x4); > + } > + > + if (is_serial0) { > + qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", name); This is related to but not really part of the UART. Should it stay in the machine, or be present as a separate helper? > + qemu_fdt_setprop_string(fdt, "/aliases", "serial0", name); How about passing a char* alias, which can be NULL: if (alais) { qemu_fdt_setprop_string(fdt, "/aliases", alias, name); } ...might pay to check that /aliases exists too? The ordering was enforced when this is part of a board, but now that they are APIs called from anywhere.
