[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Marek Vasut]

This function converts static number to string in preprocessor.
This is useful as it allows higher usage of puts() in favour of printf()

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Mike Frysinger <vapier@gentoo.org>
---
 include/common.h |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/common.h b/include/common.h
index d1dd65a..451917d 100644
--- a/include/common.h
+++ b/include/common.h
@@ -268,6 +268,13 @@ typedef void (interrupt_handler_t)(void *);
 	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
 	(type *)( (char *)__mptr - offsetof(type,member) );})
 
+/**
+ * __stringify - preprocessor magic to return string from number
+ * @x:           constant number
+ */
+#define __stringify_1(x...)     #x
+#define __stringify(x...)       __stringify_1(x)
+
 /*
  * Function Prototypes
  */
-- 
1.7.10.4

[U-Boot] [RFC] [PATCH 2/2] crazy: Sort u_boot_cmd at runtime

[Marek Vasut]

This shall eliminate the need for bubblesorting of commands at runtime.
Every command definition structure is now put into it's own subsection
of section .u_boot_cmd, that is .u_boot_cmd.<name> . These are then put
into .u_boot_cmd by linker and lastly, linker uses SORT() over these
subsections to make proper order on them. This shall eliminate some
runtime overhead.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Mike Frysinger <vapier@gentoo.org>
---
 arch/arm/cpu/u-boot.lds |    2 +-
 common/cmd_help.c       |    2 +-
 include/command.h       |    9 ++++++---
 3 files changed, 8 insertions(+), 5 deletions(-)

********
* NOTE * THIS PATCH IS CRAZY
********

Further notes:
 - This is only compile-tested with gcc-4.7 (debian 4.7.1-5, binutils 2.22)
 - This patch affects only arm926t, obviously to make it proper, every
   linkerscript would have to be adjusted
 - I'm not sure at all the macro logic is correct, please check
 - Can this crash on *BSD or with older linker/cpp?
 - Please don't rip me limb to limb ;-)

diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index e49ca0c..c39193b 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -50,7 +50,7 @@ SECTIONS
 
 	. = .;
 	__u_boot_cmd_start = .;
-	.u_boot_cmd : { *(.u_boot_cmd) }
+	.u_boot_cmd : { *(SORT(.u_boot_cmd.*)) }
 	__u_boot_cmd_end = .;
 
 	. = ALIGN(4);
diff --git a/common/cmd_help.c b/common/cmd_help.c
index 8c8178e..5d778f5 100644
--- a/common/cmd_help.c
+++ b/common/cmd_help.c
@@ -41,7 +41,7 @@ U_BOOT_CMD(
 );
 
 /* This does not use the U_BOOT_CMD macro as ? can't be used in symbol names */
-cmd_tbl_t __u_boot_cmd_question_mark Struct_Section = {
+cmd_tbl_t __u_boot_cmd_question_mark Struct_Section(?) = {
 	"?",	CONFIG_SYS_MAXARGS,	1,	do_help,
 	"alias for 'help'",
 #ifdef  CONFIG_SYS_LONGHELP
diff --git a/include/command.h b/include/command.h
index 6e1bdc2..42b4c6a 100644
--- a/include/command.h
+++ b/include/command.h
@@ -149,8 +149,11 @@ int cmd_process(int flag, int argc, char * const argv[],
 #define CMD_FLAG_REPEAT		0x0001	/* repeat last command		*/
 #define CMD_FLAG_BOOTD		0x0002	/* command is from bootd	*/
 
-#define Struct_Section  __attribute__((unused, section(".u_boot_cmd"), \
-		aligned(4)))
+#define __sectstr(__cmd,__name)	.__cmd.__name
+#define sectstr(type, __name)		__stringify(__sectstr(type, __name))
+
+#define Struct_Section(__name)	\
+	__attribute__((unused, section(sectstr(u_boot_cmd, __name)), aligned(4)))
 
 #ifdef CONFIG_AUTO_COMPLETE
 # define _CMD_COMPLETE(x) x,
@@ -170,7 +173,7 @@ int cmd_process(int flag, int argc, char * const argv[],
 	U_BOOT_CMD_MKENT_COMPLETE(name,maxargs,rep,cmd,usage,help,NULL)
 
 #define U_BOOT_CMD_COMPLETE(name,maxargs,rep,cmd,usage,help,comp) \
-	cmd_tbl_t __u_boot_cmd_##name Struct_Section = \
+	cmd_tbl_t __u_boot_cmd_##name Struct_Section(name) = \
 		U_BOOT_CMD_MKENT_COMPLETE(name,maxargs,rep,cmd,usage,help,comp)
 
 #define U_BOOT_CMD(name,maxargs,rep,cmd,usage,help) \
-- 
1.7.10.4

[U-Boot] [RFC] [PATCH 2/2] crazy: Sort u_boot_cmd at runtime

[Wolfgang Denk]

Dear Marek Vasut,

In message <1343483279-11572-2-git-send-email-marex@denx.de> you wrote:
> This shall eliminate the need for bubblesorting of commands at runtime.
> Every command definition structure is now put into it's own subsection
> of section .u_boot_cmd, that is .u_boot_cmd.<name> . These are then put
> into .u_boot_cmd by linker and lastly, linker uses SORT() over these
> subsections to make proper order on them. This shall eliminate some
> runtime overhead.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Mike Frysinger <vapier@gentoo.org>
> ---
>  arch/arm/cpu/u-boot.lds |    2 +-
>  common/cmd_help.c       |    2 +-
>  include/command.h       |    9 ++++++---
>  3 files changed, 8 insertions(+), 5 deletions(-)

Seems incomplete in several aspects:

1) what about all the non-ARM architecures and the board specific
   linker scripts?

2) what about removing the sort code?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Men of peace usually are [brave].
	-- Spock, "The Savage Curtain", stardate 5906.5

[U-Boot] [RFC] [PATCH 2/2] crazy: Sort u_boot_cmd at runtime

[Marek Vasut]

Dear Wolfgang Denk,

> Dear Marek Vasut,
> 
> In message <1343483279-11572-2-git-send-email-marex@denx.de> you wrote:
> > This shall eliminate the need for bubblesorting of commands at runtime.
> > Every command definition structure is now put into it's own subsection
> > of section .u_boot_cmd, that is .u_boot_cmd.<name> . These are then put
> > into .u_boot_cmd by linker and lastly, linker uses SORT() over these
> > subsections to make proper order on them. This shall eliminate some
> > runtime overhead.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Mike Frysinger <vapier@gentoo.org>
> > ---
> > 
> >  arch/arm/cpu/u-boot.lds |    2 +-
> >  common/cmd_help.c       |    2 +-
> >  include/command.h       |    9 ++++++---
> >  3 files changed, 8 insertions(+), 5 deletions(-)
> 
> Seems incomplete in several aspects:

Below the section:

* NOTE * THIS PATCH IS CRAZY

There are a few notes. I'd actually like to know if this approach is correct at 
all, it might break on some crazy configurations or such.

> 1) what about all the non-ARM architecures and the board specific
>    linker scripts?

- This patch affects only arm926t, obviously to make it proper, every
  linkerscript would have to be adjusted

Which sucks, since there're a lot of them. But it can probably be automated.

> 2) what about removing the sort code?

You mean in the _do_help() in common/command.c? We can do not only that, but we 
can do bisect search in find_cmd_tbl() now too. I'm still trying to figure out 
the most optimal implementation. The current one I have trimmed down the time by 
roughly 60%, but I don't like it.

> Best regards,
> 
> Wolfgang Denk

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Wolfgang Denk]

Dear Marek Vasut,

In message <1343483279-11572-1-git-send-email-marex@denx.de> you wrote:
> This function converts static number to string in preprocessor.
> This is useful as it allows higher usage of puts() in favour of printf()
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Mike Frysinger <vapier@gentoo.org>
> ---
>  include/common.h |    7 +++++++
>  1 file changed, 7 insertions(+)

We have similar things already, and we don't add dead code - you add a
macro without users here.

If you want to unify the use of such a macro, fine - but then please
replace all ocurrences of equivalent definitions all over the code.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Bus error -- driver executed.

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Marek Vasut]

Dear Wolfgang Denk,

> Dear Marek Vasut,
> 
> In message <1343483279-11572-1-git-send-email-marex@denx.de> you wrote:
> > This function converts static number to string in preprocessor.
> > This is useful as it allows higher usage of puts() in favour of printf()
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Mike Frysinger <vapier@gentoo.org>
> > ---
> > 
> >  include/common.h |    7 +++++++
> >  1 file changed, 7 insertions(+)
> 
> We have similar things already, and we don't add dead code - you add a
> macro without users here.

It's used in 2/2 ... what macro do you have in mind ?

> If you want to unify the use of such a macro, fine - but then please
> replace all ocurrences of equivalent definitions all over the code.
> 
> Thanks.
> 
> Best regards,
> 
> Wolfgang Denk

Best regards,
Marek Vasut

[U-Boot] [RFC] [PATCH 2/2] crazy: Sort u_boot_cmd at runtime

[Wolfgang Denk]

Dear Marek Vasut,

In message <201207282039.34518.marex@denx.de> you wrote:
> 
> > Seems incomplete in several aspects:
> 
> Below the section:
> 
> * NOTE * THIS PATCH IS CRAZY

Then what is actually the purpose of such a posting?  Just dumping
unsorted thoughts to community?

You are experienced enough to know what would be needed for a
semi-clean patch, even if it's "just for RFC"...

> There are a few notes. I'd actually like to know if this approach is correct at 
> all, it might break on some crazy configurations or such.

Define "correct".  It may be possible - but what would be the
advantage?  Which problem does it solve?  In which way is it better
than the current code?

> > 1) what about all the non-ARM architecures and the board specific
> >    linker scripts?
> 
> - This patch affects only arm926t, obviously to make it proper, every
>   linkerscript would have to be adjusted
> 
> Which sucks, since there're a lot of them. But it can probably be automated.

Actually I doubt it makes sense at all.

I envision a situation where some "pluggable" code (say, a standalone
application, or some form of loadable module whatever) can add new
commands - it would be nice if these would still appear in sorted
order, but this cannot be done at compile-time.

So please explain which actual problem you are rying to solve.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It [being a Vulcan] means to adopt a philosophy, a way of life  which
is logical and beneficial. We cannot disregard that philosophy merely
for personal gain, no matter how important that gain might be.
	-- Spock, "Journey to Babel", stardate 3842.4

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Wolfgang Denk]

Dear Marek Vasut,

In message <201207282139.05998.marex@denx.de> you wrote:
> 
> > >  include/common.h |    7 +++++++
> > >  1 file changed, 7 insertions(+)
> > 
> > We have similar things already, and we don't add dead code - you add a
> > macro without users here.
> 
> It's used in 2/2 ... what macro do you have in mind ?

Then add it with the patch that uses it.

As for existing use, see for example 

arch/powerpc/include/asm/processor.h:#define stringify(s)	tostring(s)
drivers/mtd/ubi/build.c:#include <linux/stringify.h>
include/configs/km/keymile-common.h:#define xstr(s)	str(s)
include/configs/imx27lite-common.h:#define xstr(s)	str(s)
include/configs/tx25.h:#define xstr(s)	str(s)
include/configs/mx35pdk.h:#define xstr(s)	str(s)
include/configs/cam_enc_4xx.h:#define xstr(s)	str(s)
include/configs/ea20.h:#define xstr(s)	str(s)
include/configs/MPC8308RDB.h:#define xstr(s)	str(s)
include/configs/mpc8308_p1m.h:#define xstr(s)	str(s)
include/configs/flea3.h:#define xstr(s)	str(s)
include/configs/enbw_cmc.h:#define xstr(s)	str(s)
include/configs/at91sam9263ek.h:#define xstr(s)   str(s)
include/configs/amcc-common.h:#define xstr(s)	str(s)
include/configs/tam3517-common.h:#define xstr(s)	str(s)
include/configs/manroland/common.h:#define xstr(s)	str(s)
include/configs/qong.h:#define xstr(s)	str(s)
include/configs/ima3-mx53.h:#define xstr(s)	str(s)

> > If you want to unify the use of such a macro, fine - but then please
> > replace all ocurrences of equivalent definitions all over the code.

All these above (and any I eventually didn't find) should be replaced,
then.



Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In the bathtub of history the truth is harder to hold than the  soap,
and much more difficult to find ...     - Terry Pratchett, _Sourcery_

[U-Boot] [RFC] [PATCH 2/2] crazy: Sort u_boot_cmd at runtime

[Marek Vasut]

Dear Wolfgang Denk,

> Dear Marek Vasut,
> 
> In message <201207282039.34518.marex@denx.de> you wrote:
> > > Seems incomplete in several aspects:
> > Below the section:
> > 
> > * NOTE * THIS PATCH IS CRAZY
> 
> Then what is actually the purpose of such a posting?  Just dumping
> unsorted thoughts to community?

I'd prefer to get some feedback, you know ...

> You are experienced enough to know what would be needed for a
> semi-clean patch, even if it's "just for RFC"...

If you mean droping the ascii art ... well, yes.

But for draft patch, I'd like to actually see further ideas.

> > There are a few notes. I'd actually like to know if this approach is
> > correct at all, it might break on some crazy configurations or such.
> 
> Define "correct".

If there's not some obvious flub in the code. If this kind of abuse of CPP is 
correct or not.

> It may be possible - but what would be the advantage?

The list of commands will be already sorted.

> Which problem does it solve?

Optimization, nothing else.

> In which way is it better than the current code?

It's a bit faster.

> > > 1) what about all the non-ARM architecures and the board specific
> > > 
> > >    linker scripts?
> > 
> > - This patch affects only arm926t, obviously to make it proper, every
> > 
> >   linkerscript would have to be adjusted
> > 
> > Which sucks, since there're a lot of them. But it can probably be
> > automated.
> 
> Actually I doubt it makes sense at all.

It actually does ... but not in such a plain context.

I did this patch because we want the driver lists sorted. So I did this research 
and implemented it on the command list. I wanted to gather some feedback on if 
this actually can be done in such a way or if there'll be problems with 
toolchains maybe. Or any other issues.

> I envision a situation where some "pluggable" code (say, a standalone
> application, or some form of loadable module whatever) can add new
> commands - it would be nice if these would still appear in sorted
> order, but this cannot be done at compile-time.

Certainly ... but we can keep a separate runtime table for these added commands.

> So please explain which actual problem you are rying to solve.
> 
> Best regards,
> 
> Wolfgang Denk

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Marek Vasut]

Dear Wolfgang Denk,

> Dear Marek Vasut,
> 
> In message <201207282139.05998.marex@denx.de> you wrote:
> > > >  include/common.h |    7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > 
> > > We have similar things already, and we don't add dead code - you add a
> > > macro without users here.
> > 
> > It's used in 2/2 ... what macro do you have in mind ?
> 
> Then add it with the patch that uses it.

I really wanted to keep this one separate, but I'll bend here. But if you 
mention those xstr()s below, it might be actually better to add __stringify() 
and then replace all those crazy xstr()s.

> As for existing use, see for example

[...]

> include/configs/ima3-mx53.h:#define xstr(s)	str(s)

Thanks for pointing this out.

> > > If you want to unify the use of such a macro, fine - but then please
> > > replace all ocurrences of equivalent definitions all over the code.
> 
> All these above (and any I eventually didn't find) should be replaced,
> then.

Good.

> Best regards,
> 
> Wolfgang Denk

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Mike Frysinger]

On Saturday 28 July 2012 15:57:33 Wolfgang Denk wrote:
> Marek Vasut wrote:
> > > >  include/common.h |    7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > 
> > > We have similar things already, and we don't add dead code - you add a
> > > macro without users here.
> > 
> > It's used in 2/2 ... what macro do you have in mind ?
> 
> Then add it with the patch that uses it.
> 
> As for existing use, see for example

there's also MK_STR() and XMK_STR().  would be good to import 
linux/stringify.h (rather than adding these macros to common.h) and converting 
all consumers over to that.
-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/20120731/0b2baae2/attachment.pgp>

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Marek Vasut]

Dear Mike Frysinger,

> On Saturday 28 July 2012 15:57:33 Wolfgang Denk wrote:
> > Marek Vasut wrote:
> > > > >  include/common.h |    7 +++++++
> > > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > We have similar things already, and we don't add dead code - you add
> > > > a macro without users here.
> > > 
> > > It's used in 2/2 ... what macro do you have in mind ?
> > 
> > Then add it with the patch that uses it.
> > 
> > As for existing use, see for example
> 
> there's also MK_STR() and XMK_STR().

Grunt ... how do you find those? Or is it that you just happened to run over 
them?

> would be good to import
> linux/stringify.h (rather than adding these macros to common.h) and
> converting all consumers over to that.

I wonder, what's the gain?

> -mike

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Mike Frysinger]

On Tuesday 31 July 2012 09:55:55 Marek Vasut wrote:
> Dear Mike Frysinger,
> > On Saturday 28 July 2012 15:57:33 Wolfgang Denk wrote:
> > > Marek Vasut wrote:
> > > > > >  include/common.h |    7 +++++++
> > > > > >  1 file changed, 7 insertions(+)
> > > > > 
> > > > > We have similar things already, and we don't add dead code - you
> > > > > add a macro without users here.
> > > > 
> > > > It's used in 2/2 ... what macro do you have in mind ?
> > > 
> > > Then add it with the patch that uses it.
> > > 
> > > As for existing use, see for example
> > 
> > there's also MK_STR() and XMK_STR().
> 
> Grunt ... how do you find those? Or is it that you just happened to run
> over them?

these are the ones we use in Blackfin boards that i happened to stumble across 
when reading some common code

> > would be good to import
> > linux/stringify.h (rather than adding these macros to common.h) and
> > converting all consumers over to that.
> 
> I wonder, what's the gain?

one less way we're different from linux for importing code
-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/20120731/42e5ba39/attachment.pgp>

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Marek Vasut]

Dear Mike Frysinger,

> On Tuesday 31 July 2012 09:55:55 Marek Vasut wrote:
> > Dear Mike Frysinger,
> > 
> > > On Saturday 28 July 2012 15:57:33 Wolfgang Denk wrote:
> > > > Marek Vasut wrote:
> > > > > > >  include/common.h |    7 +++++++
> > > > > > >  1 file changed, 7 insertions(+)
> > > > > > 
> > > > > > We have similar things already, and we don't add dead code - you
> > > > > > add a macro without users here.
> > > > > 
> > > > > It's used in 2/2 ... what macro do you have in mind ?
> > > > 
> > > > Then add it with the patch that uses it.
> > > > 
> > > > As for existing use, see for example
> > > 
> > > there's also MK_STR() and XMK_STR().
> > 
> > Grunt ... how do you find those? Or is it that you just happened to run
> > over them?
> 
> these are the ones we use in Blackfin boards that i happened to stumble
> across when reading some common code

Ugh ... makes you feel like Indy, discovering gems in crazy places and fighting 
hordes of evil code ...

> > > would be good to import
> > > linux/stringify.h (rather than adding these macros to common.h) and
> > > converting all consumers over to that.
> > 
> > I wonder, what's the gain?
> 
> one less way we're different from linux for importing code

Not that that macro is ever gonna change, but so be it.

> -mike

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/2] COMMON: Add __stringify() function

[Mike Frysinger]

On Tuesday 31 July 2012 10:08:51 Marek Vasut wrote:
> Dear Mike Frysinger,
> > On Tuesday 31 July 2012 09:55:55 Marek Vasut wrote:
> > > Dear Mike Frysinger,
> > > > would be good to import
> > > > linux/stringify.h (rather than adding these macros to common.h) and
> > > > converting all consumers over to that.
> > > 
> > > I wonder, what's the gain?
> > 
> > one less way we're different from linux for importing code
> 
> Not that that macro is ever gonna change, but so be it.

i meant #include's, not the API itself.  you're right that likely this API 
will never change.
-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/20120731/0e397e95/attachment.pgp>