Re: [PATCH 4/9] introduce printk support

[Sascha Hauer <s.hauer@pengutronix.de>]

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