[U-Boot] U-Boot and CONFIG_SYS_DAVINCI_BROKEN_ECC
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
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.
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
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.
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
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.
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
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
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
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
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@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. =======================================================================
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
participants (6)
-
David Brownell -
Hugo Villeneuve -
Jean-Christophe PLAGNIOL-VILLARD -
Scott Wood -
Stephen Irons -
Wolfgang Denk