[U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined

[U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined

[Stephen Warren]

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>
---
My personal opinion is that when u-boot-tegra/master is rebuilt, this
series should be the first patches in the branch, and the current patches
that are in u-boot-tegra/next which add LCD support for boards other than
Seaboard should be re-written to assume that tegra-common-post.h will
provide the appropriate value for TEGRA_DEVICE_SETTINGS, and hence the
board config .h files don't need to define any custom value for
TEGRA_DEVICE_SETTINGS.

Otherwise, if you "git bisect" the next version of u-boot-tegra/master,
you'll get the same compile warnings I mention above on all the other
boards until the point when patch 1 in this series is applied.

 include/configs/tegra-common-post.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h
index ab62e71..1662844 100644
--- a/include/configs/tegra-common-post.h
+++ b/include/configs/tegra-common-post.h
@@ -146,6 +146,7 @@
 	"fdt_addr_r=0x02000000\0" \
 	"ramdisk_addr_r=0x02100000\0" \
 
+#ifndef TEGRA_DEVICE_SETTINGS
 #ifdef CONFIG_TEGRA_KEYBOARD
 #define STDIN_KBD_KBC ",tegra-kbc"
 #else
@@ -165,6 +166,8 @@
 	"stdout=serial\0" \
 	"stderr=serial\0" \
 
+#endif /* TEGRA_DEVICE_SETTINGS */
+
 #define CONFIG_EXTRA_ENV_SETTINGS \
 	TEGRA_DEVICE_SETTINGS \
 	MEM_LAYOUT_ENV_SETTINGS \
-- 
1.7.10.4

[U-Boot] [PATCH 2/3] tegra: add LCD into default TEGRA_DEVICE_SETTINGS

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

If LCD support is enabled, ensure that stdout and stderr are sent to the
LCD too.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 include/configs/tegra-common-post.h |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h
index 1662844..c9c49d9 100644
--- a/include/configs/tegra-common-post.h
+++ b/include/configs/tegra-common-post.h
@@ -161,10 +161,16 @@
 #define STDIN_KBD_USB ""
 #endif
 
+#ifdef CONFIG_VIDEO_TEGRA
+#define STDOUT_LCD ",lcd"
+#else
+#define STDOUT_LCD ""
+#endif
+
 #define TEGRA_DEVICE_SETTINGS \
 	"stdin=serial" STDIN_KBD_KBC STDIN_KBD_USB "\0" \
-	"stdout=serial\0" \
-	"stderr=serial\0" \
+	"stdout=serial" STDOUT_LCD "\0" \
+	"stderr=serial" STDOUT_LCD "\0" \
 
 #endif /* TEGRA_DEVICE_SETTINGS */
 
-- 
1.7.10.4

[U-Boot] [PATCH 3/3] tegra: seaboard: remove custom TEGRA_DEVICE_SETTINGS

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Now that tegra-common-post.h includes "lcd" in the default value for
TEGRA_DEVICE_SETTINGS, there is no need for seaboard.h to provide a
custom value.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 include/configs/seaboard.h |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
index 94116f1..de0c777 100644
--- a/include/configs/seaboard.h
+++ b/include/configs/seaboard.h
@@ -106,11 +106,6 @@
 /* USB keyboard */
 #define CONFIG_USB_KEYBOARD
 
-#undef TEGRA_DEVICE_SETTINGS
-#define TEGRA_DEVICE_SETTINGS	"stdin=serial,tegra-kbc\0" \
-				"stdout=serial,lcd\0" \
-				"stderr=serial,lcd\0"
-
 /* LCD support */
 #define CONFIG_LCD
 #define CONFIG_PWM_TEGRA
-- 
1.7.10.4

[U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined

[Simon Glass]

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.

Regards,
Simon

[U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined

[Stephen Warren]

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:-)

[U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined

[Simon Glass]

Hi Stephen,

On Wed, Nov 28, 2012 at 1:03 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> 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:-)

OK so that's what you meant.

For the first two patches, I tested on seaboard:

Tested-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

[U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined

[Tom Warren]

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. 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). Ran a MAKEALL -s tegra20 and that was fine. I then applied the LCD patches that are in u-boot-tegra/next (paz00, etc.), and then I applied Marc's 'tegra: remove custom TEGRA_DEVICE_SETTINGS ...' patch on top of that. Again, MAKEALL was OK. The resulting binaries seem to have the correct settings for stdin,stdout,stderr AFAICT.

I'm going to push a new u-boot-tegra/master to denx.de for you, Marc, Simon, etc. to check out to make sure everything works the way you expect. I'll also do a git bisect through the top half-dozen commits.

Tom
--
nvpublic

[U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined

[Stephen Warren]

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.

[U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined

[Tom Warren]

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. Note that the only patch I'm manually applying per se is Allen's TEGRA_DEVICE_SETTINGS patch that went in to u-boot/master via u-boot-usb via Marek. The rest are true Tegra patches.

Take a look at tegra/master (or /next) and compare to u-boot/master, u-boot-arm, u-boot-usb, etc. if needed and let me know if you still think it's going to run into trouble if/when I do a pull request to arm/master.

Thanks,

Tom

--
nvpublic

[U-Boot] [PATCH 1/3] tegra: only define TEGRA_DEVICE_SETTINGS if not already defined

[Stephen Warren]

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.