Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address reconfigurable

2017-05-03 Thread Tom Rini 
On Tue, May 02, 2017 at 10:43:07PM +, Vikas MANOCHA wrote:
> Hi Tom,
> 
> > -Original Message-
> > From: Vikas MANOCHA
> > Sent: Wednesday, April 12, 2017 12:47 PM
> > To: 'Tom Rini' <tr...@konsulko.com>
> > Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan Agner 
> > <stefan.ag...@toradex.com>; Jeremy Hunt
> > <jeremy.h...@deshawresearch.com>
> > Subject: RE: [U-Boot] [PATCH] spl: make image arg or fdt blob address 
> > reconfigurable
> > 
> > Hi Tom,
> > 
> > > -Original Message-
> > > From: Tom Rini [mailto:tr...@konsulko.com]
> > > Sent: Wednesday, April 12, 2017 6:33 AM
> > > To: Vikas MANOCHA <vikas.mano...@st.com>
> > > Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan Agner
> > > <stefan.ag...@toradex.com>; Jeremy Hunt
> > > <jeremy.h...@deshawresearch.com>
> > > Subject: Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address
> > > reconfigurable
> > >
> > > On Tue, Apr 11, 2017 at 11:44:00PM +, Vikas MANOCHA wrote:
> > > > Hi Tom,
> > > >
> > > > > -Original Message-
> > > > > From: Tom Rini [mailto:tr...@konsulko.com]
> > > > > Sent: Monday, April 10, 2017 5:29 AM
> > > > > To: Vikas MANOCHA <vikas.mano...@st.com>
> > > > > Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan
> > > > > Agner <stefan.ag...@toradex.com>; Jeremy Hunt
> > > > > <jeremy.h...@deshawresearch.com>
> > > > > Subject: Re: [U-Boot] [PATCH] spl: make image arg or fdt blob
> > > > > address reconfigurable
> > > > >
> > > > > On Fri, Apr 07, 2017 at 03:38:13PM -0700, Vikas Manocha wrote:
> > > > >
> > > > > > At present fdt blob or argument address being passed to kernel
> > > > > > is fixed at compile time using macro CONFIG_SYS_SPL_ARGS_ADDR.
> > > > > > FDT blob from different media like nand, nor flash are copied to
> > > > > > the address pointed by the macro.
> > > > > > The problem is, it makes args/fdt blob compulsory to copy which
> > > > > > is not required in cases like for NOR Flash. This patch removes 
> > > > > > this limitation.
> > > > > >
> > > > > > Signed-off-by: Vikas Manocha <vikas.mano...@st.com>
> > > > > > ---
> > > > > >  arch/arm/lib/spl.c| 7 +++
> > > > > >  arch/microblaze/cpu/spl.c | 6 +++---
> > > > > >  arch/powerpc/lib/spl.c| 8 
> > > > > >  common/spl/spl.c  | 6 --
> > > > > >  common/spl/spl_nor.c  | 8 +---
> > > > > >  include/spl.h | 5 ++---
> > > > >
> > > > > I assume you've tested the spl_nor case afterwards, yes?  Did this
> > > > > result in some measurable boot time decrease?  Thanks!
> > > >
> > > > Yes, I tested it's working on board. Not sure how to measure the impact 
> > > > on boot time.
> > >
> > > There's always good old grabserial. But if you didn't measure bootspeed, 
> > > did this decrease the code size?  Or fix some other issue?
> > 
> > Thanks Tom for the suggestion.
> > 
> > The benefit of this patch is : It removes copying FDT blob from NOR flash 
> > to ram in case of booting from nor flash,  text size reduction
> > is only 4 Bytes, no change in data/bss size. It might reduce the boot time 
> > but boot time impact would depend on the FDT blob
> > copying time & ram vs flash read speed.
> > 
> > Also it provides a way to change fdt blob address at run-time which might 
> > be useful e.g. choosing between two FDT blobs or
> > manipulating FDT address.
> 
> Please let me know if more info is required in order to pick this patch.

I think this is fine, I'll grab it next release, thanks!

-- 
Tom


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

Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address reconfigurable

2017-05-02 Thread Vikas MANOCHA 
Hi Tom,

> -Original Message-
> From: Vikas MANOCHA
> Sent: Wednesday, April 12, 2017 12:47 PM
> To: 'Tom Rini' <tr...@konsulko.com>
> Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan Agner 
> <stefan.ag...@toradex.com>; Jeremy Hunt
> <jeremy.h...@deshawresearch.com>
> Subject: RE: [U-Boot] [PATCH] spl: make image arg or fdt blob address 
> reconfigurable
> 
> Hi Tom,
> 
> > -Original Message-
> > From: Tom Rini [mailto:tr...@konsulko.com]
> > Sent: Wednesday, April 12, 2017 6:33 AM
> > To: Vikas MANOCHA <vikas.mano...@st.com>
> > Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan Agner
> > <stefan.ag...@toradex.com>; Jeremy Hunt
> > <jeremy.h...@deshawresearch.com>
> > Subject: Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address
> > reconfigurable
> >
> > On Tue, Apr 11, 2017 at 11:44:00PM +, Vikas MANOCHA wrote:
> > > Hi Tom,
> > >
> > > > -Original Message-
> > > > From: Tom Rini [mailto:tr...@konsulko.com]
> > > > Sent: Monday, April 10, 2017 5:29 AM
> > > > To: Vikas MANOCHA <vikas.mano...@st.com>
> > > > Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan
> > > > Agner <stefan.ag...@toradex.com>; Jeremy Hunt
> > > > <jeremy.h...@deshawresearch.com>
> > > > Subject: Re: [U-Boot] [PATCH] spl: make image arg or fdt blob
> > > > address reconfigurable
> > > >
> > > > On Fri, Apr 07, 2017 at 03:38:13PM -0700, Vikas Manocha wrote:
> > > >
> > > > > At present fdt blob or argument address being passed to kernel
> > > > > is fixed at compile time using macro CONFIG_SYS_SPL_ARGS_ADDR.
> > > > > FDT blob from different media like nand, nor flash are copied to
> > > > > the address pointed by the macro.
> > > > > The problem is, it makes args/fdt blob compulsory to copy which
> > > > > is not required in cases like for NOR Flash. This patch removes this 
> > > > > limitation.
> > > > >
> > > > > Signed-off-by: Vikas Manocha <vikas.mano...@st.com>
> > > > > ---
> > > > >  arch/arm/lib/spl.c| 7 +++
> > > > >  arch/microblaze/cpu/spl.c | 6 +++---
> > > > >  arch/powerpc/lib/spl.c| 8 
> > > > >  common/spl/spl.c  | 6 --
> > > > >  common/spl/spl_nor.c  | 8 +---
> > > > >  include/spl.h | 5 ++---
> > > >
> > > > I assume you've tested the spl_nor case afterwards, yes?  Did this
> > > > result in some measurable boot time decrease?  Thanks!
> > >
> > > Yes, I tested it's working on board. Not sure how to measure the impact 
> > > on boot time.
> >
> > There's always good old grabserial. But if you didn't measure bootspeed, 
> > did this decrease the code size?  Or fix some other issue?
> 
> Thanks Tom for the suggestion.
> 
> The benefit of this patch is : It removes copying FDT blob from NOR flash to 
> ram in case of booting from nor flash,  text size reduction
> is only 4 Bytes, no change in data/bss size. It might reduce the boot time 
> but boot time impact would depend on the FDT blob
> copying time & ram vs flash read speed.
> 
> Also it provides a way to change fdt blob address at run-time which might be 
> useful e.g. choosing between two FDT blobs or
> manipulating FDT address.

Please let me know if more info is required in order to pick this patch.

Cheers,
Vikas

> 
> Cheers,
> Vikas
> 
> > Thanks!
> >
> > --
> > Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address reconfigurable

2017-04-12 Thread Vikas MANOCHA 
Hi Tom,

> -Original Message-
> From: Tom Rini [mailto:tr...@konsulko.com]
> Sent: Wednesday, April 12, 2017 6:33 AM
> To: Vikas MANOCHA <vikas.mano...@st.com>
> Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan Agner 
> <stefan.ag...@toradex.com>; Jeremy Hunt
> <jeremy.h...@deshawresearch.com>
> Subject: Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address 
> reconfigurable
> 
> On Tue, Apr 11, 2017 at 11:44:00PM +, Vikas MANOCHA wrote:
> > Hi Tom,
> >
> > > -Original Message-
> > > From: Tom Rini [mailto:tr...@konsulko.com]
> > > Sent: Monday, April 10, 2017 5:29 AM
> > > To: Vikas MANOCHA <vikas.mano...@st.com>
> > > Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan Agner
> > > <stefan.ag...@toradex.com>; Jeremy Hunt
> > > <jeremy.h...@deshawresearch.com>
> > > Subject: Re: [U-Boot] [PATCH] spl: make image arg or fdt blob
> > > address reconfigurable
> > >
> > > On Fri, Apr 07, 2017 at 03:38:13PM -0700, Vikas Manocha wrote:
> > >
> > > > At present fdt blob or argument address being passed to kernel is
> > > > fixed at compile time using macro CONFIG_SYS_SPL_ARGS_ADDR. FDT
> > > > blob from different media like nand, nor flash are copied to the
> > > > address pointed by the macro.
> > > > The problem is, it makes args/fdt blob compulsory to copy which is
> > > > not required in cases like for NOR Flash. This patch removes this 
> > > > limitation.
> > > >
> > > > Signed-off-by: Vikas Manocha <vikas.mano...@st.com>
> > > > ---
> > > >  arch/arm/lib/spl.c| 7 +++
> > > >  arch/microblaze/cpu/spl.c | 6 +++---
> > > >  arch/powerpc/lib/spl.c| 8 
> > > >  common/spl/spl.c  | 6 --
> > > >  common/spl/spl_nor.c  | 8 +---
> > > >  include/spl.h | 5 ++---
> > >
> > > I assume you've tested the spl_nor case afterwards, yes?  Did this
> > > result in some measurable boot time decrease?  Thanks!
> >
> > Yes, I tested it's working on board. Not sure how to measure the impact on 
> > boot time.
> 
> There's always good old grabserial. But if you didn't measure bootspeed, did 
> this decrease the code size?  Or fix some other issue?

Thanks Tom for the suggestion.

The benefit of this patch is : It removes copying FDT blob from NOR flash to 
ram in case of booting from nor flash,  text size reduction
is only 4 Bytes, no change in data/bss size. It might reduce the boot time but 
boot time impact would depend on the FDT blob copying
time & ram vs flash read speed.

Also it provides a way to change fdt blob address at run-time which might be 
useful e.g. choosing between two FDT blobs or manipulating FDT address.

Cheers,
Vikas 

> Thanks!
> 
> --
> Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address reconfigurable

2017-04-12 Thread Tom Rini 
On Tue, Apr 11, 2017 at 11:44:00PM +, Vikas MANOCHA wrote:
> Hi Tom,
> 
> > -Original Message-
> > From: Tom Rini [mailto:tr...@konsulko.com]
> > Sent: Monday, April 10, 2017 5:29 AM
> > To: Vikas MANOCHA <vikas.mano...@st.com>
> > Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan Agner 
> > <stefan.ag...@toradex.com>; Jeremy Hunt
> > <jeremy.h...@deshawresearch.com>
> > Subject: Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address 
> > reconfigurable
> > 
> > On Fri, Apr 07, 2017 at 03:38:13PM -0700, Vikas Manocha wrote:
> > 
> > > At present fdt blob or argument address being passed to kernel is
> > > fixed at compile time using macro CONFIG_SYS_SPL_ARGS_ADDR. FDT blob
> > > from different media like nand, nor flash are copied to the address
> > > pointed by the macro.
> > > The problem is, it makes args/fdt blob compulsory to copy which is not
> > > required in cases like for NOR Flash. This patch removes this limitation.
> > >
> > > Signed-off-by: Vikas Manocha <vikas.mano...@st.com>
> > > ---
> > >  arch/arm/lib/spl.c| 7 +++
> > >  arch/microblaze/cpu/spl.c | 6 +++---
> > >  arch/powerpc/lib/spl.c| 8 
> > >  common/spl/spl.c  | 6 --
> > >  common/spl/spl_nor.c  | 8 +---
> > >  include/spl.h | 5 ++---
> > 
> > I assume you've tested the spl_nor case afterwards, yes?  Did this
> > result in some measurable boot time decrease?  Thanks!
> 
> Yes, I tested it's working on board. Not sure how to measure the impact on 
> boot time.

There's always good old grabserial.  But if you didn't measure
bootspeed, did this decrease the code size?  Or fix some other issue?
Thanks!

-- 
Tom


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

Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address reconfigurable

2017-04-11 Thread Vikas MANOCHA 
Hi Tom,

> -Original Message-
> From: Tom Rini [mailto:tr...@konsulko.com]
> Sent: Monday, April 10, 2017 5:29 AM
> To: Vikas MANOCHA <vikas.mano...@st.com>
> Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Stefan Agner 
> <stefan.ag...@toradex.com>; Jeremy Hunt
> <jeremy.h...@deshawresearch.com>
> Subject: Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address 
> reconfigurable
> 
> On Fri, Apr 07, 2017 at 03:38:13PM -0700, Vikas Manocha wrote:
> 
> > At present fdt blob or argument address being passed to kernel is
> > fixed at compile time using macro CONFIG_SYS_SPL_ARGS_ADDR. FDT blob
> > from different media like nand, nor flash are copied to the address
> > pointed by the macro.
> > The problem is, it makes args/fdt blob compulsory to copy which is not
> > required in cases like for NOR Flash. This patch removes this limitation.
> >
> > Signed-off-by: Vikas Manocha <vikas.mano...@st.com>
> > ---
> >  arch/arm/lib/spl.c| 7 +++
> >  arch/microblaze/cpu/spl.c | 6 +++---
> >  arch/powerpc/lib/spl.c| 8 
> >  common/spl/spl.c  | 6 --
> >  common/spl/spl_nor.c  | 8 +---
> >  include/spl.h | 5 ++---
> 
> I assume you've tested the spl_nor case afterwards, yes?  Did this result in 
> some measurable boot time decrease?  Thanks!

Yes, I tested it's working on board. Not sure how to measure the impact on boot 
time.

Cheers,
Vikas

> 
> --
> Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] spl: make image arg or fdt blob address reconfigurable

2017-04-10 Thread Tom Rini 
On Fri, Apr 07, 2017 at 03:38:13PM -0700, Vikas Manocha wrote:

> At present fdt blob or argument address being passed to kernel is fixed at
> compile time using macro CONFIG_SYS_SPL_ARGS_ADDR. FDT blob from
> different media like nand, nor flash are copied to the address pointed
> by the macro.
> The problem is, it makes args/fdt blob compulsory to copy which is not 
> required
> in cases like for NOR Flash. This patch removes this limitation.
> 
> Signed-off-by: Vikas Manocha 
> ---
>  arch/arm/lib/spl.c| 7 +++
>  arch/microblaze/cpu/spl.c | 6 +++---
>  arch/powerpc/lib/spl.c| 8 
>  common/spl/spl.c  | 6 --
>  common/spl/spl_nor.c  | 8 +---
>  include/spl.h | 5 ++---

I assume you've tested the spl_nor case afterwards, yes?  Did this
result in some measurable boot time decrease?  Thanks!

-- 
Tom


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

[U-Boot] [PATCH] spl: make image arg or fdt blob address reconfigurable

2017-04-07 Thread Vikas Manocha 
At present fdt blob or argument address being passed to kernel is fixed at
compile time using macro CONFIG_SYS_SPL_ARGS_ADDR. FDT blob from
different media like nand, nor flash are copied to the address pointed
by the macro.
The problem is, it makes args/fdt blob compulsory to copy which is not required
in cases like for NOR Flash. This patch removes this limitation.

Signed-off-by: Vikas Manocha 
---
 arch/arm/lib/spl.c| 7 +++
 arch/microblaze/cpu/spl.c | 6 +++---
 arch/powerpc/lib/spl.c| 8 
 common/spl/spl.c  | 6 --
 common/spl/spl_nor.c  | 8 +---
 include/spl.h | 5 ++---
 6 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
index e606d47..8ff2c50 100644
--- a/arch/arm/lib/spl.c
+++ b/arch/arm/lib/spl.c
@@ -44,22 +44,21 @@ void __weak board_init_f(ulong dummy)
 /*
  * This function jumps to an image with argument. Normally an FDT or ATAGS
  * image.
- * arg: Pointer to paramter image in RAM
  */
 #ifdef CONFIG_SPL_OS_BOOT
-void __noreturn jump_to_image_linux(struct spl_image_info *spl_image, void 
*arg)
+void __noreturn jump_to_image_linux(struct spl_image_info *spl_image)
 {
unsigned long machid = 0x;
 #ifdef CONFIG_MACH_TYPE
machid = CONFIG_MACH_TYPE;
 #endif
 
-   debug("Entering kernel arg pointer: 0x%p\n", arg);
+   debug("Entering kernel arg pointer: 0x%p\n", spl_image->arg);
typedef void (*image_entry_arg_t)(int, int, void *)
__attribute__ ((noreturn));
image_entry_arg_t image_entry =
(image_entry_arg_t)(uintptr_t) spl_image->entry_point;
cleanup_before_linux();
-   image_entry(0, machid, arg);
+   image_entry(0, machid, spl_image->arg);
 }
 #endif
diff --git a/arch/microblaze/cpu/spl.c b/arch/microblaze/cpu/spl.c
index 8e6d926..3d57a5a 100644
--- a/arch/microblaze/cpu/spl.c
+++ b/arch/microblaze/cpu/spl.c
@@ -29,15 +29,15 @@ void spl_board_init(void)
 }
 
 #ifdef CONFIG_SPL_OS_BOOT
-void __noreturn jump_to_image_linux(struct spl_image_info *spl_image, void 
*arg)
+void __noreturn jump_to_image_linux(struct spl_image_info *spl_image)
 {
-   debug("Entering kernel arg pointer: 0x%p\n", arg);
+   debug("Entering kernel arg pointer: 0x%p\n", spl_image->arg);
typedef void (*image_entry_arg_t)(char *, ulong, ulong)
__attribute__ ((noreturn));
image_entry_arg_t image_entry =
(image_entry_arg_t)spl_image->entry_point;
 
-   image_entry(NULL, 0, (ulong)arg);
+   image_entry(NULL, 0, (ulong)spl_image->arg);
 }
 #endif /* CONFIG_SPL_OS_BOOT */
 
diff --git a/arch/powerpc/lib/spl.c b/arch/powerpc/lib/spl.c
index 080b978..b931970 100644
--- a/arch/powerpc/lib/spl.c
+++ b/arch/powerpc/lib/spl.c
@@ -14,18 +14,18 @@ DECLARE_GLOBAL_DATA_PTR;
 /*
  * This function jumps to an image with argument. Normally an FDT or ATAGS
  * image.
- * arg: Pointer to paramter image in RAM
  */
 #ifdef CONFIG_SPL_OS_BOOT
-void __noreturn jump_to_image_linux(struct spl_image_info *spl_image, void 
*arg)
+void __noreturn jump_to_image_linux(struct spl_image_info *spl_image)
 {
-   debug("Entering kernel arg pointer: 0x%p\n", arg);
+   debug("Entering kernel arg pointer: 0x%p\n", spl_image->arg);
typedef void (*image_entry_arg_t)(void *, ulong r4, ulong r5, ulong r6,
  ulong r7, ulong r8, ulong r9)
__attribute__ ((noreturn));
image_entry_arg_t image_entry =
(image_entry_arg_t)spl_image->entry_point;
 
-   image_entry(arg, 0, 0, EPAPR_MAGIC, CONFIG_SYS_BOOTMAPSZ, 0, 0);
+   image_entry(spl_image->arg, 0, 0, EPAPR_MAGIC, CONFIG_SYS_BOOTMAPSZ,
+   0, 0);
 }
 #endif /* CONFIG_SPL_OS_BOOT */
diff --git a/common/spl/spl.c b/common/spl/spl.c
index a3e73b8..50828e6 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -345,6 +345,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 #endif
 
memset(_image, '\0', sizeof(spl_image));
+#ifdef CONFIG_SYS_SPL_ARGS_ADDR
+   spl_image.arg = (void *)CONFIG_SYS_SPL_ARGS_ADDR;
+#endif
board_boot_order(spl_boot_list);
 
if (boot_from_devices(_image, spl_boot_list,
@@ -361,8 +364,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
case IH_OS_LINUX:
debug("Jumping to Linux\n");
spl_board_prepare_for_linux();
-   jump_to_image_linux(_image,
-   (void *)CONFIG_SYS_SPL_ARGS_ADDR);
+   jump_to_image_linux(_image);
 #endif
default:
debug("Unsupported OS image.. Jumping nevertheless..\n");
diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c
index d07ca84..1ef8ac8 100644
--- a/common/spl/spl_nor.c
+++ b/common/spl/spl_nor.c
@@ -39,13 +39,7 @@ static int spl_nor_load_image(struct spl_image_info 
*spl_image,
sizeof(struct