[U-Boot] [PATCH v4] NAND: environment offset in OOB (CONFIG_ENV_OFFSET_OOB)

[Ben Gardiner]

On Tue, Jul 13, 2010 at 3:26 PM, Scott Wood <scottwood at freescale.com> wrote:
> I thought I sent this yesterday when applying the patch, but I guess not...
>
> On Mon, Jul 05, 2010 at 01:27:07PM -0400, Ben Gardiner wrote:
>> This is a re-submission of the patch by Harald Welte
>> <laforge at openmoko.org> with minor modifications for rebase and changes
>> as suggested by Scott Wood <scottwood at freescale.com> [1] [2].
>
> Applied to u-boot-nand-flash.
>
> I sent a followup patch for some minor formatting issues.

Thank you, Scott.

>> I verified the patch with checkpath.pl. The checkpatch.pl output follows:
>>
>> WARNING: suspect code indent for conditional statements (8, 14)
>> +       if (strcmp(cmd, "env.oob") == 0)
>> +             return do_nand_env_oob(cmdtp, &nand_info[0], argc - 1, argv + 1);
>
> IMHO, "indent with tabs" is more important than "max 80 characters" -- but
> both can be met nicely using a continuation line.
>
>> @@ -48,7 +48,7 @@ Configuring for da830evm board...
>>   147617           4888  295320  447825   6d551 ./u-boot
>>  Configuring for da850evm board...
>>     text           data     bss     dec     hex filename
>> - 198497          10332  296608  505437   7b65d ./u-boot
>> + 199849          10332  296612  506793   7bba9 ./u-boot
>>  Configuring for edb9301 board...
>>     text           data     bss     dec     hex filename
>>   133899           3772  213400  351071   55b5f ./u-boot
>
> Don't put patch fragments outside the patch itself, it confuses git.

Thanks for the tips -- I appreciate it.

>> ---
>>  common/cmd_nand.c     |  107 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>  common/env_nand.c     |   46 +++++++++++++++++++++
>>  include/environment.h |   21 +++++++---
>>  include/nand.h        |    9 ++++
>>  4 files changed, 176 insertions(+), 7 deletions(-)
>>
>> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
>> index ea80555..a4c67c1 100644
>> --- a/common/cmd_nand.c
>> +++ b/common/cmd_nand.c
>> @@ -4,6 +4,10 @@
>>   * (c) 1999 Machine Vision Holdings, Inc.
>>   * (c) 1999, 2000 David Woodhouse <dwmw2 at infradead.org>
>>   *
>> + * Ported 'dynenv' to 'nand env.oob' command
>> + * (C) 2010 Nanometrics, Inc.
>> + * 'dynenv' -- Dynamic environment offset in NAND OOB
>> + * (C) Copyright 2006-2007 OpenMoko, Inc.
>>   * Added 16-bit nand support
>>   * (C) 2004 Texas Instruments
>>   */
>> @@ -193,6 +197,90 @@ static void do_nand_status(nand_info_t *nand)
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_ENV_OFFSET_OOB
>> +unsigned long nand_env_oob_offset;
>
> Might want to switch to loff_t, even though arg_off_size won't actually let
> you enter a 64-bit offset yet.

Sure.  I can incorporate into the next patch series that introduces
CONFIG_ENV_OFFSET_REDUND support.

>> diff --git a/include/environment.h b/include/environment.h
>> index 203f731..fbccf6a 100644
>> --- a/include/environment.h
>> +++ b/include/environment.h
>> @@ -74,15 +74,24 @@
>>  #endif       /* CONFIG_ENV_IS_IN_FLASH */
>>
>>  #if defined(CONFIG_ENV_IS_IN_NAND)
>> -# ifndef CONFIG_ENV_OFFSET
>> -#  error "Need to define CONFIG_ENV_OFFSET when using CONFIG_ENV_IS_IN_NAND"
>> -# endif
>
>> +# if defined(CONFIG_ENV_OFFSET_OOB)
>> +#  ifdef CONFIG_ENV_OFFSET_REDUND
>> +#   error "CONFIG_ENV_OFFSET_REDUND is not supported when CONFIG_ENV_OFFSET_OOB"
>> +#   error "is set"
>> +#  endif
>
> This could be supported pretty easily if we require the redundant env range
> to come immediately after the regular env range -- no need to track separate
> offsets.

Ok. I like this option best also [1]. I'm not sure if I'll get to
providing the next patch series this week.

Best Regards,

Ben Gardiner

[1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/80675

---
Nanometrics Inc.
http://www.nanometrics.ca