[Qemu-devel] [PATCH v2 0/2] integratorcp: fix and simplify flash remap code

[Qemu-devel] [PATCH v2 0/2] integratorcp: fix and simplify flash remap code

[Peter Maydell]

Same final result as patch v1, but I've split out fixing the sense
of the remap bit into its own patch; Avi rightly pointed out that
I was being lazy there.

Peter Maydell (2):
  hw/integratorcp: Fix sense of REMAP bit
  hw/integratorcp: Simplify flash remap code

 hw/integratorcp.c |   26 +++++++++-----------------
 1 files changed, 9 insertions(+), 17 deletions(-)

-- 
1.7.5.4

[Qemu-devel] [PATCH v2 1/2] hw/integratorcp: Fix sense of REMAP bit

[Peter Maydell]

Fix the sense of the REMAP bit: 0 should mean "map flash",
1 should mean "map RAM".

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/integratorcp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/integratorcp.c b/hw/integratorcp.c
index 2551236..8e4c154 100644
--- a/hw/integratorcp.c
+++ b/hw/integratorcp.c
@@ -112,7 +112,7 @@ static uint64_t integratorcm_read(void *opaque, target_phys_addr_t offset,
 
 static void integratorcm_do_remap(integratorcm_state *s, int flash)
 {
-    if (flash) {
+    if (!flash) {
         if (s->flash_mapped) {
             sysbus_del_memory(&s->busdev, &s->flash);
             s->flash_mapped = false;
-- 
1.7.5.4

[Qemu-devel] [PATCH v2 2/2] hw/integratorcp: Simplify flash remap code

[Peter Maydell]

Use the new memory mutator API to simplify the flash remap code;
this allows us to drop the flash_mapped flag.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/integratorcp.c |   26 +++++++++-----------------
 1 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/hw/integratorcp.c b/hw/integratorcp.c
index 8e4c154..fb243e1 100644
--- a/hw/integratorcp.c
+++ b/hw/integratorcp.c
@@ -21,7 +21,6 @@ typedef struct {
     MemoryRegion iomem;
     uint32_t memsz;
     MemoryRegion flash;
-    bool flash_mapped;
     uint32_t cm_osc;
     uint32_t cm_ctrl;
     uint32_t cm_lock;
@@ -110,20 +109,16 @@ static uint64_t integratorcm_read(void *opaque, target_phys_addr_t offset,
     }
 }
 
-static void integratorcm_do_remap(integratorcm_state *s, int flash)
+static void integratorcm_do_remap(integratorcm_state *s)
 {
-    if (!flash) {
-        if (s->flash_mapped) {
-            sysbus_del_memory(&s->busdev, &s->flash);
-            s->flash_mapped = false;
-        }
+    /* Sync memory region state with CM_CTRL REMAP bit:
+     * bit 0 => flash at address 0; bit 1 => RAM
+     */
+    if (s->cm_ctrl & 4) {
+        memory_region_set_enabled(&s->flash, 0);
     } else {
-        if (!s->flash_mapped) {
-            sysbus_add_memory_overlap(&s->busdev, 0, &s->flash, 1);
-            s->flash_mapped = true;
-        }
+        memory_region_set_enabled(&s->flash, 1);
     }
-    //??? tlb_flush (cpu_single_env, 1);
 }
 
 static void integratorcm_set_ctrl(integratorcm_state *s, uint32_t value)
@@ -131,9 +126,6 @@ static void integratorcm_set_ctrl(integratorcm_state *s, uint32_t value)
     if (value & 8) {
         qemu_system_reset_request();
     }
-    if ((s->cm_ctrl ^ value) & 4) {
-        integratorcm_do_remap(s, (value & 4) == 0);
-    }
     if ((s->cm_ctrl ^ value) & 1) {
         /* (value & 1) != 0 means the green "MISC LED" is lit.
          * We don't have any nice place to display LEDs. printf is a bad
@@ -143,6 +135,7 @@ static void integratorcm_set_ctrl(integratorcm_state *s, uint32_t value)
     }
     /* Note that the RESET bit [3] always reads as zero */
     s->cm_ctrl = (s->cm_ctrl & ~5) | (value & 5);
+    integratorcm_do_remap(s);
 }
 
 static void integratorcm_update(integratorcm_state *s)
@@ -262,13 +255,12 @@ static int integratorcm_init(SysBusDevice *dev)
     memcpy(integrator_spd + 73, "QEMU-MEMORY", 11);
     s->cm_init = 0x00000112;
     memory_region_init_ram(&s->flash, NULL, "integrator.flash", 0x100000);
-    s->flash_mapped = false;
 
     memory_region_init_io(&s->iomem, &integratorcm_ops, s,
                           "integratorcm", 0x00800000);
     sysbus_init_mmio(dev, &s->iomem);
 
-    integratorcm_do_remap(s, 1);
+    integratorcm_do_remap(s);
     /* ??? Save/restore.  */
     return 0;
 }
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH v2 2/2] hw/integratorcp: Simplify flash remap code

[Avi Kivity]

On 12/20/2011 06:08 PM, Peter Maydell wrote:
> Use the new memory mutator API to simplify the flash remap code;
> this allows us to drop the flash_mapped flag.
>
>  
> -static void integratorcm_do_remap(integratorcm_state *s, int flash)
> +static void integratorcm_do_remap(integratorcm_state *s)
>  {
> -    if (!flash) {
> -        if (s->flash_mapped) {
> -            sysbus_del_memory(&s->busdev, &s->flash);
> -            s->flash_mapped = false;
> -        }
> +    /* Sync memory region state with CM_CTRL REMAP bit:
> +     * bit 0 => flash at address 0; bit 1 => RAM
> +     */
> +    if (s->cm_ctrl & 4) {
> +        memory_region_set_enabled(&s->flash, 0);
>      } else {
> -        if (!s->flash_mapped) {
> -            sysbus_add_memory_overlap(&s->busdev, 0, &s->flash, 1);
> -            s->flash_mapped = true;
> -        }
> +        memory_region_set_enabled(&s->flash, 1);
>      }

  memory_region_set_enabled(&s->flash, !(s->cm_ctrl & 4));

shows that the bit is inverted and passed directly to flash.enabled.

-- 
error compiling committee.c: too many arguments to function