Re: [PATCH v1 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash

[Michael Walle <michael@walle.cc>]

Hi,

>> You write "We" in your next patch. "We" as in macronix? Then please
>> use your macronix email address for the patches. Please note, you
>> can still send them through your gmail account.
> Yes I am Macronix engineer and sorry for the company mail
> issue so that I can't send and reply patch on Macronix mail.

Thus I was saying that you should use the macronix mail for the
patch author and the SoB and send your mails with your gmail
account (git will take care of the difference).

>> While cleaning up the flash_info db I come around this and it is
>> copied all over the place. Please work on factoring this (also the
>> other code in micron-st.c and spansion.c) out into a helper.
> Let me clearify the data order for read ID on Macronix flashes.
> Read ID
> SPI mode : c2-84-3c-c2-84-3c
> OPI DTR mode : c2-c2-84-84-3c-3c

So you are basically duplicating the id bytes in DTR mode. The same
byte is transferred both on the falling and rising edge of a clock
cycle. I think this needs further changes to the core. I means that
if the core will do a rdid in octal mode, it returns "garbage" for
now.

How is the data transferred in octal DTR mode? Reading the same data
in SPI and octal DTR mode should return the exact same bytes.

So one test would be to use the flash in SPI mode, write some data,
enable octal DTR mode and read it back and compare it. They must
match.

> So that I create a specify judement for checking ID via 8D-8D-8D
> on Macronix flash.

No, please make it a common helper for all flashes.

>> > +     op = (struct spi_mem_op)
>> > +             SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1),
>> > +                        SPI_MEM_OP_ADDR(enable ? 4 : 0, 0, 1),
>> > +                        SPI_MEM_OP_DUMMY(enable ? 4 : 0, 1),
>> > +                        SPI_MEM_OP_DATA_IN(SPI_NOR_MAX_ID_LEN, buf, 1));
>> > +
>> > +     if (enable)
>> > +             spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
>> > +
>> > +     ret = spi_mem_exec_op(nor->spimem, &op);
>> > +     if (ret)
>> > +             return ret;
>> > +
>> > +     if (enable) {
>> > +             for (i = 0; i < nor->info->id_len; i++)
>> > +                     if (buf[i * 2] != nor->info->id[i])
>> > +                             return -EINVAL;
>> 
>> Why is the ID now swapped? Doesn't look right.
> Actullay 6 bytes data are c2-c2-84-84-3c-3c which are got by 8D-8D-8D 
> read id
> on Macronix flash.

See above.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/