Alessandro Rubini wrote:
From: Scott Wood scottwood@freescale.com
Unfortunately freescale.com i.e. mail.global.frontbridge.com i.e. microsoft has blacklisted me. I'm trying to do what they say but I fear you won't get direct email.
Hmm, I've e-mailed the postmaster inquiring as to why. Is there a specific IP address or range that is being blocked? Is there a rejection message?
+static inline int parity(int b) /* b is really a byte; returns 0 or ~0 */
If it's really a byte, then why not tell the compiler this with uint8_t?
Because otherwise it will add instructions to mask the value.
OK.
- __asm__ __volatile__(
Why is this volatile? The underscores are unnecessary, BTW.
Both for my own pedantry.
volatile should be left off of pure calculations; you're just removing optimization opportunity.
And I think the underscores are ugly. :-)
Have you verified that this is noticeably better than C code?
Well... it looked like I only checked without -O. I rechecked and the result is the same. Ok, will switch to the C version.
OK, good.
+/*
- This is the ECC routine used in hardware, according to the manual.
- HW claims to make the calculation but not the correction; so we must
- recalculate the bytes for a comparison.
- */
Why must you recalculate? What does the hardware do with the ECC it calculates?
It only makes it available. You must recalculate and compare.
Makes what available? If it makes the calculated ECC available, you should only need to do the code in ecc_correct, not ecc512.
However, I haven't been able to make the hardware work (nor original vendor code did actually use the hardware).
OK, that seems to be the more relevant reason. :-)
Include a comment to that effect.
- .oobfree = { {0x08, 0x08}, {0x18, 0x08}, {0x28, 0x08}, {0x38, 0x08} },
+};
Any particular reason why bytes 0x05-0x07, 0x10-0x11, 0x15-0x17, etc. aren't marked free?
Since most other ECC routines use 2..7 I chose to leave open the possibility to switch over from 2..4. Is that wrong?
It's not wrong as long as the free bytes you do claim meet all expected needs; I was just curious.
- len >>= 2;
What if "len" isn't a multiple of 4?
I thought it never is. This always reads either 512 or 64 bytes. Aligned, too.
Suppose it only wants to read a few bytes of OOB.
-#define CONFIG_SYS_NAND_BASE 0x40000000 +#define CONFIG_SYS_NAND_BASE 0x40000000 /* SMPS0n */
What is "SMPS0n"?
It's the chip select.
OK. Just making sure it wasn't something left in by accident.
-Scott