diff mbox series

[v2,2/3] spi: omap2-mcspi: Add support for MULTI-mode

Message ID 20240223-spi-omap2-mcspi-multi-mode-v2-2-afe94476b9c3@bootlin.com
State New
Headers show
Series Add multi mode support for omap-mcspi | expand

Commit Message

Louis Chauvet Feb. 23, 2024, 9:32 a.m. UTC
Introduce support for MULTI-mode in the OMAP2 MCSPI driver. Currently, the
driver always uses SINGLE mode to handle the chip select (CS). With this
enhancement, MULTI-mode is enabled for specific messages, allowing for a
shorter delay between CS enable and the message (some FPGA devices are
sensitive to this delay).

The OMAP2 MCSPI device can use two different mode to send messages, SINGLE
and MULTI:
In SINGLE mode, the controller only leverages one single FIFO, and the
host system has to manually select the CS it wants to enable.
In MULTI mode, each CS is bound to a FIFO, the host system then writes the
data to the relevant FIFO, as the hardware will take care of the CS

The drawback of multi-mode is that it's not possible to keep the CS
enabled between each words. Therefore, this patch enables multi-mode only
for specific messages: the spi_message must contain only spi_transfer of 1
word (of any size) with cs_change enabled.

A new member is introduced in the omap2_mcspi structure to keep track of
the current used mode.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/spi/spi-omap2-mcspi.c | 67 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 62 insertions(+), 5 deletions(-)

Comments

Mark Brown March 25, 2024, 2:05 p.m. UTC | #1
On Fri, Feb 23, 2024 at 10:32:12AM +0100, Louis Chauvet wrote:
> Introduce support for MULTI-mode in the OMAP2 MCSPI driver. Currently, the
> driver always uses SINGLE mode to handle the chip select (CS). With this
> enhancement, MULTI-mode is enabled for specific messages, allowing for a
> shorter delay between CS enable and the message (some FPGA devices are
> sensitive to this delay).

This breaks an allmodconfig build:

/build/stage/linux/drivers/spi/spi-omap2-mcspi.c: In function ‘omap2_mcspi_prepare_message’:
/build/stage/linux/drivers/spi/spi-omap2-mcspi.c:1280:17: error: "/*" within comment [-Werror=comment]
 1280 |                 /*
      |                  
/build/stage/linux/drivers/spi/spi-omap2-mcspi.c:1265:14: error: unused variable ‘word_delay_is_zero’ [-Werror=unused-variable]
 1265 |         bool word_delay_is_zero;
      |              ^~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Louis Chauvet March 27, 2024, 9:27 a.m. UTC | #2
Le 25/03/24 - 14:05, Mark Brown a écrit :
> On Fri, Feb 23, 2024 at 10:32:12AM +0100, Louis Chauvet wrote:
> > Introduce support for MULTI-mode in the OMAP2 MCSPI driver. Currently, the
> > driver always uses SINGLE mode to handle the chip select (CS). With this
> > enhancement, MULTI-mode is enabled for specific messages, allowing for a
> > shorter delay between CS enable and the message (some FPGA devices are
> > sensitive to this delay).
> 
> This breaks an allmodconfig build:
> 
> /build/stage/linux/drivers/spi/spi-omap2-mcspi.c: In function ‘omap2_mcspi_prepare_message’:
> /build/stage/linux/drivers/spi/spi-omap2-mcspi.c:1280:17: error: "/*" within comment [-Werror=comment]
>  1280 |                 /*
>       |                  
> /build/stage/linux/drivers/spi/spi-omap2-mcspi.c:1265:14: error: unused variable ‘word_delay_is_zero’ [-Werror=unused-variable]
>  1265 |         bool word_delay_is_zero;
>       |              ^~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors

Hi Mark,

I missed this, sorry.

I've just sent a v3 [1] without those errors.

Thanks,
Louis Chauvet

[1]: https://lore.kernel.org/all/20240327-spi-omap2-mcspi-multi-mode-v3-0-c4ac329dd5a2@bootlin.com/
diff mbox series

Patch

diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index fc7f69973334..36075c4416d5 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -133,6 +133,7 @@  struct omap2_mcspi {
 	unsigned int		pin_dir:1;
 	size_t			max_xfer_len;
 	u32			ref_clk_hz;
+	bool			use_multi_mode;
 };
 
 struct omap2_mcspi_cs {
@@ -258,10 +259,15 @@  static void omap2_mcspi_set_cs(struct spi_device *spi, bool enable)
 
 		l = mcspi_cached_chconf0(spi);
 
-		if (enable)
+		/* Only enable chip select manually if single mode is used */
+		if (mcspi->use_multi_mode) {
 			l &= ~OMAP2_MCSPI_CHCONF_FORCE;
-		else
-			l |= OMAP2_MCSPI_CHCONF_FORCE;
+		} else {
+			if (enable)
+				l &= ~OMAP2_MCSPI_CHCONF_FORCE;
+			else
+				l |= OMAP2_MCSPI_CHCONF_FORCE;
+		}
 
 		mcspi_write_chconf0(spi, l);
 
@@ -285,7 +291,12 @@  static void omap2_mcspi_set_mode(struct spi_controller *ctlr)
 		l |= (OMAP2_MCSPI_MODULCTRL_MS);
 	} else {
 		l &= ~(OMAP2_MCSPI_MODULCTRL_MS);
-		l |= OMAP2_MCSPI_MODULCTRL_SINGLE;
+
+		/* Enable single mode if needed */
+		if (mcspi->use_multi_mode)
+			l &= ~OMAP2_MCSPI_MODULCTRL_SINGLE;
+		else
+			l |= OMAP2_MCSPI_MODULCTRL_SINGLE;
 	}
 	mcspi_write_reg(ctlr, OMAP2_MCSPI_MODULCTRL, l);
 
@@ -1371,15 +1382,61 @@  static int omap2_mcspi_prepare_message(struct spi_controller *ctlr,
 	struct omap2_mcspi	*mcspi = spi_controller_get_devdata(ctlr);
 	struct omap2_mcspi_regs	*ctx = &mcspi->ctx;
 	struct omap2_mcspi_cs	*cs;
+	struct spi_transfer	*tr;
+	bool word_delay_is_zero;
+	u8 bits_per_word;
+
+	/*
+	 * The conditions are strict, it is mandatory to check each transfer of the list to see if
+	 * multi-mode is applicable.
+	 */
+	mcspi->use_multi_mode = true;
+	list_for_each_entry(tr, &msg->transfers, transfer_list) {
+		if (!tr->bits_per_word)
+			bits_per_word = msg->spi->bits_per_word;
+		else
+			bits_per_word = tr->bits_per_word;
 
 	/* Only a single channel can have the FORCE bit enabled
+		/*
+		 * Check if this transfer contains only one word;
+		 */
+		if (bits_per_word < 8 && tr->len == 1) {
+			/* multi-mode is applicable, only one word (1..7 bits) */
+		} else if (bits_per_word >= 8 && tr->len == bits_per_word / 8) {
+			/* multi-mode is applicable, only one word (8..32 bits) */
+		} else {
+			/* multi-mode is not applicable: more than one word in the transfer */
+			mcspi->use_multi_mode = false;
+		}
+
+		/* Check if transfer asks to change the CS status after the transfer */
+		if (!tr->cs_change)
+			mcspi->use_multi_mode = false;
+
+		/*
+		 * If at least one message is not compatible, switch back to single mode
+		 *
+		 * The bits_per_word of certain transfer can be different, but it will have no
+		 * impact on the signal itself.
+		 */
+		if (!mcspi->use_multi_mode)
+			break;
+	}
+
+	omap2_mcspi_set_mode(ctlr);
+
+	/* In single mode only a single channel can have the FORCE bit enabled
 	 * in its chconf0 register.
 	 * Scan all channels and disable them except the current one.
 	 * A FORCE can remain from a last transfer having cs_change enabled
+	 *
+	 * In multi mode all FORCE bits must be disabled.
 	 */
 	list_for_each_entry(cs, &ctx->cs, node) {
-		if (msg->spi->controller_state == cs)
+		if (msg->spi->controller_state == cs && !mcspi->use_multi_mode) {
 			continue;
+		}
 
 		if ((cs->chconf0 & OMAP2_MCSPI_CHCONF_FORCE)) {
 			cs->chconf0 &= ~OMAP2_MCSPI_CHCONF_FORCE;