[PATCH 0/9] introduction of dmesg support

[PATCH 0/9] introduction of dmesg support

[Jean-Christophe PLAGNIOL-VILLARD]

HI,

	we currently have 2 fifo at comsole level one for input one for output
	we fill the output fifo before any console is registered and then
	empty it

	Now we introduce a new mecanism as in the kernel dmesg

	we will always fill the output kfifo with drivers output

	as in barebox we have 2 world

	device/drivers and applicaiotn (commands)

	so for now on drivers will have to use pr_xxx and dev_xxx and
	application will use printf

	dmesg will print the kfifo at any time

	we will extend this later with more control of what the drivers output
	on the console or just in the fifo

	This new eature is optionnal if dmes is disable we continue as before

The following changes since commit e7509bfbabc46e15e66193efac64440b48e5e88f:

  Merge branch 'for-next/tftp' (2012-12-07 16:43:24 +0100)

are available in the git repository at:


  git://git.jcrosoft.org/barebox.git delivery/dmesg

for you to fetch changes up to 33c060981c15f453d88752eab8c9743c18a78247:

  dev_printf: switch to pr_info (2012-12-12 02:28:41 +0800)

----------------------------------------------------------------
Jean-Christophe PLAGNIOL-VILLARD (9):
      kfifo: introduce kfifo_dump_str to dump the fifo
      console: switch to kfifo_dump_str
      intoduce dmesg to print the barebox output ring buffer
      introduce printk support
      startup: switch to pr_xxx
      console: allow to specify ouput kfifo size via CONSOLE_KFIFO_OUTPUT_SIZE
      at91: clock switch to pr_info
      meminfo: switch to pr_xxx
      dev_printf: switch to pr_info

 arch/arm/mach-at91/clock.c      |    2 +-
 commands/Kconfig                |    6 ++++++
 common/Kconfig                  |    7 +++++++
 common/console.c                |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 common/meminfo.c                |    8 ++++----
 common/startup.c                |   10 +++++-----
 drivers/base/driver.c           |    6 +++---
 include/common.h                |   28 ++++++++++++++++++----------
 include/kfifo.h                 |    2 ++
 include/linux/barebox-wrapper.h |    2 --
 lib/kfifo.c                     |   21 +++++++++++++++++++++
 11 files changed, 142 insertions(+), 30 deletions(-)

Best Regards,
J.

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

[PATCH 1/9] kfifo: introduce kfifo_dump_str to dump the fifo

[Jean-Christophe PLAGNIOL-VILLARD]

This will allow to implement a dmesg mecanism in barebox

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 include/kfifo.h |    2 ++
 lib/kfifo.c     |   21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/include/kfifo.h b/include/kfifo.h
index 25880f4..9dbbe0d 100644
--- a/include/kfifo.h
+++ b/include/kfifo.h
@@ -74,5 +74,7 @@ static inline unsigned int kfifo_len(struct kfifo *fifo)
 void kfifo_putc(struct kfifo *fifo, unsigned char c);
 unsigned int kfifo_getc(struct kfifo *fifo, unsigned char *c);
 
+void kfifo_dump_str(struct kfifo *fifo, void (*dump)(unsigned char c));
+
 #endif
 
diff --git a/lib/kfifo.c b/lib/kfifo.c
index afd3894..7892aed 100644
--- a/lib/kfifo.c
+++ b/lib/kfifo.c
@@ -154,3 +154,24 @@ unsigned int kfifo_getc(struct kfifo *fifo, unsigned char *c)
 	return 0;
 }
 
+void kfifo_dump_str(struct kfifo *fifo, void (*dump)(unsigned char c))
+{
+	int i;
+	unsigned char *c;
+	unsigned int l;
+	unsigned int len;
+
+	len = fifo->in - fifo->out;
+
+	/* first get the data from fifo->out until the end of the buffer */
+	l = min(len, fifo->size - (fifo->out & (fifo->size - 1)));
+	c = fifo->buffer + (fifo->out & (fifo->size - 1));
+	for (i = 0; i < l; i++)
+		dump(c[i]);
+
+	/* then get the rest (if any) from the beginning of the buffer */
+	c = fifo->buffer;
+	l = len - l;
+	for (i = 0; i < l; i++)
+		dump(c[i]);
+}
-- 
1.7.10.4


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

[PATCH 2/9] console: switch to kfifo_dump_str

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 common/console.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/common/console.c b/common/console.c
index fdd5f42..96e1396 100644
--- a/common/console.c
+++ b/common/console.c
@@ -55,6 +55,11 @@ static struct kfifo __console_output_fifo;
 static struct kfifo *console_input_fifo = &__console_input_fifo;
 static struct kfifo *console_output_fifo = &__console_output_fifo;
 
+static void console_output_dump(unsigned char ch)
+{
+	console_putc(CONSOLE_STDOUT, ch);
+}
+
 static int console_std_set(struct device_d *dev, struct param_d *param,
 		const char *val)
 {
@@ -86,14 +91,12 @@ static int console_std_set(struct device_d *dev, struct param_d *param,
 	dev_param_set_generic(dev, param, active);
 
 	if (initialized < CONSOLE_INIT_FULL) {
-		char ch;
 		initialized = CONSOLE_INIT_FULL;
 		PUTS_LL("Switch to console [");
 		PUTS_LL(dev_name(dev));
 		PUTS_LL("]\n");
 		barebox_banner();
-		while (kfifo_getc(console_output_fifo, &ch) == 0)
-			console_putc(CONSOLE_STDOUT, ch);
+		kfifo_dump_str(console_output_fifo, console_output_dump);
 	}
 
 	return 0;
-- 
1.7.10.4


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

[PATCH 3/9] intoduce dmesg to print the barebox output ring buffer

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 commands/Kconfig |    6 ++++++
 common/console.c |   20 ++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/commands/Kconfig b/commands/Kconfig
index ac9b797..55a9840 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -115,6 +115,12 @@ config CMD_TIME
 	  checking for ctrl-c, so the time command can be used with commands
 	  which are interruptible with ctrl-c.
 
+config CMD_DMESG
+	bool "dmesg"
+	depends on CONSOLE_FULL
+	  help
+	  print the barebox output ring buffer
+
 config CMD_LINUX_EXEC
 	bool "linux exec"
 	depends on LINUX
diff --git a/common/console.c b/common/console.c
index 96e1396..2d2d20a 100644
--- a/common/console.c
+++ b/common/console.c
@@ -434,3 +434,23 @@ int ctrlc (void)
 }
 EXPORT_SYMBOL(ctrlc);
 #endif /* ARCH_HAS_CTRC */
+
+#include <command.h>
+#include <complete.h>
+
+static int do_dmesg(int argc, char *argv[])
+{
+	kfifo_dump_str(console_output_fifo, console_output_dump);
+
+	return 0;
+}
+
+static const __maybe_unused char cmd_dmesg_help[] =
+"print the barebox output ring buffer\n";
+
+BAREBOX_CMD_START(dmesg)
+	.cmd		= do_dmesg,
+	.usage		= "dmesg",
+	BAREBOX_CMD_HELP(cmd_dmesg_help)
+	BAREBOX_CMD_COMPLETE(empty_complete)
+BAREBOX_CMD_END
-- 
1.7.10.4


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

[PATCH 4/9] introduce printk support

[Jean-Christophe PLAGNIOL-VILLARD]

this will allow to fill the output buffer

and now have 2 output mode pr_xxx for drivers and printf for application

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 common/console.c                |   47 +++++++++++++++++++++++++++++++++++++++
 include/common.h                |   28 ++++++++++++++---------
 include/linux/barebox-wrapper.h |    2 --
 3 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/common/console.c b/common/console.c
index 2d2d20a..d8fe5b6 100644
--- a/common/console.c
+++ b/common/console.c
@@ -435,6 +435,7 @@ int ctrlc (void)
 EXPORT_SYMBOL(ctrlc);
 #endif /* ARCH_HAS_CTRC */
 
+#ifdef CONFIG_CMD_DMESG
 #include <command.h>
 #include <complete.h>
 
@@ -454,3 +455,49 @@ BAREBOX_CMD_START(dmesg)
 	BAREBOX_CMD_HELP(cmd_dmesg_help)
 	BAREBOX_CMD_COMPLETE(empty_complete)
 BAREBOX_CMD_END
+
+int vprintk (const char *fmt, va_list args)
+{
+	uint i;
+	char printbuffer[CFG_PBSIZE];
+	char *s = printbuffer;
+
+	/* For this to work, printbuffer must be larger than
+	 * anything we ever want to print.
+	 */
+	i = vsprintf (printbuffer, fmt, args);
+
+	/* Print the string */
+	puts (printbuffer);
+
+	if (initialized < CONSOLE_INIT_FULL)
+		return i;
+
+	while (*s) {
+		if (*s == '\n')
+			kfifo_putc(console_output_fifo, '\r');
+		kfifo_putc(console_output_fifo, *s);
+		s++;
+	}
+
+	return i;
+}
+EXPORT_SYMBOL(vprintk);
+
+int printk (const char *fmt, ...)
+{
+	va_list args;
+	uint i;
+
+	va_start (args, fmt);
+
+	i = vprintk(fmt, args);
+	/* For this to work, printbuffer must be larger than
+	 * anything we ever want to print.
+	 */
+	va_end (args);
+
+	return i;
+}
+EXPORT_SYMBOL(printk);
+#endif
diff --git a/include/common.h b/include/common.h
index 6256879..168bfd1 100644
--- a/include/common.h
+++ b/include/common.h
@@ -48,22 +48,30 @@
 #error "None of __LITTLE_ENDIAN and __BIG_ENDIAN are defined"
 #endif
 
-#define pr_info(fmt, arg...)	printf(fmt, ##arg)
-#define pr_notice(fmt, arg...)	printf(fmt, ##arg)
-#define pr_err(fmt, arg...)	printf(fmt, ##arg)
-#define pr_warning(fmt, arg...)	printf(fmt, ##arg)
-#define pr_crit(fmt, arg...)	printf(fmt, ##arg)
-#define pr_alert(fmt, arg...)	printf(fmt, ##arg)
-#define pr_emerg(fmt, arg...)	printf(fmt, ##arg)
+#ifdef CONFIG_CMD_DMESG
+int	printk(const char *fmt, ...) __attribute__ ((format(__printf__, 1, 2)));
+int	vprintk(const char *fmt, va_list args);
+#else
+#define printk			printf
+#define vprintk			vprintf
+#endif
+
+#define pr_info(fmt, arg...)	printk(fmt, ##arg)
+#define pr_notice(fmt, arg...)	printk(fmt, ##arg)
+#define pr_err(fmt, arg...)	printk(fmt, ##arg)
+#define pr_warning(fmt, arg...)	printk(fmt, ##arg)
+#define pr_crit(fmt, arg...)	printk(fmt, ##arg)
+#define pr_alert(fmt, arg...)	printk(fmt, ##arg)
+#define pr_emerg(fmt, arg...)	printk(fmt, ##arg)
 
 #ifdef DEBUG
-#define pr_debug(fmt, arg...)	printf(fmt, ##arg)
+#define pr_debug(fmt, arg...)	printk(fmt, ##arg)
+#define debug(fmt, arg...)	printf(fmt, ##arg)
 #else
 #define pr_debug(fmt, arg...)	do {} while(0)
+#define debug(fmt, arg...)	do {} while(0)
 #endif
 
-#define debug(fmt, arg...)	pr_debug(fmt, ##arg)
-
 #define BUG() do { \
 	printf("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
 	panic("BUG!"); \
diff --git a/include/linux/barebox-wrapper.h b/include/linux/barebox-wrapper.h
index 1d1f846..e3825f2 100644
--- a/include/linux/barebox-wrapper.h
+++ b/include/linux/barebox-wrapper.h
@@ -18,8 +18,6 @@
 #define KERN_INFO       ""   /* informational                        */
 #define KERN_DEBUG      ""   /* debug-level messages                 */
 
-#define printk			printf
-
 #define pr_warn			pr_warning
 
 #define __init
-- 
1.7.10.4


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

[PATCH 5/9] startup: switch to pr_xxx

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 common/startup.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/common/startup.c b/common/startup.c
index 14409a2..1a6352d 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -98,19 +98,19 @@ void start_barebox (void)
 
 	for (initcall = __barebox_initcalls_start;
 			initcall < __barebox_initcalls_end; initcall++) {
-		debug("initcall-> %pS\n", *initcall);
+		pr_debug("initcall-> %pS\n", *initcall);
 		result = (*initcall)();
 		if (result)
 			pr_err("initcall %pS failed: %s\n", *initcall,
 					strerror(-result));
 	}
 
-	debug("initcalls done\n");
+	pr_debug("initcalls done\n");
 
 #ifdef CONFIG_ENV_HANDLING
 	if (envfs_load(default_environment_path, "/env", 0)) {
 #ifdef CONFIG_DEFAULT_ENVIRONMENT
-		printf("no valid environment found on %s. "
+		pr_info("no valid environment found on %s. "
 			"Using default environment\n",
 			default_environment_path);
 		envfs_load("/dev/defaultenv", "/env", 0);
@@ -118,12 +118,12 @@ void start_barebox (void)
 	}
 #endif
 #ifdef CONFIG_COMMAND_SUPPORT
-	printf("running /env/bin/init...\n");
+	pr_info("running /env/bin/init...\n");
 
 	if (!stat("/env/bin/init", &s)) {
 		run_command("source /env/bin/init", 0);
 	} else {
-		printf("not found\n");
+		pr_err("not found\n");
 	}
 #endif
 	/* main_loop() can return to retry autoboot, if so just run it again. */
-- 
1.7.10.4


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

[PATCH 6/9] console: allow to specify ouput kfifo size via CONSOLE_KFIFO_OUTPUT_SIZE

[Jean-Christophe PLAGNIOL-VILLARD]

1024 by default
4096 if DEBUG_INFO

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 common/Kconfig   |    7 +++++++
 common/console.c |    4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 57119dc..6a0c345 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -477,6 +477,13 @@ config CONSOLE_FULL
 	  once the first console is registered. Recommended for most
 	  usecases.
 
+config CONSOLE_KFIFO_OUTPUT_SIZE
+	prompt "kfifo output size"
+	depends on CONSOLE_FULL
+	int
+	default 4086 if DEBUG_INFO
+	default 1024
+
 choice
 	prompt "Console activation strategy"
 	depends on CONSOLE_FULL
diff --git a/common/console.c b/common/console.c
index d8fe5b6..aeef0a2 100644
--- a/common/console.c
+++ b/common/console.c
@@ -48,7 +48,7 @@ static int initialized = 0;
 #define CONSOLE_BUFFER_SIZE	1024
 
 static char console_input_buffer[CONSOLE_BUFFER_SIZE];
-static char console_output_buffer[CONSOLE_BUFFER_SIZE];
+static char console_output_buffer[CONFIG_CONSOLE_KFIFO_OUTPUT_SIZE];
 
 static struct kfifo __console_input_fifo;
 static struct kfifo __console_output_fifo;
@@ -138,7 +138,7 @@ static void console_init_early(void)
 	kfifo_init(console_input_fifo, console_input_buffer,
 			CONSOLE_BUFFER_SIZE);
 	kfifo_init(console_output_fifo, console_output_buffer,
-			CONSOLE_BUFFER_SIZE);
+			CONFIG_CONSOLE_KFIFO_OUTPUT_SIZE);
 
 	initialized = CONSOLE_INITIALIZED_BUFFER;
 }
-- 
1.7.10.4


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

[PATCH 7/9] at91: clock switch to pr_info

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/clock.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-at91/clock.c b/arch/arm/mach-at91/clock.c
index e911270..23e4dd4 100644
--- a/arch/arm/mach-at91/clock.c
+++ b/arch/arm/mach-at91/clock.c
@@ -717,7 +717,7 @@ static int at91_clock_display(void)
 	if (pll_overclock)
 		pr_info("Clocks: PLLA overclocked, %ld MHz\n", plla.rate_hz / 1000000);
 
-	printf("Clocks: CPU %u MHz, master %u MHz, main %u.%03u MHz\n",
+	pr_info("Clocks: CPU %u MHz, master %u MHz, main %u.%03u MHz\n",
 		cpu_freq / 1000000, (unsigned) mck.rate_hz / 1000000,
 		(unsigned) main_clk.rate_hz / 1000000,
 		((unsigned) main_clk.rate_hz % 1000000) / 1000);
-- 
1.7.10.4


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

[PATCH 8/9] meminfo: switch to pr_xxx

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 common/meminfo.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/common/meminfo.c b/common/meminfo.c
index 06fce5a..989024f 100644
--- a/common/meminfo.c
+++ b/common/meminfo.c
@@ -9,12 +9,12 @@ static int display_meminfo(void)
 	ulong mend   = mem_malloc_end();
 	ulong msize  = mend - mstart + 1;
 
-	debug("barebox code: 0x%p -> 0x%p\n", _stext, _etext);
-	debug("bss segment:  0x%p -> 0x%p\n", __bss_start, __bss_stop);
-	printf("malloc space: 0x%08lx -> 0x%08lx (size %s)\n",
+	pr_debug("barebox code: 0x%p -> 0x%p\n", _stext, _etext);
+	pr_debug("bss segment:  0x%p -> 0x%p\n", __bss_start, __bss_stop);
+	pr_info("malloc space: 0x%08lx -> 0x%08lx (size %s)\n",
 		mstart, mend, size_human_readable(msize));
 #ifdef CONFIG_ARM
-	printf("stack space:  0x%08x -> 0x%08x (size %s)\n",
+	pr_info("stack space:  0x%08x -> 0x%08x (size %s)\n",
 		STACK_BASE, STACK_BASE + STACK_SIZE,
 		size_human_readable(STACK_SIZE));
 #endif
-- 
1.7.10.4


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

[PATCH 9/9] dev_printf: switch to pr_info

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/base/driver.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 5b3542b..4688a19 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -380,13 +380,13 @@ int dev_printf(const struct device_d *dev, const char *format, ...)
 	int ret = 0;
 
 	if (dev->driver && dev->driver->name)
-		ret += printf("%s ", dev->driver->name);
+		ret += pr_info("%s ", dev->driver->name);
 
-	ret += printf("%s: ", dev_name(dev));
+	ret += pr_info("%s: ", dev_name(dev));
 
 	va_start(args, format);
 
-	ret += vprintf(format, args);
+	ret += vprintk(format, args);
 
 	va_end(args);
 
-- 
1.7.10.4


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

Re: [PATCH 1/9] kfifo: introduce kfifo_dump_str to dump the fifo

[Sascha Hauer]

On Thu, Dec 13, 2012 at 05:40:02PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> This will allow to implement a dmesg mecanism in barebox
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  include/kfifo.h |    2 ++
>  lib/kfifo.c     |   21 +++++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/include/kfifo.h b/include/kfifo.h
> index 25880f4..9dbbe0d 100644
> --- a/include/kfifo.h
> +++ b/include/kfifo.h
> @@ -74,5 +74,7 @@ static inline unsigned int kfifo_len(struct kfifo *fifo)
>  void kfifo_putc(struct kfifo *fifo, unsigned char c);
>  unsigned int kfifo_getc(struct kfifo *fifo, unsigned char *c);
>  
> +void kfifo_dump_str(struct kfifo *fifo, void (*dump)(unsigned char c));
> +
>  #endif
>  
> diff --git a/lib/kfifo.c b/lib/kfifo.c
> index afd3894..7892aed 100644
> --- a/lib/kfifo.c
> +++ b/lib/kfifo.c
> @@ -154,3 +154,24 @@ unsigned int kfifo_getc(struct kfifo *fifo, unsigned char *c)
>  	return 0;
>  }
>  
> +void kfifo_dump_str(struct kfifo *fifo, void (*dump)(unsigned char c))
> +{
> +	int i;
> +	unsigned char *c;
> +	unsigned int l;
> +	unsigned int len;
> +
> +	len = fifo->in - fifo->out;
> +
> +	/* first get the data from fifo->out until the end of the buffer */
> +	l = min(len, fifo->size - (fifo->out & (fifo->size - 1)));
> +	c = fifo->buffer + (fifo->out & (fifo->size - 1));
> +	for (i = 0; i < l; i++)
> +		dump(c[i]);
> +
> +	/* then get the rest (if any) from the beginning of the buffer */
> +	c = fifo->buffer;
> +	l = len - l;
> +	for (i = 0; i < l; i++)
> +		dump(c[i]);
> +}

This function works in the innards of the fifo implementation and
replaces these two lines of code:

	while (kfifo_getc(console_output_fifo, &ch) == 0)
		console_putc(CONSOLE_STDOUT, ch);

You do not provide a reason why we should do this, so: no.

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

Re: [PATCH 4/9] introduce printk support

[Sascha Hauer]

On Thu, Dec 13, 2012 at 05:40:05PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> this will allow to fill the output buffer
> 
> and now have 2 output mode pr_xxx for drivers and printf for application
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  common/console.c                |   47 +++++++++++++++++++++++++++++++++++++++
>  include/common.h                |   28 ++++++++++++++---------
>  include/linux/barebox-wrapper.h |    2 --
>  3 files changed, 65 insertions(+), 12 deletions(-)
> 
> diff --git a/common/console.c b/common/console.c
> index 2d2d20a..d8fe5b6 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -435,6 +435,7 @@ int ctrlc (void)
>  EXPORT_SYMBOL(ctrlc);
>  #endif /* ARCH_HAS_CTRC */
>  
> +#ifdef CONFIG_CMD_DMESG
>  #include <command.h>
>  #include <complete.h>
>  
> @@ -454,3 +455,49 @@ BAREBOX_CMD_START(dmesg)
>  	BAREBOX_CMD_HELP(cmd_dmesg_help)
>  	BAREBOX_CMD_COMPLETE(empty_complete)
>  BAREBOX_CMD_END
> +
> +int vprintk (const char *fmt, va_list args)
> +{
> +	uint i;
> +	char printbuffer[CFG_PBSIZE];
> +	char *s = printbuffer;
> +
> +	/* For this to work, printbuffer must be larger than
> +	 * anything we ever want to print.
> +	 */
> +	i = vsprintf (printbuffer, fmt, args);
> +
> +	/* Print the string */
> +	puts (printbuffer);

So when the console is not initialized the characters end up in the
console_output_fifo which is emptied when the first console is
initialized. This means it's lost from the dmesg buffer afterwards.

> +
> +	if (initialized < CONSOLE_INIT_FULL)
> +		return i;
> +
> +	while (*s) {
> +		if (*s == '\n')
> +			kfifo_putc(console_output_fifo, '\r');
> +		kfifo_putc(console_output_fifo, *s);
> +		s++;
> +	}

You shouldn't use the same buffer for printk messages, use a different
one instead.

> +
> +	return i;
> +}
> +EXPORT_SYMBOL(vprintk);
> +
> +int printk (const char *fmt, ...)
> +{
> +	va_list args;
> +	uint i;
> +
> +	va_start (args, fmt);
> +
> +	i = vprintk(fmt, args);
> +	/* For this to work, printbuffer must be larger than
> +	 * anything we ever want to print.
> +	 */

This comment seems inappropriate here.

> +	va_end (args);
> +
> +	return i;
> +}
> +EXPORT_SYMBOL(printk);
> +#endif
> diff --git a/include/common.h b/include/common.h
> index 6256879..168bfd1 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -48,22 +48,30 @@
>  #error "None of __LITTLE_ENDIAN and __BIG_ENDIAN are defined"
>  #endif
>  
> -#define pr_info(fmt, arg...)	printf(fmt, ##arg)
> -#define pr_notice(fmt, arg...)	printf(fmt, ##arg)
> -#define pr_err(fmt, arg...)	printf(fmt, ##arg)
> -#define pr_warning(fmt, arg...)	printf(fmt, ##arg)
> -#define pr_crit(fmt, arg...)	printf(fmt, ##arg)
> -#define pr_alert(fmt, arg...)	printf(fmt, ##arg)
> -#define pr_emerg(fmt, arg...)	printf(fmt, ##arg)
> +#ifdef CONFIG_CMD_DMESG
> +int	printk(const char *fmt, ...) __attribute__ ((format(__printf__, 1, 2)));
> +int	vprintk(const char *fmt, va_list args);
> +#else
> +#define printk			printf
> +#define vprintk			vprintf
> +#endif
> +
> +#define pr_info(fmt, arg...)	printk(fmt, ##arg)
> +#define pr_notice(fmt, arg...)	printk(fmt, ##arg)
> +#define pr_err(fmt, arg...)	printk(fmt, ##arg)
> +#define pr_warning(fmt, arg...)	printk(fmt, ##arg)
> +#define pr_crit(fmt, arg...)	printk(fmt, ##arg)
> +#define pr_alert(fmt, arg...)	printk(fmt, ##arg)
> +#define pr_emerg(fmt, arg...)	printk(fmt, ##arg)
>  
>  #ifdef DEBUG
> -#define pr_debug(fmt, arg...)	printf(fmt, ##arg)
> +#define pr_debug(fmt, arg...)	printk(fmt, ##arg)
> +#define debug(fmt, arg...)	printf(fmt, ##arg)

'debug' is used in a huge amount of drivers, this would have to be fixed
when the two functions get a different meaning.

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

Re: [PATCH 0/9] introduction of dmesg support

[Sascha Hauer]

On Thu, Dec 13, 2012 at 05:37:58PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> HI,
> 
> 	we currently have 2 fifo at comsole level one for input one for output
> 	we fill the output fifo before any console is registered and then
> 	empty it
> 
> 	Now we introduce a new mecanism as in the kernel dmesg
> 
> 	we will always fill the output kfifo with drivers output
> 
> 	as in barebox we have 2 world
> 
> 	device/drivers and applicaiotn (commands)
> 
> 	so for now on drivers will have to use pr_xxx and dev_xxx and
> 	application will use printf
> 
> 	dmesg will print the kfifo at any time
> 
> 	we will extend this later with more control of what the drivers output
> 	on the console or just in the fifo
> 
> 	This new eature is optionnal if dmes is disable we continue as before
> 

Generally I like the idea. Overall I think it's nice to be able to
control the verbosity during runtime.

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

Re: [PATCH 0/9] introduction of dmesg support

[Jean-Christophe PLAGNIOL-VILLARD]

On 00:15 Sat 15 Dec     , Sascha Hauer wrote:
> On Thu, Dec 13, 2012 at 05:37:58PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > HI,
> > 
> > 	we currently have 2 fifo at comsole level one for input one for output
> > 	we fill the output fifo before any console is registered and then
> > 	empty it
> > 
> > 	Now we introduce a new mecanism as in the kernel dmesg
> > 
> > 	we will always fill the output kfifo with drivers output
> > 
> > 	as in barebox we have 2 world
> > 
> > 	device/drivers and applicaiotn (commands)
> > 
> > 	so for now on drivers will have to use pr_xxx and dev_xxx and
> > 	application will use printf
> > 
> > 	dmesg will print the kfifo at any time
> > 
> > 	we will extend this later with more control of what the drivers output
> > 	on the console or just in the fifo
> > 
> > 	This new eature is optionnal if dmes is disable we continue as before
> > 
> 
> Generally I like the idea. Overall I think it's nice to be able to
> control the verbosity during runtime.
This is the plan
but as a second step

Best Regards,
J.

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

Re: [PATCH 1/9] kfifo: introduce kfifo_dump_str to dump the fifo

[Jean-Christophe PLAGNIOL-VILLARD]

On 23:49 Fri 14 Dec     , Sascha Hauer wrote:
> On Thu, Dec 13, 2012 at 05:40:02PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > This will allow to implement a dmesg mecanism in barebox
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > ---
> >  include/kfifo.h |    2 ++
> >  lib/kfifo.c     |   21 +++++++++++++++++++++
> >  2 files changed, 23 insertions(+)
> > 
> > diff --git a/include/kfifo.h b/include/kfifo.h
> > index 25880f4..9dbbe0d 100644
> > --- a/include/kfifo.h
> > +++ b/include/kfifo.h
> > @@ -74,5 +74,7 @@ static inline unsigned int kfifo_len(struct kfifo *fifo)
> >  void kfifo_putc(struct kfifo *fifo, unsigned char c);
> >  unsigned int kfifo_getc(struct kfifo *fifo, unsigned char *c);
> >  
> > +void kfifo_dump_str(struct kfifo *fifo, void (*dump)(unsigned char c));
> > +
> >  #endif
> >  
> > diff --git a/lib/kfifo.c b/lib/kfifo.c
> > index afd3894..7892aed 100644
> > --- a/lib/kfifo.c
> > +++ b/lib/kfifo.c
> > @@ -154,3 +154,24 @@ unsigned int kfifo_getc(struct kfifo *fifo, unsigned char *c)
> >  	return 0;
> >  }
> >  
> > +void kfifo_dump_str(struct kfifo *fifo, void (*dump)(unsigned char c))
> > +{
> > +	int i;
> > +	unsigned char *c;
> > +	unsigned int l;
> > +	unsigned int len;
> > +
> > +	len = fifo->in - fifo->out;
> > +
> > +	/* first get the data from fifo->out until the end of the buffer */
> > +	l = min(len, fifo->size - (fifo->out & (fifo->size - 1)));
> > +	c = fifo->buffer + (fifo->out & (fifo->size - 1));
> > +	for (i = 0; i < l; i++)
> > +		dump(c[i]);
> > +
> > +	/* then get the rest (if any) from the beginning of the buffer */
> > +	c = fifo->buffer;
> > +	l = len - l;
> > +	for (i = 0; i < l; i++)
> > +		dump(c[i]);
> > +}
> 
> This function works in the innards of the fifo implementation and
> replaces these two lines of code:
> 
> 	while (kfifo_getc(console_output_fifo, &ch) == 0)
> 		console_putc(CONSOLE_STDOUT, ch);
> 
> You do not provide a reason why we should do this, so: no.
the current code empty the fifo


this new function just dump it

so you can recall it

Best Regards,
J.

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

Re: [PATCH 4/9] introduce printk support

[Jean-Christophe PLAGNIOL-VILLARD]

On 00:07 Sat 15 Dec     , Sascha Hauer wrote:
> On Thu, Dec 13, 2012 at 05:40:05PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > this will allow to fill the output buffer
> > 
> > and now have 2 output mode pr_xxx for drivers and printf for application
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > ---
> >  common/console.c                |   47 +++++++++++++++++++++++++++++++++++++++
> >  include/common.h                |   28 ++++++++++++++---------
> >  include/linux/barebox-wrapper.h |    2 --
> >  3 files changed, 65 insertions(+), 12 deletions(-)
> > 
> > diff --git a/common/console.c b/common/console.c
> > index 2d2d20a..d8fe5b6 100644
> > --- a/common/console.c
> > +++ b/common/console.c
> > @@ -435,6 +435,7 @@ int ctrlc (void)
> >  EXPORT_SYMBOL(ctrlc);
> >  #endif /* ARCH_HAS_CTRC */
> >  
> > +#ifdef CONFIG_CMD_DMESG
> >  #include <command.h>
> >  #include <complete.h>
> >  
> > @@ -454,3 +455,49 @@ BAREBOX_CMD_START(dmesg)
> >  	BAREBOX_CMD_HELP(cmd_dmesg_help)
> >  	BAREBOX_CMD_COMPLETE(empty_complete)
> >  BAREBOX_CMD_END
> > +
> > +int vprintk (const char *fmt, va_list args)
> > +{
> > +	uint i;
> > +	char printbuffer[CFG_PBSIZE];
> > +	char *s = printbuffer;
> > +
> > +	/* For this to work, printbuffer must be larger than
> > +	 * anything we ever want to print.
> > +	 */
> > +	i = vsprintf (printbuffer, fmt, args);
> > +
> > +	/* Print the string */
> > +	puts (printbuffer);
> 
> So when the console is not initialized the characters end up in the
> console_output_fifo which is emptied when the first console is
> initialized. This means it's lost from the dmesg buffer afterwards.

that's why I change this  to just dump instead

Best Regards,
J.
> 
> > +
> > +	if (initialized < CONSOLE_INIT_FULL)
> > +		return i;
> > +
> > +	while (*s) {
> > +		if (*s == '\n')
> > +			kfifo_putc(console_output_fifo, '\r');
> > +		kfifo_putc(console_output_fifo, *s);
> > +		s++;
> > +	}
> 
> You shouldn't use the same buffer for printk messages, use a different
> one instead.
> 
> > +
> > +	return i;
> > +}
> > +EXPORT_SYMBOL(vprintk);
> > +
> > +int printk (const char *fmt, ...)
> > +{
> > +	va_list args;
> > +	uint i;
> > +
> > +	va_start (args, fmt);
> > +
> > +	i = vprintk(fmt, args);
> > +	/* For this to work, printbuffer must be larger than
> > +	 * anything we ever want to print.
> > +	 */
> 
> This comment seems inappropriate here.
> 
> > +	va_end (args);
> > +
> > +	return i;
> > +}
> > +EXPORT_SYMBOL(printk);
> > +#endif
> > diff --git a/include/common.h b/include/common.h
> > index 6256879..168bfd1 100644
> > --- a/include/common.h
> > +++ b/include/common.h
> > @@ -48,22 +48,30 @@
> >  #error "None of __LITTLE_ENDIAN and __BIG_ENDIAN are defined"
> >  #endif
> >  
> > -#define pr_info(fmt, arg...)	printf(fmt, ##arg)
> > -#define pr_notice(fmt, arg...)	printf(fmt, ##arg)
> > -#define pr_err(fmt, arg...)	printf(fmt, ##arg)
> > -#define pr_warning(fmt, arg...)	printf(fmt, ##arg)
> > -#define pr_crit(fmt, arg...)	printf(fmt, ##arg)
> > -#define pr_alert(fmt, arg...)	printf(fmt, ##arg)
> > -#define pr_emerg(fmt, arg...)	printf(fmt, ##arg)
> > +#ifdef CONFIG_CMD_DMESG
> > +int	printk(const char *fmt, ...) __attribute__ ((format(__printf__, 1, 2)));
> > +int	vprintk(const char *fmt, va_list args);
> > +#else
> > +#define printk			printf
> > +#define vprintk			vprintf
> > +#endif
> > +
> > +#define pr_info(fmt, arg...)	printk(fmt, ##arg)
> > +#define pr_notice(fmt, arg...)	printk(fmt, ##arg)
> > +#define pr_err(fmt, arg...)	printk(fmt, ##arg)
> > +#define pr_warning(fmt, arg...)	printk(fmt, ##arg)
> > +#define pr_crit(fmt, arg...)	printk(fmt, ##arg)
> > +#define pr_alert(fmt, arg...)	printk(fmt, ##arg)
> > +#define pr_emerg(fmt, arg...)	printk(fmt, ##arg)
> >  
> >  #ifdef DEBUG
> > -#define pr_debug(fmt, arg...)	printf(fmt, ##arg)
> > +#define pr_debug(fmt, arg...)	printk(fmt, ##arg)
> > +#define debug(fmt, arg...)	printf(fmt, ##arg)
> 
> 'debug' is used in a huge amount of drivers, this would have to be fixed
> when the two functions get a different meaning.
> 
> 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