All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.