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

Brian Norris
2013-11-30T06:05:06Z

Hi Ezequiel,

On Fri, Nov 29, 2013 at 09:20:19AM -0300, Ezequiel Garcia wrote:
> On Tue, Nov 26, 2013 at 11:35:12PM -0800, Brian Norris wrote:
> > + Pekon, Ezequiel
> >
> > Can one of you see how this patch works with your BeagleBones w/ x16
> > NAND?

I see that you are pushing to straighten out the auto-buswidth part of
nand_base, and I think there may be good reasons to do so. But I think
that part of your problem can be resolved by a patch like Uwe's, where
rather than forcing the entire driver to be configured for x8 just to
use ONFI, we can fix the ONFI operations to use the lower 8 bits.

IOW, I expect that a patch like Uwe's can shed some better light on the
auto-buswidh situation. (This is why I CC'd you and Pekon.)

Unfortunately, I realized that Uwe's patch doesn't go far enough, I
don't think. It looks like it needs something like the following diff
(only compile-tested).

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index bd39f7b67906..1ab264457d94 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2933,7 +2933,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
int *busw)
{
struct nand_onfi_params *p = &chip->onfi_params;
- int i;
+ int i, j;
int val;

/* Try ONFI for unknown chip or LP */
@@ -2942,18 +2942,10 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I')
return 0;

- /*
- * ONFI must be probed in 8-bit mode or with NAND_BUSWIDTH_AUTO, not
- * with NAND_BUSWIDTH_16
- */
- if (chip->options & NAND_BUSWIDTH_16) {
- pr_err("ONFI cannot be probed in 16-bit mode; aborting\n");
- return 0;
- }
-
chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
for (i = 0; i < 3; i++) {
- chip->read_buf(mtd, (uint8_t *)p, sizeof(*p));
+ for (j = 0; j < sizeof(*p); j++)
+ *(uint8_t *)p = chip->read_byte(mtd);
if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==
le16_to_cpu(p->crc)) {
break;

What do you think? (And more importantly, how does this test out for
you?)

Brian