[PATCH 0/3] memtest: small cleanup

[PATCH 0/3] memtest: small cleanup

[Alexander Aring]

Hi,

this are three patches to cleanup the memtest command. It removes the the first
and last if condition in the sdram regions loop. It was stupid to handle it in
that way but since then my skills are improved and it was long time on my list.

I would be happy if somebody can test it with two banks, again...

- Alex

(People that I know they have more than one banks boards):
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Teresa Gámez <t.gamez@phytec.de>
Cc: Christian Hemp <c.hemp@phytec.de>
Cc: Jan Weitzel <j.weitzel@phytec.de>

Alexander Aring (3):
  memtest: cleanup error handling
  memtest: cleanup requests of regions
  memtest: copyright to UPPER case and fix typo

 commands/memtest.c | 74 +++++++++++++++++++++++-------------------------------
 common/memtest.c   |  4 +--
 2 files changed, 34 insertions(+), 44 deletions(-)

-- 
1.9.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] memtest: cleanup error handling

[Alexander Aring]

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 commands/memtest.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/commands/memtest.c b/commands/memtest.c
index a71576e..dc05e9e 100644
--- a/commands/memtest.c
+++ b/commands/memtest.c
@@ -232,12 +232,12 @@ out:
 		if (ret == -EINTR)
 			printf("\n");
 
-		printf("Memtest failed.\n");
+		printf("Memtest failed. Error: %d\n", ret);
 		return 1;
-	} else {
-		printf("Memtest successful.\n");
-		return 0;
 	}
+
+	printf("Memtest successful.\n");
+	return 0;
 }
 
 static const __maybe_unused char cmd_memtest_help[] =
-- 
1.9.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] memtest: cleanup requests of regions

[Alexander Aring]

This patch removes the first and last entry check inside the loop.
There should be no functional changes there.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 commands/memtest.c | 66 +++++++++++++++++++++++-------------------------------
 1 file changed, 28 insertions(+), 38 deletions(-)

diff --git a/commands/memtest.c b/commands/memtest.c
index dc05e9e..5854375 100644
--- a/commands/memtest.c
+++ b/commands/memtest.c
@@ -74,34 +74,25 @@ static int request_memtest_regions(struct list_head *list)
 			continue;
 		}
 
+		r = list_first_entry(&bank->res->children,
+				     struct resource, sibling);
+		start = PAGE_ALIGN(bank->res->start);
+		end = PAGE_ALIGN_DOWN(r->start);
+		r_prev = r;
+		if (start != end) {
+			size = end - start;
+			ret = alloc_memtest_region(list, start, size);
+			if (ret < 0)
+				return ret;
+		}
 		/*
 		 * We assume that the regions are sorted in this list
 		 * So the first element has start boundary on bank->res->start
-		 * and the last element hast end boundary on bank->res->end
+		 * and the last element hast end boundary on bank->res->end.
+		 *
+		 * Between used regions. Start from second entry.
 		 */
-		list_for_each_entry(r, &bank->res->children, sibling) {
-			/*
-			 * Do on head element for bank boundary
-			 */
-			if (r->sibling.prev == &bank->res->children) {
-				/*
-				 * remember last used element
-				 */
-				start = PAGE_ALIGN(bank->res->start);
-				end = PAGE_ALIGN_DOWN(r->start);
-				r_prev = r;
-				if (start == end)
-					continue;
-				size = end - start;
-
-				ret = alloc_memtest_region(list, start, size);
-				if (ret < 0)
-					return ret;
-				continue;
-			}
-			/*
-			 * Between used regions
-			 */
+		list_for_each_entry_from(r, &bank->res->children, sibling) {
 			start = PAGE_ALIGN(r_prev->end);
 			end = PAGE_ALIGN_DOWN(r->start);
 			r_prev = r;
@@ -112,21 +103,20 @@ static int request_memtest_regions(struct list_head *list)
 			ret = alloc_memtest_region(list, start, size);
 			if (ret < 0)
 				return ret;
+		}
 
-			if (list_is_last(&r->sibling, &bank->res->children)) {
-				/*
-				 * Do on head element for bank boundary
-				 */
-				start = PAGE_ALIGN(r->end);
-				end = PAGE_ALIGN_DOWN(bank->res->end) - 1;
-				size = end - start + 1;
-				if (start >= end)
-					continue;
-
-				ret = alloc_memtest_region(list, start, size);
-				if (ret < 0)
-					return ret;
-			}
+		/*
+		 * Do on head element for bank boundary.
+		 */
+		r = list_last_entry(&bank->res->children,
+				     struct resource, sibling);
+		start = PAGE_ALIGN(r->end);
+		end = PAGE_ALIGN_DOWN(bank->res->end) - 1;
+		size = end - start + 1;
+		if (start < end) {
+			ret = alloc_memtest_region(list, start, size);
+			if (ret < 0)
+				return ret;
 		}
 	}
 
-- 
1.9.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] memtest: copyright to UPPER case and fix typo

[Alexander Aring]

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 common/memtest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/memtest.c b/common/memtest.c
index 5303c92..541d008 100644
--- a/common/memtest.c
+++ b/common/memtest.c
@@ -1,7 +1,7 @@
 /*
- * memory_test.c
+ * memtest.c
  *
- * Copyright (c) 2013 Alexander Aring <aar@pengutronix.de>, Pengutronix
+ * Copyright (C) 2013 Alexander Aring <aar@pengutronix.de>, Pengutronix
  *
  * (C) Copyright 2000
  * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
-- 
1.9.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/3] memtest: small cleanup

[Sascha Hauer]

On Thu, Apr 10, 2014 at 02:04:03PM +0200, Alexander Aring wrote:
> Hi,
> 
> this are three patches to cleanup the memtest command. It removes the the first
> and last if condition in the sdram regions loop. It was stupid to handle it in
> that way but since then my skills are improved and it was long time on my list.
> 
> I would be happy if somebody can test it with two banks, again...

I tested it on a Karo TX25 which seems to work fine.

Applied, thanks

Sascha

> 
> - Alex
> 
> (People that I know they have more than one banks boards):
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Teresa Gámez <t.gamez@phytec.de>
> Cc: Christian Hemp <c.hemp@phytec.de>
> Cc: Jan Weitzel <j.weitzel@phytec.de>
> 
> Alexander Aring (3):
>   memtest: cleanup error handling
>   memtest: cleanup requests of regions
>   memtest: copyright to UPPER case and fix typo
> 
>  commands/memtest.c | 74 +++++++++++++++++++++++-------------------------------
>  common/memtest.c   |  4 +--
>  2 files changed, 34 insertions(+), 44 deletions(-)
> 
> -- 
> 1.9.1
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/3] memtest: small cleanup

[Alexander Aring]

On Wed, Apr 23, 2014 at 12:25:40PM +0200, Sascha Hauer wrote:
> On Thu, Apr 10, 2014 at 02:04:03PM +0200, Alexander Aring wrote:
> > Hi,
> > 
> > this are three patches to cleanup the memtest command. It removes the the first
> > and last if condition in the sdram regions loop. It was stupid to handle it in
> > that way but since then my skills are improved and it was long time on my list.
> > 
> > I would be happy if somebody can test it with two banks, again...
> 
> I tested it on a Karo TX25 which seems to work fine.
> 
> Applied, thanks
> 

Thanks.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox