RE: [PATCH v3] mtd/nand: don't use {read,write}_buf for 8-bit transfers

Gupta, Pekon
2013-12-02T19:41:17Z

>From: Ezequiel Garcia [mailto:ezequiel.garcia@free-electrons.com]
>> > > @@ -542,8 +545,8 @@ static void nand_command(struct mtd_info *mtd, unsigned int command,
>> > > /* Serially input address */
>> > > if (column != -1) {
>> > > /* Adjust columns for 16 bit buswidth */
>> > > - if (chip->options & NAND_BUSWIDTH_16)
>> > > - column >>= 1;
>> > > +// if (chip->options & NAND_BUSWIDTH_16)
>> > > +// column >>= 1;
>> > > chip->cmd_ctrl(mtd, column, ctrl);
>> > > ctrl &= ~NAND_CTRL_CHANGE;
>> > > }
>> > >
This is a very good finding ..
The ONFI spec says that data would be present only on lower 8-bits of
NAND data-bus. This mean we need to always issue 1-byte aligned address.
The above fix enforces that rule.

In absence of this fix, if (chip->options & NAND_BUSWIDTH_16) is set,
then nand_command() would issue a word-aligned address even when
we use chip->read_byte() to read a single data.

To take do this cleanly , we might need a larger fix, that is change in
interface function chip->cmdfunc()..
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
- void (*cmdfunc)(struct mtd_info *mtd, unsigned command, int column,
- int page_addr);
+ void (*cmdfunc)(struct mtd_info *mtd, unsigned command, int column,
+ int page_addr, int busw);

And replace above hack with.. @@nand_command()
- if (chip->options & NAND_BUSWIDTH_16)
- column >>= 1;
+ if (busw)
+ column >>= 1;

[...]

>> I believe we will want to use some ONFI routines (SET
>> FEATURES, especially) after initialization. This is where the rest of
>> Uwe's patch comes into play. So I don't think we can always switch
>> between call-backs and play games with NAND_BUSWIDTH_16; we should get
>> the bus width right as soon as possible, and then make sure that the
>> callbacks always work as expected.
>>
>
>Sure, I completely understand the above. The patches I've been pushing
>are meant *only* to solve the initial device detection, and in
>particular to fix the currently broken ONFI detection.
>
So as per my understanding..
*Uwe's patch*
- handles byte-wise access for onfi_set_feature()/onfi_get_feature()
*Brian's patch*
- handles byte-wise access for nand_flash_detect_onfi()
*Ezequiel's patch*
- modifies nand_command() to issue byte-aligned accesses. (as above)
- re-configuring of chip->options in nand_scan_ident before returning
back to callee (controller driver here).

If correct, then we should group these pieces together in patch-series
(with all contributors signed-off). And then test this on multiple devices.

I can do that, if all agree that above pieces are correct and required for
completion of functionality ??

with regards, pekon