| Message ID | 20210802030023.1748777-1-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0d5c3954b35eddff0da0436c31e8d721eceb7dc2 |
| Headers | show |
| Series | spi: mediatek: Fix fifo transfer | expand |
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>
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
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(®_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), ®_val, remainder); > - } > } > > mtk_spi_enable_transfer(master); >
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
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(®_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), ®_val, remainder); - } } mtk_spi_enable_transfer(master);
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(-)