dirk.behme@googlemail.com wrote:
+unsigned char cs; +volatile unsigned long gpmc_cs_base_add;
Make these static. gpmc_cs_base_add should be a pointer, not "unsigned long". Volatile isn't needed since you use I/O accessors, and definitely isn't needed on the address itself.
+/*
- omap_nand_hwcontrol - Set the address pointers corretly for the
following address/data/command operation- */
+static void omap_nand_hwcontrol(struct mtd_info *mtd, int cmd,
unsigned int ctrl)+{
- register struct nand_chip *this = mtd->priv;
- /* Point the IO_ADDR to DATA and ADDRESS registers instead
of chip address */- switch (ctrl) {
- case NAND_CTRL_CHANGE | NAND_CTRL_CLE:
this->IO_ADDR_W = (void *) gpmc_cs_base_add + GPMC_NAND_CMD;this->IO_ADDR_R = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;break;- case NAND_CTRL_CHANGE | NAND_CTRL_ALE:
this->IO_ADDR_W = (void *) gpmc_cs_base_add + GPMC_NAND_ADR;this->IO_ADDR_R = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;break;- case NAND_CTRL_CHANGE | NAND_NCE:
this->IO_ADDR_W = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;this->IO_ADDR_R = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;break;- }
IO_ADDR_R never seems to change; you can leave it out of here and omap_nand_wait.
+/*
- omap_nand_wait - called primarily after a program/erase operation
so that we access NAND again only after the device
is ready again.
- @mtd: MTD device structure
- @chip: nand_chip structure
- */
+static int omap_nand_wait(struct mtd_info *mtd, struct nand_chip *chip) +{
- register struct nand_chip *this = mtd->priv;
- int status = 0;
- this->IO_ADDR_W = (void *) gpmc_cs_base_add + GPMC_NAND_CMD;
- this->IO_ADDR_R = (void *) gpmc_cs_base_add + GPMC_NAND_DAT;
- /* Send the status command and loop until the device is free */
- while (!(status & 0x40)) {
writeb(NAND_CMD_STATUS & 0xFF, this->IO_ADDR_W);status = readb(this->IO_ADDR_R);- }
Maybe should just do this, to avoid changing client-visible state: writeb(NAND_CMD_STATUS, &gpmc_cs_base_add[GPMC_NAND_CMD]);
No need for the "& 0xFF".
- /* Init ECC Control Register */
- /* Clear all ECC | Enable Reg1 */
- val = ((0x00000001 << 8) | 0x00000001);
- writel(val, GPMC_BASE + GPMC_ECC_CONTROL);
- writel(0x3fcff000, GPMC_BASE + GPMC_ECC_SIZE_CONFIG);
Symbolic constants for the bit values would be nice.
+/*
- omap_calculate_ecc - Generate non-inverted ECC bytes.
- Using noninverted ECC can be considered ugly since writing a blank
- page ie. padding will clear the ECC bytes. This is no problem as
- long nobody is trying to write data on the seemingly unused page.
- Reading an erased page will produce an ECC mismatch between
- generated and read ECC bytes that has to be dealt with separately.
Where is it dealt with separately?
- unsigned long val = 0x0;
Unnecessary initialization.
- unsigned long reg;
- /* Start Reading from HW ECC1_Result = 0x200 */
- reg = (unsigned long) (GPMC_BASE + GPMC_ECC1_RESULT);
- val = readl(reg);
readl() takes a pointer. ARM gets away without a warning here because it uses macros rather than inline functions, but it's bad practice.
- /* Stop reading anymore ECC vals and clear old results
* enable will be called if more reads are required */- reg = (unsigned long) (GPMC_BASE + GPMC_ECC_CONFIG);
- writel(0x000, reg);
Likewise.
+void omap_nand_switch_ecc(int hardware) +{
- struct nand_chip *nand;
- if (nand_curr_device < 0 ||
nand_curr_device >= CONFIG_SYS_MAX_NAND_DEVICE ||!nand_info[nand_curr_device].name) {printf("Error: Can't switch ecc, no devices available\n");return;- }
- nand = (&nand_info[nand_curr_device])->priv;
- if (!hardware) {
nand->ecc.mode = NAND_ECC_SOFT;nand->ecc.layout = &sw_nand_oob_64;nand->ecc.size = 256; /* set default eccsize */nand->ecc.bytes = 3;nand->ecc.steps = 8;nand->ecc.hwctl = 0;nand->ecc.calculate = nand_calculate_ecc;nand->ecc.correct = nand_correct_data;- } else {
nand->ecc.mode = NAND_ECC_HW;nand->ecc.layout = &hw_nand_oob_64;nand->ecc.size = 512;nand->ecc.bytes = 3;nand->ecc.steps = 4;nand->ecc.hwctl = omap_enable_hwecc;nand->ecc.correct = omap_correct_data;nand->ecc.calculate = omap_calculate_ecc;omap_hwecc_init(nand);- }
Do you need to do anything similar to omap_hwecc_init() when switching to SW ECC to tell the hardware to stop doing ECC?
-Scott