public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V2] disk: make get_partition_info() always available to disk.c
Date: Fri, 21 Sep 2012 16:53:05 -0600	[thread overview]
Message-ID: <505CEFD1.8080405@wwwdotorg.org> (raw)
In-Reply-To: <20120921225107.GD16051@bill-the-cat>

On 09/21/2012 04:51 PM, Tom Rini wrote:
> On Fri, Sep 21, 2012 at 04:46:54PM -0600, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>> 
>> Now that get_device_and_partition() always calls
>> get_partition_info() when disk.c is compiled, we must always
>> compile the function, rather than ifdef it away.
>> 
>> The implementation must be conditional based on CONFIG_CMD_*
>> etc., since that's what e.g. part_dos.c uses to ifdef out
>> get_partition_info_dos(); CONFIG_DOS_PARTITION can be enabled
>> even without those commands being enabled.
>> 
>> Technically, this change is required before Rob's "disk/part:
>> introduce get_device_and_partition" patch. However, at least when
>> the compiler optimizer is turned on, it isn't required before
>> then in practice, since get_device_and_partition() calls
>> get_dev(), which is stubbed out in disk.c under exactly the same
>> conditions that get_partition_info() is not compiled, and hence
>> the compiler never generates code for the call to the missing
>> function. However, in my later patch "disk: 
>> get_device_and_partition() "auto" partition and cleanup", the
>> optimizer doesn't succeed at this, and may attempt to reference
>> the undefined function.
>> 
>> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- v2: Add
>> CONFIG_CMD_* etc. ifdefs around the implementation.
>> 
>> Rob, I wonder if you shouldn't squash this into your series.
>> Then, I'll need to rebase mine on your again since this causes a
>> few nasty conflicts with my series.
> 
> I _really_ want to see incremental changes.  It's one of the good 
> practices of the kernel folks and I'd like to see us do it as well
> as much as we can.

OK, well in that case, you can just apply the patch standalone before
you apply Rob's series then. I think we'll both need to rebase to
avoid conflict issues those - e.g. I edited the function that got
moved in my patch series and had to manually re-apply the change to
the new code location after I created this patch.

  reply	other threads:[~2012-09-21 22:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-21 22:46 [U-Boot] [PATCH V2] disk: make get_partition_info() always available to disk.c Stephen Warren
2012-09-21 22:51 ` Tom Rini
2012-09-21 22:53   ` Stephen Warren [this message]
2012-09-25 23:16 ` [U-Boot] [U-Boot, " Tom Rini

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=505CEFD1.8080405@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox