Linux SPI subsystem development
 help / color / mirror / Atom feed
From: Conor Dooley <conor.dooley@microchip.com>
To: <linux-spi@vger.kernel.org>
Cc: <conor@kernel.org>, <conor.dooley@microchip.com>,
	Steve Wilkins <steve.wilkins@raymarine.com>,
	Daire McNamara <daire.mcnamara@microchip.com>,
	Mark Brown <broonie@kernel.org>, <linux-kernel@vger.kernel.org>,
	"Naga Sureshkumar Relli" <nagasuresh.relli@microchip.com>
Subject: [PATCH v1 1/6] spi: microchip-core: fix the issues in the isr
Date: Mon, 15 Jul 2024 12:13:52 +0100	[thread overview]
Message-ID: <20240715-candied-deforest-585685ef3c8a@wendy> (raw)
In-Reply-To: <20240715-retail-magnolia-bbd49a657a89@wendy>

From: Naga Sureshkumar Relli <nagasuresh.relli@microchip.com>

It is possible for the TXDONE interrupt be raised if the tx FIFO becomes
temporarily empty while transmitting, resulting in recursive calls to
mchp_corespi_write_fifo() and therefore a garbage message might be
transmitted depending on when the interrupt is triggered. Moving all of
the tx FIFO writes out of the TXDONE portion of the interrupt handler
avoids this problem.

Most of rest of the TXDONE portion of the handler is problematic too.
Only reading the rx FIFO (and finalising the transfer) when the TXDONE
interrupt is raised can cause the transfer to stall, if the final bytes
of rx data are not available in the rx FIFO when the final TXDONE
interrupt is raised. The transfer should be finalised regardless of
which interrupt is raised, provided that all tx data has been set and
all rx data received.

The first issue was encountered "in the wild", the second is
theoretical.

Fixes: 9ac8d17694b6 ("spi: add support for microchip fpga spi controllers")
Signed-off-by: Naga Sureshkumar Relli <nagasuresh.relli@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/spi/spi-microchip-core.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c
index 634364c7cfe61..a81e1317d52e6 100644
--- a/drivers/spi/spi-microchip-core.c
+++ b/drivers/spi/spi-microchip-core.c
@@ -380,21 +380,18 @@ static irqreturn_t mchp_corespi_interrupt(int irq, void *dev_id)
 	if (intfield == 0)
 		return IRQ_NONE;
 
-	if (intfield & INT_TXDONE) {
+	if (intfield & INT_TXDONE)
 		mchp_corespi_write(spi, REG_INT_CLEAR, INT_TXDONE);
 
+	if (intfield & INT_RXRDY) {
+		mchp_corespi_write(spi, REG_INT_CLEAR, INT_RXRDY);
+
 		if (spi->rx_len)
 			mchp_corespi_read_fifo(spi);
-
-		if (spi->tx_len)
-			mchp_corespi_write_fifo(spi);
-
-		if (!spi->rx_len)
-			finalise = true;
 	}
 
-	if (intfield & INT_RXRDY)
-		mchp_corespi_write(spi, REG_INT_CLEAR, INT_RXRDY);
+	if (!spi->rx_len && !spi->tx_len)
+		finalise = true;
 
 	if (intfield & INT_RX_CHANNEL_OVERFLOW) {
 		mchp_corespi_write(spi, REG_INT_CLEAR, INT_RX_CHANNEL_OVERFLOW);
@@ -479,8 +476,9 @@ static int mchp_corespi_transfer_one(struct spi_controller *host,
 	mchp_corespi_set_xfer_size(spi, (spi->tx_len > FIFO_DEPTH)
 				   ? FIFO_DEPTH : spi->tx_len);
 
-	if (spi->tx_len)
+	while (spi->tx_len)
 		mchp_corespi_write_fifo(spi);
+
 	return 1;
 }
 
-- 
2.43.2


  reply	other threads:[~2024-07-15 11:14 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-15 11:13 [PATCH v1 0/6] spi-microchip-core fixes & variable word size support Conor Dooley
2024-07-15 11:13 ` Conor Dooley [this message]
2024-07-15 11:13 ` [PATCH v1 2/6] spi: microchip-core: defer asserting chip select until just before write to TX FIFO Conor Dooley
2024-07-15 11:13 ` [PATCH v1 3/6] spi: microchip-core: only disable SPI controller when register value change requires it Conor Dooley
2024-07-15 11:13 ` [PATCH v1 4/6] spi: microchip-core: fix init function not setting the master and motorola modes Conor Dooley
2024-07-15 11:13 ` [PATCH v1 5/6] spi: microchip-core: ensure TX and RX FIFOs are empty at start of a transfer Conor Dooley
2024-07-15 11:13 ` [PATCH v1 6/6] spi: microchip-core: add support for word sizes of 1 to 32 bits Conor Dooley
2024-07-16 13:26 ` [PATCH v1 0/6] spi-microchip-core fixes & variable word size support Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240715-candied-deforest-585685ef3c8a@wendy \
    --to=conor.dooley@microchip.com \
    --cc=broonie@kernel.org \
    --cc=conor@kernel.org \
    --cc=daire.mcnamara@microchip.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=nagasuresh.relli@microchip.com \
    --cc=steve.wilkins@raymarine.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox