[Qemu-devel] [PATCH] sh4: Followup to commit #5849 "Change MMIO callbacks..."

[takasi-y]

Thank you, Paul.

> You should not rely on overlapping regions doing anything sensible.
OK. fixed.

> Proably simpler to just use a single region for both.
Ah, that's right. Now the code is very simple (than I have expected).

Here is the patch. Please apply it.

Fixes to be needed for commit #5849 "Change MMIO callbacks..."
  hw/sh7750.c:
   - Divide region of CPU control registers to avoid overlapping
     to peripheral modules.
   - Delete unused var "icr", which had moved to hw/sh_intc.c.
  hw/sm501.c:
   - Merge non page aligned palette registers into the region of
     control registers.

Signed-off-by: Takashi YOSHII <address@hidden>
---
 hw/sh7750.c |   14 ++++------
 hw/sm501.c  |   82 +++++++++++++++++++++++++---------------------------------
 2 files changed, 41 insertions(+), 55 deletions(-)

diff --git a/hw/sh7750.c b/hw/sh7750.c
index 1d18010..f44e522 100644
--- a/hw/sh7750.c
+++ b/hw/sh7750.c
@@ -58,7 +58,6 @@ typedef struct SH7750State {
     uint16_t periph_portdirb;  /* Direction seen from the peripherals */
     sh7750_io_device *devices[NB_DEVICES];     /* External peripherals */
 
-    uint16_t icr;
     /* Cache */
     uint32_t ccr;
 
@@ -218,8 +217,6 @@ static uint32_t sh7750_mem_readw(void *opaque, 
target_phys_addr_t addr)
        return porta_lines(s);
     case SH7750_PDTRB_A7:
        return portb_lines(s);
-    case 0x1fd00000:
-        return s->icr;
     default:
        error_access("word read", addr);
        assert(0);
@@ -313,9 +310,6 @@ static void sh7750_mem_writew(void *opaque, 
target_phys_addr_t addr,
            assert(0);
        }
        return;
-    case 0x1fd00000:
-        s->icr = mem_value;
-       return;
     default:
        error_access("word write", addr);
        assert(0);
@@ -643,8 +637,12 @@ SH7750State *sh7750_init(CPUSH4State * cpu)
     sh7750_io_memory = cpu_register_io_memory(0,
                                              sh7750_mem_read,
                                              sh7750_mem_write, s);
-    cpu_register_physical_memory_offset(0x1c000000, 0x04000000,
-                                        sh7750_io_memory, 0x1c000000);
+    cpu_register_physical_memory_offset(0x1f000000, 0x1000,
+                                        sh7750_io_memory, 0x1f000000);
+    cpu_register_physical_memory_offset(0x1f800000, 0x1000,
+                                        sh7750_io_memory, 0x1f800000);
+    cpu_register_physical_memory_offset(0x1fc00000, 0x1000,
+                                        sh7750_io_memory, 0x1fc00000);
 
     sh7750_mm_cache_and_tlb = cpu_register_io_memory(0,
                                                     sh7750_mmct_read,
diff --git a/hw/sm501.c b/hw/sm501.c
index de61075..d496614 100644
--- a/hw/sm501.c
+++ b/hw/sm501.c
@@ -638,6 +638,32 @@ static CPUWriteMemoryFunc *sm501_system_config_writefn[] = 
{
     &sm501_system_config_write,
 };
 
+static uint32_t sm501_palette_read(void *opaque, target_phys_addr_t addr)
+{
+    SM501State * s = (SM501State *)opaque;
+    SM501_DPRINTF("sm501 palette read addr=%x\n", (int)addr);
+
+    /* TODO : consider BYTE/WORD access */
+    /* TODO : consider endian */
+
+    assert(0 <= addr && addr < 0x400 * 3);
+    return *(uint32_t*)&s->dc_palette[addr];
+}
+
+static void sm501_palette_write(void *opaque,
+                               target_phys_addr_t addr, uint32_t value)
+{
+    SM501State * s = (SM501State *)opaque;
+    SM501_DPRINTF("sm501 palette write addr=%x, val=%x\n",
+                 (int)addr, value);
+
+    /* TODO : consider BYTE/WORD access */
+    /* TODO : consider endian */
+
+    assert(0 <= addr && addr < 0x400 * 3);
+    *(uint32_t*)&s->dc_palette[addr] = value;
+}
+
 static uint32_t sm501_disp_ctrl_read(void *opaque, target_phys_addr_t addr)
 {
     SM501State * s = (SM501State *)opaque;
@@ -719,6 +745,10 @@ static uint32_t sm501_disp_ctrl_read(void *opaque, 
target_phys_addr_t addr)
        ret = s->dc_crt_hwc_addr;
        break;
 
+    case SM501_DC_PANEL_PALETTE ... SM501_DC_PANEL_PALETTE + 0x400*3 - 4:
+        ret = sm501_palette_read(opaque, addr - SM501_DC_PANEL_PALETTE);
+       break;
+
     default:
        printf("sm501 disp ctrl : not implemented register read."
               " addr=%x\n", (int)addr);
@@ -823,6 +853,10 @@ static void sm501_disp_ctrl_write(void *opaque,
        s->dc_crt_hwc_addr = value & 0x0000FFFF;
        break;
 
+    case SM501_DC_PANEL_PALETTE ... SM501_DC_PANEL_PALETTE + 0x400*3 - 4:
+        sm501_palette_write(opaque, addr - SM501_DC_PANEL_PALETTE, value);
+       break;
+
     default:
        printf("sm501 disp ctrl : not implemented register write."
               " addr=%x, val=%x\n", (int)addr, value);
@@ -842,45 +876,6 @@ static CPUWriteMemoryFunc *sm501_disp_ctrl_writefn[] = {
     &sm501_disp_ctrl_write,
 };
 
-static uint32_t sm501_palette_read(void *opaque, target_phys_addr_t addr)
-{
-    SM501State * s = (SM501State *)opaque;
-    SM501_DPRINTF("sm501 palette read addr=%x\n", (int)addr);
-
-    /* TODO : consider BYTE/WORD access */
-    /* TODO : consider endian */
-
-    assert(0 <= addr && addr < 0x400 * 3);
-    return *(uint32_t*)&s->dc_palette[addr];
-}
-
-static void sm501_palette_write(void *opaque,
-                               target_phys_addr_t addr, uint32_t value)
-{
-    SM501State * s = (SM501State *)opaque;
-    SM501_DPRINTF("sm501 palette write addr=%x, val=%x\n",
-                 (int)addr, value);
-
-    /* TODO : consider BYTE/WORD access */
-    /* TODO : consider endian */
-
-    assert(0 <= addr && addr < 0x400 * 3);
-    *(uint32_t*)&s->dc_palette[addr] = value;
-}
-
-static CPUReadMemoryFunc *sm501_palette_readfn[] = {
-    &sm501_palette_read,
-    &sm501_palette_read,
-    &sm501_palette_read,
-};
-
-static CPUWriteMemoryFunc *sm501_palette_writefn[] = {
-    &sm501_palette_write,
-    &sm501_palette_write,
-    &sm501_palette_write,
-};
-
-
 /* draw line functions for all console modes */
 
 #include "pixel_ops.h"
@@ -1070,7 +1065,6 @@ void sm501_init(DisplayState *ds, uint32_t base, unsigned 
long local_mem_base,
     SM501State * s;
     int sm501_system_config_index;
     int sm501_disp_ctrl_index;
-    int sm501_palette_index;
 
     /* allocate management data region */
     s = (SM501State *)qemu_mallocz(sizeof(SM501State));
@@ -1098,13 +1092,7 @@ void sm501_init(DisplayState *ds, uint32_t base, 
unsigned long local_mem_base,
     sm501_disp_ctrl_index = cpu_register_io_memory(0, sm501_disp_ctrl_readfn,
                                                   sm501_disp_ctrl_writefn, s);
     cpu_register_physical_memory(base + MMIO_BASE_OFFSET + SM501_DC,
-                                0x400, sm501_disp_ctrl_index);
-
-    sm501_palette_index = cpu_register_io_memory(0, sm501_palette_readfn,
-                                                  sm501_palette_writefn, s);
-    cpu_register_physical_memory(base + MMIO_BASE_OFFSET
-                                + SM501_DC + SM501_DC_PANEL_PALETTE,
-                                0x400 * 3, sm501_palette_index);
+                                0x1000, sm501_disp_ctrl_index);
 
     /* bridge to serial emulation module */
     if (chr)
-- 
1.5.6.3