[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[David Brownell]

I was looking at the DaVinci NAND support in current U-Boot
code (i.e. 2009.03 plus patches merged since that release),
and am puzzled by the above-named config option.

Before I submit a patch to remove it from U-Boot GIT (nothing
there enables it, and it will nastify 4-bit support), I thought
I'd see if anyone knows exactly what software it was trying to
emulate.  Differences from the current NAND driver in GIT:

 - Matches MVL 4.0 (2.6.10) and 5.0 (2.6.18) drivers in handling
   NANDx1ECC registers:  0PQR0stu maps to PsQRtu, while the NAND
   driver in mainline maps it to ~PQRstu.

 - Custom ECC layouts, "not compatible with Linux or RBL/UBL".

 - Does some very broken stuff for large page support.

The first of those I can understand; someone wrote some odd and
overly-complex ECC code way back, it worked and then got shipped.
(Although TI's U-Boot 1.2.0 uses soft ECC, instead...)

The other two look like they were experimental code that
should probably not have been merged anywhere...

- Dave

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:11 Sun 26 Apr     , David Brownell wrote:
> I was looking at the DaVinci NAND support in current U-Boot
> code (i.e. 2009.03 plus patches merged since that release),
> and am puzzled by the above-named config option.
> 
> Before I submit a patch to remove it from U-Boot GIT (nothing
> there enables it, and it will nastify 4-bit support), I thought
> I'd see if anyone knows exactly what software it was trying to
> emulate.  Differences from the current NAND driver in GIT:
maybe add it in the feature-removal-schedule.txt
will let people time to explain why they need it

After my only request will be to use the same ECC as the mainline kernel
or if someone explain why he need it add on other ECC algo

Best Regards,
J.

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20090426224036.GL32215@game.jcrosoft.org> you wrote:
> On 11:11 Sun 26 Apr     , David Brownell wrote:
> > I was looking at the DaVinci NAND support in current U-Boot
> > code (i.e. 2009.03 plus patches merged since that release),
> > and am puzzled by the above-named config option.
> > 
> > Before I submit a patch to remove it from U-Boot GIT (nothing
> > there enables it, and it will nastify 4-bit support), I thought
> > I'd see if anyone knows exactly what software it was trying to
> > emulate.  Differences from the current NAND driver in GIT:
> maybe add it in the feature-removal-schedule.txt
> will let people time to explain why they need it

Hm... I don't think this is needed.

Since there are no users of this code in U-Boot, we can as well
remove it without warning.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Heavier than air flying machines are impossible.
                    -- Lord Kelvin, President, Royal Society, c. 1895

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[Jean-Christophe PLAGNIOL-VILLARD]

On 00:51 Mon 27 Apr     , Wolfgang Denk wrote:
> Dear Jean-Christophe PLAGNIOL-VILLARD,
> 
> In message <20090426224036.GL32215@game.jcrosoft.org> you wrote:
> > On 11:11 Sun 26 Apr     , David Brownell wrote:
> > > I was looking at the DaVinci NAND support in current U-Boot
> > > code (i.e. 2009.03 plus patches merged since that release),
> > > and am puzzled by the above-named config option.
> > > 
> > > Before I submit a patch to remove it from U-Boot GIT (nothing
> > > there enables it, and it will nastify 4-bit support), I thought
> > > I'd see if anyone knows exactly what software it was trying to
> > > emulate.  Differences from the current NAND driver in GIT:
> > maybe add it in the feature-removal-schedule.txt
> > will let people time to explain why they need it
> 
> Hm... I don't think this is needed.
> 
> Since there are no users of this code in U-Boot, we can as well
> remove it without warning.
no necessarelly the boards Maintainer choose to use the other ECC but part
of the U-Boot user may need it.
So add it in the removal schedule make sense. We can evenif plan it for the next
Release.

Best Regards,
J.

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[David Brownell]

On Sunday 26 April 2009, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > > > Before I submit a patch to remove it from U-Boot GIT (nothing
> > > > there enables it, and it will nastify 4-bit support), I thought
> > > > I'd see if anyone knows exactly what software it was trying to
> > > > emulate. ?...
> > >
> > > maybe add it in the feature-removal-schedule.txt
> > > will let people time to explain why they need it
> > 
> > Hm... I don't think this is needed.
> > 
> > Since there are no users of this code in U-Boot, we can as well
> > remove it without warning.

That was my thought.  If it were important enough to keep in
*this* source base, someone would have submitted some board
that uses it.  It's badly enough broken that I don't know who
would bother using it, though; anyone trying to use it has some
kind of (non-Linux?) support nightmare already.


> no necessarelly the boards Maintainer choose to use the other ECC but part
> of the U-Boot user may need it.
> So add it in the removal schedule make sense. We can evenif plan it for the next
> Release.

I wouldn't mind doing that.


> > > After my only request will be to use the same ECC as the mainline kernel
> > > or if someone explain why he need it add on other ECC algo

Right, mainline does not use that "broken" ECC.  I can't
figure out who *does* use it, either...

- Dave

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[Hugo Villeneuve]

On Sun, 26 Apr 2009 16:56:40 -0700
David Brownell <david-b@pacbell.net> wrote:

> On Sunday 26 April 2009, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > 
> > > > > Before I submit a patch to remove it from U-Boot GIT (nothing
> > > > > there enables it, and it will nastify 4-bit support), I
> > > > > thought I'd see if anyone knows exactly what software it was
> > > > > trying to emulate. ?...
> > > >
> > > > maybe add it in the feature-removal-schedule.txt
> > > > will let people time to explain why they need it
> > > 
> > > Hm... I don't think this is needed.
> > > 
> > > Since there are no users of this code in U-Boot, we can as well
> > > remove it without warning.
> 
> That was my thought.  If it were important enough to keep in
> *this* source base, someone would have submitted some board
> that uses it.  It's badly enough broken that I don't know who
> would bother using it, though; anyone trying to use it has some
> kind of (non-Linux?) support nightmare already.
> 
> 
> > no necessarelly the boards Maintainer choose to use the other ECC
> > but part of the U-Boot user may need it.
> > So add it in the removal schedule make sense. We can evenif plan it
> > for the next Release.
> 
> I wouldn't mind doing that.
> 
> 
> > > > After my only request will be to use the same ECC as the
> > > > mainline kernel or if someone explain why he need it add on
> > > > other ECC algo
> 
> Right, mainline does not use that "broken" ECC.  I can't
> figure out who *does* use it, either...

We certainly don't use it for the SFFSDR board.

Hugo V.

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[Scott Wood]

On Mon, Apr 27, 2009 at 12:51:29AM +0200, Wolfgang Denk wrote:
> Dear Jean-Christophe PLAGNIOL-VILLARD,
> 
> In message <20090426224036.GL32215@game.jcrosoft.org> you wrote:
> > On 11:11 Sun 26 Apr     , David Brownell wrote:
> > > I was looking at the DaVinci NAND support in current U-Boot
> > > code (i.e. 2009.03 plus patches merged since that release),
> > > and am puzzled by the above-named config option.
> > > 
> > > Before I submit a patch to remove it from U-Boot GIT (nothing
> > > there enables it, and it will nastify 4-bit support), I thought
> > > I'd see if anyone knows exactly what software it was trying to
> > > emulate.  Differences from the current NAND driver in GIT:

It is for compatibility with a widely-deployed legacy ECC layout -- more
details can be found in the list archives.

> Hm... I don't think this is needed.
> 
> Since there are no users of this code in U-Boot, we can as well
> remove it without warning.

You're assuming that it's not being used by users manually enabling it in
their config.h.  It should have been a CONFIG_ rather than CONFIG_SYS_.

-Scott

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[David Brownell]

On Monday 27 April 2009, Scott Wood wrote:
> It is for compatibility with a widely-deployed legacy ECC layout -- more
> details can be found in the list archives.

See my original query, which IMO disproves that assertion.

What this option enables differs in two ways from what the
MontaVista code does.  (Speaking here of the 1-bit HW ECC.
The 4-bit support is another mess, which would be made far
worse by needing to carry the BROKEN_ECC mode.)

One could define a MONTAVISTA_COMPAT option, but it would
not AFAICT be this BROKEN_ECC since it would only differ
from the current Linux code in *one* of those three ways.
(Which has in turn also been claimed to be broken, by
mis-reporting some multi-bit errors as single-bit ones.)

Which is why I'm wondering what that original U-Boot code
for HW ECC was trying to be "compatible" with, since it
clearly wasn't MontaVista Linux ... or even the U-Boot
versions I've seen be distributed with it.

- Dave

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[Scott Wood]

David Brownell wrote:
> On Monday 27 April 2009, Scott Wood wrote:
>> It is for compatibility with a widely-deployed legacy ECC layout -- more
>> details can be found in the list archives.
> 
> See my original query, which IMO disproves that assertion.

The entire mess was presented as being for compatibility in these threads:

http://lists.denx.de/pipermail/u-boot/2008-June/036055.html
http://lists.denx.de/pipermail/u-boot/2008-August/039679.html

If some portions of it aren't actually needed for compatibility, then we 
can remove them.

Or we can remove the entire thing, if nobody cares anymore -- if anyone 
out there does care and is using this, please speak up now.

> What this option enables differs in two ways from what the
> MontaVista code does.  (Speaking here of the 1-bit HW ECC.
> The 4-bit support is another mess, which would be made far
> worse by needing to carry the BROKEN_ECC mode.)

I see no reason why new features would have to be supported on both 
sides of the ifdef.

> Which is why I'm wondering what that original U-Boot code
> for HW ECC was trying to be "compatible" with, since it
> clearly wasn't MontaVista Linux ... or even the U-Boot
> versions I've seen be distributed with it.

MV 2.6.10 was the claim.

-Scott

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[David Brownell]

On Monday 27 April 2009, Scott Wood wrote:
> David Brownell wrote:
> > On Monday 27 April 2009, Scott Wood wrote:
> >> It is for compatibility with a widely-deployed legacy ECC layout -- more
> >> details can be found in the list archives.
> > 
> > See my original query, which IMO disproves that assertion.
> 
> The entire mess was presented as being for compatibility in these threads:

The *non*BROKEN stuff is certainly compatible with the NAND
code in the DaVinci kernel GIT tree, which is now in mainline.

The BROKEN stuff is somewhat compatible with the MV kernels,
iff restricted to small-page support.  But ... the U-Boot with
which those kernels were shipped doesn't use 1-bit HW ECC, so
that may not be much of a concern.

(The original mainline u-boot large page support was very
broken, and wasn't MV-compatible.  So arguably there were two
separate compatibility issues:  MV, and large-page bugginess.)


> http://lists.denx.de/pipermail/u-boot/2008-June/036055.html
> http://lists.denx.de/pipermail/u-boot/2008-August/039679.html
> 
> If some portions of it aren't actually needed for compatibility, then we 
> can remove them.
> 
> Or we can remove the entire thing, if nobody cares anymore -- if anyone 
> out there does care and is using this, please speak up now.

I'm hoping for the "nobody cares any more" option...

Anyone who *does* can always use their current version
of U-Boot ... or patch it back in.  If there have been
complaints due to lack of compatibility between current
GIT kernels and the MV releases, I've not heard them.


> > What this option enables differs in two ways from what the
> > MontaVista code does.  (Speaking here of the 1-bit HW ECC.
> > The 4-bit support is another mess, which would be made far
> > worse by needing to carry the BROKEN_ECC mode.)
> 
> I see no reason why new features would have to be supported on both 
> sides of the ifdef.

I was referring to a textual mess.  One clean way to add
a broken ECC mode would be as a completely different set
of functions, instead of a pile of #ifdefs.

And 4-bit will be troublesome for other reasons.


> > Which is why I'm wondering what that original U-Boot code
> > for HW ECC was trying to be "compatible" with, since it
> > clearly wasn't MontaVista Linux ... or even the U-Boot
> > versions I've seen be distributed with it.
> 
> MV 2.6.10 was the claim.

Right.  I looked at TI's 1.20 (MV 4.0/2.6.10) and 2.0 beta
(MV 5.0/2.6.18) code, and their sibling U-Boot versions.

If that compatibilty is actually needed, the simplest way
to get it might be to define functions to mangle the the
ECC words (standard ~PQRstu <---> MV PsQRtu) and use the
current *correction* code; the OOB bit pattern changes but
not the algorithm.  I think I'll add a comment about that.

- Dave


> 
> -Scott
> 
> 

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[Stephen Irons]

Does this help at all: at any rate, it is a bit of the history of the 
u-boot code:

http://lists.denx.de/pipermail/u-boot/2008-August/039679.html

Stephen Irons


David Brownell wrote:
> I was looking at the DaVinci NAND support in current U-Boot
> code (i.e. 2009.03 plus patches merged since that release),
> and am puzzled by the above-named config option.
>
> Before I submit a patch to remove it from U-Boot GIT (nothing
> there enables it, and it will nastify 4-bit support), I thought
> I'd see if anyone knows exactly what software it was trying to
> emulate.  Differences from the current NAND driver in GIT:
>
>  - Matches MVL 4.0 (2.6.10) and 5.0 (2.6.18) drivers in handling
>    NANDx1ECC registers:  0PQR0stu maps to PsQRtu, while the NAND
>    driver in mainline maps it to ~PQRstu.
>
>  - Custom ECC layouts, "not compatible with Linux or RBL/UBL".
>
>  - Does some very broken stuff for large page support.
>
> The first of those I can understand; someone wrote some odd and
> overly-complex ECC code way back, it worked and then got shipped.
> (Although TI's U-Boot 1.2.0 uses soft ECC, instead...)
>
> The other two look like they were experimental code that
> should probably not have been merged anywhere...
>
> - Dave
>
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source at linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>   


=======================================================================
This email, including any attachments, is only for the intended
addressee.  It is subject to copyright, is confidential and may be
the subject of legal or other privilege, none of which is waived or
lost by reason of this transmission.
If the receiver is not the intended addressee, please accept our
apologies, notify us by return, delete all copies and perform no
other act on the email.
Unfortunately, we cannot warrant that the email has not been
 altered or corrupted during transmission.
=======================================================================

[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC

[David Brownell]

On Sunday 03 May 2009, Stephen Irons wrote:
> Does this help at all

Not quite; since when I looked at the MontaVista code,
I didn't see anything like that brokeness.  It's possible
that some old MV kernels had that.  If so, more recent
stuff seems to have been cured.


> at any rate, it is a bit of the history of the  
> u-boot code:
> 
> http://lists.denx.de/pipermail/u-boot/2008-August/039679.html