On Wed, Jan 20, 2010 at 11:58 AM, David Brownell <[email protected]> wrote:
> On Wednesday 20 January 2010, Řyvind Harboe wrote:
>> Attached patch flashes str710 correctly. This is a fixed version of
>> David's patch.
>
> With various bits of debugging stuff I removed from the version I posted,
> and without some comment updates...

Uh? "all" I did was to rewrite that loop :-) I didn't remove any
comments or debug stuff.

> Could you resend as a delta patch against what I sent to the list, so I
> can see what you chnaged a bit more readily?  I see you changed how that
> loop termination was done (add "done_block" etc).

Attached is your patch + my modifications on top of that.

-- 
Øyvind Harboe
US toll free 1-866-980-3434 / International +47 51 63 25 00
http://www.zylin.com/zy1000.html
ARM7 ARM9 ARM11 XScale Cortex
JTAG debugger and flash programmer
From 801bfcf86caa5690c6da9400af32d310570ef7d7 Mon Sep 17 00:00:00 2001
From: David Brownell <[email protected]>
Date: Tue, 19 Jan 2010 22:48:11 -0800
Subject: [PATCH 1/2] gdb_server: correctly report flash sector sizes

Report each region of same-size sectors separately, instead
of incorrectly reporting that every sector has the same size.

This fixes a longstanding bug, exposed by other recent bugfixes
in flash handling.
---
 src/server/gdb_server.c |   81 +++++++++++++++++++++++++++++------------------
 1 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index 6ed7243..b745732 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -1613,22 +1613,6 @@ static int decode_xfer_read(char *buf, char **annex, int *ofs, unsigned int *len
 	return 0;
 }
 
-static int gdb_calc_blocksize(struct flash_bank *bank)
-{
-	uint32_t i;
-	uint32_t block_size = 0xffffffff;
-
-	/* loop through all sectors and return smallest sector size */
-
-	for (i = 0; i < (uint32_t)bank->num_sectors; i++)
-	{
-		if (bank->sectors[i].size < block_size)
-			block_size = bank->sectors[i].size;
-	}
-
-	return block_size;
-}
-
 static int compare_bank (const void * a, const void * b)
 {
 	struct flash_bank *b1, *b2;
@@ -1666,7 +1650,6 @@ static int gdb_memory_map(struct connection *connection,
 	int offset;
 	int length;
 	char *separator;
-	int blocksize;
 	uint32_t ram_start = 0;
 	int i;
 
@@ -1701,29 +1684,65 @@ static int gdb_memory_map(struct connection *connection,
 			compare_bank);
 
 	for (i = 0; i < flash_get_bank_count(); i++) {
+		int j;
+		unsigned sector_size = 0;
+		uint32_t start, end;
+
 		p = banks[i];
+		start = p->base;
+		end = p->base + p->size;
 
 		if (ram_start < p->base)
 			xml_printf(&retval, &xml, &pos, &size,
 				"<memory type=\"ram\" start=\"0x%x\" "
 					"length=\"0x%x\"/>\n",
-				ram_start, p->base-ram_start);
+				ram_start, p->base - ram_start);
 
-		/* If device has uneven sector sizes, eg. str7, lpc
-		 * we pass the smallest sector size to gdb memory map
-		 *
-		 * FIXME Don't lie about flash regions with different
-		 * sector sizes; just tell GDB about each region as
-		 * if it were a separate flash device.
+		/* Report adjacent groups of same-size sectors.  So for
+		 * example top boot CFI flash will list an initial region
+		 * with several large sectors (maybe 128KB) and several
+		 * smaller ones at the end (maybe 32KB).  STR7 will have
+		 * regions with 8KB, 32KB, and 64KB sectors; etc.
 		 */
-		blocksize = gdb_calc_blocksize(p);
+		for (j = 0; j < p->num_sectors; j++) {
+
+			/* start a new group of sectors */
+			if (sector_size == 0) {
+				start = p->base + p->sectors[j].offset;
+				xml_printf(&retval, &xml, &pos, &size,
+					"<memory type=\"flash\" "
+						"start=\"0x%x\" ",
+					start);
+				sector_size = p->sectors[j].size;
+				continue;
+			}
+
+			/* finish a group of sectors */
+			if (p->sectors[j].size != sector_size) {
+				xml_printf(&retval, &xml, &pos, &size,
+					"length=\"0x%x\">\n"
+					"<property name=\"blocksize\">"
+						"0x%x</property>\n"
+					"</memory>\n",
+					p->sectors[j].offset - start,
+					sector_size);
+				sector_size = 0;
+			}
+
+			/* else we continue a group we started */
+		}
+
+		/* terminate the last group of sectors */
+		if (sector_size) {
+			xml_printf(&retval, &xml, &pos, &size,
+				"length=\"0x%x\">\n"
+				"<property name=\"blocksize\">"
+					"0x%x</property>\n"
+				"</memory>\n",
+				p->size - start,
+				sector_size);
+		}
 
-		xml_printf(&retval, &xml, &pos, &size,
-			"<memory type=\"flash\" start=\"0x%x\" "
-				"length=\"0x%x\">\n" \
-			"<property name=\"blocksize\">0x%x</property>\n" \
-			"</memory>\n", \
-			p->base, p->size, blocksize);
 		ram_start = p->base + p->size;
 	}
 
-- 
1.6.3.3

From ffb4fa27a1ff97b6ab36c317d7121e38842d44f3 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?=C3=98yvind=20Harboe?= <[email protected]>
Date: Wed, 20 Jan 2010 11:07:11 +0100
Subject: [PATCH 2/2] gdb_server: fix gdb load write to flash
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

The memory map packet is now reported correctly.

This covers str710 that has varying sector sizes.

Signed-off-by: Øyvind Harboe <[email protected]>
---
 src/server/gdb_server.c |   40 ++++++++++++++++++++--------------------
 1 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index b745732..3d28b76 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -1714,35 +1714,35 @@ static int gdb_memory_map(struct connection *connection,
 						"start=\"0x%x\" ",
 					start);
 				sector_size = p->sectors[j].size;
-				continue;
 			}
 
-			/* finish a group of sectors */
-			if (p->sectors[j].size != sector_size) {
+			unsigned block_size;
+			bool done_block = false;
+
+			if (j == p->num_sectors -1) {
+				/* terminate the last group of sectors */
+				block_size = (p->base + p->size) - start;
+				done_block = true;
+			} else if (p->sectors[j + 1].size != sector_size) {
+				/* finish a group of sectors */
+				block_size = (p->base + p->sectors[j + 1].offset) - start,
+				done_block = true;
+			}
+
+			if (done_block) {
 				xml_printf(&retval, &xml, &pos, &size,
-					"length=\"0x%x\">\n"
-					"<property name=\"blocksize\">"
-						"0x%x</property>\n"
-					"</memory>\n",
-					p->sectors[j].offset - start,
-					sector_size);
+						"length=\"0x%x\">\n"
+						"<property name=\"blocksize\">"
+							"0x%x</property>\n"
+						"</memory>\n",
+						block_size,
+						sector_size);
 				sector_size = 0;
 			}
 
 			/* else we continue a group we started */
 		}
 
-		/* terminate the last group of sectors */
-		if (sector_size) {
-			xml_printf(&retval, &xml, &pos, &size,
-				"length=\"0x%x\">\n"
-				"<property name=\"blocksize\">"
-					"0x%x</property>\n"
-				"</memory>\n",
-				p->size - start,
-				sector_size);
-		}
-
 		ram_start = p->base + p->size;
 	}
 
-- 
1.6.3.3

_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to