public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Troy Kisky <troy.kisky@boundarydevices.com>
To: u-boot@lists.denx.de
Subject: [U-Boot-Users] [PATCH RFC] ARM: Davinci: NAND fix for large page ECC and	linux compatibility
Date: Sat, 28 Jun 2008 15:11:21 -0700	[thread overview]
Message-ID: <4866B709.7000905@boundarydevices.com> (raw)
In-Reply-To: <20080628033118.GA27177@mersenne.largestprime.net>

>>> +#define CFG_LINUX_COMPATIBLE_ECC
>>> + */
>> It seems odd that backwards compatibility requires turning *off* an  
>> option with "compatible" in the name...  I'd invert the sense of the  
>> ifdef, and have it be something like CFG_BROKEN_ECC_COMPATIBILITY.
> 
> The concern with this is people that use their own custom config
> files will need to add this #define when they upgrade. How about
> just changing the name to CFG_NEW_NAND_ECC_FORMAT then?

I like CFG_NEW_NAND_ECC_FORMAT better as well.

> #if defined(CFG_NAND_LARGEPAGE) && !defined(CFG_LINUX_COMPATIBLE_ECC)
> /* Comment this #error out only if you really really have to. */
> #error "You are using old ECC code that is broken on large page devices. See doc/README.davinci"
> #endif
> 
> This forces the user to make a choice - they'll probably curse while
> they're doing it, but they can't plead ignorance when they find
> their large page NAND isn't detecting ECC errors.
> 
I like this too. Maybe a #warning for small pages as well. Of course
both would also depend on #ifdef CFG_NAND_HW_ECC.


>> Perhaps we could use some currently unused OOB byte as a marker
>> for new/old ECC layout?
> 
> Could do, but any filesystems which use the OOB bytes might step on
> these. It also complicates the code even moreso and creates a lot
> more scenarios to test and that could go wrong.
> 
> I really do believe it should be a clean switch from one format to
> the other, for both small and large page NAND, with no run-time
> backwards compatibility. But that's just my POV.

I hope that eventually we can remove the old format, but this patch
has my ack.

Thanks Bernard

Troy

  reply	other threads:[~2008-06-28 22:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20080627141723.GA29627@mersenne.largestprime.net>
     [not found] ` <48651D80.2010506@freescale.com>
2008-06-28  3:31   ` [U-Boot-Users] [PATCH RFC] ARM: Davinci: NAND fix for large page ECC and linux compatibility Bernard Blackham
2008-06-28 22:11     ` Troy Kisky [this message]
2008-06-30 15:26     ` Scott Wood
2008-08-30 21:18       ` [U-Boot] " Hugo Villeneuve
2008-06-27  5:12 Bernard Blackham
2008-06-27  6:44 ` Jean-Christophe PLAGNIOL-VILLARD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4866B709.7000905@boundarydevices.com \
    --to=troy.kisky@boundarydevices.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox