diff mbox

[U-Boot,5/8] tegra2: Enable SPI environment on Seaboard

Message ID 1319137409-4132-6-git-send-email-sjg@chromium.org
State New, archived
Headers show

Commit Message

Simon Glass Oct. 20, 2011, 7:03 p.m. UTC
This uses the SPI flash on Seaboard to store an 8KB environment.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 include/configs/harmony.h       |    3 +++
 include/configs/seaboard.h      |    6 ++++++
 include/configs/tegra2-common.h |    3 +--
 3 files changed, 10 insertions(+), 2 deletions(-)

Comments

Tom Warren Oct. 20, 2011, 7:31 p.m. UTC | #1
Simon,

> -----Original Message-----
> From: Simon Glass [mailto:sjg@chromium.org]
> Sent: Thursday, October 20, 2011 12:03 PM
> To: U-Boot Mailing List
> Cc: Albert ARIBAUD; Tom Warren; Stephen Warren; Simon Glass
> Subject: [PATCH 5/8] tegra2: Enable SPI environment on Seaboard
> 
> This uses the SPI flash on Seaboard to store an 8KB environment.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Great to see these patches going up!

> ---
>  include/configs/harmony.h       |    3 +++
>  include/configs/seaboard.h      |    6 ++++++
>  include/configs/tegra2-common.h |    3 +--
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/configs/harmony.h b/include/configs/harmony.h
> index 89e4911..ce0ae9f 100644
> --- a/include/configs/harmony.h
> +++ b/include/configs/harmony.h
> @@ -58,4 +58,7 @@
>  #define CONFIG_EFI_PARTITION
>  #define CONFIG_CMD_EXT2
>  #define CONFIG_CMD_FAT
> +
> +/* Environment not stored */
> +#define CONFIG_ENV_IS_NOWHERE
>  #endif /* __CONFIG_H */
> diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
> index 7e8c8cc..bd83a84 100644
> --- a/include/configs/seaboard.h
> +++ b/include/configs/seaboard.h
> @@ -63,4 +63,10 @@
>  #define CONFIG_EFI_PARTITION
>  #define CONFIG_CMD_EXT2
>  #define CONFIG_CMD_FAT
> +
> +/* Environment in SPI */
> +#define CONFIG_ENV_IS_IN_SPI_FLASH
> +
> +#define CONFIG_ENV_SECT_SIZE    CONFIG_ENV_SIZE
> +#define CONFIG_ENV_OFFSET       ((4 << 20) - CONFIG_ENV_SECT_SIZE)

First, why not use a SZ_4M equate here? 4 << 20 is a bit hard to decode on a quick read.
Second, you are assuming here that all SPI chips will be 4MB/32Mbit. While that's true on
All extant Seaboards, it may not always be true. Maybe we should provide the expected size
in a #define in the SPI section in seaboard.h?

>  #endif /* __CONFIG_H */
> diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-
> common.h
> index 73e0f05..2dd6fa4 100644
> --- a/include/configs/tegra2-common.h
> +++ b/include/configs/tegra2-common.h
> @@ -50,8 +50,7 @@
>  #define CONFIG_OF_LIBFDT		/* enable passing of devicetree */
> 
>  /* Environment */
> -#define CONFIG_ENV_IS_NOWHERE
> -#define CONFIG_ENV_SIZE			0x20000	/* Total Size
> Environment */
> +#define CONFIG_ENV_SIZE			0x2000	/* Total Size Environment */

Same deal here - using SZ_8K just reads better to me.

> 
>  /*
>   * Size of malloc() pool
> --
> 1.7.3.1
 
Thanks,

Tom

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
Simon Glass Oct. 20, 2011, 7:58 p.m. UTC | #2
Hi Tom,

On Thu, Oct 20, 2011 at 12:31 PM, Tom Warren <TWarren@nvidia.com> wrote:
> Simon,
>
>> -----Original Message-----
>> From: Simon Glass [mailto:sjg@chromium.org]
>> Sent: Thursday, October 20, 2011 12:03 PM
>> To: U-Boot Mailing List
>> Cc: Albert ARIBAUD; Tom Warren; Stephen Warren; Simon Glass
>> Subject: [PATCH 5/8] tegra2: Enable SPI environment on Seaboard
>>
>> This uses the SPI flash on Seaboard to store an 8KB environment.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Great to see these patches going up!

Yes - testing is a bit tricky at present since we are so many patches
behind. I will send a summary email to the list with what I think are
the outstanding Tegra patches.

>
>> ---
>>  include/configs/harmony.h       |    3 +++
>>  include/configs/seaboard.h      |    6 ++++++
>>  include/configs/tegra2-common.h |    3 +--
>>  3 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/configs/harmony.h b/include/configs/harmony.h
>> index 89e4911..ce0ae9f 100644
>> --- a/include/configs/harmony.h
>> +++ b/include/configs/harmony.h
>> @@ -58,4 +58,7 @@
>>  #define CONFIG_EFI_PARTITION
>>  #define CONFIG_CMD_EXT2
>>  #define CONFIG_CMD_FAT
>> +
>> +/* Environment not stored */
>> +#define CONFIG_ENV_IS_NOWHERE
>>  #endif /* __CONFIG_H */
>> diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
>> index 7e8c8cc..bd83a84 100644
>> --- a/include/configs/seaboard.h
>> +++ b/include/configs/seaboard.h
>> @@ -63,4 +63,10 @@
>>  #define CONFIG_EFI_PARTITION
>>  #define CONFIG_CMD_EXT2
>>  #define CONFIG_CMD_FAT
>> +
>> +/* Environment in SPI */
>> +#define CONFIG_ENV_IS_IN_SPI_FLASH
>> +
>> +#define CONFIG_ENV_SECT_SIZE    CONFIG_ENV_SIZE
>> +#define CONFIG_ENV_OFFSET       ((4 << 20) - CONFIG_ENV_SECT_SIZE)
>
> First, why not use a SZ_4M equate here? 4 << 20 is a bit hard to decode on a quick read.

While I may (or may not) agree with you, I didn't want to annoy
Wolfgang who doesn't like the SZ macros.

> Second, you are assuming here that all SPI chips will be 4MB/32Mbit. While that's true on
> All extant Seaboards, it may not always be true. Maybe we should provide the expected size
> in a #define in the SPI section in seaboard.h?

Yes I will add a SPI flash size into seaboard.h, thanks.

Regards,
Simon

>
>>  #endif /* __CONFIG_H */
>> diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-
>> common.h
>> index 73e0f05..2dd6fa4 100644
>> --- a/include/configs/tegra2-common.h
>> +++ b/include/configs/tegra2-common.h
>> @@ -50,8 +50,7 @@
>>  #define CONFIG_OF_LIBFDT             /* enable passing of devicetree */
>>
>>  /* Environment */
>> -#define CONFIG_ENV_IS_NOWHERE
>> -#define CONFIG_ENV_SIZE                      0x20000 /* Total Size
>> Environment */
>> +#define CONFIG_ENV_SIZE                      0x2000  /* Total Size Environment */
>
> Same deal here - using SZ_8K just reads better to me.
>
>>
>>  /*
>>   * Size of malloc() pool
>> --
>> 1.7.3.1
>
> Thanks,
>
> Tom
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
>
Tom Warren Oct. 20, 2011, 8:49 p.m. UTC | #3
Simon,

> -----Original Message-----
> From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass
> Sent: Thursday, October 20, 2011 12:58 PM
> To: Tom Warren
> Cc: U-Boot Mailing List; Albert ARIBAUD; Stephen Warren
> Subject: Re: [PATCH 5/8] tegra2: Enable SPI environment on Seaboard
> 
> Hi Tom,
> 
> On Thu, Oct 20, 2011 at 12:31 PM, Tom Warren <TWarren@nvidia.com> wrote:
> > Simon,
> >
> >> -----Original Message-----
> >> From: Simon Glass [mailto:sjg@chromium.org]
> >> Sent: Thursday, October 20, 2011 12:03 PM
> >> To: U-Boot Mailing List
> >> Cc: Albert ARIBAUD; Tom Warren; Stephen Warren; Simon Glass
> >> Subject: [PATCH 5/8] tegra2: Enable SPI environment on Seaboard
> >>
> >> This uses the SPI flash on Seaboard to store an 8KB environment.
> >>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > Great to see these patches going up!
> 
> Yes - testing is a bit tricky at present since we are so many patches
> behind. I will send a summary email to the list with what I think are
> the outstanding Tegra patches.
> 
> >
> >> ---
> >>  include/configs/harmony.h       |    3 +++
> >>  include/configs/seaboard.h      |    6 ++++++
> >>  include/configs/tegra2-common.h |    3 +--
> >>  3 files changed, 10 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/configs/harmony.h b/include/configs/harmony.h
> >> index 89e4911..ce0ae9f 100644
> >> --- a/include/configs/harmony.h
> >> +++ b/include/configs/harmony.h
> >> @@ -58,4 +58,7 @@
> >>  #define CONFIG_EFI_PARTITION
> >>  #define CONFIG_CMD_EXT2
> >>  #define CONFIG_CMD_FAT
> >> +
> >> +/* Environment not stored */
> >> +#define CONFIG_ENV_IS_NOWHERE
> >>  #endif /* __CONFIG_H */
> >> diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
> >> index 7e8c8cc..bd83a84 100644
> >> --- a/include/configs/seaboard.h
> >> +++ b/include/configs/seaboard.h
> >> @@ -63,4 +63,10 @@
> >>  #define CONFIG_EFI_PARTITION
> >>  #define CONFIG_CMD_EXT2
> >>  #define CONFIG_CMD_FAT
> >> +
> >> +/* Environment in SPI */
> >> +#define CONFIG_ENV_IS_IN_SPI_FLASH
> >> +
> >> +#define CONFIG_ENV_SECT_SIZE    CONFIG_ENV_SIZE
> >> +#define CONFIG_ENV_OFFSET       ((4 << 20) - CONFIG_ENV_SECT_SIZE)
> >
> > First, why not use a SZ_4M equate here? 4 << 20 is a bit hard to decode on
> a quick read.
> 
> While I may (or may not) agree with you, I didn't want to annoy
> Wolfgang who doesn't like the SZ macros.

OK, sorry. Didn't know Wolfgang wasn't a fan of the SZ_ equates. Note that they are not
macros, but defines/equates. No magic, no side-effects (see size.h). I still like them
a whole lot better than x << y or 0x002000, etc. Too easy to use one-too-many (or too-few)
zeroes and get the wrong value. You'll see 'em used in my T30 config files, etc.

> 
> > Second, you are assuming here that all SPI chips will be 4MB/32Mbit. While
> that's true on
> > All extant Seaboards, it may not always be true. Maybe we should provide
> the expected size
> > in a #define in the SPI section in seaboard.h?
> 
> Yes I will add a SPI flash size into seaboard.h, thanks.
> 

Cool. Thanks.

Tom

> Regards,
> Simon
> 
> >
> >>  #endif /* __CONFIG_H */
> >> diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-
> >> common.h
> >> index 73e0f05..2dd6fa4 100644
> >> --- a/include/configs/tegra2-common.h
> >> +++ b/include/configs/tegra2-common.h
> >> @@ -50,8 +50,7 @@
> >>  #define CONFIG_OF_LIBFDT             /* enable passing of devicetree */
> >>
> >>  /* Environment */
> >> -#define CONFIG_ENV_IS_NOWHERE
> >> -#define CONFIG_ENV_SIZE                      0x20000 /* Total Size
> >> Environment */
> >> +#define CONFIG_ENV_SIZE                      0x2000  /* Total Size
> Environment */
> >
> > Same deal here - using SZ_8K just reads better to me.
> >
> >>
> >>  /*
> >>   * Size of malloc() pool
> >> --
> >> 1.7.3.1
> >
> > Thanks,
> >
> > Tom
> >
> > --------------------------------------------------------------------------
> ---------
> > This email message is for the sole use of the intended recipient(s) and
> may contain
> > confidential information.  Any unauthorized review, use, disclosure or
> distribution
> > is prohibited.  If you are not the intended recipient, please contact the
> sender by
> > reply email and destroy all copies of the original message.
> > --------------------------------------------------------------------------
> ---------
> >
diff mbox

Patch

diff --git a/include/configs/harmony.h b/include/configs/harmony.h
index 89e4911..ce0ae9f 100644
--- a/include/configs/harmony.h
+++ b/include/configs/harmony.h
@@ -58,4 +58,7 @@ 
 #define CONFIG_EFI_PARTITION
 #define CONFIG_CMD_EXT2
 #define CONFIG_CMD_FAT
+
+/* Environment not stored */
+#define CONFIG_ENV_IS_NOWHERE
 #endif /* __CONFIG_H */
diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
index 7e8c8cc..bd83a84 100644
--- a/include/configs/seaboard.h
+++ b/include/configs/seaboard.h
@@ -63,4 +63,10 @@ 
 #define CONFIG_EFI_PARTITION
 #define CONFIG_CMD_EXT2
 #define CONFIG_CMD_FAT
+
+/* Environment in SPI */
+#define CONFIG_ENV_IS_IN_SPI_FLASH
+
+#define CONFIG_ENV_SECT_SIZE    CONFIG_ENV_SIZE
+#define CONFIG_ENV_OFFSET       ((4 << 20) - CONFIG_ENV_SECT_SIZE)
 #endif /* __CONFIG_H */
diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-common.h
index 73e0f05..2dd6fa4 100644
--- a/include/configs/tegra2-common.h
+++ b/include/configs/tegra2-common.h
@@ -50,8 +50,7 @@ 
 #define CONFIG_OF_LIBFDT		/* enable passing of devicetree */
 
 /* Environment */
-#define CONFIG_ENV_IS_NOWHERE
-#define CONFIG_ENV_SIZE			0x20000	/* Total Size Environment */
+#define CONFIG_ENV_SIZE			0x2000	/* Total Size Environment */
 
 /*
  * Size of malloc() pool