Re: [U-Boot] [PATCH v2 5/7] armv8: layerscape: Eanble falcon boot

[Albert ARIBAUD (U-Boot)]

Spam detection software, running on the system "lists.denx.de",
has identified this incoming email as possible spam.  The original
message has been attached to this so you can view it or label
similar future email.  If you have any questions, see
@@CONTACT_ADDRESS@@ for details.

Content preview:  Hi, Le Sun, 17 Sep 2017 11:54:56 -0600 Simon Glass 

   a écrit: > +Philippe for review > > On 14 September 2017 at 13:01, York
  Sun  wrote: > > Add jump_to_image_linux() for arm64. Add
   "noreturn" flag to > > armv8_switch_to_el2(). Add hooks to fsl-layerscape
   to enable falcon > > boot. > > > > Signed-off-by: York Sun 
   > > > > --- > > > > Changes in v2: > > Relace getenv_f() with env_get_f()
   after rebasing to latet master. > > > > 
.../arm/cpu/armv8/fsl-layerscape/doc/README.falcon
   | 140 > > + > > arch/arm/cpu/armv8/fsl-layerscape/spl.c
   | 29 + > > arch/arm/include/asm/system.h | 2 +- > > arch/arm/lib/spl.c
   | 11 ++ 4 files > > changed, 181 insertions(+), 1 deletion(-) create mode
   100644 > > arch/arm/cpu/armv8/fsl-layerscape/doc/README.falcon > > 
Reviewed-by:
   Simon Glass  [...] 

Content analysis details:   (5.6 points, 5.0 required)

 pts rule name  description
 -- --
 3.6 RCVD_IN_PBLRBL: Received via a relay in Spamhaus PBL
[82.64.3.181 listed in zen.spamhaus.org]
 0.0 KHOP_BIG_TO_CC Sent to 10+ recipients instaed of Bcc or a list
 0.0 RCVD_IN_SORBS_DUL  RBL: SORBS: sent directly from dynamic IP address
[82.64.3.181 listed in dnsbl.sorbs.net]
-0.0 SPF_HELO_PASS  SPF: HELO matches SPF record
 1.6 RCVD_IN_BRBL_LASTEXT   RBL: No description available.
[82.64.3.181 listed in bb.barracudacentral.org]
 0.4 RDNS_DYNAMIC   Delivered to internal network by host with
dynamic-looking rDNS


--- Begin Message ---
Hi,

Le Sun, 17 Sep 2017 11:54:56 -0600
Simon Glass  a écrit:

> +Philippe for review
> 
> On 14 September 2017 at 13:01, York Sun  wrote:
> > Add jump_to_image_linux() for arm64. Add "noreturn" flag to
> > armv8_switch_to_el2(). Add hooks to fsl-layerscape to enable falcon
> > boot.
> >
> > Signed-off-by: York Sun 
> >
> > ---
> >
> > Changes in v2:
> > Relace getenv_f() with env_get_f() after rebasing to latet master.
> >
> >  .../arm/cpu/armv8/fsl-layerscape/doc/README.falcon | 140
> > +
> > arch/arm/cpu/armv8/fsl-layerscape/spl.c|  29 +
> > arch/arm/include/asm/system.h  |   2 +-
> > arch/arm/lib/spl.c |  11 ++ 4 files
> > changed, 181 insertions(+), 1 deletion(-) create mode 100644
> > arch/arm/cpu/armv8/fsl-layerscape/doc/README.falcon  
> 
> Reviewed-by: Simon Glass 

Nitpick: fix typo ("Eanble") in subject (can be done when applying if
no v3 is needed).

Amicalement,
-- 
Albert.
--- End Message ---
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] arm: Update our 'ret' assembler macro slightly

[Albert ARIBAUD]

Hello Tom,

LGTM.

On Thu,  2 Mar 2017 09:59:30 -0500, Tom Rini <tr...@konsulko.com> wrote:
> We only support cores that do Thumb-1 or later.  So we add a comment to
> explain this and remove the architecture test.
> 
> Cc: Albert ARIBAUD <albert.u.b...@aribaud.net>
> Cc: Mans Rullgard <m...@mansr.com>
> Signed-off-by: Tom Rini <tr...@konsulko.com>
> ---
>  arch/arm/include/asm/assembler.h | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/include/asm/assembler.h 
> b/arch/arm/include/asm/assembler.h
> index c56daf2a1f69..d24be2d484fe 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -57,17 +57,17 @@
>  #define PLD(code...)
>  #endif
>  
> +/*
> + * We only support cores that support at least Thumb-1 and thus we use
> + * 'bx lr'
> + */
>   .irpc,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo
>   .macro  ret\c, reg
> -#if defined(__ARM_ARCH_5E__)
> - mov\c   pc, \reg
> -#else
>   .ifeqs  "\reg", "lr"
>   bx\c\reg
>   .else
>   mov\c   pc, \reg
>   .endif
> -#endif
>   .endm
>   .endr
>  
> -- 
> 1.9.1
> 



Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] armv5te: make 'ret lr' produce iinterworking 'bx lr'

[Albert ARIBAUD]

Current ARM assembler helper for the 'return to caller' pseudo-instruction
turns 'ret lr' into 'mov pc, lr' for ARMv5TE. This causes the core to remain
in its current ARM state even when the routine doing the 'ret' was called
from Thumb-1 state, triggering an undefined instruction exception.

This causes early run-time failures in all boards compiled using the Thumb-1
instruction set (for instance the Open-RD family).

ARMv5TE supports 'bx lr' which properly implements interworking and thus
correctly returns to Thumb-1 state from ARM state.

This change makes 'ret lr' turn into 'bx lr' for ARMv5TE.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---
Note: this patch supersedes patch "openrd: disable private arch memset,
memcpy and libgcc" dated Sun, 26 Feb 2017 16:29:32 +0100.

 arch/arm/include/asm/assembler.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index ae1e42fc06..c56daf2a1f 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -59,7 +59,7 @@
 
.irpc,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo
.macro  ret\c, reg
-#if defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__)
+#if defined(__ARM_ARCH_5E__)
mov\c   pc, \reg
 #else
.ifeqs  "\reg", "lr"
-- 
2.11.0

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

Re: [U-Boot] [PATCH] openrd: disable private arch memset, memcpy and libgcc

[Albert ARIBAUD]

Update:

On Mon, 27 Feb 2017 07:52:10 +0100, Albert ARIBAUD
<albert.u.b...@aribaud.net> wrote:
> Hello Tom,
> 
> On Sun, 26 Feb 2017 20:04:28 -0500, Tom Rini <tr...@konsulko.com> wrote:
> > On Sun, Feb 26, 2017 at 04:29:32PM +0100, Albert ARIBAUD wrote:
> >   
> > > The switch to private LIBGCC causes 'undefined instruction'
> > > exception in at least the Open-RD Ultimate and Client, and
> > > quite probably the Open-RD Base as well.
> > > 
> > > While debugging this issue, it appeared the switch to
> > > architecture-optimized memset and memcpy also causes the
> > > same exceptions.
> > > 
> > > Until a fix to private libgcc and architecture-optimized
> > > memcpy and memset is mainlined, disable their use in all
> > > openrd configurations.
> > > 
> > > Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
> > 
> > Implementation wise, there's two big problems here.  First, all of the
> > code we're talking about is a direct drop-in from the kernel.  So
> > whatever is wrong here is a problem that the kernel itself can trigger,
> > so needs sorting out and fixing.  Second, do you have more details on
> > how to recreate (or not recreate) these problems?  
> 
> Second answer first: the easy way to reproduce this is to build for any
> Thumb-1 board and wait for memset, memcpy or a libgcc function to be
> called, which eventually results in an 'undefined instruction' trap.
> Then add the three CONFIG...=n lines in the board's config file, build,
> and see that the issue is gone. This is 100% reproducible.
> 
> First answer:
> 
> I've looked into a 'slow general fix' in parallel to this 'fast fix',
> starting with the memset case (and assuming the issue is common to the
> memcpy and libgcc cases.
> 
> The cause seems to stem from this:
> 
> - for Thumb-1 boards, the arch/arm/lib/memset.S et al. are built for
>   ARM state, due AFAIR to incompatibilities with the Thumb-1 instruction
>   set.
> 
> - apparently the 'ret lr' in arch/arm/lib/memset.S gets translated into
>   a 'mov pc, lr' which, at least for ATMv5TE, is a simple branch, not
>   an interworking branch.
> 
> - consequently, upon return from the memset call, the CPU remains in
>   ARM state while trying to execute Thumb-1 instructions.
> 
> ISTR there are gnu as, gcc or ldoptions to be passed to cause the 'ret
> lr' to be turned into a 'bx lr', but I could not find them yesterday.
> 
> The 'slow and more correct' fix consists in building the Linux and
> U-Boot source code for the Open-RD board and looking into what code the
> memset.S produces in Linux and what compiler/assembler options were
> passed to it (or digging into the memset, memcpy and libgcc of the
> GCC C compiler, but that's a bit less simple).

I dropped the Linux trail because it builds Kirkwood (not Orion, as I
wrongly stated previously) with ARM instruction set rather than the
Thumb-1 set U-Boot uses, so the issue will never appear in the kernel
(it might possibly happen in userland code if that code uses a custom
libgcc, which I consider quite improbable).

So I dug into the 'ret' pseudo-opcode, which is in fact a macro,
introduced to produce the most adequate opcode for the exact ARM
architecture; it is defined in arch/arm/include/asm/assembler.h.
Currently, defining __ARM_ARCH_5TE__ causes 'ret lr' to be encoded
as 'mov pc, lr' rather than the 'bx lr' which should work.

I will have access again to my Open-RD board later today; I will test
if making 'ret' use 'bx' for ARMv5TE works, and also if the current
Linux version of arch/arm/include/asm/assembler.h can be used with
current U-Boot, but that would be a wider change.

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

[U-Boot] [PATCH] openrd: disable private arch memset, memcpy and libgcc

[Albert ARIBAUD]

The switch to private LIBGCC causes 'undefined instruction'
exception in at least the Open-RD Ultimate and Client, and
quite probably the Open-RD Base as well.

While debugging this issue, it appeared the switch to
architecture-optimized memset and memcpy also causes the
same exceptions.

Until a fix to private libgcc and architecture-optimized
memcpy and memset is mainlined, disable their use in all
openrd configurations.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---

 configs/openrd_base_defconfig | 4 
 configs/openrd_client_defconfig   | 4 
 configs/openrd_ultimate_defconfig | 4 
 3 files changed, 12 insertions(+)

diff --git a/configs/openrd_base_defconfig b/configs/openrd_base_defconfig
index 067ddbc5cc..4b8c51a771 100644
--- a/configs/openrd_base_defconfig
+++ b/configs/openrd_base_defconfig
@@ -21,3 +21,7 @@ CONFIG_SYS_NS16550=y
 CONFIG_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_OF_LIBFDT=y
+# Private memcpy, memset and libgcc cause undefined instruction exceptions
+CONFIG_USE_ARCH_MEMSET=n
+CONFIG_USE_ARCH_MEMCPY=n
+CONFIG_USE_PRIVATE_LIBGCC=n
diff --git a/configs/openrd_client_defconfig b/configs/openrd_client_defconfig
index b90ead158f..04f243a57b 100644
--- a/configs/openrd_client_defconfig
+++ b/configs/openrd_client_defconfig
@@ -21,3 +21,7 @@ CONFIG_SYS_NS16550=y
 CONFIG_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_OF_LIBFDT=y
+# Private memcpy, memset and libgcc cause undefined instruction exceptions
+CONFIG_USE_ARCH_MEMSET=n
+CONFIG_USE_ARCH_MEMCPY=n
+CONFIG_USE_PRIVATE_LIBGCC=n
diff --git a/configs/openrd_ultimate_defconfig 
b/configs/openrd_ultimate_defconfig
index 2bc8ace8d0..c8d4c3176f 100644
--- a/configs/openrd_ultimate_defconfig
+++ b/configs/openrd_ultimate_defconfig
@@ -21,3 +21,7 @@ CONFIG_SYS_NS16550=y
 CONFIG_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_OF_LIBFDT=y
+# Private memcpy, memset and libgcc cause undefined instruction exceptions
+CONFIG_USE_ARCH_MEMSET=n
+CONFIG_USE_ARCH_MEMCPY=n
+CONFIG_USE_PRIVATE_LIBGCC=n
-- 
2.11.0

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

Re: [U-Boot] [PATCH] armv7m: Add SysTick timer driver

[Albert ARIBAUD]

Hi Phil,

Actually this is not my /new/ e-mail address, the u-boot one should
work. I am in the process of fixing my access to it.

Le Fri,  3 Feb 2017 14:53:06 +, Phil Edworthy
<phil.edwor...@renesas.com> a écrit :

> The SysTick is a 24-bit down counter that is found on all ARM Cortex
> M3, M4, M7 devices and is always located at a fixed address.
> 
> Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> ---
> Resend with Albert's new email address - get_maintainer.pl gave the old one.
> ---

Cordialement,
Albert ARIBAUD
3ADEV
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] pcm052: fix DDR initialization sequence

[Albert ARIBAUD (3ADEV)]

The sequence erroneously launched the DDR controller
initialization before the pad muxing was done, causing
DRAM size computation to hang.

Configuring the pads first then launching DDR controller
initialization prevents the DRAM hanging.

Signed-off-by: Albert ARIBAUD (3ADEV) <albert.arib...@3adev.fr>
---
 board/phytec/pcm052/pcm052.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/board/phytec/pcm052/pcm052.c b/board/phytec/pcm052/pcm052.c
index e75ff4fc3a..54a4e4f9c3 100644
--- a/board/phytec/pcm052/pcm052.c
+++ b/board/phytec/pcm052/pcm052.c
@@ -258,8 +258,7 @@ int dram_init(void)
.wldqsen   = 25,
};
 
-   ddrmc_ctrl_init_ddr3(_ddr_timings, pcm052_cr_settings,
-pcm052_phy_settings, 1, 2);
+const int row_diff = 2;
 
 #elif defined(CONFIG_TARGET_BK4R1)
 
@@ -314,8 +313,7 @@ int dram_init(void)
.wldqsen   = 25,
};
 
-   ddrmc_ctrl_init_ddr3(_ddr_timings, pcm052_cr_settings,
-pcm052_phy_settings, 1, 1);
+const int row_diff = 1;
 
 #else /* Unknown PCM052 variant */
 
@@ -325,6 +323,9 @@ int dram_init(void)
 
imx_iomux_v3_setup_multiple_pads(pcm052_pads, ARRAY_SIZE(pcm052_pads));
 
+   ddrmc_ctrl_init_ddr3(_ddr_timings, pcm052_cr_settings,
+pcm052_phy_settings, 1, row_diff);
+
gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
 
return 0;
-- 
2.11.0

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

Re: [U-Boot] [PATCH 1/2] arm: Remove unofficial mach-type number uses

[Albert ARIBAUD]

Hi Vladimir,

Le Wed, 25 Jan 2017 03:34:13 +0200, Vladimir Zapolskiy <v...@mleia.com> a
écrit :

> Hypothetically it could be possible that a machine maintainer finds that
> after rebasing private changes on top of the latest Linux release, a board
> mach type is not recognized by the Linux kernel due to an expired and
> removed entry in mach-types, then s/he updates a record in Russell's DB
> to get the old machine type ID again. However a counterpart change in
> U-boot board config won't be synced automatically, and IMHO it should be
> stated that this problem is neglected, obviously because it causes
> maintenance burden for no gain.

I suspect there's also a case where a custom (non-mainline) kernel is
used on a given board and expects a machine ID from U-Boot, but this
machine ID is not required to be in RMK's list. Ugly, but such things
happen.

> That said, your change is good, it may produce a discontent in particular
> cases, but it won't be problematic to remove it by partial commit
> reverting and adding an explanatory comment into the code.

Agreed.

> --
> With best wishes,
> Vladimir

Cordialement,
Albert ARIBAUD
3ADEV
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 0/6] Add support for the BK4R1 variant of PCM052

[Albert ARIBAUD]

Hi Stefano,

Sorry for the delay.

Le Fri, 7 Oct 2016 12:01:02 +0200, Stefano Babic <sba...@denx.de> a
écrit :

> Hi Albert,
> 
> On 06/10/2016 15:43, Albert ARIBAUD wrote:
> 
> > 
> > Hmm... What U-Boot commit do you apply above?  
> 
> It was on top of v2016.11-rc1, and then I have already applied several
> patches for i.MX.
> 
> I am very sorry for that: generally, I check the patches in a separate
> local branch, but it seemed I made a mistake and I have applied it on my
> -master, and after my last push they are already on the server. It was
> not my intention. Of course, I will revert them back if you do not like
> / disagree.

No problem.

I could not reproduce the problem on my side (i.e., origin/master
buildman builds bk4r1 without any warning or error), but anyway, your
change appears harmless enough, so I'm fine with you adding it.

Thanks!

Cordialement,
Albert ARIBAUD
3ADEV
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 0/6] Add support for the BK4R1 variant of PCM052

[Albert ARIBAUD]

Hi Stefano,

Le Thu, 6 Oct 2016 09:21:13 +0200, Stefano Babic <sba...@denx.de> a
écrit :

> Hi Albert,
> 
> On 26/09/2016 09:08, Albert ARIBAUD (3ADEV) wrote:
> > BK4R1 is basically PCM052 with the following differences
> > or quirks:
> > 
> >   1) it has 512MB of DDR using MT41K256M16HA_125IT,
> >  while the PCM052 has 256MB using MT41J128M16HA_15EIT;
> > 
> >   2) it has 1GB of NAND. The size increase is supported
> >  by the env directly;
> > 
> >   3) its Ethernet ports are physicaly tied together until
> >  GPIO 122 is raised. As this is a safety feature U-Boot
> >  does not untie the ports except if it needs networking,
> >  for instance when doing NAND updates via TFTP;
> > 
> >   4) it has a USB hub which may remain in reset if GPIO 130
> >  is not raised. This is done unconditionally at boot;
> > 
> >   5) It has two NOR SPI flash chips on QSPI.
> > 
> > This series has been run through checkpatch and has no errors
> > or warning except the following one:
> > 
> > warning: arch/arm/Kconfig,681: please write a
> > paragraph that describes the config symbol fully
> > 
> > Which I believe does not apply, as target configs in this file
> > never have descriptions.
> > 
> > 
> > Albert ARIBAUD (3ADEV) (6):
> >   pcm052: fix MTD partitioning
> >   pcm052: remove target-specific dtb name from env
> >   pcm052: add 'm4go' command
> >   tools: mkimage: add support for Vybrid image format
> >   pcm052: allow specifying onboard DDR size in configs
> >   pcm052: add new BK4r1 target based on PCM052 SoM
> > 
> >  Makefile  |   6 ++
> >  arch/arm/Kconfig  |   4 +
> >  arch/arm/config.mk|   3 +
> >  arch/arm/cpu/armv7/vf610/Makefile |   5 +
> >  arch/arm/dts/Makefile |   3 +-
> >  arch/arm/dts/bk4r1.dts|  48 +
> >  arch/arm/dts/vf.dtsi  |   4 +-
> >  board/phytec/pcm052/Kconfig   |  24 +
> >  board/phytec/pcm052/pcm052.c  | 206 
> > --
> >  common/image.c|   1 +
> >  configs/bk4r1_defconfig   |  32 ++
> >  include/configs/bk4r1.h   |  33 ++
> >  include/configs/pcm052.h  |  78 ++-
> >  include/image.h   |   1 +
> >  tools/Makefile|   1 +
> >  tools/vybridimage.c   | 164 ++
> >  16 files changed, 535 insertions(+), 78 deletions(-)
> >  create mode 100644 arch/arm/dts/bk4r1.dts
> >  create mode 100644 configs/bk4r1_defconfig
> >  create mode 100644 include/configs/bk4r1.h
> >  create mode 100644 tools/vybridimage.c
> >   
> 
> It looks like that CONFIG_CMD_UBI for bk4r1 is not set and I get build
> errors.

Hmm... What U-Boot commit do you apply above?

> Is it ok for you if I add directly this by applying ?

I'll check this and let you know later today.

> diff --git a/configs/bk4r1_defconfig b/configs/bk4r1_defconfig
> index 3994459..26d9e81 100644
> --- a/configs/bk4r1_defconfig
> +++ b/configs/bk4r1_defconfig
> @@ -30,3 +30,4 @@ CONFIG_SPI_FLASH=y
>  CONFIG_SPI_FLASH_STMICRO=y
>  CONFIG_SPI_FLASH_MTD=y
>  CONFIG_CMD_DM=y
> +CONFIG_CMD_UBI=y
> 
> Regards,
> Stefano
> 



Cordialement,
Albert ARIBAUD
3ADEV
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/6] Add support for the BK4R1 variant of PCM052

[Albert ARIBAUD (3ADEV)]

BK4R1 is basically PCM052 with the following differences
or quirks:

  1) it has 512MB of DDR using MT41K256M16HA_125IT,
 while the PCM052 has 256MB using MT41J128M16HA_15EIT;

  2) it has 1GB of NAND. The size increase is supported
 by the env directly;

  3) its Ethernet ports are physicaly tied together until
 GPIO 122 is raised. As this is a safety feature U-Boot
 does not untie the ports except if it needs networking,
 for instance when doing NAND updates via TFTP;

  4) it has a USB hub which may remain in reset if GPIO 130
 is not raised. This is done unconditionally at boot;

  5) It has two NOR SPI flash chips on QSPI.

This series has been run through checkpatch and has no errors
or warning except the following one:

warning: arch/arm/Kconfig,681: please write a
paragraph that describes the config symbol fully

Which I believe does not apply, as target configs in this file
never have descriptions.


Albert ARIBAUD (3ADEV) (6):
  pcm052: fix MTD partitioning
  pcm052: remove target-specific dtb name from env
  pcm052: add 'm4go' command
  tools: mkimage: add support for Vybrid image format
  pcm052: allow specifying onboard DDR size in configs
  pcm052: add new BK4r1 target based on PCM052 SoM

 Makefile  |   6 ++
 arch/arm/Kconfig  |   4 +
 arch/arm/config.mk|   3 +
 arch/arm/cpu/armv7/vf610/Makefile |   5 +
 arch/arm/dts/Makefile |   3 +-
 arch/arm/dts/bk4r1.dts|  48 +
 arch/arm/dts/vf.dtsi  |   4 +-
 board/phytec/pcm052/Kconfig   |  24 +
 board/phytec/pcm052/pcm052.c  | 206 --
 common/image.c|   1 +
 configs/bk4r1_defconfig   |  32 ++
 include/configs/bk4r1.h   |  33 ++
 include/configs/pcm052.h  |  78 ++-
 include/image.h   |   1 +
 tools/Makefile|   1 +
 tools/vybridimage.c   | 164 ++
 16 files changed, 535 insertions(+), 78 deletions(-)
 create mode 100644 arch/arm/dts/bk4r1.dts
 create mode 100644 configs/bk4r1_defconfig
 create mode 100644 include/configs/bk4r1.h
 create mode 100644 tools/vybridimage.c

-- 
2.9.3

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

[U-Boot] [PATCH 6/6] pcm052: add new BK4r1 target based on PCM052 SoM

[Albert ARIBAUD (3ADEV)]

Signed-off-by: Albert ARIBAUD (3ADEV) <albert.arib...@3adev.fr>
---

 arch/arm/Kconfig |   4 ++
 arch/arm/dts/Makefile|   3 +-
 arch/arm/dts/bk4r1.dts   |  48 +
 arch/arm/dts/vf.dtsi |   4 +-
 board/phytec/pcm052/Kconfig  |  20 ++
 board/phytec/pcm052/pcm052.c | 168 +--
 configs/bk4r1_defconfig  |  32 +
 include/configs/bk4r1.h  |  33 +
 include/configs/pcm052.h |  45 ++--
 9 files changed, 297 insertions(+), 60 deletions(-)
 create mode 100644 arch/arm/dts/bk4r1.dts
 create mode 100644 configs/bk4r1_defconfig
 create mode 100644 include/configs/bk4r1.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0083bf9..3c2d33a 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -678,6 +678,10 @@ config TARGET_PCM052
bool "Support pcm-052"
select CPU_V7
 
+config TARGET_BK4R1
+   bool "Support BK4r1"
+   select CPU_V7
+
 config ARCH_ZYNQ
bool "Xilinx Zynq Platform"
select CPU_V7
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index df57288..3e3b5c3 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -274,7 +274,8 @@ dtb-$(CONFIG_MACH_SUN9I) += \
 dtb-$(CONFIG_VF610) += vf500-colibri.dtb \
vf610-colibri.dtb \
vf610-twr.dtb \
-   pcm052.dtb
+   pcm052.dtb \
+   bk4r1.dtb
 
 dtb-$(CONFIG_SOC_KEYSTONE) += k2hk-evm.dtb \
k2l-evm.dtb \
diff --git a/arch/arm/dts/bk4r1.dts b/arch/arm/dts/bk4r1.dts
new file mode 100644
index 000..197e5ab
--- /dev/null
+++ b/arch/arm/dts/bk4r1.dts
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2016 Toradex AG
+ *
+ * SPDX-License-Identifier: GPL-2.0+ or X11
+ */
+
+/dts-v1/;
+#include "vf.dtsi"
+
+/ {
+   model = "Phytec phyCORE-Vybrid";
+   compatible = "phytec,pcm052", "fsl,vf610";
+
+   chosen {
+   stdout-path = 
+   };
+
+   aliases {
+   spi0 = 
+   };
+
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   bus-num = <0>;
+   num-cs = <2>;
+   status = "okay";
+
+   qflash0: spi_flash@0 {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   compatible = "spi-flash";
+   spi-max-frequency = <10800>;
+   reg = <0>;
+   };
+
+   qflash1: spi_flash@1 {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   compatible = "spi-flash";
+   spi-max-frequency = <6600>;
+   reg = <1>;
+   };
+};
diff --git a/arch/arm/dts/vf.dtsi b/arch/arm/dts/vf.dtsi
index 1530d2f..404dfe9 100644
--- a/arch/arm/dts/vf.dtsi
+++ b/arch/arm/dts/vf.dtsi
@@ -80,7 +80,9 @@
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,vf610-qspi";
-   reg = <0x40044000 0x1000>;
+   reg = <0x40044000 0x1000>,
+ <0x2000 0x1000>;
+   reg-names = "QuadSPI", "QuadSPI-memory";
status = "disabled";
};
 
diff --git a/board/phytec/pcm052/Kconfig b/board/phytec/pcm052/Kconfig
index 88524a3..212f994 100644
--- a/board/phytec/pcm052/Kconfig
+++ b/board/phytec/pcm052/Kconfig
@@ -17,3 +17,23 @@ config PCM052_DDR_SIZE
default 256
 
 endif
+
+if TARGET_BK4R1
+
+config SYS_BOARD
+   default "pcm052"
+
+config SYS_VENDOR
+   default "phytec"
+
+config SYS_SOC
+   default "vf610"
+
+config SYS_CONFIG_NAME
+   default "bk4r1"
+
+config PCM052_DDR_SIZE
+   int
+   default 512
+
+endif
diff --git a/board/phytec/pcm052/pcm052.c b/board/phytec/pcm052/pcm052.c
index 7341899..e75ff4f 100644
--- a/board/phytec/pcm052/pcm052.c
+++ b/board/phytec/pcm052/pcm052.c
@@ -152,57 +152,6 @@ static struct ddrmc_phy_setting pcm052_phy_settings[] = {
 
 int dram_init(void)
 {
-   static const struct ddr3_jedec_timings pcm052_ddr_timings = {
-   .tinit = 5,
-   .trst_pwron= 8,
-   .cke_inactive  = 20,
-   .wrlat = 5,
-   .caslat_lin= 12,
-   .trc   = 6,
-   .trrd  = 4,
-   .tccd  = 4,
-   .tbst_int_interval = 4,
-   .tfaw  = 18,
-   .trp   = 6,
-   .twtr  = 4,
-   .tras_min  = 15,
-   .tmrd  = 4,
-   .trtp  = 4

[U-Boot] [PATCH 4/6] tools: mkimage: add support for Vybrid image format

[Albert ARIBAUD (3ADEV)]

This format can be flashed directly at address 0 of
the NAND FLASH, as it contains all necessary headers.

Signed-off-by: Albert ARIBAUD (3ADEV) <albert.arib...@3adev.fr>
---

 Makefile  |   6 ++
 arch/arm/config.mk|   3 +
 arch/arm/cpu/armv7/vf610/Makefile |   5 ++
 common/image.c|   1 +
 include/configs/pcm052.h  |  14 ++--
 include/image.h   |   1 +
 tools/Makefile|   1 +
 tools/vybridimage.c   | 164 ++
 8 files changed, 187 insertions(+), 8 deletions(-)
 create mode 100644 tools/vybridimage.c

diff --git a/Makefile b/Makefile
index c30f90a..75c74d5 100644
--- a/Makefile
+++ b/Makefile
@@ -844,6 +844,12 @@ endif
 %.imx: %.bin
$(Q)$(MAKE) $(build)=arch/arm/imx-common $@
 
+%.vyb: %.imx
+   $(Q)$(MAKE) $(build)=arch/arm/cpu/armv7/vf610 $@
+
+quiet_cmd_copy = COPY$@
+  cmd_copy = cp $< $@
+
 u-boot.dtb: dts/dt.dtb
$(call cmd,copy)
 
diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index 8f85862..542b897 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -144,4 +144,7 @@ else
 ALL-y += u-boot.imx
 endif
 endif
+ifneq ($(CONFIG_VF610),)
+ALL-y += u-boot.vyb
+endif
 endif
diff --git a/arch/arm/cpu/armv7/vf610/Makefile 
b/arch/arm/cpu/armv7/vf610/Makefile
index 68cb756..2945377 100644
--- a/arch/arm/cpu/armv7/vf610/Makefile
+++ b/arch/arm/cpu/armv7/vf610/Makefile
@@ -6,3 +6,8 @@
 
 obj-y  += generic.o
 obj-y  += timer.o
+
+MKIMAGEFLAGS_u-boot.vyb = -T vybridimage
+
+u-boot.vyb: u-boot.imx
+   $(call if_changed,mkimage)
diff --git a/common/image.c b/common/image.c
index a5d19ab..c0ad36a 100644
--- a/common/image.c
+++ b/common/image.c
@@ -161,6 +161,7 @@ static const table_entry_t uimage_type[] = {
{   IH_TYPE_RKIMAGE,"rkimage","Rockchip Boot Image" },
{   IH_TYPE_RKSD,   "rksd",   "Rockchip SD Boot Image" },
{   IH_TYPE_RKSPI,  "rkspi",  "Rockchip SPI Boot Image" },
+   {   IH_TYPE_VYBRIDIMAGE, "vybridimage",  "Vybrid Boot Image", },
{   IH_TYPE_ZYNQIMAGE,  "zynqimage",  "Xilinx Zynq Boot Image" },
{   IH_TYPE_ZYNQMPIMAGE, "zynqmpimage", "Xilinx ZynqMP Boot Image" 
},
{   IH_TYPE_FPGA,   "fpga",   "FPGA Image" },
diff --git a/include/configs/pcm052.h b/include/configs/pcm052.h
index 1858662..cd235cc 100644
--- a/include/configs/pcm052.h
+++ b/include/configs/pcm052.h
@@ -120,9 +120,8 @@
 #define CONFIG_EXTRA_ENV_SETTINGS \
"fdt_high=0x\0" \
"initrd_high=0x\0" \
-   "blimg_file=u-boot.imx\0" \
-   "blsec_addr=0x8100\0" \
-   "blimg_addr=0x81000400\0" \
+   "blimg_file=u-boot.vyb\0" \
+   "blimg_addr=0x8100\0" \
"kernel_file=zImage\0" \
"kernel_addr=0x8200\0" \
"fdt_file=zImage.dtb\0" \
@@ -164,12 +163,11 @@
"nand read ${kernel_addr} kernel; " \
"nand read ${ram_addr} root; " \
"bootz ${kernel_addr} ${ram_addr} ${fdt_addr}\0" \
-   "update_bootloader_from_tftp=mtdparts default; " \
-   "nand read ${blsec_addr} bootloader; " \
-   "mw.b ${blimg_addr} 0xff 0x5FC00; " \
-   "if tftp ${blimg_addr} ${tftpdir}${blimg_file}; then " \
+   "update_bootloader_from_tftp=if tftp ${blimg_addr} "\
+   "${tftpdir}${blimg_file}; then " \
+   "mtdparts default; " \
"nand erase.part bootloader; " \
-   "nand write ${blsec_addr} bootloader ${filesize}; fi\0" \
+   "nand write ${blimg_addr} bootloader ${filesize}; fi\0" \
"update_kernel_from_sd=if fatload mmc 0:2 ${kernel_addr} " \
"${kernel_file}; " \
"then mtdparts default; " \
diff --git a/include/image.h b/include/image.h
index 64da722..2b1296c 100644
--- a/include/image.h
+++ b/include/image.h
@@ -278,6 +278,7 @@ enum {
IH_TYPE_ZYNQIMAGE,  /* Xilinx Zynq Boot Image */
IH_TYPE_ZYNQMPIMAGE,/* Xilinx ZynqMP Boot Image */
IH_TYPE_FPGA,   /* FPGA Image */
+   IH_TYPE_VYBRIDIMAGE,/* VYBRID .vyb Image */
 
IH_TYPE_COUNT,  /* Number of image types */
 };
diff --git a/tools/Makefile b/tools/Makefile
index 421414b..e6f7993 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -89,6 +89,7 @@ dumpimage-mkimage-objs := aisimage.o \
os_support.o \
pblimage.o \
   

[U-Boot] [PATCH 5/6] pcm052: allow specifying onboard DDR size in configs

[Albert ARIBAUD (3ADEV)]

PCM052 SoMs may be equipped with various sizes of DDR.
Keep default of 256MB; new PCM052-based targets will
specify their actual DDR size.

Linux command line is auto-adjusted to DDR size.

Signed-off-by: Albert ARIBAUD (3ADEV) <albert.arib...@3adev.fr>
---

 board/phytec/pcm052/Kconfig | 4 
 include/configs/pcm052.h| 5 +++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/board/phytec/pcm052/Kconfig b/board/phytec/pcm052/Kconfig
index d67a69a..88524a3 100644
--- a/board/phytec/pcm052/Kconfig
+++ b/board/phytec/pcm052/Kconfig
@@ -12,4 +12,8 @@ config SYS_SOC
 config SYS_CONFIG_NAME
default "pcm052"
 
+config PCM052_DDR_SIZE
+   int
+   default 256
+
 endif
diff --git a/include/configs/pcm052.h b/include/configs/pcm052.h
index cd235cc..b3e5054 100644
--- a/include/configs/pcm052.h
+++ b/include/configs/pcm052.h
@@ -135,7 +135,8 @@
"tftptimeout=1000\0" \
"tftptimeoutcountmax=100\0" \
"mtdparts=" MTDPARTS_DEFAULT "\0" \
-   "bootargs_base=setenv bootargs rw mem=256M " \
+   "bootargs_base=setenv bootargs rw " \
+   " mem=" __stringify(CONFIG_PCM052_DDR_SIZE) "M " \
"console=ttyLP1,115200n8\0" \
"bootargs_sd=setenv bootargs ${bootargs} " \
"root=/dev/mmcblk0p2 rootwait\0" \
@@ -219,7 +220,7 @@
 /* Physical memory map */
 #define CONFIG_NR_DRAM_BANKS   1
 #define PHYS_SDRAM (0x8000)
-#define PHYS_SDRAM_SIZE(256 * 1024 * 1024)
+#define PHYS_SDRAM_SIZE(CONFIG_PCM052_DDR_SIZE * 1024 
* 1024)
 
 #define CONFIG_SYS_SDRAM_BASE  PHYS_SDRAM
 #define CONFIG_SYS_INIT_RAM_ADDR   IRAM_BASE_ADDR
-- 
2.9.3

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

[U-Boot] [PATCH 2/6] pcm052: remove target-specific dtb name from env

[Albert ARIBAUD (3ADEV)]

Signed-off-by: Albert ARIBAUD (3ADEV) <albert.arib...@3adev.fr>
---

 include/configs/pcm052.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/configs/pcm052.h b/include/configs/pcm052.h
index 302c7dd..1858662 100644
--- a/include/configs/pcm052.h
+++ b/include/configs/pcm052.h
@@ -125,7 +125,7 @@
"blimg_addr=0x81000400\0" \
"kernel_file=zImage\0" \
"kernel_addr=0x8200\0" \
-   "fdt_file=vf610-pcm052.dtb\0" \
+   "fdt_file=zImage.dtb\0" \
"fdt_addr=0x8100\0" \
"ram_file=uRamdisk\0" \
"ram_addr=0x8300\0" \
-- 
2.9.3

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

[U-Boot] [PATCH 3/6] pcm052: add 'm4go' command

[Albert ARIBAUD (3ADEV)]

Add the 'm4go' command to pcm052-based targets.
It loads scatter file images.

Signed-off-by: Albert ARIBAUD (3ADEV) <albert.arib...@3adev.fr>
---

 board/phytec/pcm052/pcm052.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/board/phytec/pcm052/pcm052.c b/board/phytec/pcm052/pcm052.c
index e4f61e1..7341899 100644
--- a/board/phytec/pcm052/pcm052.c
+++ b/board/phytec/pcm052/pcm052.c
@@ -513,3 +513,41 @@ int checkboard(void)
 
return 0;
 }
+
+static int do_m4go(cmd_tbl_t *cmdtp, int flag, int argc,
+  char * const argv[])
+{
+   ulong addr;
+
+   /* Consume 'm4go' */
+   argc--; argv++;
+
+   /*
+* Parse provided address - default to load_addr in case not provided.
+*/
+
+   if (argc)
+   addr = simple_strtoul(argv[0], NULL, 16);
+   else
+   addr = load_addr;
+
+   /*
+* Write boot address in PERSISTENT_ENTRY1[31:0] aka SRC_GPR2[31:0]
+*/
+   writel(addr + 0x401, 0x4006E028);
+
+   /*
+* Start secondary processor by enabling its clock
+*/
+   writel(0x15a5a, 0x4006B08C);
+
+   return 1;
+}
+
+U_BOOT_CMD(
+   m4go, 2 /* one arg max */, 1 /* repeatable */, do_m4go,
+   "start the secondary Cortex-M4 from scatter file image",
+   "[]\n"
+   "- start secondary Cortex-M4 core using a scatter file image\n"
+   "The argument needs to be a scatter file\n"
+);
-- 
2.9.3

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

[U-Boot] [PATCH 1/6] pcm052: fix MTD partitioning

[Albert ARIBAUD (3ADEV)]

Merge 'spare' into 'bootloader' partition
Use same partition for ramdisk and rootfs boot scenarios.
Remove 'ramdisk' partition, use 'rootfs' for ramdisk
(ramdisk and nand boot scenarios are mutually exclusive).
Expand last partition to end of actual NAND size.
Adjust UBIFS rootfs boot kernel arguments.

Signed-off-by: Albert ARIBAUD (3ADEV) <albert.arib...@3adev.fr>
---

 include/configs/pcm052.h | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/configs/pcm052.h b/include/configs/pcm052.h
index 57a7630..302c7dd 100644
--- a/include/configs/pcm052.h
+++ b/include/configs/pcm052.h
@@ -54,14 +54,12 @@
 #define CONFIG_MTD_PARTITIONS
 #define CONFIG_MTD_DEVICE
 #define MTDIDS_DEFAULT "nand0=NAND"
-#define MTDPARTS_DEFAULT   "mtdparts=NAND:256k(spare)"\
-   ",384k(bootloader)"\
+#define MTDPARTS_DEFAULT   "mtdparts=NAND:640k(bootloader)"\
",128k(env1)"\
",128k(env2)"\
",128k(dtb)"\
",6144k(kernel)"\
-   ",65536k(ramdisk)"\
-   ",450944k(root)"
+   ",-(root)"
 #endif
 
 #define CONFIG_MMC
@@ -145,7 +143,7 @@
"bootargs_net=setenv bootargs ${bootargs} root=/dev/nfs ip=dhcp " \
"nfsroot=${serverip}:${nfs_root},v3,tcp\0" \
"bootargs_nand=setenv bootargs ${bootargs} " \
-   "ubi.mtd=6 rootfstype=ubifs root=ubi0:rootfs\0" \
+   "ubi.mtd=5 rootfstype=ubifs root=ubi0:rootfs\0" \
"bootargs_ram=setenv bootargs ${bootargs} " \
"root=/dev/ram rw initrd=${ram_addr}\0" \
"bootargs_mtd=setenv bootargs ${bootargs} ${mtdparts}\0" \
@@ -164,7 +162,7 @@
"bootcmd_ram=run bootargs_base bootargs_ram bootargs_mtd; " \
"nand read ${fdt_addr} dtb; " \
"nand read ${kernel_addr} kernel; " \
-   "nand read ${ram_addr} ramdisk; " \
+   "nand read ${ram_addr} root; " \
"bootz ${kernel_addr} ${ram_addr} ${fdt_addr}\0" \
"update_bootloader_from_tftp=mtdparts default; " \
"nand read ${blsec_addr} bootloader; " \
@@ -196,8 +194,8 @@
"ubi write ${sys_addr} rootfs ${filesize}; fi\0" \
"update_ramdisk_from_tftp=if tftp ${ram_addr} ${tftpdir}${ram_file}; " \
"then mtdparts default; " \
-   "nand erase.part ramdisk; " \
-   "nand write ${ram_addr} ramdisk ${filesize}; fi\0"
+   "nand erase.part root; " \
+   "nand write ${ram_addr} root ${filesize}; fi\0"
 
 /* Miscellaneous configurable options */
 #define CONFIG_SYS_LONGHELP/* undef to save memory */
-- 
2.9.3

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

Re: [U-Boot] [PATCH] Revert "arm: Remove wireless_space board"

[Albert ARIBAUD]

On Sun, 31 Jan 2016 21:37:04 +0100, Albert ARIBAUD <albert.u.b...@aribaud.net> 
wrote:
> Revert commit b352182a and complete generic board support.
> 
> Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
> ---
>  arch/arm/mach-kirkwood/Kconfig  |   4 +
>  board/LaCie/wireless_space/Kconfig  |  12 ++
>  board/LaCie/wireless_space/MAINTAINERS  |   6 +
>  board/LaCie/wireless_space/Makefile |  12 ++
>  board/LaCie/wireless_space/kwbimage.cfg |  71 +++
>  board/LaCie/wireless_space/wireless_space.c | 165 +
>  configs/wireless_space_defconfig|   7 ++
>  include/configs/wireless_space.h| 179 
> 
>  8 files changed, 456 insertions(+)
>  create mode 100644 board/LaCie/wireless_space/Kconfig
>  create mode 100644 board/LaCie/wireless_space/MAINTAINERS
>  create mode 100644 board/LaCie/wireless_space/Makefile
>  create mode 100644 board/LaCie/wireless_space/kwbimage.cfg
>  create mode 100644 board/LaCie/wireless_space/wireless_space.c
>  create mode 100644 configs/wireless_space_defconfig
>  create mode 100644 include/configs/wireless_space.h
> 
> diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> index 9205b1e..9249564 100644
> --- a/arch/arm/mach-kirkwood/Kconfig
> +++ b/arch/arm/mach-kirkwood/Kconfig
> @@ -37,6 +37,9 @@ config TARGET_NET2BIG_V2
>  config TARGET_NETSPACE_V2
>   bool "LaCie netspace_v2 Board"
>  
> +config TARGET_WIRELESS_SPACE
> + bool "LaCie Wireless_space Board"
> +
>  config TARGET_IB62X0
>   bool "ib62x0 Board"
>  
> @@ -68,6 +71,7 @@ source "board/iomega/iconnect/Kconfig"
>  source "board/keymile/km_arm/Kconfig"
>  source "board/LaCie/net2big_v2/Kconfig"
>  source "board/LaCie/netspace_v2/Kconfig"
> +source "board/LaCie/wireless_space/Kconfig"
>  source "board/raidsonic/ib62x0/Kconfig"
>  source "board/Seagate/dockstar/Kconfig"
>  source "board/Seagate/goflexhome/Kconfig"
> diff --git a/board/LaCie/wireless_space/Kconfig 
> b/board/LaCie/wireless_space/Kconfig
> new file mode 100644
> index 000..75a2fc5
> --- /dev/null
> +++ b/board/LaCie/wireless_space/Kconfig
> @@ -0,0 +1,12 @@
> +if TARGET_WIRELESS_SPACE
> +
> +config SYS_BOARD
> + default "wireless_space"
> +
> +config SYS_VENDOR
> + default "LaCie"
> +
> +config SYS_CONFIG_NAME
> + default "wireless_space"
> +
> +endif
> diff --git a/board/LaCie/wireless_space/MAINTAINERS 
> b/board/LaCie/wireless_space/MAINTAINERS
> new file mode 100644
> index 000..c32ecb8
> --- /dev/null
> +++ b/board/LaCie/wireless_space/MAINTAINERS
> @@ -0,0 +1,6 @@
> +WIRELESS_SPACE BOARD
> +M:   Albert ARIBAUD <albert.u.b...@aribaud.net>
> +S:   Maintained
> +F:   board/LaCie/wireless_space/
> +F:   include/configs/wireless_space.h
> +F:   configs/wireless_space_defconfig
> diff --git a/board/LaCie/wireless_space/Makefile 
> b/board/LaCie/wireless_space/Makefile
> new file mode 100644
> index 000..90a84f4
> --- /dev/null
> +++ b/board/LaCie/wireless_space/Makefile
> @@ -0,0 +1,12 @@
> +#
> +# Copyright (C) 2011 Simon Guinot <sgui...@lacie.com>
> +#
> +# Based on Kirkwood support:
> +# (C) Copyright 2009
> +# Marvell Semiconductor 
> +# Written-by: Prafulla Wadaskar <prafu...@marvell.com>
> +#
> +# SPDX-License-Identifier:   GPL-2.0+
> +#
> +
> +obj-y:= wireless_space.o ../common/common.o
> diff --git a/board/LaCie/wireless_space/kwbimage.cfg 
> b/board/LaCie/wireless_space/kwbimage.cfg
> new file mode 100644
> index 000..037248b
> --- /dev/null
> +++ b/board/LaCie/wireless_space/kwbimage.cfg
> @@ -0,0 +1,71 @@
> +#
> +# Copyright (C) 2012 Albert ARIBAUD <albert.u.b...@aribaud.net>
> +#
> +# Based on netspace_v2 kwbimage.cfg:
> +# Copyright (C) 2011 Simon Guinot <sgui...@lacie.com>
> +#
> +# Based on Kirkwood support:
> +# (C) Copyright 2009
> +# Marvell Semiconductor 
> +# Written-by: Prafulla Wadaskar <prafu...@marvell.com>
> +#
> +# SPDX-License-Identifier:   GPL-2.0+
> +#
> +# Refer doc/README.kwbimage for more details about how-to configure
> +# and create kirkwood boot image
> +#
> +
> +# Boot Media configurations
> +BOOT_FROMnand# Boot from NAND flash
> +NAND_PAGE_SIZE 800
> +
> +# SOC registers configuration using bootrom header extension
> +# Maximum KWBIMAGE_MAX_CONFIG configurations allowed
> +
> +# Values taken from image original LaCie U-Boot header dump!
> +
> +# Configu

Re: [U-Boot] [PATCH 5/5] lib: Enable private libgcc by default

[Albert ARIBAUD]

Hello Tom,

On Sun, 27 Mar 2016 09:36:41 -0400, Tom Rini <tr...@konsulko.com> wrote:
> On Fri, Mar 25, 2016 at 07:37:25AM +0100, Albert ARIBAUD wrote:
> > Hello Tom,
> > 
> > On Thu, 24 Mar 2016 20:49:42 -0400, Tom Rini <tr...@konsulko.com> wrote:
> > > On Thu, Mar 24, 2016 at 08:50:03AM +0100, Albert ARIBAUD wrote:
> > > > Hello Tom,
> > > > 
> > > > On Wed, 23 Mar 2016 17:36:17 -0400, Tom Rini <tr...@konsulko.com> wrote:
> > > > > On Wed, Mar 23, 2016 at 06:08:45PM +0100, Albert ARIBAUD wrote:
> > > > > > Hello Tom,
> > > > > > 
> > > > > > On Wed, 23 Mar 2016 09:22:38 -0400, Tom Rini <tr...@konsulko.com> 
> > > > > > wrote:
> > > > > > > On Wed, Mar 23, 2016 at 01:53:35PM +0100, Albert ARIBAUD wrote:
> > > > > > > > Hello Marek,
> > > > > > > > 
> > > > > > > > On Sun, 20 Mar 2016 17:15:34 +0100, Marek Vasut <ma...@denx.de> 
> > > > > > > > wrote:
> > > > > > > > > This patch decouples U-Boot binary from the toolchain on 
> > > > > > > > > systems where
> > > > > > > > > private libgcc is available. Instead of pulling in functions 
> > > > > > > > > provided
> > > > > > > > > by the libgcc from the toolchain, U-Boot will use it's own 
> > > > > > > > > set of libgcc
> > > > > > > > > functions. These functions are usually imported from Linux 
> > > > > > > > > kernel, which
> > > > > > > > > also uses it's own libgcc functions instead of the ones 
> > > > > > > > > provided by the
> > > > > > > > > toolchain.
> > > > > > > > > 
> > > > > > > > > This patch solves a rather common problem. The toolchain can 
> > > > > > > > > usually
> > > > > > > > > generate code for many variants of target architecture and 
> > > > > > > > > often even
> > > > > > > > > different endianness. The libgcc on the other hand is usually 
> > > > > > > > > compiled
> > > > > > > > > for one particular configuration and the functions provided 
> > > > > > > > > by it may
> > > > > > > > > or may not be suited for use in U-Boot. This can manifest in 
> > > > > > > > > two ways,
> > > > > > > > > either the U-Boot fails to compile altogether and linker will 
> > > > > > > > > complain
> > > > > > > > > or, in the much worse case, the resulting U-Boot will build, 
> > > > > > > > > but will
> > > > > > > > > misbehave in very subtle and hard to debug ways.
> > > > > > > > 
> > > > > > > > I don't think using private libgcc by default is a good idea.
> > > > > > > > 
> > > > > > > > U-Boot's private libgcc is not a feature of U-Boot, but a fix 
> > > > > > > > for some
> > > > > > > > cases where a target cannot properly link with the libgcc 
> > > > > > > > provided by
> > > > > > > > the (specific release of the) GCC toolchain in use. Using 
> > > > > > > > private libgcc
> > > > > > > > to other cases than these does not fix or improve anything; 
> > > > > > > > those
> > > > > > > > other cases were working and did not require any fix in this 
> > > > > > > > respect. 
> > > > > > > 
> > > > > > > This isn't true, exactly.  If using clang for example everyone 
> > > > > > > needs to
> > > > > > > enable this code.  We're also using -fno-builtin -ffreestanding 
> > > > > > > which
> > > > > > > should limit the amount of interference from the toolchain.  And 
> > > > > > > we get
> > > > > > > that.
> > > > > > 
> > > > > > You mean clang does not produce self-sustained binaries?
> > > > > 
> > > > > clang does not provide "libgcc", so there's no -lgcc providing all

Re: [U-Boot] [PATCH 5/5] lib: Enable private libgcc by default

[Albert ARIBAUD]

On Fri, 25 Mar 2016 07:37:25 +0100, Albert ARIBAUD
<albert.u.b...@aribaud.net> wrote:
> Hello Tom,

> That way,
> 
> 0) U-Boot gets the stable and controlled AEABI support you want;
> 
> 1) GCC keeps its somewhat stable but uncontrolled internal "generated
>code / libgcc" interface;
> 
> 2) U-Boot won't interfere with non-aeabi-related stuff in GCC+libgcc,
>i.e. whatever ibgcc-related but non-AEABI-related changes occur in
>a GCC release, we won't break them changes in non-AEABI ;
> 
> 3) GCC+libgcc won't interfere with AEABI any more, i.e. whatever AEABI
>breakages happen in a given GCC toolchain will not break U-Boot.
> 
> 4) This design works with any ARM toolchain -- which is kind of evident
>since it separates generic ARM EABI support from specific toolchain
>support.

Addition: this does not mean we should get rid of the private libgcc
support: it can be useful in case of an issue with the non-aeabi part
of libgcc.

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

Re: [U-Boot] [PATCH 5/5] lib: Enable private libgcc by default

[Albert ARIBAUD]

Hello Sergey,

On Thu, 24 Mar 2016 18:37:52 -0700 (PDT), Sergey Kubushyn
 wrote:
> On Thu, 24 Mar 2016, Tom Rini wrote:

> U-Boot is a standalone program not supposed to coexist with any external
> applications i.e. it is totally self-sufficient, not living in some kind of
> system environment so it makes perfect sense for it not to use _ANY_
> external parts in the final binary.

Granted U-Boot is standalone as a binary system component, but this
binary, as the produce of GCC, is dependent on libgcc for more than
simply AEABI support, hence my proposal.

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

Re: [U-Boot] [PATCH 5/5] lib: Enable private libgcc by default

[Albert ARIBAUD]

Hello Tom,

On Thu, 24 Mar 2016 20:49:42 -0400, Tom Rini <tr...@konsulko.com> wrote:
> On Thu, Mar 24, 2016 at 08:50:03AM +0100, Albert ARIBAUD wrote:
> > Hello Tom,
> > 
> > On Wed, 23 Mar 2016 17:36:17 -0400, Tom Rini <tr...@konsulko.com> wrote:
> > > On Wed, Mar 23, 2016 at 06:08:45PM +0100, Albert ARIBAUD wrote:
> > > > Hello Tom,
> > > > 
> > > > On Wed, 23 Mar 2016 09:22:38 -0400, Tom Rini <tr...@konsulko.com> wrote:
> > > > > On Wed, Mar 23, 2016 at 01:53:35PM +0100, Albert ARIBAUD wrote:
> > > > > > Hello Marek,
> > > > > > 
> > > > > > On Sun, 20 Mar 2016 17:15:34 +0100, Marek Vasut <ma...@denx.de> 
> > > > > > wrote:
> > > > > > > This patch decouples U-Boot binary from the toolchain on systems 
> > > > > > > where
> > > > > > > private libgcc is available. Instead of pulling in functions 
> > > > > > > provided
> > > > > > > by the libgcc from the toolchain, U-Boot will use it's own set of 
> > > > > > > libgcc
> > > > > > > functions. These functions are usually imported from Linux 
> > > > > > > kernel, which
> > > > > > > also uses it's own libgcc functions instead of the ones provided 
> > > > > > > by the
> > > > > > > toolchain.
> > > > > > > 
> > > > > > > This patch solves a rather common problem. The toolchain can 
> > > > > > > usually
> > > > > > > generate code for many variants of target architecture and often 
> > > > > > > even
> > > > > > > different endianness. The libgcc on the other hand is usually 
> > > > > > > compiled
> > > > > > > for one particular configuration and the functions provided by it 
> > > > > > > may
> > > > > > > or may not be suited for use in U-Boot. This can manifest in two 
> > > > > > > ways,
> > > > > > > either the U-Boot fails to compile altogether and linker will 
> > > > > > > complain
> > > > > > > or, in the much worse case, the resulting U-Boot will build, but 
> > > > > > > will
> > > > > > > misbehave in very subtle and hard to debug ways.
> > > > > > 
> > > > > > I don't think using private libgcc by default is a good idea.
> > > > > > 
> > > > > > U-Boot's private libgcc is not a feature of U-Boot, but a fix for 
> > > > > > some
> > > > > > cases where a target cannot properly link with the libgcc provided 
> > > > > > by
> > > > > > the (specific release of the) GCC toolchain in use. Using private 
> > > > > > libgcc
> > > > > > to other cases than these does not fix or improve anything; those
> > > > > > other cases were working and did not require any fix in this 
> > > > > > respect. 
> > > > > 
> > > > > This isn't true, exactly.  If using clang for example everyone needs 
> > > > > to
> > > > > enable this code.  We're also using -fno-builtin -ffreestanding which
> > > > > should limit the amount of interference from the toolchain.  And we 
> > > > > get
> > > > > that.
> > > > 
> > > > You mean clang does not produce self-sustained binaries?
> > > 
> > > clang does not provide "libgcc", so there's no -lgcc providing all of
> > > the functions that are (today) in:
> > > _ashldi3.S _ashrdi3.S _divsi3.S  _lshrdi3.S _modsi3.S _udivsi3.S
> > > _umodsi3.S div0.S  _uldivmod.S
> > > which aside from __modsi3 and __umodsi3 are all __aeabi_xxx
> > 
> > (ok, that explains what you mean by AEABI functions -- those are
> > actually not functions defined by the AEABI, but functions that the GCC
> > folks prefixed with __aeabi.)
> 
> No.  For reference,
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0043d/IHI0043D_rtabi.pdf
> and chapter 4 is all about the support library.  We are entirely in our
> right to do either of (a) use the compiler-provided library (b) provide
> our own implementation of what we need.  The kernel opts for (b) and I
> would like us to follow that as well, consistently, rather than ad-hoc.

Kk, so you did not mean "whatever happens to

Re: [U-Boot] [PATCH 5/5] lib: Enable private libgcc by default

[Albert ARIBAUD]

Hello Tom,

On Wed, 23 Mar 2016 17:36:17 -0400, Tom Rini <tr...@konsulko.com> wrote:
> On Wed, Mar 23, 2016 at 06:08:45PM +0100, Albert ARIBAUD wrote:
> > Hello Tom,
> > 
> > On Wed, 23 Mar 2016 09:22:38 -0400, Tom Rini <tr...@konsulko.com> wrote:
> > > On Wed, Mar 23, 2016 at 01:53:35PM +0100, Albert ARIBAUD wrote:
> > > > Hello Marek,
> > > > 
> > > > On Sun, 20 Mar 2016 17:15:34 +0100, Marek Vasut <ma...@denx.de> wrote:
> > > > > This patch decouples U-Boot binary from the toolchain on systems where
> > > > > private libgcc is available. Instead of pulling in functions provided
> > > > > by the libgcc from the toolchain, U-Boot will use it's own set of 
> > > > > libgcc
> > > > > functions. These functions are usually imported from Linux kernel, 
> > > > > which
> > > > > also uses it's own libgcc functions instead of the ones provided by 
> > > > > the
> > > > > toolchain.
> > > > > 
> > > > > This patch solves a rather common problem. The toolchain can usually
> > > > > generate code for many variants of target architecture and often even
> > > > > different endianness. The libgcc on the other hand is usually compiled
> > > > > for one particular configuration and the functions provided by it may
> > > > > or may not be suited for use in U-Boot. This can manifest in two ways,
> > > > > either the U-Boot fails to compile altogether and linker will complain
> > > > > or, in the much worse case, the resulting U-Boot will build, but will
> > > > > misbehave in very subtle and hard to debug ways.
> > > > 
> > > > I don't think using private libgcc by default is a good idea.
> > > > 
> > > > U-Boot's private libgcc is not a feature of U-Boot, but a fix for some
> > > > cases where a target cannot properly link with the libgcc provided by
> > > > the (specific release of the) GCC toolchain in use. Using private libgcc
> > > > to other cases than these does not fix or improve anything; those
> > > > other cases were working and did not require any fix in this respect. 
> > > 
> > > This isn't true, exactly.  If using clang for example everyone needs to
> > > enable this code.  We're also using -fno-builtin -ffreestanding which
> > > should limit the amount of interference from the toolchain.  And we get
> > > that.
> > 
> > You mean clang does not produce self-sustained binaries?
> 
> clang does not provide "libgcc", so there's no -lgcc providing all of
> the functions that are (today) in:
> _ashldi3.S _ashrdi3.S _divsi3.S  _lshrdi3.S _modsi3.S _udivsi3.S
> _umodsi3.S div0.S  _uldivmod.S
> which aside from __modsi3 and __umodsi3 are all __aeabi_xxx

(ok, that explains what you mean by AEABI functions -- those are
actually not functions defined by the AEABI, but functions that the GCC
folks prefixed with __aeabi.)

I do understand that clang does not provide these functions. What I
want to understand is how come code compiled by clang would need them
unless we introduced that dependency ourselves. clang does produce
correct and self-sufficient code when using 64-bit division, right?

> > > > Also, libgcc is not a standalone project that can be frozen, forked or
> > > > improved freely; it is an internal component of the GCC toolchain. No
> > > > standard defines what libgcc is or should be, and we have no control
> > > > over the 'contract' between GCC-emitted code and libgcc. The GCC
> > > > project may decide to change that contract at any time, and produce a
> > > > new toolchain and a new libgcc. Using our private libgcc by default
> > > > will cause all targets to break for no good reason. We've already been
> > > > bitten by internal GCC changes on which we were dependent; adding more
> > > > such dependency is not the way to go IMO.
> > > > 
> > > > If we truly fear that GCC is *generally* unable to properly build our
> > > > targets due to its libgcc, then we should not only "snapshot and fix"
> > > > libgcc; we should "snapshot and fix" the whole GCC toolchain, to make
> > > > sure we keep a consistent copy of it. I don't think that would be a
> > > > viable move.
> > > > 
> > > > And if we don't believe that GCC is generally unable to properly build
> > > > U-Boot, then we should always use it as provided unless it is provably
> > >

Re: [U-Boot] [PATCH 5/5] lib: Enable private libgcc by default

[Albert ARIBAUD]

Hello Tom,

On Wed, 23 Mar 2016 09:22:38 -0400, Tom Rini <tr...@konsulko.com> wrote:
> On Wed, Mar 23, 2016 at 01:53:35PM +0100, Albert ARIBAUD wrote:
> > Hello Marek,
> > 
> > On Sun, 20 Mar 2016 17:15:34 +0100, Marek Vasut <ma...@denx.de> wrote:
> > > This patch decouples U-Boot binary from the toolchain on systems where
> > > private libgcc is available. Instead of pulling in functions provided
> > > by the libgcc from the toolchain, U-Boot will use it's own set of libgcc
> > > functions. These functions are usually imported from Linux kernel, which
> > > also uses it's own libgcc functions instead of the ones provided by the
> > > toolchain.
> > > 
> > > This patch solves a rather common problem. The toolchain can usually
> > > generate code for many variants of target architecture and often even
> > > different endianness. The libgcc on the other hand is usually compiled
> > > for one particular configuration and the functions provided by it may
> > > or may not be suited for use in U-Boot. This can manifest in two ways,
> > > either the U-Boot fails to compile altogether and linker will complain
> > > or, in the much worse case, the resulting U-Boot will build, but will
> > > misbehave in very subtle and hard to debug ways.
> > 
> > I don't think using private libgcc by default is a good idea.
> > 
> > U-Boot's private libgcc is not a feature of U-Boot, but a fix for some
> > cases where a target cannot properly link with the libgcc provided by
> > the (specific release of the) GCC toolchain in use. Using private libgcc
> > to other cases than these does not fix or improve anything; those
> > other cases were working and did not require any fix in this respect. 
> 
> This isn't true, exactly.  If using clang for example everyone needs to
> enable this code.  We're also using -fno-builtin -ffreestanding which
> should limit the amount of interference from the toolchain.  And we get
> that.

You mean clang does not produce self-sustained binaries?

> > Also, libgcc is not a standalone project that can be frozen, forked or
> > improved freely; it is an internal component of the GCC toolchain. No
> > standard defines what libgcc is or should be, and we have no control
> > over the 'contract' between GCC-emitted code and libgcc. The GCC
> > project may decide to change that contract at any time, and produce a
> > new toolchain and a new libgcc. Using our private libgcc by default
> > will cause all targets to break for no good reason. We've already been
> > bitten by internal GCC changes on which we were dependent; adding more
> > such dependency is not the way to go IMO.
> > 
> > If we truly fear that GCC is *generally* unable to properly build our
> > targets due to its libgcc, then we should not only "snapshot and fix"
> > libgcc; we should "snapshot and fix" the whole GCC toolchain, to make
> > sure we keep a consistent copy of it. I don't think that would be a
> > viable move.
> > 
> > And if we don't believe that GCC is generally unable to properly build
> > U-Boot, then we should always use it as provided unless it is provably
> > buggy, in which case if a private libgcc is a fix, then by all means we
> > should use it.
> > 
> > And whenever we find that a GCC toolchain is provably buggy, we should
> > raise a bug, either to the toolchain provider if the issue is only with
> > a given binary release (e.g. mismatched or badly supported endianness),
> > or to the GCC project if the bug is inherent to GCC (e.g. generation
> > of non-supported opcodes for a given arch/cpu).
> 
> Ah, but this shows part of the problem.  We don't need "libgcc" as in
> "the thing which provides gcc'isms".  We need "libgcc" as in "the thing
> which provides AEABI functions".

Not sure I'm getting what you mean. For one thing, I don't see that
AEABI specifies any functions. Also, I don't see where it is established
that U-Boot "needs AEABI functions". Finally, I don't see that libgcc
is a standalone project aiming at providing AEABI functions.

> Today we get these from libgcc but we
> run into cases where this doesn't work quite right (toolchain fun) or
> simply aren't available (again, clang).  So I am in favour of re-syncing
> with this part of the kernel and mirroring the decision to always
> include these functions, again, like the kernel does.

If we are using libgcc for providing AEABI services then we are using it
wrong. Its role is to support GCC-generated code.

Could you give me an example of this "need for [an] AEABI function"?

> -- 
> Tom

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

Re: [U-Boot] [PATCH 5/5] lib: Enable private libgcc by default

[Albert ARIBAUD]

Hello Marek,

On Sun, 20 Mar 2016 17:15:34 +0100, Marek Vasut  wrote:
> This patch decouples U-Boot binary from the toolchain on systems where
> private libgcc is available. Instead of pulling in functions provided
> by the libgcc from the toolchain, U-Boot will use it's own set of libgcc
> functions. These functions are usually imported from Linux kernel, which
> also uses it's own libgcc functions instead of the ones provided by the
> toolchain.
> 
> This patch solves a rather common problem. The toolchain can usually
> generate code for many variants of target architecture and often even
> different endianness. The libgcc on the other hand is usually compiled
> for one particular configuration and the functions provided by it may
> or may not be suited for use in U-Boot. This can manifest in two ways,
> either the U-Boot fails to compile altogether and linker will complain
> or, in the much worse case, the resulting U-Boot will build, but will
> misbehave in very subtle and hard to debug ways.

I don't think using private libgcc by default is a good idea.

U-Boot's private libgcc is not a feature of U-Boot, but a fix for some
cases where a target cannot properly link with the libgcc provided by
the (specific release of the) GCC toolchain in use. Using private libgcc
to other cases than these does not fix or improve anything; those
other cases were working and did not require any fix in this respect. 

Also, libgcc is not a standalone project that can be frozen, forked or
improved freely; it is an internal component of the GCC toolchain. No
standard defines what libgcc is or should be, and we have no control
over the 'contract' between GCC-emitted code and libgcc. The GCC
project may decide to change that contract at any time, and produce a
new toolchain and a new libgcc. Using our private libgcc by default
will cause all targets to break for no good reason. We've already been
bitten by internal GCC changes on which we were dependent; adding more
such dependency is not the way to go IMO.

If we truly fear that GCC is *generally* unable to properly build our
targets due to its libgcc, then we should not only "snapshot and fix"
libgcc; we should "snapshot and fix" the whole GCC toolchain, to make
sure we keep a consistent copy of it. I don't think that would be a
viable move.

And if we don't believe that GCC is generally unable to properly build
U-Boot, then we should always use it as provided unless it is provably
buggy, in which case if a private libgcc is a fix, then by all means we
should use it.

And whenever we find that a GCC toolchain is provably buggy, we should
raise a bug, either to the toolchain provider if the issue is only with
a given binary release (e.g. mismatched or badly supported endianness),
or to the GCC project if the bug is inherent to GCC (e.g. generation
of non-supported opcodes for a given arch/cpu).

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

Re: [U-Boot] [PATCH 1/6] ARM: keystone2: Split monitor code / command code

[Albert ARIBAUD]

Hello Tom,

On Wed, 16 Mar 2016 11:03:03 -0400, Tom Rini  wrote:
> When we switch to including all linker lists in U-Boot it is important
> to not include commands as that may lead to link errors due to other
> things we have already discarded.  In this case, we split the code for
> supporting the monitor out from the code for loading it.

Not sure I'm understanding this commit message. Can you clarify?

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

Re: [U-Boot] [PATCH 00/18] sf: fix support of QSPI memories and controllers

[Albert ARIBAUD]

Hello Jagan,

On Wed, 16 Mar 2016 19:44:26 +0530, Jagan Teki 
wrote:
> On Wednesday 16 March 2016 07:00 PM, Cyrille Pitchen wrote:
> > Le 15/03/2016 19:21, Jagan Teki a écrit :
> >> On Tuesday 15 March 2016 11:42 PM, Cyrille Pitchen wrote:
> >>> Hi all,
> >>>
> >>> This series of patches fixes and extend the support of QSPI memories
> >>> in the SPI flash framework. The updates are split into many parts to
> >>> make it easier to understand and review but they should be considered
> >>> as a whole.
> >>>
> >>> This was tested on a Atmel sama5d2 xplained board with a Micron n25q128a
> >>> memory.
> >>>
> >>> Best regards,
> >>>
> >>> Cyrille
> >>>
> >>> Cyrille Pitchen (18):
> >>> Revert "sf: Fix quad bit set for micron devices"
> >>> sf: call spi_claim_bus() and spi_release_bus() only once per read,
> >>>   write or erase
> >>> sf: replace spi_flash_read_common() calls by spi_flash_cmd_read()
> >>> sf: remove spi_flash_write_common()
> >>> sf: export spi_flash_wait_ready() function
> >>> sf: share erase generic algorithm
> >>> sf: share write generic algorithm
> >>> sf: share read generic algorithm
> >>> sf: add hooks to handle register read and write operations
> >>> sf: move support of SST flash into generic spi_flash_write_alg()
> >>> sf: fix selection of supported READ commands for QSPI memories
> >>> sf: fix detection of QSPI memories when they boot in Quad or Dual mode
> >>> sf: add helper function to set the number of dummy bytes
> >>> sf: add 4byte address opcodes
> >>> sf: prepare next fixes to support of QSPI memories by manufacturer
> >>> sf: fix support of Micron memories
> >>> ARM: at91: clock: add function to get QSPI clocks
> >>> sf: add driver for Atmel QSPI controller
> >>
> >> Appreciate for the work, we're working on spi-nor framework[1] planning to 
> >> push in couple of weeks. Will let you know once it merged so that you can 
> >> add your changes on top of that.
> >>
> >> [1] 
> >> http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi-nor-next
> >>
> >
> > Hi Jagan,
> >
> > I've started to have a look on your branch. I hope it's not to late for few
> > comments:
> >
> > Globally I see the new code attend to match the spi-nor framework from 
> > Linux.
> > OK that's fine but please note the current spi-nor framework in Linux has
> > incomplete and sometime not working support of QSPI memories.
> >
> > First, after a discussion with Brian and Bean on linux-mtd [1], Bean's 
> > commit
> > to add support to Micron QSPI memories was reverted since it didn't work 
> > alone.
> > In his reply, Brian agreed the code was not tested and should not have been
> > merged.
> >
> > This highlights a more general issue: currently, there is no mean for the
> > spi-nor framework to notify the SPI controller driver about a SPI protocol
> > change at the QSPI memory side. This applies to Micron memories when they 
> > enter
> > their Quad I/O mode. If so, ALL commands, even JEDEC Read ID, Read Status
> > Register, ..., MUST use the SPI 4-4-4 protocol. Commands sent using SPI 
> > 1-x-y
> > protocols are no longer decoded properly.
> > This also applies to Macronix and Winbond memories if they enter their QPI
> > mode, which is the equivalent of Micron Quad I/O mode.
> > This is why I've suggested to add 4 new fields in the struct spi_nor:
> > - .reg_proto: the SPI protocol to be used by .read_reg() and .write_reg()
> >hooks.
> > - .read_proto: the SPI protocol to be used by the .read() hooks, maybe by 
> > the
> >.read_mmap() also.
> > - .write_proto: the SPI protocol to be used by the .write() hooks
> > - .erase_proto: the SPI protocol to be used by the .erase() hooks.
> >
> > (Q)SPI controller drivers cannot guess the protocol to be used from the 
> > command
> > op code. Indeed, taking the Micron case as un example, the very same 0xeb op
> > code may be used with the SPI 1-4-4 protocol (Micron Extended SPI mode) or
> > with the SPI 4-4-4 protocol (Micron Quad I/O mode).
> >
> >
> > Also just some words about the naming of SPI x-y-z protocols:
> > - x refers to the number of I/O lines used to send the op code byte
> > - y is the number of I/O lines used to send the address, mode/dummy cycles
> >(if these cycles exist for the command op code)
> > - z is the number of I/O lines used to send/receive data (if needed)
> >
> > So the SNOR_OP_READ_1_1_2_IO macro for the Fast Read Dual I/O command (as
> > opposed to the macro SNOR_OP_READ_1_1_2 macro for the Fast Read Dual Output
> > command) doesn't make sense: it should be named SNOR_OP_READ_1_2_2.
> >
> >
> > Then about the value used for the dummy cycles, it's not always true that we
> > don't care about initializing them. Depending on the configuration of the
> > memory, some special dummy cycles, sometime called mode cycles, are used to
> > during Fast Read operations to make the memory enter/leaver its Continuous 
> > Read
> > 

Re: [U-Boot] [PATCH 00/18] sf: fix support of QSPI memories and controllers

[Albert ARIBAUD]

Hello Jagan,

On Wed, 16 Mar 2016 22:04:23 +0530, Jagan Teki <jt...@openedev.com>
wrote:
> Hi Albert,
> 
> On Wednesday 16 March 2016 09:53 PM, Albert ARIBAUD wrote:
> > Hello Jagan,
> >
> > On Wed, 16 Mar 2016 19:44:26 +0530, Jagan Teki <jt...@openedev.com>
> > wrote:
> >> On Wednesday 16 March 2016 07:00 PM, Cyrille Pitchen wrote:
> >>> Le 15/03/2016 19:21, Jagan Teki a écrit :
> >>>> On Tuesday 15 March 2016 11:42 PM, Cyrille Pitchen wrote:
> >>>>> Hi all,
> >>>>>
> >>>>> This series of patches fixes and extend the support of QSPI memories
> >>>>> in the SPI flash framework. The updates are split into many parts to
> >>>>> make it easier to understand and review but they should be considered
> >>>>> as a whole.
> >>>>>
> >>>>> This was tested on a Atmel sama5d2 xplained board with a Micron n25q128a
> >>>>> memory.
> >>>>>
> >>>>> Best regards,
> >>>>>
> >>>>> Cyrille
> >>>>>
> >>>>> Cyrille Pitchen (18):
> >>>>>  Revert "sf: Fix quad bit set for micron devices"
> >>>>>  sf: call spi_claim_bus() and spi_release_bus() only once per read,
> >>>>>write or erase
> >>>>>  sf: replace spi_flash_read_common() calls by spi_flash_cmd_read()
> >>>>>  sf: remove spi_flash_write_common()
> >>>>>  sf: export spi_flash_wait_ready() function
> >>>>>  sf: share erase generic algorithm
> >>>>>  sf: share write generic algorithm
> >>>>>  sf: share read generic algorithm
> >>>>>  sf: add hooks to handle register read and write operations
> >>>>>  sf: move support of SST flash into generic spi_flash_write_alg()
> >>>>>  sf: fix selection of supported READ commands for QSPI memories
> >>>>>  sf: fix detection of QSPI memories when they boot in Quad or Dual 
> >>>>> mode
> >>>>>  sf: add helper function to set the number of dummy bytes
> >>>>>  sf: add 4byte address opcodes
> >>>>>  sf: prepare next fixes to support of QSPI memories by manufacturer
> >>>>>  sf: fix support of Micron memories
> >>>>>  ARM: at91: clock: add function to get QSPI clocks
> >>>>>  sf: add driver for Atmel QSPI controller
> >>>>
> >>>> Appreciate for the work, we're working on spi-nor framework[1] planning 
> >>>> to push in couple of weeks. Will let you know once it merged so that you 
> >>>> can add your changes on top of that.
> >>>>
> >>>> [1] 
> >>>> http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi-nor-next
> >>>>
> >>>
> >>> Hi Jagan,
> >>>
> >>> I've started to have a look on your branch. I hope it's not to late for 
> >>> few
> >>> comments:
> >>>
> >>> Globally I see the new code attend to match the spi-nor framework from 
> >>> Linux.
> >>> OK that's fine but please note the current spi-nor framework in Linux has
> >>> incomplete and sometime not working support of QSPI memories.
> >>>
> >>> First, after a discussion with Brian and Bean on linux-mtd [1], Bean's 
> >>> commit
> >>> to add support to Micron QSPI memories was reverted since it didn't work 
> >>> alone.
> >>> In his reply, Brian agreed the code was not tested and should not have 
> >>> been
> >>> merged.
> >>>
> >>> This highlights a more general issue: currently, there is no mean for the
> >>> spi-nor framework to notify the SPI controller driver about a SPI protocol
> >>> change at the QSPI memory side. This applies to Micron memories when they 
> >>> enter
> >>> their Quad I/O mode. If so, ALL commands, even JEDEC Read ID, Read Status
> >>> Register, ..., MUST use the SPI 4-4-4 protocol. Commands sent using SPI 
> >>> 1-x-y
> >>> protocols are no longer decoded properly.
> >>> This also applies to Macronix and Winbond memories if they enter their QPI
> >>> mode, which is the equivalent of Micron Quad I/O mode.
> >>> This is why I've suggested to add 4 new fields in the struct spi_

Re: [U-Boot] [PATCH] sf: Correct data types in stm_is_locked_sr()

[Albert ARIBAUD]

Hello Jagan,

On Sat, 12 Mar 2016 00:41:25 +0530, Jagan Teki
<jagannadh.t...@gmail.com> wrote:
> Hi Albert,
> 
> On 12 March 2016 at 00:17, Albert ARIBAUD <albert.u.b...@aribaud.net> wrote:
> > Hello Jagan,
> >
> > On Fri, 11 Mar 2016 12:09:37 +0530, Jagan Teki
> > <jagannadh.t...@gmail.com> wrote:
> >> On 11 March 2016 at 07:50, Marek Vasut <ma...@denx.de> wrote:
> >> > The stm_is_locked_sr() function is picked from Linux kernel. For reason
> >> > unknown, the 64bit data types used by the function and present in Linux
> >> > were replaced with 32bit unsigned ones, which causes trouble.
> >> >
> >> > The testcase performed was done using ST M25P80 chip.
> >> > The command used was:
> >> >  => sf protect unlock 0 0x1
> >> >
> >> > The call chain starts in stm_unlock(), which calls stm_is_locked_sr()
> >> > with negative ofs argument. This works fine in Linux, where the "ofs"
> >> > is loff_t, which is signed long long, while this fails in U-Boot, where
> >> > "ofs" is u32 (unsigned int). Because of this signedness problem, the
> >> > expression past the return statement to be incorrectly evaluated to 1,
> >> > which in turn propagates back to stm_unlock() and results in -EINVAL.
> >> >
> >> > The correction is very simple, just use the correctly sized data types
> >> > with correct signedness in the function to make it work as intended.
> >> >
> >> > Signed-off-by: Marek Vasut <ma...@denx.de>
> >> > Cc: Simon Glass <s...@chromium.org>
> >> > Cc: Jagan Teki <jt...@openedev.com>
> >> > ---
> >> >  drivers/mtd/spi/spi_flash.c | 6 +++---
> >> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> >> > index 2ae2e3c..44d9e9b 100644
> >> > --- a/drivers/mtd/spi/spi_flash.c
> >> > +++ b/drivers/mtd/spi/spi_flash.c
> >> > @@ -665,7 +665,7 @@ int sst_write_bp(struct spi_flash *flash, u32 
> >> > offset, size_t len,
> >> >
> >> >  #if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST)
> >> >  static void stm_get_locked_range(struct spi_flash *flash, u8 sr, loff_t 
> >> > *ofs,
> >> > -u32 *len)
> >> > +u64 *len)
> >>
> >> What about uint64_t?
> >
> > Well, the U-Boot coding style [1] suggest that we follow the Linux
> > coding style [2] which itself suggests [chapter 5, item (d)] that when
> 
> uNN types means uint32_t/uint64_t ?

No, uNN means u8/u16/u32, but I'll admit that may not have been totally
unambiguous.

> > uNN types are being used already in some code, then changes to this
> > code should keep on using uNN types.
> 
> Sorry, I didn't understand here - if the code having these uNN types
> the changes to same uNN types?

It was better explained in the URL I gave. :)

Basically: the Linux (and therefore U-Boot) coding style guide says if
some code uses u8/u16/u32, then changes to this code should keep using
u8/u16/u32; and here, drivers/mtd/spi/spi_flash.c uses u8, u16 and u32
so the wrongly-sized u32 should be changed into a u64, not into a
uint64_t.

> thanks!
> -- 
> Jagan.

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

Re: [U-Boot] [PATCH] sf: Correct data types in stm_is_locked_sr()

[Albert ARIBAUD]

Hello Jagan,

On Fri, 11 Mar 2016 12:09:37 +0530, Jagan Teki
 wrote:
> On 11 March 2016 at 07:50, Marek Vasut  wrote:
> > The stm_is_locked_sr() function is picked from Linux kernel. For reason
> > unknown, the 64bit data types used by the function and present in Linux
> > were replaced with 32bit unsigned ones, which causes trouble.
> >
> > The testcase performed was done using ST M25P80 chip.
> > The command used was:
> >  => sf protect unlock 0 0x1
> >
> > The call chain starts in stm_unlock(), which calls stm_is_locked_sr()
> > with negative ofs argument. This works fine in Linux, where the "ofs"
> > is loff_t, which is signed long long, while this fails in U-Boot, where
> > "ofs" is u32 (unsigned int). Because of this signedness problem, the
> > expression past the return statement to be incorrectly evaluated to 1,
> > which in turn propagates back to stm_unlock() and results in -EINVAL.
> >
> > The correction is very simple, just use the correctly sized data types
> > with correct signedness in the function to make it work as intended.
> >
> > Signed-off-by: Marek Vasut 
> > Cc: Simon Glass 
> > Cc: Jagan Teki 
> > ---
> >  drivers/mtd/spi/spi_flash.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> > index 2ae2e3c..44d9e9b 100644
> > --- a/drivers/mtd/spi/spi_flash.c
> > +++ b/drivers/mtd/spi/spi_flash.c
> > @@ -665,7 +665,7 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, 
> > size_t len,
> >
> >  #if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST)
> >  static void stm_get_locked_range(struct spi_flash *flash, u8 sr, loff_t 
> > *ofs,
> > -u32 *len)
> > +u64 *len)
> 
> What about uint64_t?

Well, the U-Boot coding style [1] suggest that we follow the Linux
coding style [2] which itself suggests [chapter 5, item (d)] that when
uNN types are being used already in some code, then changes to this
code should keep on using uNN types.

[1] http://www.denx.de/wiki/U-Boot/CodingStyle
[2]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/CodingStyle

> Jagan.

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

Re: [U-Boot] [PATCH 3/5] kbuild: use $(call cmd, ) rather than $(call if_changed, ) where possible

[Albert ARIBAUD]

Hello Masahiro,

On Wed,  3 Feb 2016 21:05:12 +0900, Masahiro Yamada
 wrote:
> These build commands are constant (mostly, just concatenating images,
> or just copying).  No need to use $(call if_changed,...) for them.

I am a total kbuild ignorant, so I'm probably asking a stupid question,
but hey.

What are the exact semantics of $(call if_changed,)? I thought it meant
"call the function if any of the dependencies has changed. If it does,
then won't this patch make the corresponding action systematic, causing
unneeded concatenations or copies?

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

Re: [U-Boot] [PATCHv2] common/memsize.c: Simplify RAM size detection

[Albert ARIBAUD]

Hello Eddy,

On Tue,  2 Feb 2016 22:15:28 +0200, Eddy Petrișor
 wrote:
> The case of memory of size 0 is not that different from a memory of any other
> size, so we remove the duplicate code and treat the small differences when it
> is the case.
> 
> Signed-off-by: Eddy Petrișor 
> ---
> 
> v2: Removed patman stuff from commit message

Sorry for asking, but since you're mentioning it... If you are using
patman, then why do you alter its mails rather than just letting it
post them as-is?

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

Re: [U-Boot] [PATCH 3/5] kbuild: use $(call cmd, ) rather than $(call if_changed, ) where possible

[Albert ARIBAUD]

Hello Albert,

On Wed, 3 Feb 2016 13:18:35 +0100, Albert ARIBAUD
<albert.u.b...@aribaud.net> wrote:
> Hello Masahiro,
> 
> On Wed,  3 Feb 2016 21:05:12 +0900, Masahiro Yamada
> <yamada.masah...@socionext.com> wrote:
> > These build commands are constant (mostly, just concatenating images,
> > or just copying).  No need to use $(call if_changed,...) for them.
> 
> I am a total kbuild ignorant, so I'm probably asking a stupid question,
> but hey.
> 
> What are the exact semantics of $(call if_changed,)? I thought it meant
> "call the function if any of the dependencies has changed. If it does,
> then won't this patch make the corresponding action systematic, causing
> unneeded concatenations or copies?

NVM, the answer could be found in later patches in the series.

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

Re: [U-Boot] [PATCH 2/3] common: env_flags: include common.h even for HOST_CC

[Albert ARIBAUD]

Hello Peter,

On Wed,  3 Feb 2016 12:42:51 +, Peter Robinson
 wrote:
> When compiling with gcc 6 we get the following error due to ARRAY_SIZE being
> defined elsewhere.
> 
> common/env_flags.c:155: undefined reference to `ARRAY_SIZE'
> 
> Signed-off-by: Peter Robinson 
> ---
>  common/env_flags.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/env_flags.c b/common/env_flags.c
> index 9c3aed1..696adef 100644
> --- a/common/env_flags.c
> +++ b/common/env_flags.c
> @@ -7,6 +7,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  #ifdef USE_HOSTCC /* Eliminate "ANSI does not permit..." warnings */
>  #include 
> @@ -16,7 +17,6 @@
>  #include 
>  #define getenv fw_getenv
>  #else
> -#include 
>  #include 
>  #endif

How come this happens only with gcc-6? Previous compilers surely did not
'guess' the proper value of ARRAY_SIZE, right?

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

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

[U-Boot] Patman use (was: [PATCHv2] common/memsize.c: Simplify RAM size detection)

[Albert ARIBAUD]

Hello Eddy,

(re-titling)

On Wed, 3 Feb 2016 18:32:45 +0200, Eddy Petrișor
<eddy.petri...@gmail.com> wrote:
> 2016-02-03 15:38 GMT+02:00 Albert ARIBAUD <albert.u.b...@aribaud.net>:
> > Hello Eddy,
> >
> > On Tue,  2 Feb 2016 22:15:28 +0200, Eddy Petrișor
> > <eddy.petri...@gmail.com> wrote:
> [..]
> >> v2: Removed patman stuff from commit message
> >
> > Sorry for asking, but since you're mentioning it... If you are using
> > patman, then why do you alter its mails rather than just letting it
> > post them as-is?
> 
> Actually I tried using patman, but got all sorts of errors and wasn't
> able to use it. Then I manually sent the patches with git send-email,
> but forgot to remove the patman header. So in the repost I removed the
> patman header since it was of no use. I hope is more clear now.

If you can send with git send-email, then you should be able to, and
should, use patman. What were these errors?

> -- 
> Eddy Petrișor

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

Re: [U-Boot] [PATCH 2/3] common: env_flags: include common.h even for HOST_CC

[Albert ARIBAUD]

Hello Peter,

On Wed, 3 Feb 2016 16:11:38 +, Peter Robinson
<pbrobin...@gmail.com> wrote:
> Hi Albert,
> 
> On Wed, Feb 3, 2016 at 1:41 PM, Albert ARIBAUD
> <albert.u.b...@aribaud.net> wrote:
> > Hello Peter,
> >
> > On Wed,  3 Feb 2016 12:42:51 +, Peter Robinson
> > <pbrobin...@gmail.com> wrote:
> >> When compiling with gcc 6 we get the following error due to ARRAY_SIZE 
> >> being
> >> defined elsewhere.
> >>
> >> common/env_flags.c:155: undefined reference to `ARRAY_SIZE'
> >>
> >> Signed-off-by: Peter Robinson <pbrobin...@gmail.com>
> >> ---
> >>  common/env_flags.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/common/env_flags.c b/common/env_flags.c
> >> index 9c3aed1..696adef 100644
> >> --- a/common/env_flags.c
> >> +++ b/common/env_flags.c
> >> @@ -7,6 +7,7 @@
> >>
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>  #ifdef USE_HOSTCC /* Eliminate "ANSI does not permit..." warnings */
> >>  #include 
> >> @@ -16,7 +17,6 @@
> >>  #include 
> >>  #define getenv fw_getenv
> >>  #else
> >> -#include 
> >>  #include 
> >>  #endif
> >
> > How come this happens only with gcc-6? Previous compilers surely did not
> > 'guess' the proper value of ARRAY_SIZE, right?
> 
> So testing this RC on on Fedora 23 with gcc 5.3.1 I see the same
> failure, I didn't see it with 2016.01 when using 5.3.1 so I'm not sure
> what's changed there

OK, so maybe unrelated to gcc 6. Could you git bisect?

> Peter

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

Re: [U-Boot] 64-bit x86 U-Boot?

[Albert ARIBAUD]

Hello Bin and Simon,

On Tue, 2 Feb 2016 15:25:48 +0800, Bin Meng  wrote:
> Hi Simon,
> 
> On Tue, Feb 2, 2016 at 11:58 AM, Simon Glass  wrote:
> > +Bin (sorry, meant to copy you before)

> >>> For non-FSP devices we don't init the RAM until much later -
> >>> dram_init(). That means that a significant portion of the init
> >>> sequence would be 32-bit code. I'm not sure that will work.
> >>>
> 
> I believe we can do dram_init() in 64-bit mode as well if MRC is
> written in pure C.

Bin: not sure what you mean by "if MRC is written in pure C" -- there
is no C construct that can even approach the mrc instruction, which can
only be emitted through an asm statement.

> > I wonder whether we might need to resort to SPL for the 32-bit
> > portion, and jump to a 64-bit U-Boot from there? Tegra does something
> > similar to that.

Simon: seems like a sensible approach, as it does not mix 32 and 64
bits in one "build artefact", plus it seems logical in that SPL's
role is to get the platform ready for U-Boot; switching from
power-on32-bit mode to 64-bit mode belongs quite "naturally" in SPL.

> What's the benefit of doing a 64-bit bootloader? Intel's UEFI BIOS has
> a 32-bit and 64-bit version, and has caused some troubles for the next
> stage loader (bootia32.efi vs. bootx64.efi). I know for PowerPC, a
> 64-bit U-Boot does not exist as 32-bit U-Boot can load 32-bit and
> 64-bit kernel, just like what we have for x86. 64-bit U-Boot was only
> seen on ARMv8, but that's the architecture limitation I believe, and
> we have to do that.

Some U-Boot users who might want to get rid of x86 32-bit code in
x86 64-bit platforms just like in the past some people must have wanted
to get rid of real-mode 16-bit x86 code in order to run pure 32-bit; the
idea is that if you can do with as well as without a feature, then that
feature is potential dead code, and is candidate for removal, all the
more when that feature partly collides with another feature, as here
where 32-bit and 64-bit support sort of overlap partially.

Now, we can wait until x86 32-bit is really dead (as in "not used
except in a few legacy projects whose engineers' children are about to
retire") and then scrape dead code parts which no one really understands
any more, or we can try and anticipate and replace code while we still
have a grasp of what it does. I personally like the idea of anticipating
better.

Just in case, note that I do not mean x86 32-bit support should be
removed from U-Boot now or later. I mean that if we can make x86 64-bit
support in U-Boot less and less dependent on x86 32-bit support, then I
think we should, so that the day we completely drop x86 32-bit support,
x86 64-bit support will be (as) unaffected (as possible).

> Regards,
> Bin

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

Re: [U-Boot] [PATCH] spl: define BOOT_DEVICE_USB

[Albert ARIBAUD]

Hello Masahiro,

On Tue,  2 Feb 2016 15:45:13 +0900, Masahiro Yamada
 wrote:
> This macro is referenced from common/spl/spl.c

Nitpick: not a macro, but an enum value.

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

[U-Boot] [PATCH v2 0/1] Fix patman Series-notes handling for buildman

[Albert ARIBAUD]

Creating a branch with a Series-notes and running buildman
on that branch results in a buildman error of the form
"TypeError: cannot concatenate 'str' and 'list' objects".
This "series" fixes that by initializing series.notes as an
array, not a scalar. This is a single and short patch which
would not normally require a Cover-letter (which you are
reading now) and Series-notes (which you'll read soon), but
I thought it quite elegant to test this patch with itself.
And anyway I could not resist.

This patch was tested by actually including a Series-notes
(you are reading it now) and therefore a Cover-letter (you
have read it already) in the local commit containing this
patch, running buildman on it to check that it does not
break any more, then running patman on it to both test that
patman does not break as it would have with v1 of the patch,
and actually submit the patch.

Changes in v2:
- fix typo in commit message ("rathen" -> "rather")
- actually limit array initialization to series.notes

Albert ARIBAUD (1):
  patman: fix series-notes handling for buildman

 tools/patman/series.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.5.0

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

Re: [U-Boot] [PATCH] patman: fix series-notes handling for buildman

[Albert ARIBAUD]

Hello Simon,

> >> Hmm, actually I've had to drop this as it breaks 'Series-version'.
> >> That currently does not expect a list.
> >
> > Hmm, I can't reproduce this here. How do you trigger the Series-version
> > break?
> 
> I created a commit with a Series-version: in it. Then, running patman
> gives a run-time error because it is expecting a single value, not an
> array.

Thanks.

Weird. That's what I'd done when starting v2 (so that I could test my
commit with itself) except I also had Series-note: in it, and it would
not trigger this. Then I rolled back to v1 and did it again, and this
time I get the error. And before anyone asks, I'd run git clean- xfd
so there was no precompiled Python in the way.

Oh well.

v2 just sent.

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

[U-Boot] [PATCH v2] patman: fix series-notes handling for buildman

[Albert ARIBAUD]

A patman series with a 'Series-notes' section causes
buildman to crash with:

self.series.notes += self.section
TypeError: cannot concatenate 'str' and 'list' objects

Fix by initializing series.notes as a one-element array
rather than a scalar.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---

Changes in v2:
- fix typo in commit message ("rathen" -> "rather")
- actually limit array initialization to series.notes

 tools/patman/series.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/patman/series.py b/tools/patman/series.py
index 3399f2c..cc6f80b 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -69,7 +69,10 @@ class Series(dict):
 
 # Otherwise just set the value
 elif name in valid_series:
-self[name] = value
+if name=="notes":
+self[name] = [value]
+else:
+self[name] = value
 else:
 raise ValueError("In %s: line '%s': Unknown 'Series-%s': valid "
 "options are %s" % (commit.hash, line, name,
-- 
2.5.0

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

Re: [U-Boot] [PATCH v3]net: Wrong Initialization in davinci-emac driver

[Albert ARIBAUD]

Hello Vishwas,

On Mon, 1 Feb 2016 20:51:58 +0530, Vishwas Srivastava
 wrote:
> Author: Vishwas Srivastava 
> Date:   Mon Jan 25 21:28:17 2016 +0530

This is unneeded in the commit message, as the author and date are
already provided in the mail.

> Wrong Initialization in davinci emac driver

This is the title of the patch and does not need to be repeated.

> Changes for v2:
> -cleaned up the style format
> -addressed various comments given by Joe 
> on the first version of the patch.
> 
> Changes for v3:
> - Added the missing patch part of v2

Changes should not be part of the commit message; they should appear
after the '---' line which, by the way, is completely missing here,
along with some lines at the start of the patch ; and changes should
start with the most recent change first.

Also, you are quoting the previous patch, which is unneeded.

It seems like you put this e-mail together from the output of a manual
diff rather than producing it with git send-email or patman. I suggest
re-sending (under v3 to avoid confusion) using patman, which would ease
the work of formatting the mail and managing patch versions.

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

Re: [U-Boot] [PATCH] patman: fix series-notes handling for buildman

[Albert ARIBAUD]

Hello Simon,

On Thu, 19 Nov 2015 20:29:11 -0700, Simon Glass <s...@chromium.org>
wrote:
> Hi Albert,
> 
> On 13 November 2015 at 19:35, Simon Glass <s...@chromium.org> wrote:
> > On 9 November 2015 at 14:36, Albert ARIBAUD <albert.u.b...@aribaud.net> 
> > wrote:
> >> Hello Simon,
> >>
> >> On Mon, 9 Nov 2015 12:24:55 -0800, Simon Glass <s...@chromium.org> wrote:
> >>> On 9 November 2015 at 06:19, Albert ARIBAUD <albert.u.b...@aribaud.net> 
> >>> wrote:
> >>> > A patman series with a 'Series-notes' section causes
> >>> > buildman to crash with:
> >>> >
> >>> > self.series.notes += self.section
> >>> > TypeError: cannot concatenate 'str' and 'list' objects
> >>> >
> >>> > Fix by initializing series.notes as a one-element array
> >>> > rathen than a scalar.
> >>> >
> >>> > Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
> >>> > ---
> >>> >
> >>> >  tools/patman/series.py | 2 +-
> >>> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> Acked-by: Simon Glass <s...@chromium.org>
> >>
> >> Note: just spotted a typo, 'rathen' instead of 'rather'. If this patch
> >> gets applied, please fix that on-the-fly.
> >>
> >> Amicalement,
> >> --
> >> Albert.
> >
> > Fixed typo and
> >
> > Applied to u-boot-dm, thanks!
> 
> Hmm, actually I've had to drop this as it breaks 'Series-version'.
> That currently does not expect a list.

Hmm, I can't reproduce this here. How do you trigger the Series-version
break?

> I wonder if Series-notes needs a special case, instead?
> 
> Regards,
> Simon

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

[U-Boot] Pull request: u-boot-arm/master

[Albert ARIBAUD]

Hello Tom,

The following changes since commit 8cdae1dacde7dbe74d53a8ac1a05761a53c4f191:

  video: Correct 'tor' typo in comment (2016-01-30 10:58:47 +0100)

are available in the git repository at:

  git://git.denx.de/u-boot-arm master

for you to fetch changes up to 735b1a2e5a61401868bb35702b6e5e18bce6eb97:

  arm: novena: Fix EEPROM i2c configuration (2016-01-31 16:32:56 +0100)


Albert ARIBAUD (1):
  armv7: add cacheline sizes where missing

Marek Vasut (4):
  arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7
  arm: Remove S bit from MMU section entry
  arm: cache: Implement cache range check for v7
  arm: novena: Fix EEPROM i2c configuration

Peng Fan (1):
  arm: config: enforce -fno-pic for gcc

Wang Dongsheng (1):
  ARM: Disable "DISCARD" for secure section if CONFIG_ARMV7_SECURE_BASE 
isn't defined

 arch/arm/config.mk  |  1 +
 arch/arm/cpu/armv7/cache_v7.c   | 21 +
 arch/arm/cpu/u-boot.lds | 27 ++-
 arch/arm/include/asm/system.h   |  7 +++
 arch/arm/lib/cache-cp15.c   |  2 +-
 include/configs/am3517_crane.h  |  2 ++
 include/configs/am3517_evm.h|  2 ++
 include/configs/at91-sama5_common.h |  2 ++
 include/configs/bcm_ep_board.h  |  1 +
 include/configs/cm_t35.h|  2 ++
 include/configs/cm_t3517.h  |  2 ++
 include/configs/colibri_vf.h|  2 ++
 include/configs/kzm9g.h |  2 ++
 include/configs/nokia_rx51.h|  2 ++
 include/configs/novena.h|  3 ++-
 include/configs/pcm052.h|  2 ++
 include/configs/rcar-gen2-common.h  |  2 ++
 include/configs/rk3036_common.h |  2 ++
 include/configs/rk3288_common.h |  2 ++
 include/configs/s5p_goni.h  |  2 ++
 include/configs/smdkc100.h  |  2 ++
 include/configs/tao3530.h   |  2 ++
 include/configs/ti814x_evm.h|  2 ++
 include/configs/ti816x_evm.h|  2 ++
 include/configs/ti_omap3_common.h   |  5 +
 include/configs/tricorder.h |  2 ++
 include/configs/vexpress_common.h   |  2 ++
 include/configs/vf610twr.h  |  2 ++
 28 files changed, 88 insertions(+), 19 deletions(-)

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

[U-Boot] [PATCH] Revert "arm: Remove wireless_space board"

[Albert ARIBAUD]

Revert commit b352182a and complete generic board support.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---
 arch/arm/mach-kirkwood/Kconfig  |   4 +
 board/LaCie/wireless_space/Kconfig  |  12 ++
 board/LaCie/wireless_space/MAINTAINERS  |   6 +
 board/LaCie/wireless_space/Makefile |  12 ++
 board/LaCie/wireless_space/kwbimage.cfg |  71 +++
 board/LaCie/wireless_space/wireless_space.c | 165 +
 configs/wireless_space_defconfig|   7 ++
 include/configs/wireless_space.h| 179 
 8 files changed, 456 insertions(+)
 create mode 100644 board/LaCie/wireless_space/Kconfig
 create mode 100644 board/LaCie/wireless_space/MAINTAINERS
 create mode 100644 board/LaCie/wireless_space/Makefile
 create mode 100644 board/LaCie/wireless_space/kwbimage.cfg
 create mode 100644 board/LaCie/wireless_space/wireless_space.c
 create mode 100644 configs/wireless_space_defconfig
 create mode 100644 include/configs/wireless_space.h

diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
index 9205b1e..9249564 100644
--- a/arch/arm/mach-kirkwood/Kconfig
+++ b/arch/arm/mach-kirkwood/Kconfig
@@ -37,6 +37,9 @@ config TARGET_NET2BIG_V2
 config TARGET_NETSPACE_V2
bool "LaCie netspace_v2 Board"
 
+config TARGET_WIRELESS_SPACE
+   bool "LaCie Wireless_space Board"
+
 config TARGET_IB62X0
bool "ib62x0 Board"
 
@@ -68,6 +71,7 @@ source "board/iomega/iconnect/Kconfig"
 source "board/keymile/km_arm/Kconfig"
 source "board/LaCie/net2big_v2/Kconfig"
 source "board/LaCie/netspace_v2/Kconfig"
+source "board/LaCie/wireless_space/Kconfig"
 source "board/raidsonic/ib62x0/Kconfig"
 source "board/Seagate/dockstar/Kconfig"
 source "board/Seagate/goflexhome/Kconfig"
diff --git a/board/LaCie/wireless_space/Kconfig 
b/board/LaCie/wireless_space/Kconfig
new file mode 100644
index 000..75a2fc5
--- /dev/null
+++ b/board/LaCie/wireless_space/Kconfig
@@ -0,0 +1,12 @@
+if TARGET_WIRELESS_SPACE
+
+config SYS_BOARD
+   default "wireless_space"
+
+config SYS_VENDOR
+   default "LaCie"
+
+config SYS_CONFIG_NAME
+   default "wireless_space"
+
+endif
diff --git a/board/LaCie/wireless_space/MAINTAINERS 
b/board/LaCie/wireless_space/MAINTAINERS
new file mode 100644
index 000..c32ecb8
--- /dev/null
+++ b/board/LaCie/wireless_space/MAINTAINERS
@@ -0,0 +1,6 @@
+WIRELESS_SPACE BOARD
+M: Albert ARIBAUD <albert.u.b...@aribaud.net>
+S: Maintained
+F: board/LaCie/wireless_space/
+F: include/configs/wireless_space.h
+F: configs/wireless_space_defconfig
diff --git a/board/LaCie/wireless_space/Makefile 
b/board/LaCie/wireless_space/Makefile
new file mode 100644
index 000..90a84f4
--- /dev/null
+++ b/board/LaCie/wireless_space/Makefile
@@ -0,0 +1,12 @@
+#
+# Copyright (C) 2011 Simon Guinot <sgui...@lacie.com>
+#
+# Based on Kirkwood support:
+# (C) Copyright 2009
+# Marvell Semiconductor 
+# Written-by: Prafulla Wadaskar <prafu...@marvell.com>
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+obj-y  := wireless_space.o ../common/common.o
diff --git a/board/LaCie/wireless_space/kwbimage.cfg 
b/board/LaCie/wireless_space/kwbimage.cfg
new file mode 100644
index 000..037248b
--- /dev/null
+++ b/board/LaCie/wireless_space/kwbimage.cfg
@@ -0,0 +1,71 @@
+#
+# Copyright (C) 2012 Albert ARIBAUD <albert.u.b...@aribaud.net>
+#
+# Based on netspace_v2 kwbimage.cfg:
+# Copyright (C) 2011 Simon Guinot <sgui...@lacie.com>
+#
+# Based on Kirkwood support:
+# (C) Copyright 2009
+# Marvell Semiconductor 
+# Written-by: Prafulla Wadaskar <prafu...@marvell.com>
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Refer doc/README.kwbimage for more details about how-to configure
+# and create kirkwood boot image
+#
+
+# Boot Media configurations
+BOOT_FROM  nand# Boot from NAND flash
+NAND_PAGE_SIZE 800
+
+# SOC registers configuration using bootrom header extension
+# Maximum KWBIMAGE_MAX_CONFIG configurations allowed
+
+# Values taken from image original LaCie U-Boot header dump!
+
+# Configure RGMII-0 interface pad voltage to 1.8V
+DATA 0xFFD100e0 0x1B1B1B9B
+
+#Dram initalization for SINGLE x16 CL=5 @ 400MHz
+DATA 0xFFD01400 0x43000c30 # DDR Configuration register
+
+DATA 0xFFD01404 0x37743000 # DDR Controller Control Low
+
+DATA 0xFFD01408 0x11012228 # DDR Timing (Low) (active cycles value +1)
+
+DATA 0xFFD0140C 0x0A19 #  DDR Timing (High)
+
+DATA 0xFFD01410 0x #  DDR Address Control
+
+DATA 0xFFD01414 0x #  DDR Open Pages Control
+
+DATA 0xFFD01418 0x #  DDR Operation
+
+DATA 0xFFD0141C 0x0662 #  DDR Mode
+
+DATA 0xFFD01420 0x0004 #  DDR Extended Mode
+
+DATA 0xFFD01424 0xF07F #  DDR Controller Control High
+
+DATA 0xFFD0

Re: [U-Boot] 64-bit x86 U-Boot?

[Albert ARIBAUD]

Hello Simon,

On Sun, 31 Jan 2016 19:20:31 -0700, Simon Glass 
wrote:
> Hi Bin,
> 
> At present U-Boot supports booting a 64-bit kernel directly. It can
> also be loaded as a 64-bit payload from EFI. But it cannot be built as
> a 64-bit boot loader.
> 
> I took a bit of a look at this. It looks like we need to stay in
> 32-bit mode until the FSP is loaded. Also, to get to 64-bit mode I'm
> pretty sure we need page tables, which means we need somewhere to put
> them!
> 
> Looking at the board_f init sequence, it seems that arch_cpu_init() is
> the earlist we could switch to 64-bit. We'd need to grab some memory
> from somewhere to do this - I wonder if this can be CAR? There is no
> SRAM on x86 chips I think.
> 
> For non-FSP devices we don't init the RAM until much later -
> dram_init(). That means that a significant portion of the init
> sequence would be 32-bit code. I'm not sure that will work.
> 
> I suppose one option is to only go to 64-bit mode when relocating. But
> then we end up with lots of code that needs to run in 32-bit and
> 64-bit.
> 
> Do you have any ideas on this?

How about starting with implementing the last option, i.e. switch to 64
bits when DDR is available, mainline that, then progressively work your
way toward an earlier switch?

> Regards,
> Simon

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

Re: [U-Boot] [PATCH v3 0/7] fdt: Replace u-boot-dtb.bin with u-boot.bin

[Albert ARIBAUD]

Hello Simon,

Nitpicking:

On Sun, 31 Jan 2016 18:10:48 -0700, Simon Glass 
wrote:

> The original decision to use a separate u-boot-dtb.bin was aimed at allowing
> any device tree file to be concatenated to the u-boot.bin image after the
> build. However this no-longer seems so important. More important is the
> convenience of using the same output file regardless of the setting for
> OF_CONTROL.

Maybe explain in a few words why concatenating the DT no long seems
important, so that readers who were intending to concatenate get a hint
on what option they should follow instead?

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

Re: [U-Boot] [PATCH] OMAP3: omap3_logic: Remove YAFFS support.

[Albert ARIBAUD]

Hello Adam,

On Sun, 31 Jan 2016 17:17:24 -0600, Adam Ford 
wrote:
> UBIFS is the preferred OS, and YAFFS isn't officially included in
> Linux.  Removing this feature reduces the code size.

Typo: "preferred FS", not "preferred OS".

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

Re: [U-Boot] Include patchwork patch ID in commit message?

[Albert ARIBAUD]

Hello Masahiro,

On Fri, 29 Jan 2016 18:57:28 +0900, Masahiro Yamada
 wrote:
> 2016-01-29 14:14 GMT+09:00 Heiko Schocher :
> > Hello Joe,
> >
> >
> > Am 28.01.2016 um 16:15 schrieb Joe Hershberger:
> >>
> >> Hi Heiko,
> >>
> >> On Thu, Jan 28, 2016 at 12:39 AM, Heiko Schocher  wrote:
> >>>
> >>> Hello Tom,
> >>>
> >>> Am 28.01.2016 um 00:45 schrieb Tom Rini:
> 
> 
>  On Wed, Jan 27, 2016 at 05:15:17PM -0600, Joe Hershberger wrote:
> >
> >
> > On Wed, Jan 27, 2016 at 4:22 PM, Tom Rini  wrote:
> >>
> >>
> >> On Wed, Jan 27, 2016 at 03:08:09PM -0600, Joe Hershberger wrote:
> >>>
> >>>
> >>> Hi Tom,
> >>>
> >>> I'm playing with the idea of including the patchwork patch ID in the
> >>> commit message of each commit that I apply to provide better
> >>> cross-reference ability.
> >>>
> >>> * Access to comments on patches
> >>> * Clarity on exactly which version of a patch was applied
> >>> * No need to search by patch subject
> >>>
> >>> Here is an example in a working branch:
> >>>
> >>>
> >>>
> >>> http://git.denx.de/?p=u-boot/u-boot-net.git;a=commit;h=48f9a0c786d0a3cbfdf45846567deaebe27a334a
> >>
> >>
> >>
> >> I'd prfer Patchwork or Patchwork-ID or something not just Patch.
> >
> >
> >
> > Would it be more or less compelling if it had a format similar this?
> >
> > Patchwork: https://patchwork.ozlabs.org/patch/571773/
> 
> 
> 
>  Yes.
> >>>
> >>>
> >>>
> >>> Sorry, for dummy question ... what should I see in the above link?
> >>
> >>
> >> The link is the think to see, not what it points to. The idea is that
> >> instead of just the patch number, include the patch number in a full
> >> URL for even easier access to the patch.
> >
> >
> > Uh, ah, got it ... I think the patchworks patchnumber would be enough.
> > I like Bins proposal, that patchwork add it automatically ...
> 
> 
> 
> X-Patchwork-Id: 561384
> 
> I assume this tag points to the last version which was actually
> applied to the git tree.

It points to the message in Patchwork -- patchwork does not apply
patches on any tree; in fact, patchwork is totally unrelated to git.

> If you want to know the whole history of each patch,
> you should see former versions as well as the last one.
> 
> The last version is often mature,
> so it may end up with just having "Applied to u-boot/master, thanks!",
> which does not carry useful information at all.

The last message in the thread started by the patch will have this. But
that last message will be unknown by patchwork, which only records
patches [apart from rare false positive cases where PW thinks a message
is a patch, because there is git diff extract pasted into it].

> Nor do I want to see something as follows.
> X-Pachwork-Id-v1: 561184
> X-Pachwork-Id-v2: 561284
> X-Pachwork-Id-v3: 561384

AFAICT, patchwork is not able tell which patch is the vN-1 of a given vN
patch, and probably never will, as there is in fact no reliable way to
reconstruct this. A vN patch does not necessarily have the same subject
line as its vN-1, and does not necessarily have its vN-1's message-Id
listed in its 'Reference:' header.

So you would never see the above anyway.

What I have personally set up recently is a Python script which
goes through my Claws Mail U-Boot mail directory and, for each mail,
queries Patchwork with the Message-ID and, failing that, with each
Reference in turn, and adds X-Patchwork-{id,delegate,state...} to the
mail. There is some ad hoc logic for things like cover letters, and of
course, a caching mechanism so that I query PW only once per message-ID
-- so for a long thread about a single patch, there will be a single
actual query.

Oh, and it adds both an X-Patchwork-Id and an W-Patchwork-URL. :)

I run this regularly on my U-Boot inbox.

(if anyone knows how to query patchwork for "details for every patch
which has changed since the date I last asked", let me know.)

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

Re: [U-Boot] [PATCH V2] arm: config: enforce -fno-pic for gcc

[Albert ARIBAUD]

Hello Peng,

On Sat, 30 Jan 2016 12:10:49 +0800, Peng Fan <van.free...@gmail.com> wrote:
> Android's tool chain enable the -mandroid at default.
> This option will enable the -fpic, which cause uboot compilation
> failure:
> "
>  LD  u-boot
>  u-boot contains unexpected relocations: R_ARM_ABS32
>  R_ARM_RELATIVE
> "
> 
> In my testcase, arm-linux-androideabi-gcc-4.9 internally
> enables '-fpic', so when compiling code, there will be
> relocation entries using type R_ARM_GOT_BREL and .got
> section. When linking all the built-in.o using ld, there
> will be R_ARM_ABS32 relocation entry and .got section
> in the final u-boot elf image. This can not be handled
> by u-boot, since u-boot only expects R_ARM_RELATIVE
> relocation entry.
> arm-poky-linux-gnueabi-gcc-4.9 default does not enable '-fpic',
> so there is not .got section and R_ARM_GOT_BREL in built-in.o.
> And in the final u-boot elf image, all relocation entries are
> R_ARM_RELATIVE.
> 
> we can pass '-fno-pic' to xxx-gcc to disable pic. whether
> the toolchain internally enables or disables pic, '-fno-pic'
> can work well.
> 
> Signed-off-by: Peng Fan <peng@nxp.com>
> Cc: Albert Aribaud <albert.u.b...@aribaud.net>
> ---
> 
> V2:
>  Drop RFC.
> 
>  arch/arm/config.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> index a3e14a8..8fa57ec 100644
> --- a/arch/arm/config.mk
> +++ b/arch/arm/config.mk
> @@ -107,6 +107,7 @@ ALL-y += checkarmreloc
>  # instruction. Relocation is not supported for that case, so disable
>  # such usage by requiring word relocations.
>  PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
> +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
>  endif
>  
>  # limit ourselves to the sections we want in the .bin.
> -- 
> 2.6.2
> 

Applied to u-boot-arm/master, thanks!

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

Re: [U-Boot] [PATCH] arm: use common instructions applicable to armv7m & other arm archs

[Albert ARIBAUD]

Hello Vikas,

On Sat, 30 Jan 2016 00:36:55 +0100, Vikas MANOCHA
<vikas.mano...@st.com> wrote:
> Hi Albert,
> 
> > -Original Message-
> > From: Albert ARIBAUD [mailto:albert.u.b...@aribaud.net]
> > Sent: Friday, January 29, 2016 9:16 AM
> > To: Vikas MANOCHA
> > Cc: u-boot@lists.denx.de; Simon Glass; re...@wp.pl; Bo Shen; Przemyslaw
> > Marczak
> > Subject: Re: [PATCH] arm: use common instructions applicable to armv7m &
> > other arm archs
> > 
> > Hello Vikas,
> > 
> > On Mon, 18 Jan 2016 18:52:57 -0800, Vikas Manocha
> > <vikas.mano...@st.com> wrote:
> > > BIC instruction to clear the SP is not allowed in armv7m & is
> > > deprecated in ARMv6T2 & above. This patch cleans the code by using
> > > instructions allowed for armv7m as well as other Arm archs.
> > 
> > I am not against this patch, which has merits on its own; but the commit
> > message above raises a couple of questions.
> > 
> > 1. It seems to imply that in the current arch/arm/lib/crt0.S, BIC is
> >incorrectly being used on sp. However, it is *not*; it is used on r3,
> >precisely because it cannot be used directly on sp, as the comment
> >before the bic instruction clearly states. Why does the commit
> >message raise this non-issue?
> 
> For sure, BIC is used correctly. Currently BIC instruction is being used on 
> r3 for armv7m & on SP for others.
> The patch is to use same instruction for all ISAs & to make code cleaner.
> The intention of message " BIC instruction to clear the SP is not allowed in 
> armv7m" is to justify the usage of register (r0) instead of SP for BIC.
> Sorry if it created some confusion, how about following commit message for v2:
> 
> "This patch cleans the code by using instructions allowed for armv7m as well 
> as other Arm archs.
> Just for information, BIC instruction to clear the SP is not allowed in 
> armv7m & is deprecated in ARMv6T2 & above"

Almost good, but... the current code /already/ uses instructions
allowed for ARMv7-M. It even has a conditional on CONFIG_CPU_V7M under
which all instructions are ARMv7-compatible. Proof is, ARMv7 actually
builds (and runs fine).

Granted, if someone built for ARMv7-M but with CONFIG_CPU_V7M unset,
then the assembler would fail on the "BIC sp" instruction, which is
indeed forbidden in ARMv7-M; but such a build cannot happen unless
that same someone actively *circumvented* the U-Boot build process. 

What your patch does is therefore definitely not "Fix bad use of
BIC which breaks ARMv7", and not even "fix use of BIC that might cause
ARMv7 to break". Your patch is, however, "Streamline code and remove
ARMv7-M conditional by using only ARMv7-M-allowed instructions" -- and
this it does pretty well.

I would therefore completely drop the part about BIC from the commit
message as it is not actually a problem in the current code -- or more
to the point, as its limitations were already acknowledged and accounted
for in the current code.

However, if the bit about BIC really had to stay (with will require a
very strong reason), it should be corrected, because of the following:

> > 
> > 2. I could not find any information on BIC being deprecated in
> >Architecture Reference Manuals of either ArmV7 (section "Deprecated
> >Features in ARMv7-M", or ARMv6-M (section "Deprecated and Obsolete
> >Features"). Where does this information come from?
> 
> I got it from some of the web locations, one of them is:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473j/dom1361289864906.html

Firstly, the link above is *not* to the ARMv6-M Architecture Reference
Manual, but from the documentation of an ARM tool, namely the ARM DS-5
compiler (version 5.04). The only authoritative source to determine if
a feature is deprecated in a given ARM architecture is its Architecture
Reference Manual.

But even of the sentence above was authoritative, what it purports as
deprecated is the use of PC and SP in BIC, not the use of BIC itself,
whereas the proposed commit message might be read by a casual (or
non-casual but not quite attentive enough) reader might as "the BIC
instruction to clear the SP is not allowed in armv7m & the BIC
instruction is deprecated in ARMv6T2 & above".

At the very least, that part of the commit message should be reworded
as "Using BIC on SP (as well as PC) is deprecated in ARMv6T2 and above,
and forbidden in ARMv7-M".

However, my opinion is that the commit is not about fixing how BIC is
used but about getting rid of the ARMv7-M conditional, and therefore,
I think that its message should not mention BIC at all.

I would therefore limit the commit message to just this:

This patch cleans the code by using instructions allowed for
ARMv7-M as well as other Arm architectures.

> Cheers,
> Vikas

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

Re: [U-Boot] [RFC V1] arm: config: enforce -fno-pic for gcc

[Albert ARIBAUD]

Hello Peng,

On Sat, 5 Dec 2015 15:53:46 +0800, Peng Fan 
wrote:
> Android's tool chain enable the -mandroid at default.
> This option will enable the -fpic, which cause uboot compilation
> failure:
> "
>  LD  u-boot
>  u-boot contains unexpected relocations: R_ARM_ABS32
>  R_ARM_RELATIVE
> "

Any idea why Android's toolchain has -fpic enabled?

> we can pass '-fno-pic' to xxx-gcc to disable pic. whether
> the toolchain internally enables or disables pic, '-fno-pic'
> can work well.

No problem apparently with this RFC. If you submit a non-RFC patch
I will apply it.

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

Re: [U-Boot] [PATCH v2] ARM: Disable "DISCARD" for secure section if CONFIG_ARMV7_SECURE_BASE isn't defined

[Albert ARIBAUD]

Hello Tom,

On Tue, 19 Jan 2016 10:57:11 -0500, Tom Rini  wrote:
> On Mon, Jan 18, 2016 at 11:02:40AM +0800, Dongsheng Wang wrote:
> 
> > From: Wang Dongsheng 
> > 
> > "DISCARD" will remove ._secure.text relocate, but PSCI framework
> > has already used some absolute address those need to relocate.
> > 
> > Use readelf -t -r u-boot show us:
> > .__secure_start addr: 601408e4
> > .__secure_end   addr: 60141460
> > 
> > 60141140  0017 R_ARM_RELATIVE
> > 46  _secure_monitor:
> > 47  #ifdef CONFIG_ARMV7_PSCI
> > 48  ldr r5, =_psci_vectors
> > 
> > 60141194  0017 R_ARM_RELATIVE
> > 6014119c  0017 R_ARM_RELATIVE
> > 601411a4  0017 R_ARM_RELATIVE
> > 601411ac  0017 R_ARM_RELATIVE
> > 64  _psci_table:
> > 66  .word   psci_cpu_suspend
> > ...
> > 72  .word   psci_migrate
> > 
> > 60141344  0017 R_ARM_RELATIVE
> > 6014145c  0017 R_ARM_RELATIVE
> > 202 ldr r5, =psci_text_end
> > 
> > Solutions:
> > 1. Change absolute address to RelAdr.
> >Based on LDR (immediate, ARM), we only have 4K offset to jump.
> > Now PSCI code size is close to 4K size that is LDR limit jump size,
> > so even if the LDR is based on the current instruction address,
> > there is also have a risk for RelAdr. If we use two jump steps I
> > think we can fix this issue, but looks too hack, so give up this way.
> > 
> > 2. Enable "DISCARD" only for CONFIG_ARMV7_SECURE_BASE has defined.
> >If CONFIG_ARMV7_SECURE_BASE is defined in platform, all of secure
> > will in the BASE address that is absolute.
> > 
> > Signed-off-by: Wang Dongsheng 
> > 
> > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> > index d48a905..e148ab7 100644
> 
> Reviewed-by: Tom Rini 
> 
> -- 
> Tom

Applied to u-boot-arm/master, thanks!

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

Re: [U-Boot] [PATCH 1/2] arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7

[Albert ARIBAUD]

Hello Marek,

On Tue, 29 Dec 2015 19:44:01 +0100, Marek Vasut <ma...@denx.de> wrote:
> The arch/arm/lib/cache-cp15.c checks for CONFIG_ARMV7 and if this macro is
> set, it configures TTBR0 register. This register must be configured for the
> cache on ARMv7 to operate correctly.
> 
> The problem is that noone actually sets the CONFIG_ARMV7 macro and thus the
> TTBR0 is not configured at all. On SoCFPGA, this produces all sorts of minor
> issues which are hard to replicate, for example certain USB sticks are not
> detected or QSPI NOR sometimes fails to write pages completely.
> 
> The solution is to replace CONFIG_ARMV7 test with CONFIG_CPU_V7 one. This is
> correct because the code which added the test(s) for CONFIG_ARMV7 was added
> shortly after CONFIG_ARMV7 was replaced by CONFIG_CPU_V7 and this code was
> not adjusted correctly to reflect that change.
> 
> Signed-off-by: Marek Vasut <ma...@denx.de>
> Cc: Tom Rini <tr...@konsulko.com>
> Cc: Albert Aribaud <albert.u.b...@aribaud.net>
> Cc: Simon Glass <s...@chromium.org>
> ---
>  arch/arm/include/asm/system.h | 4 ++--
>  arch/arm/lib/cache-cp15.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> index 71b3108..dec83c7 100644
> --- a/arch/arm/include/asm/system.h
> +++ b/arch/arm/include/asm/system.h
> @@ -220,7 +220,7 @@ static inline void set_dacr(unsigned int val)
>   isb();
>  }
>  
> -#ifdef CONFIG_ARMV7
> +#ifdef CONFIG_CPU_V7
>  /* Short-Descriptor Translation Table Level 1 Bits */
>  #define TTB_SECT_NS_MASK (1 << 19)
>  #define TTB_SECT_NG_MASK (1 << 17)
> @@ -257,7 +257,7 @@ enum {
>   MMU_SECTION_SIZE= 1 << MMU_SECTION_SHIFT,
>  };
>  
> -#ifdef CONFIG_ARMV7
> +#ifdef CONFIG_CPU_V7
>  /* TTBR0 bits */
>  #define TTBR0_BASE_ADDR_MASK 0xC000
>  #define TTBR0_RGN_NC (0 << 3)
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index c65e068..8e18538 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -96,7 +96,7 @@ static inline void mmu_setup(void)
>   dram_bank_mmu_setup(i);
>   }
>  
> -#ifdef CONFIG_ARMV7
> +#ifdef CONFIG_CPU_V7
>   /* Set TTBR0 */
>   reg = gd->arch.tlb_addr & TTBR0_BASE_ADDR_MASK;
>  #if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
> -- 
> 2.1.4
> 

Series applied to u-boot-arm/master, thanks!

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

Re: [U-Boot] [PATCH] arm: use common instructions applicable to armv7m & other arm archs

[Albert ARIBAUD]

Hello Vikas,

On Mon, 18 Jan 2016 18:52:57 -0800, Vikas Manocha
 wrote:
> BIC instruction to clear the SP is not allowed in armv7m & is deprecated
> in ARMv6T2 & above. This patch cleans the code by using instructions allowed
> for armv7m as well as other Arm archs.

I am not against this patch, which has merits on its own; but the
commit message above raises a couple of questions.

1. It seems to imply that in the current arch/arm/lib/crt0.S, BIC is
   incorrectly being used on sp. However, it is *not*; it is used on r3,
   precisely because it cannot be used directly on sp, as the comment
   before the bic instruction clearly states. Why does the commit
   message raise this non-issue?

2. I could not find any information on BIC being deprecated in
   Architecture Reference Manuals of either ArmV7 (section "Deprecated
   Features in ARMv7-M", or ARMv6-M (section "Deprecated and Obsolete
   Features"). Where does this information come from?

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

Re: [U-Boot] [PATCH v2] armv7: add cacheline sizes where missing

[Albert ARIBAUD]

On Wed, 27 Jan 2016 08:46:11 +0100, Albert ARIBAUD
<albert.u.b...@aribaud.net> wrote:
> Some armv7 targets are missing a cache line size declaration.
> In preparation for "arm: cache: Implement cache range check for v7"
> patch, add these declarations with the appropriate value for
> the target's SoC or CPU.
> 
> Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
> ---
> 
> Changes in v2:
> - fix include/configs/at91-sama5_common.h (Cortex-A5: 32 bytes)

Applied to u-boot-arm/master, adding Tom's Reviewed-by from V1 since
there was no change to TI boards in V2.

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

Re: [U-Boot] [PATCH v2] armv7: add cacheline sizes where missing

[Albert ARIBAUD]

On Wed, 27 Jan 2016 08:46:11 +0100, Albert ARIBAUD
<albert.u.b...@aribaud.net> wrote:
> Some armv7 targets are missing a cache line size declaration.
> In preparation for "arm: cache: Implement cache range check for v7"
> patch, add these declarations with the appropriate value for
> the target's SoC or CPU.
> 
> Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
> ---
> 
> Changes in v2:
> - fix include/configs/at91-sama5_common.h (Cortex-A5: 32 bytes)

Absent any complaint today, I will apply this as a prerequisite for
Marek's "arm: cache: Implement cache range check for v7".

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

Re: [U-Boot] patman dies cryptially when a (valid) e-mail address contains parentheses (or is UTF-8)

[Albert ARIBAUD]

Hello Simon,

On Wed, 27 Jan 2016 15:53:45 -0700, Simon Glass <s...@chromium.org>
wrote:
> Hi Albert,
> 
> On 27 January 2016 at 01:22, Albert ARIBAUD <albert.u.b...@aribaud.net> wrote:
> > Hello Simon,
> >
> > I just noticed this while adding a Series-cc to my work address which
> > has parentheses: running patman (without -n) will result in
> >
> > sh: 1: Syntax error: "(" unexpected
> >
> > Which initially left me wondering what was happening until I realized
> > one of the Series-cc addresses had parentheses, e.g.
> >
> > Series-cc: "Name NAME (NAME)" <addr...@domain.tld>
> >
> > Note that the parentheses were in the free-form part of the address,
> > itself within double quotation marks, which appears valid wrt the RFC.
> >
> > I've tried using single quotes as a workaround:
> >
> > Series-cc: 'Name NAME (NAME)' <addr...@domain.tld>
> >
> > It kind-of-works in that patman does not die, but the resulting address
> > in the mail has outer double quotes and inner single quotes, e.g.
> >
> > "'Name NAME (NAME)'" <addr...@domain.tld>
> >
> > Aditionally, addresses with names in UTF-8 also fail, though
> > differently but still with a message somewhat unrelated to the actual
> > cause (UTF-8-name is the placeholder for a name containing UTF-8
> > diacritics):
> >
> > fatal: ambiguous argument 'UTF-8-name <add...@domain.tld>':
> > unknown revision or path not in the working tree.
> > Use '--' to separate paths from revisions, like this:
> > 'git  [...] -- [...]'
> >
> > The same single-quote hack works it around, with the resulting
> > mail Cc:ing the name surrounded by single quotes only:
> >
> > Cc: 'UTF-8-name' <add...@domain.tld>
> >
> > Cc:ing Marek who is a die-hard fan of UTF-8 names in e-mail
> > addresses. :)
> 
> I wonder if this is a problem with the cc_cmd code in patman.py?
> 
> # Called from git with a patch filename as argument
> # Printout a list of additional CC recipients for this patch
> elif options.cc_cmd:
> 
> Perhaps it should quote its output somehow?

I'll try and test that this week-end.

> Regards,
> Simon

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

[U-Boot] patman dies cryptially when a (valid) e-mail address contains parentheses (or is UTF-8)

[Albert ARIBAUD]

Hello Simon,

I just noticed this while adding a Series-cc to my work address which
has parentheses: running patman (without -n) will result in

sh: 1: Syntax error: "(" unexpected

Which initially left me wondering what was happening until I realized
one of the Series-cc addresses had parentheses, e.g.

Series-cc: "Name NAME (NAME)" 

Note that the parentheses were in the free-form part of the address,
itself within double quotation marks, which appears valid wrt the RFC.

I've tried using single quotes as a workaround:

Series-cc: 'Name NAME (NAME)' 

It kind-of-works in that patman does not die, but the resulting address
in the mail has outer double quotes and inner single quotes, e.g.

"'Name NAME (NAME)'" 

Aditionally, addresses with names in UTF-8 also fail, though
differently but still with a message somewhat unrelated to the actual
cause (UTF-8-name is the placeholder for a name containing UTF-8
diacritics):

fatal: ambiguous argument 'UTF-8-name ':
unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git  [...] -- [...]'

The same single-quote hack works it around, with the resulting
mail Cc:ing the name surrounded by single quotes only:

Cc: 'UTF-8-name' 

Cc:ing Marek who is a die-hard fan of UTF-8 names in e-mail
addresses. :)

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

Re: [U-Boot] [PATCH] ARM: zynq: Add function to get reboot status register value.

[Albert ARIBAUD]

Hello Moritz,

On Wed, 27 Jan 2016 14:53:30 +0100, Moritz Fischer
 wrote:
> Hi Michal,
> 
> On Wed, Jan 27, 2016 at 2:15 PM, Michal Simek  wrote:
> > On 27.1.2016 12:22, Moritz Fischer wrote:
> >> Signed-off-by: Moritz Fischer 
> >> ---
> >> Hi Michal,
> >>
> >>I was planning to use this in future to boot into recovery mode.
> >>The change is small enough I feel that we could directly take it.
> >>If you want to hold off until there's a user that's fine for me, too.
> >>
> >>Cheers,
> >>
> >>Moritz
> >> ---
> >>  arch/arm/mach-zynq/slcr.c | 5 +
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/arch/arm/mach-zynq/slcr.c b/arch/arm/mach-zynq/slcr.c
> >> index 05f4099..709711a 100644
> >> --- a/arch/arm/mach-zynq/slcr.c
> >> +++ b/arch/arm/mach-zynq/slcr.c
> >> @@ -166,6 +166,11 @@ u32 zynq_slcr_get_boot_mode(void)
> >>   return readl(_base->boot_mode);
> >>  }
> >>
> >> +u32 zynq_slcr_get_reboot_status(void)
> >> +{
> >> + return readl(_base->reboot_status);
> >> +}
> >> +
> >>  u32 zynq_slcr_get_idcode(void)
> >>  {
> >>   return (readl(_base->pss_idcode) & SLCR_IDCODE_MASK) >>
> >>
> >
> > Isn't this generating sparse warning if it is not used and declared?
> 
> Derp. It doesn't generate a warning for not being used, however for
> not being declared it does,
> the entry in sys_proto.h is missing:
> 
>   CHECK   arch/arm/mach-zynq/slcr.c
> arch/arm/mach-zynq/slcr.c:97:9: warning: cast to restricted __le32
> arch/arm/mach-zynq/slcr.c:97:9: warning: incorrect type in assignment
> (different base types)
> arch/arm/mach-zynq/slcr.c:97:9:expected unsigned int volatile
> [unsigned] 
> arch/arm/mach-zynq/slcr.c:97:9:got restricted __le32 [usertype] 
> arch/arm/mach-zynq/slcr.c:169:5: warning: symbol
> 'zynq_slcr_get_reboot_status' was not declared. Should it be static?
>   CC  arch/arm/mach-zynq/slcr.o
>   LD  arch/arm/mach-zynq/built-in.o
> 
> when added to header this becomes:
> 
>   CHECK   arch/arm/mach-zynq/slcr.c
> arch/arm/mach-zynq/slcr.c:97:9: warning: cast to restricted __le32
> arch/arm/mach-zynq/slcr.c:97:9: warning: incorrect type in assignment
> (different base types)
> arch/arm/mach-zynq/slcr.c:97:9:expected unsigned int volatile
> [unsigned] 
> arch/arm/mach-zynq/slcr.c:97:9:got restricted __le32 [usertype] 
>   CC  arch/arm/mach-zynq/slcr.o
>   LD  arch/arm/mach-zynq/built-in.o
> 
> These were there before ;-)
> 
> I can resend with the header line added,
> 
> Moritz

Please add the header line but I would prefer that you submit it
as part of the series where it is used. Changes small enough to be
taken directly can end up dead code just like bigger changes, only
harder to locate -- especially if the compiler does not complain about
them. :)

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

Re: [U-Boot] [PATCH v2 2/2] net: phy: Add PHY driver for mv88e61xx switches

[Albert ARIBAUD]

Hello Kevin,

On Wed, 27 Jan 2016 16:29:42 +, Kevin Smith
 wrote:
> Hi Joe,

> On 01/26/2016 06:11 PM, Joe Hershberger wrote:

> >> +   /* Replace the bus with the fake device */
> > Fake how? This is a confusing comment to me as written.
> The genphy functions assume that they can write to the PHY directly 
> using the MII bus, and the address it uses is the address of a 
> register.  This is not the case for this chip with multiple PHY 
> interfaces, which have to be accessed indirectly.  To handle this, I 
> have created a "fake" mii_dev whose read/write functions are the 
> indirection functions, and stored the actual mdio_bus in the private 
> data for the "fake" device, which is then used by the indirect 
> functions.  This allows this driver to make use of common genphy stuff 
> where appropriate.  Maybe "wrapper" or "indirect bus" is a better name 
> for it.  Let me know if you have a preference or better idea.

"Indirect bus" is better IMO, as it gives a clue about what actually
happens.

> >> +
> >> +   mac_addr = phydev->addr;
> >> +
> >> +   for (i = 0; i < PORT_COUNT; i++) {
> >> +   if ((1 << i) & CONFIG_MV88E61XX_PHY_PORTS) {
> >> +   phydev->addr = i;
> >> +   mv88e61xx_phy_enable(phydev, i);
> >> +   mv88e61xx_phy_setup(phydev, i);
> >> +   mv88e61xx_phy_config_port(phydev, i);
> > These all return status, but are ignored.
> Even if one fails, it seems appropriate to me to continue initializing 
> the others and not bail completely.  Should I catch the error, print a 
> warning and "continue" in the loop?  Or is completely bailing the right 
> thing to do?  If there are some errors, but other successes, what should 
> the function return?

Warn for each port switch initialization failure, bail out if no port
could be initialized?

> Thanks,
> Kevin

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

Re: [U-Boot] [PATCH] armv7: add cacheline sizes where missing

[Albert ARIBAUD]

Hello Marek,

On Wed, 27 Jan 2016 05:10:59 +0100, Marek Vasut <ma...@denx.de> wrote:
> On Tuesday, January 26, 2016 at 05:40:49 PM, Albert ARIBAUD wrote:
> > Some armv7 targets are missing a cache line size declaration.
> > In preparation for "arm: cache: Implement cache range check for v7"
> > patch, add these declarations with the appropriate value for
> > the target's SoC or CPU.
> > 
> > Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
> 
> [...]
> 
> > diff --git a/include/configs/at91-sama5_common.h
> > b/include/configs/at91-sama5_common.h index 9db4a4f..7b06601 100644
> > --- a/include/configs/at91-sama5_common.h
> > +++ b/include/configs/at91-sama5_common.h
> > @@ -12,6 +12,8 @@
> > 
> >  #include 
> > 
> > +#define CONFIG_SYS_CACHELINE_SIZE  64
> > +
> >  #define CONFIG_SYS_TEXT_BASE   0x26f0
> > 
> >  /* ARM asynchronous clock */
> 
> I think SAMA5 is CortexA5 and that has 32B cachelines.

Correct. V2 incoming.

> Best regards,
> Marek Vasut

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

[U-Boot] [PATCH v2] armv7: add cacheline sizes where missing

[Albert ARIBAUD]

Some armv7 targets are missing a cache line size declaration.
In preparation for "arm: cache: Implement cache range check for v7"
patch, add these declarations with the appropriate value for
the target's SoC or CPU.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---

Changes in v2:
- fix include/configs/at91-sama5_common.h (Cortex-A5: 32 bytes)

 include/configs/am3517_crane.h  | 2 ++
 include/configs/am3517_evm.h| 2 ++
 include/configs/at91-sama5_common.h | 2 ++
 include/configs/bcm_ep_board.h  | 1 +
 include/configs/cm_t35.h| 2 ++
 include/configs/cm_t3517.h  | 2 ++
 include/configs/colibri_vf.h| 2 ++
 include/configs/kzm9g.h | 2 ++
 include/configs/nokia_rx51.h| 2 ++
 include/configs/pcm052.h| 2 ++
 include/configs/rcar-gen2-common.h  | 2 ++
 include/configs/rk3036_common.h | 2 ++
 include/configs/rk3288_common.h | 2 ++
 include/configs/s5p_goni.h  | 2 ++
 include/configs/smdkc100.h  | 2 ++
 include/configs/tao3530.h   | 2 ++
 include/configs/ti814x_evm.h| 2 ++
 include/configs/ti816x_evm.h| 2 ++
 include/configs/ti_omap3_common.h   | 5 +
 include/configs/tricorder.h | 2 ++
 include/configs/vexpress_common.h   | 2 ++
 include/configs/vf610twr.h  | 2 ++
 22 files changed, 46 insertions(+)

diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
index 4ed8e00..3cc2874 100644
--- a/include/configs/am3517_crane.h
+++ b/include/configs/am3517_crane.h
@@ -13,6 +13,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index 23457d6..4547d7f 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -13,6 +13,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/at91-sama5_common.h 
b/include/configs/at91-sama5_common.h
index 9db4a4f..d692106 100644
--- a/include/configs/at91-sama5_common.h
+++ b/include/configs/at91-sama5_common.h
@@ -12,6 +12,8 @@
 
 #include 
 
+#define CONFIG_SYS_CACHELINE_SIZE  32
+
 #define CONFIG_SYS_TEXT_BASE   0x26f0
 
 /* ARM asynchronous clock */
diff --git a/include/configs/bcm_ep_board.h b/include/configs/bcm_ep_board.h
index 305864f..1d4869b 100644
--- a/include/configs/bcm_ep_board.h
+++ b/include/configs/bcm_ep_board.h
@@ -11,6 +11,7 @@
 
 #define CONFIG_SKIP_LOWLEVEL_INIT
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
 
 /*
  * Memory configuration
diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
index 2dc745e..24ae14d 100644
--- a/include/configs/cm_t35.h
+++ b/include/configs/cm_t35.h
@@ -17,6 +17,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/cm_t3517.h b/include/configs/cm_t3517.h
index 0aefec8..7a07de4 100644
--- a/include/configs/cm_t3517.h
+++ b/include/configs/cm_t3517.h
@@ -10,6 +10,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/colibri_vf.h b/include/configs/colibri_vf.h
index 5aed3a5..dd44462 100644
--- a/include/configs/colibri_vf.h
+++ b/include/configs/colibri_vf.h
@@ -12,6 +12,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  32
+
 #include 
 
 #define CONFIG_VF610
diff --git a/include/configs/kzm9g.h b/include/configs/kzm9g.h
index 94b0f03..d5cbb33 100644
--- a/include/configs/kzm9g.h
+++ b/include/configs/kzm9g.h
@@ -10,6 +10,8 @@
 
 #undef DEBUG
 
+#define CONFIG_SYS_CACHELINE_SIZE  32
+
 #define CONFIG_SH73A0
 #define CONFIG_KZM_A9_GT
 #define CONFIG_RMOBILE_BOARD_STRING"KMC KZM-A9-GT"
diff --git a/include/configs/nokia_rx51.h b/include/configs/nokia_rx51.h
index b11e43a..5c41405 100644
--- a/include/configs/nokia_rx51.h
+++ b/include/configs/nokia_rx51.h
@@ -19,6 +19,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE 64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/pcm052.h b/include/configs/pcm052.h
index 891bdb0..2628dfa 100644
--- a/include/configs/pcm052.h
+++ b/include/configs/pcm052.h
@@ -9,6 +9,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  32
+
 #include 
 
 #define CONFIG_VF610
diff --git a/include/configs/rcar-gen2-common.h 
b/include/configs/rcar-gen2-common.h
index f0a3a18..f750b53 100644
--- a/include/configs/rcar-gen2-common.h
+++ b/include/configs/rcar-gen2-common.h
@@ -9,6 +9,8 @@
 #ifndef __RCAR_GEN2_COMMON_H
 #define __RCAR_GEN2_COMMON_H
 
+#define CONFIG_SYS_CACHELINE_SIZE 64
+
 #include 
 
 #define CONFIG_CMD_DFL
diff --git a/include/configs

Re: [U-Boot] [PATCH v2 0/2] net: phy: mv88e61xx: Revise as a PHY driver

[Albert ARIBAUD]

Hello Kevin,

On Mon, 21 Dec 2015 21:45:32 +, Kevin Smith
 wrote:
> The previous version of this driver implemented a shell command to manually
> comfigure the switch.  It did not integrate with the PHY infrastructure to
> allow a MAC to use it as its PHY.  This is a complete rewrite to allow this
> switch to function as a driver.  Since none of the original driver remains, 
> the
> old driver is first removed and the new PHY driver is added.
> 
> This version configures the switch to have a CPU connected over an MII
> interface.  It will enable PHY interfaces based on the MV88E61XX_PHY_PORTS
> macro.  The switch is configured to allow PHY ports to only communicate to the
> CPU.  This allows the switch to be used as a basic PHY on any/all ports.
> 
> This was developed on a board with an mv88e6176 connected over SGMII.  It is
> intended to work with other configurations, but these could not be tested.  
> Any
> testing on other configurations or with other mv88e61xx chips is appreciated.

Actually the driver was for the Wireless Space, which was removed but
which I will resurrect as I did the Open-RDs. I will test the switch on
the WS I have.

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

[U-Boot] [PATCH] armv7: add cacheline sizes where missing

[Albert ARIBAUD]

Some armv7 targets are missing a cache line size declaration.
In preparation for "arm: cache: Implement cache range check for v7"
patch, add these declarations with the appropriate value for
the target's SoC or CPU.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---

 include/configs/am3517_crane.h  | 2 ++
 include/configs/am3517_evm.h| 2 ++
 include/configs/at91-sama5_common.h | 2 ++
 include/configs/bcm_ep_board.h  | 1 +
 include/configs/cm_t35.h| 2 ++
 include/configs/cm_t3517.h  | 2 ++
 include/configs/colibri_vf.h| 2 ++
 include/configs/kzm9g.h | 2 ++
 include/configs/nokia_rx51.h| 2 ++
 include/configs/pcm052.h| 2 ++
 include/configs/rcar-gen2-common.h  | 2 ++
 include/configs/rk3036_common.h | 2 ++
 include/configs/rk3288_common.h | 2 ++
 include/configs/s5p_goni.h  | 2 ++
 include/configs/smdkc100.h  | 2 ++
 include/configs/tao3530.h   | 2 ++
 include/configs/ti814x_evm.h| 2 ++
 include/configs/ti816x_evm.h| 2 ++
 include/configs/ti_omap3_common.h   | 5 +
 include/configs/tricorder.h | 2 ++
 include/configs/vexpress_common.h   | 2 ++
 include/configs/vf610twr.h  | 2 ++
 22 files changed, 46 insertions(+)

diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
index 4ed8e00..3cc2874 100644
--- a/include/configs/am3517_crane.h
+++ b/include/configs/am3517_crane.h
@@ -13,6 +13,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index 23457d6..4547d7f 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -13,6 +13,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/at91-sama5_common.h 
b/include/configs/at91-sama5_common.h
index 9db4a4f..7b06601 100644
--- a/include/configs/at91-sama5_common.h
+++ b/include/configs/at91-sama5_common.h
@@ -12,6 +12,8 @@
 
 #include 
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
+
 #define CONFIG_SYS_TEXT_BASE   0x26f0
 
 /* ARM asynchronous clock */
diff --git a/include/configs/bcm_ep_board.h b/include/configs/bcm_ep_board.h
index 305864f..1d4869b 100644
--- a/include/configs/bcm_ep_board.h
+++ b/include/configs/bcm_ep_board.h
@@ -11,6 +11,7 @@
 
 #define CONFIG_SKIP_LOWLEVEL_INIT
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
 
 /*
  * Memory configuration
diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
index 2dc745e..24ae14d 100644
--- a/include/configs/cm_t35.h
+++ b/include/configs/cm_t35.h
@@ -17,6 +17,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/cm_t3517.h b/include/configs/cm_t3517.h
index 0aefec8..7a07de4 100644
--- a/include/configs/cm_t3517.h
+++ b/include/configs/cm_t3517.h
@@ -10,6 +10,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/colibri_vf.h b/include/configs/colibri_vf.h
index 5aed3a5..dd44462 100644
--- a/include/configs/colibri_vf.h
+++ b/include/configs/colibri_vf.h
@@ -12,6 +12,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  32
+
 #include 
 
 #define CONFIG_VF610
diff --git a/include/configs/kzm9g.h b/include/configs/kzm9g.h
index 94b0f03..d5cbb33 100644
--- a/include/configs/kzm9g.h
+++ b/include/configs/kzm9g.h
@@ -10,6 +10,8 @@
 
 #undef DEBUG
 
+#define CONFIG_SYS_CACHELINE_SIZE  32
+
 #define CONFIG_SH73A0
 #define CONFIG_KZM_A9_GT
 #define CONFIG_RMOBILE_BOARD_STRING"KMC KZM-A9-GT"
diff --git a/include/configs/nokia_rx51.h b/include/configs/nokia_rx51.h
index b11e43a..5c41405 100644
--- a/include/configs/nokia_rx51.h
+++ b/include/configs/nokia_rx51.h
@@ -19,6 +19,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE 64
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/pcm052.h b/include/configs/pcm052.h
index 891bdb0..2628dfa 100644
--- a/include/configs/pcm052.h
+++ b/include/configs/pcm052.h
@@ -9,6 +9,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_CACHELINE_SIZE  32
+
 #include 
 
 #define CONFIG_VF610
diff --git a/include/configs/rcar-gen2-common.h 
b/include/configs/rcar-gen2-common.h
index f0a3a18..f750b53 100644
--- a/include/configs/rcar-gen2-common.h
+++ b/include/configs/rcar-gen2-common.h
@@ -9,6 +9,8 @@
 #ifndef __RCAR_GEN2_COMMON_H
 #define __RCAR_GEN2_COMMON_H
 
+#define CONFIG_SYS_CACHELINE_SIZE 64
+
 #include 
 
 #define CONFIG_CMD_DFL
diff --git a/include/configs/rk3036_common.h b/include/configs/rk3036_common.h
index d22ea74..368d046 10

Re: [U-Boot] [PATCH v2 0/2] net: phy: mv88e61xx: Revise as a PHY driver

[Albert ARIBAUD]

Hello Kevin,

On Tue, 26 Jan 2016 16:56:21 +, Kevin Smith
<kevin.sm...@elecsyscorp.com> wrote:
> Hi Albert,
> 
> On 01/26/2016 10:13 AM, Joe Hershberger wrote:
> > Hi Albert,
> >
> > On Tue, Jan 26, 2016 at 10:09 AM, Albert ARIBAUD
> > <albert.u.b...@aribaud.net> wrote:
> >> Hello Kevin,
> >>
> >> On Mon, 21 Dec 2015 21:45:32 +, Kevin Smith
> >> <kevin.sm...@elecsyscorp.com> wrote:
> >>> The previous version of this driver implemented a shell command to 
> >>> manually
> >>> comfigure the switch.  It did not integrate with the PHY infrastructure to
> >>> allow a MAC to use it as its PHY.  This is a complete rewrite to allow 
> >>> this
> >>> switch to function as a driver.  Since none of the original driver 
> >>> remains, the
> >>> old driver is first removed and the new PHY driver is added.
> >>>
> >>> This version configures the switch to have a CPU connected over an MII
> >>> interface.  It will enable PHY interfaces based on the MV88E61XX_PHY_PORTS
> >>> macro.  The switch is configured to allow PHY ports to only communicate 
> >>> to the
> >>> CPU.  This allows the switch to be used as a basic PHY on any/all ports.
> >>>
> >>> This was developed on a board with an mv88e6176 connected over SGMII.  It 
> >>> is
> >>> intended to work with other configurations, but these could not be 
> >>> tested.  Any
> >>> testing on other configurations or with other mv88e61xx chips is 
> >>> appreciated.
> >> Actually the driver was for the Wireless Space, which was removed but
> >> which I will resurrect as I did the Open-RDs. I will test the switch on
> >> the WS I have.
> > That's great. I'm currently reviewing the new driver and will
> > appreciate the testing. I don't have any boards with this switch.
> >
> > Thanks,
> > -Joe
> Could you please send me more info on the Wireless Space board?  I would 
> like something other than our custom board to test this on.  I tried to 
> get a Marvell RD-A370, but they are too expensive.  I will be glad to 
> add back in the command-line interface for configuring this board if it 
> is needed to support your work.

Not sure if you can find one... You can find some doc there:

http://lacie.nas-central.org/wiki/Category:Wireless_Space

The switch is a 88E6161-LG02.

> Thanks for testing,
> Kevin
> 
> P.S.  I did not receive your e-mail at my business address somehow; I 
> added my personal e-mail to ensure I get your responses.

Weird. Maybe your business mail exchanger relies on a blocking list
which considers that only professional mail providers should be allowed
to deliver e-mail; I am using my own mail exchanger. Thanks for letting
me know!

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

Re: [U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Albert ARIBAUD]

Hello Joe,

On Mon, 25 Jan 2016 18:45:52 -0600, Joe Hershberger
 wrote:
> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>  wrote:

> > +++ b/tools/gen_mac_addr.c
> 
> This is not "generating a mac address", right? Its point is to create
> a CRC for the user-supplied MAC address.
> 
> Perhaps a better name would be "gen_ethaddr_rom_crc".

Hmm, why the "_rom" part?

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

Re: [U-Boot] [PATCH] arm: cache: Implement cache range check for v7

[Albert ARIBAUD]

Hello Tom,

On Mon, 25 Jan 2016 11:04:59 -0500, Tom Rini <tr...@konsulko.com> wrote:
> On Sun, Jan 24, 2016 at 06:28:12PM +0100, Marek Vasut wrote:
> > On Monday, December 28, 2015 at 04:23:46 PM, Marek Vasut wrote:
> > > On Friday, December 11, 2015 at 05:36:15 AM, Marek Vasut wrote:
> > > > On Tuesday, December 01, 2015 at 06:23:17 PM, Marek Vasut wrote:
> > > > > On Monday, July 27, 2015 at 10:34:17 PM, Marek Vasut wrote:
> > > > > > Add code to aid tracking down cache alignment issues.
> > > > > > In case DEBUG is defined in the cache.c, this code will
> > > > > > check alignment of each attempt to flush/invalidate data
> > > > > > cache and print a warning if the alignment is incorrect.
> > > > > > If DEBUG is not defined, this code is optimized out.
> > > > > > 
> > > > > > Signed-off-by: Marek Vasut <ma...@denx.de>
> > > > > > Cc: Dinh Nguyen <dingu...@opensource.altera.com>
> > > > > > Cc: Albert Aribaud <albert.u.b...@aribaud.net>
> > > > > > Cc: Tom Rini <tr...@konsulko.com>
> > > > > 
> > > > > Bump ?
> > > > 
> > > > Bump ?
> > > 
> > > Bump #3 ?
> > 
> > Bump #4 ?
> 
> Albert?

Sorry. Applied to u-boot-arm/master, thanks.

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

Re: [U-Boot] axm SPL image too big

[Albert ARIBAUD]

Hello Heiko,

On Tue, 12 Jan 2016 11:48:17 +0100, Heiko Schocher <h...@denx.de> wrote:
> Hello Albert,
> 
> Am 12.01.2016 um 11:26 schrieb Albert ARIBAUD:
> > (cc:ing Heiko as the maintainer for axm/taurus)
> >
> > Hello,
> >
> > AXM is currently the only board failing 'buildman arm aarch64':
> >
> > +arm-unknown-linux-gnueabi-ld.bfd: SPL image too big
> > +make[2]: *** [spl/u-boot-spl] Error 1
> > +make[1]: *** [spl/u-boot-spl] Error 2
> > +make: *** [sub-make] Error 2
> >
> > Apparently SPL is already being built in Thumb instruction set, so no
> > way to gain anything that way.
> 
> I know, it was tricky to get SPL into 4k ...
> 
> > What else can we do to get SPL size back under limit?
> 
> Hmm.. what is your exact toolchain?

I tested with the buildman-fetched gcc, -v gives 'gcc version 4.9.0
(GCC)'.

I've deleted the toolchain and re-fetched it again through buildman in
case there was an issue with the install, and am getting the same
result.

I've also tested with the latest ARM gcc package available with Ubuntu,
that is, 'gcc version 5.2.1 20151010 (Ubuntu 5.2.1-22ubuntu1)', again
with the same result.

> I see for the axm board:
> 
> With eldk-5.4
> pollux:u-boot-smartweb hs [master] $ arm-linux-gnueabi-size u-boot
> textdata bss dec hex filename
>   212583   23792   23552  259927   3f757 u-boot
> pollux:u-boot-smartweb hs [master] $ arm-linux-gnueabi-size spl/u-boot-spl
> textdata bss dec hex filename
>1456812121140   169204218 spl/u-boot-spl
> pollux:u-boot-smartweb hs [master] $

Confirmed right now that with eldk 5.4's gcc 4.7.2 axm builds fine.

> With eldk-5.5
> pollux:u-boot-smartweb hs [master] $ arm-linux-gnueabi-size u-boot
> textdata bss dec hex filename
>   210690   23672   23556  257918   3ef7e u-boot
> pollux:u-boot-smartweb hs [master] $ arm-linux-gnueabi-size spl/u-boot-spl
> textdata bss dec hex filename
>1447212121140   1682441b8 spl/u-boot-spl
> pollux:u-boot-smartweb hs [master] $

Confirmed right now that with eldk 5.5's (or 5.5.3's) gcc 4.8.1 axm
builds fine.

The issue appears with gcc 4.9 and 5.2.1 (and I suspect any version
between 4.9 and 5.2.1 and beyond 5.2.1)

> bye,
> Heiko
> -- 
> DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[U-Boot] axm SPL image too big

[Albert ARIBAUD]

(cc:ing Heiko as the maintainer for axm/taurus)

Hello,

AXM is currently the only board failing 'buildman arm aarch64':

+arm-unknown-linux-gnueabi-ld.bfd: SPL image too big
+make[2]: *** [spl/u-boot-spl] Error 1
+make[1]: *** [spl/u-boot-spl] Error 2
+make: *** [sub-make] Error 2

Apparently SPL is already being built in Thumb instruction set, so no
way to gain anything that way.

What else can we do to get SPL size back under limit?

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

Re: [U-Boot] [PATCH 1/2] arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7

[Albert ARIBAUD]

Hello Marek,

On Tue, 29 Dec 2015 19:44:01 +0100, Marek Vasut  wrote:
> The arch/arm/lib/cache-cp15.c checks for CONFIG_ARMV7 and if this macro is
> set, it configures TTBR0 register. This register must be configured for the
> cache on ARMv7 to operate correctly.
> 
> The problem is that noone actually sets the CONFIG_ARMV7 macro and thus the
> TTBR0 is not configured at all. On SoCFPGA, this produces all sorts of minor
> issues which are hard to replicate, for example certain USB sticks are not
> detected or QSPI NOR sometimes fails to write pages completely.
> 
> The solution is to replace CONFIG_ARMV7 test with CONFIG_CPU_V7 one. This is
> correct because the code which added the test(s) for CONFIG_ARMV7 was added
> shortly after CONFIG_ARMV7 was replaced by CONFIG_CPU_V7 and this code was
> not adjusted correctly to reflect that change.

Analysis of the series shows that:

- it does not change the values of DCACHE_OFF, DCACHE_WRITETHROUGH and
  DCACHE_WRITEBACK;

- it does change the value of DCACHE_WRITEALLOC from 0x16 to 0x101E, but
  DCACHE_WRITEALLOC is only used when CONFIG_SYS_ARM_CACHE_WRITEALLOC is
  defined, which does not happen throughout U-Boot, as shown by a search
  for "WRITEALLOC".

- it sets inner and outer region cache control bits in TTBR0, to match
  the cacheability of the DDR in which the MMU table resides.

Marek performed tests of patch 1/2 only (with the S bit set) and of
the whole series (wih the S bit clear). With patch 1/2 only, Marek could
witness the performance hit described by Stefan Roese; with the whole
series, Marek saw no performance hit any more.

I will therefore take this patch series in, since it fixes an obvious
issue in the U-Boot code and does not show any adverse effect.

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

Re: [U-Boot] ARMv7 MMU shareability issue

[Albert ARIBAUD]

Hello Pavel,

On Mon, 14 Dec 2015 08:48:16 +0100, Pavel Machek  wrote:
> Hi!
> 
> > This patch has several effects:
> > 
> > - it selects proper ARMv7 translation table level 1 bit definitions;
> > - it provides proper ARMv7 definitions for WT/WB/WA;
> > - it selects proper ARMv7 settings for TTBR0.
> > 
> > All these are correct as per the docs I have (although I may have missed
> > something during the readings (and cross-readings with Marek) of these
> > last hours/days.
> > 
> > Now, one specific effect goes against performance, and it is the
> > setting of bit S in all TT entries. This bit makes the corresponding
> > region shareable, but for all I know, in U-Boot we don't have more than
> > one core accessing the same memory or registers sets so -- at least for
> > the major part of its execution -- there is no reason for any region to
> > be shareable.
> 
> Well, I'm currently working on AMP patch, which will mean two
> processors at the same time in u-boot.

Will they share memory or will they use another mechanism for sync?

> Also... we provide memory modify operations for the user. User may
> be trying to communicate with second core.
> 
> > /That/ effect I certainly don't want.
> 
> How big is the slowdown from S bit?

A 4MB memory-to-memory transfer goes from instantaneous to 2-3 seconds;
Ethernet performance drops by 40%.

> Best regards,

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

Re: [U-Boot] ARMv7 MMU shareability issue

[Albert ARIBAUD]

Hello Marek,

On Thu, 10 Dec 2015 04:53:01 +0100, Marek Vasut <ma...@denx.de> wrote:
> On Thursday, December 10, 2015 at 03:27:08 AM, Tom Rini wrote:
> > On Wed, Dec 09, 2015 at 03:09:13PM +0100, Marek Vasut wrote:
> > > On Wednesday, December 09, 2015 at 02:02:03 PM, Stefan Roese wrote:
> > > > Hi All!
> > > > 
> > > > On 09.12.2015 09:09, Albert ARIBAUD wrote:
> > > > > On Tue,  8 Dec 2015 14:43:42 +0100, Marek Vasut <ma...@denx.de> wrote:
> > > > >> The arch/arm/lib/cache-cp15.c checks for CONFIG_ARMV7 and if this
> > > > >> macro is set, it configures TTBR0 register. This register must be
> > > > >> configured for the cache on ARMv7 to operate correctly.
> > > > >> 
> > > > >> The problem is that noone actually sets the CONFIG_ARMV7 macro and
> > > > >> thus the TTBR0 is not configured at all. On SoCFPGA, this produces
> > > > >> all sorts of minor issues which are hard to replicate, for example
> > > > >> certain USB sticks are not detected or QSPI NOR sometimes fails to
> > > > >> write pages completely.
> > > > >> 
> > > > >> The solution is to replace CONFIG_ARMV7 test with CONFIG_CPU_V7 one.
> > > > >> This is correct because the code which added the test(s) for
> > > > >> CONFIG_ARMV7 was added shortly after CONFIG_ARMV7 was replaced by
> > > > >> CONFIG_CPU_V7 and this code was not adjusted correctly to reflect
> > > > >> that change.
> > > > > 
> > > > > Note:
> > > > > 
> > > > > As discussed with Marek on IRC, this patch enables what is supposed
> > > > > to be the correct MMU settings for ARMv7, which causes a sharp
> > > > > Ethernet performance drop (40%) but also a strong general memory
> > > > > access performance hit (a copy of 4 MB is almost instantaneous
> > > > > without the patch and takes 2-3 seconds with it).
> > > > 
> > > > I've tested Marek's patch on my current Armada XP platform (also
> > > > ARMv7). And also noticed a performance drop by about 30-40%.
> > > > The dhrystone command can be used for this testing as well
> > > > (CONFIG_CMD_DHRYSTONE).
> > > > 
> > > > So this is definitely a NAK from me to this current patch.
> > > 
> > > This is a wrong approach, this patch just fixes another patch which was
> > > broken from the getgo and the TTBR0 reg was not programmed correctly due
> > > to that. This patch must be applied, but what we need to decide is
> > > whether we need _another_ patch which removes the S-bit from the
> > > pagetable or how to deal with that part.
> > 
> > No, we don't have to apply this patch.  The original patch here
> > (97840b5) was likely not run-time tested when submitted as it was using
> > the then-defunct CONFIG_ARMV7 symbol, and was likely a bugfix in an
> > internal tree that was pushed upstream (which is on the whole, good).
> 
> I find it surprising that such a patch, which modifies common code even,
> was not scrutinized enough.
> 
> > So in sum U-Boot has always been as broken in some specific regard
> > before that patch as it is after that patch.  So we have time to see
> > what we need to do about enabling this feature correctly and even
> > ensuring that we don't happen to say break things once Linux is up too.
> 
> In that case, I am looking forward to better suggestions.
> 
> I still disagree that this patch should not be applied, it corrects code
> which was broken. The slowdown caused by the original patch is a separate
> issue and should be corrected by a separate patch. If these two patches
> get applied at once, that is fine.

This patch has several effects:

- it selects proper ARMv7 translation table level 1 bit definitions;
- it provides proper ARMv7 definitions for WT/WB/WA;
- it selects proper ARMv7 settings for TTBR0.

All these are correct as per the docs I have (although I may have missed
something during the readings (and cross-readings with Marek) of these
last hours/days.

Now, one specific effect goes against performance, and it is the
setting of bit S in all TT entries. This bit makes the corresponding
region shareable, but for all I know, in U-Boot we don't have more than
one core accessing the same memory or registers sets so -- at least for
the major part of its execution -- there is no reason for any region to
be shareable.

/That/ effect I certainly don't want.

The rest I am fine with.

So, if we apply this patch minus the inclusion of the S bit in the
definition of DCACHE_OFF (and hence of all DCACHE_* enum members after
it), then we get code that is more correct from a theoretical
standpoint, and does not degrade performance (which is a hint,
admittedly weak, to me that it is not incorrect from a practical
standpoint.

It does not fix the USB and QSPI issues in the socfpga case, but I am
not sure these are caused by the S bit being zero.

> Best regards,
> Marek Vasut

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

[U-Boot] ARMv7 MMU shareability issue (was: [PATCH] arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7)

[Albert ARIBAUD]

Hello Marek,

On Tue,  8 Dec 2015 14:43:42 +0100, Marek Vasut  wrote:
> The arch/arm/lib/cache-cp15.c checks for CONFIG_ARMV7 and if this macro is
> set, it configures TTBR0 register. This register must be configured for the
> cache on ARMv7 to operate correctly.
> 
> The problem is that noone actually sets the CONFIG_ARMV7 macro and thus the
> TTBR0 is not configured at all. On SoCFPGA, this produces all sorts of minor
> issues which are hard to replicate, for example certain USB sticks are not
> detected or QSPI NOR sometimes fails to write pages completely.
> 
> The solution is to replace CONFIG_ARMV7 test with CONFIG_CPU_V7 one. This is
> correct because the code which added the test(s) for CONFIG_ARMV7 was added
> shortly after CONFIG_ARMV7 was replaced by CONFIG_CPU_V7 and this code was
> not adjusted correctly to reflect that change.

Note:

As discussed with Marek on IRC, this patch enables what is supposed to
be the correct MMU settings for ARMv7, which causes a sharp Ethernet
performance drop (40%) but also a strong general memory access
performance hit (a copy of 4 MB is almost instantaneous without the
patch and takes 2-3 seconds with it).

I would like to either fix the performance or come up with an
explanation for it before I pick this patch.

Marek's analysis shows the only MMU-related effect of the patch is that
the S bit gets set in first-level MMU table entries.

The S bit makes a mmu region shareable with other IPs within the
silicon (DMA engines, other cores, possibly even a piece of bus or
interconnect). For instance, it will cause memory writes within the
region to propagate to other IPs (which [must] also have defined this
region as shareable) so that these IPs know the region has been
written to and update their cache state accordingly.

With the S bit clear, USB and QSPI fail to work on Marek's board,
probably because writes do not propagate properly between the core and
these IPs; with the S bit set, USB and QSPI work but cache performance
is very reduced.

My hypothesis right now is that some other IP(s) hamper(s) the
propagation of shareable region writes; for instance, some IP is off or
in the wrong state, and does not properly respond, causing some stall.

At first I suspected the second core, which was off during the tests;
but Marek tried with that core on and it did not improve performance.

We can keep on looking into finding IPs that might affect shareable
accesses and try to turn them on or off more or less at random, but
that is time-consuming, and I'm not even sure I'm on the right track.

I will welcome suggestions if anyone with more experience in MMU
shareability than us -- or with brilliant insight :) -- has any.

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

Re: [U-Boot] [PATCH] arm, ubifs: fix gcc5.x compiler warning

[Albert ARIBAUD]

Hello Tom,

On Mon, 30 Nov 2015 11:28:53 -0500, Tom Rini  wrote:
> On Mon, Nov 30, 2015 at 11:03:53AM +0100, Heiko Schocher wrote:
> > Hello Jeroen,
> > 
> > Am 30.11.2015 um 10:20 schrieb Jeroen Hofstee:
> > >Hello Heiko,
> > >
> > >On 30-11-15 08:47, Heiko Schocher wrote:
> > >>compiling U-Boot for openrd_base_defconfig with
> > >>gcc 5.x shows the following warning:
> > >>
> > >>   CC  fs/ubifs/super.o
> > >>In file included from fs/ubifs/ubifs.h:35:0,
> > >>  from fs/ubifs/super.c:37:
> > >>fs/ubifs/super.c: In function 'atomic_inc':
> > >>./arch/arm/include/asm/atomic.h:55:2: warning: 'flags' is used 
> > >>uninitialized in this function
> > >>[-Wuninitialized]
> > >>   local_irq_save(flags);
> > >>   ^
> > >>fs/ubifs/super.c: In function 'atomic_dec':
> > >>./arch/arm/include/asm/atomic.h:64:2: warning: 'flags' is used 
> > >>uninitialized in this function
> > >>[-Wuninitialized]
> > >>   local_irq_save(flags);
> > >>   ^
> > >>   CC  fs/ubifs/sb.o
> > >>[...]
> > >>   CC  fs/ubifs/lpt.o
> > >>In file included from include/linux/bitops.h:123:0,
> > >>  from include/common.h:20,
> > >>  from include/ubi_uboot.h:17,
> > >>  from fs/ubifs/ubifs.h:37,
> > >>  from fs/ubifs/lpt.c:35:
> > >>fs/ubifs/lpt.c: In function 'test_and_set_bit':
> > >>./arch/arm/include/asm/bitops.h:57:2: warning: 'flags' is used 
> > >>uninitialized in this function
> > >>[-Wuninitialized]
> > >>   local_irq_save(flags);
> > >>   ^
> > >>   CC  fs/ubifs/lpt_commit.o
> > >>In file included from include/linux/bitops.h:123:0,
> > >>  from include/common.h:20,
> > >>  from include/ubi_uboot.h:17,
> > >>  from fs/ubifs/ubifs.h:37,
> > >>  from fs/ubifs/lpt_commit.c:26:
> > >>fs/ubifs/lpt_commit.c: In function 'test_and_set_bit':
> > >>./arch/arm/include/asm/bitops.h:57:2: warning: 'flags' is used 
> > >>uninitialized in this function
> > >>[-Wuninitialized]
> > >>   local_irq_save(flags);
> > >>   ^
> > >>   CC  fs/ubifs/scan.o
> > >>   CC  fs/ubifs/lprops.o
> > >>   CC  fs/ubifs/tnc.o
> > >>In file included from include/linux/bitops.h:123:0,
> > >>  from include/common.h:20,
> > >>  from include/ubi_uboot.h:17,
> > >>  from fs/ubifs/ubifs.h:37,
> > >>  from fs/ubifs/tnc.c:30:
> > >>fs/ubifs/tnc.c: In function 'test_and_set_bit':
> > >>./arch/arm/include/asm/bitops.h:57:2: warning: 'flags' is used 
> > >>uninitialized in this function
> > >>[-Wuninitialized]
> > >>   local_irq_save(flags);
> > >>   ^
> > >>   CC  fs/ubifs/tnc_misc.o
> > >>
> > >>Fix it.
> > >>
> > >>Signed-off-by: Heiko Schocher 
> > >>---
> > >>
> > >>  arch/arm/include/asm/atomic.h | 14 +++---
> > >>  arch/arm/include/asm/bitops.h |  4 ++--
> > >>  2 files changed, 9 insertions(+), 9 deletions(-)
> > >>
> > >>diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h
> > >>index 34c07fe..9b79506 100644
> > >>--- a/arch/arm/include/asm/atomic.h
> > >>+++ b/arch/arm/include/asm/atomic.h
> > >>@@ -32,7 +32,7 @@ typedef struct { volatile int counter; } atomic_t;
> > >>  static inline void atomic_add(int i, volatile atomic_t *v)
> > >>  {
> > >>-unsigned long flags;
> > >>+unsigned long flags = 0;
> > >>  local_irq_save(flags);
> > >>  v->counter += i;
> > >>@@ -41,7 +41,7 @@ static inline void atomic_add(int i, volatile atomic_t 
> > >>*v)
> > >
> > >Since flags is an "out" argument, something else must be wrong.
> > >There should be no need to initialize it, since local_irq_save should
> > >do that afaik.
> > 
> > yes, you are right, it should be, but gcc 5.x seems to have problems
> > with it ... compiled code size for the openrd_base config is same with
> > my patch ...
> > 
> > Hmm... for the openrd_base compile local_irq_save() is used from:
> > arch/arm/thumb1/include/asm/proc-armv/system.h
> > 
> > with:
> > static inline void local_irq_save(
> > unsigned long flags __attribute__((unused)))
> > {
> > __asm__ __volatile__ ("" : : : "memory");
> > }
> > 
> > flasg marked as unused ... seems correct to me, but I have
> > no idea, why gcc 5.x prints a warning ... any ideas?
> 
> Well, gcc does get more vigerous in its checking now and yeah, it feels
> like it's flagging false positives.   In this case I think the answer is
> that we need to nop out the various calls a bit harder on ARM.  Glancing
> at the kernel, I think for thumb1 we should just do what we do for
> non-thumb, or translate that into thumb1 only code.

Not sure I'm following what you mean, both about nop-ing out and about
thumb-1. Can you clarify?

> Tom

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

Re: [U-Boot] [RFC] Removal of superfluous gd assignments

[Albert ARIBAUD]

Hello Stefan,

On Thu, 26 Nov 2015 07:23:51 +0100, Stefan Roese
<stefan.ro...@gmail.com> wrote:
> Hi Albert,
> 
> first, thank you very much for taking the time to go over this
> in such depth.
> 
> On 25.11.2015 16:20, Albert ARIBAUD wrote:
> > Hello all,
> >
> > This is a follow-up to discussions on the IRC chan re: the fact that gd
> > (the global data pointer) is being assigned in various places, which is
> > not too much of a good thing.
> >
> > In all architectures, common/init/board_init.c is built in, which
> > provides board_init_f_{mem,init_reserve}, in which gd is assigned by
> > calling arch_setup_gd().
> >
> > (two architectures, ARM and x86, cannot assign gd from C code, so they
> > don't call arch_setup_gd() but assign GD from another single location)
> >
> > However, there are several places apart from arch_setup_gd() where gd is
> > assigned to. Superfluous assignments lead, at best, to confusion, and
> > at worst, to subtle as well as not-so-subtle bugs, so we need to find
> > and remove such superfluous assignments.
> >
> > This post gives a list of places where gd is being assigned (courtesy
> > of a git grep '^\s\+gd\s*=\s*.*$' command) and suggestions re removal.
> >
> > Comments welcome, of course.
> >
> > ---
> >
> > arch/arm/cpu/arm926ejs/mxs/spl_boot.c:126:  gd = 
> >
> > This assignment is in mxs_common_spl_init() which is called
> > from a board_init_ll() defined by various boards, and itself is
> > called from arch/arm/cpu/arm926ejs/mxs/start.S. Setting gs is
> > required because mxs_common_spl_init() calls lots of functions
> > which depend on gd.
> >
> > Removal: requires MXS SPL to move over to using crt0.S.
> >
> > Note: since MXS SPL actually returns to ROM to get U-Boot
> > launched, it may require cooperation from crt0.S for saving
> > important registers at reset entry point and restoring them and
> > returning to ROM. See OMAP (for register saving) and FEL.
> >
> > Boards affected are:
> >
> > mx28evk_nand xfi3 bg0900 sansa_fuze_plus mx23evk m28evk
> > sc_sps_1 mx28evk_spi axm mx28evk apx4devkit mx23_olinuxino
> > firefly-rk3288 x600 mx28evk_auart_console taurus
> 
> This list of affected boards does not seem to be correct. I fail to
> see, how firefly-rk3288 or x600 are affected by this "mxs" file.

Correct -- I'd patched arch/arm/cpu/arm926ejs/mxs/spl_boot.c with
a #error directive then blindly listed all boards given by buildman -s
which included boards not building cleanly even for other reasons that
the one I was looking for.

> > arch/arm/lib/spl.c:40:  gd = 
> >
> > This assignment is in a weak board_init_f(), therefore executed
> > from crt0.S (no ARM board calls board_init_f() from elsewhere);
> > therefore gd is already set when this assignment is reached.
> >
> > Removal: unconditional.
> 
> I'm wondering, if this weak default board_init_f() function is
> called for any ARM platform at all? If not, it would be clearer
> to remove this function completely.

I'll try and find out if the weak board_init_f is used at all, and if
it is not, I'll amend the suggestion to 'removal of the whole function'

> Thanks,

Thanks for your review!

> Stefan

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

[U-Boot] [RFC] Removal of superfluous gd assignments

[Albert ARIBAUD]

Hello all,

This is a follow-up to discussions on the IRC chan re: the fact that gd
(the global data pointer) is being assigned in various places, which is
not too much of a good thing.

In all architectures, common/init/board_init.c is built in, which
provides board_init_f_{mem,init_reserve}, in which gd is assigned by
calling arch_setup_gd().

(two architectures, ARM and x86, cannot assign gd from C code, so they
don't call arch_setup_gd() but assign GD from another single location)

However, there are several places apart from arch_setup_gd() where gd is
assigned to. Superfluous assignments lead, at best, to confusion, and
at worst, to subtle as well as not-so-subtle bugs, so we need to find
and remove such superfluous assignments.

This post gives a list of places where gd is being assigned (courtesy
of a git grep '^\s\+gd\s*=\s*.*$' command) and suggestions re removal.

Comments welcome, of course.

---

arch/arm/cpu/arm926ejs/mxs/spl_boot.c:126:  gd = 

This assignment is in mxs_common_spl_init() which is called
from a board_init_ll() defined by various boards, and itself is
called from arch/arm/cpu/arm926ejs/mxs/start.S. Setting gs is
required because mxs_common_spl_init() calls lots of functions
which depend on gd.

Removal: requires MXS SPL to move over to using crt0.S.

Note: since MXS SPL actually returns to ROM to get U-Boot
launched, it may require cooperation from crt0.S for saving
important registers at reset entry point and restoring them and
returning to ROM. See OMAP (for register saving) and FEL.

Boards affected are:

mx28evk_nand xfi3 bg0900 sansa_fuze_plus mx23evk m28evk
sc_sps_1 mx28evk_spi axm mx28evk apx4devkit mx23_olinuxino
firefly-rk3288 x600 mx28evk_auart_console taurus

arch/arm/cpu/armv8/fsl-layerscape/spl.c:48: gd = 

Performed in board_init_f() by boards:

ls1043ardb_nand ls2085ardb_nand ls1043ardb_sdcard
ls2085aqds_nand

These four boards build SPL using arch/arm/lib/crt0_64.S which
unconditionally executes arch_set_gd() and thereby has gd
properly set when arch_set_ board_init_f() is entered, so that
assignment is redundant.

Removal: unconditional.

arch/arm/lib/spl.c:40:  gd = 

This assignment is in a weak board_init_f(), therefore executed
from crt0.S (no ARM board calls board_init_f() from elsewhere);
therefore gd is already set when this assignment is reached.

Removal: unconditional.

arch/arm/mach-exynos/spl_boot.c:279:gd = gdp;

This assignment occurs from withing a machine-specific
board_init_f(), therefore executed from crt0.S (no ARM board
calls board_init_f() from elsewhere); therefore gd is already
set when this assignment is reached. 

Removal: unconditional.

(non-ARM architectures skipped over)

(non-ARM boards skipped over)

common/board_f.c:982:   gd = 

Only m68k, avr32 and nds32 use this. It could be conditioned to
these architectures and later removed if and when they switch
to the same boot sequence as ARM et al.

Removal: only if and when arch switches to a boot sequence
similar to ARM's.

common/board_r.c:942:   gd = new_gd;

Done by arches other than ARM, AARCH64 and X86. For ARM, gd is
switched from early to final location in crt0. For x86? In any
case, this assignment cannot be removed unless all arches have
switched to a boot sequence similar to ARM's.

Removal: only if and when arch switches to a boot sequence
similar to ARM's.

common/init/board_init.c:28:gd = gd_ptr;

That is the only assignment we should keep, for architectures
other than ARM or x86.

Removal: no.

common/spl/spl.c:450:   gd = new_gd;

This is the second place where gd assignment should be kept,
and for ARM it must be fixed the same way arch_setup_gd was,
since this assignment to gd may not work with ARM gcc (this
change will go in v8 of my patch)

Removal: no.

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

[U-Boot] [PATCH v7 1/2] Fix board init code to respect the C runtime environment

[Albert ARIBAUD]

board_init_f_mem() alters the C runtime environment's
stack it is actually already using. This is not a valid
behaviour within a C runtime environment.

Split board_init_f_mem into C functions which do not alter
their own stack and always behave properly with respect to
their C runtime environment.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---
Copying custodians for all architectures which this
patch affects.

This patch has been build-tested for all affected
architectures except NIOS2, and run-tested on ARM
OpenRD Client.

For NIOS2, this patch contains all manual v1 and v2
fixes by Thomas.

For x86, this patch contains all manual v2 fixes by Bin.

Changes in v7: None
Changes in v6:
- Switch from size- to address-based reservation
- Add comments on gdp_tr vs gd use wrt arch_setup_gd.
- Clarify that this is about the *early* malloc arena

Changes in v5:
- Reword commit message (including summary/subject)
- Reword comments in ARC code

Changes in v4:
- Add comments on reserving GD at the lowest location
- Add comments on post-incrementing base for each "chunk"

Changes in v3:
- Rebase after commit 9ac4fc82
- Simplify malloc conditional as per commit 9ac4fc82
- Fix NIOS2 return value register (r2, not r4)
- Fix x86 subl argument inversion
- Group allocations in a single function
- Group initializations in a single function

Changes in v2:
- Fix all checkpatch issues
- Fix board_init_f_malloc prototype mismatch
- Fix board_init_[f_]xxx typo in NIOS2
- Fix aarch64 asm 'sub' syntax error

 arch/arc/lib/start.S|  12 +++--
 arch/arm/lib/crt0.S |   3 +-
 arch/arm/lib/crt0_64.S  |   4 +-
 arch/microblaze/cpu/start.S |   4 +-
 arch/nios2/cpu/start.S  |  14 --
 arch/powerpc/cpu/ppc4xx/start.S |   6 ++-
 arch/x86/cpu/start.S|   3 +-
 arch/x86/lib/fsp/fsp_common.c   |   4 +-
 common/init/board_init.c| 109 
 include/common.h|  34 ++---
 10 files changed, 144 insertions(+), 49 deletions(-)

diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
index 26a5934..90ee7e0 100644
--- a/arch/arc/lib/start.S
+++ b/arch/arc/lib/start.S
@@ -50,18 +50,20 @@ ENTRY(_start)
 1:
 #endif
 
-   /* Setup stack- and frame-pointers */
+   /* Establish C runtime stack and frame */
mov %sp, CONFIG_SYS_INIT_SP_ADDR
mov %fp, %sp
 
-   /* Allocate and zero GD, update SP */
+   /* Allocate reserved area from current top of stack */
mov %r0, %sp
-   bl  board_init_f_mem
-
-   /* Update stack- and frame-pointers */
+   bl  board_init_f_alloc_reserve
+   /* Set stack below reserved area, adjust frame pointer accordingly */
mov %sp, %r0
mov %fp, %sp
 
+   /* Initialize reserved area - note: r0 already contains address */
+   bl  board_init_f_init_reserve
+
/* Zero the one and only argument of "board_init_f" */
mov_s   %r0, 0
j   board_init_f
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 80548eb..4f2a712 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -83,8 +83,9 @@ ENTRY(_main)
bic sp, sp, #7  /* 8-byte alignment for ABI compliance */
 #endif
mov r0, sp
-   bl  board_init_f_mem
+   bl  board_init_f_alloc_reserve
mov sp, r0
+   bl  board_init_f_init_reserve
 
mov r0, #0
bl  board_init_f
diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
index cef1c71..b4fc760 100644
--- a/arch/arm/lib/crt0_64.S
+++ b/arch/arm/lib/crt0_64.S
@@ -75,8 +75,10 @@ ENTRY(_main)
ldr x0, =(CONFIG_SYS_INIT_SP_ADDR)
 #endif
bic sp, x0, #0xf/* 16-byte alignment for ABI compliance */
-   bl  board_init_f_mem
+   mov x0, sp
+   bl  board_init_f_alloc_reserve
mov sp, x0
+   bl  board_init_f_init_reserve
 
mov x0, #0
bl  board_init_f
diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
index 14f46a8..206be3e 100644
--- a/arch/microblaze/cpu/start.S
+++ b/arch/microblaze/cpu/start.S
@@ -25,7 +25,7 @@ _start:
 
addir8, r0, __end
mts rslr, r8
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 #if defined(CONFIG_SPL_BUILD)
addir1, r0, CONFIG_SPL_STACK_ADDR
mts rshr, r1
@@ -142,7 +142,7 @@ _start:
ori r12, r12, 0x1a0
mts rmsr, r12
 
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 clear_bss:
/* clear BSS segments */
addir5, r0, __bss_start
diff --git a/arch/nios2/cpu/start.S b/arch/nios2/cpu/start.S
index 54787c5..204d0cd 100644
--- a/arch/nios2/cpu/start.S
+++ b/arch/nios2/cpu/start.S
@@ -106,14 +106

[U-Boot] [PATCH v7 2/2] arm: move gd handling outside of C code

[Albert ARIBAUD]

As of gcc 5.2.1 for Thumb-1, it is not possible any
more to assign gd from C code, as gd is mapped to r9,
and r9 may now be saved in the prolog sequence, and
restored in the epilog sequence, of any C functions.

Therefore arch_setup_gd(), which is supposed to set
r9, may actually have no effect, causing U-Boot to
use a bad address to access GD.

Fix this by never calling arch_setup_gd() for ARM,
and instead setting r9 in arch/arm/lib/crt0.S, to
the value returned by board_init_f_alloc_reserve().

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---

Changes in v7:
- also fix common/spl/spl.c assignment to gd

Changes in v6:
- moved ARM r9 fix into its own patch

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/lib/crt0.S  |  3 +++
 common/init/board_init.c |  8 
 common/spl/spl.c | 26 +++---
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 4f2a712..2f4c14e 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -85,6 +85,8 @@ ENTRY(_main)
mov r0, sp
bl  board_init_f_alloc_reserve
mov sp, r0
+   /* set up gd here, outside any C code */
+   mov r9, r0
bl  board_init_f_init_reserve
 
mov r0, #0
@@ -134,6 +136,7 @@ here:
bl  spl_relocate_stack_gd
cmp r0, #0
movne   sp, r0
+   movne   r9, r0
 # endif
ldr r0, =__bss_start/* this is auto-relocated! */
 
diff --git a/common/init/board_init.c b/common/init/board_init.c
index e649e07..d98648e 100644
--- a/common/init/board_init.c
+++ b/common/init/board_init.c
@@ -21,13 +21,13 @@ DECLARE_GLOBAL_DATA_PTR;
 #define _USE_MEMCPY
 #endif
 
-/* Unfortunately x86 can't compile this code as gd cannot be assigned */
-#ifndef CONFIG_X86
+/* Unfortunately x86 or ARM can't compile this code as gd cannot be assigned */
+#if !defined(CONFIG_X86) && !defined(CONFIG_ARM)
 __weak void arch_setup_gd(struct global_data *gd_ptr)
 {
gd = gd_ptr;
 }
-#endif /* !CONFIG_X86 */
+#endif /* !CONFIG_X86 && !CONFIG_ARM */
 
 /*
  * Allocate reserved space for use as 'globals' from 'top' address and
@@ -128,7 +128,7 @@ void board_init_f_init_reserve(ulong base)
*ptr++ = 0;
 #endif
/* set GD unless architecture did it already */
-#ifndef CONFIG_X86
+#if !defined(CONFIG_X86) && !defined(CONFIG_ARM)
arch_setup_gd(gd_ptr);
 #endif
/* next alloc will be higher by one GD plus 16-byte alignment */
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 7a393dc..61d0786 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -431,8 +431,13 @@ void preloader_console_init(void)
  * more stack space for things like the MMC sub-system.
  *
  * This function calculates the stack position, copies the global_data into
- * place and returns the new stack position. The caller is responsible for
- * setting up the sp register.
+ * place, sets the new gd (except for ARM, for which setting GD within a C
+ * function may not always work) and returns the new stack position. The
+ * caller is responsible for setting up the sp register and, in the case
+ * of ARM, setting up gd.
+ *
+ * All of this is done using the same layout and alignments as done in
+ * board_init_f_init_reserve() / board_init_f_alloc_reserve().
  *
  * @return new stack location, or 0 to use the same stack
  */
@@ -440,14 +445,7 @@ ulong spl_relocate_stack_gd(void)
 {
 #ifdef CONFIG_SPL_STACK_R
gd_t *new_gd;
-   ulong ptr;
-
-   /* Get stack position: use 8-byte alignment for ABI compliance */
-   ptr = CONFIG_SPL_STACK_R_ADDR - sizeof(gd_t);
-   ptr &= ~7;
-   new_gd = (gd_t *)ptr;
-   memcpy(new_gd, (void *)gd, sizeof(gd_t));
-   gd = new_gd;
+   ulong ptr = CONFIG_SPL_STACK_R_ADDR;
 
 #ifdef CONFIG_SPL_SYS_MALLOC_SIMPLE
if (CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN) {
@@ -460,7 +458,13 @@ ulong spl_relocate_stack_gd(void)
gd->malloc_ptr = 0;
}
 #endif
-
+   /* Get stack position: use 8-byte alignment for ABI compliance */
+   ptr = CONFIG_SPL_STACK_R_ADDR - roundup(sizeof(gd_t),16);
+   new_gd = (gd_t *)ptr;
+   memcpy(new_gd, (void *)gd, sizeof(gd_t));
+#if !defined(CONFIG_ARM)
+   gd = new_gd;
+#endif
return ptr;
 #else
return 0;
-- 
2.5.0

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

[U-Boot] [PATCH v6 1/2] Fix board init code to respect the C runtime environment

[Albert ARIBAUD]

board_init_f_mem() alters the C runtime environment's
stack it is actually already using. This is not a valid
behaviour within a C runtime environment.

Split board_init_f_mem into C functions which do not alter
their own stack and always behave properly with respect to
their C runtime environment.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---
Copying custodians for all architectures which this
patch affects.

This patch has been build-tested for all affected
architectures except NIOS2, and run-tested on ARM
OpenRD Client.

For NIOS2, this patch contains all manual v1 and v2
fixes by Thomas.

For x86, this patch contains all manual v2 fixes by Bin.

Changes in v6:
- Switch from size- to address-based reservation
- Add comments on gdp_tr vs gd use wrt arch_setup_gd.
- Clarify that this is about the *early* malloc arena

Changes in v5:
- Reword commit message (including summary/subject)
- Reword comments in ARC code

Changes in v4:
- Add comments on reserving GD at the lowest location
- Add comments on post-incrementing base for each "chunk"

Changes in v3:
- Rebase after commit 9ac4fc82
- Simplify malloc conditional as per commit 9ac4fc82
- Fix NIOS2 return value register (r2, not r4)
- Fix x86 subl argument inversion
- Group allocations in a single function
- Group initializations in a single function

Changes in v2:
- Fix all checkpatch issues
- Fix board_init_f_malloc prototype mismatch
- Fix board_init_[f_]xxx typo in NIOS2
- Fix aarch64 asm 'sub' syntax error

 arch/arc/lib/start.S|  12 +++--
 arch/arm/lib/crt0.S |   3 +-
 arch/arm/lib/crt0_64.S  |   4 +-
 arch/microblaze/cpu/start.S |   4 +-
 arch/nios2/cpu/start.S  |  14 --
 arch/powerpc/cpu/ppc4xx/start.S |   6 ++-
 arch/x86/cpu/start.S|   3 +-
 arch/x86/lib/fsp/fsp_common.c   |   4 +-
 common/init/board_init.c| 109 
 include/common.h|  34 ++---
 10 files changed, 144 insertions(+), 49 deletions(-)

diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
index 26a5934..90ee7e0 100644
--- a/arch/arc/lib/start.S
+++ b/arch/arc/lib/start.S
@@ -50,18 +50,20 @@ ENTRY(_start)
 1:
 #endif
 
-   /* Setup stack- and frame-pointers */
+   /* Establish C runtime stack and frame */
mov %sp, CONFIG_SYS_INIT_SP_ADDR
mov %fp, %sp
 
-   /* Allocate and zero GD, update SP */
+   /* Allocate reserved area from current top of stack */
mov %r0, %sp
-   bl  board_init_f_mem
-
-   /* Update stack- and frame-pointers */
+   bl  board_init_f_alloc_reserve
+   /* Set stack below reserved area, adjust frame pointer accordingly */
mov %sp, %r0
mov %fp, %sp
 
+   /* Initialize reserved area - note: r0 already contains address */
+   bl  board_init_f_init_reserve
+
/* Zero the one and only argument of "board_init_f" */
mov_s   %r0, 0
j   board_init_f
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 80548eb..4f2a712 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -83,8 +83,9 @@ ENTRY(_main)
bic sp, sp, #7  /* 8-byte alignment for ABI compliance */
 #endif
mov r0, sp
-   bl  board_init_f_mem
+   bl  board_init_f_alloc_reserve
mov sp, r0
+   bl  board_init_f_init_reserve
 
mov r0, #0
bl  board_init_f
diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
index cef1c71..b4fc760 100644
--- a/arch/arm/lib/crt0_64.S
+++ b/arch/arm/lib/crt0_64.S
@@ -75,8 +75,10 @@ ENTRY(_main)
ldr x0, =(CONFIG_SYS_INIT_SP_ADDR)
 #endif
bic sp, x0, #0xf/* 16-byte alignment for ABI compliance */
-   bl  board_init_f_mem
+   mov x0, sp
+   bl  board_init_f_alloc_reserve
mov sp, x0
+   bl  board_init_f_init_reserve
 
mov x0, #0
bl  board_init_f
diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
index 14f46a8..206be3e 100644
--- a/arch/microblaze/cpu/start.S
+++ b/arch/microblaze/cpu/start.S
@@ -25,7 +25,7 @@ _start:
 
addir8, r0, __end
mts rslr, r8
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 #if defined(CONFIG_SPL_BUILD)
addir1, r0, CONFIG_SPL_STACK_ADDR
mts rshr, r1
@@ -142,7 +142,7 @@ _start:
ori r12, r12, 0x1a0
mts rmsr, r12
 
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 clear_bss:
/* clear BSS segments */
addir5, r0, __bss_start
diff --git a/arch/nios2/cpu/start.S b/arch/nios2/cpu/start.S
index 54787c5..204d0cd 100644
--- a/arch/nios2/cpu/start.S
+++ b/arch/nios2/cpu/start.S
@@ -106,14 +106,18 @@ _reloc:
stw

[U-Boot] [PATCH v6 2/2] arm: move gd handling outside of C code

[Albert ARIBAUD]

As of gcc 5.2.1 for Thumb-1, it is not possible any
more to assign gd from C code, as gd is mapped to r9,
and r9 may now be saved in the prolog sequence, and
restored in the epilog sequence, of any C functions.

Therefore arch_setup_gd(), which is supposed to set
r9, may actually have no effect, causing U-Boot to
use a bad address to access GD.

Fix this by never calling arch_setup_gd() for ARM,
and instead setting r9 in arch/arm/lib/crt0.S, to
the value returned by board_init_f_alloc_reserve().

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---

Changes in v6:
- moved ARM r9 fix into its own patch

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/lib/crt0.S  | 2 ++
 common/init/board_init.c | 8 
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 4f2a712..9c34f2c 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -85,6 +85,8 @@ ENTRY(_main)
mov r0, sp
bl  board_init_f_alloc_reserve
mov sp, r0
+   /* set up gd here, outside any C code */
+   mov r9, r0
bl  board_init_f_init_reserve
 
mov r0, #0
diff --git a/common/init/board_init.c b/common/init/board_init.c
index e649e07..d98648e 100644
--- a/common/init/board_init.c
+++ b/common/init/board_init.c
@@ -21,13 +21,13 @@ DECLARE_GLOBAL_DATA_PTR;
 #define _USE_MEMCPY
 #endif
 
-/* Unfortunately x86 can't compile this code as gd cannot be assigned */
-#ifndef CONFIG_X86
+/* Unfortunately x86 or ARM can't compile this code as gd cannot be assigned */
+#if !defined(CONFIG_X86) && !defined(CONFIG_ARM)
 __weak void arch_setup_gd(struct global_data *gd_ptr)
 {
gd = gd_ptr;
 }
-#endif /* !CONFIG_X86 */
+#endif /* !CONFIG_X86 && !CONFIG_ARM */
 
 /*
  * Allocate reserved space for use as 'globals' from 'top' address and
@@ -128,7 +128,7 @@ void board_init_f_init_reserve(ulong base)
*ptr++ = 0;
 #endif
/* set GD unless architecture did it already */
-#ifndef CONFIG_X86
+#if !defined(CONFIG_X86) && !defined(CONFIG_ARM)
arch_setup_gd(gd_ptr);
 #endif
/* next alloc will be higher by one GD plus 16-byte alignment */
-- 
2.5.0

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

Re: [U-Boot] [PATCH v5] Fix board init code to respect the C runtime environment

[Albert ARIBAUD]

Hello Simon,

On Wed, 18 Nov 2015 18:05:07 -0700, Simon Glass 
wrote:
> Hi Albert,
> 
> >> early malloc() area
> >
> > "Arena", and "malloc arena", are established designations for the
> > memory space used by malloc implementations; and I prefer to use this
> > more specific term, as people may use it as a search keyword when
> > looking for malloc related stuff.
> 
> Arena is OK, but can you please mention 'early' each time? It's
> confusing otherwise. I think we should have a clear distinction
> between the early malloc() and full malloc().

Good point, will do.

> If you can have it so that the stack top equals the global_data
> bottom, then we should be OK.

Will do that in v6.

> > Note, however, that it will not simplify assembly code: it will turn a
> > subtraction from sp into an assignment to sp, which is not simpler, and
> > it will add an assignment to whatever register represents the first
> > argument, since we'll turn a (void) function into a (ulong top)
> > function, so all in all, it will add one assembly instruction with
> > respect to the 'ulong board_init_f_get_reserve_size(void)' approach.
> 
> True, but now we are just passing values around rather than doing
> arithmetic in assembler.

Well, at the C level an addition may be more complex than an
assignment, but at the assembly level, that's pretty much equally
simple. Anyway, the change is ok by me.

> > gd works at this point, and I want to avoid any aliasing issue.
> 
> I don't really understand that, but if you want to use gd I think it
> would be worth a one-line comment.

I believe it is already present in v5:

+ * ALSO IMPORTANT:
+ *
+ * On some architectures, gd will only be set once arch_setup_gd() is
+ * called. Therefore:
+ *
+ * Do not use 'gd->' until arch_setup_gd() has been called!

I will expand that comment a bit, and add a one-liner in the code too.

Thanks again for your comments!

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

Re: [U-Boot] [PATCH v5] Fix board init code to respect the C runtime environment

[Albert ARIBAUD]

... and of course, I forgot to add a note at '(*)' re the malloc arena
size alignment...

On Tue, 17 Nov 2015 13:59:32 +0100, Albert ARIBAUD
<albert.u.b...@aribaud.net> wrote:

> Hello Simon,
> Indeed, no attempt is made to check that it is aligned (and no attempt
> is made in the original code either) -- all the more since there is no
> strict definition of what it should be aligned to. There is, actually,
> no indication that it should be aligned, although it will probably only
> be used up until the last full 4-byte-word (or 8-byte word for 64-bit
> architectures). In fact, the alignment of CONFIG_SYS_MALLOC_F_LEN does
> not matter much, see (*) below.

> (*)

And, in fact, there is not real need for CONFIG_SYS_MALLOC_F_LEN to be
aligned, whether is is reserved above or below GD.

If the malloc arena is reserved above GD (my preferred scenario, as
explained elsewhere), then what counts is aligning its *base*, not
*size*, in the probable case that it should support word accesses and
some architecture may require word alignment.

If the malloc arena is reserved below GD, then what counts is aligning
*GD's base*, not the malloc arena's size.

(that's a general principle, BTW: each 'chunk' allocation should decrease
the current top address by the chunk size requested then round it further
down by the alignment constraint applicable to that chunk. The chunk size
does not matter much.)

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

Re: [U-Boot] [PATCH v5] Fix board init code to respect the C runtime environment

[Albert ARIBAUD]

Hello Simon,

On Mon, 16 Nov 2015 21:11:38 -0700, Simon Glass 
wrote:

> > +++ b/arch/arm/lib/crt0.S
> > @@ -82,9 +82,11 @@ ENTRY(_main)
> >  #else
> > bic sp, sp, #7  /* 8-byte alignment for ABI compliance */
> >  #endif
> > +   bl  board_init_f_get_reserve_size
> > +   sub sp, sp, r0
> > +   mov r9, sp
> 
> Why do you need that?

To set gd in ARM architecture, as arch_setup_gd() may not work for
ARM, see my 'Note about the ARM case' in my answer dated Sun, 15 Nov
2015 14:51:04 +0100 for details:

https://www.mail-archive.com/u-boot@lists.denx.de/msg192494.html

I shall post a 2-patch v6 with this fix standalone as 1/2 and the rest
of the changes as 2/2.

> > +++ b/common/init/board_init.c
> > @@ -21,39 +21,121 @@ DECLARE_GLOBAL_DATA_PTR;
> >  #define _USE_MEMCPY
> >  #endif
> >
> > -/* Unfortunately x86 can't compile this code as gd cannot be assigned */
> > -#ifndef CONFIG_X86
> > +/* Unfortunately x86 or ARM can't compile this code as gd cannot be 
> > assigned */
> > +#if !defined(CONFIG_X86) && !defined(CONFIG_ARM)
> >  __weak void arch_setup_gd(struct global_data *gd_ptr)
> >  {
> > gd = gd_ptr;
> >  }
> > -#endif /* !CONFIG_X86 */
> > +#endif /* !CONFIG_X86 && !CONFIG_SYS_THUMB_BUILD */
> >
> > -ulong board_init_f_mem(ulong top)
> > +/*
> > + * Compute size of space to reserve on stack for use as 'globals'
> > + * and return size request for reserved area.
> > + *
> > + * Notes:
> > + *
> > + * Actual reservation cannot be done from within this function as
> > + * it requires altering the C stack pointer, so this will be done by
> > + * the caller upon return from this function.
> > + *
> > + * IMPORTANT:
> > + *
> > + * The return value of this function has two uses:
> > + * - it determines the amount of stack reserved for global data and
> > + *   the malloc arena;
> 
> early malloc() area

"Arena", and "malloc arena", are established designations for the
memory space used by malloc implementations; and I prefer to use this
more specific term, as people may use it as a search keyword when
looking for malloc related stuff.

> Another option would be to pass the address and have this function
> return the new address. That would simplify the assembly code slightly
> for all archs I think. It would also allow you to align the address
> for sure, whereas at present it only works if the original address was
> aligned, and CONFIG_SYS_MALLOC_F_LEN is aligned.

Good point, with a few caveats though.

Regarding alignment of CONFIG_SYS_MALLOC_F_LEN:

Indeed, no attempt is made to check that it is aligned (and no attempt
is made in the original code either) -- all the more since there is no
strict definition of what it should be aligned to. There is, actually,
no indication that it should be aligned, although it will probably only
be used up until the last full 4-byte-word (or 8-byte word for 64-bit
architectures). In fact, the alignment of CONFIG_SYS_MALLOC_F_LEN does
not matter much, see (*) below.

Regarding alignment of the original/'top' address:

This top address is the SP for all architectures, and at least for ARM,
it is aligned to an 8-byte boundary, as this is an ARM architecture
EABI requirement. For other architectures, I cannot claim it is, but I
suspect all initial values of SP, which generally are CONFIG_SPL_STACK
or CONFIG_SYS_INIT_SP_ADDR, are (sufficiently) aligned.

And if CONFIG_SPL_STACK and CONFIG_SYS_INIT_SP_ADDR are not
(sufficiently) aligned in their definition, then either we can fix
these definitions (and that of CONFIG_SYS_MALLOC_F_LEN too, while we
are at it), or, if we can only fix this at run time, then this should be
done where the stack pointer is altered, in the crt0.S file or whatever
its equivalent is for any given architecture, outside the C environment.

But that will have to go in another patch dealing with alignment.

(*)

Indeed board_init_f_get_reserve_size may have to respect some location
or size alignment for each of the chunks it reserves. Right now, for
instance, GD is aligned to a 16-byte boundary, and the malloc arena is
aligned by virtue of the rounded value of CONFIG_SYS_MALLOC_F_LEN.

And yes, should some new reservation be made beside GD and the malloc
arena, it might require some specific alignment not dealt with so far;
for instance, we may need to reserve some 4KB-aligned chunk for memory
management purposes, or whatever, and there is no guarantee that the
original stack is 4KB-aligned.

In that light, ulong board_init_f_get_reserve_size(void) does not
permit controlled alignment, whereas ulong board_init_f_reserve(ulong
top) does, since we can round 'top' down to any value we like.

Note, however, that it will not simplify assembly code: it will turn a
subtraction from sp into an assignment to sp, which is not simpler, and
it will add an assignment to whatever register represents the first
argument, since we'll turn a (void) function into a (ulong top)
function, so all in all, it will 

[U-Boot] [PATCH v5] Fix board init code to respect the C runtime environment

[Albert ARIBAUD]

board_init_f_mem() alters the C runtime environment's
stack it is actually already using. This is not a valid
behaviour within a C runtime environment.

Split board_init_f_mem into C functions which do not alter
their own stack and always behave properly with respect to
their C runtime environment.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---
Cc:ing custodians for all architectures which this
patch affects.

This patch has been build-tested for all there
architectures, and run-tested on ARM OpenRD Client.

For NIOS2, this patch contains all manual v1 and v2
fixes by Thomas.

For x86, this patch contains all manual v2 fixes by Bin.

Changes in v5:
- Reword commit message (including summary/subject)
- Reword comments in ARC code

Changes in v4:
- Add comments on reserving GD at the lowest location
- Add comments on post-incrementing base for each "chunk"

Changes in v3:
- Rebase after commit 9ac4fc82
- Simplify malloc conditional as per commit 9ac4fc82
- Fix NIOS2 return value register (r2, not r4)
- Fix x86 subl argument inversion
- Group allocations in a single function
- Group initializations in a single function

Changes in v2:
- Fix all checkpatch issues
- Fix board_init_f_malloc prototype mismatch
- Fix board_init_[f_]xxx typo in NIOS2
- Fix aarch64 asm 'sub' syntax error

 arch/arc/lib/start.S|  16 +++---
 arch/arm/lib/crt0.S |   6 ++-
 arch/arm/lib/crt0_64.S  |   6 ++-
 arch/microblaze/cpu/start.S |   4 +-
 arch/nios2/cpu/start.S  |  13 +++--
 arch/powerpc/cpu/ppc4xx/start.S |  10 ++--
 arch/x86/cpu/start.S|   5 +-
 arch/x86/lib/fsp/fsp_common.c   |   4 +-
 common/init/board_init.c| 108 +++-
 include/common.h|  33 +---
 10 files changed, 146 insertions(+), 59 deletions(-)

diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
index 26a5934..fddd536 100644
--- a/arch/arc/lib/start.S
+++ b/arch/arc/lib/start.S
@@ -50,18 +50,20 @@ ENTRY(_start)
 1:
 #endif
 
-   /* Setup stack- and frame-pointers */
+   /* Establish C runtime stack and frame */
mov %sp, CONFIG_SYS_INIT_SP_ADDR
mov %fp, %sp
 
-   /* Allocate and zero GD, update SP */
-   mov %r0, %sp
-   bl  board_init_f_mem
-
-   /* Update stack- and frame-pointers */
-   mov %sp, %r0
+   /* Get reserved area size */
+   bl  board_init_f_get_reserve_size
+   /* Allocate reserved size on stack, adjust frame pointer accordingly */
+   sub %sp, %sp, %r0
mov %fp, %sp
 
+   /* Initialize reserved area */
+   mov %r0, %sp
+   bl  board_init_f_init_reserve
+
/* Zero the one and only argument of "board_init_f" */
mov_s   %r0, 0
j   board_init_f
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 80548eb..4ec89a4 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -82,9 +82,11 @@ ENTRY(_main)
 #else
bic sp, sp, #7  /* 8-byte alignment for ABI compliance */
 #endif
+   bl  board_init_f_get_reserve_size
+   sub sp, sp, r0
+   mov r9, sp
mov r0, sp
-   bl  board_init_f_mem
-   mov sp, r0
+   bl  board_init_f_init_reserve
 
mov r0, #0
bl  board_init_f
diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
index cef1c71..2891071 100644
--- a/arch/arm/lib/crt0_64.S
+++ b/arch/arm/lib/crt0_64.S
@@ -75,8 +75,10 @@ ENTRY(_main)
ldr x0, =(CONFIG_SYS_INIT_SP_ADDR)
 #endif
bic sp, x0, #0xf/* 16-byte alignment for ABI compliance */
-   bl  board_init_f_mem
-   mov sp, x0
+   bl  board_init_f_get_reserve_size
+   sub sp, sp, x0
+   mov x0, sp
+   bl  board_init_f_init_reserve
 
mov x0, #0
bl  board_init_f
diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
index 14f46a8..206be3e 100644
--- a/arch/microblaze/cpu/start.S
+++ b/arch/microblaze/cpu/start.S
@@ -25,7 +25,7 @@ _start:
 
addir8, r0, __end
mts rslr, r8
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 #if defined(CONFIG_SPL_BUILD)
addir1, r0, CONFIG_SPL_STACK_ADDR
mts rshr, r1
@@ -142,7 +142,7 @@ _start:
ori r12, r12, 0x1a0
mts rmsr, r12
 
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 clear_bss:
/* clear BSS segments */
addir5, r0, __bss_start
diff --git a/arch/nios2/cpu/start.S b/arch/nios2/cpu/start.S
index 54787c5..45bf0fd 100644
--- a/arch/nios2/cpu/start.S
+++ b/arch/nios2/cpu/start.S
@@ -106,14 +106,17 @@ _reloc:
stw r0, 4(sp)
mov fp, sp
 
-   /* Allocate and zero GD, update SP */
+   

Re: [U-Boot] U-Boot as a next bootloader in a chain - reading the kernel boot args provided by the previous stage bootlader?

[Albert ARIBAUD]

Hello Wojciech,

On Mon, 16 Nov 2015 13:09:43 +0100, Wojciech Zabolotny
<wza...@gmail.com> wrote:
> 2015-11-16 11:52 GMT+01:00 Albert ARIBAUD <albert.u.b...@aribaud.net>:
> > Hello Wojciech,
> >
> > On Mon, 16 Nov 2015 10:42:50 +0100, Wojciech Zabolotny
> > <wza...@gmail.com> wrote:
> >> Hi,
> >>
> >> I need to use U-Boot (or barebox) as a bootloader in a Raspberry Pi
> >> based system to extend booting flexibility.
> >> The problem however is that certain kernel boot arguments are prepared
> >> by the previous stage bootloader (start.elf)
> >> basing on the properties of the individual board.
> >> For example in one of the boards I use, the kernel command line when a
> >> standard kernel is booted, looks as follows (MAC address and serial
> >> number are partially masked for privacy):
> >>
> >> dma.dmachans=0x7f35 bcm2708_fb.fbwidth=720 bcm2708_fb.fbheight=480
> >> bcm2708.boardrev=0xd bcm2708.serial=0x6f15
> >> smsc95xx.macaddr=B8:27:EB:XX:XX:XX bcm2708_fb.fbswap=1
> >> sdhci-bcm2708.emmc_clock_freq=25000 vc_mem.mem_base=0x1ec0
> >> vc_mem.mem_size=0x2000  console=ttyAMA0 root=/dev/mmcblk0p2
> >> rootwait
> >>
> >> Only the "console=ttyAMA0 root=/dev/mmcblk0p2 rootwait" is provided by
> >> the user (in the cmdline.txt file). The rest is generated by the
> >> start.elf.
> >>
> >> Of course when I boot the u-boot.bin instead of zImage, the same
> >> parameters are passed to it, but the U-Boot is not able to read them
> >> and to pass them to the finally booted kernel.
> >> As U-Boot shares a lot of code with the Linux kernel it should be
> >> possible to add necessary functions.
> >> It could be useful in all applications where U-Boot is used as an
> >> additional stage in the boot chain e.g., to add new booting
> >> functionalities (booting from the network, booting from the flash disk
> >> etc.).
> >>
> >> I have found a nice description, how the paremeters are passed in ARM
> >> architecture:
> >> http://www.simtec.co.uk/products/SWLINUX/files/booting%5Farticle.html
> >> but of course the solution should be probably portable (or implemented
> >> for each platform independently with unified API).
> >
> > Not sure what kind of answer you're asking for here. Do you want to
> > know if what you're suggesting is feasible? Are you looking for someone
> > to implement it? Are you going to implement it yourself but asking for
> > feedback?
> >
> >> --
> >
> > (aside: if the above should be a signature delimiter, then it lacks a
> > space after the dashes)
> >
> > Amicalement,
> > --
> > Albert.
> 
> Dear Albert,
> 
> Yes the first question is if this feature is feasible.

In software, just about anything is feasible; ask any PHB. :)

Specifically, catching information passed to U-Boot's entry point is
something that e.g. OMAPs do already. It is probably not going to be
portable, though, because the passing method is inherently specific to
your platform and pre-U-Boot loader.

> If yes, then I'd like to propose such functionality as a possible improvement.
> I think that may be more people interested in such a feature.
>
> I'll appreciate any sugestions/pointers regarding the possible implementation.
> 
> Probably I can try to implement it. Of course if there are other
> interested users
> who can do it, I'll be definitely happy to discuss that with them.

Best is that you post a patch with a working implementation which can
be build above current u-boot/master branch; people interested in it
will comment.

> Regards,

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

Re: [U-Boot] [PATCH v4] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Alexey,

On Mon, 16 Nov 2015 13:12:15 +, Alexey Brodkin
<alexey.brod...@synopsys.com> wrote:
> Hi Albert,
> 
> On Sun, 2015-11-15 at 19:25 +0100, Albert ARIBAUD wrote:
> > board_init_f_mem() alters the C runtime environment's
> > stack it ls actually already using. This is not a valid
> > C runtime environment.
> > 
> > Split board_init_f_mem into C functions which do not
> > alter their own stack and therefore run in a valid C
> > runtime environment.
> > 
> > Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
> > ---
> > Cc:ing custodians for all architectures which this
> > patch affects.
> > 
> > This patch has been build-tested for all there
> > architectures, and run-tested on ARM OpenRD Client.
> > 
> > For NIOS2, this patch contains all manual v1 and v2
> > fixes by Thomas.
> > 
> > For x86, this patch contains all manual v2 fixes by Bin.
> > 
> > Changes in v4:
> > - Add comments on reserving GD at the lowest location
> > - Add comments on post-incrementing base for each "chunk"
> > 
> > Changes in v3:
> > - Rebase after commit 9ac4fc82
> > - Simplify malloc conditional as per commit 9ac4fc82
> > - Fix NIOS2 return value register (r2, not r4)
> > - Fix x86 subl argument inversion
> > - Group allocations in a single function
> > - Group initializations in a single function
> > 
> > Changes in v2:
> > - Fix all checkpatch issues
> > - Fix board_init_f_malloc prototype mismatch
> > - Fix board_init_[f_]xxx typo in NIOS2
> > - Fix aarch64 asm 'sub' syntax error
> > 
> >  arch/arc/lib/start.S|  12 +++--
> >  arch/arm/lib/crt0.S |   6 ++-
> >  arch/arm/lib/crt0_64.S  |   6 ++-
> >  arch/microblaze/cpu/start.S |   4 +-
> >  arch/nios2/cpu/start.S  |  13 +++--
> >  arch/powerpc/cpu/ppc4xx/start.S |  10 ++--
> >  arch/x86/cpu/start.S|   5 +-
> >  arch/x86/lib/fsp/fsp_common.c   |   4 +-
> >  common/init/board_init.c| 108 
> > +++-
> >  include/common.h|  33 +---
> >  10 files changed, 144 insertions(+), 57 deletions(-)
> > 
> > diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
> > index 26a5934..2a99193 100644
> > --- a/arch/arc/lib/start.S
> > +++ b/arch/arc/lib/start.S
> > @@ -54,14 +54,16 @@ ENTRY(_start)
> > mov %sp, CONFIG_SYS_INIT_SP_ADDR
> > mov %fp, %sp
> >  
> > -   /* Allocate and zero GD, update SP */
> > -   mov %r0, %sp
> > -   bl  board_init_f_mem
> > -
> > +   /* Get reserved area size, update SP and FP */
> > +   bl  board_init_f_get_reserve_size
> > /* Update stack- and frame-pointers */
> 
> I think we don't need to mention SP/FP update in comments twice here.
> I.e. either strip ", update SP and FP" from your introduced comment or
> which I really like more remove following line with comment entirely:
> -->8--
>   /* Update stack- and frame-pointers */
> -->8--

Not sure where you see two SP+FP 'update' comments here; probably
you're referring to the 'setup' comment on line 53 and the 'update'
one on line 59. If that is what you meant, I tink these comments are
different and deserve staying both...

... However, these comments also pretty much just paraphrase the code
which follows them and thus serve little purpose; they could be
reworded to show less of what is being done and more of why it is being
done:

- the "Update stack- and frame-pointer" comment could be turned into
  "Allocate reserved size on stack and adjust frame pointer
  accordingly", and

- the "Setup stack- and frame-pointers" comment could be turned into
  "Establish C runtime stack and frame".

Opinions?

> Not sure if this tiny nitpick worth respinning thought.

That's fine, since I've got the commit message to fix too.

> Otherwise build and run tested on free nSIM,
> see "Running U-Boot on ARC in Free nSIM" section in
> http://www.denx.de/wiki/U-Boot/ARCNotes if of any interest how to do it
> yourself.

Thanks, I'll try this, and I4ll certainly keep it in my bookmarks.

> Feel free to add
> -->8--
> Acked-by: Alexey Brodkin <abrod...@synopsys.com>
> -->8--
> 
> -Alexey

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

Re: [U-Boot] [PATCH v4] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Alexey,

On Mon, 16 Nov 2015 13:43:19 +, Alexey Brodkin
<alexey.brod...@synopsys.com> wrote:
> Hi Albert,
> 
> On Mon, 2015-11-16 at 14:34 +0100, Albert ARIBAUD wrote:
> > Hello Alexey,
> > 
> > On Mon, 16 Nov 2015 13:12:15 +, Alexey Brodkin
> > <alexey.brod...@synopsys.com> wrote:
> > > Hi Albert,
> 
> > > >  
> > > > -   /* Allocate and zero GD, update SP */
> > > > -   mov %r0, %sp
> > > > -   bl  board_init_f_mem
> > > > -
> > > > +   /* Get reserved area size, update SP and FP */
> > > > +   bl  board_init_f_get_reserve_size
> > > > /* Update stack- and frame-pointers */
> > > 
> > > I think we don't need to mention SP/FP update in comments twice here.
> > > I.e. either strip ", update SP and FP" from your introduced comment or
> > > which I really like more remove following line with comment entirely:
> > > -->8--
> > >   /* Update stack- and frame-pointers */
> > > -->8--
> > 
> > Not sure where you see two SP+FP 'update' comments here; probably
> > you're referring to the 'setup' comment on line 53 and the 'update'
> > one on line 59. If that is what you meant, I tink these comments are
> > different and deserve staying both...
> 
> Ok, that's what I have after your patch application:
> 
> -->8--
>   /* Setup stack- and frame-pointers */
>   mov %sp, CONFIG_SYS_INIT_SP_ADDR
>   mov %fp, %sp
> 
>   /* Get reserved area size, update SP and FP */
>   bl  board_init_f_get_reserve_size
>   /* Update stack- and frame-pointers */  <-- that's already mentioned 2 
> lines above
>   sub %sp, %sp, %r0
>   mov %fp, %sp
> -->8--

My bad, I'd missed that one. I'll turn

/* Get reserved area size, update SP and FP */

Into

/* Get reserved area size */

> > ... However, these comments also pretty much just paraphrase the code
> > which follows them and thus serve little purpose; they could be
> > reworded to show less of what is being done and more of why it is being
> > done:
> > 
> > - the "Update stack- and frame-pointer" comment could be turned into
> >   "Allocate reserved size on stack and adjust frame pointer
> >   accordingly", and
> > 
> > - the "Setup stack- and frame-pointers" comment could be turned into
> >   "Establish C runtime stack and frame".
> > 
> > Opinions?
> 
> Totally agree, care to implement it?

That, and the removal of the repetition. v5 in approach.

> -Alexey

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

Re: [U-Boot] U-Boot as a next bootloader in a chain - reading the kernel boot args provided by the previous stage bootlader?

[Albert ARIBAUD]

Hello Wojciech,

On Mon, 16 Nov 2015 10:42:50 +0100, Wojciech Zabolotny
 wrote:
> Hi,
> 
> I need to use U-Boot (or barebox) as a bootloader in a Raspberry Pi
> based system to extend booting flexibility.
> The problem however is that certain kernel boot arguments are prepared
> by the previous stage bootloader (start.elf)
> basing on the properties of the individual board.
> For example in one of the boards I use, the kernel command line when a
> standard kernel is booted, looks as follows (MAC address and serial
> number are partially masked for privacy):
> 
> dma.dmachans=0x7f35 bcm2708_fb.fbwidth=720 bcm2708_fb.fbheight=480
> bcm2708.boardrev=0xd bcm2708.serial=0x6f15
> smsc95xx.macaddr=B8:27:EB:XX:XX:XX bcm2708_fb.fbswap=1
> sdhci-bcm2708.emmc_clock_freq=25000 vc_mem.mem_base=0x1ec0
> vc_mem.mem_size=0x2000  console=ttyAMA0 root=/dev/mmcblk0p2
> rootwait
> 
> Only the "console=ttyAMA0 root=/dev/mmcblk0p2 rootwait" is provided by
> the user (in the cmdline.txt file). The rest is generated by the
> start.elf.
> 
> Of course when I boot the u-boot.bin instead of zImage, the same
> parameters are passed to it, but the U-Boot is not able to read them
> and to pass them to the finally booted kernel.
> As U-Boot shares a lot of code with the Linux kernel it should be
> possible to add necessary functions.
> It could be useful in all applications where U-Boot is used as an
> additional stage in the boot chain e.g., to add new booting
> functionalities (booting from the network, booting from the flash disk
> etc.).
> 
> I have found a nice description, how the paremeters are passed in ARM
> architecture:
> http://www.simtec.co.uk/products/SWLINUX/files/booting%5Farticle.html
> but of course the solution should be probably portable (or implemented
> for each platform independently with unified API).

Not sure what kind of answer you're asking for here. Do you want to
know if what you're suggesting is feasible? Are you looking for someone
to implement it? Are you going to implement it yourself but asking for
feedback?

> --

(aside: if the above should be a signature delimiter, then it lacks a
space after the dashes)

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

Re: [U-Boot] [PATCH v3] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Simon,

On Sat, 14 Nov 2015 08:41:01 -0700, Simon Glass 
wrote:

> Reading between the lines, there isn't actually a problem with this in
> practice, but it is doing things that C should not really be doing. Is
> that right?

You did not even need to read between the lines :) as the very first
comment to the v1 of this patch was an explicit question whether I had
seen actual failure cause by board_init_f_mem() -- and my explicit
answer was that no, I hadn't, but as an analogy, I hadn't personally
seen any car crash caused by worn tires either, and that did not deter
me from changing worn tires as soon as I noticed them rather than wait
for the tires to fail.

> To me it seems OK provided we can assume that the stack
> needed to call the function is less than 64 bytes. But I'm sure there
> is a spec somewhere that the current function violates. Your idea of
> what constitutes the stack seems slightly different to mine - I think
> of it as the data already written, whereas you think of it as data yet
> to write, There is some associated uncertainty since the compiler is
> free to fiddle with the stack pointer. In practice in a boot loader we
> always make some architectural assumptions based on current operation.
> But I agree we should minimise this.

Ok.

> Anyway, I think your solution only adds one more function and still
> has most of the code in C rather than assembler, so that it is common
> to all archs. That was my main goal with the change.

Just to clarify, keeping as much C as possible is my goal too.
Especially, the patch keeps the size computation in a C function rather
than reverting back to computing it in crt0.S as it was 'in olden
days'; that is because computing the size in a C function is much more
flexible than it would be in assembly language, and accessible to many
more developers.

My general view of assembly vs C language is that assembly language
should only be used to perform things that cannot be done in C language
without violating the C runtime environment, or for which a C runtime
environment is not available, or which may fail depending on the
compiler implementation. This essentially covers:

- the entry point: when it is directly pointed at by the reset vector,
  it obviously does not have a C runtime environment, and it usually
  does not have one either even when it it entered from ROM code. In
  fact, the general goal of the entry point is to set up the C runtime
  environment(s).

- exception vectors: except for rare architectures for which the
  compiler provides a way to directly write interrupt routines in C,
  exception vectors have to be written in assembly language; again,
  their role *is* to set up the C runtime environment for the C
  language exception handler.

- exceptional cases where any implementation of some design in C
  conflicts with the compiler implementation. Of course, these case are
  specific to an architecture and frequently even a compiler version
  (see below for the r9-as-GD issue) and must only resort to assembly
  language for said architecture, and use as little of it as possible.
  Typically, such designs are related to an architecture's system ABI,
  which a compiler does not necessarily handle -- again, r9 as a system
  wide constant register comes to the mind of the ARM custodian -- or
  the need for specific instructions for which the compiler does not
  provide support in C (mcr and mrc are ARM examples).

Also, while the patch does add one C function, it does not change the C
code functionally overall, and adds very little assembly code, thereby
resulting in a negligible global code increase (actually, on ARM, I see
more boards experiencing a slight global code size *decrease* than boards
experiencing an *increase*).

> I prefer to set up gd in the C function if we can, to avoid the
> oddness that gd has to be at the bottom and the associated 16-byte
> alignment issue. But let's see how that goes. If we need to do it in
> assembler, it might be better to require the caller to do it before
> calling board_init_f_reserve().

Right now I only see two architectures which cannot set GD in
arch_setup_gd(): x86 and ARM; my patch takes care not to affect other
arches, nor do I have an intent to do so. They can set GD from C, and
that's fine.

Note about the ARM case:

Regarding ARM, I do change the behaviour for all of ARM, whereas the
issue of arch_setup_gd() failing to set r9 only happens for Thumb-1 code
generation and only with gcc 5.2.1 (and possibly a few other versions).

This is because, as I said elsewhere, I fear that the gcc -ffixed-reg
behaviour change is compliant with a strict interpretation of the
option (all accesses to r9 from user C code will return the same
value) and therefore I expect later gcc versions to not change the
Thumb-1 behaviour; it might even happen that this behaviour change be
'ported over' to Thumb-2 or even ARM code generation.

As the ARM custodian, I do not view tracking gcc 

[U-Boot] [PATCH v4] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

board_init_f_mem() alters the C runtime environment's
stack it ls actually already using. This is not a valid
C runtime environment.

Split board_init_f_mem into C functions which do not
alter their own stack and therefore run in a valid C
runtime environment.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---
Cc:ing custodians for all architectures which this
patch affects.

This patch has been build-tested for all there
architectures, and run-tested on ARM OpenRD Client.

For NIOS2, this patch contains all manual v1 and v2
fixes by Thomas.

For x86, this patch contains all manual v2 fixes by Bin.

Changes in v4:
- Add comments on reserving GD at the lowest location
- Add comments on post-incrementing base for each "chunk"

Changes in v3:
- Rebase after commit 9ac4fc82
- Simplify malloc conditional as per commit 9ac4fc82
- Fix NIOS2 return value register (r2, not r4)
- Fix x86 subl argument inversion
- Group allocations in a single function
- Group initializations in a single function

Changes in v2:
- Fix all checkpatch issues
- Fix board_init_f_malloc prototype mismatch
- Fix board_init_[f_]xxx typo in NIOS2
- Fix aarch64 asm 'sub' syntax error

 arch/arc/lib/start.S|  12 +++--
 arch/arm/lib/crt0.S |   6 ++-
 arch/arm/lib/crt0_64.S  |   6 ++-
 arch/microblaze/cpu/start.S |   4 +-
 arch/nios2/cpu/start.S  |  13 +++--
 arch/powerpc/cpu/ppc4xx/start.S |  10 ++--
 arch/x86/cpu/start.S|   5 +-
 arch/x86/lib/fsp/fsp_common.c   |   4 +-
 common/init/board_init.c| 108 +++-
 include/common.h|  33 +---
 10 files changed, 144 insertions(+), 57 deletions(-)

diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
index 26a5934..2a99193 100644
--- a/arch/arc/lib/start.S
+++ b/arch/arc/lib/start.S
@@ -54,14 +54,16 @@ ENTRY(_start)
mov %sp, CONFIG_SYS_INIT_SP_ADDR
mov %fp, %sp
 
-   /* Allocate and zero GD, update SP */
-   mov %r0, %sp
-   bl  board_init_f_mem
-
+   /* Get reserved area size, update SP and FP */
+   bl  board_init_f_get_reserve_size
/* Update stack- and frame-pointers */
-   mov %sp, %r0
+   sub %sp, %sp, %r0
mov %fp, %sp
 
+   /* Initialize reserved area */
+   mov %r0, %sp
+   bl  board_init_f_init_reserve
+
/* Zero the one and only argument of "board_init_f" */
mov_s   %r0, 0
j   board_init_f
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 80548eb..4ec89a4 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -82,9 +82,11 @@ ENTRY(_main)
 #else
bic sp, sp, #7  /* 8-byte alignment for ABI compliance */
 #endif
+   bl  board_init_f_get_reserve_size
+   sub sp, sp, r0
+   mov r9, sp
mov r0, sp
-   bl  board_init_f_mem
-   mov sp, r0
+   bl  board_init_f_init_reserve
 
mov r0, #0
bl  board_init_f
diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
index cef1c71..2891071 100644
--- a/arch/arm/lib/crt0_64.S
+++ b/arch/arm/lib/crt0_64.S
@@ -75,8 +75,10 @@ ENTRY(_main)
ldr x0, =(CONFIG_SYS_INIT_SP_ADDR)
 #endif
bic sp, x0, #0xf/* 16-byte alignment for ABI compliance */
-   bl  board_init_f_mem
-   mov sp, x0
+   bl  board_init_f_get_reserve_size
+   sub sp, sp, x0
+   mov x0, sp
+   bl  board_init_f_init_reserve
 
mov x0, #0
bl  board_init_f
diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
index 14f46a8..206be3e 100644
--- a/arch/microblaze/cpu/start.S
+++ b/arch/microblaze/cpu/start.S
@@ -25,7 +25,7 @@ _start:
 
addir8, r0, __end
mts rslr, r8
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 #if defined(CONFIG_SPL_BUILD)
addir1, r0, CONFIG_SPL_STACK_ADDR
mts rshr, r1
@@ -142,7 +142,7 @@ _start:
ori r12, r12, 0x1a0
mts rmsr, r12
 
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 clear_bss:
/* clear BSS segments */
addir5, r0, __bss_start
diff --git a/arch/nios2/cpu/start.S b/arch/nios2/cpu/start.S
index 54787c5..45bf0fd 100644
--- a/arch/nios2/cpu/start.S
+++ b/arch/nios2/cpu/start.S
@@ -106,14 +106,17 @@ _reloc:
stw r0, 4(sp)
mov fp, sp
 
-   /* Allocate and zero GD, update SP */
+   /* Allocate and initialize reserved area, update SP */
+   movhi   r2, %hi(board_init_f_get_reserve_size@h)
+   ori r2, r2, %lo(board_init_f_get_reserve_size@h)
+   callr   r2
+   sub sp, sp, r2
mov r4, sp
-   movhi   r2, %hi(board_init_f_mem@h)
-   ori r2, 

Re: [U-Boot] [PATCH v4] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Albert,

On Sun, 15 Nov 2015 19:25:25 +0100, Albert ARIBAUD
<albert.u.b...@aribaud.net> wrote:
> board_init_f_mem() alters the C runtime environment's
> stack it ls actually already using. This is not a valid
> C runtime environment.
> 
> Split board_init_f_mem into C functions which do not
> alter their own stack and therefore run in a valid C
> runtime environment.

Of course, I only see the typos above now: that paragraph
should read:

board_init_f_mem() alters the C runtime environment's
stack it is actually already using. This is not a valid
behaviour within a C runtime environment.

Split board_init_f_mem into C functions which do not alter
their own stack and always behave properly with respect to
their C runtime environment.

If no comment calls for a v5, then whoever applies this v4 please fix
the commit message accordingly.

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

Re: [U-Boot] [PATCH v3] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Simon,

On Fri, 13 Nov 2015 19:29:21 -0700, Simon Glass <s...@chromium.org>
wrote:
> Hi Albert,
> 
> On 13 November 2015 at 07:43, Albert ARIBAUD <albert.u.b...@aribaud.net> 
> wrote:
> > board_init_f_mem() alters the C runtime environment's
> > stack it ls actually already using. This is not a valid
> > C runtime environment.
> 
> I think it is doing something like alloca() and reserving space on the
> stack.

AFAICT, board_init_f_mem does *not* "do something like alloca()",
it does something that alloca() precisely does *not* do.

What alloca() does is, it creates a local variable on the stack, only
the size of that local is determined at run time; but that is still a
*local* variable, that will be freed like all locals when the current
function returns. The location of the alloca reservation is determined
by the compiler, which knows how which way the stack works and how much
of it is being used by the calling function already; and the resulting
pointer cannot be accessed beyond the size that was allocated.

What board_init_f_mem does is *completely* different: it wild-guesses
how much of the stack it is using, it assumes which way the stack grows,
and it purposefully accesses space that it was not given access to, and
it expects that allocation to survive upon returning to the calling
context.

Now, if you look at alloca's definition in glibc, you'll see it is
defined to be a gcc built-in, not a C function. There is a reason for
this: stack allocation within a C function requires the compiler's help,
because the compiler must be made aware of the alloca use in order to
properly free the alloca()ed space on every exit point in the function.
Only a built-in can get that level of compiler support.

board_init_f_mem's case is worse yet, as, contrary to alloca() which
only expects /local/ effects, board_init_f_mem expects the allocated
space to survive the return to the calling context. Doing this would
require *more* compiler help than alloca does, because it is not more
a matter of managing the stack pointer; it is a matter of moving
things around so that the allocated space ends up *above* the call
frame for board_init_f.

> It does not actually change the stack pointer in C code.

Non, it does not indeed; it could not, in fact, do this at all without
resorting to the same technique that is resorted to for implementing
alloca(), that is, it would have to be a built-in of gcc, as alloca()
is -- and, as I have just explained, it would require extensive help
from the compiler, as all compiler-generated epilog code would have to
account for that potential allocation.

> It changes data that is below the stack pointer, and therefore by
> definition not part of the stack, I think.

It changes data that it /assumes/ is below the stack pointer and that it
/assumes/ it can write to; but these two assumptions are unfounded, see
below.

> What actually goes wrong here? I read your info in the other thread
> but I'm afraid I still don't understand it.

At least three things are formally wrong here with board_init_f_mem:

- it assumes the stack grows downward.

- it assumes it never uses more than 0x40 bytes of the stack.

- it assumes it can write to the stack beyond what it was allocated for
  local variables, temporaries and arguments.

Each of these assumptions is unfounded in code which is supposed to be
architecture-agnostic. Not all stacks grow downward. There is no way to
ensure that only 0x40 bytes are used by board_init_f_mem. There is no
guarantee that a C function can write to its own stack beyond what it
was allocated -- in fact, there is no guarantee that a C function can
write to its stack anywhere else than its declared or alloca()ed
locals. Such writes are undefined behavior.

These assumptions are just as bad as assuming any pointer-to-long value
is valid when we could ensure it is; and we would fix such a situation,
would we not?

Same here. This patch ensures that the code in board_init_f_mem becomes
compliant with C runtime environment constraints. This requires
splitting the C code in two functions so that the first one does not
access its stack (except the allowed accesses generated by the
compiler) and the second accesses an area which is in fact not the stack
any more, thus is safe to access.

> Regards,
> Simon

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

Re: [U-Boot] [PATCH v3] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Thomas,

On Sat, 14 Nov 2015 14:23:31 +0800, Thomas Chou <tho...@wytron.com.tw>
wrote:
> Hi Albert,
> 
> On 2015年11月13日 22:43, Albert ARIBAUD wrote:
> > +/*
> > + * Initialize reserved space (which has been safely allocated on the C
> > + * stack from the C runtime environment handling code).
> > + *
> > + * Do not use 'gd->' until arch_setup_gd() has been called!
> > + */
> > +
> > +void board_init_f_init_reserve(ulong base)
> >   {
> > struct global_data *gd_ptr;
> >   #ifndef _USE_MEMCPY
> > int *ptr;
> >   #endif
> >
> > -   /* Leave space for the stack we are running with now */
> > -   top -= 0x40;
> > +   /*
> > +* clear GD entirely
> > +*/
> >
> > -   top -= sizeof(struct global_data);
> > -   top = ALIGN(top, 16);
> > -   gd_ptr = (struct global_data *)top;
> > +   gd_ptr = (struct global_data *)base;
> > +   /* go down one GD plus 16-byte alignment */
> > +   base += roundup(sizeof(struct global_data), 16);
> 
> Maybe it can keep the original order, top--gd--malloc--base.

Actually, I inverted the two between v2 and v3 for a reason, but I
should have made it more explicit.

This reason is, for architectures which may not be able to write to
gd from arch_setup_gd(), the C runtime environment handling code (crt0.S
for ARM, etc) needs a way to easily find our where gd_ptr was allocated.

So, it was either:

- keeping *gd_ptr above the malloc arena and having to pass gd_ptr back
  to board_init_f_reserve_size's caller; but board_init_f_reserve_size
  already returns a value back to its caller, and returning two values
  from a C function takes resources (a pointer argument and a memory
  location at least);

or:
 
- putting *gd_ptr at the base of the allocated space, below the malloc
  arena, so that the C runtime environment handling code knows where to
  point gd to without incurring the cost of passing additional results
  back from board_init_f_reserve_init.

This is the reason why I placed the global data below the malloc arena.

[besides, considering that our malloc implementation starts from the
base of the arena and allocates upward, it is (admittedly very slightly)
safer to have GD placed below it than above.]

I will add a note in v4 about the reason for this (re)ordering of
allocations.

> > +   base += CONFIG_SYS_MALLOC_F_LEN;
> 
> Useless statement.

Right now it is indeed useless. However, if and when some other space
gets reserved and initialized besides GD and the malloc arena, this
statement will be needed before allocating / accessing the space
allocated above the malloc arena.

Note that this statement will not result in any useless *code*, as the
compiler's optimizer detects that base remains unused after it, and
thus removes it from the generated object code.

So I'll insist on keeping this statement, although I will add a comment
next to it explaining why it matters despite not having any visible
effect.

> Best regards,
> Thomas

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

Re: [U-Boot] [PATCH] ARM: crt0: Pass malloc base address

[Albert ARIBAUD]

Hello Simon,

On Fri, 13 Nov 2015 19:12:13 -0700, Simon Glass 
wrote:

> > IMO, keeping it simple does not play well with making DM and malloc
> > available pre-SDRAM init :) --- but don't miscontrue my meaning: I'm
> > fine with DM and malloc pre-SDRAM on platforms which have enough IRAM /
> > SRAM / lockable cache / whatever for it. My point is just that sooner
> > or later, someone will start wanting not to do again after SDRAM init
> > what they've already done before it, because it avoids double HW init
> > issues, becaue it saves time, etc. And that's going to be way sooner
> > than later, IMO.
> 
> Yes that is quite possibility true. I'm happy to review any patches if
> it helps. My comment about it possibly not being worthwhile solving is
> just that. Things have a way of changing, so what looks unnecessary
> now might look essential tomorrow. In a way, the simple malloc() fits
> into that category.

And so evolve the uses of DM, of the DT, etc.

Alright, so I will post my RFC and read your comments with much
interest!

> Regards,
> Simon

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

[U-Boot] [PATCH v3] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

board_init_f_mem() alters the C runtime environment's
stack it ls actually already using. This is not a valid
C runtime environment.

Split board_init_f_mem into C functions which do not
alter their own stack and therefore run in a valid C
runtime environment.

Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net>
---
For NIOS2, this patch contains all manual v1 and v2
fixes by Thomas.

For x86, this patch contains all manual v2 fixes by Bin.

Changes in v3:
- Rebase after commit 9ac4fc82
- Simplify malloc conditional as per commit 9ac4fc82
- Fix NIOS2 return value register (r2, not r4)
- Fix x86 subl argument inversion
- Group allocations in a single function
- Group initializations in a single function

Changes in v2:
- Fix all checkpatch issues
- Fix board_init_f_malloc prototype mismatch
- Fix board_init_[f_]xxx typo in NIOS2
- Fix aarch64 asm 'sub' syntax error

 arch/arc/lib/start.S| 12 +---
 arch/arm/lib/crt0.S |  6 ++--
 arch/arm/lib/crt0_64.S  |  6 ++--
 arch/microblaze/cpu/start.S |  4 +--
 arch/nios2/cpu/start.S  | 13 +---
 arch/powerpc/cpu/ppc4xx/start.S | 14 ++---
 arch/x86/cpu/start.S|  7 +++--
 arch/x86/lib/fsp/fsp_common.c   |  4 +--
 common/init/board_init.c| 67 +
 include/common.h| 33 
 10 files changed, 108 insertions(+), 58 deletions(-)

diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
index 26a5934..2a99193 100644
--- a/arch/arc/lib/start.S
+++ b/arch/arc/lib/start.S
@@ -54,14 +54,16 @@ ENTRY(_start)
mov %sp, CONFIG_SYS_INIT_SP_ADDR
mov %fp, %sp
 
-   /* Allocate and zero GD, update SP */
-   mov %r0, %sp
-   bl  board_init_f_mem
-
+   /* Get reserved area size, update SP and FP */
+   bl  board_init_f_get_reserve_size
/* Update stack- and frame-pointers */
-   mov %sp, %r0
+   sub %sp, %sp, %r0
mov %fp, %sp
 
+   /* Initialize reserved area */
+   mov %r0, %sp
+   bl  board_init_f_init_reserve
+
/* Zero the one and only argument of "board_init_f" */
mov_s   %r0, 0
j   board_init_f
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 80548eb..4ec89a4 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -82,9 +82,11 @@ ENTRY(_main)
 #else
bic sp, sp, #7  /* 8-byte alignment for ABI compliance */
 #endif
+   bl  board_init_f_get_reserve_size
+   sub sp, sp, r0
+   mov r9, sp
mov r0, sp
-   bl  board_init_f_mem
-   mov sp, r0
+   bl  board_init_f_init_reserve
 
mov r0, #0
bl  board_init_f
diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
index cef1c71..d0f8db8 100644
--- a/arch/arm/lib/crt0_64.S
+++ b/arch/arm/lib/crt0_64.S
@@ -75,8 +75,10 @@ ENTRY(_main)
ldr x0, =(CONFIG_SYS_INIT_SP_ADDR)
 #endif
bic sp, x0, #0xf/* 16-byte alignment for ABI compliance */
-   bl  board_init_f_mem
-   mov sp, x0
+   bl  board_init_f_reserve_size
+   sub sp, sp, x0
+   mov x0, sp
+   bl  board_init_f_init_reserve
 
mov x0, #0
bl  board_init_f
diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
index 14f46a8..206be3e 100644
--- a/arch/microblaze/cpu/start.S
+++ b/arch/microblaze/cpu/start.S
@@ -25,7 +25,7 @@ _start:
 
addir8, r0, __end
mts rslr, r8
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 #if defined(CONFIG_SPL_BUILD)
addir1, r0, CONFIG_SPL_STACK_ADDR
mts rshr, r1
@@ -142,7 +142,7 @@ _start:
ori r12, r12, 0x1a0
mts rmsr, r12
 
-   /* TODO: Redo this code to call board_init_f_mem() */
+   /* TODO: Redo this code to call board_init_f_*() */
 clear_bss:
/* clear BSS segments */
addir5, r0, __bss_start
diff --git a/arch/nios2/cpu/start.S b/arch/nios2/cpu/start.S
index 54787c5..c95f1f4 100644
--- a/arch/nios2/cpu/start.S
+++ b/arch/nios2/cpu/start.S
@@ -106,14 +106,17 @@ _reloc:
stw r0, 4(sp)
mov fp, sp
 
-   /* Allocate and zero GD, update SP */
+   /* Allocate and initialize reserved area, update SP */
+   movhi   r2, %hi(board_init_f_reserve_size@h)
+   ori r2, r2, %lo(board_init_f_reserve_size@h)
+   callr   r2
+   sub sp, sp, r2
mov r4, sp
-   movhi   r2, %hi(board_init_f_mem@h)
-   ori r2, r2, %lo(board_init_f_mem@h)
+   movhi   r2, %hi(board_init_f_init_reserve@h)
+   ori r2, r2, %lo(board_init_f_init_reserve@h)
callr   r2
 
-   /* Update stack- and frame-pointers */
-   mov sp, r2
+   /* Update frame-pointer */
mov fp, sp
 
/* C

Re: [U-Boot] [PATCH v2] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Bin,

On Fri, 13 Nov 2015 17:08:22 +0800, Bin Meng  wrote:
> Hi Albert,
> 
> > +   subl%esp, %eax
> 
> I guess you are confused by the AT assembly syntax. This should be:
> subl %eax, %esp.

> > +   subl%esp, %eax
> 
> Ditto.

Thanks -- last time I touched Intel assembly was more than a decade
ago...

> > -   arch_setup_gd(gd_ptr);
> 
> This arch_setup_gd(gd_ptr) should not be deleted.

Correct.

> Regards,
> Bin

Thanks! These fixes will go in v3.

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

Re: [U-Boot] [PATCH v2] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Thomas,

On Thu, 12 Nov 2015 16:28:38 +0800, Thomas Chou <tho...@wytron.com.tw>
wrote:
> Hi Albert,
> 
> On 2015年11月12日 15:17, Albert ARIBAUD wrote:
> >> 
> >> diff --git a/common/init/board_init.c b/common/init/board_init.c
> >> index 8839a4a..703e6d8 100644
> >> --- a/common/init/board_init.c
> >> +++ b/common/init/board_init.c
> >> @@ -46,6 +46,7 @@ void board_init_f_gd(struct global_data *gd_ptr)
> >>for (ptr = (int *)gd_ptr; ptr < (int *)(gd_ptr + 1); )
> >>*ptr++ = 0;
> >>#endif
> >> +  arch_setup_gd(gd_ptr);
> >
> > Correct -- in ARM (Thumb-1 at least) we cannot use arch_setup_gd() so
> > we set GD (in r9) from within arch/arm/lib/crt0.S, but for NIOS2 it
> > might (and apparently does) work. Where is GD stored in NIOS2?
> >
> 
> It is a register, r26 "gp".

Ok. In ARM it is r9, and gcc is prevented from ever using r9 by adding
the compiler option -ffixed-r9 to the compiler command lines. I guess
the same goes for NIOS2 -- maybe the NIOS2 gcc does not even need an
option, and always leaves gp/r26 alone.

> I have another question. Will it be simpler to have two calls instead of 
> four?
> 
> 1. get size of gd plus malloc.
> 
> 2. init gd and malloc.

I'd hinted at reducing the number of functions, but not the number of
calls, in my reply to Simon Glass in this thread, see here:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/240520

Your proposal might indeed help reducing the number of calls as well.

The first function would receive the current top of the stack and would
subtract the (aligned) gd then the (aligned) malloc arena size, and
return the new top-of-stack, which the C runtime code would enforce
in sp (or whatever the equivalent is in each arch) -- BUT it would not
write anything in that space as it would not be reserved at that point.

The second function would receive this new top-of-stack again, this
time as a base of the reserved space (or it could receive the old
top-of-stack and work its way downward) and would be able to safely
write whatever it wants inside this space.

The only caveat is, we need to be sure that the second function can
reconstruct the base addresses of all allocated chunks (gd, malloc,
whatever they'll be wanting to add later) just like the first function 
computed them (it could still be a single function called twice as I'd
suggested, BTW).

Thanks for the suggestion! I'll consider it for v3.

> Best regards,
> Thomas

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

Re: [U-Boot] QSPI XIP boot on am437x

[Albert ARIBAUD]

Hello Vignesh,

On Fri, 13 Nov 2015 11:22:19 +0530, Vignesh R <vigne...@ti.com> wrote:
> Hi,
> 
> On 11/11/2015 02:33 PM, Albert ARIBAUD wrote:
> 
> [...]
> 
> > Alternatively, you could test the patch at
> > 
> > http://patchwork.ozlabs.org/patch/542558/
> > 
> > Let us know if this solves your issue. If it does, then it will confirm
> > the issue is with arch_setup_gd not being able to set up your GD for
> > some reason.
> > 
> > IMPORTANT: patch 542558 is *not* a final patch (there will be at least
> > a v3), and may even never land in u-boot/master; but some equivalent will
> > eventually do. Right now, patch 542558 is only a way to test if your
> > issue is a GD one.
> > 
> 
> The above patch affects the code that executes after s_init(), therefore
> I don't think above patch matters (_main is run after s_init()).

See (1) below.

> As you said in your first reply "If s_init() runs before
> board_init_f_mem(), then you must move it to
> run after board_init_f_mem().", this is the problem with my case.
> s_init() is running *before* setting up of global_data in
> arch/arm/lib/crt0.S (in _main), hence IMO, calling serial_init() in
> s_init() is wrong. This has to be moved to board_init_f() (in
> arch/arm/cpu/armv7/am33xx/board.c).
> The comment in arch/arm/cpu/armv7/lowlevel_init.S that calls s_init()
> also says *not* to access global_data and not to try to start console.
> Therefore, I intend to do something like this:
> 
> diff --git a/arch/arm/cpu/armv7/am33xx/board.c
> b/arch/arm/cpu/armv7/am33xx/board.c
> index bd14326cf479..351fc37b0483 100644
> --- a/arch/arm/cpu/armv7/am33xx/board.c
> +++ b/arch/arm/cpu/armv7/am33xx/board.c
> @@ -256,6 +256,11 @@ void board_init_f(ulong dummy)
>  {
> board_early_init_f();
> sdram_init();
> +#if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)
> +   gd->baudrate = CONFIG_BAUDRATE;
> +   serial_init();
> +   gd->have_console = 1;
> +#endif
>  }
>  #endif /* end  CONFIG_SPL_BUILD */
> 
> @@ -273,12 +278,6 @@ void s_init(void)
> set_uart_mux_conf();
> setup_clocks_for_console();
> uart_soft_reset();
> -#if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)
> -   /* TODO: This does not work, gd is not available yet */
> -   gd->baudrate = CONFIG_BAUDRATE;
> -   serial_init();
> -   gd->have_console = 1;
> -#endif
>  #if defined(CONFIG_SPL_AM33XX_ENABLE_RTC32K_OSC)
> /* Enable RTC32K clock */
> rtc32k_enable();
> 
> Do you see any issues with above change?

(1) So your s_init runs even before board_init_f_mem(), right?

Your working fix seems to imply that as long as s_init() is run after
board_init_f_mem (and any time before board_init_f) it will work. If
so, then another, fix, preferable to the above, would be that the call
to s_init be moved between those to board_init_f_mem and board_init_f.
Can you test that?

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

Re: [U-Boot] [PATCH v2] Fix board init code to use a valid C runtime environment

[Albert ARIBAUD]

Hello Thomas,

On Thu, 12 Nov 2015 13:59:28 +0800, Thomas Chou 
wrote:
> Hi Albert,

> - /* Update stack- and frame-pointers */
> - mov sp, r2
> - mov fp, sp

Just a sec here on second thought.

I understand the 'mov sp, r2' must be removed as it is replaced by a
'sub sp, sp, r2' a few lines earlier.

But doesn't the 'mov fp, sp' need to remain? Without it, fp is not set
any more, is it?

> Best regards,
> Thomas

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