All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined
Date: Thu, 29 Nov 2012 12:58:39 -0700	[thread overview]
Message-ID: <50B7BE6F.2020602@wwwdotorg.org> (raw)
In-Reply-To: <5FBF8E85CA34454794F0F7ECBA79798F37A1FA98F9@HQMAIL04.nvidia.com>

On 11/29/2012 12:50 PM, Tom Warren wrote:
> Stephen,
> 
>> -----Original Message-----
>> From: Stephen Warren [mailto:swarren at wwwdotorg.org]
>> Sent: Thursday, November 29, 2012 12:10 PM
>> To: Tom Warren
>> Cc: Simon Glass; u-boot at lists.denx.de; Stephen Warren; Marc Dietrich;
>> Thierry Reding
>> Subject: Re: [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not
>> already defined
>>
>> On 11/29/2012 11:40 AM, Tom Warren wrote:
>>> Stephen,
>>>
>>>> -----Original Message-----
>>>> From: Stephen Warren [mailto:swarren at wwwdotorg.org]
>>>> Sent: Wednesday, November 28, 2012 2:03 PM
>>>> To: Simon Glass
>>>> Cc: u-boot at lists.denx.de; Tom Warren; Stephen Warren; Marc Dietrich;
>>>> Thierry Reding
>>>> Subject: Re: [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if
>>>> not already defined
>>>>
>>>> On 11/28/2012 02:01 PM, Simon Glass wrote:
>>>>> Hi Stephen,
>>>>>
>>>>> On Wed, Nov 28, 2012 at 9:50 AM, Stephen Warren
>>>>> <swarren@wwwdotorg.org>
>>>> wrote:
>>>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>>>
>>>>>> seaboard.h attempts to undefine TEGRA_DEVICE_SETTINGS and provide a
>>>>>> custom value. This worked when the "pre" included tegra20-common.h
>>>>>> provided the default. However, changes in the main U-Boot repo
>>>>>> removed this default from the "pre" included tegra20-common.h to
>>>>>> the
>>>> "post"
>>>>>> included tegra-common-post.h, which uncondtionally provides the value.
>>>>>> This causes the following compile warnings:
>>>>>>
>>>>>> In file included from /home/swarren/shared/git_wa/u-
>>>> boot/include/configs/seaboard.h:129:0,
>>>>>>                  from /home/swarren/shared/git_wa/u-
>>>> boot/include/config.h:10,
>>>>>>                  from /home/swarren/shared/git_wa/u-
>>>> boot/include/common.h:37,
>>>>>>                  from lib/asm-offsets.c:18:
>>>>>> /home/swarren/shared/git_wa/u-boot/include/configs/tegra-common-post.
>>>>>> h:163:0: warning: "TEGRA_DEVICE_SETTINGS" redefined
>>>>>> /home/swarren/shared/git_wa/u-boot/include/configs/seaboard.h:110:0:
>>>>>> note: this is the location of the previous definition
>>>>>>
>>>>>> Solve this by modifying tegra-common-post.h to only provide a value
>>>>>> for TEGRA_DEVICE_SETTINGS if the board-specific header has not
>>>>>> already provided a custom value.
>>>>>>
>>>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>>>
>>>>> These series doesn't apply to u-boot-tegra/master or /next for me,
>>>>> and the last one doesn't seem to apply to u-boot/master either. Can
>>>>> you please take a look, may be a timing issue.
>>>>
>>>> Yes, as I mentioned this problem will only exist once u-boot/master
>>>> and u- boot-arm/master are merged together, so this patch series
>>>> applies to the result of the merge, which will be (at least part of)
>>>> the state of
>>>> u-boot-tegra/* at some unspecified future time:-)
>>>
>>> Allen's TEGRA_DEVICE_SETTINGS patch is already in u-boot/master.
>>
>> Yup.
>>
>>> I applied it to u-boot-tegra/master, then applied your 1/3 & 2/3
>>> patches
>>> (3/3 isn't needed if Allen's patch is already in).
>>
>> Maybe. It depends how Tom Rini does the u-boot-arm/master -> u-boot/master
>> merge; u-boot-arm/master contains a patch that edits TEGRA_DEVICE_SETTINGS
>> in seaboard.h (Simon's to add LCD support), and it's not obvious when
>> merging those two branches that the merge result should be to remove that
>> change from seaboard.h. No actually, that shouldn't be the merge result; if
>> it was, the LCD additions would be removed from TEGRA_DEVICE_SETTINGS
>> without patch 2/3 that I posted. That is, unless patch 2/3 of mine gets into
>> u-boot-arm/master before Albert sends a pull request to Tom Rini...
>>
>> So, I think that once you rebase onto an upstream branch that itself
>> includes all these patches rather than manually applying them, you will
>> probably find patch 3/3 is required. Admittedly it's not if you just apply
>> Allen's patch to the current u-boot-arm/master.
>  
> I'm not anticipating doing a new pull request for u-boot-tegra/master
> any time real soon, so we can wait for upstream (u-boot-arm &
> u-boot/master) to settle a bit. But regardless, Albert has said that
> he's fine with custodians submitting pull requests that contain/depend
> upon patches that are already in an upstream repo, or with the custodian
> stating explicitly which upstream patches are needed for a pull
> request to succeed/build/work.

I believe he said that about git commits (i.e. you can branch from a
commit in u-boot/master just fine) not about patches (i.e. you can't
manually apply a patch that's already upstream, since that will cause
the patch itself to be duplicated in git).

So, if you apply Allen's patch manually to u-boot-tegra/master, you'll
need to rebase u-boot-tegra/master onto u-boot/master or
u-boot-arm/master once Allen's patch is there, to remove the duplicate
patch, rather than just sending a pull request containing a duplicate patch.

BTW, your messages have suddenly stopped being word-wrapped, which makes
quoting/reading them a bit hard.

      reply	other threads:[~2012-11-29 19:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-28 17:50 [U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined Stephen Warren
2012-11-28 17:50 ` [U-Boot] [PATCH 2/3] tegra: add LCD into default TEGRA_DEVICE_SETTINGS Stephen Warren
2012-11-28 17:50 ` [U-Boot] [PATCH 3/3] tegra: seaboard: remove custom TEGRA_DEVICE_SETTINGS Stephen Warren
2012-11-28 21:01 ` [U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined Simon Glass
2012-11-28 21:03   ` Stephen Warren
2012-11-28 21:15     ` Simon Glass
2012-11-29 18:40     ` Tom Warren
2012-11-29 19:10       ` Stephen Warren
2012-11-29 19:50         ` Tom Warren
2012-11-29 19:58           ` Stephen Warren [this message]

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=50B7BE6F.2020602@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 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.