Re: [PATCH 2/2] hppa: Fix serial port pass-through

[Peter Maydell]

On Thu, 26 May 2022 at 12:52, Helge Deller <deller@gmx.de> wrote:
>
> This fixes the serial ports in the emulation to behave as on original
> hardware.
>
> On the real hardware, the LASI UART is serial port #0 and the DINO UART
> is serial port #1. This is fixed in SEABIOS_HPPA_VERSION >= 6, which is
> why the latest firmware is required.
>
> The serial port addresses in hppa/hppa_hardware.h are swapped and the
> file is synced with the version in SeaBIOS-hppa. Please note that this
> file is shared between qemu and SeaBIOS-hppa, which is why a comment was
> added at the top of the file.
>
> When creating the virtual serial ports the correct port addresses are
> now used.
>
> Finally, this patch now allows the user to give:
>     -serial mon:stdio -serial /dev/ttyS4
> to use emulated ttyS0 in the guest for console output, and pass ttyS4
> from the host to ttyS1 in the guest.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> @@ -236,18 +236,15 @@ static void machine_hppa_init(MachineState *machine)
>      /* Realtime clock, used by firmware for PDC_TOD call. */
>      mc146818_rtc_init(isa_bus, 2000, NULL);
>
> -    /* Serial code setup.  */
> +    /* Serial ports - Lasi and Dino use a 7.272727 MHz clock. */
>      if (serial_hd(0)) {
> -        uint32_t addr = DINO_UART_HPA + 0x800;
> -        serial_mm_init(addr_space, addr, 0,
> -                       qdev_get_gpio_in(dino_dev, DINO_IRQ_RS232INT),
> -                       115200, serial_hd(0), DEVICE_BIG_ENDIAN);
> +        serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0,
> +                qdev_get_gpio_in(lasi_dev, LASI_IRQ_UART_HPA), 7272727 / 16,
> +                serial_hd(0), DEVICE_BIG_ENDIAN);
>      }
> -
>      if (serial_hd(1)) {
> -        /* Serial port */
> -        serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0,
> -                qdev_get_gpio_in(lasi_dev, LASI_IRQ_UART_HPA), 8000000 / 16,
> +        serial_mm_init(addr_space, DINO_UART_HPA + 0x800, 0,
> +                qdev_get_gpio_in(dino_dev, DINO_IRQ_RS232INT), 7272727 / 16,
>                  serial_hd(1), DEVICE_BIG_ENDIAN);
>      }

Not related to this change, but you should consider removing these
"if (serial_hd(n))" conditionals. They used to be necessary because
the chardev backend infrastructure couldn't cope with being handed
a NULL pointer, but these days that is guaranteed to work (with
a NULL pointer being equivalent to the 'throw everything away'
null chardev), so the if()s are no longer necessary. They also
mean that the guest sees the hardware it expects, ie the UARTs
are always there and programmable whether there's an RS232 cable
plugged in the back of the machine or not.

thanks
-- PMM