RE: [PATCH 1/2] spi: spi-mem: Add the spi_set_xfer_bpw function

[Chuanhua Han <chuanhua.han@nxp.com>]

> -----Original Message-----
> From: Esben Haabendal <esbenhaabendal@gmail.com> On Behalf Of Esben
> Haabendal
> Sent: 2018年10月9日 19:21
> To: Chuanhua Han <chuanhua.han@nxp.com>
> Cc: Boris Brezillon <boris.brezillon@bootlin.com>; broonie@kernel.org;
> linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] spi: spi-mem: Add the spi_set_xfer_bpw function
> 
> Chuanhua Han <chuanhua.han@nxp.com> writes:
> 
> >> -----Original Message-----
> >> From: Boris Brezillon <boris.brezillon@bootlin.com>
> >> Sent: 2018年10月9日 18:05
> >> To: Chuanhua Han <chuanhua.han@nxp.com>
> >> Cc: broonie@kernel.org; linux-spi@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; eha@deif.com
> >> Subject: Re: [PATCH 1/2] spi: spi-mem: Add the spi_set_xfer_bpw
> >> function
> >>
> >> On Tue, 9 Oct 2018 09:52:23 +0000
> >> Chuanhua Han <chuanhua.han@nxp.com> wrote:
> >>
> >> > 1. In the dspi driver (spi controller), bits_per_word
> >> > (dspi->bits_per_word = transfer->bits_per_word) passed from the
> >> > upper layer (spi-mem.c) is used. In this way, I can only assign the
> >> > appropriate value of transfer->bits_per_word before passing to the
> >> > controller, that is, the controller driver does not know the value
> >> > of bits_per_word, and it will use this value when the upper level
> >> > sets what value is passed.
> >>
> >> I think you're missing my point: ->bits_per_word is not what you're
> >> looking for if what you're trying to do is use 32-bits accesses when
> >> things are properly aligned.
> >>
> > In the dspi driver (spi controller driver), it is based on whether
> > ->bits_per_word is
> > larger than 16 to decide whether to use the XSPI mode (32bit) to transfer
> data.
> 
> Not completely true.  XSPI mode is enabled, also for words smaller than or
> equal to 16 bits.  But TX FIFO and CMD FIFO is written together, just as for
> non-XSPI mode.
> 
> > If ->bits_per_word is not set and the default bits_per_word =8 is
> > passed to the dspi driver, the XSPI mode (32bit) is not used for data
> > transfer in the dspi driver
> 
> Not true.  XSPI mode is unconditionally enabled in dspi_init().  But XSPI
> mode does not overrule the value of transfer->bits_per_word.
> The meaning of XSPI mode is the following:
> 
> 1. Frame (word) size is max 32 bits (with normal SPI mode, max is 16 bits).
> 2. For frames (words) with more than 16 bits per frame (word), each frame
> transfer results in 2 TX FIFO pop operations.
> 3. Command cycling is possible, enabled when SPI_CTARE[DTCP] > 1.
> 
> Command cycling is currently not implemented.  If implemented, it would be
> possible to send multiple frames (words) by writing one time to CMD FIFO and
> multiple times to TX FIFO.  This could possibly improve performance
> 
> Another possibility would be to use EOQ mode, if supported by the DSPI
> controller in the CPU.  It allows for filling TX FIFO, and getting IRQ only after
> last TX FIFO entry is sent.  This is also a likely performance improvement.
> 
> >> > 2. As I understand, bits_per_word does not exist for non-byte
> >> > alignment, but for the need to reserve non-byte transmission mode
> >> > that meets the controller.
> >>
> >> Exactly. It's an optimization you have to take care of inside your
> >> driver. The core cannot help you with that.
> >>
> > The core layer is the upper layer. If you don't set ->bits_per_word,
> > bits_per_word will use the default value of 8, so that the
> > controller's specific mode for transferring data cannot be used (eg:
> > XSPI mode).
> 
> XSPI mode is independent of bits_per_word.  In XSPI mode, you can send
> frames as small as 4 bits, and up to 32 bits.  In normal SPI mode, you can
> send frames from 4 to 16 bits.
> 
> >> > 3. In addition, now the
> >> > XSPI of dspi cannot transfer data normally, so this problem needs
> >> > to be solved.
> >>
> >> I still don't understand what the problem is.
> >>
> > The problem is that I tested the XSPI mode and could not work, that
> > is, the data could not be transmitted normally.
> 
> What does "could not be transmitted normally" mean?
> 
> I am using XSPI mode on LS1021A, talking to a lot of different SPI devices.
> And they all work, and I believe everything is quite "normal".
> 
Since I don't have the board of LS1021, I can't test it. I use other boards with DSPI (such as LS1043, LS2088, etc.), 
and I test sp-flash connected on DSPI. If XSPI mode is used at this time, data cannot be transmitted normally.
> /Esben