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

[liao jaime]

Hi Michael

>
> 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).
Got it, thanks.

>
> >> 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.
Do you mean a common function for ID comapre in core.c?

>
> 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.
For all flashed in this patch, data reading are all same between SPI mode
and octal DTR mode.

>
> 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

Thank you
Jaime