No subject

The code was written at 4 years ago.
I'll see if there is a better way, and it'll be updated at next version
when I got luck.

>> +       /* page read */
>> +       for (off = 0; len > 0; len -= 4, off += 4) {
>> +               while (!(NAND_READ(&regs->ior) & IOR_READY))
>> +                       ;
>> +               *(uint32_t *)(buf + off) = NAND_READ(&regs->dr);
>> +       }
>
>
> Why do you need to check IOR_READY here but not in read_byte?
>
>

Because there is no any hardware access in 'read_byte'.
The FTNANDC021 simplily does not support byte access by default.
(I mean it's possible to update hardware RTL code,
but it's not the case to A369)
So I add some tricks to READ_ID, READ_OOB and READ_STATUS.
The actual hardware access is performed at ftnandc021_cmdfunc(),
The ftnandc021_read_byte() simpily access the cached data buffer.

>> +static void
>> +ftnandc021_cmdfunc(struct mtd_info *mtd, unsigned cmd, int column, int
>> pgidx)
>> +{
>> +       struct nand_chip *chip = mtd->priv;
>> +       struct ftnandc021_chip *priv = chip->priv;
>> +       struct ftnandc021_regs *regs = priv->iobase;
>> +
>> +       priv->cmd   = cmd;
>> +       priv->pgidx = pgidx;
>> +
>> +       switch (cmd) {
>> +       case NAND_CMD_READID:   /* 0x90 */
>> +               if (ftnandc021_command(priv, FTNANDC021_CMD_RDID)) {
>> +                       printf("ftnandc021: RDID failed.\n");
>> +               } else {
>> +                       put_unaligned_le32(NAND_READ(&regs->idr[0]),
>> +                               priv->buf);
>> +                       put_unaligned_le32(NAND_READ(&regs->idr[1]),
>> +                               priv->buf + 4);
>> +                       priv->off = 0;
>> +               }
>> +               break;
>
>
> Do error handling like this:
>
>         if (ftnandc021_command(priv, FTNANDC021_CMD_RDID)) {
>                 printf(...);
>                 break;
>         }
>
>         put_unaligned...
>         ...
>
> Why would it be unaligned?
> Why _le32?

It's a historic issue, I think.
It's too long (4 years..) to me to recall the reason,
and it's obviously not necessary right now, so it would be
updated at next version.

> Can you not read a byte at a time here?
>
>

No, the FTNANDC021 does not support byte access.

>> +/**
>> + * hardware specific access to control-lines
>> + * @mtd: MTD device structure
>> + * @cmd: command to device
>> + * @ctrl:
>> + * NAND_NCE: bit 0 -> don't care
>> + * NAND_CLE: bit 1 -> Command Latch
>> + * NAND_ALE: bit 2 -> Address Latch
>> + *
>> + * NOTE: boards may use different bits for these!!
>> + */
>> +static void
>> +ftnandc021_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
>> +{
>> +}
>
>
> Just leave the function pointer NULL.
>
>

Got it, thanks

>> +       chip->ecc.mode   = NAND_ECC_NONE;
>
>
> Really, no ECC at all?  That is quite broken.  There is absolutely no way
> to get access to the full OOB in order to do software ECC?
>
> Or is it doing hardware ECC in a way that is transparent?  You should
> still use NAND_ECC_HARD in that case.
>
>
>> +       chip->ecc.layout = &ftnandc021_oob_2k;
>
>
> What if it's not 2K NAND?
>
>

Please see the comments above.
And it would be clean up at next version.

>> diff --git a/include/faraday/nand.h b/include/faraday/nand.h
>> new file mode 100644
>> index 0000000..6d8efb2
>> --- /dev/null
>> +++ b/include/faraday/nand.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * Faraday NAND Flash Controller
>> + *
>> + * (C) Copyright 2010 Faraday Technology
>> + * Dante Su <dantesu at faraday-tech.com>
>> + *
>> + * This file is released under the terms of GPL v2 and any later version.
>> + * See the file COPYING in the root directory of the source tree for
>> details.
>> + */
>> +
>> +#ifndef _FARADAY_NAND_H
>> +#define _FARADAY_NAND_H
>> +
>> +int ftnandc021_probe(struct nand_chip *chip);
>
>
> New drivers should use CONFIG_SYS_NAND_SELF_INIT
>
> -Scott



--
Best wishes,
Kuo-Jung Su