[U-Boot] a couple questions about CONFIG_SYS_LONGHELP
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
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,
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
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,
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
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,
participants (2)
-
Albert ARIBAUD -
Robert P. J. Day