All of lore.kernel.org
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] common/cmd_disk.c doesn't actually define any "commands"
Date: Mon, 4 Feb 2013 17:01:20 +0100	[thread overview]
Message-ID: <20130204170120.791778b1@lilith> (raw)
In-Reply-To: <ED3E0BCACD909541BA94A34C4A164D4C429633D3@post.tritech.se>

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.

  reply	other threads:[~2013-02-04 16:01 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-04 12:53 [U-Boot] common/cmd_disk.c doesn't actually define any "commands" Robert P. J. Day
2013-02-04 14:26 ` Albert ARIBAUD
2013-02-04 15:04   ` Robert P. J. Day
2013-02-04 15:17     ` Mats Kärrman
2013-02-04 15:29       ` Robert P. J. Day
2013-02-04 15:30       ` Albert ARIBAUD
2013-02-04 15:42         ` Mats Kärrman
2013-02-04 16:01           ` Albert ARIBAUD [this message]
2013-02-04 15:18     ` Albert ARIBAUD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130204170120.791778b1@lilith \
    --to=albert.u.boot@aribaud.net \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.