[3/3] vexpress64: store env in flash

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

Commit Message

Ryan Harkin Oct. 21, 2015, 9:54 a.m.
Add support for storing the environment in CFI NOR flash on Juno and FVP
models.

I also removed some config values that are not used by CFI flash parts.

Juno has 1 flash part with 259 sectors.  The first 255 sectors are
0x40000 (256kb) and are followed by 4 sectors of 0x10000 (64KB).

FVP models simulate a 64MB NOR flash part at base address 0x0FFC0000.
This part has 256 x 256kb sectors.  We use the last sector to store the
environment.

To save the NOR flash to a file, the following parameters should be
passed to the model:

    -C bp.flashloader1.fname=${FILENAME}
    -C bp.flashloader1.fnameWrite=${FILENAME}

Foundation models don't simulate the NOR flash, but having NOR support
in the u-boot binary does not harm:  attempting to write to the NOR will
fail gracefully.

Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
---
 configs/vexpress_aemv8a_dram_defconfig |  1 -
 configs/vexpress_aemv8a_semi_defconfig |  1 -
 include/configs/vexpress_aemv8a.h      | 37 ++++++++++++++++++----------------
 3 files changed, 20 insertions(+), 19 deletions(-)

Comments

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

> Juno has 1 flash part with 259 sectors.  The first 255 sectors are

> 0x40000 (256kb) and are followed by 4 sectors of 0x10000 (64KB).

>

> FVP models simulate a 64MB NOR flash part at base address 0x0FFC0000.

> This part has 256 x 256kb sectors.  We use the last sector to store the

> environment.


Does this mean it appears in a AFS partition so that we see it
sitting around there in the flash with afs (no arguments)?

I'm asking because that seems to be how it work on the
RealView PB11MPCore which was based on some out-of-tree
U-Boot that Peter Pearse was maintaining at the time.

If there is not a AFS partition for the U-Boot environment,
we should take measures to create one, since the flash
is handled in AFS partitions on these machines.

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:49 p.m. | #2
Hi Linus,

On 27 October 2015 at 11:49, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Oct 21, 2015 at 11:54 AM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>

>> Juno has 1 flash part with 259 sectors.  The first 255 sectors are

>> 0x40000 (256kb) and are followed by 4 sectors of 0x10000 (64KB).

>>

>> FVP models simulate a 64MB NOR flash part at base address 0x0FFC0000.

>> This part has 256 x 256kb sectors.  We use the last sector to store the

>> environment.

>

> Does this mean it appears in a AFS partition so that we see it

> sitting around there in the flash with afs (no arguments)?


Yes and no.  But mostly no.


>

> I'm asking because that seems to be how it work on the

> RealView PB11MPCore which was based on some out-of-tree

> U-Boot that Peter Pearse was maintaining at the time.

>


That seems like a nice feature.


> If there is not a AFS partition for the U-Boot environment,

> we should take measures to create one, since the flash

> is handled in AFS partitions on these machines.

>


On FVP, afs does nothing because the simulated NOR flash will come up
empty each time.

On Juno, I have created an entry in the motherboard firmware's
images.txt for a file called blank.img that lives in the same region
as the config.  The config current consumes 1 x 64kb sector, but can
consume all 4 x 64kb sectors; blank.img spans 4 x 64kb sectors.

The main idea for blank.img is two-fold:

1) let other people who edit images.txt know that we are using that region
2) allow users to erase the config by mounting the motherboard's mass
storage device and touching blank.img.  On reboot, the firmware will
see the timestamp update and re-write the blank.img file, this erasing
the config.  This is mostly useful because both EDK2 and U-Boot use
the same area of flash to store their config and EDK2 gets quite upset
if the config isn't what it expected it to be.

A potential problem:  If the config expands to consume all 4 x 64kb
partitions, then the CFI code's sector erase will wipe the footer that
afs uses to signal the blank.img file.

Of course, the afs command showing the area as "blank.img" is not
representative of what u-boot has done with it.

Either way, AFAIK, the existing CFI code does not handle AFS support.
I think the CFI code would need extending to write the footer if we
wanted the afs command to show the config in another way.  Could that
be what Peter Pearse did?

Thanks,
Ryan.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Linus Walleij Oct. 27, 2015, 9:39 p.m. | #3
On Tue, Oct 27, 2015 at 1:49 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

> Either way, AFAIK, the existing CFI code does not handle AFS support.

> I think the CFI code would need extending to write the footer if we

> wanted the afs command to show the config in another way.  Could that

> be what Peter Pearse did?


Uh, I'd have to dig up the code ... actually that's a very good question.

Yours,
Linus Walleij
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Ryan Harkin Oct. 28, 2015, 3:07 p.m. | #4
On 27 October 2015 at 21:39, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Oct 27, 2015 at 1:49 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>

>> Either way, AFAIK, the existing CFI code does not handle AFS support.

>> I think the CFI code would need extending to write the footer if we

>> wanted the afs command to show the config in another way.  Could that

>> be what Peter Pearse did?

>

> Uh, I'd have to dig up the code ... actually that's a very good question.

>


In the meantime, are you happy with my two patches?  One to remove the
#error lines and this one?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Linus Walleij Oct. 28, 2015, 3:10 p.m. | #5
On Wed, Oct 28, 2015 at 4:07 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 27 October 2015 at 21:39, Linus Walleij <linus.walleij@linaro.org> wrote:

>> On Tue, Oct 27, 2015 at 1:49 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>>

>>> Either way, AFAIK, the existing CFI code does not handle AFS support.

>>> I think the CFI code would need extending to write the footer if we

>>> wanted the afs command to show the config in another way.  Could that

>>> be what Peter Pearse did?

>>

>> Uh, I'd have to dig up the code ... actually that's a very good question.

>>

>

> In the meantime, are you happy with my two patches?  One to remove the

> #error lines and this one?


This one:
Acked-by: Linus Walleij <linus.walleij@linaro.org>


The other one I need to look at again, I'm not sure I understand
it or why it's needed.... was there a v2 coming?

Linus
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Ryan Harkin Oct. 28, 2015, 4:06 p.m. | #6
On 28 October 2015 at 15:10, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Oct 28, 2015 at 4:07 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>> On 27 October 2015 at 21:39, Linus Walleij <linus.walleij@linaro.org> wrote:

>>> On Tue, Oct 27, 2015 at 1:49 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>>>

>>>> Either way, AFAIK, the existing CFI code does not handle AFS support.

>>>> I think the CFI code would need extending to write the footer if we

>>>> wanted the afs command to show the config in another way.  Could that

>>>> be what Peter Pearse did?

>>>

>>> Uh, I'd have to dig up the code ... actually that's a very good question.

>>>

>>

>> In the meantime, are you happy with my two patches?  One to remove the

>> #error lines and this one?

>

> This one:

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

>

> The other one I need to look at again, I'm not sure I understand

> it or why it's needed....


Yeah, have a look again.  I hoped my last reply explained that the
#error stuff happens when we include envcrc.c in the build.  It's
nothing to do with Juno r0/1/2 or FVP.

For reasons I don't understand, envcrc.c includes config.h which
includes vexpress64_aemv8a.h without the board #defines set at all, no
matter which board you are.  So the #error gets hit no matter if
you're building for FVP or Juno.

> was there a v2 coming?


Yes.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Linus Walleij Oct. 28, 2015, 9:43 p.m. | #7
On Wed, Oct 28, 2015 at 5:06 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

> For reasons I don't understand, envcrc.c includes config.h which

> includes vexpress64_aemv8a.h without the board #defines set at all, no

> matter which board you are.  So the #error gets hit no matter if

> you're building for FVP or Juno.


This is what we should try to avoid, we need to know why this happens :/

Yours,
Linus Walleij
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Ryan Harkin Oct. 29, 2015, 9:17 a.m. | #8
Hi Tom,

On 28 October 2015 at 22:07, Tom Rini <trini@konsulko.com> wrote:
> On Wed, Oct 28, 2015 at 10:43:41PM +0100, Linus Walleij wrote:

>> On Wed, Oct 28, 2015 at 5:06 PM, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>>

>> > For reasons I don't understand, envcrc.c includes config.h which

>> > includes vexpress64_aemv8a.h without the board #defines set at all, no

>> > matter which board you are.  So the #error gets hit no matter if

>> > you're building for FVP or Juno.

>>

>> This is what we should try to avoid, we need to know why this happens :/

>

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

> target board env.  Keep that in mind.


So that means we can't use #error in the target board include file
(eg. vexpress_aemv8a.h) to indicate that no board was set, correct?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/configs/vexpress_aemv8a_dram_defconfig b/configs/vexpress_aemv8a_dram_defconfig
index e9fc870..a8e4daa 100644
--- a/configs/vexpress_aemv8a_dram_defconfig
+++ b/configs/vexpress_aemv8a_dram_defconfig
@@ -8,7 +8,6 @@  CONFIG_DEFAULT_DEVICE_TREE="vexpress64"
 # CONFIG_CMD_EDITENV is not set
 # CONFIG_CMD_ENV_EXISTS is not set
 # CONFIG_CMD_LOADS is not set
-# CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_FPGA is not set
 # CONFIG_CMD_ITEST is not set
 # CONFIG_CMD_SETEXPR is not set
diff --git a/configs/vexpress_aemv8a_semi_defconfig b/configs/vexpress_aemv8a_semi_defconfig
index a082d27..e899b90 100644
--- a/configs/vexpress_aemv8a_semi_defconfig
+++ b/configs/vexpress_aemv8a_semi_defconfig
@@ -10,7 +10,6 @@  CONFIG_SYS_PROMPT="VExpress64# "
 # CONFIG_CMD_EDITENV is not set
 # CONFIG_CMD_ENV_EXISTS is not set
 # CONFIG_CMD_LOADS is not set
-# CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_FPGA is not set
 # CONFIG_CMD_ITEST is not set
 # CONFIG_CMD_SETEXPR is not set
diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h
index c014c14..e795806 100644
--- a/include/configs/vexpress_aemv8a.h
+++ b/include/configs/vexpress_aemv8a.h
@@ -274,10 +274,6 @@ 
 
 #endif
 
-/* Do not preserve environment */
-#define CONFIG_ENV_IS_NOWHERE		1
-#define CONFIG_ENV_SIZE			0x1000
-
 /* Monitor Command Prompt */
 #define CONFIG_SYS_CBSIZE		512	/* Console I/O Buffer Size */
 #define CONFIG_SYS_PBSIZE		(CONFIG_SYS_CBSIZE + \
@@ -288,28 +284,35 @@ 
 #define CONFIG_CMDLINE_EDITING
 #define CONFIG_SYS_MAXARGS		64	/* max command args */
 
-/* Flash memory is available on the Juno board only */
-#ifndef CONFIG_TARGET_VEXPRESS64_JUNO
-#define CONFIG_SYS_NO_FLASH
+#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
+#define CONFIG_SYS_FLASH_BASE		0x08000000
+/* 255 x 256KiB sectors + 4 x 64KiB sectors at the end = 259 */
+#define CONFIG_SYS_MAX_FLASH_SECT	259
+/* Store environment at top of flash in the same location as blank.img */
+/* in the Juno firmware. */
+#define CONFIG_ENV_ADDR			0x0BFC0000
+#define CONFIG_ENV_SECT_SIZE		0x00010000
 #else
+#define CONFIG_SYS_FLASH_BASE		0x0C000000
+/* 256 x 256KiB sectors */
+#define CONFIG_SYS_MAX_FLASH_SECT	256
+/* Store environment at top of flash */
+#define CONFIG_ENV_ADDR			0x0FFC0000
+#define CONFIG_ENV_SECT_SIZE		0x00040000
+#endif
+
 #define CONFIG_CMD_ARMFLASH
 #define CONFIG_SYS_FLASH_CFI		1
 #define CONFIG_FLASH_CFI_DRIVER		1
 #define CONFIG_SYS_FLASH_CFI_WIDTH	FLASH_CFI_32BIT
-#define CONFIG_SYS_FLASH_BASE		0x08000000
-#define CONFIG_SYS_FLASH_SIZE		0x04000000 /* 64 MiB */
-#define CONFIG_SYS_MAX_FLASH_BANKS	2
+#define CONFIG_SYS_MAX_FLASH_BANKS	1
 
-/* Timeout values in ticks */
-#define CONFIG_SYS_FLASH_ERASE_TOUT	(2 * CONFIG_SYS_HZ) /* Erase Timeout */
-#define CONFIG_SYS_FLASH_WRITE_TOUT	(2 * CONFIG_SYS_HZ) /* Write Timeout */
-
-/* 255 0x40000 sectors + first or last sector may have 4 erase regions = 259 */
-#define CONFIG_SYS_MAX_FLASH_SECT	259		/* Max sectors */
 #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE /* use buffered writes */
 #define CONFIG_SYS_FLASH_PROTECTION	/* The devices have real protection */
 #define CONFIG_SYS_FLASH_EMPTY_INFO	/* flinfo indicates empty blocks */
+#define FLASH_MAX_SECTOR_SIZE		0x00040000
+#define CONFIG_ENV_SIZE			CONFIG_ENV_SECT_SIZE
+#define CONFIG_ENV_IS_IN_FLASH		1
 
-#endif
 
 #endif /* __VEXPRESS_AEMV8A_H */