All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes
@ 2016-09-05  0:32 Andre Przywara
  2016-09-05  0:32 ` [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80" Andre Przywara
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Andre Przywara @ 2016-09-05  0:32 UTC (permalink / raw)
  To: u-boot

Some patches to fix the Pine64 support:
The first two patches revert two patches that actually broke booting
Pine64 via the boot0 blob, already in 2016.07.
This has been discussed on IRC before, the commit messages contain
some details on the reasons for the revert. As the intent of those
original fixes was to help the (not yet upstream) SPL support, we don't
loose anything as the Pine64 uses the U-Boot proper only at the moment.

Patch 3/4 removes the usage of a non-existing config symbol.
Patch 4/4 fixes a compiler warning when compiling sunxi's board.c
with a 64-bit compiler.

Please apply for the 2016.09 release to allow booting Pine64s with
a recent U-Boot.

Thanks,
Andre.


Andre Przywara (4):
  Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80"
  Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64"
  sunxi: Kconfig: rename non-existent SUN50I_A64 config symbol
  sunxi: fix 64-bit compiler warning for SPL header parsing

 arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 5 -----
 board/sunxi/Kconfig                           | 2 +-
 board/sunxi/board.c                           | 2 +-
 include/configs/sunxi-common.h                | 5 +++--
 4 files changed, 5 insertions(+), 9 deletions(-)

-- 
2.8.2

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80"
  2016-09-05  0:32 [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Andre Przywara
@ 2016-09-05  0:32 ` Andre Przywara
  2016-09-05  4:12   ` Siarhei Siamashka
  2016-09-05  0:32 ` [U-Boot] [PATCH 2/4] Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64" Andre Przywara
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Andre Przywara @ 2016-09-05  0:32 UTC (permalink / raw)
  To: u-boot

This commit moved the SPL stack into SRAM C, which worked when the SPL
set the AHB1 clock down to 100 MHz to cope with the flaky SRAM C access
from the CPU.
However booting with boot0 (and thus not using SPL at all) we still run
with a 200 MHz AHB1, so any access to SRAM C is prone to fail.
Since this commit does _not_ only affect the SPL code, but also the
U-Boot proper, we fail when booting with boot0.

As the introduction of tiny-printf reduced the size of the SPL, we
can afford to have the SPL stack in SRAM A1.

This reverts commit 1a83fb4a17d959d7b037999ab7ed7e62429abe34
and fixes booting the Pine64 when using boot0.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/configs/sunxi-common.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index f64edd4..708ab17 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -99,7 +99,7 @@
  * the 1 actually activates the mapping of the first 32 KiB to 0x00000000.
  */
 #define CONFIG_SYS_INIT_RAM_ADDR	0x10000
-#define CONFIG_SYS_INIT_RAM_SIZE	0xA000	/* 40 KiB */
+#define CONFIG_SYS_INIT_RAM_SIZE	0x08000	/* FIXME: 40 KiB ? */
 #else
 #define CONFIG_SYS_INIT_RAM_ADDR	0x0
 #define CONFIG_SYS_INIT_RAM_SIZE	0x8000	/* 32 KiB */
@@ -220,7 +220,8 @@
 #define CONFIG_SPL_PAD_TO		32768		/* decimal for 'dd' */
 
 #if defined(CONFIG_MACH_SUN9I) || defined(CONFIG_MACH_SUN50I)
-#define LOW_LEVEL_SRAM_STACK		0x0001A000
+/* FIXME: 40 KiB instead of 32 KiB ? */
+#define LOW_LEVEL_SRAM_STACK		0x00018000
 #define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
 #else
 /* end of 32 KiB in sram */
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 2/4] Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64"
  2016-09-05  0:32 [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Andre Przywara
  2016-09-05  0:32 ` [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80" Andre Przywara
@ 2016-09-05  0:32 ` Andre Przywara
  2016-09-05  4:48   ` Siarhei Siamashka
  2016-09-05  0:32 ` [U-Boot] [PATCH 3/4] sunxi: Kconfig: rename non-existent SUN50I_A64 config symbol Andre Przywara
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Andre Przywara @ 2016-09-05  0:32 UTC (permalink / raw)
  To: u-boot

Now that we don't use SRAM C for the SPL stack anymore, there is no
need to clock down AHB1 to 100 MHz.
Keeping it at the recommended 200 MHz allows faster peripherals.

This reverts commit 5bc88cc2be3a962005b6e5768e06ca8f6ffcb88d.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
index be9fcfd..f7e93b0 100644
--- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
+++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
@@ -230,12 +230,7 @@ struct sunxi_ccm_reg {
 #define CCM_PLL5_TUN_INIT_FREQ(x)	(((x) & 0x7f) << 16)
 #define CCM_PLL5_TUN_INIT_FREQ_MASK	CCM_PLL5_TUN_INIT_FREQ(0x7f)
 
-#if defined(CONFIG_MACH_SUN50I)
-/* AHB1=100MHz failsafe setup from the FEL mode, usable with PMIC defaults */
-#define AHB1_ABP1_DIV_DEFAULT		0x00003190 /* AHB1=PLL6/6,APB1=AHB1/2 */
-#else
 #define AHB1_ABP1_DIV_DEFAULT		0x00003180 /* AHB1=PLL6/3,APB1=AHB1/2 */
-#endif
 
 #define AXI_GATE_OFFSET_DRAM		0
 
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/4] sunxi: Kconfig: rename non-existent SUN50I_A64 config symbol
  2016-09-05  0:32 [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Andre Przywara
  2016-09-05  0:32 ` [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80" Andre Przywara
  2016-09-05  0:32 ` [U-Boot] [PATCH 2/4] Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64" Andre Przywara
@ 2016-09-05  0:32 ` Andre Przywara
  2016-09-05  0:32 ` [U-Boot] [PATCH 4/4] sunxi: fix 64-bit compiler warning for SPL header parsing Andre Przywara
  2016-09-07 15:13 ` [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Hans de Goede
  4 siblings, 0 replies; 19+ messages in thread
From: Andre Przywara @ 2016-09-05  0:32 UTC (permalink / raw)
  To: u-boot

There is no "CONFIG_MACH_SUN50I_A64" in upstream U-Boot, so fix
the name to prevent the option to be enabled.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 board/sunxi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 1b30669..3ec011a 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -426,7 +426,7 @@ config AXP_GPIO
 
 config VIDEO
 	bool "Enable graphical uboot console on HDMI, LCD or VGA"
-	depends on !MACH_SUN8I_A83T && !MACH_SUN8I_H3 && !MACH_SUN9I && !MACH_SUN50I_A64
+	depends on !MACH_SUN8I_A83T && !MACH_SUN8I_H3 && !MACH_SUN9I && !MACH_SUN50I
 	default y
 	---help---
 	Say Y here to add support for using a cfb console on the HDMI, LCD
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 4/4] sunxi: fix 64-bit compiler warning for SPL header parsing
  2016-09-05  0:32 [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Andre Przywara
                   ` (2 preceding siblings ...)
  2016-09-05  0:32 ` [U-Boot] [PATCH 3/4] sunxi: Kconfig: rename non-existent SUN50I_A64 config symbol Andre Przywara
@ 2016-09-05  0:32 ` Andre Przywara
  2016-09-05  5:00   ` Siarhei Siamashka
  2016-09-07 15:13 ` [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Hans de Goede
  4 siblings, 1 reply; 19+ messages in thread
From: Andre Przywara @ 2016-09-05  0:32 UTC (permalink / raw)
  To: u-boot

Casting "int"s to pointers is only valid for 32-bit systems.
Add the appropriate pointer type cast to avoid a compiler warning
when compiling for AArch64.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 board/sunxi/board.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 209fb1c..6281c9d 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -602,7 +602,7 @@ static void parse_spl_header(const uint32_t spl_addr)
 		 * data is expected in uEnv.txt compatible format, so "env
 		 * import -t" the string(s) at fel_script_address right away.
 		 */
-		himport_r(&env_htab, (char *)spl->fel_script_address,
+		himport_r(&env_htab, (char *)(uintptr_t)spl->fel_script_address,
 			  spl->fel_uEnv_length, '\n', H_NOCLEAR, 0, 0, NULL);
 		return;
 	}
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80"
  2016-09-05  0:32 ` [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80" Andre Przywara
@ 2016-09-05  4:12   ` Siarhei Siamashka
  2016-09-05  8:23     ` Andre Przywara
  0 siblings, 1 reply; 19+ messages in thread
From: Siarhei Siamashka @ 2016-09-05  4:12 UTC (permalink / raw)
  To: u-boot

On Mon,  5 Sep 2016 01:32:38 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

> This commit moved the SPL stack into SRAM C, which worked when the SPL
> set the AHB1 clock down to 100 MHz to cope with the flaky SRAM C access
> from the CPU.
> However booting with boot0 (and thus not using SPL at all) we still run
> with a 200 MHz AHB1, so any access to SRAM C is prone to fail.
> Since this commit does _not_ only affect the SPL code, but also the
> U-Boot proper, we fail when booting with boot0.

Yes, it unfortunately affected both the SPL and the U-Boot
proper because currently both CONFIG_SPL_STACK and
CONFIG_SYS_INIT_SP_ADDR defines affect the SPL stack
location and in practice this only works in a predictable
way if they are set to the same value. I have sent a patch
to address this problem (but the fix may be unsafe for
v2016.09 because many ARM platforms are affected):

    https://patchwork.ozlabs.org/patch/665608/

After this problem is resolved, the CONFIG_SYS_INIT_SP_ADDR
define can be decoupled from CONFIG_SPL_STACK and configured to
even use the DRAM instead of thrashing some part of the scarce
SRAM space (which may be already occupied by the OpenRISC
firmware and/or the ATF at the time when the U-Boot proper is
starting).

> As the introduction of tiny-printf reduced the size of the SPL, we
> can afford to have the SPL stack in SRAM A1.

We still need to check how much space is really available. The FIT
support is rather heavyweight and we may want to enable some other
features too.
 
> This reverts commit 1a83fb4a17d959d7b037999ab7ed7e62429abe34
> and fixes booting the Pine64 when using boot0.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

But as discussed earlier, reverting this patch is a reasonable
solution for v2016.09, so it is

Reviewed-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>

> ---
>  include/configs/sunxi-common.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index f64edd4..708ab17 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -99,7 +99,7 @@
>   * the 1 actually activates the mapping of the first 32 KiB to 0x00000000.
>   */
>  #define CONFIG_SYS_INIT_RAM_ADDR	0x10000
> -#define CONFIG_SYS_INIT_RAM_SIZE	0xA000	/* 40 KiB */
> +#define CONFIG_SYS_INIT_RAM_SIZE	0x08000	/* FIXME: 40 KiB ? */
>  #else
>  #define CONFIG_SYS_INIT_RAM_ADDR	0x0
>  #define CONFIG_SYS_INIT_RAM_SIZE	0x8000	/* 32 KiB */
> @@ -220,7 +220,8 @@
>  #define CONFIG_SPL_PAD_TO		32768		/* decimal for 'dd' */
>  
>  #if defined(CONFIG_MACH_SUN9I) || defined(CONFIG_MACH_SUN50I)
> -#define LOW_LEVEL_SRAM_STACK		0x0001A000
> +/* FIXME: 40 KiB instead of 32 KiB ? */
> +#define LOW_LEVEL_SRAM_STACK		0x00018000
>  #define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
>  #else
>  /* end of 32 KiB in sram */



-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 2/4] Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64"
  2016-09-05  0:32 ` [U-Boot] [PATCH 2/4] Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64" Andre Przywara
@ 2016-09-05  4:48   ` Siarhei Siamashka
  2016-09-05  8:32     ` Andre Przywara
  0 siblings, 1 reply; 19+ messages in thread
From: Siarhei Siamashka @ 2016-09-05  4:48 UTC (permalink / raw)
  To: u-boot

On Mon,  5 Sep 2016 01:32:39 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

> Now that we don't use SRAM C for the SPL stack anymore, there is no
> need to clock down AHB1 to 100 MHz.

It's just another way to say it, but we are not clocking the AHB1
down, but rather keeping it at a failsafe default. If I understand it
correctly, reclocking the AHB1 to 200 MHz early in the SPL code is
not really necessary for the v2016.09 release and does not fix anything.

Moreover this revert affects USB FEL boot, which currently uses 8K
of SRAM C as a backup storage. Yes, we can also move the FEL backup
storage to SRAM A2, but the real question is whether we really want
to have it this way in the long run.

Is it really needed to reclock AHB1 early in the SPL? Can't ATF or
U-Boot proper do this a bit later? Also it's best to unmap the SRAM C
from the CPU address space at the same time as the AHB1 is reclocked
to 200 MHz. So that nobody can accidentally access it. There is a
special "bootmode" bit, which configures the SRAM C mapping:

    https://irclog.whitequark.org/linux-sunxi/2016-06-29#16863309;

Writing 0x00000000 to 0x01c00004 hides the SRAM C from the CPU. And
writing 0x01000000 there enables it again. Some SRAM C experiments
can be done with the sunxi-fel tool:

$ sunxi-fel hex 0x18000 64
00018000: 32 a6 21 9a da f7 16 d7 7c 59 e9 b3 db a2 5f 9e  2.!.....|Y...._.
00018010: d2 0c 54 b8 79 7e 7a ff 7f 0d b5 e7 96 27 34 c8  ..T.y~z......'4.
00018020: c7 d1 66 f8 4b dc a6 cb d5 ba e3 ce 26 18 49 c4  ..f.K.......&.I.
00018030: 4f f6 3f a9 56 f7 92 d4 50 b6 7f e6 73 e8 e5 69  O.?.V...P...s..i

$ sunxi-fel fill 0x18000 64 0xCC && sunxi-fel hex 0x18000 64
00018000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
00018010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
00018020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
00018030: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................

$ sunxi-fel writel 0x01c00004 0x00000000 && sunxi-fel hex 0x18000 64
00018000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00018010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00018020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00018030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................

$ sunxi-fel writel 0x01c00004 0x01000000 && sunxi-fel hex 0x18000 64
00018000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
00018010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
00018020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
00018030: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................

The commands above first show the uninitialized garbage in the SRAM C,
then initialize it to 0xCC, then disable the SRAM C access from the
CPU (it is then read as all zeros), then enable it back (it is read
as 0xCC again).

> Keeping it at the recommended 200 MHz allows faster peripherals.

I usually prefer to see some performance numbers in such commit
messages ;) Not that I doubt that something becomes faster, but it
is always interesting to know how big is the real practical effect.

> This reverts commit 5bc88cc2be3a962005b6e5768e06ca8f6ffcb88d.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

We are not fixing any bugs with this revert. Or do we?

> ---
>  arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> index be9fcfd..f7e93b0 100644
> --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> @@ -230,12 +230,7 @@ struct sunxi_ccm_reg {
>  #define CCM_PLL5_TUN_INIT_FREQ(x)	(((x) & 0x7f) << 16)
>  #define CCM_PLL5_TUN_INIT_FREQ_MASK	CCM_PLL5_TUN_INIT_FREQ(0x7f)
>  
> -#if defined(CONFIG_MACH_SUN50I)
> -/* AHB1=100MHz failsafe setup from the FEL mode, usable with PMIC defaults */
> -#define AHB1_ABP1_DIV_DEFAULT		0x00003190 /* AHB1=PLL6/6,APB1=AHB1/2 */
> -#else
>  #define AHB1_ABP1_DIV_DEFAULT		0x00003180 /* AHB1=PLL6/3,APB1=AHB1/2 */
> -#endif
>  
>  #define AXI_GATE_OFFSET_DRAM		0
>  

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 4/4] sunxi: fix 64-bit compiler warning for SPL header parsing
  2016-09-05  0:32 ` [U-Boot] [PATCH 4/4] sunxi: fix 64-bit compiler warning for SPL header parsing Andre Przywara
@ 2016-09-05  5:00   ` Siarhei Siamashka
  0 siblings, 0 replies; 19+ messages in thread
From: Siarhei Siamashka @ 2016-09-05  5:00 UTC (permalink / raw)
  To: u-boot

On Mon,  5 Sep 2016 01:32:41 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

> Casting "int"s to pointers is only valid for 32-bit systems.
> Add the appropriate pointer type cast to avoid a compiler warning
> when compiling for AArch64.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  board/sunxi/board.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 209fb1c..6281c9d 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -602,7 +602,7 @@ static void parse_spl_header(const uint32_t spl_addr)
>  		 * data is expected in uEnv.txt compatible format, so "env
>  		 * import -t" the string(s) at fel_script_address right away.
>  		 */
> -		himport_r(&env_htab, (char *)spl->fel_script_address,
> +		himport_r(&env_htab, (char *)(uintptr_t)spl->fel_script_address,
>  			  spl->fel_uEnv_length, '\n', H_NOCLEAR, 0, 0, NULL);
>  		return;
>  	}

Reviewed-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80"
  2016-09-05  4:12   ` Siarhei Siamashka
@ 2016-09-05  8:23     ` Andre Przywara
  2016-09-08 10:51       ` Siarhei Siamashka
  0 siblings, 1 reply; 19+ messages in thread
From: Andre Przywara @ 2016-09-05  8:23 UTC (permalink / raw)
  To: u-boot

Hi,

On 05/09/16 05:12, Siarhei Siamashka wrote:
> On Mon,  5 Sep 2016 01:32:38 +0100
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
>> This commit moved the SPL stack into SRAM C, which worked when the SPL
>> set the AHB1 clock down to 100 MHz to cope with the flaky SRAM C access
>> from the CPU.
>> However booting with boot0 (and thus not using SPL at all) we still run
>> with a 200 MHz AHB1, so any access to SRAM C is prone to fail.
>> Since this commit does _not_ only affect the SPL code, but also the
>> U-Boot proper, we fail when booting with boot0.
> 
> Yes, it unfortunately affected both the SPL and the U-Boot
> proper because currently both CONFIG_SPL_STACK and
> CONFIG_SYS_INIT_SP_ADDR defines affect the SPL stack
> location and in practice this only works in a predictable
> way if they are set to the same value. I have sent a patch
> to address this problem (but the fix may be unsafe for
> v2016.09 because many ARM platforms are affected):
> 
>     https://patchwork.ozlabs.org/patch/665608/
> 
> After this problem is resolved, the CONFIG_SYS_INIT_SP_ADDR
> define can be decoupled from CONFIG_SPL_STACK and configured to
> even use the DRAM instead of thrashing some part of the scarce
> SRAM space (which may be already occupied by the OpenRISC
> firmware and/or the ATF at the time when the U-Boot proper is
> starting).
> 
>> As the introduction of tiny-printf reduced the size of the SPL, we
>> can afford to have the SPL stack in SRAM A1.
> 
> We still need to check how much space is really available. The FIT
> support is rather heavyweight and we may want to enable some other
> features too.

Yes, I had to learn this yesterday ;-)
So 64-bit SPL works for me now with Jens' DRAM support patches (yeah!),
but enabling FIT support makes mksunxiboot barf about the file being to
big. The actual SPL code is about 31K, so maybe I can talk mksunxiboot
into relaxing its alignment requirements a bit (from 8K down to 512) and
also increase the available SRAM size - it says 0x7600 for sun4i, is
this still true to newer SoCs/BROMs?

Trying this in the past (with libdram) and compiling for (32-bit) Thumb2
worked, but I need to check what the actual size with Jens' patches are
these days for Thumb2.

Anyway, thanks for your patch, I will try tonight if I can squeeze all
the bits in.

>  
>> This reverts commit 1a83fb4a17d959d7b037999ab7ed7e62429abe34
>> and fixes booting the Pine64 when using boot0.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> But as discussed earlier, reverting this patch is a reasonable
> solution for v2016.09, so it is
> 
> Reviewed-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>

Thanks for that!

Cheers,
Andre.

> 
>> ---
>>  include/configs/sunxi-common.h | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
>> index f64edd4..708ab17 100644
>> --- a/include/configs/sunxi-common.h
>> +++ b/include/configs/sunxi-common.h
>> @@ -99,7 +99,7 @@
>>   * the 1 actually activates the mapping of the first 32 KiB to 0x00000000.
>>   */
>>  #define CONFIG_SYS_INIT_RAM_ADDR	0x10000
>> -#define CONFIG_SYS_INIT_RAM_SIZE	0xA000	/* 40 KiB */
>> +#define CONFIG_SYS_INIT_RAM_SIZE	0x08000	/* FIXME: 40 KiB ? */
>>  #else
>>  #define CONFIG_SYS_INIT_RAM_ADDR	0x0
>>  #define CONFIG_SYS_INIT_RAM_SIZE	0x8000	/* 32 KiB */
>> @@ -220,7 +220,8 @@
>>  #define CONFIG_SPL_PAD_TO		32768		/* decimal for 'dd' */
>>  
>>  #if defined(CONFIG_MACH_SUN9I) || defined(CONFIG_MACH_SUN50I)
>> -#define LOW_LEVEL_SRAM_STACK		0x0001A000
>> +/* FIXME: 40 KiB instead of 32 KiB ? */
>> +#define LOW_LEVEL_SRAM_STACK		0x00018000
>>  #define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
>>  #else
>>  /* end of 32 KiB in sram */
> 
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 2/4] Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64"
  2016-09-05  4:48   ` Siarhei Siamashka
@ 2016-09-05  8:32     ` Andre Przywara
  0 siblings, 0 replies; 19+ messages in thread
From: Andre Przywara @ 2016-09-05  8:32 UTC (permalink / raw)
  To: u-boot

Hi,

On 05/09/16 05:48, Siarhei Siamashka wrote:
> On Mon,  5 Sep 2016 01:32:39 +0100
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
>> Now that we don't use SRAM C for the SPL stack anymore, there is no
>> need to clock down AHB1 to 100 MHz.
> 
> It's just another way to say it, but we are not clocking the AHB1
> down, but rather keeping it at a failsafe default. If I understand it
> correctly, reclocking the AHB1 to 200 MHz early in the SPL code is
> not really necessary for the v2016.09 release and does not fix anything.

Yes, this patch is indeed somewhat optional. It was just a logical
consequence of the other patch being reverted.

> Moreover this revert affects USB FEL boot, which currently uses 8K
> of SRAM C as a backup storage.

Ah, that may explain the FEL issues I was trying to track down on the
weekend.

> Yes, we can also move the FEL backup
> storage to SRAM A2, but the real question is whether we really want
> to have it this way in the long run.
> 
> Is it really needed to reclock AHB1 early in the SPL? Can't ATF or
> U-Boot proper do this a bit later?

Good point. My current ATF branch at home sets up all the basic clocks
to their desired values, so indeed U-Boot wouldn't need to touch the
clocks anymore (also I think it shouldn't care).
In my current setup ATF is expected to run after the SPL, but before the
U-Boot proper.

> Also it's best to unmap the SRAM C
> from the CPU address space at the same time as the AHB1 is reclocked
> to 200 MHz. So that nobody can accidentally access it.

Yes, that sounds reasonable.

> There is a
> special "bootmode" bit, which configures the SRAM C mapping:
> 
>     https://irclog.whitequark.org/linux-sunxi/2016-06-29#16863309;
> 
> Writing 0x00000000 to 0x01c00004 hides the SRAM C from the CPU. And
> writing 0x01000000 there enables it again. Some SRAM C experiments
> can be done with the sunxi-fel tool:
> 
> $ sunxi-fel hex 0x18000 64
> 00018000: 32 a6 21 9a da f7 16 d7 7c 59 e9 b3 db a2 5f 9e  2.!.....|Y...._.
> 00018010: d2 0c 54 b8 79 7e 7a ff 7f 0d b5 e7 96 27 34 c8  ..T.y~z......'4.
> 00018020: c7 d1 66 f8 4b dc a6 cb d5 ba e3 ce 26 18 49 c4  ..f.K.......&.I.
> 00018030: 4f f6 3f a9 56 f7 92 d4 50 b6 7f e6 73 e8 e5 69  O.?.V...P...s..i
> 
> $ sunxi-fel fill 0x18000 64 0xCC && sunxi-fel hex 0x18000 64
> 00018000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018030: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 
> $ sunxi-fel writel 0x01c00004 0x00000000 && sunxi-fel hex 0x18000 64
> 00018000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00018010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00018020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00018030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 
> $ sunxi-fel writel 0x01c00004 0x01000000 && sunxi-fel hex 0x18000 64
> 00018000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018030: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 
> The commands above first show the uninitialized garbage in the SRAM C,
> then initialize it to 0xCC, then disable the SRAM C access from the
> CPU (it is then read as all zeros), then enable it back (it is read
> as 0xCC again).

Nice one! Didn't know about that. I will do some experiments tonight.

>> Keeping it at the recommended 200 MHz allows faster peripherals.
> 
> I usually prefer to see some performance numbers in such commit
> messages ;) Not that I doubt that something becomes faster, but it
> is always interesting to know how big is the real practical effect.
> 
>> This reverts commit 5bc88cc2be3a962005b6e5768e06ca8f6ffcb88d.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> We are not fixing any bugs with this revert. Or do we?

Probably not. Let me give it actually a try without this patch and then
we can drop this one.

Thanks,
Andre.

>> ---
>>  arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>> index be9fcfd..f7e93b0 100644
>> --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>> @@ -230,12 +230,7 @@ struct sunxi_ccm_reg {
>>  #define CCM_PLL5_TUN_INIT_FREQ(x)	(((x) & 0x7f) << 16)
>>  #define CCM_PLL5_TUN_INIT_FREQ_MASK	CCM_PLL5_TUN_INIT_FREQ(0x7f)
>>  
>> -#if defined(CONFIG_MACH_SUN50I)
>> -/* AHB1=100MHz failsafe setup from the FEL mode, usable with PMIC defaults */
>> -#define AHB1_ABP1_DIV_DEFAULT		0x00003190 /* AHB1=PLL6/6,APB1=AHB1/2 */
>> -#else
>>  #define AHB1_ABP1_DIV_DEFAULT		0x00003180 /* AHB1=PLL6/3,APB1=AHB1/2 */
>> -#endif
>>  
>>  #define AXI_GATE_OFFSET_DRAM		0
>>  
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes
  2016-09-05  0:32 [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Andre Przywara
                   ` (3 preceding siblings ...)
  2016-09-05  0:32 ` [U-Boot] [PATCH 4/4] sunxi: fix 64-bit compiler warning for SPL header parsing Andre Przywara
@ 2016-09-07 15:13 ` Hans de Goede
  2016-09-07 15:18   ` Andre Przywara
  4 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2016-09-07 15:13 UTC (permalink / raw)
  To: u-boot

Hi,

On 05-09-16 02:32, Andre Przywara wrote:
> Some patches to fix the Pine64 support:
> The first two patches revert two patches that actually broke booting
> Pine64 via the boot0 blob, already in 2016.07.
> This has been discussed on IRC before, the commit messages contain
> some details on the reasons for the revert. As the intent of those
> original fixes was to help the (not yet upstream) SPL support, we don't
> loose anything as the Pine64 uses the U-Boot proper only at the moment.
>
> Patch 3/4 removes the usage of a non-existing config symbol.
> Patch 4/4 fixes a compiler warning when compiling sunxi's board.c
> with a 64-bit compiler.
>
> Please apply for the 2016.09 release to allow booting Pine64s with
> a recent U-Boot.

I've send out a pull-req with patches 1, 3 and 4 yesterday and
this has been merged.

Regards,

Hans


>
> Thanks,
> Andre.
>
>
> Andre Przywara (4):
>   Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80"
>   Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64"
>   sunxi: Kconfig: rename non-existent SUN50I_A64 config symbol
>   sunxi: fix 64-bit compiler warning for SPL header parsing
>
>  arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 5 -----
>  board/sunxi/Kconfig                           | 2 +-
>  board/sunxi/board.c                           | 2 +-
>  include/configs/sunxi-common.h                | 5 +++--
>  4 files changed, 5 insertions(+), 9 deletions(-)
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes
  2016-09-07 15:13 ` [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Hans de Goede
@ 2016-09-07 15:18   ` Andre Przywara
  2016-09-07 15:48     ` Hans de Goede
  0 siblings, 1 reply; 19+ messages in thread
From: Andre Przywara @ 2016-09-07 15:18 UTC (permalink / raw)
  To: u-boot

Hi,

On 07/09/16 16:13, Hans de Goede wrote:
> Hi,
> 
> On 05-09-16 02:32, Andre Przywara wrote:
>> Some patches to fix the Pine64 support:
>> The first two patches revert two patches that actually broke booting
>> Pine64 via the boot0 blob, already in 2016.07.
>> This has been discussed on IRC before, the commit messages contain
>> some details on the reasons for the revert. As the intent of those
>> original fixes was to help the (not yet upstream) SPL support, we don't
>> loose anything as the Pine64 uses the U-Boot proper only at the moment.
>>
>> Patch 3/4 removes the usage of a non-existing config symbol.
>> Patch 4/4 fixes a compiler warning when compiling sunxi's board.c
>> with a 64-bit compiler.
>>
>> Please apply for the 2016.09 release to allow booting Pine64s with
>> a recent U-Boot.
> 
> I've send out a pull-req with patches 1, 3 and 4 yesterday and
> this has been merged.

Thanks a lot! That's very helpful.

Cheers,
Andre.

P.S. I have the SPL support with proper sunxi DRAM init mostly working,
I am fighting with some bugs still (FIT loading related), but I am
confident that this will be ready for the upcoming merge window.

>>
>>
>> Andre Przywara (4):
>>   Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80"
>>   Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64"
>>   sunxi: Kconfig: rename non-existent SUN50I_A64 config symbol
>>   sunxi: fix 64-bit compiler warning for SPL header parsing
>>
>>  arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 5 -----
>>  board/sunxi/Kconfig                           | 2 +-
>>  board/sunxi/board.c                           | 2 +-
>>  include/configs/sunxi-common.h                | 5 +++--
>>  4 files changed, 5 insertions(+), 9 deletions(-)
>>
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes
  2016-09-07 15:18   ` Andre Przywara
@ 2016-09-07 15:48     ` Hans de Goede
  2016-09-07 15:55       ` Andre Przywara
  0 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2016-09-07 15:48 UTC (permalink / raw)
  To: u-boot

Hi,

On 07-09-16 17:18, Andre Przywara wrote:
> Hi,
>
> On 07/09/16 16:13, Hans de Goede wrote:
>> Hi,
>>
>> On 05-09-16 02:32, Andre Przywara wrote:
>>> Some patches to fix the Pine64 support:
>>> The first two patches revert two patches that actually broke booting
>>> Pine64 via the boot0 blob, already in 2016.07.
>>> This has been discussed on IRC before, the commit messages contain
>>> some details on the reasons for the revert. As the intent of those
>>> original fixes was to help the (not yet upstream) SPL support, we don't
>>> loose anything as the Pine64 uses the U-Boot proper only at the moment.
>>>
>>> Patch 3/4 removes the usage of a non-existing config symbol.
>>> Patch 4/4 fixes a compiler warning when compiling sunxi's board.c
>>> with a 64-bit compiler.
>>>
>>> Please apply for the 2016.09 release to allow booting Pine64s with
>>> a recent U-Boot.
>>
>> I've send out a pull-req with patches 1, 3 and 4 yesterday and
>> this has been merged.
>
> Thanks a lot! That's very helpful.
>
> Cheers,
> Andre.
>
> P.S. I have the SPL support with proper sunxi DRAM init mostly working,
> I am fighting with some bugs still (FIT loading related), but I am
> confident that this will be ready for the upcoming merge window.

Cool, so do we then still need some closed firmware bits, or is everything
open then ?

Regards,

Hans

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes
  2016-09-07 15:48     ` Hans de Goede
@ 2016-09-07 15:55       ` Andre Przywara
  2016-09-07 16:12         ` Tom Rini
  2016-09-08  8:26         ` Hans de Goede
  0 siblings, 2 replies; 19+ messages in thread
From: Andre Przywara @ 2016-09-07 15:55 UTC (permalink / raw)
  To: u-boot

Hi,

On 07/09/16 16:48, Hans de Goede wrote:
> Hi,
> 
> On 07-09-16 17:18, Andre Przywara wrote:
>> Hi,
>>
>> On 07/09/16 16:13, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 05-09-16 02:32, Andre Przywara wrote:
>>>> Some patches to fix the Pine64 support:
>>>> The first two patches revert two patches that actually broke booting
>>>> Pine64 via the boot0 blob, already in 2016.07.
>>>> This has been discussed on IRC before, the commit messages contain
>>>> some details on the reasons for the revert. As the intent of those
>>>> original fixes was to help the (not yet upstream) SPL support, we don't
>>>> loose anything as the Pine64 uses the U-Boot proper only at the moment.
>>>>
>>>> Patch 3/4 removes the usage of a non-existing config symbol.
>>>> Patch 4/4 fixes a compiler warning when compiling sunxi's board.c
>>>> with a 64-bit compiler.
>>>>
>>>> Please apply for the 2016.09 release to allow booting Pine64s with
>>>> a recent U-Boot.
>>>
>>> I've send out a pull-req with patches 1, 3 and 4 yesterday and
>>> this has been merged.
>>
>> Thanks a lot! That's very helpful.
>>
>> Cheers,
>> Andre.
>>
>> P.S. I have the SPL support with proper sunxi DRAM init mostly working,
>> I am fighting with some bugs still (FIT loading related), but I am
>> confident that this will be ready for the upcoming merge window.
> 
> Cool, so do we then still need some closed firmware bits, or is everything
> open then ?

Everything is proper FOSS, thanks to Jens, who took the existing U-Boot
H3 DRAM init bits and ported them to support the A64 as well.

Also we don't need any boot0img anymore, mksunxiboot works as expected.
Appending the U-Boot proper directly to the SPL image works already, but
leaves out the ATF and thus SMP :-(
So I have improved SPL FIT support to load multiple images: load both
U-Boot and ATF, but jump to ATF, which in turn drops to U-Boot.
I chose the have the SPL in AArch64 (with a handful of AArch32
instructions in the beginning), which greatly simplifies the build
process. The drawback is increased code size, but I think we can cope
with that.

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes
  2016-09-07 15:55       ` Andre Przywara
@ 2016-09-07 16:12         ` Tom Rini
  2016-09-07 16:20           ` Andre Przywara
  2016-09-08  8:26         ` Hans de Goede
  1 sibling, 1 reply; 19+ messages in thread
From: Tom Rini @ 2016-09-07 16:12 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 07, 2016 at 04:55:53PM +0100, Andre Przywara wrote:
> Hi,
> 
> On 07/09/16 16:48, Hans de Goede wrote:
> > Hi,
> > 
> > On 07-09-16 17:18, Andre Przywara wrote:
> >> Hi,
> >>
> >> On 07/09/16 16:13, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 05-09-16 02:32, Andre Przywara wrote:
> >>>> Some patches to fix the Pine64 support:
> >>>> The first two patches revert two patches that actually broke booting
> >>>> Pine64 via the boot0 blob, already in 2016.07.
> >>>> This has been discussed on IRC before, the commit messages contain
> >>>> some details on the reasons for the revert. As the intent of those
> >>>> original fixes was to help the (not yet upstream) SPL support, we don't
> >>>> loose anything as the Pine64 uses the U-Boot proper only at the moment.
> >>>>
> >>>> Patch 3/4 removes the usage of a non-existing config symbol.
> >>>> Patch 4/4 fixes a compiler warning when compiling sunxi's board.c
> >>>> with a 64-bit compiler.
> >>>>
> >>>> Please apply for the 2016.09 release to allow booting Pine64s with
> >>>> a recent U-Boot.
> >>>
> >>> I've send out a pull-req with patches 1, 3 and 4 yesterday and
> >>> this has been merged.
> >>
> >> Thanks a lot! That's very helpful.
> >>
> >> Cheers,
> >> Andre.
> >>
> >> P.S. I have the SPL support with proper sunxi DRAM init mostly working,
> >> I am fighting with some bugs still (FIT loading related), but I am
> >> confident that this will be ready for the upcoming merge window.
> > 
> > Cool, so do we then still need some closed firmware bits, or is everything
> > open then ?
> 
> Everything is proper FOSS, thanks to Jens, who took the existing U-Boot
> H3 DRAM init bits and ported them to support the A64 as well.
> 
> Also we don't need any boot0img anymore, mksunxiboot works as expected.
> Appending the U-Boot proper directly to the SPL image works already, but
> leaves out the ATF and thus SMP :-(
> So I have improved SPL FIT support to load multiple images: load both
> U-Boot and ATF, but jump to ATF, which in turn drops to U-Boot.
> I chose the have the SPL in AArch64 (with a handful of AArch32
> instructions in the beginning), which greatly simplifies the build
> process. The drawback is increased code size, but I think we can cope
> with that.

Nice!  Will it all also work with FEL mode?  That's how I'm integrating
sunxi boards into my test farm.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160907/0c6d7ff3/attachment.sig>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes
  2016-09-07 16:12         ` Tom Rini
@ 2016-09-07 16:20           ` Andre Przywara
  0 siblings, 0 replies; 19+ messages in thread
From: Andre Przywara @ 2016-09-07 16:20 UTC (permalink / raw)
  To: u-boot

Hi,

On 07/09/16 17:12, Tom Rini wrote:
> On Wed, Sep 07, 2016 at 04:55:53PM +0100, Andre Przywara wrote:
>> Hi,
>>
>> On 07/09/16 16:48, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 07-09-16 17:18, Andre Przywara wrote:
>>>> Hi,
>>>>
>>>> On 07/09/16 16:13, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 05-09-16 02:32, Andre Przywara wrote:
>>>>>> Some patches to fix the Pine64 support:
>>>>>> The first two patches revert two patches that actually broke booting
>>>>>> Pine64 via the boot0 blob, already in 2016.07.
>>>>>> This has been discussed on IRC before, the commit messages contain
>>>>>> some details on the reasons for the revert. As the intent of those
>>>>>> original fixes was to help the (not yet upstream) SPL support, we don't
>>>>>> loose anything as the Pine64 uses the U-Boot proper only at the moment.
>>>>>>
>>>>>> Patch 3/4 removes the usage of a non-existing config symbol.
>>>>>> Patch 4/4 fixes a compiler warning when compiling sunxi's board.c
>>>>>> with a 64-bit compiler.
>>>>>>
>>>>>> Please apply for the 2016.09 release to allow booting Pine64s with
>>>>>> a recent U-Boot.
>>>>>
>>>>> I've send out a pull-req with patches 1, 3 and 4 yesterday and
>>>>> this has been merged.
>>>>
>>>> Thanks a lot! That's very helpful.
>>>>
>>>> Cheers,
>>>> Andre.
>>>>
>>>> P.S. I have the SPL support with proper sunxi DRAM init mostly working,
>>>> I am fighting with some bugs still (FIT loading related), but I am
>>>> confident that this will be ready for the upcoming merge window.
>>>
>>> Cool, so do we then still need some closed firmware bits, or is everything
>>> open then ?
>>
>> Everything is proper FOSS, thanks to Jens, who took the existing U-Boot
>> H3 DRAM init bits and ported them to support the A64 as well.
>>
>> Also we don't need any boot0img anymore, mksunxiboot works as expected.
>> Appending the U-Boot proper directly to the SPL image works already, but
>> leaves out the ATF and thus SMP :-(
>> So I have improved SPL FIT support to load multiple images: load both
>> U-Boot and ATF, but jump to ATF, which in turn drops to U-Boot.
>> I chose the have the SPL in AArch64 (with a handful of AArch32
>> instructions in the beginning), which greatly simplifies the build
>> process. The drawback is increased code size, but I think we can cope
>> with that.
> 
> Nice!  Will it all also work with FEL mode?  That's how I'm integrating
> sunxi boards into my test farm.  Thanks!

Kind of ;-) Be assured that due to it's convenience FEL is quite
important to me too - especially when hacking on firmware.

I have some code which saves the FEL state before switching to AArch64,
and restores it after having switched back to AArch32 just after the SPL
calls return_to_fel(). That scheme worked with some experimental code (I
saw a UART character written by AArch64 code), but not with the real
SPL, I guess due to some stack trouble (IIRC FEL put some data into SRAM
C on the A64) or some state which the SPL has changed.
I have to revisit this, but wouldn't hold back a post if FEL is still
broken. If I can't fix it myself, I will put it somewhere and leave it
up to the pros to attack this.

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes
  2016-09-07 15:55       ` Andre Przywara
  2016-09-07 16:12         ` Tom Rini
@ 2016-09-08  8:26         ` Hans de Goede
  1 sibling, 0 replies; 19+ messages in thread
From: Hans de Goede @ 2016-09-08  8:26 UTC (permalink / raw)
  To: u-boot

Hi,

On 07-09-16 17:55, Andre Przywara wrote:
> Hi,
>
> On 07/09/16 16:48, Hans de Goede wrote:
>> Hi,
>>
>> On 07-09-16 17:18, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 07/09/16 16:13, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 05-09-16 02:32, Andre Przywara wrote:
>>>>> Some patches to fix the Pine64 support:
>>>>> The first two patches revert two patches that actually broke booting
>>>>> Pine64 via the boot0 blob, already in 2016.07.
>>>>> This has been discussed on IRC before, the commit messages contain
>>>>> some details on the reasons for the revert. As the intent of those
>>>>> original fixes was to help the (not yet upstream) SPL support, we don't
>>>>> loose anything as the Pine64 uses the U-Boot proper only at the moment.
>>>>>
>>>>> Patch 3/4 removes the usage of a non-existing config symbol.
>>>>> Patch 4/4 fixes a compiler warning when compiling sunxi's board.c
>>>>> with a 64-bit compiler.
>>>>>
>>>>> Please apply for the 2016.09 release to allow booting Pine64s with
>>>>> a recent U-Boot.
>>>>
>>>> I've send out a pull-req with patches 1, 3 and 4 yesterday and
>>>> this has been merged.
>>>
>>> Thanks a lot! That's very helpful.
>>>
>>> Cheers,
>>> Andre.
>>>
>>> P.S. I have the SPL support with proper sunxi DRAM init mostly working,
>>> I am fighting with some bugs still (FIT loading related), but I am
>>> confident that this will be ready for the upcoming merge window.
>>
>> Cool, so do we then still need some closed firmware bits, or is everything
>> open then ?
>
> Everything is proper FOSS, thanks to Jens, who took the existing U-Boot
> H3 DRAM init bits and ported them to support the A64 as well.
>
> Also we don't need any boot0img anymore, mksunxiboot works as expected.
> Appending the U-Boot proper directly to the SPL image works already, but
> leaves out the ATF and thus SMP :-(
> So I have improved SPL FIT support to load multiple images: load both
> U-Boot and ATF, but jump to ATF, which in turn drops to U-Boot.
> I chose the have the SPL in AArch64 (with a handful of AArch32
> instructions in the beginning), which greatly simplifies the build
> process. The drawback is increased code size, but I think we can cope
> with that.

Yeah we should be able to make things work with a somewhat increased
code size.

Otherwise this all sounds very nice. I'm looking forward to the
first posting of the patches for this.

Regards,

Hans

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80"
  2016-09-05  8:23     ` Andre Przywara
@ 2016-09-08 10:51       ` Siarhei Siamashka
  2016-09-08 13:23         ` Andre Przywara
  0 siblings, 1 reply; 19+ messages in thread
From: Siarhei Siamashka @ 2016-09-08 10:51 UTC (permalink / raw)
  To: u-boot

On Mon, 5 Sep 2016 09:23:00 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

> Hi,
> 
> On 05/09/16 05:12, Siarhei Siamashka wrote:
> > On Mon,  5 Sep 2016 01:32:38 +0100
> > Andre Przywara <andre.przywara@arm.com> wrote:
> >   
> >> This commit moved the SPL stack into SRAM C, which worked when the SPL
> >> set the AHB1 clock down to 100 MHz to cope with the flaky SRAM C access
> >> from the CPU.
> >> However booting with boot0 (and thus not using SPL at all) we still run
> >> with a 200 MHz AHB1, so any access to SRAM C is prone to fail.
> >> Since this commit does _not_ only affect the SPL code, but also the
> >> U-Boot proper, we fail when booting with boot0.  
> > 
> > Yes, it unfortunately affected both the SPL and the U-Boot
> > proper because currently both CONFIG_SPL_STACK and
> > CONFIG_SYS_INIT_SP_ADDR defines affect the SPL stack
> > location and in practice this only works in a predictable
> > way if they are set to the same value. I have sent a patch
> > to address this problem (but the fix may be unsafe for
> > v2016.09 because many ARM platforms are affected):
> > 
> >     https://patchwork.ozlabs.org/patch/665608/
> > 
> > After this problem is resolved, the CONFIG_SYS_INIT_SP_ADDR
> > define can be decoupled from CONFIG_SPL_STACK and configured to
> > even use the DRAM instead of thrashing some part of the scarce
> > SRAM space (which may be already occupied by the OpenRISC
> > firmware and/or the ATF at the time when the U-Boot proper is
> > starting).
> >   
> >> As the introduction of tiny-printf reduced the size of the SPL, we
> >> can afford to have the SPL stack in SRAM A1.  
> > 
> > We still need to check how much space is really available. The FIT
> > support is rather heavyweight and we may want to enable some other
> > features too.  
> 
> Yes, I had to learn this yesterday ;-)
> So 64-bit SPL works for me now with Jens' DRAM support patches (yeah!),
> but enabling FIT support makes mksunxiboot barf about the file being to
> big. The actual SPL code is about 31K, so maybe I can talk mksunxiboot
> into relaxing its alignment requirements a bit (from 8K down to 512) and
> also increase the available SRAM size - it says 0x7600 for sun4i, is
> this still true to newer SoCs/BROMs?

We have this information in the linux-sunxi wiki since a long time ago
(at least for the SoC variants that I have and could experiment with)
and it is available here:

    https://linux-sunxi.org/BROM#U-Boot_SPL_limitations

All the new SoCs have a 32K size limit for the SPL code, which can be
loaded by the BROM. Older A10/A20 SoCs artificially limit it to 24K,
probably trying to forcefully encourage the users to have 8K stack in
the remaining part of the SRAM A1.

On A64, we have 32K of SRAM A1. Then we have 108K of SRAM C, which is a
continuation of SRAM A1 in the address space thus making it look like a
nice single 140K chunk. Then we also have 64K of SRAM A2, which is
supposed to be used by the OpenRISC core and is the only memory area,
which has a reasonable performance when used by OpenRISC:

    https://linux-sunxi.org/AR100#Memory_Map

The idea was to let the BROM load up to 32K of the SPL code to the
SRAM A1 (like it normally does) and then have 8K of stack a bit higher
in the address space in SRAM C. But it turned out that the SRAM C is a
bit quirky and suffers from data corruption problems if we reclock
AHB1 too early.

Now there are two possible ways to move forward on A64:
  1) Try to use SRAM C in such a way that it does not fail (and hope
     that no additional quirks get discovered later).
  2) Move the initial SPL stack to SRAM A2.

If we move everything to SRAM A2, then we will have to make sure that
all the SRAM users (the FEL storage area, the SPL stack, the ATF and
the yet to be implemented OpenRISC firmware) never clash with each
other.

About the 31K code size. This does not look good and is very close to
the BROM limit (32K). Just using a different compiler may bring us into
a trouble. Or some minor code tweaks and feature additions.

> Trying this in the past (with libdram) and compiling for (32-bit) Thumb2
> worked, but I need to check what the actual size with Jens' patches are
> these days for Thumb2.

We have already discussed this off-list a long time ago. I know that
both you and Alexander Graf are generally in favour of compiling the
SPL as 64-bit code.

I think that this is the usual case of utility versus fashion. Everyone
wants to plug every hole with 64-bit ARM code right now just because it
is new and innovative. But this fad will fade away in a few years. Now
just imagine an alternative reality, where ARM64 is an old and boring
thing, while Thumb2 is a recent invention to improve code density in
microcontrollers and other code space constrained systems. I'm sure
that everyone would be trying to find a way to replace the legacy
bloated 64-bit ARM code in the SPL with the new and shiny Thumb2
stuff for improving code density ;-)

If we take a pragmatic approach and try to evaluate pro- and cons-
factors, then we can see that the 64-bit code in the SPL on
Allwinner A64 hardware does not give us any real improvements.
Quite the contrary: it offers worse code density than 32-bit
Thumb2 and also a functional USB FEL boot support becomes much
more tricky (because the boot ROM implements FEL as a 32-bit code).

The only real argument in favour of having a 64-bit SPL is that we
can use a single AArch64 toolchain to build both the SPL and the
main U-Boot. And we are in this situation only because the AArch64
toolchain does not support the "-m32" option. There is no technical
justification for this. ARM decided to be different just for the
sake of being different (every other architecture has the -m32
option in GCC if the processor is able to work in both modes).
If the "-m32" option was supported, then building a 32-bit SPL
would have been mostly a trivial matter of adding "-m32 -mthumb"
options to CFLAGS.

But we can try to do a 32-bit SPL build by introducing something like
a CROSS_COMPILE_SPL environment variable, just like suggested some
time ago: http://lists.denx.de/pipermail/u-boot/2012-April/122236.html

Also I'm finally going to submit the runtime SPL code decompression
patches for the next U-Boot release, because there is no need to delay
the implementation of this feature any longer. Yes, I know that any
saved space will be wasted almost instantly by various gimmicks, but
that's just how it is.

> Anyway, thanks for your patch, I will try tonight if I can squeeze all
> the bits in.

If you mean https://patchwork.ozlabs.org/patch/665608/ then it only
gives us the freedom to move CONFIG_SYS_INIT_SP_ADDR somewhere else.

And moving the initial stack if the U-Boot proper into the DRAM would
make a lot of sense. We only need to agree what kind of DRAM address to
use. After all, even the SPL relocates the stack into the DRAM. Why
does the U-Boot proper want to use the SRAM for its stack again?

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80"
  2016-09-08 10:51       ` Siarhei Siamashka
@ 2016-09-08 13:23         ` Andre Przywara
  0 siblings, 0 replies; 19+ messages in thread
From: Andre Przywara @ 2016-09-08 13:23 UTC (permalink / raw)
  To: u-boot

Hi,

On 08/09/16 11:51, Siarhei Siamashka wrote:
> On Mon, 5 Sep 2016 09:23:00 +0100
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
>> Hi,
>>
>> On 05/09/16 05:12, Siarhei Siamashka wrote:
>>> On Mon,  5 Sep 2016 01:32:38 +0100
>>> Andre Przywara <andre.przywara@arm.com> wrote:
>>>   
>>>> This commit moved the SPL stack into SRAM C, which worked when the SPL
>>>> set the AHB1 clock down to 100 MHz to cope with the flaky SRAM C access
>>>> from the CPU.
>>>> However booting with boot0 (and thus not using SPL at all) we still run
>>>> with a 200 MHz AHB1, so any access to SRAM C is prone to fail.
>>>> Since this commit does _not_ only affect the SPL code, but also the
>>>> U-Boot proper, we fail when booting with boot0.  
>>>
>>> Yes, it unfortunately affected both the SPL and the U-Boot
>>> proper because currently both CONFIG_SPL_STACK and
>>> CONFIG_SYS_INIT_SP_ADDR defines affect the SPL stack
>>> location and in practice this only works in a predictable
>>> way if they are set to the same value. I have sent a patch
>>> to address this problem (but the fix may be unsafe for
>>> v2016.09 because many ARM platforms are affected):
>>>
>>>     https://patchwork.ozlabs.org/patch/665608/
>>>
>>> After this problem is resolved, the CONFIG_SYS_INIT_SP_ADDR
>>> define can be decoupled from CONFIG_SPL_STACK and configured to
>>> even use the DRAM instead of thrashing some part of the scarce
>>> SRAM space (which may be already occupied by the OpenRISC
>>> firmware and/or the ATF at the time when the U-Boot proper is
>>> starting).
>>>   
>>>> As the introduction of tiny-printf reduced the size of the SPL, we
>>>> can afford to have the SPL stack in SRAM A1.  
>>>
>>> We still need to check how much space is really available. The FIT
>>> support is rather heavyweight and we may want to enable some other
>>> features too.  
>>
>> Yes, I had to learn this yesterday ;-)
>> So 64-bit SPL works for me now with Jens' DRAM support patches (yeah!),
>> but enabling FIT support makes mksunxiboot barf about the file being to
>> big. The actual SPL code is about 31K, so maybe I can talk mksunxiboot
>> into relaxing its alignment requirements a bit (from 8K down to 512) and
>> also increase the available SRAM size - it says 0x7600 for sun4i, is
>> this still true to newer SoCs/BROMs?
> 
> We have this information in the linux-sunxi wiki since a long time ago
> (at least for the SoC variants that I have and could experiment with)
> and it is available here:
> 
>     https://linux-sunxi.org/BROM#U-Boot_SPL_limitations
> 
> All the new SoCs have a 32K size limit for the SPL code, which can be
> loaded by the BROM. Older A10/A20 SoCs artificially limit it to 24K,
> probably trying to forcefully encourage the users to have 8K stack in
> the remaining part of the SRAM A1.
> 
> On A64, we have 32K of SRAM A1. Then we have 108K of SRAM C, which is a
> continuation of SRAM A1 in the address space thus making it look like a
> nice single 140K chunk. Then we also have 64K of SRAM A2, which is
> supposed to be used by the OpenRISC core and is the only memory area,
> which has a reasonable performance when used by OpenRISC:
> 
>     https://linux-sunxi.org/AR100#Memory_Map
> 
> The idea was to let the BROM load up to 32K of the SPL code to the
> SRAM A1 (like it normally does) and then have 8K of stack a bit higher
> in the address space in SRAM C. But it turned out that the SRAM C is a
> bit quirky and suffers from data corruption problems if we reclock
> AHB1 too early.
> 
> Now there are two possible ways to move forward on A64:
>   1) Try to use SRAM C in such a way that it does not fail (and hope
>      that no additional quirks get discovered later).
>   2) Move the initial SPL stack to SRAM A2.
> 
> If we move everything to SRAM A2, then we will have to make sure that
> all the SRAM users (the FEL storage area, the SPL stack, the ATF and
> the yet to be implemented OpenRISC firmware) never clash with each
> other.

So I moved the initial stack into SRAM A2 already, which made SPL to
work in AArch64 (in contrast to having the stack at the end of SRAM A1,
which breaks quite early - though I managed to see the SPL banner ;-)
But I agree that we need to teach FEL about it as well and this may
break actually loading things like ATF into SRAM A2 with FEL then.

> About the 31K code size. This does not look good and is very close to
> the BROM limit (32K). Just using a different compiler may bring us into
> a trouble. Or some minor code tweaks and feature additions.

I totally agree. A nasty drawback is already that I can't enable debug.

>> Trying this in the past (with libdram) and compiling for (32-bit) Thumb2
>> worked, but I need to check what the actual size with Jens' patches are
>> these days for Thumb2.
> 
> We have already discussed this off-list a long time ago. I know that
> both you and Alexander Graf are generally in favour of compiling the
> SPL as 64-bit code.
> 
> I think that this is the usual case of utility versus fashion. Everyone
> wants to plug every hole with 64-bit ARM code right now just because it
> is new and innovative. But this fad will fade away in a few years. Now
> just imagine an alternative reality, where ARM64 is an old and boring
> thing, while Thumb2 is a recent invention to improve code density in
> microcontrollers and other code space constrained systems. I'm sure
> that everyone would be trying to find a way to replace the legacy
> bloated 64-bit ARM code in the SPL with the new and shiny Thumb2
> stuff for improving code density ;-)
> 
> If we take a pragmatic approach and try to evaluate pro- and cons-
> factors, then we can see that the 64-bit code in the SPL on
> Allwinner A64 hardware does not give us any real improvements.
> Quite the contrary: it offers worse code density than 32-bit
> Thumb2 and also a functional USB FEL boot support becomes much
> more tricky (because the boot ROM implements FEL as a 32-bit code).

Well, when we did 64-bit SPL experiments earlier this year, we found the
code size to be quite reasonable (around 20-24K?), but apparently this
was missing some stuff (FIT for instance, which includes some libfdt
code, or SPI flash).

Now contrary to your apparent belief I am not married to aarch64 ;-)
Actually I started yesterday with going back to a 32-bit SPL, and the
code size is amazingly small there, up to a point where I wonder if
there is some bug somewhere in the build which either optimizes armv7
more or includes unneeded stuff in armv8. I need to investigate this.

So after I spend an evening with cutting of bytes from the 64-bit build
(don't link ccn and GIC code, creating tiny-ctype, only instantiating
two MMC devices, ...) I am quite open to the idea of going with a 32-bit
SPL ;-)

> The only real argument in favour of having a 64-bit SPL is that we
> can use a single AArch64 toolchain to build both the SPL and the
> main U-Boot. And we are in this situation only because the AArch64
> toolchain does not support the "-m32" option. There is no technical
> justification for this.

Well, there is. In contrast to other both 32 and 64-bit capable
architectures the ISA is quite different between the both: assembly
isn't compatible (w0 vs. r0), the encoding is _totally_ different, many
instructions are different, different capabilities of relative
addressing and immediate encoding (you can't orr w0, w0, #0x28000, for
instance ;-)
Yes, they also share many ideas and the assembly bears some
similarities, so in the end I guess there are an equal number of
arguments in favour and against this. Plus the GCC and binutils code
base is reportedly not in a shape which would encourage such mergers ;-)

> ARM decided to be different just for the
> sake of being different (every other architecture has the -m32
> option in GCC if the processor is able to work in both modes).

As mentioned above this isn't entirely true, and I think there were
quite some discussions about that - and there still are (I heard
something about -m32 over lunch a few months back).
Feel free to bring this up again on the respective toolchain mailing
lists ;-)

> If the "-m32" option was supported, then building a 32-bit SPL
> would have been mostly a trivial matter of adding "-m32 -mthumb"
> options to CFLAGS.
> 
> But we can try to do a 32-bit SPL build by introducing something like
> a CROSS_COMPILE_SPL environment variable, just like suggested some
> time ago: http://lists.denx.de/pipermail/u-boot/2012-April/122236.html

Which seemed to have been shut down by Wolfgang ;-), though with
arguments not really applying to our case.
That being said, I remember having written a small wrapper script back
when we had this discussion, which scans for a -m32 option in the
command line and calling the arm(32) compiler (with the -m32 removed)
then - or passing everything to the aarch64 cross compiler.
This was surprisingly simple, I wonder if this could be integrated into
the (sunxi) U-Boot build environment somehow.
Still it would require people to have two cross-compilers installed - or
(probably more annoying) to have a cross-compiler in the first place
even if one compiles natively.

> Also I'm finally going to submit the runtime SPL code decompression
> patches for the next U-Boot release, because there is no need to delay
> the implementation of this feature any longer. Yes, I know that any
> saved space will be wasted almost instantly by various gimmicks, but
> that's just how it is.

Great! I was wondering about the state of that. Can you give some
ballpark figures already?

>> Anyway, thanks for your patch, I will try tonight if I can squeeze all
>> the bits in.
> 
> If you mean https://patchwork.ozlabs.org/patch/665608/ then it only
> gives us the freedom to move CONFIG_SYS_INIT_SP_ADDR somewhere else.
> 
> And moving the initial stack if the U-Boot proper into the DRAM would
> make a lot of sense.

Oh, I was under the impression that it would that already?

> We only need to agree what kind of DRAM address to
> use. After all, even the SPL relocates the stack into the DRAM. Why
> does the U-Boot proper want to use the SRAM for its stack again?
> 

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-09-08 13:23 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-05  0:32 [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Andre Przywara
2016-09-05  0:32 ` [U-Boot] [PATCH 1/4] Revert "sunxi: Move the SPL stack top to 0x1A000 on Allwinner A64/A80" Andre Przywara
2016-09-05  4:12   ` Siarhei Siamashka
2016-09-05  8:23     ` Andre Przywara
2016-09-08 10:51       ` Siarhei Siamashka
2016-09-08 13:23         ` Andre Przywara
2016-09-05  0:32 ` [U-Boot] [PATCH 2/4] Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64" Andre Przywara
2016-09-05  4:48   ` Siarhei Siamashka
2016-09-05  8:32     ` Andre Przywara
2016-09-05  0:32 ` [U-Boot] [PATCH 3/4] sunxi: Kconfig: rename non-existent SUN50I_A64 config symbol Andre Przywara
2016-09-05  0:32 ` [U-Boot] [PATCH 4/4] sunxi: fix 64-bit compiler warning for SPL header parsing Andre Przywara
2016-09-05  5:00   ` Siarhei Siamashka
2016-09-07 15:13 ` [U-Boot] [PATCH 0/4] sunxi: Pine64 fixes Hans de Goede
2016-09-07 15:18   ` Andre Przywara
2016-09-07 15:48     ` Hans de Goede
2016-09-07 15:55       ` Andre Przywara
2016-09-07 16:12         ` Tom Rini
2016-09-07 16:20           ` Andre Przywara
2016-09-08  8:26         ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.