* [PATCH] Add an option to set a board specific banner
[not found] <20101230034811.GS19266@game.jcrosoft.org>
@ 2010-12-30 12:49 ` Franck JULLIEN
2010-12-31 7:24 ` Jean-Christophe PLAGNIOL-VILLARD
2011-01-03 11:41 ` Sascha Hauer
0 siblings, 2 replies; 8+ messages in thread
From: Franck JULLIEN @ 2010-12-30 12:49 UTC (permalink / raw)
To: barebox
Allow a board specific fancy banner
---
I removed the default fancy banner and do it like Jean-Christophe suggested it.
common/Kconfig | 6 ++++++
common/console.c | 7 ++++++-
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig
index 617f640..d32c1ce 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -248,6 +248,12 @@ config HUSH_FANCY_PROMPT
Allow to set PS1 from the command line. PS1 can have several escaped commands
like \h for CONFIG_BOARDINFO or \w for the current working directory.
+config BOARD_BANNER
+ bool
+ prompt "allow a board specific fancy banner"
+ help
+ Allow to define a custom board banner (you can define CONFIG_BOARD_BANNER_TEXT in your config.h)
+
config HUSH_GETOPT
bool
depends on SHELL_HUSH
diff --git a/common/console.c b/common/console.c
index 82786f2..7caef11 100644
--- a/common/console.c
+++ b/common/console.c
@@ -46,7 +46,12 @@ EXPORT_SYMBOL(console_list);
static void display_banner (void)
{
printf (RELOC("\n\n%s\n\n"), RELOC_VAR(version_string));
- printf(RELOC("Board: " CONFIG_BOARDINFO "\n"));
+
+#ifndef CONFIG_BOARD_BANNER
+#undef CONFIG_BOARD_BANNER_TEXT
+#define CONFIG_BOARD_BANNER_TEXT "Board: " CONFIG_BOARDINFO "\n"
+#endif
+ printf(RELOC(CONFIG_BOARD_BANNER_TEXT));
}
static int __early_initdata initialized = 0;
--
1.7.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add an option to set a board specific banner
2010-12-30 12:49 ` [PATCH] Add an option to set a board specific banner Franck JULLIEN
@ 2010-12-31 7:24 ` Jean-Christophe PLAGNIOL-VILLARD
2011-01-03 11:41 ` Sascha Hauer
1 sibling, 0 replies; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2010-12-31 7:24 UTC (permalink / raw)
To: Franck JULLIEN; +Cc: barebox
On 13:49 Thu 30 Dec , Franck JULLIEN wrote:
> Allow a board specific fancy banner
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Best Regards,
J.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add an option to set a board specific banner
2010-12-30 12:49 ` [PATCH] Add an option to set a board specific banner Franck JULLIEN
2010-12-31 7:24 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2011-01-03 11:41 ` Sascha Hauer
2011-01-03 12:45 ` Franck JULLIEN
1 sibling, 1 reply; 8+ messages in thread
From: Sascha Hauer @ 2011-01-03 11:41 UTC (permalink / raw)
To: Franck JULLIEN; +Cc: barebox
Hi Franck,
On Thu, Dec 30, 2010 at 01:49:34PM +0100, Franck JULLIEN wrote:
> Allow a board specific fancy banner
>
> ---
>
> I removed the default fancy banner and do it like Jean-Christophe suggested it.
>
> common/Kconfig | 6 ++++++
> common/console.c | 7 ++++++-
> 2 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 617f640..d32c1ce 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -248,6 +248,12 @@ config HUSH_FANCY_PROMPT
> Allow to set PS1 from the command line. PS1 can have several escaped commands
> like \h for CONFIG_BOARDINFO or \w for the current working directory.
>
> +config BOARD_BANNER
> + bool
> + prompt "allow a board specific fancy banner"
> + help
> + Allow to define a custom board banner (you can define CONFIG_BOARD_BANNER_TEXT in your config.h)
> +
> config HUSH_GETOPT
> bool
> depends on SHELL_HUSH
> diff --git a/common/console.c b/common/console.c
> index 82786f2..7caef11 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -46,7 +46,12 @@ EXPORT_SYMBOL(console_list);
> static void display_banner (void)
> {
> printf (RELOC("\n\n%s\n\n"), RELOC_VAR(version_string));
> - printf(RELOC("Board: " CONFIG_BOARDINFO "\n"));
> +
> +#ifndef CONFIG_BOARD_BANNER
> +#undef CONFIG_BOARD_BANNER_TEXT
> +#define CONFIG_BOARD_BANNER_TEXT "Board: " CONFIG_BOARDINFO "\n"
> +#endif
> + printf(RELOC(CONFIG_BOARD_BANNER_TEXT));
> }
I do not really understand this patch. CONFIG_BOARDINFO is board
specific already, why would we want to add another option?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add an option to set a board specific banner
2011-01-03 11:41 ` Sascha Hauer
@ 2011-01-03 12:45 ` Franck JULLIEN
2011-01-03 13:50 ` Sascha Hauer
0 siblings, 1 reply; 8+ messages in thread
From: Franck JULLIEN @ 2011-01-03 12:45 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
[-- Attachment #1.1: Type: text/plain, Size: 2934 bytes --]
2011/1/3 Sascha Hauer <s.hauer@pengutronix.de>
> Hi Franck,
>
> On Thu, Dec 30, 2010 at 01:49:34PM +0100, Franck JULLIEN wrote:
> > Allow a board specific fancy banner
> >
> > ---
> >
> > I removed the default fancy banner and do it like Jean-Christophe
> suggested it.
> >
> > common/Kconfig | 6 ++++++
> > common/console.c | 7 ++++++-
> > 2 files changed, 12 insertions(+), 1 deletions(-)
> >
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 617f640..d32c1ce 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -248,6 +248,12 @@ config HUSH_FANCY_PROMPT
> > Allow to set PS1 from the command line. PS1 can have several
> escaped commands
> > like \h for CONFIG_BOARDINFO or \w for the current working
> directory.
> >
> > +config BOARD_BANNER
> > + bool
> > + prompt "allow a board specific fancy banner"
> > + help
> > + Allow to define a custom board banner (you can define
> CONFIG_BOARD_BANNER_TEXT in your config.h)
> > +
> > config HUSH_GETOPT
> > bool
> > depends on SHELL_HUSH
> > diff --git a/common/console.c b/common/console.c
> > index 82786f2..7caef11 100644
> > --- a/common/console.c
> > +++ b/common/console.c
> > @@ -46,7 +46,12 @@ EXPORT_SYMBOL(console_list);
> > static void display_banner (void)
> > {
> > printf (RELOC("\n\n%s\n\n"), RELOC_VAR(version_string));
> > - printf(RELOC("Board: " CONFIG_BOARDINFO "\n"));
> > +
> > +#ifndef CONFIG_BOARD_BANNER
> > +#undef CONFIG_BOARD_BANNER_TEXT
> > +#define CONFIG_BOARD_BANNER_TEXT "Board: " CONFIG_BOARDINFO "\n"
> > +#endif
> > + printf(RELOC(CONFIG_BOARD_BANNER_TEXT));
> > }
>
> I do not really understand this patch. CONFIG_BOARDINFO is board
> specific already, why would we want to add another option?
>
> 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 |
>
Hello,
I know CONFIG_BOARDINFO is board specific. However, I think it is more
conveniant
to have the possibility to define a banner text outside the Kconfig. For
example, I use
this in my config.h:
#define CONFIG_BOARD_BANNER_TEXT "\e[1;32m\
***********************************************************************\n\
* Communication board bootloader (ODSFT0152) *\n\
************************************************************************\
\e[0m\n\n"
So may be we could define this kind of banner in the Kconfig.....
Let me know if you don't like this. This patch was kind of a test for me and
we could
forget about it.
Although this patch was very small, we had a lot of discussion around it and
I don't
want to imagine what it is going to be when I submit the nios2 port :)
Best regards,
Franck.
[-- Attachment #1.2: Type: text/html, Size: 3888 bytes --]
[-- Attachment #2: Type: text/plain, Size: 149 bytes --]
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add an option to set a board specific banner
2011-01-03 12:45 ` Franck JULLIEN
@ 2011-01-03 13:50 ` Sascha Hauer
2011-01-03 19:42 ` Franck JULLIEN
2011-01-03 23:53 ` Peter Korsgaard
0 siblings, 2 replies; 8+ messages in thread
From: Sascha Hauer @ 2011-01-03 13:50 UTC (permalink / raw)
To: Franck JULLIEN; +Cc: barebox
On Mon, Jan 03, 2011 at 01:45:18PM +0100, Franck JULLIEN wrote:
> 2011/1/3 Sascha Hauer <s.hauer@pengutronix.de>
>
>
> I know CONFIG_BOARDINFO is board specific. However, I think it is more
> conveniant
> to have the possibility to define a banner text outside the Kconfig. For
> example, I use
> this in my config.h:
>
> #define CONFIG_BOARD_BANNER_TEXT "\e[1;32m\
> ***********************************************************************\n\
> * Communication board bootloader (ODSFT0152) *\n\
> ************************************************************************\
> \e[0m\n\n"
>
> So may be we could define this kind of banner in the Kconfig.....
Ok, I can see your problem. I think we should rather define a board
specific function to display a banner than a string, so something like:
config BOARD_BANNER
bool
#ifdef CONFIG_BOARD_BANNER
display_board_banner();
#else
printf(RELOC("Board: " CONFIG_BOARDINFO "\n"));
#endif
This way we can also show runtime dependent information in the banner
(like baseboard or whatever).
>
> Let me know if you don't like this. This patch was kind of a test for me and
> we could
> forget about it.
>
> Although this patch was very small, we had a lot of discussion around it and
> I don't want to imagine what it is going to be when I submit the nios2 port :)
Don't worry, the amount of comments you receive usually is not
proportional to the patch size you send ;)
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add an option to set a board specific banner
2011-01-03 13:50 ` Sascha Hauer
@ 2011-01-03 19:42 ` Franck JULLIEN
2011-01-03 23:53 ` Peter Korsgaard
1 sibling, 0 replies; 8+ messages in thread
From: Franck JULLIEN @ 2011-01-03 19:42 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
[-- Attachment #1.1: Type: text/plain, Size: 2470 bytes --]
2011/1/3 Sascha Hauer <s.hauer@pengutronix.de>
> On Mon, Jan 03, 2011 at 01:45:18PM +0100, Franck JULLIEN wrote:
> > 2011/1/3 Sascha Hauer <s.hauer@pengutronix.de>
> >
> >
> > I know CONFIG_BOARDINFO is board specific. However, I think it is more
> > conveniant
> > to have the possibility to define a banner text outside the Kconfig. For
> > example, I use
> > this in my config.h:
> >
> > #define CONFIG_BOARD_BANNER_TEXT "\e[1;32m\
> >
> ***********************************************************************\n\
> > * Communication board bootloader (ODSFT0152) *\n\
> > ************************************************************************\
> > \e[0m\n\n"
> >
> > So may be we could define this kind of banner in the Kconfig.....
>
> Ok, I can see your problem. I think we should rather define a board
> specific function to display a banner than a string, so something like:
>
> config BOARD_BANNER
> bool
>
> #ifdef CONFIG_BOARD_BANNER
> display_board_banner();
> #else
> printf(RELOC("Board: " CONFIG_BOARDINFO "\n"));
> #endif
>
> This way we can also show runtime dependent information in the banner
> (like baseboard or whatever).
>
Sounds great, I'll do this. Is there anyone responsible for the list in
first place ?
Because if I do this in one way but someone would prefer do it another way
and
finally the git boss wants it a third way.......
I'll try to post the new patch in response to this thread
using the right git command...By the way, I there anything more user
friendly
than the git command line to send email in the right format ?
I told you this is a practice/test patch so I have a lot of
questions/comments !
>
> >
> > Let me know if you don't like this. This patch was kind of a test for me
> and
> > we could
> > forget about it.
> >
> > Although this patch was very small, we had a lot of discussion around it
> and
> > I don't want to imagine what it is going to be when I submit the nios2
> port :)
>
> Don't worry, the amount of comments you receive usually is not
> proportional to the patch size you send ;)
>
> Sascha
>
Ok, because I was worry :)
>
> --
> 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 |
>
Best regards,
FJ.
[-- Attachment #1.2: Type: text/html, Size: 3934 bytes --]
[-- Attachment #2: Type: text/plain, Size: 149 bytes --]
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add an option to set a board specific banner
2011-01-03 13:50 ` Sascha Hauer
2011-01-03 19:42 ` Franck JULLIEN
@ 2011-01-03 23:53 ` Peter Korsgaard
2011-01-04 9:00 ` Sascha Hauer
1 sibling, 1 reply; 8+ messages in thread
From: Peter Korsgaard @ 2011-01-03 23:53 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
>>>>> "Sascha" == Sascha Hauer <s.hauer@pengutronix.de> writes:
Sascha> Ok, I can see your problem. I think we should rather define a
Sascha> board specific function to display a banner than a string, so
Sascha> something like:
Sascha> config BOARD_BANNER
Sascha> bool
Sascha> #ifdef CONFIG_BOARD_BANNER
Sascha> display_board_banner();
Sascha> #else
Sascha> printf(RELOC("Board: " CONFIG_BOARDINFO "\n"));
Sascha> #endif
Even better, stick:
#ifdef CONFIG_BOARD_BANNER
extern void display_board_banner(void);
#else
static inline void display_board_banner(void)
{
printf(RELOC("Board: " CONFIG_BOARDINFO "\n"));
}
#endif
And get rid of the #ifdef in the C file.
--
Bye, Peter Korsgaard
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add an option to set a board specific banner
2011-01-03 23:53 ` Peter Korsgaard
@ 2011-01-04 9:00 ` Sascha Hauer
0 siblings, 0 replies; 8+ messages in thread
From: Sascha Hauer @ 2011-01-04 9:00 UTC (permalink / raw)
To: Peter Korsgaard; +Cc: barebox
On Tue, Jan 04, 2011 at 12:53:43AM +0100, Peter Korsgaard wrote:
> >>>>> "Sascha" == Sascha Hauer <s.hauer@pengutronix.de> writes:
>
> Sascha> Ok, I can see your problem. I think we should rather define a
> Sascha> board specific function to display a banner than a string, so
> Sascha> something like:
>
> Sascha> config BOARD_BANNER
> Sascha> bool
>
> Sascha> #ifdef CONFIG_BOARD_BANNER
> Sascha> display_board_banner();
> Sascha> #else
> Sascha> printf(RELOC("Board: " CONFIG_BOARDINFO "\n"));
> Sascha> #endif
>
> Even better, stick:
>
> #ifdef CONFIG_BOARD_BANNER
> extern void display_board_banner(void);
> #else
> static inline void display_board_banner(void)
> {
> printf(RELOC("Board: " CONFIG_BOARDINFO "\n"));
> }
> #endif
>
The printing of version_string is missing. We could add it to
display_board_banner, but I really would like to keep this in a
generic place to make sure barebox is recognizable by both users
and automatic scripts. Or does anybody wants to fully customize
the initial output of barebox?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-01-04 9:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20101230034811.GS19266@game.jcrosoft.org>
2010-12-30 12:49 ` [PATCH] Add an option to set a board specific banner Franck JULLIEN
2010-12-31 7:24 ` Jean-Christophe PLAGNIOL-VILLARD
2011-01-03 11:41 ` Sascha Hauer
2011-01-03 12:45 ` Franck JULLIEN
2011-01-03 13:50 ` Sascha Hauer
2011-01-03 19:42 ` Franck JULLIEN
2011-01-03 23:53 ` Peter Korsgaard
2011-01-04 9:00 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox