Re: [PATCH] spi: mediatek: Fix fifo transfer

[Guenter Roeck <linux@xxxxxxxxxxxx>]

On 8/1/21 8:00 PM, Guenter Roeck wrote:
> Commit 3a70dd2d0503 ("spi: mediatek: fix fifo rx mode") claims that
> fifo RX mode was never handled, and adds the presumably missing code
> to the FIFO transfer function. However, the claim that receive data
> was not handled is incorrect. It was handled as part of interrupt
> handling after the transfer was complete. The code added with the above
> mentioned commit reads data from the receive FIFO before the transfer
> is started, which is wrong. This results in an actual transfer error
> on a Hayato Chromebook.
>
> Remove the code trying to handle receive data before the transfer is
> started to fix the problem.
>
> Fixes: 3a70dd2d0503 ("spi: mediatek: fix fifo rx mode")
> Cc: Peter Hess <peter.hess@xxxxxxxxxx>
> Cc: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
> Cc: Tzung-Bi Shih <tzungbi@xxxxxxxxxx>
> Cc: Hsin-Yi Wang <hsinyi@xxxxxxxxxx>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---

I should have added here: If this patch isn't acceptable for some reason,
commit 3a70dd2d0503 should be reverted because it is obviously wrong
and introduces a severe regression.

Thanks,
Guenter

>   drivers/spi/spi-mt65xx.c | 19 +++++--------------
>   1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> index 68dca8ceb3ad..7914255521c3 100644
> --- a/drivers/spi/spi-mt65xx.c
> +++ b/drivers/spi/spi-mt65xx.c
> @@ -426,24 +426,15 @@ static int mtk_spi_fifo_transfer(struct spi_master *master,
>   	mtk_spi_prepare_transfer(master, xfer);
>   	mtk_spi_setup_packet(master);
>   
> -	cnt = xfer->len / 4;
> -	if (xfer->tx_buf)
> +	if (xfer->tx_buf) {
> +		cnt = xfer->len / 4;
>   		iowrite32_rep(mdata->base + SPI_TX_DATA_REG, xfer->tx_buf, cnt);
> -
> -	if (xfer->rx_buf)
> -		ioread32_rep(mdata->base + SPI_RX_DATA_REG, xfer->rx_buf, cnt);
> -
> -	remainder = xfer->len % 4;
> -	if (remainder > 0) {
> -		reg_val = 0;
> -		if (xfer->tx_buf) {
> +		remainder = xfer->len % 4;
> +		if (remainder > 0) {
> +			reg_val = 0;
>   			memcpy(&reg_val, xfer->tx_buf + (cnt * 4), remainder);
>   			writel(reg_val, mdata->base + SPI_TX_DATA_REG);
>   		}
> -		if (xfer->rx_buf) {
> -			reg_val = readl(mdata->base + SPI_RX_DATA_REG);
> -			memcpy(xfer->rx_buf + (cnt * 4), &reg_val, remainder);
> -		}
>   	}
>   
>   	mtk_spi_enable_transfer(master);