All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Lemon <jonathan.lemon@gmail.com>
To: <ribalda@kernel.org>, <broonie@kernel.org>
Cc: <linux-spi@vger.kernel.org>, <kernel-team@fb.com>
Subject: [PATCH] spi: xilinx: Inhibit transmitter while filling TX FIFO
Date: Fri, 7 May 2021 14:53:19 -0700	[thread overview]
Message-ID: <20210507215319.3882138-1-jonathan.lemon@gmail.com> (raw)

Contrary to the comment in xilinx_spi_txrx_bufs(), the transmitter
was not disabled on entry.  On my particular board, when sending a PP
sequence, the first address byte was clocked out 3 times, writing data
into the wrong location, and generally locking up the chip.

Fix this by inhibiting the transmitter at initialization time, and
then enabling/disabling it appropriately.

With this patch, I can successfully read/erase/program the flash.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/spi/spi-xilinx.c | 54 +++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-xilinx.c b/drivers/spi/spi-xilinx.c
index 523edfdf5dcd..10eccfb09e20 100644
--- a/drivers/spi/spi-xilinx.c
+++ b/drivers/spi/spi-xilinx.c
@@ -179,8 +179,8 @@ static void xspi_init_hw(struct xilinx_spi *xspi)
 	/* Disable the transmitter, enable Manual Slave Select Assertion,
 	 * put SPI controller into master mode, and enable it */
 	xspi->write_fn(XSPI_CR_MANUAL_SSELECT |	XSPI_CR_MASTER_MODE |
-		XSPI_CR_ENABLE | XSPI_CR_TXFIFO_RESET |	XSPI_CR_RXFIFO_RESET,
-		regs_base + XSPI_CR_OFFSET);
+		XSPI_CR_ENABLE | XSPI_CR_TXFIFO_RESET |	XSPI_CR_RXFIFO_RESET |
+		XSPI_CR_TRANS_INHIBIT, regs_base + XSPI_CR_OFFSET);
 }
 
 static void xilinx_spi_chipselect(struct spi_device *spi, int is_on)
@@ -235,14 +235,46 @@ static int xilinx_spi_setup_transfer(struct spi_device *spi,
 	return 0;
 }
 
+static void
+xilinx_spi_inhibit_master(struct xilinx_spi *xspi, bool inhibit)
+{
+	u16 cr;
+
+	cr = xspi->read_fn(xspi->regs + XSPI_CR_OFFSET);
+	if (inhibit)
+		cr |= XSPI_CR_TRANS_INHIBIT;
+	else
+		cr &= ~XSPI_CR_TRANS_INHIBIT;
+	xspi->write_fn(cr, xspi->regs + XSPI_CR_OFFSET);
+}
+
+static void
+xilinx_spi_enable_transfer(struct xilinx_spi *xspi)
+{
+	xilinx_spi_inhibit_master(xspi, false);
+}
+
+static void
+xilinx_spi_disable_transfer(struct xilinx_spi *xspi)
+{
+	xilinx_spi_inhibit_master(xspi, true);
+}
+
+static bool
+xilinx_spi_is_transfer_disabled(struct xilinx_spi *xspi)
+{
+	u16 cr = xspi->read_fn(xspi->regs + XSPI_CR_OFFSET);
+	return !!(cr & XSPI_CR_TRANS_INHIBIT);
+}
+
 static int xilinx_spi_txrx_bufs(struct spi_device *spi, struct spi_transfer *t)
 {
 	struct xilinx_spi *xspi = spi_master_get_devdata(spi->master);
 	int remaining_words;	/* the number of words left to transfer */
 	bool use_irq = false;
-	u16 cr = 0;
 
 	/* We get here with transmitter inhibited */
+	WARN_ON_ONCE(!xilinx_spi_is_transfer_disabled(xspi));
 
 	xspi->tx_ptr = t->tx_buf;
 	xspi->rx_ptr = t->rx_buf;
@@ -252,14 +284,13 @@ static int xilinx_spi_txrx_bufs(struct spi_device *spi, struct spi_transfer *t)
 		u32 isr;
 		use_irq = true;
 		/* Inhibit irq to avoid spurious irqs on tx_empty*/
-		cr = xspi->read_fn(xspi->regs + XSPI_CR_OFFSET);
-		xspi->write_fn(cr | XSPI_CR_TRANS_INHIBIT,
-			       xspi->regs + XSPI_CR_OFFSET);
+
 		/* ACK old irqs (if any) */
 		isr = xspi->read_fn(xspi->regs + XIPIF_V123B_IISR_OFFSET);
 		if (isr)
 			xspi->write_fn(isr,
 				       xspi->regs + XIPIF_V123B_IISR_OFFSET);
+
 		/* Enable the global IPIF interrupt */
 		xspi->write_fn(XIPIF_V123B_GINTR_ENABLE,
 				xspi->regs + XIPIF_V123B_DGIER_OFFSET);
@@ -280,9 +311,9 @@ static int xilinx_spi_txrx_bufs(struct spi_device *spi, struct spi_transfer *t)
 		/* Start the transfer by not inhibiting the transmitter any
 		 * longer
 		 */
+		xilinx_spi_enable_transfer(xspi);
 
 		if (use_irq) {
-			xspi->write_fn(cr, xspi->regs + XSPI_CR_OFFSET);
 			wait_for_completion(&xspi->done);
 			/* A transmit has just completed. Process received data
 			 * and check for more data to transmit. Always inhibit
@@ -290,8 +321,7 @@ static int xilinx_spi_txrx_bufs(struct spi_device *spi, struct spi_transfer *t)
 			 * register/FIFO, or make sure it is stopped if we're
 			 * done.
 			 */
-			xspi->write_fn(cr | XSPI_CR_TRANS_INHIBIT,
-				       xspi->regs + XSPI_CR_OFFSET);
+			xilinx_spi_disable_transfer(xspi);
 			sr = XSPI_SR_TX_EMPTY_MASK;
 		} else
 			sr = xspi->read_fn(xspi->regs + XSPI_SR_OFFSET);
@@ -325,10 +355,10 @@ static int xilinx_spi_txrx_bufs(struct spi_device *spi, struct spi_transfer *t)
 		remaining_words -= n_words;
 	}
 
-	if (use_irq) {
+	if (use_irq)
 		xspi->write_fn(0, xspi->regs + XIPIF_V123B_DGIER_OFFSET);
-		xspi->write_fn(cr, xspi->regs + XSPI_CR_OFFSET);
-	}
+
+	xilinx_spi_disable_transfer(xspi);
 
 	return t->len;
 }
-- 
2.27.0


             reply	other threads:[~2021-05-07 21:53 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-07 21:53 Jonathan Lemon [this message]
2021-05-07 23:02 ` [PATCH] spi: xilinx: Inhibit transmitter while filling TX FIFO Ricardo Ribalda Delgado
     [not found]   ` <3E382801-224D-4B11-961D-4822F51F5496@gmail.com>
2021-05-08  2:44     ` Jonathan Lemon
2021-05-08 10:02       ` Ricardo Ribalda Delgado
2021-05-08 10:01     ` Ricardo Ribalda Delgado

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=20210507215319.3882138-1-jonathan.lemon@gmail.com \
    --to=jonathan.lemon@gmail.com \
    --cc=broonie@kernel.org \
    --cc=kernel-team@fb.com \
    --cc=linux-spi@vger.kernel.org \
    --cc=ribalda@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.