Re: [U-Boot] [U-Boot, V4, 10/13] spl: add support for alternative boot device

[Tom Rini]

On Thu, Nov 19, 2015 at 12:46:58PM +0100, Stefan Roese wrote:
> On 19.11.2015 12:19, Nikita Kiryanov wrote:
> >Hi Tom,
> >
> >On Wed, Nov 18, 2015 at 05:33:20PM -0500, Tom Rini wrote:
> >>On Sun, Nov 08, 2015 at 05:11:51PM +0200, Nikita Kiryanov wrote:
> >>
> >>>Introduce spl_boot_list array, which defines a list of boot devices
> >>>that SPL will try before hanging. By default this list will consist
> >>>of only spl_boot_device(), but board_boot_order() can be overridden
> >>>by board code to populate the array with custom values.
> >>>
> >>>Signed-off-by: Nikita Kiryanov 
> >>>Cc: Igor Grinberg 
> >>>Cc: Tom Rini 
> >>>Cc: Simon Glass 
> >>>Reviewed-by: Tom Rini 
> >>>Reviewed-by: Simon Glass 
> >>
> >>So, a problem with this patch is that we push the x600 board, which is
> >>an 8KiB SPL target, over the line.  I feel like maybe we need a
> >>follow-up patch that makes announcing depend not on libcommon (which
> >>x600 needs) but something else to know that there's a reason to
> >>announce.
> >
> >Based on the content of your reply I'm guessing you're referring to the
> >next patch, not this one.
> >
> >I suppose that announcing can be made into an optional feature. However,
> >I also think that since printing is an optional feature that can greatly
> >increase binary size, it shouldn't be coupled with other, often
> >non-optional libcommon features the way it currently is via
> >CONFIG_SPL_LIBCOMMON_SUPPORT. The best fix in my opinion would be to
> >implement a way to exclude printing support from SPL even if libcommon
> >is included (CONFIG_SPL_SILENT that replaces printfs with empty stubs?).
> >
> >This will also make it possible to remove all those #ifdef
> >CONFIG_SPL_LIBCOMMON_SUPPORT checks that appear all over the SPL code.
> 
> I think that my recently posted tiny-printf patches:
> 
> https://patchwork.ozlabs.org/patch/545034/
> https://patchwork.ozlabs.org/patch/545033/
> https://patchwork.ozlabs.org/patch/545036/
> https://patchwork.ozlabs.org/patch/545035/
> 
> can solve this size issue on x600 (and perhaps other) board.

If you can see if x600 builds again in mainline that would be good :)

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [U-Boot, V4, 10/13] spl: add support for alternative boot device

[Tom Rini]

On Thu, Nov 19, 2015 at 01:19:39PM +0200, Nikita Kiryanov wrote:
> Hi Tom,
> 
> On Wed, Nov 18, 2015 at 05:33:20PM -0500, Tom Rini wrote:
> > On Sun, Nov 08, 2015 at 05:11:51PM +0200, Nikita Kiryanov wrote:
> > 
> > > Introduce spl_boot_list array, which defines a list of boot devices
> > > that SPL will try before hanging. By default this list will consist
> > > of only spl_boot_device(), but board_boot_order() can be overridden
> > > by board code to populate the array with custom values.
> > > 
> > > Signed-off-by: Nikita Kiryanov 
> > > Cc: Igor Grinberg 
> > > Cc: Tom Rini 
> > > Cc: Simon Glass 
> > > Reviewed-by: Tom Rini 
> > > Reviewed-by: Simon Glass 
> > 
> > So, a problem with this patch is that we push the x600 board, which is
> > an 8KiB SPL target, over the line.  I feel like maybe we need a
> > follow-up patch that makes announcing depend not on libcommon (which
> > x600 needs) but something else to know that there's a reason to
> > announce.
> 
> Based on the content of your reply I'm guessing you're referring to the
> next patch, not this one.

Oh yes, oops.

> I suppose that announcing can be made into an optional feature. However,
> I also think that since printing is an optional feature that can greatly
> increase binary size, it shouldn't be coupled with other, often
> non-optional libcommon features the way it currently is via
> CONFIG_SPL_LIBCOMMON_SUPPORT. The best fix in my opinion would be to
> implement a way to exclude printing support from SPL even if libcommon
> is included (CONFIG_SPL_SILENT that replaces printfs with empty stubs?).
> 
> This will also make it possible to remove all those #ifdef
> CONFIG_SPL_LIBCOMMON_SUPPORT checks that appear all over the SPL code.

So the reason I'm thinking that we want this announce thing separate is
that (due to the way you re-worked it I think, based on Simons' request)
this added a huge amount of space.  We went from OK to overflowing by
more than 1KiB.  So I'm not immediately sure that we can regain that
space with a more (space) efficient printing infrastructure.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [U-Boot, V4, 10/13] spl: add support for alternative boot device

[Nikita Kiryanov]

Hi Tom,

On Wed, Nov 18, 2015 at 05:33:20PM -0500, Tom Rini wrote:
> On Sun, Nov 08, 2015 at 05:11:51PM +0200, Nikita Kiryanov wrote:
> 
> > Introduce spl_boot_list array, which defines a list of boot devices
> > that SPL will try before hanging. By default this list will consist
> > of only spl_boot_device(), but board_boot_order() can be overridden
> > by board code to populate the array with custom values.
> > 
> > Signed-off-by: Nikita Kiryanov 
> > Cc: Igor Grinberg 
> > Cc: Tom Rini 
> > Cc: Simon Glass 
> > Reviewed-by: Tom Rini 
> > Reviewed-by: Simon Glass 
> 
> So, a problem with this patch is that we push the x600 board, which is
> an 8KiB SPL target, over the line.  I feel like maybe we need a
> follow-up patch that makes announcing depend not on libcommon (which
> x600 needs) but something else to know that there's a reason to
> announce.

Based on the content of your reply I'm guessing you're referring to the
next patch, not this one.

I suppose that announcing can be made into an optional feature. However,
I also think that since printing is an optional feature that can greatly
increase binary size, it shouldn't be coupled with other, often
non-optional libcommon features the way it currently is via
CONFIG_SPL_LIBCOMMON_SUPPORT. The best fix in my opinion would be to
implement a way to exclude printing support from SPL even if libcommon
is included (CONFIG_SPL_SILENT that replaces printfs with empty stubs?).

This will also make it possible to remove all those #ifdef
CONFIG_SPL_LIBCOMMON_SUPPORT checks that appear all over the SPL code.

> 
> -- 
> Tom


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [U-Boot, V4, 10/13] spl: add support for alternative boot device

[Stefan Roese]

On 19.11.2015 12:19, Nikita Kiryanov wrote:

Hi Tom,

On Wed, Nov 18, 2015 at 05:33:20PM -0500, Tom Rini wrote:

On Sun, Nov 08, 2015 at 05:11:51PM +0200, Nikita Kiryanov wrote:


Introduce spl_boot_list array, which defines a list of boot devices
that SPL will try before hanging. By default this list will consist
of only spl_boot_device(), but board_boot_order() can be overridden
by board code to populate the array with custom values.

Signed-off-by: Nikita Kiryanov 
Cc: Igor Grinberg 
Cc: Tom Rini 
Cc: Simon Glass 
Reviewed-by: Tom Rini 
Reviewed-by: Simon Glass 


So, a problem with this patch is that we push the x600 board, which is
an 8KiB SPL target, over the line.  I feel like maybe we need a
follow-up patch that makes announcing depend not on libcommon (which
x600 needs) but something else to know that there's a reason to
announce.


Based on the content of your reply I'm guessing you're referring to the
next patch, not this one.

I suppose that announcing can be made into an optional feature. However,
I also think that since printing is an optional feature that can greatly
increase binary size, it shouldn't be coupled with other, often
non-optional libcommon features the way it currently is via
CONFIG_SPL_LIBCOMMON_SUPPORT. The best fix in my opinion would be to
implement a way to exclude printing support from SPL even if libcommon
is included (CONFIG_SPL_SILENT that replaces printfs with empty stubs?).

This will also make it possible to remove all those #ifdef
CONFIG_SPL_LIBCOMMON_SUPPORT checks that appear all over the SPL code.


I think that my recently posted tiny-printf patches:

https://patchwork.ozlabs.org/patch/545034/
https://patchwork.ozlabs.org/patch/545033/
https://patchwork.ozlabs.org/patch/545036/
https://patchwork.ozlabs.org/patch/545035/

can solve this size issue on x600 (and perhaps other) board.

Comments welcome...

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [U-Boot, V4, 10/13] spl: add support for alternative boot device

[Stefan Roese]

Hi Tom,

On 19.11.2015 23:11, Tom Rini wrote:

On Thu, Nov 19, 2015 at 12:46:58PM +0100, Stefan Roese wrote:

On 19.11.2015 12:19, Nikita Kiryanov wrote:

Hi Tom,

On Wed, Nov 18, 2015 at 05:33:20PM -0500, Tom Rini wrote:

On Sun, Nov 08, 2015 at 05:11:51PM +0200, Nikita Kiryanov wrote:


Introduce spl_boot_list array, which defines a list of boot devices
that SPL will try before hanging. By default this list will consist
of only spl_boot_device(), but board_boot_order() can be overridden
by board code to populate the array with custom values.

Signed-off-by: Nikita Kiryanov 
Cc: Igor Grinberg 
Cc: Tom Rini 
Cc: Simon Glass 
Reviewed-by: Tom Rini 
Reviewed-by: Simon Glass 


So, a problem with this patch is that we push the x600 board, which is
an 8KiB SPL target, over the line.  I feel like maybe we need a
follow-up patch that makes announcing depend not on libcommon (which
x600 needs) but something else to know that there's a reason to
announce.


Based on the content of your reply I'm guessing you're referring to the
next patch, not this one.

I suppose that announcing can be made into an optional feature. However,
I also think that since printing is an optional feature that can greatly
increase binary size, it shouldn't be coupled with other, often
non-optional libcommon features the way it currently is via
CONFIG_SPL_LIBCOMMON_SUPPORT. The best fix in my opinion would be to
implement a way to exclude printing support from SPL even if libcommon
is included (CONFIG_SPL_SILENT that replaces printfs with empty stubs?).

This will also make it possible to remove all those #ifdef
CONFIG_SPL_LIBCOMMON_SUPPORT checks that appear all over the SPL code.


I think that my recently posted tiny-printf patches:

https://patchwork.ozlabs.org/patch/545034/
https://patchwork.ozlabs.org/patch/545033/
https://patchwork.ozlabs.org/patch/545036/
https://patchwork.ozlabs.org/patch/545035/

can solve this size issue on x600 (and perhaps other) board.


If you can see if x600 builds again in mainline that would be good :)


Yes, I can confirm, that build with the tiny-printf fixes the
build issue on x600. So once you add this tiny-printf patchset,
I'll send a patch to move x600 over to use this smaller version.

Thanks,
Stefan

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [U-Boot, V4, 10/13] spl: add support for alternative boot device

[Tom Rini]

On Sun, Nov 08, 2015 at 05:11:51PM +0200, Nikita Kiryanov wrote:

> Introduce spl_boot_list array, which defines a list of boot devices
> that SPL will try before hanging. By default this list will consist
> of only spl_boot_device(), but board_boot_order() can be overridden
> by board code to populate the array with custom values.
> 
> Signed-off-by: Nikita Kiryanov 
> Cc: Igor Grinberg 
> Cc: Tom Rini 
> Cc: Simon Glass 
> Reviewed-by: Tom Rini 
> Reviewed-by: Simon Glass 

So, a problem with this patch is that we push the x600 board, which is
an 8KiB SPL target, over the line.  I feel like maybe we need a
follow-up patch that makes announcing depend not on libcommon (which
x600 needs) but something else to know that there's a reason to
announce.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [U-Boot, V4, 10/13] spl: add support for alternative boot device

[Tom Rini]

On Sun, Nov 08, 2015 at 05:11:51PM +0200, Nikita Kiryanov wrote:

> Introduce spl_boot_list array, which defines a list of boot devices
> that SPL will try before hanging. By default this list will consist
> of only spl_boot_device(), but board_boot_order() can be overridden
> by board code to populate the array with custom values.
> 
> Signed-off-by: Nikita Kiryanov 
> Cc: Igor Grinberg 
> Cc: Tom Rini 
> Cc: Simon Glass 
> Reviewed-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot