[PATCH v2 0/3] spi: microchip-core-qspi: Add regular transfers

[PATCH v2 0/3] spi: microchip-core-qspi: Add regular transfers

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

Hey,

This is a v2 of a patchset I sent about this time last year, adding the
regular transfer_one_message op to the microchip-core-qspi driver. In that
v1 Mark expressed his dislike for that op, so v2 is using
prepare/unprepare/transfer_one instead. The unprepare implementation still
contains the 750 us delay that the driver had back in v1. I've heard a
suggestion internally as to why this is needed, but it was unsubstantiated,
so I still have no justification for it. I held off on sending a v2 because
of a lack of explanation for the delay, but I don't wanna hold off forever
for something I might never understand.

Cheers,
Conor.

v1: https://lore.kernel.org/linux-spi/20240612-brigade-shell-1f626e7e592f@spud/

CC: Conor Dooley <conor.dooley@microchip.com>
CC: Daire McNamara <daire.mcnamara@microchip.com>
CC: Mark Brown <broonie@kernel.org>
CC: linux-spi@vger.kernel.org
CC: linux-kernel@vger.kernel.org

Conor Dooley (2):
  spi: microchip-core-qspi: set min_speed_hz during probe
  spi: microchip-core-qspi: remove unused param from
    mchp_coreqspi_write_op()

Cyril Jean (1):
  spi: microchip-core-qspi: Add regular transfers

 drivers/spi/spi-microchip-core-qspi.c | 226 +++++++++++++++++++++++---
 1 file changed, 204 insertions(+), 22 deletions(-)

-- 
2.45.2

[PATCH v2 1/3] spi: microchip-core-qspi: set min_speed_hz during probe

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The controller's minimum possible bus clock is 1/30 the rate of the
input clock. Naively set the minimum bus clock speed the controller
is capable of during probe, assuming that the rate will never reduce
further.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/spi/spi-microchip-core-qspi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-microchip-core-qspi.c b/drivers/spi/spi-microchip-core-qspi.c
index fa828fcaaef2d..111ae6519ff41 100644
--- a/drivers/spi/spi-microchip-core-qspi.c
+++ b/drivers/spi/spi-microchip-core-qspi.c
@@ -562,6 +562,7 @@ static int mchp_coreqspi_probe(struct platform_device *pdev)
 	ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
 			  SPI_TX_DUAL | SPI_TX_QUAD;
 	ctlr->dev.of_node = np;
+	ctlr->min_speed_hz = clk_get_rate(qspi->clk) / 30;
 
 	ret = devm_spi_register_controller(&pdev->dev, ctlr);
 	if (ret)
-- 
2.45.2

[PATCH v2 2/3] spi: microchip-core-qspi: remove unused param from mchp_coreqspi_write_op()

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

"word" is unused in mchp_coreqspi_write_op(), so delete it.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/spi/spi-microchip-core-qspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-microchip-core-qspi.c b/drivers/spi/spi-microchip-core-qspi.c
index 111ae6519ff41..67ff5f8aa84d0 100644
--- a/drivers/spi/spi-microchip-core-qspi.c
+++ b/drivers/spi/spi-microchip-core-qspi.c
@@ -194,7 +194,7 @@ static inline void mchp_coreqspi_read_op(struct mchp_coreqspi *qspi)
 	}
 }
 
-static inline void mchp_coreqspi_write_op(struct mchp_coreqspi *qspi, bool word)
+static inline void mchp_coreqspi_write_op(struct mchp_coreqspi *qspi)
 {
 	u32 control, data;
 
@@ -415,7 +415,7 @@ static int mchp_coreqspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *o
 		qspi->rxbuf = NULL;
 		qspi->tx_len = op->cmd.nbytes;
 		qspi->rx_len = 0;
-		mchp_coreqspi_write_op(qspi, false);
+		mchp_coreqspi_write_op(qspi);
 	}
 
 	qspi->txbuf = &opaddr[0];
@@ -426,7 +426,7 @@ static int mchp_coreqspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *o
 		qspi->rxbuf = NULL;
 		qspi->tx_len = op->addr.nbytes;
 		qspi->rx_len = 0;
-		mchp_coreqspi_write_op(qspi, false);
+		mchp_coreqspi_write_op(qspi);
 	}
 
 	if (op->data.nbytes) {
@@ -435,7 +435,7 @@ static int mchp_coreqspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *o
 			qspi->rxbuf = NULL;
 			qspi->rx_len = 0;
 			qspi->tx_len = op->data.nbytes;
-			mchp_coreqspi_write_op(qspi, true);
+			mchp_coreqspi_write_op(qspi);
 		} else {
 			qspi->txbuf = NULL;
 			qspi->rxbuf = (u8 *)op->data.buf.in;
-- 
2.45.2

[PATCH v2 3/3] spi: microchip-core-qspi: Add regular transfers

[Conor Dooley]

From: Cyril Jean <cyril.jean@microchip.com>

The driver for CoreQSPI only supports memory operations at present, so
add support for regular transfers so that the SD card slot and ADC on
the BeagleV Fire can be used.

Signed-off-by: Cyril Jean <cyril.jean@microchip.com>
Co-developed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/spi/spi-microchip-core-qspi.c | 217 +++++++++++++++++++++++---
 1 file changed, 199 insertions(+), 18 deletions(-)

diff --git a/drivers/spi/spi-microchip-core-qspi.c b/drivers/spi/spi-microchip-core-qspi.c
index 67ff5f8aa84d0..d13a9b755c7f8 100644
--- a/drivers/spi/spi-microchip-core-qspi.c
+++ b/drivers/spi/spi-microchip-core-qspi.c
@@ -222,6 +222,87 @@ static inline void mchp_coreqspi_write_op(struct mchp_coreqspi *qspi)
 	}
 }
 
+static inline void mchp_coreqspi_write_read_op(struct mchp_coreqspi *qspi)
+{
+	u32 control, data;
+
+	qspi->rx_len = qspi->tx_len;
+
+	control = readl_relaxed(qspi->regs + REG_CONTROL);
+	control |= CONTROL_FLAGSX4;
+	writel_relaxed(control, qspi->regs + REG_CONTROL);
+
+	while (qspi->tx_len >= 4) {
+		while (readl_relaxed(qspi->regs + REG_STATUS) & STATUS_TXFIFOFULL)
+			;
+
+		data = qspi->txbuf ? *((u32 *)qspi->txbuf) : 0xaa;
+		if (qspi->txbuf)
+			qspi->txbuf += 4;
+		qspi->tx_len -= 4;
+		writel_relaxed(data, qspi->regs + REG_X4_TX_DATA);
+
+		/*
+		 * The rx FIFO is twice the size of the tx FIFO, so there is
+		 * no requirement to block transmission if receive data is not
+		 * ready, and it is fine to let the tx FIFO completely fill
+		 * without reading anything from the rx FIFO. Once the tx FIFO
+		 * has been filled and becomes non-full due to a transmission
+		 * occurring there will always be something to receive.
+		 * IOW, this is safe as TX_FIFO_SIZE + 4 < 2 * TX_FIFO_SIZE
+		 */
+		if (qspi->rx_len >= 4) {
+			if (readl_relaxed(qspi->regs + REG_STATUS) & STATUS_RXAVAILABLE) {
+				data = readl_relaxed(qspi->regs + REG_X4_RX_DATA);
+				*(u32 *)qspi->rxbuf = data;
+				qspi->rxbuf += 4;
+				qspi->rx_len -= 4;
+			}
+		}
+	}
+
+	/*
+	 * Since transmission is not being blocked by clearing the rx FIFO,
+	 * loop here until all received data "leaked" by the loop above has
+	 * been dealt with.
+	 */
+	while (qspi->rx_len >= 4) {
+		while (readl_relaxed(qspi->regs + REG_STATUS) & STATUS_RXFIFOEMPTY)
+			;
+		data = readl_relaxed(qspi->regs + REG_X4_RX_DATA);
+		*(u32 *)qspi->rxbuf = data;
+		qspi->rxbuf += 4;
+		qspi->rx_len -= 4;
+	}
+
+	/*
+	 * Since rx_len and tx_len must be < 4 bytes at this point, there's no
+	 * concern about overflowing the rx or tx FIFOs any longer. It's
+	 * therefore safe to loop over the remainder of the transmit data before
+	 * handling the remaining receive data.
+	 */
+	if (!qspi->tx_len)
+		return;
+
+	control &= ~CONTROL_FLAGSX4;
+	writel_relaxed(control, qspi->regs + REG_CONTROL);
+
+	while (qspi->tx_len--) {
+		while (readl_relaxed(qspi->regs + REG_STATUS) & STATUS_TXFIFOFULL)
+			;
+		data = qspi->txbuf ? *qspi->txbuf : 0xaa;
+		qspi->txbuf++;
+		writel_relaxed(data, qspi->regs + REG_TX_DATA);
+	}
+
+	while (qspi->rx_len--) {
+		while (readl_relaxed(qspi->regs + REG_STATUS) & STATUS_RXFIFOEMPTY)
+			;
+		data = readl_relaxed(qspi->regs + REG_RX_DATA);
+		*qspi->rxbuf++ = (data & 0xFF);
+	}
+}
+
 static void mchp_coreqspi_enable_ints(struct mchp_coreqspi *qspi)
 {
 	u32 mask = IEN_TXDONE |
@@ -266,7 +347,7 @@ static irqreturn_t mchp_coreqspi_isr(int irq, void *dev_id)
 }
 
 static int mchp_coreqspi_setup_clock(struct mchp_coreqspi *qspi, struct spi_device *spi,
-				     const struct spi_mem_op *op)
+				     u32 max_freq)
 {
 	unsigned long clk_hz;
 	u32 control, baud_rate_val = 0;
@@ -275,11 +356,11 @@ static int mchp_coreqspi_setup_clock(struct mchp_coreqspi *qspi, struct spi_devi
 	if (!clk_hz)
 		return -EINVAL;
 
-	baud_rate_val = DIV_ROUND_UP(clk_hz, 2 * op->max_freq);
+	baud_rate_val = DIV_ROUND_UP(clk_hz, 2 * max_freq);
 	if (baud_rate_val > MAX_DIVIDER || baud_rate_val < MIN_DIVIDER) {
 		dev_err(&spi->dev,
 			"could not configure the clock for spi clock %d Hz & system clock %ld Hz\n",
-			op->max_freq, clk_hz);
+			max_freq, clk_hz);
 		return -EINVAL;
 	}
 
@@ -367,23 +448,13 @@ static inline void mchp_coreqspi_config_op(struct mchp_coreqspi *qspi, const str
 	writel_relaxed(frames, qspi->regs + REG_FRAMES);
 }
 
-static int mchp_qspi_wait_for_ready(struct spi_mem *mem)
+static int mchp_coreqspi_wait_for_ready(struct mchp_coreqspi *qspi)
 {
-	struct mchp_coreqspi *qspi = spi_controller_get_devdata
-				    (mem->spi->controller);
 	u32 status;
-	int ret;
 
-	ret = readl_poll_timeout(qspi->regs + REG_STATUS, status,
+	return readl_poll_timeout(qspi->regs + REG_STATUS, status,
 				 (status & STATUS_READY), 0,
 				 TIMEOUT_MS);
-	if (ret) {
-		dev_err(&mem->spi->dev,
-			"Timeout waiting on QSPI ready.\n");
-		return -ETIMEDOUT;
-	}
-
-	return ret;
 }
 
 static int mchp_coreqspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
@@ -396,11 +467,13 @@ static int mchp_coreqspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *o
 	int err, i;
 
 	mutex_lock(&qspi->op_lock);
-	err = mchp_qspi_wait_for_ready(mem);
-	if (err)
+	err = mchp_coreqspi_wait_for_ready(qspi);
+	if (err) {
+		dev_err(&mem->spi->dev, "Timeout waiting on QSPI ready.\n");
 		goto error;
+	}
 
-	err = mchp_coreqspi_setup_clock(qspi, mem->spi, op);
+	err = mchp_coreqspi_setup_clock(qspi, mem->spi, op->max_freq);
 	if (err)
 		goto error;
 
@@ -515,6 +588,109 @@ static const struct spi_controller_mem_caps mchp_coreqspi_mem_caps = {
 	.per_op_freq = true,
 };
 
+static int mchp_coreqspi_unprepare_message(struct spi_controller *ctlr, struct spi_message *m)
+{
+	struct mchp_coreqspi *qspi = spi_controller_get_devdata(ctlr);
+
+	/*
+	 * This delay is required for the driver to function correctly,
+	 * but no explanation has been determined for why it is required.
+	 */
+	udelay(750);
+
+	mutex_unlock(&qspi->op_lock);
+
+	return 0;
+}
+
+static int mchp_coreqspi_prepare_message(struct spi_controller *ctlr, struct spi_message *m)
+{
+	struct mchp_coreqspi *qspi = spi_controller_get_devdata(ctlr);
+	struct spi_transfer *t = NULL;
+	u32 control, frames;
+	u32 total_bytes = 0, cmd_bytes = 0, idle_cycles = 0;
+	int ret;
+	bool quad = false, dual = false;
+
+	mutex_lock(&qspi->op_lock);
+	ret = mchp_coreqspi_wait_for_ready(qspi);
+	if (ret) {
+		mutex_unlock(&qspi->op_lock);
+		dev_err(&ctlr->dev, "Timeout waiting on QSPI ready.\n");
+		return ret;
+	}
+
+	ret = mchp_coreqspi_setup_clock(qspi, m->spi, m->spi->max_speed_hz);
+	if (ret) {
+		mutex_unlock(&qspi->op_lock);
+		return ret;
+	}
+
+	control = readl_relaxed(qspi->regs + REG_CONTROL);
+	control &= ~(CONTROL_MODE12_MASK | CONTROL_MODE0);
+	writel_relaxed(control, qspi->regs + REG_CONTROL);
+
+	reinit_completion(&qspi->data_completion);
+
+	list_for_each_entry(t, &m->transfers, transfer_list) {
+		total_bytes += t->len;
+		if (!cmd_bytes && !(t->tx_buf && t->rx_buf))
+			cmd_bytes = t->len;
+		if (!t->rx_buf)
+			cmd_bytes = total_bytes;
+		if (t->tx_nbits == SPI_NBITS_QUAD || t->rx_nbits == SPI_NBITS_QUAD)
+			quad = true;
+		else if (t->tx_nbits == SPI_NBITS_DUAL || t->rx_nbits == SPI_NBITS_DUAL)
+			dual = true;
+	}
+
+	control = readl_relaxed(qspi->regs + REG_CONTROL);
+	if (quad) {
+		control |= (CONTROL_MODE0 | CONTROL_MODE12_EX_RW);
+	} else if (dual) {
+		control &= ~CONTROL_MODE0;
+		control |= CONTROL_MODE12_FULL;
+	} else {
+		control &= ~(CONTROL_MODE12_MASK | CONTROL_MODE0);
+	}
+	writel_relaxed(control, qspi->regs + REG_CONTROL);
+
+	frames = total_bytes & BYTESUPPER_MASK;
+	writel_relaxed(frames, qspi->regs + REG_FRAMESUP);
+	frames = total_bytes & BYTESLOWER_MASK;
+	frames |= cmd_bytes << FRAMES_CMDBYTES_SHIFT;
+	frames |= idle_cycles << FRAMES_IDLE_SHIFT;
+	control = readl_relaxed(qspi->regs + REG_CONTROL);
+	if (control & CONTROL_MODE12_MASK)
+		frames |= (1 << FRAMES_SHIFT);
+
+	frames |= FRAMES_FLAGWORD;
+	writel_relaxed(frames, qspi->regs + REG_FRAMES);
+
+	return 0;
+};
+
+static int mchp_coreqspi_transfer_one(struct spi_controller *ctlr, struct spi_device *spi,
+				      struct spi_transfer *t)
+{
+	struct mchp_coreqspi *qspi = spi_controller_get_devdata(ctlr);
+
+	qspi->tx_len = t->len;
+
+	if (t->tx_buf)
+		qspi->txbuf = (u8 *)t->tx_buf;
+
+	if (!t->rx_buf) {
+		mchp_coreqspi_write_op(qspi);
+	} else {
+		qspi->rxbuf = (u8 *)t->rx_buf;
+		qspi->rx_len = t->len;
+		mchp_coreqspi_write_read_op(qspi);
+	}
+
+	return 0;
+}
+
 static int mchp_coreqspi_probe(struct platform_device *pdev)
 {
 	struct spi_controller *ctlr;
@@ -563,6 +739,11 @@ static int mchp_coreqspi_probe(struct platform_device *pdev)
 			  SPI_TX_DUAL | SPI_TX_QUAD;
 	ctlr->dev.of_node = np;
 	ctlr->min_speed_hz = clk_get_rate(qspi->clk) / 30;
+	ctlr->prepare_message = mchp_coreqspi_prepare_message;
+	ctlr->unprepare_message = mchp_coreqspi_unprepare_message;
+	ctlr->transfer_one = mchp_coreqspi_transfer_one;
+	ctlr->num_chipselect = 2;
+	ctlr->use_gpio_descriptors = true;
 
 	ret = devm_spi_register_controller(&pdev->dev, ctlr);
 	if (ret)
-- 
2.45.2

Re: [PATCH v2 0/3] spi: microchip-core-qspi: Add regular transfers

[Mark Brown]

On Fri, 20 Jun 2025 14:28:23 +0100, Conor Dooley wrote:
> Hey,
> 
> This is a v2 of a patchset I sent about this time last year, adding the
> regular transfer_one_message op to the microchip-core-qspi driver. In that
> v1 Mark expressed his dislike for that op, so v2 is using
> prepare/unprepare/transfer_one instead. The unprepare implementation still
> contains the 750 us delay that the driver had back in v1. I've heard a
> suggestion internally as to why this is needed, but it was unsubstantiated,
> so I still have no justification for it. I held off on sending a v2 because
> of a lack of explanation for the delay, but I don't wanna hold off forever
> for something I might never understand.
> 
> [...]

Applied to

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

Thanks!

[1/3] spi: microchip-core-qspi: set min_speed_hz during probe
      commit: 76f03ce1c6f22805ecf689b1f3ecfb56582eddd5
[2/3] spi: microchip-core-qspi: remove unused param from mchp_coreqspi_write_op()
      commit: 75ca45c472dac206df2ebbc1c0f1f9c3bbdace50
[3/3] spi: microchip-core-qspi: Add regular transfers
      commit: 1256eb42db5d1635f4c6da5b1b58db0b53320883

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