[PATCH] [SPI] build as module and fix priority inversion problem

[PATCH] [SPI] build as module and fix priority inversion problem

[Mark Underwood]

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

Sorry, made a bit of a mess with the last e-mail :(. Trying again :) 
 
This patch fix's the possible priority inversion that vitaly pointed out and allows the driver to
be built as a module. 
 
David, please can you test this patch. Can you also submit a patch which adds the module author,
description and license please, thanks. Kconfig |    2 +-

 spi.c   |   23 ++++++++++++++++++-----
 2 files changed, 19 insertions(+), 6 deletions(-)


	
	
		
___________________________________________________________ 
Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 1747405766-spi-dir.patch --]
[-- Type: text/x-patch; name="spi-dir.patch", Size: 2191 bytes --]

diff -uprN spi-org/Kconfig spi/Kconfig
--- spi-org/Kconfig	2005-12-05 19:49:52.000000000 +0000
+++ spi/Kconfig	2005-12-05 18:38:06.000000000 +0000
@@ -22,7 +22,7 @@ config SPI_ARCH_HAS_SLAVE
 	default y if ARCH_PXA
 
 config SPI
-	bool "SPI support"
+	tristate "SPI support"
 	depends on SPI_ARCH_HAS_MASTER || SPI_ARCH_HAS_SLAVE
 	help
 	  The "Serial Peripheral Interface" is a low level synchronous
diff -uprN spi-org/spi.c spi/spi.c
--- spi-org/spi.c	2005-12-05 19:49:52.000000000 +0000
+++ spi/spi.c	2005-12-05 19:06:05.000000000 +0000
@@ -232,6 +232,8 @@ EXPORT_SYMBOL_GPL(spi_new_device);
  * The board info passed can safely be __initdata ... but be careful of
  * any embedded pointers (platform_data, etc), they're copied as-is.
  */
+
+#ifdef CONFIG_SPI
 int __init
 spi_register_board_info(struct spi_board_info const *info, unsigned n)
 {
@@ -249,6 +251,7 @@ spi_register_board_info(struct spi_board
 	return 0;
 }
 EXPORT_SYMBOL_GPL(spi_register_board_info);
+#endif
 
 /* FIXME someone should add support for a __setup("spi", ...) that
  * creates board info from kernel command lines
@@ -505,6 +508,7 @@ int spi_write_then_read(struct spi_devic
 	int			status;
 	struct spi_message	message;
 	struct spi_transfer	x[2];
+	u8			*local_buf;
 
 	/* Use preallocated DMA-safe buffer.  We can't avoid copying here,
 	 * (as a pure convenience thing), but we can keep heap costs
@@ -513,14 +517,19 @@ int spi_write_then_read(struct spi_devic
 	if ((n_tx + n_rx) > SPI_BUFSIZ)
 		return -EINVAL;
 
-	down(&lock);
+	if (down_trylock(&lock))
+		/* Someone else is using the main buffer, kalloc a new one */
+		local_buf = kmalloc(SPI_BUFSIZ, GFP_ATOMIC);
+	else
+		local_buf = buf;
+
 	memset(x, 0, sizeof x);
 
-	memcpy(buf, txbuf, n_tx);
-	x[0].tx_buf = buf;
+	memcpy(local_buf, txbuf, n_tx);
+	x[0].tx_buf = local_buf;
 	x[0].len = n_tx;
 
-	x[1].rx_buf = buf + n_tx;
+	x[1].rx_buf = local_buf + n_tx;
 	x[1].len = n_rx;
 
 	/* do the i/o */
@@ -532,7 +541,11 @@ int spi_write_then_read(struct spi_devic
 		status = message.status;
 	}
 
-	up(&lock);
+	if (x[0].tx_buf == buf)
+		up(&lock);
+	else
+		kfree(local_buf);
+
 	return status;
 }
 EXPORT_SYMBOL_GPL(spi_write_then_read);

Re: [PATCH] [SPI] build as module and fix priority inversion problem

[David Brownell]

On Monday 05 December 2005 12:26 pm, Mark Underwood wrote:
> Sorry, made a bit of a mess with the last e-mail :(. Trying again :) 
>  
> This patch fix's the possible priority inversion that vitaly pointed out

That part looks OK to me, though I don't have time to test it just.
It's the change to "use kmalloc if we must".

I'd like to see you split that part out so it can be merged into the
next MM kernel.  (I don't know when that is, but I'd guess RSN.)


> and allows the driver to be built as a module. 

That part won't work right.  Remember, the spi_board_info setup
must be safe to call from arch_init() code; that support needs
to be statically linked.  The full patch for this would split
those parts into a separate file, and expose some internal hooks
to the module.  (Like maybe the struct boardinfo type, and the
board_list of instances, and the lock protecting that list.)

- Dave