linux-kernel - RE: [PATCH 2/2] spi: spi-fsl-dspi: Fix support for XSPI transport mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AM6PR04MB43575CFD4D00B8EAC1F52BD097EE0@AM6PR04MB4357.eurprd04.prod.outlook.com>
Date:   Sun, 30 Sep 2018 02:49:20 +0000
From:   Chuanhua Han <chuanhua.han@....com>
To:     Esben Haabendal <esben.haabendal@...il.com>
CC:     "broonie@...nel.org" <broonie@...nel.org>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "boris.brezillon@...tlin.com" <boris.brezillon@...tlin.com>
Subject: RE: [PATCH 2/2] spi: spi-fsl-dspi: Fix support for XSPI transport
 mode



> -----Original Message-----
> From: Esben Haabendal <esbenhaabendal@...il.com> On Behalf Of Esben
> Haabendal
> Sent: 2018年9月29日 22:56
> To: Chuanhua Han <chuanhua.han@....com>
> Cc: broonie@...nel.org; linux-spi@...r.kernel.org;
> linux-kernel@...r.kernel.org; boris.brezillon@...tlin.com
> Subject: Re: [PATCH 2/2] spi: spi-fsl-dspi: Fix support for XSPI transport mode
> 
> Chuanhua Han <chuanhua.han@....com> writes:
> 
> > This patch fixes the problem that the XSPI mode of the dspi controller
> > cannot transfer data properly.
> > In XSPI mode, cmd_fifo is written before tx_fifo, which transforms the
> > byte order of sending and receiving data.
> 
> Did you find documentation on proper ordering of writes to related TX FIFO
> and CMD FIFO entries?
> 
> I have failed to find such information, and thus opted for what I believed would
> be the safe approach, writing to TX FIFO first, so that when CMD FIFO is
> written, it will already have data in place.  And it seems to work.
> 
> But, I now see that SPIx_SR[TFIWF] hints that it should be done the other way
> around.
> 
>     Tranmit FIFO Invalid Write Flag - Indicates Data Write on TX FIFO
>     while CMD FIFO is empty. Without a Command, the Data entries present
>     in TXFIFO are invalid.
> 
> But I fail to see how that should be related to byte ordering.
> 
> So I believe this patch is doing two things.
> 
> 1. Changing write ordering of TX FIFO and CMD FIFO.
> 2. Handling byte ordering based on SPIx_CTARn[LSBFE] flag.
> 
> It would be nice if we could get clarification from NXP on what is the right way
> to do the TX FIFO and CMD FIFO write ordering.
> 
> But as for the byte ordering changes, I don't think it looks write.  The meaning
> of SPIx_CTARn[LSBFE] is according to the documentaiton the bit ordering on
> the wire, and should not be related to register byte ordering.
> 
> You should probably split this patch in two, so they can be reviewed and
> merged independently.
> 
> /Esben
Hi, Esben
First of all, thank you for your valuable advice. I'm going to do two things:
1. Divide this patch into multiple patches.
2. Verify (if you can) from the relevant NXP documentation.

Second, let me mention the issues I found on fixing fsl_dspi's XSPI pattern not working:
1. When I executed TX FIFO first and then CMD FIFO for XSPI transmission, I found that SPIx_SR[TFIWF]=1 (Invalid Data present in TX FIFO since CMD FIFO is empty).
 This is the time when no Data can be read or written (all the Data obtained is equal to 0).
2. When I read and write data without converting data byte order, and read and write data directly,
 I tested spi-flash connected by fsl_dspi controller, and found that data byte order was reversed with the correct byte order. 
When I changed the byte order according to the SPIx_CTARn[LSBFE] flag, the correct data was obtained (not explained in this data manual).
3. In the dspi_push_rx function, if you add your "rxdata &= (1 << dspi->bits_per_word) -1" statement, the rxdata=0 is obtained when xspi is transmitted, 
and the correct data cannot be transmitted at this time (I don't quite understand why you added this statement).
Thanks,
Chuanhua