[PATCH] mtd: nand: Add support for Micron on-die ECC controller (rev2).

[Gupta, Pekon]

Hi Gerhard,
[...]

>> >when "on die ECC" is enabled, the chip requires a sequence of
>> >- read page command (READ0, addresses, READSTART)
>> >- status check (mandatory! cmd 70, to check FAIL and REWRITE bits)
>> >- re-enter data read mode (READ0, _without_ addresses and START)
>> >- fetch data bytes
>> >
>> I checked Micron Datasheet for "MT29F4G16ABADAWP by name
>> "m60a_4gb_8gb_16gb_ecc_nand.pdf" and on its
>> "Figure 39: READ PAGE (00h-30h) Operation with Internal ECC Enabled"
>> There is no mention of above third step.
>>
>> As per the "figure 39" a READ_PAGE Operation with internal ECC enabled
>> requires following sequence..
>> - [00h] <address> <address> ... <address> [30h]  /* READ_PAGE command */
>> - [70h] <status> [00h]        /* READ_STATUS command */
>> And after that data comes our serial on the bus. So in that case you can
>
>They cheat. :)  You'll only notice when you look closely, I
>misread this stuff too at first.  The Micron datasheet uses
>different terms than the MTD implementation in Linux does.
>
>Their "READ_PAGE" command, referred to as 00-30, actually is in
>Linux the "READ0 command (0x00) and READSTART command (0x30)"
>sequence.  (I'd like to see them as sequences or aggregates of
>existing commands, instead of even more commands that happen to
>re-use other command's opcodes, which I feel is confusing.)
>
Yes, I think its correctly implemented in nand_command_lp()
(Kindly cross check if I misunderstood below)

For chip->cmd(mtd, NAND_CMD_READ0, column, page);

@@nand_command_lp(..)
(Step-1) : Sends READ0 == [00h]
         /* Command latch cycle */
	chip->cmd_ctrl(mtd, command, NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);

(Step-2): Sends <column-address> .. <page-address>
        if (column != -1) {
        ...
        }
        if (page != -1) {
        ...
        }

(Step-3): Send READ_START == [30h]
                switch (command) {
                ...
                case NAND_CMD_READ0:
                         chip->cmd_ctrl(mtd, NAND_CMD_READSTART,
			          NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
                         chip->cmd_ctrl(mtd, NAND_CMD_NONE,
			          NAND_NCE | NAND_CTRL_CHANGE);
                }

So, I think nand_command_lp() follows the sequence as per Micron device.
As it has been working for normal nand_reads also.


>The above "70 status 00" actually is "STATUS (command 70 and a
>status byte) plus READ0 (command 00)".  Not re-emitting the READ0
>(this time without the addresses and READSTART) would make you
>fetch more status bytes instead of the cached data bytes.
>
Yes, this is why I said ("this may require some tweaks").
So, actually you can modify nand_command_lp()
- to add new command = NAND_CMD_END_STATUS which just issues [00h],
- OR modify existing NAND_CMD_STATUS to do something similar.

But I think for most of the command sequences nand_command_lp()
can be re-used, if not then please improve it.

with regards, pekon