RE: [PATCH 3/5] spi: spi-nxp-fspi: add DTR mode support

[Bough Chen <haibo.chen@xxxxxxx>]

> -----Original Message-----
> From: Michael Walle <mwalle@xxxxxxxxxx>
> Sent: 2023年12月14日 0:53
> To: Bough Chen <haibo.chen@xxxxxxx>
> Cc: broonie@xxxxxxxxxx; Han Xu <han.xu@xxxxxxx>; dl-linux-imx
> <linux-imx@xxxxxxx>; linux-spi@xxxxxxxxxxxxxxx; yogeshgaur.83@xxxxxxxxx;
> Michael Walle <mwalle@xxxxxxxxxx>
> Subject: Re: [PATCH 3/5] spi: spi-nxp-fspi: add DTR mode support
> 
> > From: Haibo Chen <haibo.chen@xxxxxxx>
> >
> > For LUT, add DTR command support.
> 
> Please elaborate a bit more.

Okay.

> 
> 
> > Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx>
> > ---
> >  drivers/spi/spi-nxp-fspi.c | 27 ++++++++++++++++++++++-----
> >  1 file changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> > index 9d6b4d22263c..2562d524149e 100644
> > --- a/drivers/spi/spi-nxp-fspi.c
> > +++ b/drivers/spi/spi-nxp-fspi.c
> > @@ -552,12 +552,22 @@ static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
> >  	int lutidx = 1, i;
> >
> >  	/* cmd */
> > -	lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
> > -			     op->cmd.opcode);
> > +	if (op->cmd.dtr) {
> > +		lutval[0] |= LUT_DEF(0, LUT_CMD_DDR,
> LUT_PAD(op->cmd.buswidth),
> > +				     op->cmd.opcode >> 8);
> 
> Shouldn't we check cmd.nbytes here? You seem to mix dtr with cmd.nbytes ==
> 2 here.

Currently, for DTR mode, all cmd.nbytes == 2. Refer to drivers/mtd/spi-nor/core.c : spi_nor_spimem_setup_op()
But better to check the cmd.nbytes here to make the code more strong.

> 
> > +		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_CMD_DDR,
> > +					      LUT_PAD(op->cmd.buswidth),
> > +					      op->cmd.opcode & 0x00ff);
> 
> And you seem to assume dtr is always octal mode?

Currently, I only test the octa dtr mode(8D-8D-8D). but here, we config the op->cmd.buswidth, op->addr.buswidth, op->dummy.buswidth, op->data.buswidth.
So I think current LUT config can cover other dts mode, like 1D-8D-8D, 1D-4D-4D, 1D-2D-2D, 1D-1D-1D

Best Regards
Haibo Chen
> 
> -michael
> 
> > +		lutidx++;
> > +	} else {
> > +		lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
> > +				     op->cmd.opcode);
> > +	}
> >
> >  	/* addr bytes */
> >  	if (op->addr.nbytes) {
> > -		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_ADDR,
> > +		lutval[lutidx / 2] |= LUT_DEF(lutidx, op->addr.dtr ?
> > +					      LUT_ADDR_DDR : LUT_ADDR,
> >  					      LUT_PAD(op->addr.buswidth),
> >  					      op->addr.nbytes * 8);
> >  		lutidx++;
> > @@ -565,7 +575,8 @@ static void nxp_fspi_prepare_lut(struct nxp_fspi
> > *f,
> >
> >  	/* dummy bytes, if needed */
> >  	if (op->dummy.nbytes) {
> > -		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_DUMMY,
> > +		lutval[lutidx / 2] |= LUT_DEF(lutidx, op->dummy.dtr ?
> > +					      LUT_DUMMY_DDR : LUT_DUMMY,
> >  		/*
> >  		 * Due to FlexSPI controller limitation number of PAD for dummy
> >  		 * buswidth needs to be programmed as equal to data buswidth.
> > @@ -580,7 +591,8 @@ static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
> >  	if (op->data.nbytes) {
> >  		lutval[lutidx / 2] |= LUT_DEF(lutidx,
> >  					      op->data.dir == SPI_MEM_DATA_IN ?
> > -					      LUT_NXP_READ : LUT_NXP_WRITE,
> > +					      (op->data.dtr ? LUT_READ_DDR :
> LUT_NXP_READ) :
> > +					      (op->data.dtr ? LUT_WRITE_DDR :
> LUT_NXP_WRITE),
> >  					      LUT_PAD(op->data.buswidth),
> >  					      0);
> >  		lutidx++;
> > @@ -1152,6 +1164,10 @@ static const struct spi_controller_mem_ops
> nxp_fspi_mem_ops = {
> >  	.get_name = nxp_fspi_get_name,
> >  };
> >
> > +static struct spi_controller_mem_caps nxp_fspi_mem_caps = {
> > +	.dtr = true,
> > +};
> > +
> >  static int nxp_fspi_probe(struct platform_device *pdev)  {
> >  	struct spi_controller *ctlr;
> > @@ -1254,6 +1270,7 @@ static int nxp_fspi_probe(struct platform_device
> *pdev)
> >  	ctlr->bus_num = -1;
> >  	ctlr->num_chipselect = NXP_FSPI_MAX_CHIPSELECT;
> >  	ctlr->mem_ops = &nxp_fspi_mem_ops;
> > +	ctlr->mem_caps = &nxp_fspi_mem_caps;
> >
> >  	nxp_fspi_default_setup(f);
> >
> > --
> > 2.34.1