Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Esben Haabendal <esben.haabendal@gmail.com>
To:     Chuanhua Han <chuanhua.han@nxp.com>
Cc:     broonie@kernel.org, linux-spi@vger.kernel.org,
        linux-kernel@vger.kernel.org, boris.brezillon@bootlin.com
Subject: Re: [PATCH 2/2] spi: spi-fsl-dspi: Fix support for XSPI transport mode
References: <20180921070628.35153-1-chuanhua.han@nxp.com>
        <20180921070628.35153-2-chuanhua.han@nxp.com>
Date:   Sat, 29 Sep 2018 16:56:17 +0200
In-Reply-To: <20180921070628.35153-2-chuanhua.han@nxp.com> (Chuanhua Han's
        message of "Fri, 21 Sep 2018 15:06:27 +0800")
Message-ID: <87y3bkcpym.fsf@gmail.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Chuanhua Han <chuanhua.han@nxp.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