[U-Boot] [PATCH v2] env: don't generate callback list entries for SPL

[U-Boot] [PATCH v2] env: don't generate callback list entries for SPL

[Scott Wood]

SPL doesn't write to the environment.  These list entries prevent the
functions from being garbage-collected, even though nothing will look at
the list.  This caused several SPL builds (e.g.  P2020RDB-PC_NAND) to
break due to size limitations and/or unresolved symbols.

A static inline function is used to provide a context in which we
can consume the callback, and thus avoid unused function warnings.

Signed-off-by: Scott Wood <scottwood@freescale.com>
Acked-by: Joe Hershberger <joe.hershberger@gmail.com>
---
v2: Update commit message to reflect that some SPLs do use the
environment, in a read-only fashion.  Also update commit message
to indicate that the size of the included functions wasn't the only
problem seen.
---
 include/env_callback.h |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/env_callback.h b/include/env_callback.h
index 47fdc6f..c583120 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -68,8 +68,16 @@ void env_callback_init(ENTRY *var_entry);
  * when associated through the ".callbacks" environment variable, the callback
  * will be executed any time the variable is inserted, overwritten, or deleted.
  */
+#ifdef CONFIG_SPL_BUILD
+#define U_BOOT_ENV_CALLBACK(name, callback) \
+	static inline void _u_boot_env_noop_##name(void) \
+	{ \
+		(void)callback; \
+	}
+#else
 #define U_BOOT_ENV_CALLBACK(name, callback) \
 	ll_entry_declare(struct env_clbk_tbl, name, env_clbk, env_clbk) = \
 	{#name, callback}
+#endif
 
 #endif /* __ENV_CALLBACK_H__ */
-- 
1.7.9.5

[U-Boot] [PATCH v2] env: don't generate callback list entries for SPL

[Kim Phillips]

On Thu, 20 Dec 2012 15:51:05 -0600
Scott Wood <scottwood@freescale.com> wrote:

> SPL doesn't write to the environment.  These list entries prevent the
> functions from being garbage-collected, even though nothing will look at
> the list.  This caused several SPL builds (e.g.  P2020RDB-PC_NAND) to
> break due to size limitations and/or unresolved symbols.
> 
> A static inline function is used to provide a context in which we
> can consume the callback, and thus avoid unused function warnings.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> Acked-by: Joe Hershberger <joe.hershberger@gmail.com>
> ---

Acked-by: Kim Phillips <kim.phillips@freescale.com>

Thanks,

Kim

[U-Boot] [PATCH v2] env: don't generate callback list entries for SPL

[Tom Rini]

On Thu, Dec 20, 2012 at 04:49:51PM -0600, Kim Phillips wrote:
> On Thu, 20 Dec 2012 15:51:05 -0600
> Scott Wood <scottwood@freescale.com> wrote:
> 
> > SPL doesn't write to the environment.  These list entries prevent the
> > functions from being garbage-collected, even though nothing will look at
> > the list.  This caused several SPL builds (e.g.  P2020RDB-PC_NAND) to
> > break due to size limitations and/or unresolved symbols.
> > 
> > A static inline function is used to provide a context in which we
> > can consume the callback, and thus avoid unused function warnings.
> > 
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > Acked-by: Joe Hershberger <joe.hershberger@gmail.com>
> > ---
> 
> Acked-by: Kim Phillips <kim.phillips@freescale.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121222/584b195d/attachment.pgp>

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Tom Rini]

On Thu, Dec 20, 2012 at 11:51:05AM -0000, Scott Wood wrote:

> SPL doesn't write to the environment.  These list entries prevent the
> functions from being garbage-collected, even though nothing will look at
> the list.  This caused several SPL builds (e.g.  P2020RDB-PC_NAND) to
> break due to size limitations and/or unresolved symbols.
> 
> A static inline function is used to provide a context in which we
> can consume the callback, and thus avoid unused function warnings.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> Acked-by: Joe Hershberger <joe.hershberger@gmail.com>
> Acked-by: Kim Phillips <kim.phillips@freescale.com>

OK, this isn't quite right.
On am335x_evm where SPL does use the "full" version of the environment,
rather than the restricted version that say a3m071 we need these these
callbacks to be generated.  We usually build successfully since in these
cases our #include of <u-boot.lst> picks up the one in include that the
main SPL generates.   But with enough cores we build SPL before we build
this list for non-SPL, and the build fails.  I shall submit a patch
shortly for this.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130308/a6ce8582/attachment.pgp>

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Scott Wood]

On 03/08/2013 02:27:48 PM, Tom Rini wrote:
> On Thu, Dec 20, 2012 at 11:51:05AM -0000, Scott Wood wrote:
> 
> > SPL doesn't write to the environment.  These list entries prevent  
> the
> > functions from being garbage-collected, even though nothing will  
> look at
> > the list.  This caused several SPL builds (e.g.  P2020RDB-PC_NAND)  
> to
> > break due to size limitations and/or unresolved symbols.
> >
> > A static inline function is used to provide a context in which we
> > can consume the callback, and thus avoid unused function warnings.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > Acked-by: Joe Hershberger <joe.hershberger@gmail.com>
> > Acked-by: Kim Phillips <kim.phillips@freescale.com>
> 
> OK, this isn't quite right.
> On am335x_evm where SPL does use the "full" version of the  
> environment,
> rather than the restricted version that say a3m071 we need these these
> callbacks to be generated.  We usually build successfully since in  
> these
> cases our #include of <u-boot.lst> picks up the one in include that  
> the
> main SPL generates.   But with enough cores we build SPL before we  
> build
> this list for non-SPL, and the build fails.  I shall submit a patch
> shortly for this.

What does am335x_evm do in the SPL that requires modifying the  
environment, and how does omitting the callbacks cause a build break?

The u-boot.lst issue sounds unrelated to this patch.

-Scott

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/08/2013 03:34 PM, Scott Wood wrote:
> On 03/08/2013 02:27:48 PM, Tom Rini wrote:
>> On Thu, Dec 20, 2012 at 11:51:05AM -0000, Scott Wood wrote:
>> 
>>> SPL doesn't write to the environment.  These list entries 
>>> prevent the functions from being garbage-collected, even
>>> though nothing will
>> look at
>>> the list.  This caused several SPL builds (e.g. 
>>> P2020RDB-PC_NAND) to break due to size limitations and/or 
>>> unresolved symbols.
>>> 
>>> A static inline function is used to provide a context in which 
>>> we can consume the callback, and thus avoid unused function 
>>> warnings.
>>> 
>>> Signed-off-by: Scott Wood <scottwood@freescale.com> Acked-by: 
>>> Joe Hershberger <joe.hershberger@gmail.com> Acked-by: Kim 
>>> Phillips <kim.phillips@freescale.com>
>> 
>> OK, this isn't quite right. On am335x_evm where SPL does use the 
>> "full" version of the environment, rather than the restricted 
>> version that say a3m071 we need these these callbacks to be 
>> generated.  We usually build successfully since in these cases 
>> our #include of <u-boot.lst> picks up the one in include that the
>> main SPL generates.   But with enough cores we build SPL before
>> we build this list for non-SPL, and the build fails.  I shall
>> submit a patch shortly for this.
> 
> What does am335x_evm do in the SPL that requires modifying the 
> environment, and how does omitting the callbacks cause a build 
> break?

It requires the full network stack which in turn means we can't just
discard most of the environment related functions.  And some parts of
the callback infrastructure aren't opt-in'able.

> The u-boot.lst issue sounds unrelated to this patch.

The problem is undefined references at link time to
_u_boot_env_clbk__start/__end from common/env_callbacks.c where it
tries to look at our empty list of callbacks (we are able to discard
all of those).  And part of the issue is that we're always using the
wrong u-boot.lst file for SPL.  It's just that in most cases the wrong
one (the main U-Boot one) is also fine enough for SPL since it's just
a few extra symbols and we aren't so constrained for space that we've
noticed.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJROlFDAAoJENk4IS6UOR1WEvwQAJtEjtnUs/Zu38rQ5oFV/W2V
a/MX3hTmFF1bhJ9oiP52j1liB/V+hLCAhzveGllWLnvICFvv6F5nFrDwhhlUIRkX
ZC99JvkYFHbQexnpwWqwR35reXsQWIhISLN9yeKtQ3GF0b9nztIXARLwpcLRJp95
fctdiTPrWi/jqLsmX63BkQyhbfpc5ltgDu1kn7PQSgzDt49OJUlqvMrMGM9xY98h
wfKkuUE8Ez4Xq9rsTwa21oECEcgNRuAbsHyePMyuNeVuSfyjwin4d8y3xK7H0Kp5
Z12tnSNet4G4e06Hz+NRdpEa6DqSbyvQpX8rhctEYBxEaA/VpGUIFrrUOEa8jdDh
J49/rF8xhYBTeKfpbYrfCS5iQUKnuJxGW7+2PYH8+y1RxQdLkkhuJdQVoAJ4cNsL
HP3AeoI7Urc7QYVINyBNCG8Ak6/skDI9Ia6eFqJbNm1X0jDnvV6wh1JoK0lxgJio
Uw2NdFIlIHUMHAGMW8R/Zj1eCqaLkwWtTRPqgENPwcOtTdaf/Y/57R2yYRDvz52l
Dl9AikeDu8tr0y431b3WJo5aXi5XXAeQiEJzWj48nExSv29fh48gc3IEVO6+Xnnp
2tQbUarWwoptp1QALmuMJlPeIdfGNg1/AoQeise8WtoS0GSl868QG+NJVpmKhEl+
GYiBfxNk+pGlJZmRjHhQ
=Bx5L
-----END PGP SIGNATURE-----

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Scott Wood]

On 03/08/2013 02:59:47 PM, Tom Rini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 03/08/2013 03:34 PM, Scott Wood wrote:
> > On 03/08/2013 02:27:48 PM, Tom Rini wrote:
> >> On Thu, Dec 20, 2012 at 11:51:05AM -0000, Scott Wood wrote:
> >>
> >>> SPL doesn't write to the environment.  These list entries
> >>> prevent the functions from being garbage-collected, even
> >>> though nothing will
> >> look at
> >>> the list.  This caused several SPL builds (e.g.
> >>> P2020RDB-PC_NAND) to break due to size limitations and/or
> >>> unresolved symbols.
> >>>
> >>> A static inline function is used to provide a context in which
> >>> we can consume the callback, and thus avoid unused function
> >>> warnings.
> >>>
> >>> Signed-off-by: Scott Wood <scottwood@freescale.com> Acked-by:
> >>> Joe Hershberger <joe.hershberger@gmail.com> Acked-by: Kim
> >>> Phillips <kim.phillips@freescale.com>
> >>
> >> OK, this isn't quite right. On am335x_evm where SPL does use the
> >> "full" version of the environment, rather than the restricted
> >> version that say a3m071 we need these these callbacks to be
> >> generated.  We usually build successfully since in these cases
> >> our #include of <u-boot.lst> picks up the one in include that the
> >> main SPL generates.   But with enough cores we build SPL before
> >> we build this list for non-SPL, and the build fails.  I shall
> >> submit a patch shortly for this.
> >
> > What does am335x_evm do in the SPL that requires modifying the
> > environment, and how does omitting the callbacks cause a build
> > break?
> 
> It requires the full network stack which in turn means we can't just
> discard most of the environment related functions.  And some parts of
> the callback infrastructure aren't opt-in'able.

I still don't follow -- the only effect of this patch should be that  
the callbacks don't get called, which is only relevant when writing to  
the environment.  We're not ripping out anything, just declining to  
reference the callback functions.  If something else still references  
them, they'll be retained.

> > The u-boot.lst issue sounds unrelated to this patch.
> 
> The problem is undefined references at link time to
> _u_boot_env_clbk__start/__end from common/env_callbacks.c where it
> tries to look at our empty list of callbacks (we are able to discard
> all of those).

Why would eliminating all individual callbacks cause start/end to go  
away?  If that's the way the list mechanism works, the mechanism needs  
fixing.

> And part of the issue is that we're always using the
> wrong u-boot.lst file for SPL.  It's just that in most cases the wrong
> one (the main U-Boot one) is also fine enough for SPL since it's just
> a few extra symbols and we aren't so constrained for space that we've
> noticed.

That sounds familiar: a6d0f62a0ccac7669b1efe320e28c276b1b8084b
:-)

-Scott

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Tom Rini]

On Fri, Mar 08, 2013 at 06:35:04PM -0600, Scott Wood wrote:
> On 03/08/2013 02:59:47 PM, Tom Rini wrote:
> >-----BEGIN PGP SIGNED MESSAGE-----
> >Hash: SHA1
> >
> >On 03/08/2013 03:34 PM, Scott Wood wrote:
> >> On 03/08/2013 02:27:48 PM, Tom Rini wrote:
> >>> On Thu, Dec 20, 2012 at 11:51:05AM -0000, Scott Wood wrote:
> >>>
> >>>> SPL doesn't write to the environment.  These list entries
> >>>> prevent the functions from being garbage-collected, even
> >>>> though nothing will
> >>> look at
> >>>> the list.  This caused several SPL builds (e.g.
> >>>> P2020RDB-PC_NAND) to break due to size limitations and/or
> >>>> unresolved symbols.
> >>>>
> >>>> A static inline function is used to provide a context in which
> >>>> we can consume the callback, and thus avoid unused function
> >>>> warnings.
> >>>>
> >>>> Signed-off-by: Scott Wood <scottwood@freescale.com> Acked-by:
> >>>> Joe Hershberger <joe.hershberger@gmail.com> Acked-by: Kim
> >>>> Phillips <kim.phillips@freescale.com>
> >>>
> >>> OK, this isn't quite right. On am335x_evm where SPL does use the
> >>> "full" version of the environment, rather than the restricted
> >>> version that say a3m071 we need these these callbacks to be
> >>> generated.  We usually build successfully since in these cases
> >>> our #include of <u-boot.lst> picks up the one in include that the
> >>> main SPL generates.   But with enough cores we build SPL before
> >>> we build this list for non-SPL, and the build fails.  I shall
> >>> submit a patch shortly for this.
> >>
> >> What does am335x_evm do in the SPL that requires modifying the
> >> environment, and how does omitting the callbacks cause a build
> >> break?
> >
> >It requires the full network stack which in turn means we can't just
> >discard most of the environment related functions.  And some parts of
> >the callback infrastructure aren't opt-in'able.
> 
> I still don't follow -- the only effect of this patch should be that
> the callbacks don't get called, which is only relevant when writing
> to the environment.  We're not ripping out anything, just declining
> to reference the callback functions.  If something else still
> references them, they'll be retained.

It's not that they don't get called, it's that they don't get put into
the special section.

> >> The u-boot.lst issue sounds unrelated to this patch.
> >
> >The problem is undefined references at link time to
> >_u_boot_env_clbk__start/__end from common/env_callbacks.c where it
> >tries to look at our empty list of callbacks (we are able to discard
> >all of those).
> 
> Why would eliminating all individual callbacks cause start/end to go
> away?  If that's the way the list mechanism works, the mechanism
> needs fixing.

Yes, that's how the mechanism works.  Rather than having to declare that
you expect to have a linker list of name $foo, we dynamically determine
what linker lists we have and setup the linker section entry.  I'm not
sure it's broken exactly, I think maybe we just need to say no env
callback support in SPL since it's not really user editable.

I've got an idea that I'm going to go test now and then if it works,
post patches for.

> >And part of the issue is that we're always using the
> >wrong u-boot.lst file for SPL.  It's just that in most cases the wrong
> >one (the main U-Boot one) is also fine enough for SPL since it's just
> >a few extra symbols and we aren't so constrained for space that we've
> >noticed.
> 
> That sounds familiar: a6d0f62a0ccac7669b1efe320e28c276b1b8084b
> :-)

Not quite the same problem.  This one shows up with separate obj
directories too.  It really is that with a small enough job number we
generate include/u-boot.lst before trying to link u-boot-spl, and that is
used, and gives us the start/end symbols (at the same address as we've
discarded the callbacks themselves).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130312/ae466f08/attachment.pgp>

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Scott Wood]

On 03/12/2013 10:30:40 AM, Tom Rini wrote:
> On Fri, Mar 08, 2013 at 06:35:04PM -0600, Scott Wood wrote:
> > Why would eliminating all individual callbacks cause start/end to go
> > away?  If that's the way the list mechanism works, the mechanism
> > needs fixing.
> 
> Yes, that's how the mechanism works.  Rather than having to declare  
> that
> you expect to have a linker list of name $foo, we dynamically  
> determine
> what linker lists we have and setup the linker section entry.

So it would break just as hard if we happened to turn off all of the  
things that register callbacks.

> I'm not sure it's broken exactly, I think maybe we just need to say  
> no env
> callback support in SPL since it's not really user editable.

That's fine, but it's still a bad mechanism.

-SCott

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Tom Rini]

On Tue, Mar 12, 2013 at 11:55:22AM -0500, Scott Wood wrote:
> On 03/12/2013 10:30:40 AM, Tom Rini wrote:
> >On Fri, Mar 08, 2013 at 06:35:04PM -0600, Scott Wood wrote:
> >> Why would eliminating all individual callbacks cause start/end to go
> >> away?  If that's the way the list mechanism works, the mechanism
> >> needs fixing.
> >
> >Yes, that's how the mechanism works.  Rather than having to
> >declare that
> >you expect to have a linker list of name $foo, we dynamically
> >determine
> >what linker lists we have and setup the linker section entry.
> 
> So it would break just as hard if we happened to turn off all of the
> things that register callbacks.
> 
> >I'm not sure it's broken exactly, I think maybe we just need to
> >say no env
> >callback support in SPL since it's not really user editable.
> 
> That's fine, but it's still a bad mechanism.

Yes, the mechanism has a breaking condition on trying to reference an
empty list (which is what SPL ends up with, in this case).  Poking
Albert and Marek in case they have any ideas, but this seems like a
feature not a bug.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130312/c453fed1/attachment.pgp>

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Scott Wood]

On 03/12/2013 12:02:56 PM, Tom Rini wrote:
> On Tue, Mar 12, 2013 at 11:55:22AM -0500, Scott Wood wrote:
> > On 03/12/2013 10:30:40 AM, Tom Rini wrote:
> > >On Fri, Mar 08, 2013 at 06:35:04PM -0600, Scott Wood wrote:
> > >> Why would eliminating all individual callbacks cause start/end  
> to go
> > >> away?  If that's the way the list mechanism works, the mechanism
> > >> needs fixing.
> > >
> > >Yes, that's how the mechanism works.  Rather than having to
> > >declare that
> > >you expect to have a linker list of name $foo, we dynamically
> > >determine
> > >what linker lists we have and setup the linker section entry.
> >
> > So it would break just as hard if we happened to turn off all of the
> > things that register callbacks.
> >
> > >I'm not sure it's broken exactly, I think maybe we just need to
> > >say no env
> > >callback support in SPL since it's not really user editable.
> >
> > That's fine, but it's still a bad mechanism.
> 
> Yes, the mechanism has a breaking condition on trying to reference an
> empty list (which is what SPL ends up with, in this case).  Poking
> Albert and Marek in case they have any ideas, but this seems like a
> feature not a bug.

How is it a feature?  One of the main benefit of linker lists is for  
things to just work when things are configured in/out without needing  
ifdefs and such.  Why should "everything configured out" be a special  
case requiring an ifdef?

If we want to save some code by ifdeffing the listwalking code for SPL,  
that's a separate matter.

-Scott

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Albert ARIBAUD]

Hi Scott,

On Tue, 12 Mar 2013 12:06:53 -0500, Scott Wood
<scottwood@freescale.com> wrote:

> On 03/12/2013 12:02:56 PM, Tom Rini wrote:
> > On Tue, Mar 12, 2013 at 11:55:22AM -0500, Scott Wood wrote:
> > > On 03/12/2013 10:30:40 AM, Tom Rini wrote:
> > > >On Fri, Mar 08, 2013 at 06:35:04PM -0600, Scott Wood wrote:
> > > >> Why would eliminating all individual callbacks cause start/end  
> > to go
> > > >> away?  If that's the way the list mechanism works, the mechanism
> > > >> needs fixing.
> > > >
> > > >Yes, that's how the mechanism works.  Rather than having to
> > > >declare that
> > > >you expect to have a linker list of name $foo, we dynamically
> > > >determine
> > > >what linker lists we have and setup the linker section entry.
> > >
> > > So it would break just as hard if we happened to turn off all of the
> > > things that register callbacks.
> > >
> > > >I'm not sure it's broken exactly, I think maybe we just need to
> > > >say no env
> > > >callback support in SPL since it's not really user editable.
> > >
> > > That's fine, but it's still a bad mechanism.
> > 
> > Yes, the mechanism has a breaking condition on trying to reference an
> > empty list (which is what SPL ends up with, in this case).  Poking
> > Albert and Marek in case they have any ideas, but this seems like a
> > feature not a bug.
> 
> How is it a feature?  One of the main benefit of linker lists is for  
> things to just work when things are configured in/out without needing  
> ifdefs and such.  Why should "everything configured out" be a special  
> case requiring an ifdef?
> 
> If we want to save some code by ifdeffing the listwalking code for SPL,  
> that's a separate matter.

Normally my reworked linker_list code should work fine with some code
going through an empty list, precisely because list start and end
symbols, like list entries, are generated by the compiler irrespective
of one another and then whatever was generated is sorted by the linker.
So an empty list ends up as two symbols at the same address (and
indeed, env callbacks, in many boards, showed this pattern in the map
file).

> -Scott

Amicalement,
-- 
Albert.

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/12/2013 01:19 PM, Albert ARIBAUD wrote:
> Hi Scott,
> 
> On Tue, 12 Mar 2013 12:06:53 -0500, Scott Wood 
> <scottwood@freescale.com> wrote:
> 
>> On 03/12/2013 12:02:56 PM, Tom Rini wrote:
>>> On Tue, Mar 12, 2013 at 11:55:22AM -0500, Scott Wood wrote:
>>>> On 03/12/2013 10:30:40 AM, Tom Rini wrote:
>>>>> On Fri, Mar 08, 2013 at 06:35:04PM -0600, Scott Wood 
>>>>> wrote:
>>>>>> Why would eliminating all individual callbacks cause 
>>>>>> start/end
>>> to go
>>>>>> away?  If that's the way the list mechanism works, the 
>>>>>> mechanism needs fixing.
>>>>> 
>>>>> Yes, that's how the mechanism works.  Rather than having to
>>>>> declare that you expect to have a linker list of name $foo,
>>>>> we dynamically determine what linker lists we have and
>>>>> setup the linker section entry.
>>>> 
>>>> So it would break just as hard if we happened to turn off
>>>> all of the things that register callbacks.
>>>> 
>>>>> I'm not sure it's broken exactly, I think maybe we just 
>>>>> need to say no env callback support in SPL since it's not 
>>>>> really user editable.
>>>> 
>>>> That's fine, but it's still a bad mechanism.
>>> 
>>> Yes, the mechanism has a breaking condition on trying to 
>>> reference an empty list (which is what SPL ends up with, in 
>>> this case).  Poking Albert and Marek in case they have any 
>>> ideas, but this seems like a feature not a bug.
>> 
>> How is it a feature?  One of the main benefit of linker lists is 
>> for things to just work when things are configured in/out
>> without needing ifdefs and such.  Why should "everything
>> configured out" be a special case requiring an ifdef?
>> 
>> If we want to save some code by ifdeffing the listwalking code 
>> for SPL, that's a separate matter.
> 
> Normally my reworked linker_list code should work fine with some 
> code going through an empty list, precisely because list start and 
> end symbols, like list entries, are generated by the compiler 
> irrespective of one another and then whatever was generated is 
> sorted by the linker. So an empty list ends up as two symbols at 
> the same address (and indeed, env callbacks, in many boards,
> showed this pattern in the map file).

OK, where's the latest/greatest of this re-work?  I'll see if it also
solves the problem I have.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRP2o0AAoJENk4IS6UOR1WbE0QAKZDcVBQZYlWtqSOExpuYBEk
mfCiRw9kyCWDQSsZo9yfqEi2CkPoexZWphqNjI0oCsAemGfh2UeK1Foqlbb3oAS5
A/R2p1zd5eNZbFx9SfUlMr09FhaYwOe1O7PQcHohsCnzU/8yXXAHGe+kz5HePtpU
3R6wZNUjcA7wXGex8o4ygvI8GUctZgDT95hlrdNihrD+TkwQdjmMG3XR2ifz/LrM
I5VXq/9mT/UMvWvrtbpeLGd4VGwYZywrHBAkI0s1GjxQsjGoFfkA1HmZUdS2Hf6x
BzniSGWYypnecWQPhbxetQaRmxUgGolbK2G+JOM5xDHVqUgZJ2zuEeGR9BdBqVGx
slhrI7FNTy2vRmlcYTMoma0q5+gEh+0/YvACXSDNrhySB5y/9Q/pCYFQisvX2ohA
n6IxTGiiQ3SYwvYeLGSx6OLneDzM08IV0nixY0lbPrWKndlPP6lkO+IwjotYcynO
P8fLjXzcTLtU30VkTKchOYt+M6jqMz8eiPgsfifS5CrEll/fPCa9m+ri1/w7O5ZI
zuHN32DlJzdj8DZxoyRsyvED0pUHU1ji4ECzk22ZJ+fcm2uZgLlRvZmzNwIW7zzk
Xmopl2/OCTPl9BDahNxeZCE8Tg6prgBclKQgnLi2hargxPI2L1GUepm0Xm6gdz1H
+IXXBJUL4VGugf8IZPKR
=VwGC
-----END PGP SIGNATURE-----

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Albert ARIBAUD]

Hi Tom,

On Tue, 12 Mar 2013 13:47:33 -0400, Tom Rini <trini@ti.com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 03/12/2013 01:19 PM, Albert ARIBAUD wrote:
> > Hi Scott,
> > 
> > On Tue, 12 Mar 2013 12:06:53 -0500, Scott Wood 
> > <scottwood@freescale.com> wrote:
> > 
> >> On 03/12/2013 12:02:56 PM, Tom Rini wrote:
> >>> On Tue, Mar 12, 2013 at 11:55:22AM -0500, Scott Wood wrote:
> >>>> On 03/12/2013 10:30:40 AM, Tom Rini wrote:
> >>>>> On Fri, Mar 08, 2013 at 06:35:04PM -0600, Scott Wood 
> >>>>> wrote:
> >>>>>> Why would eliminating all individual callbacks cause 
> >>>>>> start/end
> >>> to go
> >>>>>> away?  If that's the way the list mechanism works, the 
> >>>>>> mechanism needs fixing.
> >>>>> 
> >>>>> Yes, that's how the mechanism works.  Rather than having to
> >>>>> declare that you expect to have a linker list of name $foo,
> >>>>> we dynamically determine what linker lists we have and
> >>>>> setup the linker section entry.
> >>>> 
> >>>> So it would break just as hard if we happened to turn off
> >>>> all of the things that register callbacks.
> >>>> 
> >>>>> I'm not sure it's broken exactly, I think maybe we just 
> >>>>> need to say no env callback support in SPL since it's not 
> >>>>> really user editable.
> >>>> 
> >>>> That's fine, but it's still a bad mechanism.
> >>> 
> >>> Yes, the mechanism has a breaking condition on trying to 
> >>> reference an empty list (which is what SPL ends up with, in 
> >>> this case).  Poking Albert and Marek in case they have any 
> >>> ideas, but this seems like a feature not a bug.
> >> 
> >> How is it a feature?  One of the main benefit of linker lists is 
> >> for things to just work when things are configured in/out
> >> without needing ifdefs and such.  Why should "everything
> >> configured out" be a special case requiring an ifdef?
> >> 
> >> If we want to save some code by ifdeffing the listwalking code 
> >> for SPL, that's a separate matter.
> > 
> > Normally my reworked linker_list code should work fine with some 
> > code going through an empty list, precisely because list start and 
> > end symbols, like list entries, are generated by the compiler 
> > irrespective of one another and then whatever was generated is 
> > sorted by the linker. So an empty list ends up as two symbols at 
> > the same address (and indeed, env callbacks, in many boards,
> > showed this pattern in the map file).
> 
> OK, where's the latest/greatest of this re-work?  I'll see if it also
> solves the problem I have.

In patchwork (assigned to me, and soon to be applied on u-boot-arm if
this works for you):

http://patchwork.ozlabs.org/patch/222904/
http://patchwork.ozlabs.org/patch/222905/
http://patchwork.ozlabs.org/patch/222906/
http://patchwork.ozlabs.org/patch/222908/

(http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/152754/focus=154634)

Amicalement,
-- 
Albert.

[U-Boot] [U-Boot, v2] env: don't generate callback list entries for SPL

[Tom Rini]

On Tue, Mar 12, 2013 at 11:07:46PM +0100, Albert ARIBAUD wrote:
> Hi Tom,
> 
> On Tue, 12 Mar 2013 13:47:33 -0400, Tom Rini <trini@ti.com> wrote:
> 
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > On 03/12/2013 01:19 PM, Albert ARIBAUD wrote:
> > > Hi Scott,
> > > 
> > > On Tue, 12 Mar 2013 12:06:53 -0500, Scott Wood 
> > > <scottwood@freescale.com> wrote:
> > > 
> > >> On 03/12/2013 12:02:56 PM, Tom Rini wrote:
> > >>> On Tue, Mar 12, 2013 at 11:55:22AM -0500, Scott Wood wrote:
> > >>>> On 03/12/2013 10:30:40 AM, Tom Rini wrote:
> > >>>>> On Fri, Mar 08, 2013 at 06:35:04PM -0600, Scott Wood 
> > >>>>> wrote:
> > >>>>>> Why would eliminating all individual callbacks cause 
> > >>>>>> start/end
> > >>> to go
> > >>>>>> away?  If that's the way the list mechanism works, the 
> > >>>>>> mechanism needs fixing.
> > >>>>> 
> > >>>>> Yes, that's how the mechanism works.  Rather than having to
> > >>>>> declare that you expect to have a linker list of name $foo,
> > >>>>> we dynamically determine what linker lists we have and
> > >>>>> setup the linker section entry.
> > >>>> 
> > >>>> So it would break just as hard if we happened to turn off
> > >>>> all of the things that register callbacks.
> > >>>> 
> > >>>>> I'm not sure it's broken exactly, I think maybe we just 
> > >>>>> need to say no env callback support in SPL since it's not 
> > >>>>> really user editable.
> > >>>> 
> > >>>> That's fine, but it's still a bad mechanism.
> > >>> 
> > >>> Yes, the mechanism has a breaking condition on trying to 
> > >>> reference an empty list (which is what SPL ends up with, in 
> > >>> this case).  Poking Albert and Marek in case they have any 
> > >>> ideas, but this seems like a feature not a bug.
> > >> 
> > >> How is it a feature?  One of the main benefit of linker lists is 
> > >> for things to just work when things are configured in/out
> > >> without needing ifdefs and such.  Why should "everything
> > >> configured out" be a special case requiring an ifdef?
> > >> 
> > >> If we want to save some code by ifdeffing the listwalking code 
> > >> for SPL, that's a separate matter.
> > > 
> > > Normally my reworked linker_list code should work fine with some 
> > > code going through an empty list, precisely because list start and 
> > > end symbols, like list entries, are generated by the compiler 
> > > irrespective of one another and then whatever was generated is 
> > > sorted by the linker. So an empty list ends up as two symbols at 
> > > the same address (and indeed, env callbacks, in many boards,
> > > showed this pattern in the map file).
> > 
> > OK, where's the latest/greatest of this re-work?  I'll see if it also
> > solves the problem I have.
> 
> In patchwork (assigned to me, and soon to be applied on u-boot-arm if
> this works for you):
> 
> http://patchwork.ozlabs.org/patch/222904/
> http://patchwork.ozlabs.org/patch/222905/
> http://patchwork.ozlabs.org/patch/222906/
> http://patchwork.ozlabs.org/patch/222908/
> 
> (http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/152754/focus=154634)

For clarity over here, fixed the problem I had so I'll drop 2/2 from
what I posted (1/2 is still applicable as a general thing).  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130313/74df18ce/attachment.pgp>