[v2] vexpress64: use 2nd DRAM bank only on juno

Message ID 1445857222-13294-1-git-send-email-ryan.harkin@linaro.org
State Superseded
Headers show

Commit Message

Ryan Harkin Oct. 26, 2015, 11 a.m.
This patch makes the 2nd DRAM bank available on Juno only and not on
other vexpress64 targets, eg. the FVP models.

The commit below added a 2nd bank of NOR flash for Juno, but also for
all vexpress64 targets:

    commit 2d0cee1ca2b9d977fa3214896bb2e30cfec77059
    Author: Liviu Dudau <Liviu.Dudau@foss.arm.com>
    Date:   Mon Oct 19 11:08:31 2015 +0100

    vexpress64: Juno: Declare all 8GB of RAM and make them visible to the kernel.

    Juno comes with 8GB RAM, but U-Boot only passes 2GB to the kernel.
    Declare a secondary memory bank and set the sizes correctly.

    Signed-off-by: Liviu Dudau <Liviu.Dudau@foss.arm.com>

    Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

    Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org>

    Tested-by: Ryan Harkin <ryan.harkin@linaro.org>


Unfortunately, I only fully tested on Juno R0, R1 and the FVP Foundation
model.  Whilst FVP Base AEMV8 models run U-Boot OK, they fail to boot
the kernel.

Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>

Acked-by: Liviu Dudau <liviu.dudau@foss.arm.com>

---
 board/armltd/vexpress64/vexpress64.c |  2 ++
 include/configs/vexpress_aemv8a.h    | 11 ++++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

-- 
2.1.4

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

Comments

Linus Walleij Oct. 27, 2015, 12:10 p.m. | #1
On Mon, Oct 26, 2015 at 12:00 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

> This patch makes the 2nd DRAM bank available on Juno only and not on

> other vexpress64 targets, eg. the FVP models.

>

> The commit below added a 2nd bank of NOR flash for Juno, but also for

> all vexpress64 targets:

>

>     commit 2d0cee1ca2b9d977fa3214896bb2e30cfec77059

>     Author: Liviu Dudau <Liviu.Dudau@foss.arm.com>

>     Date:   Mon Oct 19 11:08:31 2015 +0100

>

>     vexpress64: Juno: Declare all 8GB of RAM and make them visible to the kernel.

>

>     Juno comes with 8GB RAM, but U-Boot only passes 2GB to the kernel.

>     Declare a secondary memory bank and set the sizes correctly.

>

>     Signed-off-by: Liviu Dudau <Liviu.Dudau@foss.arm.com>

>     Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

>     Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org>

>     Tested-by: Ryan Harkin <ryan.harkin@linaro.org>

>

> Unfortunately, I only fully tested on Juno R0, R1 and the FVP Foundation

> model.  Whilst FVP Base AEMV8 models run U-Boot OK, they fail to boot

> the kernel.

>

> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>

> Acked-by: Liviu Dudau <liviu.dudau@foss.arm.com>


Reviewed-by: Linus Walleij <linus.walleij@linaro.org>


This relates to the discussion about "unknown board variants"
that we discuss in another thread, but let's keep that there.

Yours,
Linus Walleij
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Ryan Harkin Nov. 11, 2015, 4:03 p.m. | #2
Hi,

This is just a ping in case this patch has been forgotten.  It was
Acked and Reviewed.

Thanks,
Ryan.


On 27 October 2015 at 12:10, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Oct 26, 2015 at 12:00 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>

>> This patch makes the 2nd DRAM bank available on Juno only and not on

>> other vexpress64 targets, eg. the FVP models.

>>

>> The commit below added a 2nd bank of NOR flash for Juno, but also for

>> all vexpress64 targets:

>>

>>     commit 2d0cee1ca2b9d977fa3214896bb2e30cfec77059

>>     Author: Liviu Dudau <Liviu.Dudau@foss.arm.com>

>>     Date:   Mon Oct 19 11:08:31 2015 +0100

>>

>>     vexpress64: Juno: Declare all 8GB of RAM and make them visible to the kernel.

>>

>>     Juno comes with 8GB RAM, but U-Boot only passes 2GB to the kernel.

>>     Declare a secondary memory bank and set the sizes correctly.

>>

>>     Signed-off-by: Liviu Dudau <Liviu.Dudau@foss.arm.com>

>>     Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

>>     Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org>

>>     Tested-by: Ryan Harkin <ryan.harkin@linaro.org>

>>

>> Unfortunately, I only fully tested on Juno R0, R1 and the FVP Foundation

>> model.  Whilst FVP Base AEMV8 models run U-Boot OK, they fail to boot

>> the kernel.

>>

>> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>

>> Acked-by: Liviu Dudau <liviu.dudau@foss.arm.com>

>

> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

>

> This relates to the discussion about "unknown board variants"

> that we discuss in another thread, but let's keep that there.

>

> Yours,

> Linus Walleij

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Ryan Harkin Nov. 13, 2015, 8:26 a.m. | #3
Hi Tom,

Thanks for coming back to this.

On 12 November 2015 at 20:58, Tom Rini <trini@konsulko.com> wrote:
> On Mon, Oct 26, 2015 at 11:00:22AM +0000, Ryan Harkin wrote:

>

>> This patch makes the 2nd DRAM bank available on Juno only and not on

>> other vexpress64 targets, eg. the FVP models.

>>

>> The commit below added a 2nd bank of NOR flash for Juno, but also for

>> all vexpress64 targets:

>>

>>     commit 2d0cee1ca2b9d977fa3214896bb2e30cfec77059

>>     Author: Liviu Dudau <Liviu.Dudau@foss.arm.com>

>>     Date:   Mon Oct 19 11:08:31 2015 +0100

>>

>>     vexpress64: Juno: Declare all 8GB of RAM and make them visible to the kernel.

>>

>>     Juno comes with 8GB RAM, but U-Boot only passes 2GB to the kernel.

>>     Declare a secondary memory bank and set the sizes correctly.

>>

>>     Signed-off-by: Liviu Dudau <Liviu.Dudau@foss.arm.com>

>>     Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

>>     Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org>

>>     Tested-by: Ryan Harkin <ryan.harkin@linaro.org>

>>

>> Unfortunately, I only fully tested on Juno R0, R1 and the FVP Foundation

>> model.  Whilst FVP Base AEMV8 models run U-Boot OK, they fail to boot

>> the kernel.

>>

>> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>

>> Acked-by: Liviu Dudau <liviu.dudau@foss.arm.com>

>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

>

> This breaks building of vexpress_aemv8a_semi :(


Yes, I see the problem now :(

If I apply this to pure upstream, I get this error, which I assume is
the same as yours:

board/armltd/vexpress64/pcie.c: In function 'xr3pci_setup_atr':
board/armltd/vexpress64/pcie.c:111:29: error: 'PHYS_SDRAM_2'
undeclared (first use in this function)
  xr3pci_set_atr_entry(base, PHYS_SDRAM_2, PHYS_SDRAM_2,

This error was fixed by Linus Walleij's patch that I carry in my u-boot fork:

[PATCH] vexpress64: compile Juno PCIe conditionally

I'd prefer that Linus' patch is merged, then mine, but if you don't
want to do that, I can change my patch so that PHYS_SDRAM_2 isn't used
if it isn't defined.

Cheers,
Ryan.

>

> --

> Tom

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Ryan Harkin Nov. 13, 2015, 1:36 p.m. | #4
On 13 November 2015 at 13:25, Tom Rini <trini@konsulko.com> wrote:
> On Fri, Nov 13, 2015 at 08:26:31AM +0000, Ryan Harkin wrote:

>> Hi Tom,

>>

>> Thanks for coming back to this.

>>

>> On 12 November 2015 at 20:58, Tom Rini <trini@konsulko.com> wrote:

>> > On Mon, Oct 26, 2015 at 11:00:22AM +0000, Ryan Harkin wrote:

>> >

>> >> This patch makes the 2nd DRAM bank available on Juno only and not on

>> >> other vexpress64 targets, eg. the FVP models.

>> >>

>> >> The commit below added a 2nd bank of NOR flash for Juno, but also for

>> >> all vexpress64 targets:

>> >>

>> >>     commit 2d0cee1ca2b9d977fa3214896bb2e30cfec77059

>> >>     Author: Liviu Dudau <Liviu.Dudau@foss.arm.com>

>> >>     Date:   Mon Oct 19 11:08:31 2015 +0100

>> >>

>> >>     vexpress64: Juno: Declare all 8GB of RAM and make them visible to the kernel.

>> >>

>> >>     Juno comes with 8GB RAM, but U-Boot only passes 2GB to the kernel.

>> >>     Declare a secondary memory bank and set the sizes correctly.

>> >>

>> >>     Signed-off-by: Liviu Dudau <Liviu.Dudau@foss.arm.com>

>> >>     Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

>> >>     Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org>

>> >>     Tested-by: Ryan Harkin <ryan.harkin@linaro.org>

>> >>

>> >> Unfortunately, I only fully tested on Juno R0, R1 and the FVP Foundation

>> >> model.  Whilst FVP Base AEMV8 models run U-Boot OK, they fail to boot

>> >> the kernel.

>> >>

>> >> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>

>> >> Acked-by: Liviu Dudau <liviu.dudau@foss.arm.com>

>> >> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

>> >

>> > This breaks building of vexpress_aemv8a_semi :(

>>

>> Yes, I see the problem now :(

>>

>> If I apply this to pure upstream, I get this error, which I assume is

>> the same as yours:

>>

>> board/armltd/vexpress64/pcie.c: In function 'xr3pci_setup_atr':

>> board/armltd/vexpress64/pcie.c:111:29: error: 'PHYS_SDRAM_2'

>> undeclared (first use in this function)

>>   xr3pci_set_atr_entry(base, PHYS_SDRAM_2, PHYS_SDRAM_2,

>>

>> This error was fixed by Linus Walleij's patch that I carry in my u-boot fork:

>>

>> [PATCH] vexpress64: compile Juno PCIe conditionally

>>

>> I'd prefer that Linus' patch is merged, then mine, but if you don't

>> want to do that, I can change my patch so that PHYS_SDRAM_2 isn't used

>> if it isn't defined.

>

> Didn't I ask for some changes on that PCIe patch?

>


Good point.  I thought you'd grudgingly accepted it, but I see your
last comment was:

"Yes, thanks.  I think it's just a style thing then.  We don't do a lot
of static inline nop functions, we do __weak functions in the main file
(and comment about what it should be doing in a real function) and then
provide the strong version in another file.  So just the pcie.h part
needs changing then."

I'll ping Linus about it on his thread and see if he wants to issue a
v2.  Then I'll come back to this patch later.

Thanks,
Ryan.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Linus Walleij Nov. 13, 2015, 8:23 p.m. | #5
On Fri, Nov 13, 2015 at 2:36 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>> Didn't I ask for some changes on that PCIe patch?

>>

>

> Good point.  I thought you'd grudgingly accepted it, but I see your

> last comment was:

>

> "Yes, thanks.  I think it's just a style thing then.  We don't do a lot

> of static inline nop functions, we do __weak functions in the main file

> (and comment about what it should be doing in a real function) and then

> provide the strong version in another file.  So just the pcie.h part

> needs changing then."

>

> I'll ping Linus about it on his thread and see if he wants to issue a

> v2.  Then I'll come back to this patch later.


Argh, yeah I kind of dropped that patch because I wanted it my way
but don't have energy to argue about it...

I think it is better that the preprocessor just remove code that is never
used than to have the compiler patch out functions at compile time
by marking them __weak. It certainly will compile a few cycles quicker.

Ryan if you would rather want to do it in the __weak way so as to get
things upstream, go ahead.

Yours,
Linus Walleij
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/board/armltd/vexpress64/vexpress64.c b/board/armltd/vexpress64/vexpress64.c
index f4e8084..22d7e6c 100644
--- a/board/armltd/vexpress64/vexpress64.c
+++ b/board/armltd/vexpress64/vexpress64.c
@@ -44,8 +44,10 @@  void dram_init_banksize(void)
 {
 	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
 	gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
+#ifdef PHYS_SDRAM_2
 	gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
 	gd->bd->bi_dram[1].size = PHYS_SDRAM_2_SIZE;
+#endif
 }
 
 /*
diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h
index 0f2f1a3..ef2aa50 100644
--- a/include/configs/vexpress_aemv8a.h
+++ b/include/configs/vexpress_aemv8a.h
@@ -168,15 +168,20 @@ 
 #define CONFIG_SYS_LOAD_ADDR		(V2M_BASE + 0x10000000)
 
 /* Physical Memory Map */
-#define CONFIG_NR_DRAM_BANKS		2
 #define PHYS_SDRAM_1			(V2M_BASE)	/* SDRAM Bank #1 */
-#define PHYS_SDRAM_2			(0x880000000)
 /* Top 16MB reserved for secure world use */
 #define DRAM_SEC_SIZE		0x01000000
 #define PHYS_SDRAM_1_SIZE	0x80000000 - DRAM_SEC_SIZE
-#define PHYS_SDRAM_2_SIZE	0x180000000
 #define CONFIG_SYS_SDRAM_BASE	PHYS_SDRAM_1
 
+#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
+#define CONFIG_NR_DRAM_BANKS		2
+#define PHYS_SDRAM_2			(0x880000000)
+#define PHYS_SDRAM_2_SIZE		0x180000000
+#else
+#define CONFIG_NR_DRAM_BANKS		1
+#endif
+
 /* Enable memtest */
 #define CONFIG_CMD_MEMTEST
 #define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM_1