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: <20180929174340.53290075@bbrezillon>
Date:   Sat, 29 Sep 2018 17:43:40 +0200
From:   Boris Brezillon <boris.brezillon@...tlin.com>
To:     Esben Haabendal <esben.haabendal@...il.com>
Cc:     Chuanhua Han <chuanhua.han@....com>, broonie@...nel.org,
        linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] spi: spi-fsl-dspi: Fix support for XSPI transport
 mode

Hi Esben,

On Sat, 29 Sep 2018 16:56:17 +0200
Esben Haabendal <esben.haabendal@...il.com> wrote:

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

Yes, I forgot to mention that, but this patch should definitely be
split in at least 2 patches.

Regards,

Boris