Re: [PATCH v2 0/5] mtd: spi-nor: Add support for Octal 8D-8D-8D mode

masonccyang
2020-05-11T06:55:54Z

Hi Boris,

> > > >
> > > > To clarify a bit more, the idea is that we transmit the opcode MSB

> > > > first, just we do for the address. Assume we want to issue the
command
> > > > 0x05. In 1S mode, we set cmd.opcode to 0x05. Here cmd.nbytes == 1.
Treat
> > >
> > > > is as a 1-byte value, so the MSB is the same as the LSB. We
directly
> > > > send 0x5 on the bus.
> > >
> > > There are many SPI controllers driver use "op->cmd.opcode" directly,
> > > so is spi-mxic.c.
> > >
> > > i.e,.
> > > ret = mxic_spi_data_xfer(mxic, &op->cmd.opcode, NULL,
op->cmd.nbytes);
> >
> > Just because you do it doesn't mean it's right. And most controllers
use
> > the opcode value, they don't dereference the pointer as you do here.
> >
> > >
> > > >
> > > > If cmd.nbytes == 2, then the opcode would be 0x05FA (assuming
extension
> > > > is invert of command). So we send the MSB (0x05) first, and LSB
(0xFA)
> > > > next.
> > >
> > > My platform is Xilinx Zynq platform which CPU is ARMv7 processor.
> > >
> > > In 1-1-1 mode, it's OK to send 1 byte command by u16 opcode but
> > > in 8D-8D-8D mode, I need to patch
> > >
> > > i.e.,
> > > op->cmd.opcode = op->cmd.opcode | (ext << 8);
> > >
> > > rather than your patch.
> >
> > Seriously, how hard is it to extract each byte from the u16 if your
> > controller needs to pass things in a different order? I mean, that's
> > already how it's done for the address cycle, so why is it a problem
> > here? This sounds like bikeshedding to me. If the order is properly
> > documented in the kernel doc, I see no problem having it grouped in
one
> > u16, with the first cmd cycle placed in the MSB and the second one in
> > the LSB.
>
> So, I gave it a try, and we're talking about something as simple as the
> below diff. And yes, the mxic controller needs to be patched before
> extending the cmd.opcode field, but we're talking about one driver here
> (all other drivers should be fine). Oh, and if you look a few lines
below
> the changed lines, you'll notice that we do exactly the same for the
> address.

yup,
thanks again for your time & comments.

>
> --->8---
> diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
> index 69491f3a515d..c3f4136a7c1d 100644
> --- a/drivers/spi/spi-mxic.c
> +++ b/drivers/spi/spi-mxic.c
> @@ -356,6 +356,7 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
> int nio = 1, i, ret;
> u32 ss_ctrl;
> u8 addr[8];
> + u8 cmd[2];
>
> ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz);
> if (ret)
> @@ -393,7 +394,10 @@ static int mxic_spi_mem_exec_op(struct spi_mem
*mem,
> writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT,
> mxic->regs + HC_CFG);
>
> - ret = mxic_spi_data_xfer(mxic, &op->cmd.opcode, NULL, 1);
> + for (i = 0; i < op->cmd.nbytes; i++)
> + cmd[i] = op->cmd.opcode >> (8 * (op->cmd.nbytes - i -
1));
> +
> + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes);
> if (ret)
> goto out;
>

best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================