[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[Robert P. J. Day]

   another observation from my weekend perusal of all of the  
common/cmd_*.c files is that, despite its "cmd_" filename prefix, the  
source file cmd_disk.c doesn't define any actual u-boot commands.   
according to what i see as u-boot filename naming conventions, it  
shouldn't be named "cmd_*", should it?

rday

[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[Albert ARIBAUD]

Hi Robert,

On Mon, 04 Feb 2013 07:53:43 -0500, "Robert P. J. Day"
<rpjday@crashcourse.ca> wrote:

>    another observation from my weekend perusal of all of the  
> common/cmd_*.c files is that, despite its "cmd_" filename prefix, the  
> source file cmd_disk.c doesn't define any actual u-boot commands.   
> according to what i see as u-boot filename naming conventions, it  
> shouldn't be named "cmd_*", should it?

That's arguable: apparently it provides common_diskboot() which
implements commands for cmd_ide, cmd_scsi, cmd_usb which use it for
implementing their do_diskboot, du_scsiboot, do_usbboot commands
respectively. It's purely related to cmd_*.

> rday

Amicalement,
-- 
Albert.

[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[Robert P. J. Day]

Quoting Albert ARIBAUD <albert.u.boot@aribaud.net>:

> Hi Robert,
>
> On Mon, 04 Feb 2013 07:53:43 -0500, "Robert P. J. Day"
> <rpjday@crashcourse.ca> wrote:
>
>>    another observation from my weekend perusal of all of the
>> common/cmd_*.c files is that, despite its "cmd_" filename prefix, the
>> source file cmd_disk.c doesn't define any actual u-boot commands.
>> according to what i see as u-boot filename naming conventions, it
>> shouldn't be named "cmd_*", should it?
>
> That's arguable: apparently it provides common_diskboot() which
> implements commands for cmd_ide, cmd_scsi, cmd_usb which use it for
> implementing their do_diskboot, du_scsiboot, do_usbboot commands
> respectively. It's purely related to cmd_*.

   just to be clear, i have no strong opinion on this either way, but
my understanding is that source files with the name of "common/cmd_*.c"
typically define at least one u-boot "command" with the U_BOOT_CMD macro.
as far as i can tell, cmd_disk.c is the only counterexample of that.

   it may be true that that file provides utility routines for other
actual "commmand" files, but so do many others.  for example, cmd_fdt.c
defines the behaviour of the actual "fdt" command, while fdt-related
utility routines are in fdt_support.c or even in libfdt/.

   it's no big deal, i'm just pointing out that cmd_disk.c flies in the
face of a fairly obvious pattern in u-boot filenaming convention.  and
on that note, i will shut up about it. :-)

rday

[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[Mats Kärrman]

Hi Robert, Albert,

> Robert P. J. Day wrote:
>   just to be clear, i have no strong opinion on this either way, but
> my understanding is that source files with the name of "common/cmd_*.c"
> typically define at least one u-boot "command" with the U_BOOT_CMD macro.
> as far as i can tell, cmd_disk.c is the only counterexample of that.

I was just looking into this myself but from another perspective.
I'm using "CONFIG_USB:STORAGE" and because of the way things are made, I then automatically
get "usbboot" for which I have no use. Things would be more logical if it was up
to cmd_disk to define the commands based on ifdef's for the various backends -- but
ONLY if CONFIG_CMD_DISK was in the config file.

Maybe this upsets a lot of other people though...?
 
Best regards,
Mats

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[Robert P. J. Day]

   i know i mentioned it this weekend but the only reason i tripped
over that cmd_disk.c thingie is that i built myself a cmd_*.c
reference list, matching source files with defined commands and
any preprocessor conditions that defined them here:

http://www.crashcourse.ca/wiki/index.php/U-Boot_command/file_reference

   i just wanted a truly up-to-date list of u-boot commands, where
they come from, and any CONFIG_ settings necessary for compiling them
into the u-boot image.  it was, unsurprisingly, a very educational
experience, where i wandered across the occasional oddity.

rday

[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[Albert ARIBAUD]

Hi Mats,

On Mon, 4 Feb 2013 15:17:17 +0000, Mats K?rrman
<Mats.Karrman@tritech.se> wrote:

> Hi Robert, Albert,
> 
> > Robert P. J. Day wrote:
> >   just to be clear, i have no strong opinion on this either way, but
> > my understanding is that source files with the name of "common/cmd_*.c"
> > typically define at least one u-boot "command" with the U_BOOT_CMD macro.
> > as far as i can tell, cmd_disk.c is the only counterexample of that.
> 
> I was just looking into this myself but from another perspective.
> I'm using "CONFIG_USB:STORAGE" and because of the way things are made, I then automatically
> get "usbboot" for which I have no use. Things would be more logical if it was up
> to cmd_disk to define the commands based on ifdef's for the various backends -- but
> ONLY if CONFIG_CMD_DISK was in the config file.

> Maybe this upsets a lot of other people though...?

Maybe, but the problem you state is not about cmd_disk (or I am
missing the point). USB commands are in USB related files, e.g.
do_usbboot() is in cmd_usb.c, so that's where a conditional should be
put if you want to compile the command out, rather than in cmd_disk,
which does not add to the U-Boot commands table at all.

> Best regards,
> Mats

Amicalement,
-- 
Albert.

[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[Mats Kärrman]

Hi Albert,

Albert ARIBAUD wrote:
> Maybe, but the problem you state is not about cmd_disk (or I am
> missing the point). USB commands are in USB related files, e.g.
> do_usbboot() is in cmd_usb.c, so that's where a conditional should be
> put if you want to compile the command out, rather than in cmd_disk,
> which does not add to the U-Boot commands table at all.

The cmd_usb file contains all the other USB commands through "do_usb"
and "usbboot" through "do_usbboot" that is just a forward to "common_diskboot".
Maybe the major miss-feature here is that you get usbboot and a bunch
of extra code, just because you want to be able to read USB memories.
This could of course be fixed by revising the ifdefs in cmd_usb etc.
instead but in that case I support Robert in his remark about the file naming ;)

BR // Mats

[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[Albert ARIBAUD]

Hi Mats,

On Mon, 4 Feb 2013 15:42:17 +0000, Mats K?rrman
<Mats.Karrman@tritech.se> wrote:

> Hi Albert,
> 
> Albert ARIBAUD wrote:
> > Maybe, but the problem you state is not about cmd_disk (or I am
> > missing the point). USB commands are in USB related files, e.g.
> > do_usbboot() is in cmd_usb.c, so that's where a conditional should be
> > put if you want to compile the command out, rather than in cmd_disk,
> > which does not add to the U-Boot commands table at all.
> 
> The cmd_usb file contains all the other USB commands through "do_usb"
> and "usbboot" through "do_usbboot" that is just a forward to "common_diskboot".
> Maybe the major miss-feature here is that you get usbboot and a bunch
> of extra code, just because you want to be able to read USB memories.
> This could of course be fixed by revising the ifdefs in cmd_usb etc.
> instead but in that case I support Robert in his remark about the file naming ;)

Actually there is no way to fulfill your need to make the usbboot
command conditionally compiled by modifying cmd_disk, because cmd_disk
simply does not define any command -- so you will have to put the
conditionals in cmd_usb.c no matter what.

Now you may want to also conditionally compile cmd_disk.c only of USB,
SCSI or IDE require it, but this you can and should do in the Makefile;
remember cmd_disk.c is only useful to provide common_diskboot(), so
either you completely compile it, or you don't compile it at all.

(now this could be different if we use gcc's -fdata-sections,
-ffunction-sections along with ld's --gc-sections, as then we could
argue that even if compiled, cmd_disk.c would be linked out if
unreferenced.)

As for the name, Robert's issue stems from his assumption that a file
with cmd_ necessarily declares a listable U-Boot command. I assume that
files with cmd_ contain command-related code not necessarily including
a listable command -- for instance here, a command execution function
which is mapped to a (set of three) listable command(s) elsewhere.

> BR // Mats

Amicalement,
-- 
Albert.

[U-Boot] common/cmd_disk.c doesn't actually define any "commands"

[Albert ARIBAUD]

Hi Robert,

On Mon, 04 Feb 2013 10:04:20 -0500, "Robert P. J. Day"
<rpjday@crashcourse.ca> wrote:

> Quoting Albert ARIBAUD <albert.u.boot@aribaud.net>:
> 
> > Hi Robert,
> >
> > On Mon, 04 Feb 2013 07:53:43 -0500, "Robert P. J. Day"
> > <rpjday@crashcourse.ca> wrote:
> >
> >>    another observation from my weekend perusal of all of the
> >> common/cmd_*.c files is that, despite its "cmd_" filename prefix, the
> >> source file cmd_disk.c doesn't define any actual u-boot commands.
> >> according to what i see as u-boot filename naming conventions, it
> >> shouldn't be named "cmd_*", should it?
> >
> > That's arguable: apparently it provides common_diskboot() which
> > implements commands for cmd_ide, cmd_scsi, cmd_usb which use it for
> > implementing their do_diskboot, du_scsiboot, do_usbboot commands
> > respectively. It's purely related to cmd_*.
> 
>    just to be clear, i have no strong opinion on this either way, but
> my understanding is that source files with the name of "common/cmd_*.c"
> typically define at least one u-boot "command" with the U_BOOT_CMD macro.
> as far as i can tell, cmd_disk.c is the only counterexample of that.
> 
>    it may be true that that file provides utility routines for other
> actual "commmand" files, but so do many others.  for example, cmd_fdt.c
> defines the behaviour of the actual "fdt" command, while fdt-related
> utility routines are in fdt_support.c or even in libfdt/.

That's understandable as libfdt provides support for much more than
commands; OTOH, cmd_disk provides support *only* to commands.

>    it's no big deal, i'm just pointing out that cmd_disk.c flies in the
> face of a fairly obvious pattern in u-boot filenaming convention.  and
> on that note, i will shut up about it. :-)
> 
> rday

Amicalement,
-- 
Albert.