[2/3] vexpress64: remove #error

Message ID 1445421247-15013-3-git-send-email-ryan.harkin@linaro.org
State Superseded
Headers show

Commit Message

Ryan Harkin Oct. 21, 2015, 9:54 a.m.
This patch allows vexpress64 targets to be compiled when
CONFIG_SYS_FLASH_CFI is enabled.

I considered using #warning instead of #error, but this just clutters up
the build output and hides real warnings.

Without this patch, you see errors during compilation like this:

include/configs/vexpress_aemv8a.h:42:2: error: #error "Unknown board
variant"
 #error "Unknown board variant"
include/configs/vexpress_aemv8a.h:115:2: error: #error "Unknown board
variant"
 #error "Unknown board variant"
include/configs/vexpress_aemv8a.h:280:2: error: #error "Unknown board
variant"
 #error "Unknown board variant"
make[1]: *** [tools/envcrc.o] Error 1
make: *** [tools] Error 2
In file included from include/config.h:5:0,
                 from tools/envcrc.c:19:

Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
---
 include/configs/vexpress_aemv8a.h | 6 ------
 1 file changed, 6 deletions(-)

Comments

Linus Walleij Oct. 27, 2015, 11:36 a.m. | #1
On Wed, Oct 21, 2015 at 11:54 AM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

> This patch allows vexpress64 targets to be compiled when

> CONFIG_SYS_FLASH_CFI is enabled.


I can't see what this has to do with enabling CFI?

> I considered using #warning instead of #error, but this just clutters up

> the build output and hides real warnings.

>

> Without this patch, you see errors during compilation like this:

>

> include/configs/vexpress_aemv8a.h:42:2: error: #error "Unknown board

> variant"

>  #error "Unknown board variant"


The #error is there because noone could answer the question I
posed: what AEMv8 boards are there that are neither FVP nor
Juno?

So what board variant are you compiling for here? I suspect a
non-upstream thingofabob? Maybe there is a better way to cater
for that than this magic catch-all?

Yours,
Linus Walleij
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Ryan Harkin Oct. 27, 2015, 12:29 p.m. | #2
On 27 October 2015 at 11:36, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Oct 21, 2015 at 11:54 AM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>

>> This patch allows vexpress64 targets to be compiled when

>> CONFIG_SYS_FLASH_CFI is enabled.

>

> I can't see what this has to do with enabling CFI?


The errors happen when I enable CFI on Juno or FVP.  Enabling CFI
support includes envcrc.c into the build, which then includes
include/config.h, which in turn includes
include/configs/vexpress_aemv8a.h, but without a board defined,
despite building a Juno or FVP configuration.  I don't really know why
the board isn't defined at that point.

Looking deeper into why envcrc is included into the build, it was
another #define that included it.

I can see that tools/Makefile adds envcrc if CONFIG_BUILD_ENVCRC is
defined, which in turn is enabled if ENVCRC-y is defined, which seems
to happen if CONFIG_ENV_IS_IN_FLASH is defined.

So my comment is wrong.

>

>> I considered using #warning instead of #error, but this just clutters up

>> the build output and hides real warnings.

>>

>> Without this patch, you see errors during compilation like this:

>>

>> include/configs/vexpress_aemv8a.h:42:2: error: #error "Unknown board

>> variant"

>>  #error "Unknown board variant"

>

> The #error is there because noone could answer the question I

> posed: what AEMv8 boards are there that are neither FVP nor

> Juno?


AEMv8 may be the wrong term anyway.  I *think* AEMv8 really only
refers to one flavour of the FVP:  AEMv8 is an representation of the
complete ARMv8 architecture, not of a specific CPU variant such as
Cortex A57, which omits some of the arch.

Everything we're including in this vexpress_aemv8a file is really more
like just vexpress64, with variants inside it for Juno and FVP.

And with a bit of code to detect and handle the platform differences,
we could probably create a single binary that works on everything
covered by this config.  Although I think the #define for the CFI base
address could be a sticking point there.

Currently, ARM have:

Models:
- FVP Foundation models
- FVP AEMv8 models
- FVP Cortex A57/A53 models

^ the same binary runs on all three, although I never test on Cortex
models.  There is a semihosting variant and I've created a DRAM
variant.

Real boards:
- Juno R0
- Juno R1

^ the same binary runs on both

There are no other public platforms, to my knowledge.  ARM have some
internal models that have different peripheral sets, CPU
configurations, etc... but these aren't covered by the public code at
all.

>

> So what board variant are you compiling for here? I suspect a

> non-upstream thingofabob? Maybe there is a better way to cater

> for that than this magic catch-all?


I was building for Juno.


>

> 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:11 p.m. | #3
Hi,

This is a ping to revive this patch and the next one in the series:
"[PATCH 3/3] vexpress64: store env in flash"

Patch 1/3 was resent separately and has been merged.

This patch (and the subsequent one in the series) hasn't been
Reviewed-by or Acked-by anyone, but in a thread for patch 3/3 in this
series, Tom Rini and I had this conversation:

Tom> The host tools are not board-independent as they include a copy of the
Tom> target board env.  Keep that in mind.

Me> So that means we can't use #error in the target board include file
Me> (eg. vexpress_aemv8a.h) to indicate that no board was set, correct?

Tom> Correct.

So if Juno wants to use the NOR flash and therefore, the host tools,
then first we need this patch to remove the #error statements.

Thanks,
Ryan.


On 27 October 2015 at 12:29, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 27 October 2015 at 11:36, Linus Walleij <linus.walleij@linaro.org> wrote:

>> On Wed, Oct 21, 2015 at 11:54 AM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>>

>>> This patch allows vexpress64 targets to be compiled when

>>> CONFIG_SYS_FLASH_CFI is enabled.

>>

>> I can't see what this has to do with enabling CFI?

>

> The errors happen when I enable CFI on Juno or FVP.  Enabling CFI

> support includes envcrc.c into the build, which then includes

> include/config.h, which in turn includes

> include/configs/vexpress_aemv8a.h, but without a board defined,

> despite building a Juno or FVP configuration.  I don't really know why

> the board isn't defined at that point.

>

> Looking deeper into why envcrc is included into the build, it was

> another #define that included it.

>

> I can see that tools/Makefile adds envcrc if CONFIG_BUILD_ENVCRC is

> defined, which in turn is enabled if ENVCRC-y is defined, which seems

> to happen if CONFIG_ENV_IS_IN_FLASH is defined.

>

> So my comment is wrong.

>

>>

>>> I considered using #warning instead of #error, but this just clutters up

>>> the build output and hides real warnings.

>>>

>>> Without this patch, you see errors during compilation like this:

>>>

>>> include/configs/vexpress_aemv8a.h:42:2: error: #error "Unknown board

>>> variant"

>>>  #error "Unknown board variant"

>>

>> The #error is there because noone could answer the question I

>> posed: what AEMv8 boards are there that are neither FVP nor

>> Juno?

>

> AEMv8 may be the wrong term anyway.  I *think* AEMv8 really only

> refers to one flavour of the FVP:  AEMv8 is an representation of the

> complete ARMv8 architecture, not of a specific CPU variant such as

> Cortex A57, which omits some of the arch.

>

> Everything we're including in this vexpress_aemv8a file is really more

> like just vexpress64, with variants inside it for Juno and FVP.

>

> And with a bit of code to detect and handle the platform differences,

> we could probably create a single binary that works on everything

> covered by this config.  Although I think the #define for the CFI base

> address could be a sticking point there.

>

> Currently, ARM have:

>

> Models:

> - FVP Foundation models

> - FVP AEMv8 models

> - FVP Cortex A57/A53 models

>

> ^ the same binary runs on all three, although I never test on Cortex

> models.  There is a semihosting variant and I've created a DRAM

> variant.

>

> Real boards:

> - Juno R0

> - Juno R1

>

> ^ the same binary runs on both

>

> There are no other public platforms, to my knowledge.  ARM have some

> internal models that have different peripheral sets, CPU

> configurations, etc... but these aren't covered by the public code at

> all.

>

>>

>> So what board variant are you compiling for here? I suspect a

>> non-upstream thingofabob? Maybe there is a better way to cater

>> for that than this magic catch-all?

>

> I was building for Juno.

>

>

>>

>> Yours,

>> Linus Walleij

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Linus Walleij Nov. 17, 2015, 1:36 p.m. | #4
On Wed, Nov 11, 2015 at 5:11 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

> This patch (and the subsequent one in the series) hasn't been

> Reviewed-by or Acked-by anyone, but in a thread for patch 3/3 in this

> series, Tom Rini and I had this conversation:

>

> Tom> The host tools are not board-independent as they include a copy of the

> Tom> target board env.  Keep that in mind.

>

> Me> So that means we can't use #error in the target board include file

> Me> (eg. vexpress_aemv8a.h) to indicate that no board was set, correct?

>

> Tom> Correct.

>

> So if Juno wants to use the NOR flash and therefore, the host tools,

> then first we need this patch to remove the #error statements.


Sorry, been busy.
Acked-by: Linus Walleij <linus.walleij@linaro.org>


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

Patch

diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h
index 0f2f1a3..c014c14 100644
--- a/include/configs/vexpress_aemv8a.h
+++ b/include/configs/vexpress_aemv8a.h
@@ -38,8 +38,6 @@ 
 #elif CONFIG_TARGET_VEXPRESS64_JUNO
 #define CONFIG_SYS_TEXT_BASE		0xe0000000
 #define CONFIG_SYS_INIT_SP_ADDR         (CONFIG_SYS_SDRAM_BASE + 0x7fff0)
-#else
-#error "Unknown board variant"
 #endif
 
 #define CONFIG_SYS_BOOTM_LEN (64 << 20)      /* Increase max gunzip size */
@@ -111,8 +109,6 @@ 
 #elif CONFIG_TARGET_VEXPRESS64_JUNO
 #define GICD_BASE			(0x2C010000)
 #define GICC_BASE			(0x2C02f000)
-#else
-#error "Unknown board variant"
 #endif
 #endif /* !CONFIG_GICV3 */
 
@@ -276,8 +272,6 @@ 
 
 #define CONFIG_BOOTDELAY		1
 
-#else
-#error "Unknown board variant"
 #endif
 
 /* Do not preserve environment */