- Prepend the device description with "AT91" to reflect its nature. [1]

- Move DMA tag and map creature to at91_spi_activate() where the other
  resource allocation also lives. [1]
- Flesh out at91_spi_deactivate(). [1]
- Work around the "Software Reset must be Written Twice" erratum.
- For now, run the bus at the slowest speed possible in order to work
  around data corruption on transit even seen with 9 MHz on ETHERNUT5
  (15 MHz maximum) and AT45DB321D (20 MHz maximum). This also serves as
  a poor man's work-around for the "NPCSx rises if no data data is to be
  transmitted" erratum of RM9200. Being able to use the appropriate bus
  speed would require:
  1) Adding a proper work-around for the RM9200 bug consisting of taking
     the chip select control away from the SPI peripheral and managing it
     directly as a GPIO line.
  2) Taking the maximum frequencies supported by the actual board and the
     slave devices into account and basing the whole thing on the master
     clock instead of hardcoding a divisor as previously done.
  3) Fixing the above mentioned data corruption.
- KASSERT that TX/RX command and data sizes match on transfers.
- Introduce a mutex ensuring that only one child device is running a SPI
  transfer at a time. [1]
- Add preliminary, #ifdef'ed out support for setting the chip select. [1]
- Use the RX instead of the TX commando size when setting up the RX side
  of a transfer.
- For controllers having SPI_SR_TXEMPTY, i.e. !RM9200, also wait for the
  completion of the TX part of transfers before stopping the whole thing
  again.
- Use DEVMETHOD_END. [1]
- Use NULL instead of 0 for pointers. [1, partially]

Additional testing by:  Ian Lepore

Submitted by:   Ian Lepore [1]
MFC after:      1 week

2 files changed, 193 insertions, 72 deletions

diff --git a/sys/arm/at91/at91_spi.c b/sys/arm/at91/at91_spi.c
index e5bf52f..ed513f8 100644
--- a/sys/arm/at91/at91_spi.c
+++ b/sys/arm/at91/at91_spi.c
@@ -1,5 +1,7 @@
 /*-
- * Copyright (c) 2006 M. Warner Losh.  All rights reserved.
+ * Copyright (c) 2006 M. Warner Losh.
+ * Copyright (c) 2011-2012 Ian Lepore.
+ * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -31,17 +33,21 @@ __FBSDID("$FreeBSD$");
 #include <sys/bus.h>
 #include <sys/conf.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/mbuf.h>
 #include <sys/malloc.h>
 #include <sys/module.h>
-#include <sys/mutex.h>
 #include <sys/rman.h>
+#include <sys/sx.h>
+
 #include <machine/bus.h>
 
 #include <arm/at91/at91_spireg.h>
 #include <arm/at91/at91_pdcreg.h>
+#include <arm/at91/at91var.h>
 
 #include <dev/spibus/spi.h>
+
 #include "spibus_if.h"
 
 struct at91_spi_softc
@@ -50,29 +56,39 @@ struct at91_spi_softc
 	void *intrhand;			/* Interrupt handle */
 	struct resource *irq_res;	/* IRQ resource */
 	struct resource	*mem_res;	/* Memory resource */
-	bus_dma_tag_t dmatag;		/* bus dma tag for mbufs */
+	bus_dma_tag_t dmatag;		/* bus dma tag for transfers */
 	bus_dmamap_t map[4];		/* Maps for the transaction */
-	int rxdone;
+	struct sx xfer_mtx;		/* Enforce one transfer at a time */
+	uint32_t xfer_mask;		/* Bits to wait on for completion */
+	uint32_t xfer_done;		/* interrupt<->mainthread signaling */
 };
 
+#define CS_TO_MR(cs)	((~(1 << (cs)) & 0x0f) << 16)
+
 static inline uint32_t
 RD4(struct at91_spi_softc *sc, bus_size_t off)
 {
-	return bus_read_4(sc->mem_res, off);
+
+	return (bus_read_4(sc->mem_res, off));
 }
 
 static inline void
 WR4(struct at91_spi_softc *sc, bus_size_t off, uint32_t val)
 {
+
 	bus_write_4(sc->mem_res, off, val);
 }
 
 /* bus entry points */
-static int at91_spi_probe(device_t dev);
 static int at91_spi_attach(device_t dev);
 static int at91_spi_detach(device_t dev);
+static int at91_spi_probe(device_t dev);
+static int at91_spi_transfer(device_t dev, device_t child,
+    struct spi_command *cmd);
 
 /* helper routines */
+static void at91_getaddr(void *arg, bus_dma_segment_t *segs, int nsegs,
+    int error);
 static int at91_spi_activate(device_t dev);
 static void at91_spi_deactivate(device_t dev);
 static void at91_spi_intr(void *arg);
@@ -80,43 +96,64 @@ static void at91_spi_intr(void *arg);
 static int
 at91_spi_probe(device_t dev)
 {
-	device_set_desc(dev, "SPI");
+
+	device_set_desc(dev, "AT91 SPI");
 	return (0);
 }
 
 static int
 at91_spi_attach(device_t dev)
 {
-	struct at91_spi_softc *sc = device_get_softc(dev);
-	int err, i;
+	struct at91_spi_softc *sc;
+	int err;
+	uint32_t csr;
+
+	sc = device_get_softc(dev);
 
 	sc->dev = dev;
+	sx_init(&sc->xfer_mtx, device_get_nameunit(dev));
+
+	/*
+	 * Allocate resources.
+	 */
 	err = at91_spi_activate(dev);
 	if (err)
 		goto out;
 
 	/*
-	 * Allocate DMA tags and maps
+	 * Set up the hardware.
 	 */
-	err = bus_dma_tag_create(bus_get_dma_tag(dev), 1, 0,
-	    BUS_SPACE_MAXADDR_32BIT, BUS_SPACE_MAXADDR, NULL, NULL, 2058, 1,
-	    2048, BUS_DMA_ALLOCNOW, NULL, NULL, &sc->dmatag);
-	if (err != 0)
-		goto out;
-	for (i = 0; i < 4; i++) {
-		err = bus_dmamap_create(sc->dmatag, 0,  &sc->map[i]);
-		if (err != 0)
-			goto out;
-	}
 
-	// reset the SPI
+	sc->xfer_mask = SPI_SR_RXBUFF | (at91_is_rm92() ? 0 : SPI_SR_TXEMPTY);
+	WR4(sc, SPI_CR, SPI_CR_SWRST);
+	/* "Software Reset must be Written Twice" erratum */
 	WR4(sc, SPI_CR, SPI_CR_SWRST);
 	WR4(sc, SPI_IDR, 0xffffffff);
 
 	WR4(sc, SPI_MR, (0xf << 24) | SPI_MR_MSTR | SPI_MR_MODFDIS |
-	    (0xE << 16));
+	    CS_TO_MR(0));
+
+	/*
+	 * For now, run the bus at the slowest speed possible as otherwise we
+	 * may encounter data corruption on transmit as seen with ETHERNUT5
+	 * and AT45DB321D even though both board and slave device can take
+	 * more.
+	 * This also serves as a work-around for the "NPCSx rises if no data
+	 * data is to be transmitted" erratum.  The ideal workaround for the
+	 * latter is to take the chip select control away from the peripheral
+	 * and manage it directly as a GPIO line.  The easy solution is to
+	 * slow down the bus so dramatically that it just never gets starved
+	 * as may be seen when the OCHI controller is running and consuming
+	 * memory and APB bandwidth.
+	 * Also, currently we lack a way for lettting both the board and the
+	 * slave devices take their maximum supported SPI clocks into account.
+	 */
+	csr = SPI_CSR_CPOL | (4 << 16) | (0xff << 8);
+	WR4(sc, SPI_CSR0, csr);
+	WR4(sc, SPI_CSR1, csr);
+	WR4(sc, SPI_CSR2, csr);
+	WR4(sc, SPI_CSR3, csr);
 
-	WR4(sc, SPI_CSR0, SPI_CSR_CPOL | (4 << 16) | (2 << 8));
 	WR4(sc, SPI_CR, SPI_CR_SPIEN);
 
 	WR4(sc, PDC_PTCR, PDC_PTCR_TXTDIS);
@@ -143,6 +180,7 @@ out:
 static int
 at91_spi_detach(device_t dev)
 {
+
 	return (EBUSY);	/* XXX */
 }
 
@@ -150,26 +188,41 @@ static int
 at91_spi_activate(device_t dev)
 {
 	struct at91_spi_softc *sc;
-	int rid, err = ENOMEM;
+	int err, i, rid;
 
 	sc = device_get_softc(dev);
+	err = ENOMEM;
+
 	rid = 0;
 	sc->mem_res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid,
 	    RF_ACTIVE);
 	if (sc->mem_res == NULL)
-		goto errout;
+		goto out;
+
 	rid = 0;
 	sc->irq_res = bus_alloc_resource_any(dev, SYS_RES_IRQ, &rid,
 	    RF_ACTIVE);
 	if (sc->irq_res == NULL)
-		goto errout;
+		goto out;
 	err = bus_setup_intr(dev, sc->irq_res, INTR_TYPE_MISC | INTR_MPSAFE,
 	    NULL, at91_spi_intr, sc, &sc->intrhand);
 	if (err != 0)
-		goto errout;
-	return (0);
-errout:
-	at91_spi_deactivate(dev);
+		goto out;
+
+	err = bus_dma_tag_create(bus_get_dma_tag(dev), 1, 0,
+	    BUS_SPACE_MAXADDR_32BIT, BUS_SPACE_MAXADDR, NULL, NULL, 2048, 1,
+	    2048, BUS_DMA_ALLOCNOW, NULL, NULL, &sc->dmatag);
+	if (err != 0)
+		goto out;
+
+	for (i = 0; i < 4; i++) {
+		err = bus_dmamap_create(sc->dmatag, 0,  &sc->map[i]);
+		if (err != 0)
+			goto out;
+	}
+out:
+	if (err != 0)
+		at91_spi_deactivate(dev);
 	return (err);
 }
 
@@ -177,26 +230,37 @@ static void
 at91_spi_deactivate(device_t dev)
 {
 	struct at91_spi_softc *sc;
+	int i;
 
 	sc = device_get_softc(dev);
+	bus_generic_detach(dev);
+
+	for (i = 0; i < 4; i++)
+		if (sc->map[i])
+			bus_dmamap_destroy(sc->dmatag, sc->map[i]);
+
+	if (sc->dmatag)
+		bus_dma_tag_destroy(sc->dmatag);
+
 	if (sc->intrhand)
 		bus_teardown_intr(dev, sc->irq_res, sc->intrhand);
-	sc->intrhand = 0;
-	bus_generic_detach(sc->dev);
-	if (sc->mem_res)
-		bus_release_resource(dev, SYS_RES_IOPORT,
-		    rman_get_rid(sc->mem_res), sc->mem_res);
-	sc->mem_res = 0;
+	sc->intrhand = NULL;
 	if (sc->irq_res)
 		bus_release_resource(dev, SYS_RES_IRQ,
 		    rman_get_rid(sc->irq_res), sc->irq_res);
-	sc->irq_res = 0;
-	return;
+	sc->irq_res = NULL;
+
+	if (sc->mem_res)
+		bus_release_resource(dev, SYS_RES_MEMORY,
+		    rman_get_rid(sc->mem_res), sc->mem_res);
+	sc->mem_res = NULL;
 }
 
 static void
-at91_getaddr(void *arg, bus_dma_segment_t *segs, int nsegs, int error)
+at91_getaddr(void *arg, bus_dma_segment_t *segs, int nsegs __unused,
+    int error)
 {
+
 	if (error != 0)
 		return;
 	*(bus_addr_t *)arg = segs[0].ds_addr;
@@ -206,80 +270,133 @@ static int
 at91_spi_transfer(device_t dev, device_t child, struct spi_command *cmd)
 {
 	struct at91_spi_softc *sc;
-	int i, j, rxdone, err, mode[4];
 	bus_addr_t addr;
+	int err, i, j, mode[4];
+	uint32_t mask;
+
+	KASSERT(cmd->tx_cmd_sz == cmd->rx_cmd_sz,
+	    ("%s: TX/RX command sizes should be equal", __func__));
+	KASSERT(cmd->tx_data_sz == cmd->rx_data_sz,
+	    ("%s: TX/RX data sizes should be equal", __func__));
 
 	sc = device_get_softc(dev);
-	WR4(sc, PDC_PTCR, PDC_PTCR_TXTDIS | PDC_PTCR_RXTDIS);
 	i = 0;
-	if (bus_dmamap_load(sc->dmatag, sc->map[i], cmd->tx_cmd,
-	    cmd->tx_cmd_sz, at91_getaddr, &addr, 0) != 0)
+
+	sx_xlock(&sc->xfer_mtx);
+
+	/*
+	 * Disable transfers while we set things up.
+	 */
+	WR4(sc, PDC_PTCR, PDC_PTCR_TXTDIS | PDC_PTCR_RXTDIS);
+
+#ifdef SPI_CHIPSEL_SUPPORT
+	if (cmd->cs < 0 || cmd->cs > 3) {
+		device_printf(dev,
+		    "Invalid chip select %d requested by %s\n", cmd->cs,
+		    device_get_nameunit(child));
+		err = EINVAL;
+		goto out;
+	}
+#ifdef SPI_CHIP_SELECT_HIGH_SUPPORT
+	if (at91_is_rm92() && cmd->cs == 0 &&
+	    (cmd->flags & SPI_CHIP_SELECT_HIGH) != 0) {
+		device_printf(dev,
+		    "Invalid chip select high requested by %s\n",
+		    device_get_nameunit(child));
+		err = EINVAL;
+		goto out;
+	}
+#endif
+	WR4(sc, SPI_MR, (RD4(sc, SPI_MR) & ~0x000f0000) | CS_TO_MR(cmd->cs));
+#endif
+
+	/*
+	 * Set up the TX side of the transfer.
+	 */
+	if ((err = bus_dmamap_load(sc->dmatag, sc->map[i], cmd->tx_cmd,
+	    cmd->tx_cmd_sz, at91_getaddr, &addr, 0)) != 0)
 		goto out;
 	WR4(sc, PDC_TPR, addr);
 	WR4(sc, PDC_TCR, cmd->tx_cmd_sz);
 	bus_dmamap_sync(sc->dmatag, sc->map[i], BUS_DMASYNC_PREWRITE);
 	mode[i++] = BUS_DMASYNC_POSTWRITE;
 	if (cmd->tx_data_sz > 0) {
-		if (bus_dmamap_load(sc->dmatag, sc->map[i], cmd->tx_data,
-			cmd->tx_data_sz, at91_getaddr, &addr, 0) != 0)
+		if ((err = bus_dmamap_load(sc->dmatag, sc->map[i],
+		    cmd->tx_data, cmd->tx_data_sz, at91_getaddr, &addr, 0)) !=
+		    0)
 			goto out;
 		WR4(sc, PDC_TNPR, addr);
 		WR4(sc, PDC_TNCR, cmd->tx_data_sz);
 		bus_dmamap_sync(sc->dmatag, sc->map[i], BUS_DMASYNC_PREWRITE);
 		mode[i++] = BUS_DMASYNC_POSTWRITE;
 	}
-	if (bus_dmamap_load(sc->dmatag, sc->map[i], cmd->rx_cmd,
-	    cmd->tx_cmd_sz, at91_getaddr, &addr, 0) != 0)
+
+	/*
+	 * Set up the RX side of the transfer.
+	 */
+	if ((err = bus_dmamap_load(sc->dmatag, sc->map[i], cmd->rx_cmd,
+	    cmd->rx_cmd_sz, at91_getaddr, &addr, 0)) != 0)
 		goto out;
 	WR4(sc, PDC_RPR, addr);
-	WR4(sc, PDC_RCR, cmd->tx_cmd_sz);
+	WR4(sc, PDC_RCR, cmd->rx_cmd_sz);
 	bus_dmamap_sync(sc->dmatag, sc->map[i], BUS_DMASYNC_PREREAD);
 	mode[i++] = BUS_DMASYNC_POSTREAD;
 	if (cmd->rx_data_sz > 0) {
-		if (bus_dmamap_load(sc->dmatag, sc->map[i], cmd->rx_data,
-			cmd->tx_data_sz, at91_getaddr, &addr, 0) != 0)
+		if ((err = bus_dmamap_load(sc->dmatag, sc->map[i],
+		    cmd->rx_data, cmd->rx_data_sz, at91_getaddr, &addr, 0)) !=
+		    0)
 			goto out;
 		WR4(sc, PDC_RNPR, addr);
 		WR4(sc, PDC_RNCR, cmd->rx_data_sz);
 		bus_dmamap_sync(sc->dmatag, sc->map[i], BUS_DMASYNC_PREREAD);
 		mode[i++] = BUS_DMASYNC_POSTREAD;
 	}
-	WR4(sc, SPI_IER, SPI_SR_ENDRX);
+
+	/*
+	 * Start the transfer, wait for it to complete.
+	 */
+	sc->xfer_done = 0;
+	mask = sc->xfer_mask;
+	WR4(sc, SPI_IER, mask);
 	WR4(sc, PDC_PTCR, PDC_PTCR_TXTEN | PDC_PTCR_RXTEN);
+	do
+		err = tsleep(&sc->xfer_done, PCATCH | PZERO, "at91_spi", hz);
+	while (sc->xfer_done != mask && err != EINTR);
 
-	rxdone = sc->rxdone;
-	do {
-		err = tsleep(&sc->rxdone, PCATCH | PZERO, "spi", hz);
-	} while (rxdone == sc->rxdone && err != EINTR);
+	/*
+	 * Stop the transfer and clean things up.
+	 */
 	WR4(sc, PDC_PTCR, PDC_PTCR_TXTDIS | PDC_PTCR_RXTDIS);
-	if (err == 0) {
-		for (j = 0; j < i; j++) 
+	if (err == 0)
+		for (j = 0; j < i; j++)
 			bus_dmamap_sync(sc->dmatag, sc->map[j], mode[j]);
-	}
-	for (j = 0; j < i; j++)
-		bus_dmamap_unload(sc->dmatag, sc->map[j]);
-	return (err);
 out:
 	for (j = 0; j < i; j++)
 		bus_dmamap_unload(sc->dmatag, sc->map[j]);
-	return (EIO);
+
+	sx_xunlock(&sc->xfer_mtx);
+
+	return (err);
 }
 
 static void
 at91_spi_intr(void *arg)
 {
-	struct at91_spi_softc *sc = (struct at91_spi_softc*)arg;
-	uint32_t sr;
+	struct at91_spi_softc *sc;
+	uint32_t mask, sr;
 
+	sc = (struct at91_spi_softc*)arg;
+
+	mask = sc->xfer_mask;
 	sr = RD4(sc, SPI_SR) & RD4(sc, SPI_IMR);
-	if (sr & SPI_SR_ENDRX) {
-		sc->rxdone++;
-		WR4(sc, SPI_IDR, SPI_SR_ENDRX);
-		wakeup(&sc->rxdone);
+	if ((sr & mask) != 0) {
+		sc->xfer_done |= sr & mask;
+		WR4(sc, SPI_IDR, mask);
+		wakeup(&sc->xfer_done);
 	}
-	if (sr & ~SPI_SR_ENDRX) {
+	if ((sr & ~mask) != 0) {
 		device_printf(sc->dev, "Unexpected ISR %#x\n", sr);
-		WR4(sc, SPI_IDR, sr & ~SPI_SR_ENDRX);
+		WR4(sc, SPI_IDR, sr & ~mask);
 	}
 }
 
@@ -293,7 +410,8 @@ static device_method_t at91_spi_methods[] = {
 
 	/* spibus interface */
 	DEVMETHOD(spibus_transfer,	at91_spi_transfer),
-	{ 0, 0 }
+
+	DEVMETHOD_END
 };
 
 static driver_t at91_spi_driver = {
@@ -302,4 +420,5 @@ static driver_t at91_spi_driver = {
 	sizeof(struct at91_spi_softc),
 };
 
-DRIVER_MODULE(at91_spi, atmelarm, at91_spi_driver, at91_spi_devclass, 0, 0);
+DRIVER_MODULE(at91_spi, atmelarm, at91_spi_driver, at91_spi_devclass, NULL,
+    NULL);
diff --git a/sys/arm/at91/at91_spireg.h b/sys/arm/at91/at91_spireg.h
index c57edab..251564f 100644
--- a/sys/arm/at91/at91_spireg.h
+++ b/sys/arm/at91/at91_spireg.h
@@ -54,6 +54,8 @@
 #define	  SPI_SR_ENDTX		0x00020
 #define	  SPI_SR_RXBUFF		0x00040
 #define	  SPI_SR_TXBUFE		0x00080
+#define	  SPI_SR_NSSR		0x00100
+#define	  SPI_SR_TXEMPTY	0x00200
 #define	  SPI_SR_SPIENS		0x10000
 #define	SPI_IER		0x14		/* IER: Interrupt Enable Regsiter */
 #define	SPI_IDR		0x18		/* IDR: Interrupt Disable Regsiter */