Am Fri, 8 Mar 2024 10:08:12 +0100
schrieb Angelo Dureghello <ang...@kernel-space.org>:
Add reset support for mcf5208.
Signed-off-by: Angelo Dureghello <ang...@kernel-space.org>
---
hw/m68k/mcf5208.c | 49 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index 0cfb806c20..d8a38274d0 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -40,6 +40,8 @@
#define PCSR_PRE_SHIFT 8
#define PCSR_PRE_MASK 0x0f00
+#define RCR_SOFTRST 0x80
+
typedef struct {
MemoryRegion iomem;
qemu_irq irq;
@@ -185,13 +187,54 @@ static const MemoryRegionOps m5208_sys_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic)
+static uint64_t m5208_rcm_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
+ __func__, addr);
I'd maybe rather not print this message if the guest tries to read from RCR
or RSR - reading back a 0 should be fine here without logging a "bad
offset".
+ return 0;
+}
+
+static void m5208_rcm_write(void *opaque, hwaddr addr,
+ uint64_t value, unsigned size)
+{
+ M68kCPU *cpu = opaque;
+ CPUState *cs = CPU(cpu);
+ switch (addr) {
+ case 0x0: /* RCR */
+ if (value & RCR_SOFTRST) {
+ cpu_reset(cs);
+ cpu->env.aregs[7] = ldl_phys(cs->as, 0);
+ cpu->env.pc = ldl_phys(cs->as, 4);
+ }
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
+ __func__, addr);
+ break;
+ }
+}
+
+static const MemoryRegionOps m5208_rcm_ops = {
+ .read = m5208_rcm_read,
+ .write = m5208_rcm_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic,
+ M68kCPU *cpu)
{
- MemoryRegion *iomem = g_new(MemoryRegion, 1);
+ MemoryRegion *iomem;
I think it's more common coding style in the QEMU sources to have separate
variables for separate MemoryRegions instead of re-using one variable for
separate MemoryRegions like you do it below. See e.g.
microchip_pfsoc_soc_realize() in hw/riscv/microchip_pfsoc.c.
m5208_timer_state *s;
int i;
+ /* RCM */
+ iomem = g_new(MemoryRegion, 1);
+ memory_region_init_io(iomem, NULL, &m5208_rcm_ops, cpu,
+ "m5208-rcm", 0x00000080);
+ memory_region_add_subregion(address_space, 0xfc0a0000, iomem);
/* SDRAMC. */
+ iomem = g_new(MemoryRegion, 1);
memory_region_init_io(iomem, NULL, &m5208_sys_ops, NULL, "m5208-sys",
0x00004000);
memory_region_add_subregion(address_space, 0xfc0a8000, iomem);
/* Timers. */
@@ -265,7 +308,7 @@ static void mcf5208evb_init(MachineState *machine)
mcf_uart_create_mmap(0xfc064000, pic[27], serial_hd(1));
mcf_uart_create_mmap(0xfc068000, pic[28], serial_hd(2));
- mcf5208_sys_init(address_space_mem, pic);
+ mcf5208_sys_init(address_space_mem, pic, cpu);
mcf_fec_init(address_space_mem, 0xfc030000, pic + 36);
All in all, patch looks fine to me ... if you could update the above two
nits by Monday, I can try to get this merged before the next soft freeze of
QEMU (on Tuesday).
