Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 2 Sep 2012 09:37:24 -0300
From:      Luiz Otavio O Souza <loos.br@gmail.com>
To:        freebsd-arch@freebsd.org
Subject:   spibus access serialization
Message-ID:  <B54C4C9A-76F4-45CC-94C3-628DDF901051@gmail.com>

next in thread | raw e-mail | index | archive | help

[-- Attachment #1 --]
Hi,

I've some embedded systems with spi devices that share the same spibus and because of that i'm working to get some kind of access serialization on spibus and also protection to devices which need a series of spi transfers to achieve some goal.

The SPI usage (with all the patches applied) goes like this:

/* Wait until the spibus is free. When free, acquire the bus and select the device */
SPIBUS_ACQUIRE_BUS(spibus, device);

/* Program the CPLD to read data from NAND */
SPIBUS_TRANSFER(spibus, device, &cmd);

/* Read 'n' bytes from CPLD */
SPIBUS_TRANSFER(spibus, device, &cmd);

/* Release the spibus and deselect the device */
SPIBUS_RELEASE_BUS(spibus, device);

While today everything is done inside SPIBUS_TRANSFER().

The patch spibus-01.diff adds the bus methods and the default methods.

The spibus-02-devices.diff adds the needed glue to all spi devices (dev/flash/at45d.c, dev/flash/mx25l.c, arm/lpc/ssd1289.c, mips/atheros/pcf2123_rtc.c). As the default methods are just nops, there are no functional changes.

On spibus-03-controllers.diff we finally add the serialization methods to spi controllers (mips/atheros/ar71xx_spi.c and arm/lpc/lpc_spi.c) and change the device CS to happen on bus acquire and release and not on start and end of each transfer.

The spi controller on arm/at91/at91_spi.c wasn't changed because looks like it will be need to move the device CS control from the controller and use it as a GPIO pin. I need to read more of at91 code before i can suggest some change there. Until there it will work just as now (no serialization and selecting/deselecting the device within each transfer).

Comments ?

Thanks,
Luiz




[-- Attachment #2 --]
Index: dev/spibus/spibus.c
===================================================================
--- dev/spibus/spibus.c	(revision 239916)
+++ dev/spibus/spibus.c	(working copy)
@@ -155,6 +155,20 @@
 	resource_int_value(dname, dunit, "cs", &devi->cs);
 }
 
+static void
+spibus_acquire_bus_impl(device_t dev, device_t child)
+{
+
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), child);
+}
+
+static void
+spibus_release_bus_impl(device_t dev, device_t child)
+{
+
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), child);
+}
+
 static int
 spibus_transfer_impl(device_t dev, device_t child, struct spi_command *cmd)
 {
@@ -180,6 +194,8 @@
 	DEVMETHOD(bus_hinted_child,	spibus_hinted_child),
 
 	/* spibus interface */
+	DEVMETHOD(spibus_acquire_bus,	spibus_acquire_bus_impl),
+	DEVMETHOD(spibus_release_bus,	spibus_release_bus_impl),
 	DEVMETHOD(spibus_transfer,	spibus_transfer_impl),
 
 	DEVMETHOD_END
Index: dev/spibus/spibus_if.m
===================================================================
--- dev/spibus/spibus_if.m	(revision 239916)
+++ dev/spibus/spibus_if.m	(working copy)
@@ -32,6 +32,37 @@
 INTERFACE spibus;
 
 #
+# Default implementation
+#
+CODE {
+	static void
+	null_acquire_bus(device_t dev, device_t child)
+	{
+	}
+
+	static void
+	null_release_bus(device_t dev, device_t child)
+	{
+	}
+};
+
+#
+# Acquire bus and select the device
+#
+METHOD void acquire_bus {
+	device_t dev;
+	device_t child;
+} DEFAULT null_acquire_bus;
+
+#
+# Release bus and deselect the device
+#
+METHOD void release_bus {
+	device_t dev;
+	device_t child;
+} DEFAULT null_release_bus;
+
+#
 # Do a spi command
 #
 METHOD int transfer {

[-- Attachment #3 --]
Index: dev/flash/at45d.c
===================================================================
--- dev/flash/at45d.c	(revision 239916)
+++ dev/flash/at45d.c	(working copy)
@@ -132,7 +132,9 @@
 	cmd.tx_cmd = txBuf;
 	cmd.rx_cmd = rxBuf;
 	cmd.rx_cmd_sz = cmd.tx_cmd_sz = 2;
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
 	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), dev);
 	*status = rxBuf[1];
 	return (err);
 }
@@ -152,7 +154,9 @@
 	cmd.tx_cmd = &txBuf;
 	cmd.rx_cmd = &rxBuf;
 	cmd.tx_cmd_sz = cmd.rx_cmd_sz = 5;
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
 	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), dev);
 	if (err)
 		return (err);
 	memcpy(resp, rxBuf + 1, 4);
@@ -365,8 +369,10 @@
 					txBuf[2] = ((addr >> 8) & 0xff);
 					txBuf[3] = 0;
 					cmd.tx_data_sz = cmd.rx_data_sz = 0;
+					SPIBUS_ACQUIRE_BUS(pdev, dev);
 					err = SPIBUS_TRANSFER(pdev, dev,
 					    &cmd);
+					SPIBUS_RELEASE_BUS(pdev, dev);
 					if (err == 0)
 						err = at45d_wait_ready(dev,
 						    &status);
@@ -383,7 +389,9 @@
 			txBuf[2] = ((addr >> 8) & 0xff);
 			txBuf[3] = (addr & 0xff);
 			cmd.tx_data_sz = cmd.rx_data_sz = len;
+			SPIBUS_ACQUIRE_BUS(pdev, dev);
 			err = SPIBUS_TRANSFER(pdev, dev, &cmd);
+			SPIBUS_RELEASE_BUS(pdev, dev);
 			if (err == 0 && bp->bio_cmd != BIO_READ)
 				err = at45d_wait_ready(dev, &status);
 			if (err != 0) {
@@ -397,7 +405,9 @@
 				txBuf[2] = ((addr >> 8) & 0xff);
 				txBuf[3] = 0;
 				cmd.tx_data_sz = cmd.rx_data_sz = 0;
+				SPIBUS_ACQUIRE_BUS(pdev, dev);
 				err = SPIBUS_TRANSFER(pdev, dev, &cmd);
+				SPIBUS_RELEASE_BUS(pdev, dev);
 				if (err == 0)
 					err = at45d_wait_ready(dev, &status);
 				if (err != 0 || (status & 0x40) != 0) {
Index: dev/flash/mx25l.c
===================================================================
--- dev/flash/mx25l.c	(revision 239916)
+++ dev/flash/mx25l.c	(working copy)
@@ -123,7 +123,9 @@
 	cmd.rx_cmd = rxBuf;
 	cmd.rx_cmd_sz = 2;
 	cmd.tx_cmd_sz = 2;
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
 	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), dev);
 	return (rxBuf[1]);
 }
 
@@ -157,7 +159,9 @@
 	 */
 	cmd.tx_cmd_sz = 4;
 	cmd.rx_cmd_sz = 4;
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
 	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), dev);
 	if (err)
 		return (NULL);
 
@@ -192,7 +196,9 @@
 	cmd.rx_cmd = rxBuf;
 	cmd.rx_cmd_sz = 1;
 	cmd.tx_cmd_sz = 1;
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
 	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), dev);
 }
 
 static void
@@ -217,7 +223,9 @@
 	txBuf[1] = ((sector >> 16) & 0xff);
 	txBuf[2] = ((sector >> 8) & 0xff);
 	txBuf[3] = (sector & 0xff);
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
 	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), dev);
 }
 
 static int
@@ -290,7 +298,9 @@
 		mx25l_wait_for_device_ready(dev);
 		mx25l_set_writable(dev, 1);
 
+		SPIBUS_ACQUIRE_BUS(pdev, dev);
 		err = SPIBUS_TRANSFER(pdev, dev, &cmd);
+		SPIBUS_RELEASE_BUS(pdev, dev);
 		if (err)
 			break;
 
@@ -339,7 +349,9 @@
 	cmd.rx_data = data;
 	cmd.rx_data_sz = count;
 
+	SPIBUS_ACQUIRE_BUS(pdev, dev);
 	err = SPIBUS_TRANSFER(pdev, dev, &cmd);
+	SPIBUS_RELEASE_BUS(pdev, dev);
 
 	return (err);
 }
@@ -480,7 +492,7 @@
 	DEVMETHOD(device_attach,	mx25l_attach),
 	DEVMETHOD(device_detach,	mx25l_detach),
 
-	{ 0, 0 }
+	DEVMETHOD_END
 };
 
 static driver_t mx25l_driver = {
Index: arm/lpc/ssd1289.c
===================================================================
--- arm/lpc/ssd1289.c	(revision 239916)
+++ arm/lpc/ssd1289.c	(working copy)
@@ -179,6 +179,7 @@
 {
 	uint8_t buffer[2];
 
+	SPIBUS_ACQUIRE_BUS(device_get_parent(sc->ss_dev), sc->ss_dev);
 	ssd1289_set_dc(sc, 0);
 	buffer[0] = 0x00;
 	buffer[1] = addr & 0xff;
@@ -188,6 +189,7 @@
 	buffer[0] = (value >> 8) & 0xff;
 	buffer[1] = value & 0xff;
 	ssd1289_spi_send(sc, buffer, 2);
+	SPIBUS_RELEASE_BUS(device_get_parent(sc->ss_dev), sc->ss_dev);
 }
 
 static device_method_t ssd1289_methods[] = {
@@ -195,7 +197,7 @@
 	DEVMETHOD(device_probe,		ssd1289_probe),
 	DEVMETHOD(device_attach,	ssd1289_attach),
 
-	{ 0, 0 }
+	DEVMETHOD_END
 };
 
 static devclass_t ssd1289_devclass;
Index: mips/atheros/pcf2123_rtc.c
===================================================================
--- mips/atheros/pcf2123_rtc.c	(revision 239916)
+++ mips/atheros/pcf2123_rtc.c	(working copy)
@@ -91,7 +91,9 @@
 	cmd.tx_cmd = txBuf;
 	cmd.rx_cmd_sz = sizeof(rxBuf);
 	cmd.tx_cmd_sz = sizeof(txBuf);
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
 	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), dev);
 	DELAY(PCF2123_DELAY);
 
 	return (0);
@@ -120,7 +122,9 @@
 	cmd.tx_cmd = txTimedate;
 	cmd.rx_cmd_sz = sizeof(rxTimedate);
 	cmd.tx_cmd_sz = sizeof(txTimedate);
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
 	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), dev);
 	DELAY(PCF2123_DELAY);
 
 	ct.nsec = 0;
@@ -178,7 +182,9 @@
 	txTimedate[6] = TOBCD(ct.mon);
 	txTimedate[7] = TOBCD(ct.year - YEAR_BASE);
 
+	SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
 	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	SPIBUS_RELEASE_BUS(device_get_parent(dev), dev);
 	DELAY(PCF2123_DELAY);
 
 	return (err);
@@ -191,7 +197,7 @@
 	DEVMETHOD(clock_gettime,	pcf2123_rtc_gettime),
 	DEVMETHOD(clock_settime,	pcf2123_rtc_settime),
 
-	{ 0, 0 },
+	DEVMETHOD_END
 };
 
 static driver_t pcf2123_rtc_driver = {

[-- Attachment #4 --]
Index: mips/atheros/ar71xx_spi.c
===================================================================
--- mips/atheros/ar71xx_spi.c	(revision 239916)
+++ mips/atheros/ar71xx_spi.c	(working copy)
@@ -35,7 +35,9 @@
 #include <sys/interrupt.h>
 #include <sys/malloc.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/module.h>
+#include <sys/mutex.h>
 #include <sys/rman.h>
 
 #include <vm/vm.h>
@@ -76,10 +78,21 @@
 
 struct ar71xx_spi_softc {
 	device_t		sc_dev;
+	device_t		sc_owner;
+	struct mtx		sc_mtx;
 	struct resource		*sc_mem_res;
 	uint32_t		sc_reg_ctrl;
 };
 
+#define	AR71XX_SPI_LOCK(_sc) mtx_lock(&(_sc)->sc_mtx)
+#define	AR71XX_SPI_UNLOCK(_sc) mtx_unlock(&(_sc)->sc_mtx)
+#define	AR71XX_SPI_LOCK_INIT(_sc) \
+	mtx_init(&_sc->sc_mtx, device_get_nameunit(_sc->sc_dev), \
+	    "ar71xx_spi", MTX_DEF)
+#define	AR71XX_SPI_LOCK_DESTROY(_sc) mtx_destroy(&_sc->sc_mtx)
+#define	AR71XX_SPI_ASSERT_LOCKED(_sc) mtx_assert(&_sc->sc_mtx, MA_OWNED)
+#define	AR71XX_SPI_ASSERT_UNLOCKED(_sc) mtx_assert(&_sc->sc_mtx, MA_NOTOWNED)
+
 static int
 ar71xx_spi_probe(device_t dev)
 {
@@ -102,6 +115,7 @@
 		return (ENXIO);
 	}
 
+	AR71XX_SPI_LOCK_INIT(sc);
 
 	SPI_WRITE(sc, AR71XX_SPI_FS, 1);
 	sc->sc_reg_ctrl  = SPI_READ(sc, AR71XX_SPI_CTRL);
@@ -133,6 +147,49 @@
 	SPI_WRITE(sc, AR71XX_SPI_IO_CTRL, SPI_IO_CTRL_CSMASK);
 }
 
+static void
+ar71xx_spi_acquire_bus(device_t dev, device_t child)
+{
+	struct spibus_ivar *devi = SPIBUS_IVAR(child);
+	struct ar71xx_spi_softc *sc;
+
+	sc = device_get_softc(dev);
+	AR71XX_SPI_ASSERT_UNLOCKED(sc);
+
+	/* Wait until bus is free and then set the new bus owner. */
+	AR71XX_SPI_LOCK(sc);
+	while (sc->sc_owner != NULL) {
+		mtx_sleep(sc, &sc->sc_mtx, 0, "ar71xx_spi", 0);
+	}
+	sc->sc_owner = child;
+	AR71XX_SPI_UNLOCK(sc);
+
+	/* Select the SPI device. */
+	ar71xx_spi_chip_activate(sc, devi->cs);
+}
+
+static void
+ar71xx_spi_release_bus(device_t dev, device_t child)
+{
+	struct spibus_ivar *devi = SPIBUS_IVAR(child);
+	struct ar71xx_spi_softc *sc;
+
+	sc = device_get_softc(dev);
+	AR71XX_SPI_ASSERT_UNLOCKED(sc);
+
+	/* Free the SPI BUS. */
+	AR71XX_SPI_LOCK(sc);
+	if (sc->sc_owner == NULL)
+		panic("ar71xx_spi: releasing unowned bus.");
+	if (sc->sc_owner != child)
+		panic("ar71xx_spi: you don't own the bus. game over.");
+	sc->sc_owner = NULL;
+	AR71XX_SPI_UNLOCK(sc);
+
+	/* Deselect the SPI device. */
+	ar71xx_spi_chip_deactivate(sc, devi->cs);
+}
+
 static uint8_t
 ar71xx_spi_txrx(struct ar71xx_spi_softc *sc, int cs, uint8_t data)
 {
@@ -173,7 +230,7 @@
 
 	sc = device_get_softc(dev);
 
-	ar71xx_spi_chip_activate(sc, devi->cs);
+	KASSERT(sc->sc_owner != NULL, ("SPI transfer on unowned bus"));
 
 	KASSERT(cmd->tx_cmd_sz == cmd->rx_cmd_sz, 
 	    ("TX/RX command sizes should be equal"));
@@ -196,8 +253,6 @@
 	for (i = 0; i < cmd->tx_data_sz; i++)
 		buf_in[i] = ar71xx_spi_txrx(sc, devi->cs, buf_out[i]);
 
-	ar71xx_spi_chip_deactivate(sc, devi->cs);
-
 	return (0);
 }
 
@@ -209,6 +264,8 @@
 	SPI_WRITE(sc, AR71XX_SPI_CTRL, sc->sc_reg_ctrl);
 	SPI_WRITE(sc, AR71XX_SPI_FS, 0);
 
+	AR71XX_SPI_LOCK_DESTROY(sc);
+
 	if (sc->sc_mem_res)
 		bus_release_resource(dev, SYS_RES_MEMORY, 0, sc->sc_mem_res);
 
@@ -221,9 +278,11 @@
 	DEVMETHOD(device_attach,	ar71xx_spi_attach),
 	DEVMETHOD(device_detach,	ar71xx_spi_detach),
 
+	DEVMETHOD(spibus_acquire_bus,	ar71xx_spi_acquire_bus),
+	DEVMETHOD(spibus_release_bus,	ar71xx_spi_release_bus),
 	DEVMETHOD(spibus_transfer,	ar71xx_spi_transfer),
 
-	{0, 0}
+	DEVMETHOD_END
 };
 
 static driver_t ar71xx_spi_driver = {
Index: arm/lpc/lpc_spi.c
===================================================================
--- arm/lpc/lpc_spi.c	(revision 239916)
+++ arm/lpc/lpc_spi.c	(working copy)
@@ -67,6 +67,8 @@
 struct lpc_spi_softc
 {
 	device_t		ls_dev;
+	device_t		ls_owner;
+	struct mtx		ls_mtx;
 	struct resource *	ls_mem_res;
 	struct resource *	ls_irq_res;
 	bus_space_tag_t		ls_bst;
@@ -83,6 +85,15 @@
 #define	lpc_spi_write_4(_sc, _reg, _val)	\
     bus_space_write_4(_sc->ls_bst, _sc->ls_bsh, _reg, _val)
 
+#define	LPC_SPI_LOCK(_sc) mtx_lock(&(_sc)->ls_mtx)
+#define	LPC_SPI_UNLOCK(_sc) mtx_unlock(&(_sc)->ls_mtx)
+#define	LPC_SPI_LOCK_INIT(_sc) \
+	mtx_init(&_sc->sc_mtx, device_get_nameunit(_sc->ls_dev), \
+	    "lpc_spi", MTX_DEF)
+#define	LPC_SPI_LOCK_DESTROY(_sc) mtx_destroy(&_sc->ls_mtx)
+#define	LPC_SPI_ASSERT_LOCKED(_sc) mtx_assert(&_sc->ls_mtx, MA_OWNED)
+#define	LPC_SPI_ASSERT_UNLOCKED(_sc) mtx_assert(&_sc->ls_mtx, MA_NOTOWNED)
+
 static int
 lpc_spi_probe(device_t dev)
 {
@@ -126,6 +137,8 @@
 	lpc_spi_write_4(sc, LPC_SSP_CR1, LPC_SSP_CR1_SSE);
 	lpc_spi_write_4(sc, LPC_SSP_CPSR, 128);
 
+	LPC_SPI_LOCK_INIT(sc);
+
 	device_add_child(dev, "spibus", 0);
 	return (bus_generic_attach(dev));
 }
@@ -136,6 +149,49 @@
 	return (EBUSY);
 }
 
+static void
+lpc_spi_acquire_bus(device_t dev, device_t child)
+{
+	struct spibus_ivar *devi = SPIBUS_IVAR(child);
+	struct lpc_spi_softc *sc;
+
+	sc = device_get_softc(dev);
+	LPC_SPI_ASSERT_UNLOCKED(sc);
+
+	/* Wait until bus is free and then set the new bus owner. */
+	LPC_SPI_LOCK(sc);
+	while (sc->ls_owner != NULL) {
+		mtx_sleep(sc, &sc->ls_mtx, 0, "lpc_spi", 0);
+	}
+	sc->ls_owner = child;
+	LPC_SPI_UNLOCK(sc);
+
+	/* Set CS active */
+	lpc_gpio_set_state(child, devi->cs, 0);
+}
+
+static void
+lpc_spi_release_bus(device_t dev, device_t child)
+{
+	struct spibus_ivar *devi = SPIBUS_IVAR(child);
+	struct ar71xx_spi_softc *sc;
+
+	sc = device_get_softc(dev);
+	LPC_SPI_ASSERT_UNLOCKED(sc);
+
+	/* Free the SPI BUS. */
+	LPC_SPI_LOCK(sc);
+	if (sc->ls_owner == NULL)
+		panic("lpc_spi: releasing unowned bus.");
+	if (sc->ls_owner != child)
+		panic("lpc_spi: you don't own the bus. game over.");
+	sc->ls_owner = NULL;
+	LPC_SPI_UNLOCK(sc);
+
+	/* Set CS inactive */
+	lpc_gpio_set_state(child, devi->cs, 1);
+}
+
 static int
 lpc_spi_transfer(device_t dev, device_t child, struct spi_command *cmd)
 {
@@ -144,8 +200,7 @@
 	uint8_t *in_buf, *out_buf;
 	int i;
 
-	/* Set CS active */
-	lpc_gpio_set_state(child, devi->cs, 0);
+	KASSERT(sc->ls_owner != NULL, ("SPI transfer on unowned bus"));
 
 	/* Wait for FIFO to be ready */
 	while ((lpc_spi_read_4(sc, LPC_SSP_SR) & LPC_SSP_SR_TNF) == 0);
@@ -166,9 +221,6 @@
 		in_buf[i] = lpc_spi_read_4(sc, LPC_SSP_DR);
 	}
 
-	/* Set CS inactive */
-	lpc_gpio_set_state(child, devi->cs, 1);
-
 	return (0);
 }
 
@@ -179,9 +231,11 @@
 	DEVMETHOD(device_detach,	lpc_spi_detach),
 
 	/* SPI interface */
+	DEVMETHOD(spibus_acquire_bus,	lpc_spi_acquire_bus),
+	DEVMETHOD(spibus_release_bus,	lpc_spi_release_bus),
 	DEVMETHOD(spibus_transfer,	lpc_spi_transfer),
 
-	{ 0, 0 }
+	DEVMETHOD_END
 };
 
 static devclass_t lpc_spi_devclass;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B54C4C9A-76F4-45CC-94C3-628DDF901051>