[RFC PATCH] hw/arm/musicpal: Map the UART devices unconditionally

Philippe Mathieu-Daudé posted 1 patch 5 years, 6 months ago
Test docker-mingw@fedora failed
Test checkpatch failed
Test asan failed
Test docker-quick@centos7 failed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200505095945.23146-1-f4bug@amsat.org
hw/arm/musicpal.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
[RFC PATCH] hw/arm/musicpal: Map the UART devices unconditionally
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago
I can't find proper documentation or datasheet, but it is likely
a MMIO mapped serial device mapped in the 0x80000000..0x8000ffff
range belongs to the SoC address space, thus is always mapped in
the memory bus.
Map the devices on the bus regardless a chardev is attached to it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
from 2019... found while doing housekeeping
---
 hw/arm/musicpal.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index b2d0cfdac8..92f33ed87e 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1619,14 +1619,10 @@ static void musicpal_init(MachineState *machine)
                           pic[MP_TIMER2_IRQ], pic[MP_TIMER3_IRQ],
                           pic[MP_TIMER4_IRQ], NULL);
 
-    if (serial_hd(0)) {
-        serial_mm_init(address_space_mem, MP_UART1_BASE, 2, pic[MP_UART1_IRQ],
-                       1825000, serial_hd(0), DEVICE_NATIVE_ENDIAN);
-    }
-    if (serial_hd(1)) {
-        serial_mm_init(address_space_mem, MP_UART2_BASE, 2, pic[MP_UART2_IRQ],
-                       1825000, serial_hd(1), DEVICE_NATIVE_ENDIAN);
-    }
+    serial_mm_init(address_space_mem, MP_UART1_BASE, 2, pic[MP_UART1_IRQ],
+                   1825000, serial_hd(0), DEVICE_NATIVE_ENDIAN);
+    serial_mm_init(address_space_mem, MP_UART2_BASE, 2, pic[MP_UART2_IRQ],
+                   1825000, serial_hd(1), DEVICE_NATIVE_ENDIAN);
 
     /* Register flash */
     dinfo = drive_get(IF_PFLASH, 0, 0);
-- 
2.21.3


Re: [RFC PATCH] hw/arm/musicpal: Map the UART devices unconditionally
Posted by Jan Kiszka 5 years, 6 months ago
On 05.05.20 11:59, Philippe Mathieu-Daudé wrote:
> I can't find proper documentation or datasheet, but it is likely
> a MMIO mapped serial device mapped in the 0x80000000..0x8000ffff
> range belongs to the SoC address space, thus is always mapped in
> the memory bus.
> Map the devices on the bus regardless a chardev is attached to it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> from 2019... found while doing housekeeping
> ---
>   hw/arm/musicpal.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
> index b2d0cfdac8..92f33ed87e 100644
> --- a/hw/arm/musicpal.c
> +++ b/hw/arm/musicpal.c
> @@ -1619,14 +1619,10 @@ static void musicpal_init(MachineState *machine)
>                             pic[MP_TIMER2_IRQ], pic[MP_TIMER3_IRQ],
>                             pic[MP_TIMER4_IRQ], NULL);
>
> -    if (serial_hd(0)) {
> -        serial_mm_init(address_space_mem, MP_UART1_BASE, 2, pic[MP_UART1_IRQ],
> -                       1825000, serial_hd(0), DEVICE_NATIVE_ENDIAN);
> -    }
> -    if (serial_hd(1)) {
> -        serial_mm_init(address_space_mem, MP_UART2_BASE, 2, pic[MP_UART2_IRQ],
> -                       1825000, serial_hd(1), DEVICE_NATIVE_ENDIAN);
> -    }
> +    serial_mm_init(address_space_mem, MP_UART1_BASE, 2, pic[MP_UART1_IRQ],
> +                   1825000, serial_hd(0), DEVICE_NATIVE_ENDIAN);
> +    serial_mm_init(address_space_mem, MP_UART2_BASE, 2, pic[MP_UART2_IRQ],
> +                   1825000, serial_hd(1), DEVICE_NATIVE_ENDIAN);
>
>       /* Register flash */
>       dinfo = drive_get(IF_PFLASH, 0, 0);
>

I don't recall details anymore either (more than 10 year ago now...),
but this looks reasonable.

Reviewed-by: Jan Kiszka <jan.kiszka@web.de>

Jan

Re: [RFC PATCH] hw/arm/musicpal: Map the UART devices unconditionally
Posted by Peter Maydell 5 years, 6 months ago
On Tue, 5 May 2020 at 11:09, Jan Kiszka <jan.kiszka@web.de> wrote:
>
> On 05.05.20 11:59, Philippe Mathieu-Daudé wrote:
> I don't recall details anymore either (more than 10 year ago now...),
> but this looks reasonable.

My guess is that it dates back to when the serial code would
crash if passed a NULL pointer for the backend rather than
treating it as "same as if connected to a /dev/null backend".

thanks
-- PMM

Re: [RFC PATCH] hw/arm/musicpal: Map the UART devices unconditionally
Posted by Peter Maydell 5 years, 6 months ago
On Tue, 5 May 2020 at 10:59, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> I can't find proper documentation or datasheet, but it is likely
> a MMIO mapped serial device mapped in the 0x80000000..0x8000ffff
> range belongs to the SoC address space, thus is always mapped in
> the memory bus.
> Map the devices on the bus regardless a chardev is attached to it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> from 2019... found while doing housekeeping
> ---




Applied to target-arm.next, thanks.

-- PMM