[U-Boot] [PATCH v2 1/8] sandbox: fdt: Add support for CONFIG_OF_CONTROL

[U-Boot] [PATCH v2 1/8] sandbox: fdt: Add support for CONFIG_OF_CONTROL

[Simon Glass]

This adds support for a controlling fdt, mirroring the ARM implementation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/include/asm/global_data.h |    1 +
 arch/sandbox/lib/board.c               |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/sandbox/include/asm/global_data.h b/arch/sandbox/include/asm/global_data.h
index 8d47191..01a7063 100644
--- a/arch/sandbox/include/asm/global_data.h
+++ b/arch/sandbox/include/asm/global_data.h
@@ -45,6 +45,7 @@ typedef	struct global_data {
 	unsigned long	fb_base;	/* base address of frame buffer */
 	u8		*ram_buf;	/* emulated RAM buffer */
 	phys_size_t	ram_size;	/* RAM size */
+	const void	*fdt_blob;	/* Our device tree, NULL if none */
 	void		**jt;		/* jump table */
 	char		env_buf[32];	/* buffer for getenv() before reloc. */
 } gd_t;
diff --git a/arch/sandbox/lib/board.c b/arch/sandbox/lib/board.c
index b7997e9..c5a1177 100644
--- a/arch/sandbox/lib/board.c
+++ b/arch/sandbox/lib/board.c
@@ -156,6 +156,14 @@ void board_init_f(ulong bootflag)
 
 	memset((void *)gd, 0, sizeof(gd_t));
 
+#ifdef CONFIG_OF_EMBED
+	/* Get a pointer to the FDT */
+	gd->fdt_blob = _binary_dt_dtb_start;
+#elif defined CONFIG_OF_SEPARATE
+	/* FDT is at end of image */
+	gd->fdt_blob = (void *)(_end_ofs + _TEXT_BASE);
+#endif
+
 	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
 		if ((*init_fnc_ptr)() != 0)
 			hang();
-- 
1.7.3.1

[U-Boot] [PATCH v2 2/8] sandbox: config: Enable fdt and snprintf() options

[Simon Glass]

Enable fdt code and safe snprintf() options for sandbox.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/configs/sandbox.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 10565e6..6790216 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -28,6 +28,12 @@
 /* Number of bits in a C 'long' on this architecture */
 #define CONFIG_SANDBOX_BITS_PER_LONG	64
 
+#define CONFIG_OF_CONTROL
+#define CONFIG_OF_LIBFDT
+#define CONFIG_LMB
+
+#define CONFIG_SYS_VSNPRINTF
+
 /*
  * Size of malloc() pool, although we don't actually use this yet.
  */
-- 
1.7.3.1

[U-Boot] [PATCH v2 3/8] sandbox: gpio: Add basic driver for simulating GPIOs

[Simon Glass]

This provides a way of simulating GPIOs by setting values which are seen
by the normal gpio_get/set_value() calls.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Use generic GPIO command and interface
- Fix gpio_direction_output() to actually set the value

 arch/sandbox/include/asm/gpio.h |   61 ++++++++++++++++
 drivers/gpio/Makefile           |    1 +
 drivers/gpio/sandbox.c          |  152 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 214 insertions(+), 0 deletions(-)
 create mode 100644 arch/sandbox/include/asm/gpio.h
 create mode 100644 drivers/gpio/sandbox.c

diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h
new file mode 100644
index 0000000..405d875
--- /dev/null
+++ b/arch/sandbox/include/asm/gpio.h
@@ -0,0 +1,61 @@
+/*
+ * This is the interface to the sandbox GPIO driver for test code which
+ * wants to change the GPIO values reported to U-Boot.
+ *
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __ASM_SANDBOX_GPIO_H
+#define __ASM_SANDBOX_GPIO_H
+
+/* We use the generic interface, and add a back channel */
+#include <asm-generic/gpio.h>
+
+/**
+ * Return the value of a GPIO
+ *
+ * @param gp	GPIO number
+ * @return -1 on error, 0 if GPIO is low, >0 if high
+ */
+int sandbox_gpio_get_value(int gp);
+
+/**
+ * @param gp	GPIO number
+ * @param value	value to set (0 for low, non-zero for high)
+ * @return -1 on error, 0 if ok
+ */
+int sandbox_gpio_set_value(int gp, int value);
+
+/**
+ * Return the direction of a GPIO
+ *
+ * @param gp	GPIO number
+ * @return -1 on error, 0 if GPIO is input, >0 if output
+ */
+int sandbox_gpio_get_direction(int gp);
+
+/**
+ * @param gp	GPIO number
+ * @param output 0 to set as input, 1 to set as output
+ * @return -1 on error, 0 if ok
+ */
+int sandbox_gpio_set_direction(int gp, int output);
+
+#endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index e22c096..7315cfc 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -34,6 +34,7 @@ COBJS-$(CONFIG_MXS_GPIO)	+= mxs_gpio.o
 COBJS-$(CONFIG_PCA953X)		+= pca953x.o
 COBJS-$(CONFIG_PCA9698)		+= pca9698.o
 COBJS-$(CONFIG_S5P)		+= s5p_gpio.o
+COBJS-$(CONFIG_SANDBOX_GPIO)	+= sandbox.o
 COBJS-$(CONFIG_TEGRA2_GPIO)	+= tegra2_gpio.o
 COBJS-$(CONFIG_DA8XX_GPIO)	+= da8xx_gpio.o
 COBJS-$(CONFIG_ALTERA_PIO)	+= altera_pio.o
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
new file mode 100644
index 0000000..2c36b89
--- /dev/null
+++ b/drivers/gpio/sandbox.c
@@ -0,0 +1,152 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/bitops.h>
+#include <asm-generic/gpio.h>
+#include <asm/gpio.h>
+
+enum {
+	CMD_INFO,
+	CMD_PORT,
+	CMD_OUTPUT,
+	CMD_INPUT,
+};
+
+/* Flags for each GPIO */
+enum {
+	GPIOF_OUTPUT	= 1 << 1,
+	GPIOF_HIGH	= 1 << 2,
+};
+
+/* TODO: Put this into sandbox state */
+static u8 state[CONFIG_SANDBOX_GPIO_COUNT];	/* State of GPIOs */
+
+
+/* Access routines for GPIO state */
+static u8 *get_gpio(int gp)
+{
+	assert(gp >= 0 && gp < CONFIG_SANDBOX_GPIO_COUNT);
+	return &state[gp];
+}
+
+static int get_gpio_flag(int gp, int flag)
+{
+	return *get_gpio(gp) & flag;
+}
+
+static void set_gpio_flag(int gp, int flag, int value)
+{
+	u8 *gpio = get_gpio(gp);
+
+	if (value)
+		*gpio |= flag;
+	else
+		*gpio &= ~flag;
+}
+
+int sandbox_gpio_get_value(int gp)
+{
+	if (get_gpio_flag(gp, GPIOF_OUTPUT))
+		printf("sandbox_gpio: get_value on output GPIO %d\n", gp);
+	return *get_gpio(gp) & GPIOF_HIGH;
+}
+
+int sandbox_gpio_set_value(int gp, int value)
+{
+	set_gpio_flag(gp, GPIOF_HIGH, value);
+	return 0;
+}
+
+int sandbox_gpio_get_direction(int gp)
+{
+	return get_gpio_flag(gp, GPIOF_OUTPUT);
+}
+
+int sandbox_gpio_set_direction(int gp, int output)
+{
+	set_gpio_flag(gp, GPIOF_OUTPUT, output);
+	return 0;
+}
+
+
+/* These functions implement the public interface within U-Boot */
+
+/* set GPIO port 'gp' as an input */
+int gpio_direction_input(int gp)
+{
+	debug("gpio_direction_input: gp = %d\n", gp);
+	set_gpio_flag(gp, GPIOF_OUTPUT, 0);
+	return 0;
+}
+
+/* set GPIO port 'gp' as an output, with polarity 'value' */
+int gpio_direction_output(int gp, int value)
+{
+	debug("gpio_direction_output: gp = %d, value = %d\n",
+	      gp, value);
+	set_gpio_flag(gp, GPIOF_OUTPUT, 1);
+	set_gpio_flag(gp, GPIOF_HIGH, value);
+	return 0;
+}
+
+/* read GPIO IN value of port 'gp' */
+int gpio_get_value(int gp)
+{
+	debug("gpio_get_value: gp = %d\n", gp);
+	if (get_gpio_flag(gp, GPIOF_OUTPUT))
+		printf("sandbox_gpio: get_value on output GPIO %d\n", gp);
+	return *get_gpio(gp) & GPIOF_HIGH;
+}
+
+/* write GPIO OUT value to port 'gp' */
+int gpio_set_value(int gp, int value)
+{
+	debug("gpio_set_value: gp = %d, value = %d\n", gp, value);
+	if (get_gpio_flag(gp, GPIOF_OUTPUT)) {
+		set_gpio_flag(gp, GPIOF_HIGH, value);
+	} else {
+		printf("sandbox_gpio: set_value on input GPIO %d\n", gp);
+		return -1;
+	}
+
+	return 0;
+}
+
+int gpio_request(unsigned gpio, const char *label)
+{
+	/* for now, do nothing */
+	return 0;
+}
+
+int gpio_free(unsigned gpio)
+{
+	/* for now, do nothing */
+	return 0;
+}
+
+/* Display GPIO information */
+int gpio_info(int gp)
+{
+	printf("Sandbox GPIOs\n");
+	return 0;
+}
-- 
1.7.3.1

[U-Boot] [PATCH v2 4/8] sandbox: Enable GPIO driver

[Simon Glass]

Enable the new GPIO driver for sandbox.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/configs/sandbox.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 6790216..e7e97cc 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -34,6 +34,10 @@
 
 #define CONFIG_SYS_VSNPRINTF
 
+#define CONFIG_CMD_GPIO
+#define CONFIG_SANDBOX_GPIO
+#define CONFIG_SANDBOX_GPIO_COUNT	224
+
 /*
  * Size of malloc() pool, although we don't actually use this yet.
  */
-- 
1.7.3.1

[U-Boot] [PATCH v2 5/8] sandbox: Add concept of sandbox state

[Simon Glass]

The state exists through the life of U-Boot. It can be adjusted by command
line options and perhaps later through a config file. It is available
to U-Boot through state_...() calls (within sandbox code).

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/cpu/Makefile                     |    2 +-
 arch/sandbox/cpu/start.c                      |   10 ++++
 arch/sandbox/cpu/state.c                      |   57 ++++++++++++++++++++++++
 arch/sandbox/include/asm/arch-sandbox/state.h |   59 +++++++++++++++++++++++++
 4 files changed, 127 insertions(+), 1 deletions(-)
 create mode 100644 arch/sandbox/cpu/state.c
 create mode 100644 arch/sandbox/include/asm/arch-sandbox/state.h

diff --git a/arch/sandbox/cpu/Makefile b/arch/sandbox/cpu/Makefile
index 2ae0f71..6fd09ff 100644
--- a/arch/sandbox/cpu/Makefile
+++ b/arch/sandbox/cpu/Makefile
@@ -27,7 +27,7 @@ include $(TOPDIR)/config.mk
 
 LIB	= $(obj)lib$(CPU).o
 
-COBJS	:= cpu.o start.o os.o
+COBJS	:= cpu.o os.o start.o state.o
 
 SRCS	:= $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS))
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index a429e29..b3442e8 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -20,9 +20,19 @@
  */
 
 #include <common.h>
+#include <asm/arch/state.h>
 
 int main(int argc, char *argv[])
 {
+	struct sandbox_state *state = NULL;
+	int err;
+
+	err = state_init();
+	if (!err) {
+		state = state_get_current();
+		assert(state);
+	}
+
 	/*
 	 * Do pre- and post-relocation init, then start up U-Boot. This will
 	 * never return.
diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c
new file mode 100644
index 0000000..a1eb785
--- /dev/null
+++ b/arch/sandbox/cpu/state.c
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/arch/gpio.h>
+#include <asm/arch/state.h>
+
+/* Main state record for the sandbox */
+struct sandbox_state main_state;
+struct sandbox_state *state;	/* Pointer to current state record */
+
+int state_is_processor_reset(void)
+{
+	return 1;
+}
+
+void state_record_exit(enum exit_type_id exit_type)
+{
+	state->exit_type = exit_type;
+}
+
+struct sandbox_state *state_get_current(void)
+{
+	assert(state);
+	return state;
+}
+
+int state_init(void)
+{
+	state = &main_state;
+
+	/*
+	 * Example of how to use GPIOs:
+	 *
+	 * sandbox_gpio_set_direction(170, 0);
+	 * sandbox_gpio_set_value(170, 0);
+	 */
+	return 0;
+}
diff --git a/arch/sandbox/include/asm/arch-sandbox/state.h b/arch/sandbox/include/asm/arch-sandbox/state.h
new file mode 100644
index 0000000..3023368
--- /dev/null
+++ b/arch/sandbox/include/asm/arch-sandbox/state.h
@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/* How we exited U-Boot */
+enum exit_type_id {
+	STATE_EXIT_NORMAL,
+	STATE_EXIT_COLD_REBOOT,
+	STATE_EXIT_POWER_OFF,
+};
+
+/* The complete state of the test system */
+struct sandbox_state {
+	const char *cmd;		/* Command to execute */
+	enum exit_type_id exit_type;	/* How we exited U-Boot */
+};
+
+/**
+ * Check the type of reset that caused U-Boot to run.
+ *
+ * @return 1 = processor reset (cold start), 0 = jump from previous U-Boot
+ */
+int state_is_processor_reset(void);
+
+/**
+ * Record the exit type to be reported by the test program.
+ *
+ * @param exit_type	Exit type to record
+ */
+void state_record_exit(enum exit_type_id exit_type);
+
+/**
+ * Gets a pointer to the current state.
+ *
+ * @return pointer to state
+ */
+struct sandbox_state *state_get_current(void);
+
+/**
+ * Initialize the test system state
+ */
+int state_init(void);
-- 
1.7.3.1

[U-Boot] [PATCH v2 6/8] sandbox: Allow processing instead of or before main loop

[Simon Glass]

In order to pass command line arguments to sandbox we need to be able
to act on them. So take control back at the end of board_init_r() from
where we can call the main loop or do something else.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Call cpu_main_loop() from board_init_r()

 arch/sandbox/cpu/start.c                  |    8 ++++++++
 arch/sandbox/include/asm/u-boot-sandbox.h |    3 +++
 arch/sandbox/lib/board.c                  |    7 ++++---
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index b3442e8..d7402be 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -22,6 +22,14 @@
 #include <common.h>
 #include <asm/arch/state.h>
 
+#include <os.h>
+
+void start_main_loop(void)
+{
+	for (;;)
+		main_loop();
+}
+
 int main(int argc, char *argv[])
 {
 	struct sandbox_state *state = NULL;
diff --git a/arch/sandbox/include/asm/u-boot-sandbox.h b/arch/sandbox/include/asm/u-boot-sandbox.h
index 236b4ee..3743289 100644
--- a/arch/sandbox/include/asm/u-boot-sandbox.h
+++ b/arch/sandbox/include/asm/u-boot-sandbox.h
@@ -35,4 +35,7 @@
 int board_init(void);
 int dram_init(void);
 
+/* start.c */
+void start_main_loop(void);
+
 #endif	/* _U_BOOT_SANDBOX_H_ */
diff --git a/arch/sandbox/lib/board.c b/arch/sandbox/lib/board.c
index c5a1177..31e09d1 100644
--- a/arch/sandbox/lib/board.c
+++ b/arch/sandbox/lib/board.c
@@ -270,11 +270,12 @@ void board_init_r(gd_t *id, ulong dest_addr)
 #endif
 
 	/*
-	 * For now, run the main loop. Later we might let this be done
-	 * in the main program.
+	 * This function can't return (to match other archs) so call back
+	 * into start.c so that sandbox can do something other than the main
+	 * loop if it likes
 	 */
 	while (1)
-		main_loop();
+		start_main_loop();
 
 	/* NOTREACHED - no way out of command loop except booting */
 }
-- 
1.7.3.1

[U-Boot] [PATCH v2 7/8] sandbox: Add flags for open() call

[Simon Glass]

This provides a way for callers to create files for writing. We define
flags which mirror the POSIX values.

Another approach would be to translate the flags at runtime. Perhaps we can
leave to whoever wants to port this to another OS?

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/cpu/os.c |    2 +-
 include/os.h          |   18 +++++++++++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 093e7dc..9f93f83 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -47,7 +47,7 @@ ssize_t os_write(int fd, const void *buf, size_t count)
 
 int os_open(const char *pathname, int flags)
 {
-	return open(pathname, flags);
+	return open(pathname, flags, 0777);
 }
 
 int os_close(int fd)
diff --git a/include/os.h b/include/os.h
index f3af4f0..66e60f8 100644
--- a/include/os.h
+++ b/include/os.h
@@ -1,4 +1,9 @@
 /*
+ * Operating System Interface
+ *
+ * This provides access to useful OS routines for the sandbox architecture.
+ * They are kept in a separate file so we can include system headers.
+ *
  * Copyright (c) 2011 The Chromium OS Authors.
  * See file CREDITS for list of people who contributed to this
  * project.
@@ -19,11 +24,7 @@
  * MA 02111-1307 USA
  */
 
-/*
- * Operating System Interface
- *
- * This provides access to useful OS routines from the sandbox architecture
- */
+struct sandbox_state;
 
 /**
  * Access to the OS read() system call
@@ -45,6 +46,13 @@ ssize_t os_read(int fd, void *buf, size_t count);
  */
 ssize_t os_write(int fd, const void *buf, size_t count);
 
+enum {
+	OS_O_RDONLY,
+	OS_O_WRONLY,
+	OS_O_RDWR,
+	OS_O_CREAT	= 0100,
+};
+
 /**
  * Access to the OS open() system call
  *
-- 
1.7.3.1

[U-Boot] [PATCH v2 8/8] sandbox: Add basic command line parsing

[Simon Glass]

This adds simple command-line parssing to sandbox. The idea is that it
sets up the state with options provided, and this state can then be
queried as needed later.

For now we just allow it to run a command.

Passing a command to U-Boot on stdin is not as convenient IMO.

The parsing code is in os.c since it gives us easy access to getopt and
friends. We could create a separate parse.c file if this grows.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Clean up, added a few fdt and config patches
- Rebase on master

 arch/sandbox/cpu/os.c                         |   41 +++++++++++++++++++++++++
 arch/sandbox/cpu/start.c                      |   23 +++++++++++++-
 arch/sandbox/include/asm/arch-sandbox/state.h |    1 +
 include/os.h                                  |   20 ++++++++++++
 4 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 9f93f83..1b50442 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -21,6 +21,8 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <getopt.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <termios.h>
 #include <time.h>
@@ -30,6 +32,7 @@
 #include <sys/time.h>
 #include <sys/types.h>
 #include <linux/types.h>
+#include <asm/arch/state.h>
 
 #include <os.h>
 
@@ -122,3 +125,41 @@ u64 os_get_nsec(void)
 	return tv.tv_sec * 1000000000ULL + tv.tv_usec * 1000;
 #endif
 }
+
+static struct option long_options[] = {
+	{"command", required_argument, NULL, 'c'},
+	{"help", no_argument, NULL, 'h'},
+	{NULL, 0, NULL, 0}
+};
+
+void os_usage(int err)
+{
+	if (err < 0)
+		fprintf(stderr, "Try `--help' for more information.\n");
+	fprintf(stderr, "u-boot, "
+		"a command line test interface to U-Boot\n\n"
+		"usage:\tu-boot [-ch]\n"
+		"Options:\n"
+		"\t-h\tDisplay help\n"
+		"\t-c <command>\tExecute U-Boot command\n");
+	exit(1);
+}
+
+int os_parse_args(struct sandbox_state *state, int argc, char *argv[])
+{
+	int c;
+
+	while ((c = getopt_long(argc, argv, "c:h",
+		long_options, NULL)) != EOF) {
+		switch (c) {
+		case 'c':
+			state->cmd = optarg;
+			break;
+		case 'h':
+			return 1;
+		default: /* '?' */
+			return -1;
+		}
+	}
+	return 0;
+}
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index d7402be..fa7a907 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -26,8 +26,26 @@
 
 void start_main_loop(void)
 {
-	for (;;)
-		main_loop();
+	struct sandbox_state *state = state_get_current();
+
+	if (state->err)
+		os_usage(state->err);	/* does not return */
+
+	/* Execute command if required */
+	if (state->cmd) {
+		/* TODO: redo this when cmd tidy-up series lands */
+#ifdef CONFIG_SYS_HUSH_PARSER
+		run_command(state->cmd, 0);
+#else
+		parse_string_outer(state->cmd, FLAG_PARSE_SEMICOLON |
+				    FLAG_EXIT_FROM_LOOP);
+#endif
+	} else {
+		for (;;)
+			main_loop();
+	}
+	printf("U-Boot exited with state %d\n", state->exit_type);
+	os_exit(state->exit_type);
 }
 
 int main(int argc, char *argv[])
@@ -39,6 +57,7 @@ int main(int argc, char *argv[])
 	if (!err) {
 		state = state_get_current();
 		assert(state);
+		state->err = os_parse_args(state, argc, argv);
 	}
 
 	/*
diff --git a/arch/sandbox/include/asm/arch-sandbox/state.h b/arch/sandbox/include/asm/arch-sandbox/state.h
index 3023368..1e035fa 100644
--- a/arch/sandbox/include/asm/arch-sandbox/state.h
+++ b/arch/sandbox/include/asm/arch-sandbox/state.h
@@ -30,6 +30,7 @@ enum exit_type_id {
 struct sandbox_state {
 	const char *cmd;		/* Command to execute */
 	enum exit_type_id exit_type;	/* How we exited U-Boot */
+	int err;			/* Error to report from parsing */
 };
 
 /**
diff --git a/include/os.h b/include/os.h
index 66e60f8..2b66813 100644
--- a/include/os.h
+++ b/include/os.h
@@ -106,3 +106,23 @@ void os_usleep(unsigned long usec);
  * \return A monotonic increasing time scaled in nano seconds
  */
 u64 os_get_nsec(void);
+
+/**
+ * Parse arguments and update sandbox state.
+ *
+ * @param state		Sandbox state to update
+ * @param argc		Argument count
+ * @param argv		Argument vector
+ * @return 0 if ok, and program should continue;
+ *	1 if ok, but program should stop;
+ *	-1 on error: program should terminate.
+ */
+int os_parse_args(struct sandbox_state *state, int argc, char *argv[]);
+
+/**
+ * Display a usage message on stderr and exit
+ *
+ * @param err		Error code (-ve means the user entered an invalid
+ *			option)
+ */
+void os_usage(int err);
-- 
1.7.3.1

[U-Boot] [PATCH v2 7/8] sandbox: Add flags for open() call

[Mike Frysinger]

On Tuesday 10 January 2012 19:45:51 Simon Glass wrote:
> This provides a way for callers to create files for writing. We define
> flags which mirror the POSIX values.
> 
> Another approach would be to translate the flags at runtime. Perhaps we can
> leave to whoever wants to port this to another OS?

some of the flags differ between Linux/arches, so i think we'll have to do it 
now :/

> +enum {
> +	OS_O_RDONLY,
> +	OS_O_WRONLY,
> +	OS_O_RDWR,
> +	OS_O_CREAT	= 0100,
> +};

the flags are bit based, so an enum won't work i don't think
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120114/ab96103f/attachment.pgp>

[U-Boot] [PATCH v2 7/8] sandbox: Add flags for open() call

[Simon Glass]

Hi Mike,

On Sat, Jan 14, 2012 at 8:19 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 10 January 2012 19:45:51 Simon Glass wrote:
>> This provides a way for callers to create files for writing. We define
>> flags which mirror the POSIX values.
>>
>> Another approach would be to translate the flags at runtime. Perhaps we can
>> leave to whoever wants to port this to another OS?
>
> some of the flags differ between Linux/arches, so i think we'll have to do it
> now :/
>
>> +enum {
>> + ? ? OS_O_RDONLY,
>> + ? ? OS_O_WRONLY,
>> + ? ? OS_O_RDWR,
>> + ? ? OS_O_CREAT ? ? ?= 0100,
>> +};
>
> the flags are bit based, so an enum won't work i don't think

Mostly, but in /usr/include/bits/fcntl.h:

#define O_RDONLY	     00
#define O_WRONLY	     01
#define O_RDWR		     02
#define O_CREAT		   0100	/* not fcntl */

Regards,
Simon

Regards,
Simon

> -mike

[U-Boot] [PATCH v2 7/8] sandbox: Add flags for open() call

[Mike Frysinger]

On Saturday 14 January 2012 23:36:40 Simon Glass wrote:
> On Sat, Jan 14, 2012 at 8:19 PM, Mike Frysinger wrote:
> > On Tuesday 10 January 2012 19:45:51 Simon Glass wrote:
> >> +enum {
> >> +     OS_O_RDONLY,
> >> +     OS_O_WRONLY,
> >> +     OS_O_RDWR,
> >> +     OS_O_CREAT      = 0100,
> >> +};
> > 
> > the flags are bit based, so an enum won't work i don't think
> 
> Mostly, but in /usr/include/bits/fcntl.h:
> 
> #define O_RDONLY	     00
> #define O_WRONLY	     01
> #define O_RDWR		     02
> #define O_CREAT		   0100	/* not fcntl */

what i mean is that people don't do bitwise operations with enums.  so to open 
a few for writing and create it, you have to do:
	open(..., O_CREAT|O_WRONLY)

which is just weird with enums
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120114/249ed228/attachment.pgp>

[U-Boot] [PATCH v2 7/8] sandbox: Add flags for open() call

[Simon Glass]

Hi Mike,

On Sat, Jan 14, 2012 at 8:44 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Saturday 14 January 2012 23:36:40 Simon Glass wrote:
>> On Sat, Jan 14, 2012 at 8:19 PM, Mike Frysinger wrote:
>> > On Tuesday 10 January 2012 19:45:51 Simon Glass wrote:
>> >> +enum {
>> >> + ? ? OS_O_RDONLY,
>> >> + ? ? OS_O_WRONLY,
>> >> + ? ? OS_O_RDWR,
>> >> + ? ? OS_O_CREAT ? ? ?= 0100,
>> >> +};
>> >
>> > the flags are bit based, so an enum won't work i don't think
>>
>> Mostly, but in /usr/include/bits/fcntl.h:
>>
>> #define O_RDONLY ? ? ? ? ? 00
>> #define O_WRONLY ? ? ? ? ? 01
>> #define O_RDWR ? ? ? ? ? ? ? ? ? ? 02
>> #define O_CREAT ? ? ? ? ? ? ? ? ?0100 /* not fcntl */
>
> what i mean is that people don't do bitwise operations with enums. ?so to open
> a few for writing and create it, you have to do:
> ? ? ? ?open(..., O_CREAT|O_WRONLY)
>
> which is just weird with enums

OK I see. I have a higher tolerance to enums than most people and
debuggers like them. I will change these to #define.

Regards,
Simon

> -mike

[U-Boot] [PATCH v2 1/8] sandbox: fdt: Add support for CONFIG_OF_CONTROL

[Mike Frysinger]

On Tuesday 10 January 2012 19:45:45 Simon Glass wrote:
> This adds support for a controlling fdt, mirroring the ARM implementation.

OK, but what's the point if we aren't doing FDT's ?

> +#ifdef CONFIG_OF_EMBED
> +	/* Get a pointer to the FDT */
> +	gd->fdt_blob = _binary_dt_dtb_start;
> +#elif defined CONFIG_OF_SEPARATE
> +	/* FDT is at end of image */
> +	gd->fdt_blob = (void *)(_end_ofs + _TEXT_BASE);
> +#endif

i'd be inclined to do:
+#if defined(CONFIG_OF_EMBED)
	...
+#elif defined(CONFIG_OF_SEPARATE)
	...
+#endif
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120120/5b74d2f0/attachment.pgp>

[U-Boot] [PATCH v2 2/8] sandbox: config: Enable fdt and snprintf() options

[Mike Frysinger]

On Tuesday 10 January 2012 19:45:46 Simon Glass wrote:
> +#define CONFIG_LMB

do we need this ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120120/a53024aa/attachment.pgp>

[U-Boot] [PATCH v2 3/8] sandbox: gpio: Add basic driver for simulating GPIOs

[Mike Frysinger]

On Tuesday 10 January 2012 19:45:47 Simon Glass wrote:
> This provides a way of simulating GPIOs by setting values which are seen
> by the normal gpio_get/set_value() calls.

seems to be a desync in types ... all "gpio" fields should be "unsigned" and 
not "int"

> --- /dev/null
> +++ b/arch/sandbox/include/asm/gpio.h
>
> +int sandbox_gpio_get_value(int gp);

why bother with parallel sandbox gpio API ?  why can't we just implement the 
gpio API directly and throw away sandbox_gpio_xxx ?  then we can also stub out 
sandbox/include/asm/gpio.h ...

also, missing gpio_status() define to gpio_info()

> --- /dev/null
> +++ b/drivers/gpio/sandbox.c
>
> +enum {
> +	CMD_INFO,
> +	CMD_PORT,
> +	CMD_OUTPUT,
> +	CMD_INPUT,
> +};

CMD_XXX enums are unused -> punt

> +enum {
> +	GPIOF_OUTPUT	= 1 << 1,
> +	GPIOF_HIGH	= 1 << 2,
> +};

turn enum bit flags into defines

also, add a "reserved" bit flag and check it in gpio_request()

> +/* read GPIO IN value of port 'gp' */
> +int gpio_get_value(int gp)
> ...
> +	if (get_gpio_flag(gp, GPIOF_OUTPUT))
> ...
> +int gpio_set_value(int gp, int value)
> ...
> +	if (get_gpio_flag(gp, GPIOF_OUTPUT)) {

drop valid gpio checking in these funcs ... only the request func should do 
this

> +int gpio_request(unsigned gpio, const char *label)
> +{
> +	/* for now, do nothing */
> +	return 0;
> +}

add (gpio <= CONFIG_SANDBOX_GPIO_COUNT) check to verify the gpio is valid

> +int gpio_info(int gp)
> +{
> +	printf("Sandbox GPIOs\n");
> +	return 0;
> +}

implement it ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120120/5fba4e7f/attachment.pgp>

[U-Boot] [PATCH v2 4/8] sandbox: Enable GPIO driver

[Mike Frysinger]

On Tuesday 10 January 2012 19:45:48 Simon Glass wrote:
> --- a/include/configs/sandbox.h
> +++ b/include/configs/sandbox.h
> 
> +#define CONFIG_SANDBOX_GPIO_COUNT	224

do we really need 224 examples GPIOs ?  can't we do with like 20 ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120120/9d3355d3/attachment.pgp>

[U-Boot] [PATCH v2 6/8] sandbox: Allow processing instead of or before main loop

[Mike Frysinger]

On Tuesday 10 January 2012 19:45:50 Simon Glass wrote:
> In order to pass command line arguments to sandbox we need to be able
> to act on them. So take control back at the end of board_init_r() from
> where we can call the main loop or do something else.

does this need to be done this early ?  parsing args can easily be done inside 
main() (as i showed in my patch that added command line support).

in terms of processing the state, i think we can just make this into an init 
func that runs before we let board_init_r tail into main_loop
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120120/850bed92/attachment.pgp>

[U-Boot] [PATCH v2 8/8] sandbox: Add basic command line parsing

[Mike Frysinger]

On Tuesday 10 January 2012 19:45:52 Simon Glass wrote:
> --- a/arch/sandbox/cpu/os.c
> +++ b/arch/sandbox/cpu/os.c
>
> +static struct option long_options[] = {

constify

> +void os_usage(int err)
> +{
> +	if (err < 0)
> +		fprintf(stderr, "Try `--help' for more information.\n");
> +	fprintf(stderr, "u-boot, "
> +		"a command line test interface to U-Boot\n\n"
> +		"usage:\tu-boot [-ch]\n"
> +		"Options:\n"
> +		"\t-h\tDisplay help\n"
> +		"\t-c <command>\tExecute U-Boot command\n");
> +	exit(1);
> +}

os_usage() should only write to stderr/exit(1) if it's an error, otherwise it 
should use stdout/exit(0)

> +int os_parse_args(struct sandbox_state *state, int argc, char *argv[])
> +{
> ...
> +	while ((c = getopt_long(argc, argv, "c:h",
> +		long_options, NULL)) != EOF) {

pull the optstring ("c:h") out of the getopt_long call and put it into a const 
next to the long_options[] variable

and next to opts array

> +	/* Execute command if required */
> +	if (state->cmd) {
> +		/* TODO: redo this when cmd tidy-up series lands */
> +#ifdef CONFIG_SYS_HUSH_PARSER
> +		run_command(state->cmd, 0);
> +#else
> +		parse_string_outer(state->cmd, FLAG_PARSE_SEMICOLON |
> +				    FLAG_EXIT_FROM_LOOP);
> +#endif

i'm not sure how useful -c is since we can already do:
          ./u-boot <<<"some command"

each to their own i guess ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120120/24202753/attachment.pgp>

[U-Boot] [PATCH v2 1/8] sandbox: fdt: Add support for CONFIG_OF_CONTROL

[Simon Glass]

Hi Mike,

On Fri, Jan 20, 2012 at 10:54 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 10 January 2012 19:45:45 Simon Glass wrote:
>> This adds support for a controlling fdt, mirroring the ARM implementation.
>
> OK, but what's the point if we aren't doing FDT's ?

Aren't we? I hope that sandbox provides a way of testing the FDT
stuff. I used it for the fdtdec alias stuff which was complicated
enough that it needed unit tests. We need to enable these options to
bring in that code, even if we don't actually have a run-time FDT yet.

>
>> +#ifdef CONFIG_OF_EMBED
>> + ? ? /* Get a pointer to the FDT */
>> + ? ? gd->fdt_blob = _binary_dt_dtb_start;
>> +#elif defined CONFIG_OF_SEPARATE
>> + ? ? /* FDT is at end of image */
>> + ? ? gd->fdt_blob = (void *)(_end_ofs + _TEXT_BASE);
>> +#endif
>
> i'd be inclined to do:
> +#if defined(CONFIG_OF_EMBED)
> ? ? ? ?...
> +#elif defined(CONFIG_OF_SEPARATE)
> ? ? ? ?...
> +#endif

OK done.

Regards,
Simon

> -mike

[U-Boot] [PATCH v2 2/8] sandbox: config: Enable fdt and snprintf() options

[Simon Glass]

Hi Mike,

On Fri, Jan 20, 2012 at 10:54 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 10 January 2012 19:45:46 Simon Glass wrote:
>> +#define CONFIG_LMB
>
> do we need this ?

Without it I think bootm fails to build with FDT enabled.

Regards,
Simon

> -mike

[U-Boot] [PATCH v2 3/8] sandbox: gpio: Add basic driver for simulating GPIOs

[Simon Glass]

Hi Mike,

On Fri, Jan 20, 2012 at 10:59 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 10 January 2012 19:45:47 Simon Glass wrote:
>> This provides a way of simulating GPIOs by setting values which are seen
>> by the normal gpio_get/set_value() calls.
>
> seems to be a desync in types ... all "gpio" fields should be "unsigned" and
> not "int"

OK I see that changed in November. I will update.

>
>> --- /dev/null
>> +++ b/arch/sandbox/include/asm/gpio.h
>>
>> +int sandbox_gpio_get_value(int gp);
>
> why bother with parallel sandbox gpio API ? ?why can't we just implement the
> gpio API directly and throw away sandbox_gpio_xxx ? ?then we can also stub out
> sandbox/include/asm/gpio.h ...

Because the current state of the GPIOs needs to be stored somewhere.
Test code which wants a GPIO to appear to be high to U-Boot can call
sandbox_gpio_set_value() and that value will be recorded and provided
to future gpio_get_value() calls.

Without this virtualisation, the driver would have no purpose.

>
> also, missing gpio_status() define to gpio_info()

I see gpio_info() there at the bottom - gpio_status() is optional I
think. Do we need to bring this in?

>
>> --- /dev/null
>> +++ b/drivers/gpio/sandbox.c
>>
>> +enum {
>> + ? ? CMD_INFO,
>> + ? ? CMD_PORT,
>> + ? ? CMD_OUTPUT,
>> + ? ? CMD_INPUT,
>> +};
>
> CMD_XXX enums are unused -> punt

Done

>
>> +enum {
>> + ? ? GPIOF_OUTPUT ? ?= 1 << 1,
>> + ? ? GPIOF_HIGH ? ? ?= 1 << 2,
>> +};
>
> turn enum bit flags into defines

Done

>
> also, add a "reserved" bit flag and check it in gpio_request()

Oh ok then, might as well do it now. I will also complain if someone
uses a GPIO before requesting it.

>
>> +/* read GPIO IN value of port 'gp' */
>> +int gpio_get_value(int gp)
>> ...
>> + ? ? if (get_gpio_flag(gp, GPIOF_OUTPUT))
>> ...
>> +int gpio_set_value(int gp, int value)
>> ...
>> + ? ? if (get_gpio_flag(gp, GPIOF_OUTPUT)) {
>
> drop valid gpio checking in these funcs ... only the request func should do
> this

What if someone passes in garbage value? Don't we want to catch this?

>
>> +int gpio_request(unsigned gpio, const char *label)
>> +{
>> + ? ? /* for now, do nothing */
>> + ? ? return 0;
>> +}
>
> add (gpio <= CONFIG_SANDBOX_GPIO_COUNT) check to verify the gpio is valid

Done as part of implementing this function.

>
>> +int gpio_info(int gp)
>> +{
>> + ? ? printf("Sandbox GPIOs\n");
>> + ? ? return 0;
>> +}
>
> implement it ?

Yes OK.

> -mike

Regards,
Simon

[U-Boot] [PATCH v2 4/8] sandbox: Enable GPIO driver

[Simon Glass]

Hi Mike,

On Fri, Jan 20, 2012 at 10:59 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 10 January 2012 19:45:48 Simon Glass wrote:
>> --- a/include/configs/sandbox.h
>> +++ b/include/configs/sandbox.h
>>
>> +#define CONFIG_SANDBOX_GPIO_COUNT ? ?224
>
> do we really need 224 examples GPIOs ? ?can't we do with like 20 ?

Yes, should be plenty to cover the zero we need right now :-)

> -mike

Regards,
Simon

[U-Boot] [PATCH v2 6/8] sandbox: Allow processing instead of or before main loop

[Simon Glass]

Hi Mike,

On Fri, Jan 20, 2012 at 11:00 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 10 January 2012 19:45:50 Simon Glass wrote:
>> In order to pass command line arguments to sandbox we need to be able
>> to act on them. So take control back at the end of board_init_r() from
>> where we can call the main loop or do something else.
>
> does this need to be done this early ? ?parsing args can easily be done inside
> main() (as i showed in my patch that added command line support).

My thought here is that I would like the command line to influence
U-Boot right from the start, before board init. If we don't know our
parameters until then, then we might do something we shouldn't. For
one thing, if the arguments are --help then we are wasting our time.

One little niggle is that it isn't really possible at present to call
printf() too early. I have left it along for now, and just come back
later and print usage, but it *might* be necessary at some point to
implement os_printf() or similar to allow messages from Sandbox.

>
> in terms of processing the state, i think we can just make this into an init
> func that runs before we let board_init_r tail into main_loop

Well the requirement may be that we don't even want to run main_loop -
see the next patch.

> -mike

Regards,
Simon

[U-Boot] [PATCH v2 8/8] sandbox: Add basic command line parsing

[Simon Glass]

Hi Mike,

On Fri, Jan 20, 2012 at 11:05 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 10 January 2012 19:45:52 Simon Glass wrote:
>> --- a/arch/sandbox/cpu/os.c
>> +++ b/arch/sandbox/cpu/os.c
>>
>> +static struct option long_options[] = {
>
> constify
>
Done

>> +void os_usage(int err)
>> +{
>> + ? ? if (err < 0)
>> + ? ? ? ? ? ? fprintf(stderr, "Try `--help' for more information.\n");
>> + ? ? fprintf(stderr, "u-boot, "
>> + ? ? ? ? ? ? "a command line test interface to U-Boot\n\n"
>> + ? ? ? ? ? ? "usage:\tu-boot [-ch]\n"
>> + ? ? ? ? ? ? "Options:\n"
>> + ? ? ? ? ? ? "\t-h\tDisplay help\n"
>> + ? ? ? ? ? ? "\t-c <command>\tExecute U-Boot command\n");
>> + ? ? exit(1);
>> +}
>
> os_usage() should only write to stderr/exit(1) if it's an error, otherwise it
> should use stdout/exit(0)

OK, done. I knew there was a reason I wanted the error code.

>
>> +int os_parse_args(struct sandbox_state *state, int argc, char *argv[])
>> +{
>> ...
>> + ? ? while ((c = getopt_long(argc, argv, "c:h",
>> + ? ? ? ? ? ? long_options, NULL)) != EOF) {
>
> pull the optstring ("c:h") out of the getopt_long call and put it into a const
> next to the long_options[] variable
>
> and next to opts array

Much better thank you.

>
>> + ? ? /* Execute command if required */
>> + ? ? if (state->cmd) {
>> + ? ? ? ? ? ? /* TODO: redo this when cmd tidy-up series lands */
>> +#ifdef CONFIG_SYS_HUSH_PARSER
>> + ? ? ? ? ? ? run_command(state->cmd, 0);
>> +#else
>> + ? ? ? ? ? ? parse_string_outer(state->cmd, FLAG_PARSE_SEMICOLON |
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? FLAG_EXIT_FROM_LOOP);
>> +#endif
>
> i'm not sure how useful -c is since we can already do:
> ? ? ? ? ?./u-boot <<<"some command"
>
> each to their own i guess ...

I had trouble getting that to work properly, ending up with a runaway
process. Probably we don't handle EOF correctly since we just loop
forever.

I do think this argument is useful and it (and help) provide a
reasonable excuse for a command line parser.

> -mike

Regards,
Simon

[U-Boot] [PATCH v2 1/8] sandbox: fdt: Add support for CONFIG_OF_CONTROL

[Mike Frysinger]

On Monday 23 January 2012 00:27:56 Simon Glass wrote:
> On Fri, Jan 20, 2012 at 10:54 AM, Mike Frysinger wrote:
> > On Tuesday 10 January 2012 19:45:45 Simon Glass wrote:
> >> This adds support for a controlling fdt, mirroring the ARM
> >> implementation.
> > 
> > OK, but what's the point if we aren't doing FDT's ?
> 
> Aren't we?

i don't see FDT's getting used anywhere in this patch or this patch series.  
so no, we aren't currently doing FDT's in sandbox.

> I hope that sandbox provides a way of testing the FDT
> stuff. I used it for the fdtdec alias stuff which was complicated
> enough that it needed unit tests.

if it actually can be used/tested under sandbox, then i'm fine with this.  but 
can that actually be done now or are other things necessary first ?

> We need to enable these options to bring in that code, even if we don't
> actually have a run-time FDT yet.

i don't see how run-time FDT's would be used by sandbox ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120123/7775ffe6/attachment.pgp>

[U-Boot] [PATCH v2 2/8] sandbox: config: Enable fdt and snprintf() options

[Mike Frysinger]

On Monday 23 January 2012 00:30:32 Simon Glass wrote:
> On Fri, Jan 20, 2012 at 10:54 AM, Mike Frysinger wrote:
> > On Tuesday 10 January 2012 19:45:46 Simon Glass wrote:
> >> +#define CONFIG_LMB
> > 
> > do we need this ?
> 
> Without it I think bootm fails to build with FDT enabled.

looks like CONFIG_OF_LIBFDT hard depends on CONFIG_LMB, and a lot of lmb stuff 
leaks when it isn't defined.  i think i'll send a patch to tighten up the lmb.h 
header when that define isn't available.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120124/1bc335c1/attachment.pgp>

[U-Boot] [PATCH v2 3/8] sandbox: gpio: Add basic driver for simulating GPIOs

[Mike Frysinger]

On Monday 23 January 2012 01:20:16 Simon Glass wrote:
> On Fri, Jan 20, 2012 at 10:59 AM, Mike Frysinger wrote:
> > On Tuesday 10 January 2012 19:45:47 Simon Glass wrote:
> >> --- /dev/null
> >> +++ b/arch/sandbox/include/asm/gpio.h
> >> 
> >> +int sandbox_gpio_get_value(int gp);
> > 
> > why bother with parallel sandbox gpio API ?  why can't we just implement
> > the gpio API directly and throw away sandbox_gpio_xxx ?  then we can
> > also stub out sandbox/include/asm/gpio.h ...
> 
> Because the current state of the GPIOs needs to be stored somewhere.
> Test code which wants a GPIO to appear to be high to U-Boot can call
> sandbox_gpio_set_value() and that value will be recorded and provided
> to future gpio_get_value() calls.
> 
> Without this virtualisation, the driver would have no purpose.

i'm not seeing it.  why does external code need to reach into the guts at all 
when we have a gpio API for them to use ?

> > also, missing gpio_status() define to gpio_info()
> 
> I see gpio_info() there at the bottom - gpio_status() is optional I
> think. Do we need to bring this in?

gpio_info is pointless without gpio_status.  honestly, i don't even know why 
that symbol exists at all ... the public API is gpio_status().  might as well 
just call it that.

> >> +/* read GPIO IN value of port 'gp' */
> >> +int gpio_get_value(int gp)
> >> ...
> >> +     if (get_gpio_flag(gp, GPIOF_OUTPUT))
> >> ...
> >> +int gpio_set_value(int gp, int value)
> >> ...
> >> +     if (get_gpio_flag(gp, GPIOF_OUTPUT)) {
> > 
> > drop valid gpio checking in these funcs ... only the request func should
> > do this
> 
> What if someone passes in garbage value? Don't we want to catch this?

normally, no.  in the sandbox world, i guess we do, but there should be 
verbose dump messages ... perhaps an assert() ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120124/a52b82a3/attachment.pgp>

[U-Boot] [PATCH v2 8/8] sandbox: Add basic command line parsing

[Mike Frysinger]

On Monday 23 January 2012 01:30:52 Simon Glass wrote:
> On Fri, Jan 20, 2012 at 11:05 AM, Mike Frysinger wrote:
> > i'm not sure how useful -c is since we can already do:
> >          ./u-boot <<<"some command"
> > 
> > each to their own i guess ...
> 
> I had trouble getting that to work properly, ending up with a runaway
> process. Probably we don't handle EOF correctly since we just loop
> forever.

yes, it does do that by (current) design :)

> I do think this argument is useful and it (and help) provide a
> reasonable excuse for a command line parser.

np
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120124/bdce994f/attachment.pgp>

[U-Boot] [PATCH v2 3/8] sandbox: gpio: Add basic driver for simulating GPIOs

[Mike Frysinger]

On Tuesday 24 January 2012 16:35:00 Mike Frysinger wrote:
> On Monday 23 January 2012 01:20:16 Simon Glass wrote:
> > On Fri, Jan 20, 2012 at 10:59 AM, Mike Frysinger wrote:
> > > On Tuesday 10 January 2012 19:45:47 Simon Glass wrote:
> > >> --- /dev/null
> > >> +++ b/arch/sandbox/include/asm/gpio.h
> > >> 
> > >> +int sandbox_gpio_get_value(int gp);
> > > 
> > > why bother with parallel sandbox gpio API ?  why can't we just
> > > implement the gpio API directly and throw away sandbox_gpio_xxx ? 
> > > then we can also stub out sandbox/include/asm/gpio.h ...
> > 
> > Because the current state of the GPIOs needs to be stored somewhere.
> > Test code which wants a GPIO to appear to be high to U-Boot can call
> > sandbox_gpio_set_value() and that value will be recorded and provided
> > to future gpio_get_value() calls.
> > 
> > Without this virtualisation, the driver would have no purpose.
> 
> i'm not seeing it.  why does external code need to reach into the guts at
> all when we have a gpio API for them to use ?

ok, you clarified it for me, so now we just need some comments in asm/gpio.h 
explaining the expected users of the internal API
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120124/ee78d5d2/attachment.pgp>

[U-Boot] [PATCH v2 1/8] sandbox: fdt: Add support for CONFIG_OF_CONTROL

[Simon Glass]

Hi Mike,

On Sun, Jan 22, 2012 at 11:32 PM, Mike Frysinger <vapier@gentoo.org> wrote:

> On Monday 23 January 2012 00:27:56 Simon Glass wrote:
> > On Fri, Jan 20, 2012 at 10:54 AM, Mike Frysinger wrote:
> > > On Tuesday 10 January 2012 19:45:45 Simon Glass wrote:
> > >> This adds support for a controlling fdt, mirroring the ARM
> > >> implementation.
> > >
> > > OK, but what's the point if we aren't doing FDT's ?
> >
> > Aren't we?
>
> i don't see FDT's getting used anywhere in this patch or this patch series.
> so no, we aren't currently doing FDT's in sandbox.
>

I have a patch which tests fdtdec and this uses FDT support in Sandbox. I
don't see any reason to have it turned off - it's just a pain for people to
turn it on.


>
> > I hope that sandbox provides a way of testing the FDT
> > stuff. I used it for the fdtdec alias stuff which was complicated
> > enough that it needed unit tests.
>
> if it actually can be used/tested under sandbox, then i'm fine with this.
>  but
> can that actually be done now or are other things necessary first ?
>

It can be done now - it works fine as is.


>
> > We need to enable these options to bring in that code, even if we don't
> > actually have a run-time FDT yet.
>
> i don't see how run-time FDT's would be used by sandbox ...
>

Well they could be used to configure the SPI flash driver, instead of
command line, for example. But that's a separate issue. Really I just want
to have the feature enabled so that the code is compiled in.

Regards,
Simon



> -mike
>

[U-Boot] [PATCH v2 2/8] sandbox: config: Enable fdt and snprintf() options

[Simon Glass]

Hi Mike,

On Tue, Jan 24, 2012 at 1:24 PM, Mike Frysinger <vapier@gentoo.org> wrote:

> On Monday 23 January 2012 00:30:32 Simon Glass wrote:
> > On Fri, Jan 20, 2012 at 10:54 AM, Mike Frysinger wrote:
> > > On Tuesday 10 January 2012 19:45:46 Simon Glass wrote:
> > >> +#define CONFIG_LMB
> > >
> > > do we need this ?
> >
> > Without it I think bootm fails to build with FDT enabled.
>
> looks like CONFIG_OF_LIBFDT hard depends on CONFIG_LMB, and a lot of lmb
> stuff
> leaks when it isn't defined.  i think i'll send a patch to tighten up the
> lmb.h
> header when that define isn't available.
> -mike
>

OK - did you do this? Just wondering if I should drop this CONFIG now...

Regards,
Simon

[U-Boot] [PATCH v2 3/8] sandbox: gpio: Add basic driver for simulating GPIOs

[Simon Glass]

Hi Mike,

On Tue, Jan 24, 2012 at 3:06 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 24 January 2012 16:35:00 Mike Frysinger wrote:
>> On Monday 23 January 2012 01:20:16 Simon Glass wrote:
>> > On Fri, Jan 20, 2012 at 10:59 AM, Mike Frysinger wrote:
>> > > On Tuesday 10 January 2012 19:45:47 Simon Glass wrote:
>> > >> --- /dev/null
>> > >> +++ b/arch/sandbox/include/asm/gpio.h
>> > >>
>> > >> +int sandbox_gpio_get_value(int gp);
>> > >
>> > > why bother with parallel sandbox gpio API ? ?why can't we just
>> > > implement the gpio API directly and throw away sandbox_gpio_xxx ?
>> > > then we can also stub out sandbox/include/asm/gpio.h ...
>> >
>> > Because the current state of the GPIOs needs to be stored somewhere.
>> > Test code which wants a GPIO to appear to be high to U-Boot can call
>> > sandbox_gpio_set_value() and that value will be recorded and provided
>> > to future gpio_get_value() calls.
>> >
>> > Without this virtualisation, the driver would have no purpose.
>>
>> i'm not seeing it. ?why does external code need to reach into the guts at
>> all when we have a gpio API for them to use ?
>
> ok, you clarified it for me, so now we just need some comments in asm/gpio.h
> explaining the expected users of the internal API
> -mike

OK I will add a few more comments.

Regards,
Simon

[U-Boot] [PATCH v2 2/8] sandbox: config: Enable fdt and snprintf() options

[Mike Frysinger]

On Wednesday 15 February 2012 01:26:10 Simon Glass wrote:
> On Tue, Jan 24, 2012 at 1:24 PM, Mike Frysinger wrote:
> > On Monday 23 January 2012 00:30:32 Simon Glass wrote:
> > > On Fri, Jan 20, 2012 at 10:54 AM, Mike Frysinger wrote:
> > > > On Tuesday 10 January 2012 19:45:46 Simon Glass wrote:
> > > >> +#define CONFIG_LMB
> > > > 
> > > > do we need this ?
> > > 
> > > Without it I think bootm fails to build with FDT enabled.
> > 
> > looks like CONFIG_OF_LIBFDT hard depends on CONFIG_LMB, and a lot of lmb
> > stuff
> > leaks when it isn't defined.  i think i'll send a patch to tighten up the
> > lmb.h
> > header when that define isn't available.
> > -mike
> 
> OK - did you do this? Just wondering if I should drop this CONFIG now...

i posted a patch, but we'll still need the lmb define
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120221/3557cbac/attachment.pgp>

[U-Boot] [PATCH v2 2/8] sandbox: config: Enable fdt and snprintf() options

[Simon Glass]

Hi Mike,

On Mon, Feb 20, 2012 at 9:24 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 15 February 2012 01:26:10 Simon Glass wrote:
>> On Tue, Jan 24, 2012 at 1:24 PM, Mike Frysinger wrote:
>> > On Monday 23 January 2012 00:30:32 Simon Glass wrote:
>> > > On Fri, Jan 20, 2012 at 10:54 AM, Mike Frysinger wrote:
>> > > > On Tuesday 10 January 2012 19:45:46 Simon Glass wrote:
>> > > >> +#define CONFIG_LMB
>> > > >
>> > > > do we need this ?
>> > >
>> > > Without it I think bootm fails to build with FDT enabled.
>> >
>> > looks like CONFIG_OF_LIBFDT hard depends on CONFIG_LMB, and a lot of lmb
>> > stuff
>> > leaks when it isn't defined. ?i think i'll send a patch to tighten up the
>> > lmb.h
>> > header when that define isn't available.
>> > -mike
>>
>> OK - did you do this? Just wondering if I should drop this CONFIG now...
>
> i posted a patch, but we'll still need the lmb define

OK. So this patch stays as is?

> -mike

Regards,
Simon

[U-Boot] [PATCH v2 2/8] sandbox: config: Enable fdt and snprintf() options

[Mike Frysinger]

On Tuesday 21 February 2012 00:37:16 Simon Glass wrote:
> On Mon, Feb 20, 2012 at 9:24 PM, Mike Frysinger wrote:
> > On Wednesday 15 February 2012 01:26:10 Simon Glass wrote:
> >> On Tue, Jan 24, 2012 at 1:24 PM, Mike Frysinger wrote:
> >> > On Monday 23 January 2012 00:30:32 Simon Glass wrote:
> >> > > On Fri, Jan 20, 2012 at 10:54 AM, Mike Frysinger wrote:
> >> > > > On Tuesday 10 January 2012 19:45:46 Simon Glass wrote:
> >> > > >> +#define CONFIG_LMB
> >> > > > 
> >> > > > do we need this ?
> >> > > 
> >> > > Without it I think bootm fails to build with FDT enabled.
> >> > 
> >> > looks like CONFIG_OF_LIBFDT hard depends on CONFIG_LMB, and a lot of
> >> > lmb stuff
> >> > leaks when it isn't defined.  i think i'll send a patch to tighten up
> >> > the lmb.h
> >> > header when that define isn't available.
> >> > -mike
> >> 
> >> OK - did you do this? Just wondering if I should drop this CONFIG now...
> > 
> > i posted a patch, but we'll still need the lmb define
> 
> OK. So this patch stays as is?

correct ... i responded to your v4 that i merged this as is
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120221/fe5b477a/attachment.pgp>