On Fri, Jun 11, 2010 at 2:18 PM, Øyvind Harboe <[email protected]> wrote: > The return value for MMU translation was a mess, either > error or value. > > Signed-off-by: Øyvind Harboe <[email protected]> > ---
Hi Øyvind, I did not tested your patch before you commit it, since I'm not using MMU yet. Now that is committed, I get a compile time warning. Attached patch 0001-... fixes the warning. The function interested by this patch computes "retval" in all branches of if-then-else, but "retval" is already tested and in case of error the function returns immediately. So, at exit of if-then-else "retval" is initialized and signals no-error. I believe that patch 0002-... increases the readability of default return value, by retuning the constant ERROR_OK. I think the cleanup on return value of MMU translation is not completed. In most cases (but not all cases), function armv4_5_mmu_translate_va() sets "type=-1" when there is an error. The value of "type" was previously used to test the error condition. Now, error condition is correctly provided by the returned value, but still we have code testing "type==-1". Moreover, the actions in case "type==-1" are fuction returns with incorrect error code. Patch 0003-... : - removes the useless/redundant code that sets "type=-1" in armv4_5_mmu_translate_va(); - removes the useless/redundant/wrong code that tests "type==-1". But, now the variable "type" is unused. Patch 0004-... removes it completely. Please pay attention that I did not tested the functionality, since not using MMU. I can only guarantee that the patches makes the code compiling without any error. Best Regards, Antonio Borneo
From a44659343bff265eb062f9a4555bbf555a0946b7 Mon Sep 17 00:00:00 2001 From: Antonio Borneo <[email protected]> Date: Sat, 12 Jun 2010 11:01:24 +0800 Subject: [PATCH] TARGET/ARM920T: fix compile warning MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Commit 0538081246fafbfb74d554bb1b758412534aa254 introduces a compile time warning: arm920t.c: In function ‘arm920t_write_memory’: arm920t.c:567: warning: ‘retval’ may be used uninitialized in this function Signed-off-by: Antonio Borneo <[email protected]> --- src/target/arm920t.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/target/arm920t.c b/src/target/arm920t.c index 3175196..03aa233 100644 --- a/src/target/arm920t.c +++ b/src/target/arm920t.c @@ -592,7 +592,7 @@ int arm920t_write_memory(struct target *target, uint32_t address, /* * We need physical address and cb */ - int retval = armv4_5_mmu_translate_va(target, &arm920t->armv4_5_mmu, + retval = armv4_5_mmu_translate_va(target, &arm920t->armv4_5_mmu, address, &type, &cb, &domain, &ap, &pa); if (retval != ERROR_OK) return retval; -- 1.5.2.2
From 7c2fc2b17f44a95cbc69cfc34b4d4694ccf215e3 Mon Sep 17 00:00:00 2001 From: Antonio Borneo <[email protected]> Date: Sat, 12 Jun 2010 11:07:47 +0800 Subject: [PATCH] TARGET/ARM920T: fix return value Function arm920t_write_memory() default return value should be ERROR_OK. All cases of local errors are handled immediately and not further propagated. Signed-off-by: Antonio Borneo <[email protected]> --- src/target/arm920t.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/target/arm920t.c b/src/target/arm920t.c index 03aa233..d709648 100644 --- a/src/target/arm920t.c +++ b/src/target/arm920t.c @@ -711,7 +711,7 @@ int arm920t_write_memory(struct target *target, uint32_t address, } } - return retval; + return ERROR_OK; } // EXPORTED to FA256 -- 1.5.2.2
From f2e892f953535579d9804ea1f9504554970e492b Mon Sep 17 00:00:00 2001 From: Antonio Borneo <[email protected]> Date: Sat, 12 Jun 2010 11:46:56 +0800 Subject: [PATCH] TARGET: fix handling return code of MMU translation Function armv4_5_mmu_translate_va() now properly signals errors in the return value. Remove former error handling by setting variable "type" to value "-1". Signed-off-by: Antonio Borneo <[email protected]> --- src/target/arm720t.c | 4 ---- src/target/arm920t.c | 6 ------ src/target/arm926ejs.c | 4 ---- src/target/armv4_5_mmu.c | 4 ---- src/target/cortex_a8.c | 4 ---- src/target/xscale.c | 4 ---- 6 files changed, 0 insertions(+), 26 deletions(-) diff --git a/src/target/arm720t.c b/src/target/arm720t.c index 867eb94..e7672b4 100644 --- a/src/target/arm720t.c +++ b/src/target/arm720t.c @@ -264,10 +264,6 @@ static int arm720_virt2phys(struct target *target, int retval = armv4_5_mmu_translate_va(target, &arm720t->armv4_5_mmu, virtual, &type, &cb, &domain, &ap, &ret); if (retval != ERROR_OK) return retval; - if (type == -1) - { - return ret; - } *physical = ret; return ERROR_OK; } diff --git a/src/target/arm920t.c b/src/target/arm920t.c index d709648..fe2ff01 100644 --- a/src/target/arm920t.c +++ b/src/target/arm920t.c @@ -519,10 +519,6 @@ static int arm920_virt2phys(struct target *target, &arm920t->armv4_5_mmu, virt, &type, &cb, &domain, &ap, &ret); if (retval != ERROR_OK) return retval; - if (type == -1) - { - return ret; - } *phys = ret; return ERROR_OK; } @@ -596,8 +592,6 @@ int arm920t_write_memory(struct target *target, uint32_t address, address, &type, &cb, &domain, &ap, &pa); if (retval != ERROR_OK) return retval; - if (type == -1) - return pa; if (arm920t->armv4_5_mmu.armv4_5_cache.d_u_cache_enabled) { diff --git a/src/target/arm926ejs.c b/src/target/arm926ejs.c index a7aac55..bfa2ab4 100644 --- a/src/target/arm926ejs.c +++ b/src/target/arm926ejs.c @@ -730,10 +730,6 @@ static int arm926ejs_virt2phys(struct target *target, uint32_t virtual, uint32_t int retval = armv4_5_mmu_translate_va(target, &arm926ejs->armv4_5_mmu, virtual, &type, &cb, &domain, &ap, &ret); if (retval != ERROR_OK) return retval; - if (type == -1) - { - return ret; - } *physical = ret; return ERROR_OK; } diff --git a/src/target/armv4_5_mmu.c b/src/target/armv4_5_mmu.c index 6990d13..52756c1 100644 --- a/src/target/armv4_5_mmu.c +++ b/src/target/armv4_5_mmu.c @@ -44,14 +44,12 @@ int armv4_5_mmu_translate_va(struct target *target, struct armv4_5_mmu_common *a if ((first_lvl_descriptor & 0x3) == 0) { - *type = -1; LOG_ERROR("Address translation failure"); return ERROR_TARGET_TRANSLATION_FAULT; } if (!armv4_5_mmu->has_tiny_pages && ((first_lvl_descriptor & 0x3) == 3)) { - *type = -1; LOG_ERROR("Address translation failure"); return ERROR_TARGET_TRANSLATION_FAULT; } @@ -94,7 +92,6 @@ int armv4_5_mmu_translate_va(struct target *target, struct armv4_5_mmu_common *a if ((second_lvl_descriptor & 0x3) == 0) { - *type = -1; LOG_ERROR("Address translation failure"); return ERROR_TARGET_TRANSLATION_FAULT; } @@ -130,7 +127,6 @@ int armv4_5_mmu_translate_va(struct target *target, struct armv4_5_mmu_common *a } /* should not happen */ - *type = -1; LOG_ERROR("Address translation failure"); return ERROR_TARGET_TRANSLATION_FAULT; } diff --git a/src/target/cortex_a8.c b/src/target/cortex_a8.c index 2edb9e3..f154179 100644 --- a/src/target/cortex_a8.c +++ b/src/target/cortex_a8.c @@ -1825,10 +1825,6 @@ static int cortex_a8_virt2phys(struct target *target, /* Reset the flag. We don't want someone else to use it by error */ cortex_a8->current_address_mode = ARM_MODE_ANY; - if (type == -1) - { - return ret; - } *phys = ret; return ERROR_OK; } diff --git a/src/target/xscale.c b/src/target/xscale.c index ab7eee3..d16f8ec 100644 --- a/src/target/xscale.c +++ b/src/target/xscale.c @@ -3230,10 +3230,6 @@ static int xscale_virt2phys(struct target *target, int retval = armv4_5_mmu_translate_va(target, &xscale->armv4_5_mmu, virtual, &type, &cb, &domain, &ap, &ret); if (retval != ERROR_OK) return retval; - if (type == -1) - { - return ret; - } *physical = ret; return ERROR_OK; } -- 1.5.2.2
From c2834d4fc7e2488ae2e24eac4a6c817d0ecfe5cd Mon Sep 17 00:00:00 2001 From: Antonio Borneo <[email protected]> Date: Sat, 12 Jun 2010 11:58:50 +0800 Subject: [PATCH] TARGET: removed unsed parameter Parameter "type" of function armv4_5_mmu_translate_va() is now not used. Remove the parameter and the "enum" listing its values. Signed-off-by: Antonio Borneo <[email protected]> --- src/target/arm720t.c | 4 ++-- src/target/arm920t.c | 6 ++---- src/target/arm926ejs.c | 4 ++-- src/target/armv4_5_mmu.c | 6 +----- src/target/armv4_5_mmu.h | 9 +-------- src/target/cortex_a8.c | 3 +-- src/target/xscale.c | 4 ++-- 7 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/target/arm720t.c b/src/target/arm720t.c index e7672b4..d450224 100644 --- a/src/target/arm720t.c +++ b/src/target/arm720t.c @@ -254,14 +254,14 @@ static int arm720_mmu(struct target *target, int *enabled) static int arm720_virt2phys(struct target *target, uint32_t virtual, uint32_t *physical) { - int type; uint32_t cb; int domain; uint32_t ap; struct arm720t_common *arm720t = target_to_arm720(target); uint32_t ret; - int retval = armv4_5_mmu_translate_va(target, &arm720t->armv4_5_mmu, virtual, &type, &cb, &domain, &ap, &ret); + int retval = armv4_5_mmu_translate_va(target, + &arm720t->armv4_5_mmu, virtual, &cb, &domain, &ap, &ret); if (retval != ERROR_OK) return retval; *physical = ret; diff --git a/src/target/arm920t.c b/src/target/arm920t.c index fe2ff01..b8ff819 100644 --- a/src/target/arm920t.c +++ b/src/target/arm920t.c @@ -508,7 +508,6 @@ static int arm920_mmu(struct target *target, int *enabled) static int arm920_virt2phys(struct target *target, uint32_t virt, uint32_t *phys) { - int type; uint32_t cb; int domain; uint32_t ap; @@ -516,7 +515,7 @@ static int arm920_virt2phys(struct target *target, uint32_t ret; int retval = armv4_5_mmu_translate_va(target, - &arm920t->armv4_5_mmu, virt, &type, &cb, &domain, &ap, &ret); + &arm920t->armv4_5_mmu, virt, &cb, &domain, &ap, &ret); if (retval != ERROR_OK) return retval; *phys = ret; @@ -579,7 +578,6 @@ int arm920t_write_memory(struct target *target, uint32_t address, * in memory marked read only * by MMU */ - int type; uint32_t cb; int domain; uint32_t ap; @@ -589,7 +587,7 @@ int arm920t_write_memory(struct target *target, uint32_t address, * We need physical address and cb */ retval = armv4_5_mmu_translate_va(target, &arm920t->armv4_5_mmu, - address, &type, &cb, &domain, &ap, &pa); + address, &cb, &domain, &ap, &pa); if (retval != ERROR_OK) return retval; diff --git a/src/target/arm926ejs.c b/src/target/arm926ejs.c index bfa2ab4..dd1d365 100644 --- a/src/target/arm926ejs.c +++ b/src/target/arm926ejs.c @@ -720,14 +720,14 @@ COMMAND_HANDLER(arm926ejs_handle_cache_info_command) static int arm926ejs_virt2phys(struct target *target, uint32_t virtual, uint32_t *physical) { - int type; uint32_t cb; int domain; uint32_t ap; struct arm926ejs_common *arm926ejs = target_to_arm926(target); uint32_t ret; - int retval = armv4_5_mmu_translate_va(target, &arm926ejs->armv4_5_mmu, virtual, &type, &cb, &domain, &ap, &ret); + int retval = armv4_5_mmu_translate_va(target, &arm926ejs->armv4_5_mmu, + virtual, &cb, &domain, &ap, &ret); if (retval != ERROR_OK) return retval; *physical = ret; diff --git a/src/target/armv4_5_mmu.c b/src/target/armv4_5_mmu.c index 52756c1..78163f1 100644 --- a/src/target/armv4_5_mmu.c +++ b/src/target/armv4_5_mmu.c @@ -26,7 +26,7 @@ #include "armv4_5_mmu.h" -int armv4_5_mmu_translate_va(struct target *target, struct armv4_5_mmu_common *armv4_5_mmu, uint32_t va, int *type, uint32_t *cb, int *domain, uint32_t *ap, uint32_t *val) +int armv4_5_mmu_translate_va(struct target *target, struct armv4_5_mmu_common *armv4_5_mmu, uint32_t va, uint32_t *cb, int *domain, uint32_t *ap, uint32_t *val) { uint32_t first_lvl_descriptor = 0x0; uint32_t second_lvl_descriptor = 0x0; @@ -60,7 +60,6 @@ int armv4_5_mmu_translate_va(struct target *target, struct armv4_5_mmu_common *a if ((first_lvl_descriptor & 0x3) == 2) { /* section descriptor */ - *type = ARMV4_5_SECTION; *cb = (first_lvl_descriptor & 0xc) >> 2; *ap = (first_lvl_descriptor & 0xc00) >> 10; *val = (first_lvl_descriptor & 0xfff00000) | (va & 0x000fffff); @@ -102,7 +101,6 @@ int armv4_5_mmu_translate_va(struct target *target, struct armv4_5_mmu_common *a if ((second_lvl_descriptor & 0x3) == 1) { /* large page descriptor */ - *type = ARMV4_5_LARGE_PAGE; *ap = (second_lvl_descriptor & 0xff0) >> 4; *val = (second_lvl_descriptor & 0xffff0000) | (va & 0x0000ffff); return ERROR_OK; @@ -111,7 +109,6 @@ int armv4_5_mmu_translate_va(struct target *target, struct armv4_5_mmu_common *a if ((second_lvl_descriptor & 0x3) == 2) { /* small page descriptor */ - *type = ARMV4_5_SMALL_PAGE; *ap = (second_lvl_descriptor & 0xff0) >> 4; *val = (second_lvl_descriptor & 0xfffff000) | (va & 0x00000fff); return ERROR_OK; @@ -120,7 +117,6 @@ int armv4_5_mmu_translate_va(struct target *target, struct armv4_5_mmu_common *a if ((second_lvl_descriptor & 0x3) == 3) { /* tiny page descriptor */ - *type = ARMV4_5_TINY_PAGE; *ap = (second_lvl_descriptor & 0x30) >> 4; *val = (second_lvl_descriptor & 0xfffffc00) | (va & 0x000003ff); return ERROR_OK; diff --git a/src/target/armv4_5_mmu.h b/src/target/armv4_5_mmu.h index 3a6851f..8f540a6 100644 --- a/src/target/armv4_5_mmu.h +++ b/src/target/armv4_5_mmu.h @@ -36,15 +36,8 @@ struct armv4_5_mmu_common int mmu_enabled; }; -enum -{ - ARMV4_5_SECTION, ARMV4_5_LARGE_PAGE, ARMV4_5_SMALL_PAGE, ARMV4_5_TINY_PAGE -}; - -extern char* armv4_5_page_type_names[]; - int armv4_5_mmu_translate_va(struct target *target, - struct armv4_5_mmu_common *armv4_5_mmu, uint32_t va, int *type, + struct armv4_5_mmu_common *armv4_5_mmu, uint32_t va, uint32_t *cb, int *domain, uint32_t *ap, uint32_t *val); int armv4_5_mmu_read_physical(struct target *target, diff --git a/src/target/cortex_a8.c b/src/target/cortex_a8.c index f154179..e26bb3d 100644 --- a/src/target/cortex_a8.c +++ b/src/target/cortex_a8.c @@ -1801,7 +1801,6 @@ static int cortex_a8_mmu(struct target *target, int *enabled) static int cortex_a8_virt2phys(struct target *target, uint32_t virt, uint32_t *phys) { - int type; uint32_t cb; int domain; uint32_t ap; @@ -1819,7 +1818,7 @@ static int cortex_a8_virt2phys(struct target *target, cortex_a8->current_address_mode = ARM_MODE_SVC; uint32_t ret; int retval = armv4_5_mmu_translate_va(target, - &armv7a->armv4_5_mmu, virt, &type, &cb, &domain, &ap, &ret); + &armv7a->armv4_5_mmu, virt, &cb, &domain, &ap, &ret); if (retval != ERROR_OK) return retval; /* Reset the flag. We don't want someone else to use it by error */ diff --git a/src/target/xscale.c b/src/target/xscale.c index d16f8ec..e2b4b6d 100644 --- a/src/target/xscale.c +++ b/src/target/xscale.c @@ -3216,7 +3216,6 @@ static int xscale_virt2phys(struct target *target, uint32_t virtual, uint32_t *physical) { struct xscale_common *xscale = target_to_xscale(target); - int type; uint32_t cb; int domain; uint32_t ap; @@ -3227,7 +3226,8 @@ static int xscale_virt2phys(struct target *target, } uint32_t ret; - int retval = armv4_5_mmu_translate_va(target, &xscale->armv4_5_mmu, virtual, &type, &cb, &domain, &ap, &ret); + int retval = armv4_5_mmu_translate_va(target, &xscale->armv4_5_mmu, + virtual, &cb, &domain, &ap, &ret); if (retval != ERROR_OK) return retval; *physical = ret; -- 1.5.2.2
_______________________________________________ Openocd-development mailing list [email protected] https://lists.berlios.de/mailman/listinfo/openocd-development
