On Sun, Feb 08, 2009 at 12:19:56AM +0100, Alessandro Rubini wrote:
> +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?
> +{
> + __asm__ __volatile__(
> + "eor %0, %0, %0, lsr #4\n\t"
> + "eor %0, %0, %0, lsr #2\n\t"
> + "eor %0, %0, %0, lsr #1\n\t"
> + "ands %0, %0, #1\n\t"
> + "subne %0, %0, #2\t"
> + : "=r" (b) : "0" (b));
> + return b;
> +}
Why is this volatile?
The underscores are unnecessary, BTW.
Have you verified that this is noticeably better than C code?
> +/*
> + * 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?
> + diff = (r ^ c) & ((1<<24)-1); /* use 24 bits only */
Put spaces around binary operators.
> +/* This is the layout used by older installations, we keep compatible */
> +struct nand_ecclayout nomadik_ecc_layout = {
> + .eccbytes = 3 * 4,
> + .eccpos = { /* each subpage has 16 bytes: pos 2,3,4 hosts ECC */
> + 0x02, 0x03, 0x04,
> + 0x12, 0x13, 0x14,
> + 0x22, 0x23, 0x24,
> + 0x32, 0x33, 0x34},
> + .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?
> +/* Copy a buffer 32bits at a time: faster than defualt method which is 8bit
> */
> +static void nomadik_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int
> len)
> +{
> + int i;
> + struct nand_chip *chip = mtd->priv;
> + u32 *p = (u32 *) buf;
> +
> + len >>= 2;
> + writel(0, REG_FSMC_ECCR0);
> + for (i = 0; i < len; i++)
> + p[i] = readl(chip->IO_ADDR_R);
> +}
What if "len" isn't a multiple of 4?
> +int board_nand_init(struct nand_chip *chip)
> +{
> + /* Set up the FSMC_PCR0 for nand access*/
> + writel(0x0000004a, REG_FSMC_PCR0);
> + /* Set up FSMC_PMEM0, FSMC_PATT0 with timing data for access */
> + writel(0x00020401, REG_FSMC_PMEM0);
> + writel(0x00020404, REG_FSMC_PATT0);
> +
> + chip->options = NAND_COPYBACK | NAND_CACHEPRG | NAND_NO_PADDING;
> + chip->cmd_ctrl = nomadik_nand_hwcontrol;
> + chip->dev_ready = nomadik_nand_ready;
> + /* The chip allows 32bit reads, so avoid the default 8bit copy */
> + chip->read_buf = nomadik_nand_read_buf;
> +
> + /* ECC: follow the hardware-defined rulse, but do it in sw */
"rules"
> -#define CONFIG_SYS_NAND_BASE 0x40000000
> +#define CONFIG_SYS_NAND_BASE 0x40000000 /* SMPS0n */
What is "SMPS0n"?
-Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
