public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] a couple questions about CONFIG_SYS_LONGHELP
@ 2013-01-26 11:38 Robert P. J. Day
  2013-01-26 11:45 ` Albert ARIBAUD
  0 siblings, 1 reply; 6+ messages in thread
From: Robert P. J. Day @ 2013-01-26 11:38 UTC (permalink / raw)
  To: u-boot


  first, is there any need for so many header files to define the
macro CONFIG_SYS_LONGHELP individually?  a quick count of how many
u-boot source files do just that:

$ grep -r "define.*CONFIG_SYS_LONGHELP" * | wc -l
479
$

is it really necessary for almost 500 source files to each define that
macro?

  and second, i'm not sure how to read this out of cmd_pci.c:

===== start

#ifdef CONFIG_SYS_LONGHELP
static char pci_help_text[] =
        "[bus] [long]\n"
        "    - short or long list of PCI devices on bus 'bus'\n"
#ifdef CONFIG_CMD_PCI_ENUM
        "pci enum\n"
        "    - re-enumerate PCI buses\n"
#endif
        "pci header b.d.f\n"
        "    - show header of PCI device 'bus.device.function'\n"
        "pci display[.b, .w, .l] b.d.f [address] [# of objects]\n"
        "    - display PCI configuration space (CFG)\n"
        "pci next[.b, .w, .l] b.d.f address\n"
        "    - modify, read and keep CFG address\n"
        "pci modify[.b, .w, .l] b.d.f address\n"
        "    -  modify, auto increment CFG address\n"
        "pci write[.b, .w, .l] b.d.f address value\n"
        "    - write to CFG address";
#endif

U_BOOT_CMD(
        pci,    5,      1,      do_pci,
        "list and access PCI Configuration Space", pci_help_text
);

===== end

  note how, if CONFIG_SYS_LONGHELP is defined, the symbol
"pci_help_text" is created as the text, but its *usage* just below in
the U_BOOT_CMD macro is *outside* of that preprocessor check.  how
would that work if CONFIG_SYS_LONGHELP is undefined?  not at my dev
host right this minute so i can't test, but it just looks ... weird.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] a couple questions about CONFIG_SYS_LONGHELP
  2013-01-26 11:38 [U-Boot] a couple questions about CONFIG_SYS_LONGHELP Robert P. J. Day
@ 2013-01-26 11:45 ` Albert ARIBAUD
  2013-01-26 12:11   ` Robert P. J. Day
  0 siblings, 1 reply; 6+ messages in thread
From: Albert ARIBAUD @ 2013-01-26 11:45 UTC (permalink / raw)
  To: u-boot

Hi Robert,

On Sat, 26 Jan 2013 06:38:51 -0500 (EST), "Robert P. J. Day"
<rpjday@crashcourse.ca> wrote:

> 
>   first, is there any need for so many header files to define the
> macro CONFIG_SYS_LONGHELP individually?  a quick count of how many
> u-boot source files do just that:
> 
> $ grep -r "define.*CONFIG_SYS_LONGHELP" * | wc -l
> 479
> $
> 
> is it really necessary for almost 500 source files to each define that
> macro?

Yes, is is, because each target should decide individually if they want
to embed the long help, which costs space and time, two valuable
resources in embedded development.

>   and second, i'm not sure how to read this out of cmd_pci.c:
> 
> ===== start
> 
> #ifdef CONFIG_SYS_LONGHELP
> static char pci_help_text[] =
>         "[bus] [long]\n"
>         "    - short or long list of PCI devices on bus 'bus'\n"
> #ifdef CONFIG_CMD_PCI_ENUM
>         "pci enum\n"
>         "    - re-enumerate PCI buses\n"
> #endif
>         "pci header b.d.f\n"
>         "    - show header of PCI device 'bus.device.function'\n"
>         "pci display[.b, .w, .l] b.d.f [address] [# of objects]\n"
>         "    - display PCI configuration space (CFG)\n"
>         "pci next[.b, .w, .l] b.d.f address\n"
>         "    - modify, read and keep CFG address\n"
>         "pci modify[.b, .w, .l] b.d.f address\n"
>         "    -  modify, auto increment CFG address\n"
>         "pci write[.b, .w, .l] b.d.f address value\n"
>         "    - write to CFG address";
> #endif
> 
> U_BOOT_CMD(
>         pci,    5,      1,      do_pci,
>         "list and access PCI Configuration Space", pci_help_text
> );
> 
> ===== end
> 
>   note how, if CONFIG_SYS_LONGHELP is defined, the symbol
> "pci_help_text" is created as the text, but its *usage* just below in
> the U_BOOT_CMD macro is *outside* of that preprocessor check.  how
> would that work if CONFIG_SYS_LONGHELP is undefined?  not at my dev
> host right this minute so i can't test, but it just looks ... weird.

Probably would not work. Submit a fix. :)

> rday

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] a couple questions about CONFIG_SYS_LONGHELP
  2013-01-26 11:45 ` Albert ARIBAUD
@ 2013-01-26 12:11   ` Robert P. J. Day
  2013-01-26 12:27     ` Albert ARIBAUD
  0 siblings, 1 reply; 6+ messages in thread
From: Robert P. J. Day @ 2013-01-26 12:11 UTC (permalink / raw)
  To: u-boot

On Sat, 26 Jan 2013, Albert ARIBAUD wrote:

> Hi Robert,
>
> On Sat, 26 Jan 2013 06:38:51 -0500 (EST), "Robert P. J. Day"
> <rpjday@crashcourse.ca> wrote:

... snip ...

> >   and second, i'm not sure how to read this out of cmd_pci.c:
> >
> > ===== start
> >
> > #ifdef CONFIG_SYS_LONGHELP
> > static char pci_help_text[] =
> >         "[bus] [long]\n"
> >         "    - short or long list of PCI devices on bus 'bus'\n"
> > #ifdef CONFIG_CMD_PCI_ENUM
> >         "pci enum\n"
> >         "    - re-enumerate PCI buses\n"
> > #endif
> >         "pci header b.d.f\n"
> >         "    - show header of PCI device 'bus.device.function'\n"
> >         "pci display[.b, .w, .l] b.d.f [address] [# of objects]\n"
> >         "    - display PCI configuration space (CFG)\n"
> >         "pci next[.b, .w, .l] b.d.f address\n"
> >         "    - modify, read and keep CFG address\n"
> >         "pci modify[.b, .w, .l] b.d.f address\n"
> >         "    -  modify, auto increment CFG address\n"
> >         "pci write[.b, .w, .l] b.d.f address value\n"
> >         "    - write to CFG address";
> > #endif
> >
> > U_BOOT_CMD(
> >         pci,    5,      1,      do_pci,
> >         "list and access PCI Configuration Space", pci_help_text
> > );
> >
> > ===== end
> >
> >   note how, if CONFIG_SYS_LONGHELP is defined, the symbol
> > "pci_help_text" is created as the text, but its *usage* just below in
> > the U_BOOT_CMD macro is *outside* of that preprocessor check.  how
> > would that work if CONFIG_SYS_LONGHELP is undefined?  not at my dev
> > host right this minute so i can't test, but it just looks ... weird.
>
> Probably would not work. Submit a fix. :)

  there appears to be a number of common/cmd_*.c files that have that
structure:

$ grep "ifdef.*CONFIG_SYS_LONGHELP" cmd*.c
cmd_bootm.c:#ifdef CONFIG_SYS_LONGHELP
cmd_bootm.c:#ifdef CONFIG_SYS_LONGHELP
cmd_fdt.c:#ifdef CONFIG_SYS_LONGHELP
cmd_help.c:#ifdef  CONFIG_SYS_LONGHELP
cmd_i2c.c:#ifdef CONFIG_SYS_LONGHELP
cmd_mp.c:#ifdef CONFIG_SYS_LONGHELP
cmd_mtdparts.c:#ifdef CONFIG_SYS_LONGHELP
cmd_nand.c:#ifdef CONFIG_SYS_LONGHELP
cmd_nvedit.c:#ifdef CONFIG_SYS_LONGHELP
cmd_pci.c:#ifdef CONFIG_SYS_LONGHELP
cmd_source.c:#ifdef CONFIG_SYS_LONGHELP
cmd_ximg.c:#ifdef CONFIG_SYS_LONGHELP
$

  so it's not just one file.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] a couple questions about CONFIG_SYS_LONGHELP
  2013-01-26 12:11   ` Robert P. J. Day
@ 2013-01-26 12:27     ` Albert ARIBAUD
  2013-01-26 12:37       ` Robert P. J. Day
  0 siblings, 1 reply; 6+ messages in thread
From: Albert ARIBAUD @ 2013-01-26 12:27 UTC (permalink / raw)
  To: u-boot

Hi Robert,

On Sat, 26 Jan 2013 07:11:18 -0500 (EST), "Robert P. J. Day"
<rpjday@crashcourse.ca> wrote:

> On Sat, 26 Jan 2013, Albert ARIBAUD wrote:
> 
> > Hi Robert,
> >
> > On Sat, 26 Jan 2013 06:38:51 -0500 (EST), "Robert P. J. Day"
> > <rpjday@crashcourse.ca> wrote:
> 
> ... snip ...
> 
> > >   and second, i'm not sure how to read this out of cmd_pci.c:
> > >
> > > ===== start
> > >
> > > #ifdef CONFIG_SYS_LONGHELP
> > > static char pci_help_text[] =
> > >         "[bus] [long]\n"
> > >         "    - short or long list of PCI devices on bus 'bus'\n"
> > > #ifdef CONFIG_CMD_PCI_ENUM
> > >         "pci enum\n"
> > >         "    - re-enumerate PCI buses\n"
> > > #endif
> > >         "pci header b.d.f\n"
> > >         "    - show header of PCI device 'bus.device.function'\n"
> > >         "pci display[.b, .w, .l] b.d.f [address] [# of objects]\n"
> > >         "    - display PCI configuration space (CFG)\n"
> > >         "pci next[.b, .w, .l] b.d.f address\n"
> > >         "    - modify, read and keep CFG address\n"
> > >         "pci modify[.b, .w, .l] b.d.f address\n"
> > >         "    -  modify, auto increment CFG address\n"
> > >         "pci write[.b, .w, .l] b.d.f address value\n"
> > >         "    - write to CFG address";
> > > #endif
> > >
> > > U_BOOT_CMD(
> > >         pci,    5,      1,      do_pci,
> > >         "list and access PCI Configuration Space", pci_help_text
> > > );
> > >
> > > ===== end
> > >
> > >   note how, if CONFIG_SYS_LONGHELP is defined, the symbol
> > > "pci_help_text" is created as the text, but its *usage* just below in
> > > the U_BOOT_CMD macro is *outside* of that preprocessor check.  how
> > > would that work if CONFIG_SYS_LONGHELP is undefined?  not at my dev
> > > host right this minute so i can't test, but it just looks ... weird.
> >
> > Probably would not work. Submit a fix. :)
> 
>   there appears to be a number of common/cmd_*.c files that have that
> structure:
> 
> $ grep "ifdef.*CONFIG_SYS_LONGHELP" cmd*.c
> cmd_bootm.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_bootm.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_fdt.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_help.c:#ifdef  CONFIG_SYS_LONGHELP
> cmd_i2c.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_mp.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_mtdparts.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_nand.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_nvedit.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_pci.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_source.c:#ifdef CONFIG_SYS_LONGHELP
> cmd_ximg.c:#ifdef CONFIG_SYS_LONGHELP
> $
> 
>   so it's not just one file.

The grep above just shows that there are files have conditionally
compiled code for CONFIG_SYS_LONGHELP; however it does not show per
se that these files wound not compile without it.

Even assuming some of these files would not build without
CONFIG_SYS_LONGHELP (cmd_pci.c is already proven not to), at least for
ARM (an architecture for which I frequently build all targets) no
target fails with an undefined variable, which implies that no target
uses any of those files without defining CONFIG_SYS_LONGHELP.

My conclusion is: if you need to build one or more targets without
CONFIG_SYS_LONGHELP and it fails in some cmd_*.c file, then submit a
patch that fixes the cmd_*.c files and undefines CONFIG_SYS_LONGHELP
in the include/configs/*.h header files for those targets.

> rday

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] a couple questions about CONFIG_SYS_LONGHELP
  2013-01-26 12:27     ` Albert ARIBAUD
@ 2013-01-26 12:37       ` Robert P. J. Day
  2013-01-26 15:55         ` Albert ARIBAUD
  0 siblings, 1 reply; 6+ messages in thread
From: Robert P. J. Day @ 2013-01-26 12:37 UTC (permalink / raw)
  To: u-boot

On Sat, 26 Jan 2013, Albert ARIBAUD wrote:

> Hi Robert,
>
> On Sat, 26 Jan 2013 07:11:18 -0500 (EST), "Robert P. J. Day"
> <rpjday@crashcourse.ca> wrote:
>
> > On Sat, 26 Jan 2013, Albert ARIBAUD wrote:
> >
> > > Hi Robert,
> > >
> > > On Sat, 26 Jan 2013 06:38:51 -0500 (EST), "Robert P. J. Day"
> > > <rpjday@crashcourse.ca> wrote:
> >
> > ... snip ...
> >
> > > >   and second, i'm not sure how to read this out of cmd_pci.c:
> > > >
> > > > ===== start
> > > >
> > > > #ifdef CONFIG_SYS_LONGHELP
> > > > static char pci_help_text[] =
> > > >         "[bus] [long]\n"
> > > >         "    - short or long list of PCI devices on bus 'bus'\n"
> > > > #ifdef CONFIG_CMD_PCI_ENUM
> > > >         "pci enum\n"
> > > >         "    - re-enumerate PCI buses\n"
> > > > #endif
> > > >         "pci header b.d.f\n"
> > > >         "    - show header of PCI device 'bus.device.function'\n"
> > > >         "pci display[.b, .w, .l] b.d.f [address] [# of objects]\n"
> > > >         "    - display PCI configuration space (CFG)\n"
> > > >         "pci next[.b, .w, .l] b.d.f address\n"
> > > >         "    - modify, read and keep CFG address\n"
> > > >         "pci modify[.b, .w, .l] b.d.f address\n"
> > > >         "    -  modify, auto increment CFG address\n"
> > > >         "pci write[.b, .w, .l] b.d.f address value\n"
> > > >         "    - write to CFG address";
> > > > #endif
> > > >
> > > > U_BOOT_CMD(
> > > >         pci,    5,      1,      do_pci,
> > > >         "list and access PCI Configuration Space", pci_help_text
> > > > );
> > > >
> > > > ===== end
> > > >
> > > >   note how, if CONFIG_SYS_LONGHELP is defined, the symbol
> > > > "pci_help_text" is created as the text, but its *usage* just below in
> > > > the U_BOOT_CMD macro is *outside* of that preprocessor check.  how
> > > > would that work if CONFIG_SYS_LONGHELP is undefined?  not at my dev
> > > > host right this minute so i can't test, but it just looks ... weird.
> > >
> > > Probably would not work. Submit a fix. :)
> >
> >   there appears to be a number of common/cmd_*.c files that have that
> > structure:
> >
> > $ grep "ifdef.*CONFIG_SYS_LONGHELP" cmd*.c
> > cmd_bootm.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_bootm.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_fdt.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_help.c:#ifdef  CONFIG_SYS_LONGHELP
> > cmd_i2c.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_mp.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_mtdparts.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_nand.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_nvedit.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_pci.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_source.c:#ifdef CONFIG_SYS_LONGHELP
> > cmd_ximg.c:#ifdef CONFIG_SYS_LONGHELP
> > $
> >
> >   so it's not just one file.
>
> The grep above just shows that there are files have conditionally
> compiled code for CONFIG_SYS_LONGHELP; however it does not show per
> se that these files wound not compile without it.

  i'm aware of that -- but i manually examined some of the files
above, and all of the ones i looked at had the same "issue" as
cmd_pci.c.  the grep command was not meant as proof, i just used it as
a guide for which files i wanted to look at more closely, and i see
the same problem with all of them.  so someone higher up the food
chain than me can decide if this is something worth addressing.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] a couple questions about CONFIG_SYS_LONGHELP
  2013-01-26 12:37       ` Robert P. J. Day
@ 2013-01-26 15:55         ` Albert ARIBAUD
  0 siblings, 0 replies; 6+ messages in thread
From: Albert ARIBAUD @ 2013-01-26 15:55 UTC (permalink / raw)
  To: u-boot

Hi Robert,

On Sat, 26 Jan 2013 07:37:45 -0500 (EST), "Robert P. J. Day"
<rpjday@crashcourse.ca> wrote:

> On Sat, 26 Jan 2013, Albert ARIBAUD wrote:
> 
> > Hi Robert,
> >
> > On Sat, 26 Jan 2013 07:11:18 -0500 (EST), "Robert P. J. Day"
> > <rpjday@crashcourse.ca> wrote:
> >
> > > On Sat, 26 Jan 2013, Albert ARIBAUD wrote:
> > >
> > > > Hi Robert,
> > > >
> > > > On Sat, 26 Jan 2013 06:38:51 -0500 (EST), "Robert P. J. Day"
> > > > <rpjday@crashcourse.ca> wrote:
> > >
> > > ... snip ...
> > >
> > > > >   and second, i'm not sure how to read this out of cmd_pci.c:
> > > > >
> > > > > ===== start
> > > > >
> > > > > #ifdef CONFIG_SYS_LONGHELP
> > > > > static char pci_help_text[] =
> > > > >         "[bus] [long]\n"
> > > > >         "    - short or long list of PCI devices on bus 'bus'\n"
> > > > > #ifdef CONFIG_CMD_PCI_ENUM
> > > > >         "pci enum\n"
> > > > >         "    - re-enumerate PCI buses\n"
> > > > > #endif
> > > > >         "pci header b.d.f\n"
> > > > >         "    - show header of PCI device 'bus.device.function'\n"
> > > > >         "pci display[.b, .w, .l] b.d.f [address] [# of objects]\n"
> > > > >         "    - display PCI configuration space (CFG)\n"
> > > > >         "pci next[.b, .w, .l] b.d.f address\n"
> > > > >         "    - modify, read and keep CFG address\n"
> > > > >         "pci modify[.b, .w, .l] b.d.f address\n"
> > > > >         "    -  modify, auto increment CFG address\n"
> > > > >         "pci write[.b, .w, .l] b.d.f address value\n"
> > > > >         "    - write to CFG address";
> > > > > #endif
> > > > >
> > > > > U_BOOT_CMD(
> > > > >         pci,    5,      1,      do_pci,
> > > > >         "list and access PCI Configuration Space", pci_help_text
> > > > > );
> > > > >
> > > > > ===== end
> > > > >
> > > > >   note how, if CONFIG_SYS_LONGHELP is defined, the symbol
> > > > > "pci_help_text" is created as the text, but its *usage* just below in
> > > > > the U_BOOT_CMD macro is *outside* of that preprocessor check.  how
> > > > > would that work if CONFIG_SYS_LONGHELP is undefined?  not at my dev
> > > > > host right this minute so i can't test, but it just looks ... weird.
> > > >
> > > > Probably would not work. Submit a fix. :)
> > >
> > >   there appears to be a number of common/cmd_*.c files that have that
> > > structure:
> > >
> > > $ grep "ifdef.*CONFIG_SYS_LONGHELP" cmd*.c
> > > cmd_bootm.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_bootm.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_fdt.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_help.c:#ifdef  CONFIG_SYS_LONGHELP
> > > cmd_i2c.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_mp.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_mtdparts.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_nand.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_nvedit.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_pci.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_source.c:#ifdef CONFIG_SYS_LONGHELP
> > > cmd_ximg.c:#ifdef CONFIG_SYS_LONGHELP
> > > $
> > >
> > >   so it's not just one file.
> >
> > The grep above just shows that there are files have conditionally
> > compiled code for CONFIG_SYS_LONGHELP; however it does not show per
> > se that these files wound not compile without it.
> 
>   i'm aware of that -- but i manually examined some of the files
> above, and all of the ones i looked at had the same "issue" as
> cmd_pci.c.  the grep command was not meant as proof, i just used it as
> a guide for which files i wanted to look at more closely, and i see
> the same problem with all of them.  so someone higher up the food
> chain than me can decide if this is something worth addressing.

Actually there is no such hierarchical decision process where decision
to fix would come from some 'upper layer' -- which simply does not
exist as such in U-Boot. Decision comes from whoever feels something has
to be changed, and these persons just submit a patch or patch-set which
embodies their change. At this point board maintainers or custodians,
or plain readers on the list too, may comment the submission and
request changes; and custodians and maintainers can NAK it if it is
wrong or had dead code or does not match the overall design or some
other technical reason ; but submitting in the first place requires no
authorization.

> rday

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-01-26 15:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-26 11:38 [U-Boot] a couple questions about CONFIG_SYS_LONGHELP Robert P. J. Day
2013-01-26 11:45 ` Albert ARIBAUD
2013-01-26 12:11   ` Robert P. J. Day
2013-01-26 12:27     ` Albert ARIBAUD
2013-01-26 12:37       ` Robert P. J. Day
2013-01-26 15:55         ` Albert ARIBAUD

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox