Re: [U-Boot] [PATCH v2 1/2] Add README for the "Falcon" mode

2012-11-14 Thread Otavio Salvador
On Wed, Nov 14, 2012 at 10:19 AM, Stefano Babic  wrote:

> On 14/11/2012 11:29, Otavio Salvador wrote:
> >
>
> Hi Otavio,
>
> >
> > In the text you have the offset to save the image onto a NAND offset so
> > I fail to see how it'd be used for SD-Card.
> >
> > Can you elaborate it a bit?
>
> No, I can't without introducing errors. Current code supports Falcon
> only booting from NAND - I do not think that it is a big thing to use
> another media, but it is not yet done. This document must


So I'd say to drop the SD-Card from the document.


> > +and can be generalized seen as a way to start an image performing
> > the minimum
> > +required initialization. SPL initializes mainly the RAM controller,
> > and after
> > +that copies U-Boot image into the memory. The "Falcon" mode extends
> > this way
> > +allowing to start the Linux kernel directly from SPL. A new command
> > is added
> > +to U-Boot to prepare the parameters that SPL must pass to the
> > kernel, using
> > +ATAGS or Device Tree.
> > +
> > +Falcon adds a command under U-Boot to reuse all code responsible to
> > prepare
> > +the interface with the kernel. In usual U-boot systems, these
> > parameters are
> > +generated each time before loading the kernel, passing to Linux the
> > address
> > +in memory where the parameters can be read.
> > +With falcon, this snapshot can be saved into persistent storage and
> > SPL is
> > +informed to load it before running the kernel.
> >
> >
> > You mix Falcon and falcon.
>
> You're right, there are already comments about this. This will be fixed
> globally in V3 (I will use Falcon Mode consistently).
>
> > I'd say you always use Falcon as it is the
> > name of the feature so it ought to be in upper case. Another thing,
> > 'With falcon, ' ought to be move to the previous line or have an empty
> > line before it.
>
> Ok
>
> >
> >
> > +To boot the kernel, these steps under a Falcon-aware U-Boot are
> > required:
> > +
> > +1. Boot the board into U-Boot.
> > +Use the "spl export" command to generate the kernel parameters area
> > or the DT.
> > +U-boot runs as when it boots the kernel, but stops before passing
> > the control
> > +to the kernel.
> > +
> > +2. Saves the prepared snapshot into persistent media.
> > +The address where to save it must be configured into board
> > configuration
> > +file (CONFIG_CMD_SPL_NAND_OFS for NAND).
> >
> >
> > And for others?
>
> Not yet implemented, patches welcome ! But you are right, there is
> already a patch to use a NOR flash that should flow soon into mainline.
> I will check and add documentation for it. For the orher media, the
> document must be updated together when they will be full supported.
>
>
> >
> >
> > +3. Boot the board into "Falcon" mode. SPL will load the kernel and
> copy
> > +the parameters area to the required address.
> > +
> > +It is required to implement a custom mechanism to select if SPL
> > loads U-Boot
> > +or another image.
> > +The value of a GPIO is a simple way to operate the selection, as
> > well as
> > +reading a character from the SPL console if CONFIG_SPL_CONSOLE is
> set.
> >
> >
> > An empty line before "The value"?
>
> Ok
>
> Best regards,
> Stefano Babic
>
> --
> =
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
> =
>



-- 
Otavio Salvador O.S. Systems
E-mail: ota...@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854  http://projetos.ossystems.com.br
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/2] Add README for the "Falcon" mode

2012-11-14 Thread Stefano Babic
On 14/11/2012 11:29, Otavio Salvador wrote:
> 
> 

Hi Otavio,

> 
> In the text you have the offset to save the image onto a NAND offset so
> I fail to see how it'd be used for SD-Card.
> 
> Can you elaborate it a bit?

No, I can't without introducing errors. Current code supports Falcon
only booting from NAND - I do not think that it is a big thing to use
another media, but it is not yet done. This document must

> +and can be generalized seen as a way to start an image performing
> the minimum
> +required initialization. SPL initializes mainly the RAM controller,
> and after
> +that copies U-Boot image into the memory. The "Falcon" mode extends
> this way
> +allowing to start the Linux kernel directly from SPL. A new command
> is added
> +to U-Boot to prepare the parameters that SPL must pass to the
> kernel, using
> +ATAGS or Device Tree.
> +
> +Falcon adds a command under U-Boot to reuse all code responsible to
> prepare
> +the interface with the kernel. In usual U-boot systems, these
> parameters are
> +generated each time before loading the kernel, passing to Linux the
> address
> +in memory where the parameters can be read.
> +With falcon, this snapshot can be saved into persistent storage and
> SPL is
> +informed to load it before running the kernel.
> 
> 
> You mix Falcon and falcon.

You're right, there are already comments about this. This will be fixed
globally in V3 (I will use Falcon Mode consistently).

> I'd say you always use Falcon as it is the
> name of the feature so it ought to be in upper case. Another thing,
> 'With falcon, ' ought to be move to the previous line or have an empty
> line before it.

Ok

>  
> 
> +To boot the kernel, these steps under a Falcon-aware U-Boot are
> required:
> +
> +1. Boot the board into U-Boot.
> +Use the "spl export" command to generate the kernel parameters area
> or the DT.
> +U-boot runs as when it boots the kernel, but stops before passing
> the control
> +to the kernel.
> +
> +2. Saves the prepared snapshot into persistent media.
> +The address where to save it must be configured into board
> configuration
> +file (CONFIG_CMD_SPL_NAND_OFS for NAND).
> 
> 
> And for others?

Not yet implemented, patches welcome ! But you are right, there is
already a patch to use a NOR flash that should flow soon into mainline.
I will check and add documentation for it. For the orher media, the
document must be updated together when they will be full supported.


>  
> 
> +3. Boot the board into "Falcon" mode. SPL will load the kernel and copy
> +the parameters area to the required address.
> +
> +It is required to implement a custom mechanism to select if SPL
> loads U-Boot
> +or another image.
> +The value of a GPIO is a simple way to operate the selection, as
> well as
> +reading a character from the SPL console if CONFIG_SPL_CONSOLE is set.
> 
> 
> An empty line before "The value"?

Ok

Best regards,
Stefano Babic

-- 
=
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/2] Add README for the "Falcon" mode

2012-11-14 Thread Otavio Salvador
On Tue, Nov 13, 2012 at 9:11 AM, Stefano Babic  wrote:

> Simple howto to add support to a board
> for booting the kernel from SPL ("Falcon" mode).
>
> Signed-off-by: Stefano Babic 
> ---
> Changes in v2:
> - spelling, language fixes (Andreas Biessman)
> - rewrite some unclear sentences
> - drop CONFIG_SPL_OS_BOOT_KEY
> - make example with twister more exhaustive
>
>  doc/README.falcon |  163
> +
>  1 file changed, 163 insertions(+)
>  create mode 100644 doc/README.falcon
>
> diff --git a/doc/README.falcon b/doc/README.falcon
> new file mode 100644
> index 000..94126fd
> --- /dev/null
> +++ b/doc/README.falcon
> @@ -0,0 +1,163 @@
> +U-Boot "Falcon" Mode
> +
> +
> +Introduction
> +
> +
> +This documents provides an overview how to add support for "Falcon" mode
> +to a board.
> +Falcon mode is introduced to speed up the booting process, allowing
> +to boot a Linux kernel (or whatever image) without a full blown U-Boot.
> +
> +Falcon mode relies on the SPL framework. In fact, to make booting faster,
> +U-Boot is split into two parts: the SPL (Secondary Program Loader) and
> U-Boot
> +image. In most implementations, SPL is used to start U-Boot when booting
> from
> +a mass storage, such as NAND or SD-Card. SPL has now support for other
> media,
>

In the text you have the offset to save the image onto a NAND offset so I
fail to see how it'd be used for SD-Card.

Can you elaborate it a bit?


> +and can be generalized seen as a way to start an image performing the
> minimum
> +required initialization. SPL initializes mainly the RAM controller, and
> after
> +that copies U-Boot image into the memory. The "Falcon" mode extends this
> way
> +allowing to start the Linux kernel directly from SPL. A new command is
> added
> +to U-Boot to prepare the parameters that SPL must pass to the kernel,
> using
> +ATAGS or Device Tree.
> +
> +Falcon adds a command under U-Boot to reuse all code responsible to
> prepare
> +the interface with the kernel. In usual U-boot systems, these parameters
> are
> +generated each time before loading the kernel, passing to Linux the
> address
> +in memory where the parameters can be read.
> +With falcon, this snapshot can be saved into persistent storage and SPL is
> +informed to load it before running the kernel.
>

You mix Falcon and falcon. I'd say you always use Falcon as it is the name
of the feature so it ought to be in upper case. Another thing, 'With
falcon, ' ought to be move to the previous line or have an empty line
before it.


> +To boot the kernel, these steps under a Falcon-aware U-Boot are required:
> +
> +1. Boot the board into U-Boot.
> +Use the "spl export" command to generate the kernel parameters area or
> the DT.
> +U-boot runs as when it boots the kernel, but stops before passing the
> control
> +to the kernel.
> +
> +2. Saves the prepared snapshot into persistent media.
> +The address where to save it must be configured into board configuration
> +file (CONFIG_CMD_SPL_NAND_OFS for NAND).
>

And for others?


> +3. Boot the board into "Falcon" mode. SPL will load the kernel and copy
> +the parameters area to the required address.
> +
> +It is required to implement a custom mechanism to select if SPL loads
> U-Boot
> +or another image.
> +The value of a GPIO is a simple way to operate the selection, as well as
> +reading a character from the SPL console if CONFIG_SPL_CONSOLE is set.
>

An empty line before "The value"?


> +Falcon mode is generally activated by setting CONFIG_SPL_OS_BOOT. This
> tells
> +SPL that U-Boot is not the only available image that SPL is able to start.
> +
> +Configuration
> +
> +CONFIG_CMD_SPL Enable the "spl export" command.
> +   The command "spl export" is then available in
> U-Boot
> +   mode
> +CONFIG_SPL_OS_BOOT Activate Falcon mode.
> +   A board should implement the following functions:
> +
> +CONFIG_SYS_SPL_ARGS_ADDR   Address in RAM where the parameters must be
> +   copied by SPL.
> +   In most cases, it is  + 0x100
> +
> +CONFIG_SYS_NAND_SPL_KERNEL_OFFSOffset in NAND where the kernel is
> stored
>

And other media?


> +CONFIG_CMD_SPL_NAND_OFSOffset in NAND where the parameters area
> was saved.
> +
> +CONFIG_CMD_SPL_WRITE_SIZE  Size of the parameters area to be copied
> +
> +Function that a board must implement
> +
>

Is there others optional? Otherwise you could call it as:

Adding Falcon mode support for a board

+void spl_board_prepare_for_linux(void) : optional
> +   Called from SPL before starting the kernel
> +
> +spl_start_uboot() : required
> +   Returns "0" if SPL starts the kernel, "1" if U-Boot
> +   must be started.
> +
> +
> +Using spl command
> +-
> +
> +spl - SPL configurat

Re: [U-Boot] [PATCH v2 1/2] Add README for the "Falcon" mode

2012-11-13 Thread Stefano Babic
On 13/11/2012 16:55, Andreas Bießmann wrote:

>> +CONFIG_SPL_OS_BOOT  Activate Falcon mode.
>> +A board should implement the following functions:
> 
> I think reordering this list to have the required functions directly
> after the colon would be helpful. Alternatively add a pointer to the
> list of functions after the list of defines.

Yes, it is what I wanted to do. In fact, the phrase now before the colon
seems incomplete, because the functions are missing. I will this entry
at the end of the define, so that the functions will follow after it.

> 
>> +spl - SPL configuration
>> +
>> +Usage:
>> +
>> +spl export  [kernel_addr] [fdt_addr if  = fdt] - export 
>> a kernel parameter image
>> +
>> +img : "atags" or "fdt"
>> +kernel_addr : kernel is loaded as part of the boot process, but it is not 
>> started.
>> +  This is the address where a kernel image is stored.
>> +fdt_addr: in case of fdt, the address of the device tree.
>> +
>> +
>> +The spl puts its result at a self gained position. The position is defined 
>> at compile
>> +time or when generating the uImage but not at command line for 'spl export'
>> +(see spl_export(): gd->bd->bi_boot_params vs. images.ft_addr).
> 
> I think you got me wrong by my last remarks about the usage() of 'spl
> export'.

Probably...

> 
> 
> First of all there is presumably a typo in the descriptive text.
> 
> The current code has this usage():
> ---8<---
>   "export  [kernel_addr] [initrd_addr] "
>   "[fdt_addr if  = fdt] - export a kernel parameter image\n"
>   "\t initrd_img can be set to \"-\" if fdt_addr without initrd img is"
>   "used")
> --->8---
> The arguments say something about 'initrd_addr' but the descriptive text
> has 'initrd_img'. I think (but do not know since I have not used the spl
> command so often) that the 'initrd_img' should be written 'initrd_addr'
> (cc Simon Schwarz for that).

Ok, I cannot avoid to fix this typo the command with a separate patch.
Else documentation and code remain inconsistent.

> 
> The second thing in my last review of your patch was maybe shortly
> described. You did write there the correct usage() of 'spl export' but
> later on add a more descriptive text about the parameters of 'spl
> export'. In these descriptive text you did write 'init_addr' instead of
> 'initrd_addr' - that was my comment about.

Ok, it seems I misunderstood. But I think your comment about where spl
stores the result is helpful, even if the address is printed by the
command itself.

> Additionally I thought your descriptive text is way better than the
> current descriptive text of 'spl export', so I asked to add your
> description to the command.

Ok - I will put it into the spl help as well.

> 
> Now the main reason for my writing here ;)
> Your usage() of 'spl export' some lines above is wrong cause it is
> missing the 'initrd_addr' parameter.


Correct - to be fixed ;-)

Best regards,
Stefano Babic

-- 
=
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/2] Add README for the "Falcon" mode

2012-11-13 Thread Andreas Bießmann
Dear Stefano Babic,

On 13.11.2012 12:11, Stefano Babic wrote:
> Simple howto to add support to a board
> for booting the kernel from SPL ("Falcon" mode).
> 
> Signed-off-by: Stefano Babic 
> ---
> Changes in v2:
> - spelling, language fixes (Andreas Biessman)
> - rewrite some unclear sentences
> - drop CONFIG_SPL_OS_BOOT_KEY
> - make example with twister more exhaustive
> 
>  doc/README.falcon |  163 
> +
>  1 file changed, 163 insertions(+)
>  create mode 100644 doc/README.falcon
> 



> +Configuration
> +
> +CONFIG_CMD_SPL   Enable the "spl export" command.
> + The command "spl export" is then available in U-Boot
> + mode
> +CONFIG_SPL_OS_BOOT   Activate Falcon mode.
> + A board should implement the following functions:

I think reordering this list to have the required functions directly
after the colon would be helpful. Alternatively add a pointer to the
list of functions after the list of defines.

> +
> +CONFIG_SYS_SPL_ARGS_ADDR Address in RAM where the parameters must be
> + copied by SPL.
> + In most cases, it is  + 0x100
> +
> +CONFIG_SYS_NAND_SPL_KERNEL_OFFS  Offset in NAND where the kernel is 
> stored
> +
> +CONFIG_CMD_SPL_NAND_OFS  Offset in NAND where the parameters area was 
> saved.
> +
> +CONFIG_CMD_SPL_WRITE_SIZESize of the parameters area to be copied
> +
> +Function that a board must implement
> +
> +
> +void spl_board_prepare_for_linux(void) : optional
> + Called from SPL before starting the kernel
> +
> +spl_start_uboot() : required
> + Returns "0" if SPL starts the kernel, "1" if U-Boot
> + must be started.
> +
> +
> +Using spl command
> +-
> +
> +spl - SPL configuration
> +
> +Usage:
> +
> +spl export  [kernel_addr] [fdt_addr if  = fdt] - export 
> a kernel parameter image
> +
> +img  : "atags" or "fdt"
> +kernel_addr  : kernel is loaded as part of the boot process, but it is not 
> started.
> +   This is the address where a kernel image is stored.
> +fdt_addr : in case of fdt, the address of the device tree.
> +
> +
> +The spl puts its result at a self gained position. The position is defined 
> at compile
> +time or when generating the uImage but not at command line for 'spl export'
> +(see spl_export(): gd->bd->bi_boot_params vs. images.ft_addr).

I think you got me wrong by my last remarks about the usage() of 'spl
export'.


First of all there is presumably a typo in the descriptive text.

The current code has this usage():
---8<---
"export  [kernel_addr] [initrd_addr] "
"[fdt_addr if  = fdt] - export a kernel parameter image\n"
"\t initrd_img can be set to \"-\" if fdt_addr without initrd img is"
"used")
--->8---
The arguments say something about 'initrd_addr' but the descriptive text
has 'initrd_img'. I think (but do not know since I have not used the spl
command so often) that the 'initrd_img' should be written 'initrd_addr'
(cc Simon Schwarz for that).

The second thing in my last review of your patch was maybe shortly
described. You did write there the correct usage() of 'spl export' but
later on add a more descriptive text about the parameters of 'spl
export'. In these descriptive text you did write 'init_addr' instead of
'initrd_addr' - that was my comment about.
Additionally I thought your descriptive text is way better than the
current descriptive text of 'spl export', so I asked to add your
description to the command.

Now the main reason for my writing here ;)
Your usage() of 'spl export' some lines above is wrong cause it is
missing the 'initrd_addr' parameter.



Best regards

Andreas Bießmann

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


Re: [U-Boot] [PATCH v2 1/2] Add README for the "Falcon" mode

2012-11-13 Thread Thomas Weber
Hello Stefano,

there are some inconsistency for the writing of the Falcon mode:
"Falcon" mode, Falcon mode, Falcon, falcon, "Falcon mode"

On 11/13/2012 12:11 PM, Stefano Babic wrote:
> Simple howto to add support to a board
> for booting the kernel from SPL ("Falcon" mode).
>
> Signed-off-by: Stefano Babic 
> ---
> Changes in v2:
> - spelling, language fixes (Andreas Biessman)
> - rewrite some unclear sentences
> - drop CONFIG_SPL_OS_BOOT_KEY
> - make example with twister more exhaustive
>
>  doc/README.falcon |  163 
> +
>  1 file changed, 163 insertions(+)
>  create mode 100644 doc/README.falcon
>
> diff --git a/doc/README.falcon b/doc/README.falcon
> new file mode 100644
> index 000..94126fd
> --- /dev/null
> +++ b/doc/README.falcon
> @@ -0,0 +1,163 @@
> +U-Boot "Falcon" Mode
> +
> +
> +Introduction
> +
> +
> +This documents provides an overview how to add support for "Falcon" mode
This document provides ...
> +to a board.
> +Falcon mode is introduced to speed up the booting process, allowing
> +to boot a Linux kernel (or whatever image) without a full blown U-Boot.
> +
> +Falcon mode relies on the SPL framework. In fact, to make booting faster,
> +U-Boot is split into two parts: the SPL (Secondary Program Loader) and U-Boot
> +image. In most implementations, SPL is used to start U-Boot when booting from
> +a mass storage, such as NAND or SD-Card. SPL has now support for other media,
> +and can be generalized seen as a way to start an image performing the minimum
> +required initialization. SPL initializes mainly the RAM controller, and after
> +that copies U-Boot image into the memory. The "Falcon" mode extends this way
> +allowing to start the Linux kernel directly from SPL. A new command is added
> +to U-Boot to prepare the parameters that SPL must pass to the kernel, using
> +ATAGS or Device Tree.
> +
> +Falcon adds a command under U-Boot to reuse all code responsible to prepare
> +the interface with the kernel. In usual U-boot systems, these parameters are
U-boot => U-Boot
> +generated each time before loading the kernel, passing to Linux the address
> +in memory where the parameters can be read.
> +With falcon, this snapshot can be saved into persistent storage and SPL is
> +informed to load it before running the kernel.
> +
> +To boot the kernel, these steps under a Falcon-aware U-Boot are required:
> +
> +1. Boot the board into U-Boot.
> +Use the "spl export" command to generate the kernel parameters area or the 
> DT.
> +U-boot runs as when it boots the kernel, but stops before passing the control
> +to the kernel.
> +
> +2. Saves the prepared snapshot into persistent media.
Saves => Save
> +The address where to save it must be configured into board configuration
> +file (CONFIG_CMD_SPL_NAND_OFS for NAND).
> +
> +3. Boot the board into "Falcon" mode. SPL will load the kernel and copy
> +the parameters area to the required address.
> +
> +It is required to implement a custom mechanism to select if SPL loads U-Boot
> +or another image.
> +The value of a GPIO is a simple way to operate the selection, as well as
> +reading a character from the SPL console if CONFIG_SPL_CONSOLE is set.
> +
> +Falcon mode is generally activated by setting CONFIG_SPL_OS_BOOT. This tells
> +SPL that U-Boot is not the only available image that SPL is able to start.
> +
> +Configuration
> +
> +CONFIG_CMD_SPL   Enable the "spl export" command.
> + The command "spl export" is then available in U-Boot
> + mode
> +CONFIG_SPL_OS_BOOT   Activate Falcon mode.
> + A board should implement the following functions:
> +
> +CONFIG_SYS_SPL_ARGS_ADDR Address in RAM where the parameters must be
> + copied by SPL.
> + In most cases, it is  + 0x100
> +
> +CONFIG_SYS_NAND_SPL_KERNEL_OFFS  Offset in NAND where the kernel is 
> stored
> +
> +CONFIG_CMD_SPL_NAND_OFS  Offset in NAND where the parameters area was 
> saved.
> +
> +CONFIG_CMD_SPL_WRITE_SIZESize of the parameters area to be copied
> +
> +Function that a board must implement
> +
> +
> +void spl_board_prepare_for_linux(void) : optional
> + Called from SPL before starting the kernel
> +
> +spl_start_uboot() : required
> + Returns "0" if SPL starts the kernel, "1" if U-Boot
> + must be started.
> +
> +
> +Using spl command
> +-
> +
> +spl - SPL configuration
> +
> +Usage:
> +
> +spl export  [kernel_addr] [fdt_addr if  = fdt] - export 
> a kernel parameter image
> +
> +img  : "atags" or "fdt"
> +kernel_addr  : kernel is loaded as part of the boot process, but it is not 
> started.
> +   This is the address where a kernel image is stored.
> +fdt_addr : in case of fdt, the address of the device tree.
> +
> +
> +The spl puts its