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

Ben Gardiner
2010-07-13T21:17:42Z

On Tue, Jul 13, 2010 at 3:26 PM, Scott Wood <scottwood@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@openmoko.org> with minor modifications for rebase and changes
>> as suggested by Scott Wood <scottwood@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@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.