mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/8] memtest.c: Print iterations count if -i is 0
@ 2015-10-11 18:43 Andrey Smirnov
  2015-10-11 18:43 ` [PATCH 2/8] arm: Disable unaligned accesses Andrey Smirnov
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Andrey Smirnov @ 2015-10-11 18:43 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Even if memtest is started in endless mode it is still usefult to see
current iteration count. Add the code to print that.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 commands/memtest.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/commands/memtest.c b/commands/memtest.c
index 7561230..b88290c 100644
--- a/commands/memtest.c
+++ b/commands/memtest.c
@@ -180,8 +180,12 @@ static int do_memtest(int argc, char *argv[])
 		goto out;

 	for (i = 1; (i <= max_i) || !max_i; i++) {
+		printf("Start iteration %u", i);
 		if (max_i)
-			printf("Start iteration %u of %u.\n", i, max_i);
+			printf(" of %u.\n", max_i);
+		else
+			putchar('\n');
+
 		/*
 		 * First try a memtest with caching enabled.
 		 */
--
2.1.4

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/8] arm: Disable unaligned accesses
  2015-10-11 18:43 [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Andrey Smirnov
@ 2015-10-11 18:43 ` Andrey Smirnov
  2015-10-11 18:43 ` [PATCH 3/8] arm/cpu/start.c: Distil some common code in __start() Andrey Smirnov
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Andrey Smirnov @ 2015-10-11 18:43 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Form reading ARM architecture related documentation if appears that
while unaligned memory access is supported by the processor in general
it is not supported if MMU is disabled.

The problem in question can be easilty reproduced by building the code
without this patch, MMU disabled, and trying to run 'memtest'
command. Which would in turn call mem_test() which would eventually
call show_progress(). That last function, if build without
-mno-unaligned-access would result in unaligned memory access which
would result in Barebox hanging.

This patch instructs the compiler to not generate any unaligned
accesses to memory thus avoiding the problem.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/Makefile | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 721aa9b..cae05ff 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -13,6 +13,13 @@ AS		+= -EL
 LD		+= -EL
 endif

+# Unaligned access is not supported when MMU is disabled, so given how
+# at least some of the code would be executed with MMU off, lets be
+# conservative and instruct the compiler not to generate any unaligned
+# accesses
+CFLAGS += -mno-unaligned-access
+
+
 # This selects which instruction set is used.
 # Note that GCC does not numerically define an architecture version
 # macro, but instead defines a whole series of macros which makes
--
2.1.4

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/8] arm/cpu/start.c: Distil some common code in __start().
  2015-10-11 18:43 [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Andrey Smirnov
  2015-10-11 18:43 ` [PATCH 2/8] arm: Disable unaligned accesses Andrey Smirnov
@ 2015-10-11 18:43 ` Andrey Smirnov
  2015-10-12  9:44   ` Sascha Hauer
  2015-10-11 18:43 ` [PATCH 4/8] arm/cpu: Avoid multiple definitions of barebox_arm_entry Andrey Smirnov
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Andrey Smirnov @ 2015-10-11 18:43 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/start.c | 49 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
index 8e5097b..7ffde7c 100644
--- a/arch/arm/cpu/start.c
+++ b/arch/arm/cpu/start.c
@@ -53,6 +53,18 @@ void *barebox_arm_boot_dtb(void)
 	return barebox_boot_dtb;
 }
 
+static uint32_t get_any_boarddata_magic(const void *boarddata)
+{
+	if (get_unaligned_be32(boarddata) == FDT_MAGIC)
+		return FDT_MAGIC;
+
+	if (((struct barebox_arm_boarddata *)boarddata)->magic ==
+	    BAREBOX_ARM_BOARDDATA_MAGIC)
+		return BAREBOX_ARM_BOARDDATA_MAGIC;
+
+	return 0;
+}
+
 static noinline __noreturn void __start(unsigned long membase,
 		unsigned long memsize, void *boarddata)
 {
@@ -89,21 +101,30 @@ static noinline __noreturn void __start(unsigned long membase,
 	}
 
 	if (boarddata) {
-		if (get_unaligned_be32(boarddata) == FDT_MAGIC) {
-			uint32_t totalsize = get_unaligned_be32(boarddata + 4);
+		uint32_t totalsize;
+		void **var = NULL;
+		const char *name;
+
+		switch (get_any_boarddata_magic(boarddata)) {
+		case FDT_MAGIC:
+			totalsize = get_unaligned_be32(boarddata + 4);
+			var = &barebox_boot_dtb;
+			name = "DTB";
+			break;
+		case BAREBOX_ARM_BOARDDATA_MAGIC:
+			totalsize = sizeof(struct barebox_arm_boarddata);
+			var = &barebox_boarddata;
+			name = "machine type";
+			break;
+		default:
+			break;
+		}
+
+		if (var) {
 			endmem -= ALIGN(totalsize, 64);
-			barebox_boot_dtb = (void *)endmem;
-			pr_debug("found DTB in boarddata, copying to 0x%p\n",
-					barebox_boot_dtb);
-			memcpy(barebox_boot_dtb, boarddata, totalsize);
-		} else if (((struct barebox_arm_boarddata *)boarddata)->magic ==
-				BAREBOX_ARM_BOARDDATA_MAGIC) {
-			endmem -= ALIGN(sizeof(struct barebox_arm_boarddata), 64);
-			barebox_boarddata = (void *)endmem;
-			pr_debug("found machine type in boarddata, copying to 0x%p\n",
-					barebox_boarddata);
-			memcpy(barebox_boarddata, boarddata,
-					sizeof(struct barebox_arm_boarddata));
+			pr_debug("found %s in boarddata, copying to 0x%lu\n",
+				 name, endmem);
+			*var = memcpy((void *)endmem, boarddata, totalsize);
 		}
 	}
 
-- 
2.1.4


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/8] arm/cpu: Avoid multiple definitions of barebox_arm_entry
  2015-10-11 18:43 [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Andrey Smirnov
  2015-10-11 18:43 ` [PATCH 2/8] arm: Disable unaligned accesses Andrey Smirnov
  2015-10-11 18:43 ` [PATCH 3/8] arm/cpu/start.c: Distil some common code in __start() Andrey Smirnov
@ 2015-10-11 18:43 ` Andrey Smirnov
  2015-10-12  9:37   ` Sascha Hauer
  2015-10-11 18:43 ` [PATCH 5/8] freescale-mx6-sabresd: Add CONFIG_DEBUG_LL support Andrey Smirnov
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Andrey Smirnov @ 2015-10-11 18:43 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

All versions of barebox_arm_entry (in uncompress.c, start.c and
start-pbl.c) appera to be doing exacty the same thing. So move the
definition into a separate file and use IS_ENABLED macro to avoid
re-definition.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/Makefile     |  4 ++--
 arch/arm/cpu/entry.c      | 37 ++++++++++++++++++++++++++++++++++++
 arch/arm/cpu/entry.h      | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/cpu/start-pbl.c  | 29 +---------------------------
 arch/arm/cpu/start.c      | 27 +-------------------------
 arch/arm/cpu/uncompress.c | 16 +---------------
 6 files changed, 90 insertions(+), 71 deletions(-)
 create mode 100644 arch/arm/cpu/entry.c
 create mode 100644 arch/arm/cpu/entry.h

diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
index fb3929c..418bcab 100644
--- a/arch/arm/cpu/Makefile
+++ b/arch/arm/cpu/Makefile
@@ -1,7 +1,7 @@
 obj-y += cpu.o
 obj-$(CONFIG_ARM_EXCEPTIONS) += exceptions.o
 obj-$(CONFIG_ARM_EXCEPTIONS) += interrupts.o
-obj-y += start.o setupc.o
+obj-y += start.o setupc.o entry.o
 
 #
 # Any variants can be called as start-armxyz.S
@@ -23,7 +23,7 @@ AFLAGS_pbl-cache-armv7.o       :=-Wa,-march=armv7-a
 pbl-$(CONFIG_CPU_32v7) += cache-armv7.o
 obj-$(CONFIG_CACHE_L2X0) += cache-l2x0.o
 
-pbl-y += setupc.o
+pbl-y += setupc.o entry.o
 pbl-$(CONFIG_PBL_SINGLE_IMAGE) += start-pbl.o
 pbl-$(CONFIG_PBL_MULTI_IMAGES) += uncompress.o
 
diff --git a/arch/arm/cpu/entry.c b/arch/arm/cpu/entry.c
new file mode 100644
index 0000000..adc4aec
--- /dev/null
+++ b/arch/arm/cpu/entry.c
@@ -0,0 +1,37 @@
+#include <types.h>
+#include <asm/cache.h>
+
+#include "entry.h"
+
+/*
+ * Main ARM entry point. Call this with the memory region you can
+ * spare for barebox. This doesn't necessarily have to be the full
+ * SDRAM. The currently running binary can be inside or outside of
+ * this region. TEXT_BASE can be inside or outside of this
+ * region. boarddata will be preserved and can be accessed later with
+ * barebox_arm_boarddata().
+ *
+ * -> membase + memsize
+ *   STACK_SIZE              - stack
+ *   16KiB, aligned to 16KiB - First level page table if early MMU support
+ *                             is enabled
+ *   128KiB                  - early memory space
+ * -> maximum end of barebox binary
+ *
+ * Usually a TEXT_BASE of 1MiB below your lowest possible end of memory should
+ * be fine.
+ */
+
+void __naked __noreturn barebox_arm_entry(unsigned long membase,
+					  unsigned long memsize, void *boarddata)
+{
+	arm_setup_stack(membase + memsize - 16);
+	arm_early_mmu_cache_invalidate();
+
+	if (IS_ENABLED(CONFIG_PBL_MULTI_IMAGES))
+		uncompress_start_payload(membase, memsize, boarddata);
+	else if (IS_ENABLED(CONFIG_PBL_SINGLE_IMAGE))
+		__barebox_arm_entry(membase, memsize, boarddata);
+	else
+		__start(membase, memsize, boarddata);
+}
diff --git a/arch/arm/cpu/entry.h b/arch/arm/cpu/entry.h
new file mode 100644
index 0000000..a4b5b60
--- /dev/null
+++ b/arch/arm/cpu/entry.h
@@ -0,0 +1,48 @@
+#ifndef __ENTRY_H__
+#define __ENTRY_H__
+
+#include <common.h>
+
+#if !defined (__PBL__)
+void __noreturn __start(unsigned long membase,
+			unsigned long memsize,
+			void *boarddata);
+#else
+static inline
+void __noreturn __start(unsigned long membase,
+			unsigned long memsize,
+			void *boarddata)
+{
+	hang();
+};
+#endif
+
+#if defined (__PBL__) && defined(CONFIG_PBL_MULTI_IMAGES)
+void __noreturn uncompress_start_payload(unsigned long membase,
+					 unsigned long memsize,
+					 void *boarddata);
+#else
+static inline
+void __noreturn uncompress_start_payload(unsigned long membase,
+					 unsigned long memsize,
+					 void *boarddata)
+{
+	hang();
+}
+#endif
+
+#if defined (__PBL__) && defined(CONFIG_PBL_SINGLE_IMAGE)
+void __noreturn __barebox_arm_entry(unsigned long membase,
+				    unsigned long memsize,
+				    void *boarddata);
+#else
+static inline
+void __noreturn __barebox_arm_entry(unsigned long membase,
+				    unsigned long memsize,
+				    void *boarddata)
+{
+	hang();
+}
+#endif
+
+#endif
diff --git a/arch/arm/cpu/start-pbl.c b/arch/arm/cpu/start-pbl.c
index f2490fd..36954bf 100644
--- a/arch/arm/cpu/start-pbl.c
+++ b/arch/arm/cpu/start-pbl.c
@@ -45,7 +45,7 @@ void __naked __section(.text_head_entry) pbl_start(void)
 extern void *input_data;
 extern void *input_data_end;
 
-static noinline __noreturn void __barebox_arm_entry(unsigned long membase,
+__noreturn void __barebox_arm_entry(unsigned long membase,
 		unsigned long memsize, void *boarddata)
 {
 	uint32_t offset;
@@ -56,8 +56,6 @@ static noinline __noreturn void __barebox_arm_entry(unsigned long membase,
 
 	endmem -= STACK_SIZE; /* stack */
 
-	arm_early_mmu_cache_invalidate();
-
 	if (IS_ENABLED(CONFIG_PBL_RELOCATABLE))
 		relocate_to_current_adr();
 
@@ -106,28 +104,3 @@ static noinline __noreturn void __barebox_arm_entry(unsigned long membase,
 
 	barebox(membase, memsize, boarddata);
 }
-
-/*
- * Main ARM entry point in the compressed image. Call this with the memory
- * region you can spare for barebox. This doesn't necessarily have to be the
- * full SDRAM. The currently running binary can be inside or outside of this
- * region. TEXT_BASE can be inside or outside of this region. boarddata will
- * be preserved and can be accessed later with barebox_arm_boarddata().
- *
- * -> membase + memsize
- *   STACK_SIZE              - stack
- *   16KiB, aligned to 16KiB - First level page table if early MMU support
- *                             is enabled
- *   128KiB                  - early memory space
- * -> maximum end of barebox binary
- *
- * Usually a TEXT_BASE of 1MiB below your lowest possible end of memory should
- * be fine.
- */
-void __naked __noreturn barebox_arm_entry(unsigned long membase,
-		unsigned long memsize, void *boarddata)
-{
-	arm_setup_stack(membase + memsize - 16);
-
-	__barebox_arm_entry(membase, memsize, boarddata);
-}
diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
index 7ffde7c..4de6d28 100644
--- a/arch/arm/cpu/start.c
+++ b/arch/arm/cpu/start.c
@@ -65,7 +65,7 @@ static uint32_t get_any_boarddata_magic(const void *boarddata)
 	return 0;
 }
 
-static noinline __noreturn void __start(unsigned long membase,
+__noreturn void __start(unsigned long membase,
 		unsigned long memsize, void *boarddata)
 {
 	unsigned long endmem = membase + memsize;
@@ -170,31 +170,6 @@ void __naked __section(.text_entry) start(void)
 	barebox_arm_head();
 }
 
-/*
- * Main ARM entry point in the uncompressed image. Call this with the memory
- * region you can spare for barebox. This doesn't necessarily have to be the
- * full SDRAM. The currently running binary can be inside or outside of this
- * region. TEXT_BASE can be inside or outside of this region. boarddata will
- * be preserved and can be accessed later with barebox_arm_boarddata().
- *
- * -> membase + memsize
- *   STACK_SIZE              - stack
- *   16KiB, aligned to 16KiB - First level page table if early MMU support
- *                             is enabled
- * -> maximum end of barebox binary
- *
- * Usually a TEXT_BASE of 1MiB below your lowest possible end of memory should
- * be fine.
- */
-void __naked __noreturn barebox_arm_entry(unsigned long membase,
-		unsigned long memsize, void *boarddata)
-{
-	arm_setup_stack(membase + memsize - 16);
-
-	arm_early_mmu_cache_invalidate();
-
-	__start(membase, memsize, boarddata);
-}
 #else
 /*
  * First function in the uncompressed image. We get here from
diff --git a/arch/arm/cpu/uncompress.c b/arch/arm/cpu/uncompress.c
index b0b7c6d..ca54331 100644
--- a/arch/arm/cpu/uncompress.c
+++ b/arch/arm/cpu/uncompress.c
@@ -41,7 +41,7 @@ static int __attribute__((__used__))
 	__attribute__((__section__(".image_end")))
 	__image_end_dummy = 0xdeadbeef;
 
-static void __noreturn noinline uncompress_start_payload(unsigned long membase,
+void __noreturn uncompress_start_payload(unsigned long membase,
 		unsigned long memsize, void *boarddata)
 {
 	uint32_t pg_len;
@@ -52,8 +52,6 @@ static void __noreturn noinline uncompress_start_payload(unsigned long membase,
 	void *pg_start;
 	unsigned long pc = get_pc();
 
-	arm_early_mmu_cache_invalidate();
-
 	endmem -= STACK_SIZE; /* stack */
 
 	image_end = (void *)ld_var(__image_end) - get_runtime_offset();
@@ -114,15 +112,3 @@ static void __noreturn noinline uncompress_start_payload(unsigned long membase,
 
 	barebox(membase, memsize, boarddata);
 }
-
-/*
- * For the multi images startup process board code jumps here. We will uncompress
- * the attached barebox image and start it.
- */
-void __naked __noreturn barebox_arm_entry(unsigned long membase,
-		unsigned long memsize, void *boarddata)
-{
-	arm_setup_stack(membase + memsize - 16);
-
-	uncompress_start_payload(membase, memsize, boarddata);
-}
-- 
2.1.4


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 5/8] freescale-mx6-sabresd: Add CONFIG_DEBUG_LL support
  2015-10-11 18:43 [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Andrey Smirnov
                   ` (2 preceding siblings ...)
  2015-10-11 18:43 ` [PATCH 4/8] arm/cpu: Avoid multiple definitions of barebox_arm_entry Andrey Smirnov
@ 2015-10-11 18:43 ` Andrey Smirnov
  2015-10-11 18:43 ` [PATCH 6/8] common/command.c: Replace magic number with appropriate constant Andrey Smirnov
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Andrey Smirnov @ 2015-10-11 18:43 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/boards/freescale-mx6-sabresd/lowlevel.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/boards/freescale-mx6-sabresd/lowlevel.c b/arch/arm/boards/freescale-mx6-sabresd/lowlevel.c
index 73eed1d..b329f46 100644
--- a/arch/arm/boards/freescale-mx6-sabresd/lowlevel.c
+++ b/arch/arm/boards/freescale-mx6-sabresd/lowlevel.c
@@ -1,9 +1,28 @@
+#include <debug_ll.h>
 #include <common.h>
 #include <linux/sizes.h>
 #include <mach/generic.h>
 #include <asm/barebox-arm-head.h>
 #include <asm/barebox-arm.h>
 
+static inline void setup_uart(void)
+{
+	void __iomem *iomuxbase = IOMEM(MX6_IOMUXC_BASE_ADDR);
+
+	imx6_ungate_all_peripherals();
+
+	writel(0x1b0b1, iomuxbase + 0x0650);
+	writel(3, iomuxbase + 0x0280);
+
+	writel(0x1b0b1, iomuxbase + 0x0654);
+	writel(3, iomuxbase + 0x0284);
+	writel(1, iomuxbase + 0x0920);
+
+	imx6_uart_setup_ll();
+
+	putc_ll('>');
+}
+
 extern char __dtb_imx6q_sabresd_start[];
 
 ENTRY_FUNCTION(start_imx6q_sabresd, r0, r1, r2)
@@ -12,6 +31,9 @@ ENTRY_FUNCTION(start_imx6q_sabresd, r0, r1, r2)
 
 	imx6_cpu_lowlevel_init();
 
+	if (IS_ENABLED(CONFIG_DEBUG_LL))
+		setup_uart();
+
 	fdt = __dtb_imx6q_sabresd_start - get_runtime_offset();
 
 	barebox_arm_entry(0x10000000, SZ_1G, fdt);
-- 
2.1.4


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 6/8] common/command.c: Replace magic number with appropriate constant
  2015-10-11 18:43 [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Andrey Smirnov
                   ` (3 preceding siblings ...)
  2015-10-11 18:43 ` [PATCH 5/8] freescale-mx6-sabresd: Add CONFIG_DEBUG_LL support Andrey Smirnov
@ 2015-10-11 18:43 ` Andrey Smirnov
  2015-10-11 18:43 ` [PATCH 7/8] common/parser.c: Do not conflate error reporting disciplines Andrey Smirnov
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Andrey Smirnov @ 2015-10-11 18:43 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/command.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/command.c b/common/command.c
index dc2cb88..03c7083 100644
--- a/common/command.c
+++ b/common/command.c
@@ -83,7 +83,7 @@ int execute_command(int argc, char **argv)
 #else
 		printf ("Unknown command '%s'\n", argv[0]);
 #endif
-		ret = 1;	/* give up after bad command */
+		ret = COMMAND_ERROR;	/* give up after bad command */
 	}
 
 	getopt_context_restore(&gc);
-- 
2.1.4


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 7/8] common/parser.c: Do not conflate error reporting disciplines
  2015-10-11 18:43 [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Andrey Smirnov
                   ` (4 preceding siblings ...)
  2015-10-11 18:43 ` [PATCH 6/8] common/command.c: Replace magic number with appropriate constant Andrey Smirnov
@ 2015-10-11 18:43 ` Andrey Smirnov
  2015-10-11 18:43 ` [PATCH 8/8] common/parser.c: Do not treat zero return code as error Andrey Smirnov
  2015-10-12  9:46 ` [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Sascha Hauer
  7 siblings, 0 replies; 11+ messages in thread
From: Andrey Smirnov @ 2015-10-11 18:43 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Run_command() uses 0 to indicate success and negative values to
indicate errors, whereas execute_command() uses 0 for success and
positive integers to represent error codes. Conflating the two breaks
the code that calls run_command and then checks for error code sign to
detect problems, so avoid that by doing a very simple transformation
on the result of execute_command

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/parser.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/parser.c b/common/parser.c
index ed414d0..2b95aed 100644
--- a/common/parser.c
+++ b/common/parser.c
@@ -253,7 +253,8 @@ int run_command(const char *cmd)
 			continue;
 		}
 
-		rc = execute_command(argc, argv);
+		if (execute_command(argc, argv) != COMMAND_SUCCESS)
+			rc = -1;
 	}
 
 	return rc;
-- 
2.1.4


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 8/8] common/parser.c: Do not treat zero return code as error
  2015-10-11 18:43 [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Andrey Smirnov
                   ` (5 preceding siblings ...)
  2015-10-11 18:43 ` [PATCH 7/8] common/parser.c: Do not conflate error reporting disciplines Andrey Smirnov
@ 2015-10-11 18:43 ` Andrey Smirnov
  2015-10-12  9:46 ` [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Sascha Hauer
  7 siblings, 0 replies; 11+ messages in thread
From: Andrey Smirnov @ 2015-10-11 18:43 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Run_command() would return zero to indicated success so treating it as
error case would completely break command repetition logic, so change
comparinson operator from "less or equal" to "less then"

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/parser.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/common/parser.c b/common/parser.c
index 2b95aed..6136dbf 100644
--- a/common/parser.c
+++ b/common/parser.c
@@ -266,7 +266,6 @@ int run_shell(void)
 {
 	static char lastcommand[CONFIG_CBSIZE] = { 0, };
 	int len;
-	int rc = 1;
 
 	login();
 
@@ -276,14 +275,14 @@ int run_shell(void)
 		if (len > 0)
 			strcpy (lastcommand, console_buffer);
 
-		if (len == -1)
+		if (len == -1) {
 			puts ("<INTERRUPT>\n");
-		else
-			rc = run_command(lastcommand);
-
-		if (rc <= 0) {
-			/* invalid command or not repeatable, forget it */
-			lastcommand[0] = 0;
+		} else {
+			const int rc = run_command(lastcommand);
+			if (rc < 0) {
+				/* invalid command or not repeatable, forget it */
+				lastcommand[0] = 0;
+			}
 		}
 	}
 	return 0;
-- 
2.1.4


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/8] arm/cpu: Avoid multiple definitions of barebox_arm_entry
  2015-10-11 18:43 ` [PATCH 4/8] arm/cpu: Avoid multiple definitions of barebox_arm_entry Andrey Smirnov
@ 2015-10-12  9:37   ` Sascha Hauer
  0 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2015-10-12  9:37 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox

On Sun, Oct 11, 2015 at 11:43:37AM -0700, Andrey Smirnov wrote:
> All versions of barebox_arm_entry (in uncompress.c, start.c and
> start-pbl.c) appera to be doing exacty the same thing. So move the
> definition into a separate file and use IS_ENABLED macro to avoid
> re-definition.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/Makefile     |  4 ++--
>  arch/arm/cpu/entry.c      | 37 ++++++++++++++++++++++++++++++++++++
>  arch/arm/cpu/entry.h      | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/cpu/start-pbl.c  | 29 +---------------------------
>  arch/arm/cpu/start.c      | 27 +-------------------------
>  arch/arm/cpu/uncompress.c | 16 +---------------
>  6 files changed, 90 insertions(+), 71 deletions(-)
>  create mode 100644 arch/arm/cpu/entry.c
>  create mode 100644 arch/arm/cpu/entry.h
> 
> diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
> index fb3929c..418bcab 100644
> --- a/arch/arm/cpu/Makefile
> +++ b/arch/arm/cpu/Makefile
> @@ -1,7 +1,7 @@
>  obj-y += cpu.o
>  obj-$(CONFIG_ARM_EXCEPTIONS) += exceptions.o
>  obj-$(CONFIG_ARM_EXCEPTIONS) += interrupts.o
> -obj-y += start.o setupc.o
> +obj-y += start.o setupc.o entry.o
>  
>  #
>  # Any variants can be called as start-armxyz.S
> @@ -23,7 +23,7 @@ AFLAGS_pbl-cache-armv7.o       :=-Wa,-march=armv7-a
>  pbl-$(CONFIG_CPU_32v7) += cache-armv7.o
>  obj-$(CONFIG_CACHE_L2X0) += cache-l2x0.o
>  
> -pbl-y += setupc.o
> +pbl-y += setupc.o entry.o
>  pbl-$(CONFIG_PBL_SINGLE_IMAGE) += start-pbl.o
>  pbl-$(CONFIG_PBL_MULTI_IMAGES) += uncompress.o
>  
> diff --git a/arch/arm/cpu/entry.c b/arch/arm/cpu/entry.c
> new file mode 100644
> index 0000000..adc4aec
> --- /dev/null
> +++ b/arch/arm/cpu/entry.c
> @@ -0,0 +1,37 @@
> +#include <types.h>
> +#include <asm/cache.h>
> +
> +#include "entry.h"
> +
> +/*
> + * Main ARM entry point. Call this with the memory region you can
> + * spare for barebox. This doesn't necessarily have to be the full
> + * SDRAM. The currently running binary can be inside or outside of
> + * this region. TEXT_BASE can be inside or outside of this
> + * region. boarddata will be preserved and can be accessed later with
> + * barebox_arm_boarddata().
> + *
> + * -> membase + memsize
> + *   STACK_SIZE              - stack
> + *   16KiB, aligned to 16KiB - First level page table if early MMU support
> + *                             is enabled
> + *   128KiB                  - early memory space
> + * -> maximum end of barebox binary
> + *
> + * Usually a TEXT_BASE of 1MiB below your lowest possible end of memory should
> + * be fine.
> + */
> +
> +void __naked __noreturn barebox_arm_entry(unsigned long membase,
> +					  unsigned long memsize, void *boarddata)
> +{
> +	arm_setup_stack(membase + memsize - 16);
> +	arm_early_mmu_cache_invalidate();
> +
> +	if (IS_ENABLED(CONFIG_PBL_MULTI_IMAGES))
> +		uncompress_start_payload(membase, memsize, boarddata);
> +	else if (IS_ENABLED(CONFIG_PBL_SINGLE_IMAGE))
> +		__barebox_arm_entry(membase, memsize, boarddata);
> +	else
> +		__start(membase, memsize, boarddata);
> +}

I like the idea but I think to be exported the __barebox_arm_entry and
__start functions should get a more meaningful name.

Sascha

-- 
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/8] arm/cpu/start.c: Distil some common code in __start().
  2015-10-11 18:43 ` [PATCH 3/8] arm/cpu/start.c: Distil some common code in __start() Andrey Smirnov
@ 2015-10-12  9:44   ` Sascha Hauer
  0 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2015-10-12  9:44 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox

On Sun, Oct 11, 2015 at 11:43:36AM -0700, Andrey Smirnov wrote:
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/start.c | 49 +++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 35 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
> index 8e5097b..7ffde7c 100644
> --- a/arch/arm/cpu/start.c
> +++ b/arch/arm/cpu/start.c
> @@ -53,6 +53,18 @@ void *barebox_arm_boot_dtb(void)
>  	return barebox_boot_dtb;
>  }
>  
> +static uint32_t get_any_boarddata_magic(const void *boarddata)
> +{
> +	if (get_unaligned_be32(boarddata) == FDT_MAGIC)
> +		return FDT_MAGIC;
> +
> +	if (((struct barebox_arm_boarddata *)boarddata)->magic ==
> +	    BAREBOX_ARM_BOARDDATA_MAGIC)
> +		return BAREBOX_ARM_BOARDDATA_MAGIC;
> +
> +	return 0;
> +}
> +
>  static noinline __noreturn void __start(unsigned long membase,
>  		unsigned long memsize, void *boarddata)
>  {
> @@ -89,21 +101,30 @@ static noinline __noreturn void __start(unsigned long membase,
>  	}
>  
>  	if (boarddata) {
> -		if (get_unaligned_be32(boarddata) == FDT_MAGIC) {
> -			uint32_t totalsize = get_unaligned_be32(boarddata + 4);
> +		uint32_t totalsize;
> +		void **var = NULL;
> +		const char *name;
> +
> +		switch (get_any_boarddata_magic(boarddata)) {
> +		case FDT_MAGIC:
> +			totalsize = get_unaligned_be32(boarddata + 4);
> +			var = &barebox_boot_dtb;
> +			name = "DTB";
> +			break;
> +		case BAREBOX_ARM_BOARDDATA_MAGIC:
> +			totalsize = sizeof(struct barebox_arm_boarddata);
> +			var = &barebox_boarddata;
> +			name = "machine type";
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		if (var) {
>  			endmem -= ALIGN(totalsize, 64);
> -			barebox_boot_dtb = (void *)endmem;
> -			pr_debug("found DTB in boarddata, copying to 0x%p\n",
> -					barebox_boot_dtb);
> -			memcpy(barebox_boot_dtb, boarddata, totalsize);
> -		} else if (((struct barebox_arm_boarddata *)boarddata)->magic ==
> -				BAREBOX_ARM_BOARDDATA_MAGIC) {
> -			endmem -= ALIGN(sizeof(struct barebox_arm_boarddata), 64);
> -			barebox_boarddata = (void *)endmem;
> -			pr_debug("found machine type in boarddata, copying to 0x%p\n",
> -					barebox_boarddata);
> -			memcpy(barebox_boarddata, boarddata,
> -					sizeof(struct barebox_arm_boarddata));
> +			pr_debug("found %s in boarddata, copying to 0x%lu\n",
> +				 name, endmem);
> +			*var = memcpy((void *)endmem, boarddata, totalsize);

I do not see a real improvement in this patch. The previously duplicated
code which is now shared is limited to the pr_debug message and the call
to memcpy. This is done at the cost of making the code slightly more
complicated due to the additional void **var indirection. Dropped this
patch for now. Maybe some additional cleanup or feature may make this
patch more useful, in that case I'll take it then later.

Sascha

-- 
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/8] memtest.c: Print iterations count if -i is 0
  2015-10-11 18:43 [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Andrey Smirnov
                   ` (6 preceding siblings ...)
  2015-10-11 18:43 ` [PATCH 8/8] common/parser.c: Do not treat zero return code as error Andrey Smirnov
@ 2015-10-12  9:46 ` Sascha Hauer
  7 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2015-10-12  9:46 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox

Hi Andrey,

Applied the patches I have not commented on from this series.

Sascha

On Sun, Oct 11, 2015 at 11:43:34AM -0700, Andrey Smirnov wrote:
> Even if memtest is started in endless mode it is still usefult to see
> current iteration count. Add the code to print that.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  commands/memtest.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/commands/memtest.c b/commands/memtest.c
> index 7561230..b88290c 100644
> --- a/commands/memtest.c
> +++ b/commands/memtest.c
> @@ -180,8 +180,12 @@ static int do_memtest(int argc, char *argv[])
>  		goto out;
> 
>  	for (i = 1; (i <= max_i) || !max_i; i++) {
> +		printf("Start iteration %u", i);
>  		if (max_i)
> -			printf("Start iteration %u of %u.\n", i, max_i);
> +			printf(" of %u.\n", max_i);
> +		else
> +			putchar('\n');
> +
>  		/*
>  		 * First try a memtest with caching enabled.
>  		 */
> --
> 2.1.4
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-10-12  9:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-11 18:43 [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Andrey Smirnov
2015-10-11 18:43 ` [PATCH 2/8] arm: Disable unaligned accesses Andrey Smirnov
2015-10-11 18:43 ` [PATCH 3/8] arm/cpu/start.c: Distil some common code in __start() Andrey Smirnov
2015-10-12  9:44   ` Sascha Hauer
2015-10-11 18:43 ` [PATCH 4/8] arm/cpu: Avoid multiple definitions of barebox_arm_entry Andrey Smirnov
2015-10-12  9:37   ` Sascha Hauer
2015-10-11 18:43 ` [PATCH 5/8] freescale-mx6-sabresd: Add CONFIG_DEBUG_LL support Andrey Smirnov
2015-10-11 18:43 ` [PATCH 6/8] common/command.c: Replace magic number with appropriate constant Andrey Smirnov
2015-10-11 18:43 ` [PATCH 7/8] common/parser.c: Do not conflate error reporting disciplines Andrey Smirnov
2015-10-11 18:43 ` [PATCH 8/8] common/parser.c: Do not treat zero return code as error Andrey Smirnov
2015-10-12  9:46 ` [PATCH 1/8] memtest.c: Print iterations count if -i is 0 Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox