LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] spi: mediatek: Fix fifo transfer
@ 2021-08-02  3:00 Guenter Roeck
  2021-08-02  3:27 ` Tzung-Bi Shih
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Guenter Roeck @ 2021-08-02  3:00 UTC (permalink / raw)
  To: Mark Brown
  Cc: Matthias Brugger, linux-spi, linux-arm-kernel, linux-mediatek,
	linux-kernel, Guenter Roeck, Peter Hess, Frank Wunderlich,
	Tzung-Bi Shih, Hsin-Yi Wang

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@ph-home.de>
Cc: Frank Wunderlich <frank-w@public-files.de>
Cc: Tzung-Bi Shih <tzungbi@google.com>
Cc: Hsin-Yi Wang <hsinyi@google.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 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);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] spi: mediatek: Fix fifo transfer
  2021-08-02  3:00 [PATCH] spi: mediatek: Fix fifo transfer Guenter Roeck
@ 2021-08-02  3:27 ` Tzung-Bi Shih
  2021-08-02  3:40   ` Hsin-Yi Wang
  2021-08-02 13:57 ` Guenter Roeck
  2021-08-03 22:35 ` Mark Brown
  2 siblings, 1 reply; 5+ messages in thread
From: Tzung-Bi Shih @ 2021-08-02  3:27 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Brown, Matthias Brugger, linux-spi, linux-arm-kernel,
	linux-mediatek, linux-kernel, Peter Hess, Frank Wunderlich,
	Hsin-Yi Wang

On Sun, Aug 01, 2021 at 08:00:23PM -0700, Guenter Roeck wrote:
> Fixes: 3a70dd2d0503 ("spi: mediatek: fix fifo rx mode")
> Cc: Peter Hess <peter.hess@ph-home.de>
> Cc: Frank Wunderlich <frank-w@public-files.de>
> Cc: Tzung-Bi Shih <tzungbi@google.com>
> Cc: Hsin-Yi Wang <hsinyi@google.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Tested-by: Tzung-Bi Shih <tzungbi@google.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] spi: mediatek: Fix fifo transfer
  2021-08-02  3:27 ` Tzung-Bi Shih
@ 2021-08-02  3:40   ` Hsin-Yi Wang
  0 siblings, 0 replies; 5+ messages in thread
From: Hsin-Yi Wang @ 2021-08-02  3:40 UTC (permalink / raw)
  To: Tzung-Bi Shih
  Cc: Guenter Roeck, Mark Brown, Matthias Brugger, linux-spi,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:ARM/Mediatek SoC support, lkml, Peter Hess,
	Frank Wunderlich, Hsin-Yi Wang

On Mon, Aug 2, 2021 at 11:27 AM Tzung-Bi Shih <tzungbi@google.com> wrote:
>
> On Sun, Aug 01, 2021 at 08:00:23PM -0700, Guenter Roeck wrote:
> > Fixes: 3a70dd2d0503 ("spi: mediatek: fix fifo rx mode")
> > Cc: Peter Hess <peter.hess@ph-home.de>
> > Cc: Frank Wunderlich <frank-w@public-files.de>
> > Cc: Tzung-Bi Shih <tzungbi@google.com>
> > Cc: Hsin-Yi Wang <hsinyi@google.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> Tested-by: Tzung-Bi Shih <tzungbi@google.com>
Tested-by: Hsin-Yi Wang <hsinyi@google.com>

tested on a mt8183 device.
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] spi: mediatek: Fix fifo transfer
  2021-08-02  3:00 [PATCH] spi: mediatek: Fix fifo transfer Guenter Roeck
  2021-08-02  3:27 ` Tzung-Bi Shih
@ 2021-08-02 13:57 ` Guenter Roeck
  2021-08-03 22:35 ` Mark Brown
  2 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2021-08-02 13:57 UTC (permalink / raw)
  To: Mark Brown
  Cc: Matthias Brugger, linux-spi, linux-arm-kernel, linux-mediatek,
	linux-kernel, Peter Hess, Frank Wunderlich, Tzung-Bi Shih,
	Hsin-Yi Wang

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@ph-home.de>
> Cc: Frank Wunderlich <frank-w@public-files.de>
> Cc: Tzung-Bi Shih <tzungbi@google.com>
> Cc: Hsin-Yi Wang <hsinyi@google.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---

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);
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] spi: mediatek: Fix fifo transfer
  2021-08-02  3:00 [PATCH] spi: mediatek: Fix fifo transfer Guenter Roeck
  2021-08-02  3:27 ` Tzung-Bi Shih
  2021-08-02 13:57 ` Guenter Roeck
@ 2021-08-03 22:35 ` Mark Brown
  2 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2021-08-03 22:35 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Brown, linux-spi, linux-mediatek, linux-kernel,
	Tzung-Bi Shih, Peter Hess, Hsin-Yi Wang, linux-arm-kernel,
	Matthias Brugger, Frank Wunderlich

On Sun, 1 Aug 2021 20:00:23 -0700, 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.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: mediatek: Fix fifo transfer
      commit: 0d5c3954b35eddff0da0436c31e8d721eceb7dc2

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-03 22:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02  3:00 [PATCH] spi: mediatek: Fix fifo transfer Guenter Roeck
2021-08-02  3:27 ` Tzung-Bi Shih
2021-08-02  3:40   ` Hsin-Yi Wang
2021-08-02 13:57 ` Guenter Roeck
2021-08-03 22:35 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).