Re: [PULL 1/2] hw/openrisc/openrisc_sim: keep serial@90000000 as default

[Ahmad Fatoum]

Hello Stafford,

On 03.12.24 12:15, Stafford Horne wrote:
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> [1]: 
> https://lore.barebox.org/barebox/707e7c50-aad1-4459-8796-0cc54bab32e2@pengutronix.de/T/#m5da26e8a799033301489a938b5d5667b81cef6ad
> 
> Fixes: 777784bda468 ("hw/openrisc: support 4 serial ports in or1ksim")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> [stafford: Change to serial0 alias and update change message, reverse
>  uart registration order]

Thanks a lot for your subsequent iterations on the patch.
The changes look good to me.

Thanks,
Ahmad

> Signed-off-by: Stafford Horne <shorne@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/openrisc/openrisc_sim.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index 9fb63515ef..42f002985b 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -250,7 +250,7 @@ static void openrisc_sim_serial_init(Or1ksimState *state, 
> hwaddr base,
>      void *fdt = state->fdt;
>      char *nodename;
>      qemu_irq serial_irq;
> -    char alias[sizeof("uart0")];
> +    char alias[sizeof("serial0")];
>      int i;
>  
>      if (num_cpus > 1) {
> @@ -265,7 +265,7 @@ static void openrisc_sim_serial_init(Or1ksimState *state, 
> hwaddr base,
>          serial_irq = get_cpu_irq(cpus, 0, irq_pin);
>      }
>      serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
> -                   serial_hd(OR1KSIM_UART_COUNT - uart_idx - 1),
> +                   serial_hd(uart_idx),
>                     DEVICE_NATIVE_ENDIAN);
>  
>      /* Add device tree node for serial. */
> @@ -277,10 +277,13 @@ static void openrisc_sim_serial_init(Or1ksimState 
> *state, hwaddr base,
>      qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", OR1KSIM_CLK_MHZ);
>      qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0);
>  
> -    /* The /chosen node is created during fdt creation. */
> -    qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
> -    snprintf(alias, sizeof(alias), "uart%d", uart_idx);
> +    if (uart_idx == 0) {
> +        /* The /chosen node is created during fdt creation. */
> +        qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
> +    }
> +    snprintf(alias, sizeof(alias), "serial%d", uart_idx);
>      qemu_fdt_setprop_string(fdt, "/aliases", alias, nodename);
> +
>      g_free(nodename);
>  }
>  
> @@ -326,11 +329,22 @@ static void openrisc_sim_init(MachineState *machine)
>                                  smp_cpus, cpus, OR1KSIM_OMPIC_IRQ);
>      }
>  
> -    for (n = 0; n < OR1KSIM_UART_COUNT; ++n)
> +    /*
> +     * We create the UART nodes starting with the highest address and
> +     * working downwards, because in QEMU the DTB nodes end up in the
> +     * DTB in reverse order of creation. Correctly-written guest software
> +     * will not care about the node order (it will look at stdout-path
> +     * or the alias nodes), but for the benefit of guest software which
> +     * just looks for the first UART node in the DTB, make sure the
> +     * lowest-address UART (which is QEMU's first serial port) appears
> +     * first in the DTB.
> +     */
> +    for (n = OR1KSIM_UART_COUNT - 1; n >= 0; n--) {
>          openrisc_sim_serial_init(state, or1ksim_memmap[OR1KSIM_UART].base +
>                                          or1ksim_memmap[OR1KSIM_UART].size * 
> n,
>                                   or1ksim_memmap[OR1KSIM_UART].size,
>                                   smp_cpus, cpus, OR1KSIM_UART_IRQ, n);
> +    }
>  
>      load_addr = openrisc_load_kernel(ram_size, kernel_filename,
>                                       &boot_info.bootstrap_pc);


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |