| Message ID | 1449268061-805-5-git-send-email-sjoerd.simons@collabora.co.uk |
|---|---|
| State | New |
| Headers | show |
On Mon, 2015-12-07 at 17:39 -0700, Simon Glass wrote: > > diff --git a/lib/Makefile b/lib/Makefile > > index 1f1ff6f..ae84833 100644 > > --- a/lib/Makefile > > +++ b/lib/Makefile > > @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o > > ifdef CONFIG_SPL_BUILD > > # SPL U-Boot may use full-printf, tiny-printf or none at all > > ifdef CONFIG_USE_TINY_PRINTF > > -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o > > +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o > > else > > -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o > > +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o > > endif > > else > > # Main U-Boot always uses the full printf support > > -obj-y += vsprintf.o > > +obj-y += vsprintf.o panic.o > > endif > > Why not just add this outside all the ifdef stuff: > > obj-y += panic.o Just keeping the old behaviour, that code was not build for SPL builds without serial support before. Do you see a benefit of just always building it ? > > subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2 > > diff --git a/lib/panic.c b/lib/panic.c > > new file mode 100644 > > index 0000000..e2b8b74 > > --- /dev/null > > +++ b/lib/panic.c > > @@ -0,0 +1,45 @@ > > +/* > > + * linux/lib/vsprintf.c > > nit: can you please drop this line or fix it? Sure it's pointless anyway > > + * > > + * Copyright (C) 1991, 1992 Linus Torvalds > > + */ > > + > > +/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */ > > +/* > > + * Wirzenius wrote this portably, Torvalds fucked it up :-) > > + */ > > Did any of this code actually come from Linux? If not perhaps invent > your own copyright? I actually went back and checked, vsprintf.c was imported in de180e6daa529dc78668c99bdf17a9cdd440782d which is a helpful commit with the name "Initial revisions". Most of the code in vsprintf.c is likely to just come from from linux afaik (see lib/vsprintf.c in the linux source) especial in the initial linux git repository. Unfortunate unless you actually want to go trawling back through pre-git linux versions to work out how similar the were at the branching point. The panic functions don't appear in git versions of linux, but may or may not be there in pre-git versions. In this case I just took the simple/conservative path and copied the copyright header (assuming correctness which seems reasonable enough) when splitting things up. I'd be fine with adjusting the copyright header _if_ there was more (easily) available historical data about the authors. Given that doesn't seem to be the case, I would prefer to keep the copyright as-is unless someone wants to take the time to do some code archaeology :)
On Sun, 2015-12-13 at 20:45 -0700, Simon Glass wrote: > On 8 December 2015 at 12:38, Simon Glass <sjg@chromium.org> wrote: > > Hi Scott, > > > > On 8 December 2015 at 12:36, Scott Wood <scottwood@freescale.com> > > wrote: > > > On Tue, 2015-12-08 at 12:34 -0700, Simon Glass wrote: > > > > Hi Sjoerd, > > > > > > OK, so how about this: > > > > > > ifdef CONFIG_SPL_BUILD > > > > # SPL U-Boot may use full-printf, tiny-printf or none at all > > > > obj-$(CONFIG_SPL_SERIAL_SUPPORT) += panic.o strto.o > > > > > > ifdef CONFIG_USE_TINY_PRINTF > > obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o > > > > else > > obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o > > > > endif > > > > else > > > > # Main U-Boot always uses the full printf support > > > > obj-y += vsprintf.o panic.o strto.o > > > > endif > > It's just a nit so I'm going to leave it as is for now. > > Applied to u-boot-rockchip, thanks! Heh, i just did an update series last night to send this morning :) lovely timing. I'll drop you some patches for these nits later this week while i still remember them :) (I agree your suggestion here is nicer).
diff --git a/lib/Makefile b/lib/Makefile index 1f1ff6f..ae84833 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o +obj-y += vsprintf.o panic.o endif subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2 diff --git a/lib/panic.c b/lib/panic.c new file mode 100644 index 0000000..e2b8b74 --- /dev/null +++ b/lib/panic.c @@ -0,0 +1,45 @@ +/* + * linux/lib/vsprintf.c + * + * Copyright (C) 1991, 1992 Linus Torvalds + */ + +/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */ +/* + * Wirzenius wrote this portably, Torvalds fucked it up :-) + */ + +#include <common.h> +#if !defined(CONFIG_PANIC_HANG) +#include <command.h> +#endif + +static void panic_finish(void) __attribute__ ((noreturn)); + +static void panic_finish(void) +{ + putc('\n'); +#if defined(CONFIG_PANIC_HANG) + hang(); +#else + udelay(100000); /* allow messages to go out */ + do_reset(NULL, 0, 0, NULL); +#endif + while (1) + ; +} + +void panic_str(const char *str) +{ + puts(str); + panic_finish(); +} + +void panic(const char *fmt, ...) +{ + va_list args; + va_start(args, fmt); + vprintf(fmt, args); + va_end(args); + panic_finish(); +} diff --git a/lib/vsprintf.c b/lib/vsprintf.c index dd8380b..bf5fd01 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -897,35 +897,6 @@ int vprintf(const char *fmt, va_list args) return i; } -static void panic_finish(void) __attribute__ ((noreturn)); - -static void panic_finish(void) -{ - putc('\n'); -#if defined(CONFIG_PANIC_HANG) - hang(); -#else - udelay(100000); /* allow messages to go out */ - do_reset(NULL, 0, 0, NULL); -#endif - while (1) - ; -} - -void panic_str(const char *str) -{ - puts(str); - panic_finish(); -} - -void panic(const char *fmt, ...) -{ - va_list args; - va_start(args, fmt); - vprintf(fmt, args); - va_end(args); - panic_finish(); -} void __assert_fail(const char *assertion, const char *file, unsigned line, const char *function)
To allow panic and panic_str to still be used when using tiny-printf, split them out into their own file which gets build regardless of what printf implementation is used. Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> --- lib/Makefile | 6 +++--- lib/panic.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ lib/vsprintf.c | 29 ----------------------------- 3 files changed, 48 insertions(+), 32 deletions(-) create mode 100644 lib/panic.c -- 2.6.2 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot