mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* the "CONFIG_" namespace and what's with "CFG_CMD_" prefixes?
@ 2009-12-20 14:06 Robert P. J. Day
  2009-12-21 10:01 ` Sascha Hauer
  0 siblings, 1 reply; 2+ messages in thread
From: Robert P. J. Day @ 2009-12-20 14:06 UTC (permalink / raw)
  To: U-Boot Version 2 (barebox)


  based on some scanning i just did, i notice the following
occurrences of the "CMD_CFG_" prefix on a number of preprocessor
variables:

$ grep -r CFG_CMD *
arch/ppc/mach-mpc5xxx/interrupts.c:#if (CONFIG_COMMANDS & CFG_CMD_IRQ)
arch/ppc/mach-mpc5xxx/ide.c:#ifdef CFG_CMD_IDE
arch/ppc/mach-mpc5xxx/ide.c:#endif /* CFG_CMD_IDE */
arch/ppc/lib/kgdb.c:#endif /* CFG_CMD_KGDB */
board/pcm030/pcm030.c:#if defined (CFG_CMD_IDE) && defined (CONFIG_IDE_RESET)
board/pcm030/pcm030.c:#endif /* defined (CFG_CMD_IDE) && defined (CONFIG_IDE_RESET) */
commands/loads.c:# if (CONFIG_COMMANDS & CFG_CMD_SAVES)
commands/loads.c:# endif /* CFG_CMD_SAVES */
commands/loads.c:#if (CONFIG_COMMANDS & CFG_CMD_SAVES)
commands/loads.c:# endif /* CFG_CMD_SAVES */
commands/loads.c:#if (CONFIG_COMMANDS & CFG_CMD_SAVES)
commands/loads.c:#endif	/* CFG_CMD_SAVES */
common/Kconfig:	  automatically enabled when you select CFG_CMD_DATE .
drivers/net/at91_ether.c:#if defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII)
drivers/net/at91_ether.c:#endif	/* defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII) */
drivers/net/at91_ether.c:#if defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII)
lib/crc32.c:#include <common.h>	/* to get command definitions like CFG_CMD_JFFS2 */
$

  it's not clear what that prefix is supposed to represent since, in
at least one case, it might be a typo -- "CMD_SAVES":

$ grep -r CMD_SAVES *
commands/loads.c:# if (CONFIG_COMMANDS & CFG_CMD_SAVES)
commands/loads.c:# endif /* CFG_CMD_SAVES */
commands/loads.c:#if (CONFIG_COMMANDS & CFG_CMD_SAVES)
commands/loads.c:# endif /* CFG_CMD_SAVES */
commands/loads.c:#if (CONFIG_COMMANDS & CFG_CMD_SAVES)
commands/loads.c:#endif	/* CFG_CMD_SAVES */
commands/Kconfig:config CMD_SAVES
$

  if those tests are meant to match that Kconfig variable, then
obviously they're misspelled.  if not, then i'm not sure what's going
on.  where *are* CFG_CMD_ variables supposed to be defined?  as in:

$ grep -rw CFG_CMD_MII *
drivers/net/at91_ether.c:#if defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII)
drivers/net/at91_ether.c:#endif	/* defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII) */
drivers/net/at91_ether.c:#if defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII)
$

  as a final point, it's standard practise that the "CONFIG_"-prefixed
variable namespace should be reserved *exclusively* for Kconfig
variables.  at least that's the way it works in the kernel world.

  thoughts?

rday
--


========================================================================
Robert P. J. Day                               Waterloo, Ontario, CANADA

            Linux Consulting, Training and Kernel Pedantry.

Web page:                                          http://crashcourse.ca
Twitter:                                       http://twitter.com/rpjday
========================================================================

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: the "CONFIG_" namespace and what's with "CFG_CMD_" prefixes?
  2009-12-20 14:06 the "CONFIG_" namespace and what's with "CFG_CMD_" prefixes? Robert P. J. Day
@ 2009-12-21 10:01 ` Sascha Hauer
  0 siblings, 0 replies; 2+ messages in thread
From: Sascha Hauer @ 2009-12-21 10:01 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: U-Boot Version 2 (barebox)

On Sun, Dec 20, 2009 at 09:06:07AM -0500, Robert P. J. Day wrote:
> 
>   based on some scanning i just did, i notice the following
> occurrences of the "CMD_CFG_" prefix on a number of preprocessor
> variables:
> 
> $ grep -r CFG_CMD *
> arch/ppc/mach-mpc5xxx/interrupts.c:#if (CONFIG_COMMANDS & CFG_CMD_IRQ)
> arch/ppc/mach-mpc5xxx/ide.c:#ifdef CFG_CMD_IDE
> arch/ppc/mach-mpc5xxx/ide.c:#endif /* CFG_CMD_IDE */

This file can be removed (and taken again from U-Boot if we want to add
ide support for barebox)

> arch/ppc/lib/kgdb.c:#endif /* CFG_CMD_KGDB */
> board/pcm030/pcm030.c:#if defined (CFG_CMD_IDE) && defined (CONFIG_IDE_RESET)
> board/pcm030/pcm030.c:#endif /* defined (CFG_CMD_IDE) && defined (CONFIG_IDE_RESET) */

unused code. I just created a patch to remove it.

> commands/loads.c:# if (CONFIG_COMMANDS & CFG_CMD_SAVES)
> commands/loads.c:# endif /* CFG_CMD_SAVES */
> commands/loads.c:#if (CONFIG_COMMANDS & CFG_CMD_SAVES)
> commands/loads.c:# endif /* CFG_CMD_SAVES */
> commands/loads.c:#if (CONFIG_COMMANDS & CFG_CMD_SAVES)
> commands/loads.c:#endif	/* CFG_CMD_SAVES */

commands/loads.c hasn't been touched for ages and won't even compile. If
somebody wants to add loads support he should start from scratch or with
original U-Boot code. When I started with U-Boot-V2 or now barebox there
was much old code and some of it is still there. I think we can safely
remove code which obviously wasn't touched for long time and still looks
like U-Boot. The original U-Boot code is still there for reference and
as a base to do a fresh start.

> common/Kconfig:	  automatically enabled when you select CFG_CMD_DATE .
> drivers/net/at91_ether.c:#if defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII)
> drivers/net/at91_ether.c:#endif	/* defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII) */
> drivers/net/at91_ether.c:#if defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII)

The CFG_CMD_MII part can be removed here.

> lib/crc32.c:#include <common.h>	/* to get command definitions like CFG_CMD_JFFS2 */

This comment is obsolete

> $
> 
>   it's not clear what that prefix is supposed to represent since, in
> at least one case, it might be a typo -- "CMD_SAVES":
> 
> $ grep -r CMD_SAVES *
> commands/loads.c:# if (CONFIG_COMMANDS & CFG_CMD_SAVES)
> commands/loads.c:# endif /* CFG_CMD_SAVES */
> commands/loads.c:#if (CONFIG_COMMANDS & CFG_CMD_SAVES)
> commands/loads.c:# endif /* CFG_CMD_SAVES */
> commands/loads.c:#if (CONFIG_COMMANDS & CFG_CMD_SAVES)
> commands/loads.c:#endif	/* CFG_CMD_SAVES */
> commands/Kconfig:config CMD_SAVES
> $
> 
>   if those tests are meant to match that Kconfig variable, then
> obviously they're misspelled.  if not, then i'm not sure what's going
> on.  where *are* CFG_CMD_ variables supposed to be defined?  as in:
> 
> $ grep -rw CFG_CMD_MII *
> drivers/net/at91_ether.c:#if defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII)
> drivers/net/at91_ether.c:#endif	/* defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII) */
> drivers/net/at91_ether.c:#if defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII)
> $
> 
>   as a final point, it's standard practise that the "CONFIG_"-prefixed
> variable namespace should be reserved *exclusively* for Kconfig
> variables.  at least that's the way it works in the kernel world.

Ack.

As a general note all CFG_CMD_ variables are leftovers from original
U-Boot code and should be either converted to Kconfig variables or
removed if not needed.

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] 2+ messages in thread

end of thread, other threads:[~2009-12-21 10:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-20 14:06 the "CONFIG_" namespace and what's with "CFG_CMD_" prefixes? Robert P. J. Day
2009-12-21 10:01 ` Sascha Hauer

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