spi: driver for Cirrus EP93xx SPI controller

This patch adds an SPI master driver for the Cirrus EP93xx SPI controller found in EP93xx chips (EP9301, EP9302, EP9307, EP9312 and EP9315).

Signed-off-by Mika Westerberg <[email protected]>

drivers/spi/ep93xx_spi.c new file +23 -0

+/**
+ * ep93xx_spi_select_device() - select given SPI device
+ * @espi: ep93xx SPI controller struct
+ * @spi: SPI device to select
+ *
+ * Function asserts chipselect line as specified in @spi->chip_select in board
+ * specific manner.
+ *
+ * Note that this function is called from a thread context and can sleep.
+ */
+static inline void ep93xx_spi_select_device(const struct ep93xx_spi *espi,
+					    struct spi_device *spi)
+{
+	if (espi->cs_control)
+		espi->cs_control(spi, !!(spi->mode & SPI_CS_HIGH));
+}
+
+/**
+ * ep93xx_spi_deselect_device() - deselects given SPI device
+ * @espi: ep93xx SPI controller struct
+ * @spi: SPI device to deselect
+ *
+ * Function deasserts chipselect line as specified in @spi->chip_select in board

Quoted 4 lines · Mika Westerberg

This patch adds an SPI master driver for the Cirrus EP93xx SPI controller found in EP93xx chips (EP9301, EP9302, EP9307, EP9312 and EP9315).

Signed-off-by Mika Westerberg <[email protected]>

Mika,

I'm still looking this over but I have one quick comment.

Quoted 23 lines

drivers/spi/ep93xx_spi.c new file +23 -0

+/**
+ * ep93xx_spi_select_device() - select given SPI device
+ * @espi: ep93xx SPI controller struct
+ * @spi: SPI device to select
+ *
+ * Function asserts chipselect line as specified in @spi->chip_select in board
+ * specific manner.
+ *
+ * Note that this function is called from a thread context and can sleep.
+ */
+static inline void ep93xx_spi_select_device(const struct ep93xx_spi *espi,
+					    struct spi_device *spi)
+{
+	if (espi->cs_control)
+		espi->cs_control(spi, !!(spi->mode & SPI_CS_HIGH));
+}
+
+/**
+ * ep93xx_spi_deselect_device() - deselects given SPI device
+ * @espi: ep93xx SPI controller struct
+ * @spi: SPI device to deselect
+ *
+ * Function deasserts chipselect line as specified in @spi->chip_select in board

drivers/spi/ep93xx_spi.c line 317

Quoted 10 lines

drivers/spi/ep93xx_spi.c new file +10 -0

+ * specific manner.
+ *
+ * Note that this function is called from a thread context and can sleep.
+ */
+static inline void ep93xx_spi_deselect_device(const struct ep93xx_spi *espi,
+					      struct spi_device *spi)
+{
+	if (espi->cs_control)
+		espi->cs_control(spi, !(spi->mode & SPI_CS_HIGH));
+}

These two functions can be combined.

/**
* ep93xx_spi_chip_select() - select/deselect a given SPI device
* @espi: ep93xx SPI controller struct
* @spi: SPI device to select
* @assert: flag to assert the chip select line
*
* Note that this function is called from a thread context and can sleep. */
static void ep93xx_spi_chip_select(const struct ep93xx_spi *espi,
struct spi_device *spi, int assert)

{
	int value = (spi->mode & SPI_CS_HIGH) ? !!assert : !assert;

	if (espi->cs_control)
		espi->cs_control(spi, value);
}

Then just do:

ep93xx_spi_select_device(espi, msg->spi, 1); /* assert the spi chip select */

ep93xx_spi_select_device(espi, msg->spi, 0); /* deassert the spi chip select */

Quoted 20 lines

These two functions can be combined.

/**
* ep93xx_spi_chip_select() - select/deselect a given SPI device
* @espi: ep93xx SPI controller struct
* @spi: SPI device to select
* @assert: flag to assert the chip select line
*
* Note that this function is called from a thread context and can sleep. */
static void ep93xx_spi_chip_select(const struct ep93xx_spi *espi,
struct spi_device *spi, int assert)

{
	int value = (spi->mode & SPI_CS_HIGH) ? !!assert : !assert;

	if (espi->cs_control)
		espi->cs_control(spi, value);
}

Quoted 5 lines

Then just do:

ep93xx_spi_select_device(espi, msg->spi, 1); /* assert the spi chip select */

ep93xx_spi_select_device(espi, msg->spi, 0); /* deassert the spi chip select */

I think it is more readable to do:

	ep93xx_spi_select_device(espi, msg->spi);

and

	ep93xx_spi_deselect_device(espi, msg->spi);

It can be seen from the function names what we are doing.

Quoted 8 lines · Mika Westerberg

I think it is more readable to do:

	ep93xx_spi_select_device(espi, msg->spi);

and

	ep93xx_spi_deselect_device(espi, msg->spi);

Quoted 1 line

It can be seen from the function names what we are doing.

So rename the combined function to ep93xx_spi_cs_control and pass the "control" parameter as a bool; true = selected, false = deselected.

Having the two almost identical functions is a bit redundant. It also gives you two places to have to maintain basically the same operation. I think this is what Grant was referring to with his previous comment.

First, every spi transaction, including a single byte transfer, is going to generate at least two interrupts. One when the interrupts are first enabled because the TX FIFO is empty. And a second when that byte has been transferred and the TX FIFO is again empty.

The first interrupt can be prevented by priming the TX FIFO before enabling the interrupts. All you need to do is call ep93xx_spi_read_write right before ep93xx_spi_enable_interrupts.

Second, at high clock rates the RX FIFO will actually start to fill as you are putting data into the TX FIFO. If you add an inner reader loop to the writer loop in ep93xx_spi_read_write you can take advantage of this and reduce the number of interrupts generated for large transfers.

I have added some debug messages to the driver trying to figure out how to chain the transfers in a message together in order to keep the SFRM signal asserted for the entire message. I still haven't worked out a good solution but I did notice something else.

First, every spi transaction, including a single byte transfer, is going to generate at least two interrupts. One when the interrupts are first enabled because the TX FIFO is empty. And a second when that byte has been transferred and the TX FIFO is again empty.

The first interrupt can be prevented by priming the TX FIFO before enabling the interrupts. All you need to do is call ep93xx_spi_read_write right before ep93xx_spi_enable_interrupts.

Second, at high clock rates the RX FIFO will actually start to fill as you are putting data into the TX FIFO. If you add an inner reader loop to the writer loop in ep93xx_spi_read_write you can take advantage of this and reduce the number of interrupts generated for large transfers.

For instance the mmc_spi driver regularly does 3 transfer messages where the last transfer is a 512 byte read. With your current v4 driver this message averages 69 interrupts to complete. By adding the inner reader it reduces the number of interrupts to an average of 40.

First, every spi transaction, including a single byte transfer, is going to generate at least two interrupts. One when the interrupts are first enabled because the TX FIFO is empty. And a second when that byte has been transferred and the TX FIFO is again empty.

The first interrupt can be prevented by priming the TX FIFO before enabling the interrupts. All you need to do is call ep93xx_spi_read_write right before ep93xx_spi_enable_interrupts.

Quoted 5 lines · H Hartley Sweeten

First, every spi transaction, including a single byte transfer, is going to generate at least two interrupts. One when the interrupts are first enabled because the TX FIFO is empty. And a second when that byte has been transferred and the TX FIFO is again empty.

Quoted 3 lines

The first interrupt can be prevented by priming the TX FIFO before enabling the interrupts. All you need to do is call ep93xx_spi_read_write right before ep93xx_spi_enable_interrupts.

Nice. That increases the data throughput from 367 to 372 kB/sec and reduces the CPU usage from 61 to 60.2% for large transfers

Quoted 11 lines

Second, at high clock rates the RX FIFO will actually start to fill as you are putting data into the TX FIFO. If you add an inner reader loop to the writer loop in ep93xx_spi_read_write you can take advantage of this and reduce the number of interrupts generated for large transfers.

For instance the mmc_spi driver regularly does 3 transfer messages where the last transfer is a 512 byte read. With your current v4 driver this message averages 69 interrupts to complete. By adding the inner reader it reduces the number of interrupts to an average of 40.

I assume you mean something like

       /* read as long as RX FIFO has frames in it */
       while (ep93xx_spi_read_u8(espi, SSPSR) & SSPSR_RNE) {
               ep93xx_do_read(espi, t);
               espi->fifo_level--;
       }

       /* write as long as TX FIFO has room */
       while (espi->fifo_level < SPI_FIFO_SIZE && espi->tx < t->len) {
               ep93xx_do_write(espi, t);
               espi->fifo_level++;

Well, you're gonna love this :)

I've run some timing tests on this, and it turns out to give the same data transfer speed but take more system CPU time: 64% instead of 60%, i.e it is slower.

You are seeing a lower interrupt count, not because more useful work is being done in each interrupt, but only because the extra code in the inner loop makes the interrupt routine slower. Bear with me...

I'm using v4 with mika's continuity hack compiled for speed not space and averaging the vmstat "sy" output column over 50 seconds while block-reading 20 meg with dd from /dev/mmcblk0 to /dev/null using a fairly old Sandisk 1MB SD card and clocking at 3.7 MHz.
All tests report 367 or 368 kB/sec; the variability of the system time average in successive runs is about 1%

Figures are (commas separate values from successive runs) v4 with Mika's hack:

     61.9,60.2%  234724,234727 irq/MB
with "while" reader in write loop:
     63.4,64.5%  197772,97801 irq/MB
with "if" reader in while loop:
     61.9,62.5%  197845,197847  irq/MB
with an "if" reader after the write loop:
     66.0,67.1%  278207,277166 irq/MB
with a "while" reader after the write loop:
     67.0%  275000 irq/MB

Further, on a suspicion about the silliness of the per-transfer bits_per_word being checked right down the bottom of the lowest lever byte-read/writing routine instead of once per transfer, I split ep93xx_spi_read and _write into 8 and 16-bit variants, and checked t->bits_per_word once outside the read and write loops.

This gives:

     60.0% 59.3%  285906 irq/MB

Note: it takes *more* interrupts to transfer a megabyte when the interrupt routine is more efficient!

The reason is that almost all of those interrupts happen while waiting for the last 4, 3, 2 or 1 bytes of a transfer to arrive in the RX FIFO. On this dreadful chip there is no interrupt mechanism to say "the TX buffer is empty and the received data has finished arriving". There is only "the TX buffer is half empty or less" and "the RX buffer is half full or more", so repeated interrupts now perform the function that the old busy-wait loop performed in v1.

This is another good reason to unite multiple transfers into one continuous stream, to have less of this effective busy-waiting with interrupts spinning madly.

It now occurs to me that another step in CPU efficiency could be had by abusing the receive-fifo-is-half-full interrupt to signal the completion of a transfer. This would only work for transfers of four or more words and would need some careful jiggery-pokery at end of transfer to turn the tx-fifo-is-half-empty interrupt enable off and ensure that exactly four words would end up in the RX FIFO.

It's a horrible thought, and I suspect that the DMA engine is the real answer.

Quoted 6 lines

Further, on a suspicion about the silliness of the per-transfer bits_per_word being checked right down the bottom of the lowest lever byte-read/writing routine instead of once per transfer, I split ep93xx_spi_read and _write into 8 and 16-bit variants, and checked t->bits_per_word once outside the read and write loops.

Quoted 2 lines

This gives:

     60.0% 59.3%  285906 irq/MB

It looks like splitting the 8/16-bit read/write routines does help
with cpu usage compared to the tests above. Was the data transfer (kB/sec) any better?

This patch adds an SPI master driver for the Cirrus EP93xx SPI controller found in EP93xx chips (EP9301, EP9302, EP9307, EP9312 and EP9315).

Signed-off-by Mika Westerberg <[email protected]>

Quoted 2 lines · Mika Westerberg

This patch adds an SPI master driver for the Cirrus EP93xx SPI controller found in EP93xx chips (EP9301, EP9302, EP9307, EP9312 and EP9315).

I discovered one gotcha with this driver. Bear with me through this...

1. ep93xx_spi_process_message is called by the workqueue worker function to handle a spi_message queued in by the ep93xx_spi_transfer function. Before starting the actual transfer the chip select to the device is asserted.
2. ep93xx_spi_process_transfer processes each transfer in the spi_message, one at a time, and does a wait_for_completion on each transfer.
3. When the entire spi_message has been transferred the chip select to the device is deasserted.

The problem is if a hardware design uses the SFRM1 pin as part of the chip select logic. The EP93xx User's Guide states that "the SFRMOUT signal behaves as a slave Select" in Motorola SPI mode. This signal behaves differently depending on the SPI_MODE_* used.

Modes 0 and 2:
SFRMOUT is forced HIGH during idle periods.
Start of transmission, SFRMOUT is driven LOW.
Single word transmission, SFRMOUT is returned to its idle HIGH state one
SCLKOUT period after the last bit has been captured.
Back-to-back transmissions, SFRMOUT signal is pulsed HIGH between each
data word transfer. On completion the SFRMOUT pin returns to its idle HIGH state one SCLKOUT period after the last bit has been captured.

Modes 1 and 3:
SFRMOUT is forced HIGH during idle periods.
Start of transmission, SFRMOUT is driven LOW.
Single word transmission, SFRMOUT is returned to its idle HIGH state one
SCLKOUT period after the last bit has been captured.
Back-to-back transmissions, SFRMOUT signal is held LOW between each
data word transfer. On completion the SFRMOUT pin returns to its idle HIGH state one SCLKOUT period after the last bit has been captured.

So, since each transfer does a wait_for_completion, all the data is transmitted which causes the SFRMOUT pin to go HIGH between each transfer in the message.

For devices like the sst25lf040a SPI Flash on the EDB93xx boards this causes a problem. To read data from that device you need to send a two part message. The first part is a write transfer with the one byte command and three byte address. This is then followed by a read transfer to get the data. But since the SFRMOUT pin goes high this ends the command.

The driver Ryan and I worked on actually does work with the sst25lf040a on the EDB93xx boards. But now that I know what the issue is, it was actually a race condition. Sometimes it worked and sometimes it didn't...

I would think the Sim.One board (Martin?) would have the same issue with the mmc card since that design uses the SFRMOUT pin directly for the chip select.

Is there anyway to keep the transfers going in a muiltipart message?

Quoted 46 lines · H Hartley Sweeten

Quoted 2 lines · Mika Westerberg

This patch adds an SPI master driver for the Cirrus EP93xx SPI controller found in EP93xx chips (EP9301, EP9302, EP9307, EP9312 and EP9315).

I discovered one gotcha with this driver. Bear with me through this...

1. ep93xx_spi_process_message is called by the workqueue worker function to handle a spi_message queued in by the ep93xx_spi_transfer function. Before starting the actual transfer the chip select to the device is asserted.
2. ep93xx_spi_process_transfer processes each transfer in the spi_message, one at a time, and does a wait_for_completion on each transfer.
3. When the entire spi_message has been transferred the chip select to the device is deasserted.

The problem is if a hardware design uses the SFRM1 pin as part of the chip select logic. The EP93xx User's Guide states that "the SFRMOUT signal behaves as a slave Select" in Motorola SPI mode. This signal behaves differently depending on the SPI_MODE_* used.

Modes 0 and 2:
SFRMOUT is forced HIGH during idle periods.
Start of transmission, SFRMOUT is driven LOW.
Single word transmission, SFRMOUT is returned to its idle HIGH state one
SCLKOUT period after the last bit has been captured.
Back-to-back transmissions, SFRMOUT signal is pulsed HIGH between each
data word transfer. On completion the SFRMOUT pin returns to its idle HIGH state one SCLKOUT period after the last bit has been captured.

Modes 1 and 3:
SFRMOUT is forced HIGH during idle periods.
Start of transmission, SFRMOUT is driven LOW.
Single word transmission, SFRMOUT is returned to its idle HIGH state one
SCLKOUT period after the last bit has been captured.
Back-to-back transmissions, SFRMOUT signal is held LOW between each
data word transfer. On completion the SFRMOUT pin returns to its idle HIGH state one SCLKOUT period after the last bit has been captured.

So, since each transfer does a wait_for_completion, all the data is transmitted which causes the SFRMOUT pin to go HIGH between each transfer in the message.

For devices like the sst25lf040a SPI Flash on the EDB93xx boards this causes a problem. To read data from that device you need to send a two part message. The first part is a write transfer with the one byte command and three byte address. This is then followed by a read transfer to get the data. But since the SFRMOUT pin goes high this ends the command.

Quoted 8 lines

The driver Ryan and I worked on actually does work with the sst25lf040a on the EDB93xx boards. But now that I know what the issue is, it was actually a race condition. Sometimes it worked and sometimes it didn't...

I would think the Sim.One board (Martin?) would have the same issue with the mmc card since that design uses the SFRMOUT pin directly for the chip select.

Is there anyway to keep the transfers going in a muiltipart message?

I just changed the SPI mode selection, which is in the board definition file, from Mode 0 to Mode 3 (the two modes that sample data on the rising clock edge, as required by MMC/SD). It dramatically increases linear block read throughput:
MMC: from 307kB/sec to 353kB/sec
SD: from 317 to 365
SD MLC: from 327 to 378
with 82000 interrupts per second and 60% cpu usage

presumably because the poor card isn't being deselected once every byte.

Quoted 1 line · Martin Guy

On 4/20/10, H Hartley Sweeten <[email protected]> wrote:

Quoted 11 lines

Quoted 1 line

Is there anyway to keep the transfers going in a muiltipart message?

I just changed the SPI mode selection, which is in the board definition file, from Mode 0 to Mode 3 (the two modes that sample data on the rising clock edge, as required by MMC/SD). It dramatically increases linear block read throughput:
MMC: from 307kB/sec to 353kB/sec
SD: from 317 to 365
SD MLC: from 327 to 378
with 82000 interrupts per second and 60% cpu usage

Quoted 1 line

presumably because the poor card isn't being deselected once every byte.

Yes, Mode 3 should perform better since the protocol is not enforcing the chip select deassertion between every byte.

You should still be getting a chip select deassertion between every transfer in a multipart message since the Sim.One uses the SFRM1 line for the chip select. I haven't actually looked through the mmc_spi driver to see if this happens.

Is there any possibility you could look at the actual bus with a logic analyzer?

My concern with the comments above is that currently this driver will not work on the EDB93xx boards with the SPI Flash device. I did hack one of my boards to remove the SFRM1 signal from the chip select and it does "fix" the problem. But, that's kind of an ugly solution for most users.

Quoted 7 lines · Martin Guy

Quoted 6 lines · H Hartley Sweeten

So, since each transfer does a wait_for_completion, all the data is transmitted which causes the SFRMOUT pin to go HIGH between each transfer in the message.
...
I would think the Sim.One board (Martin?) would have the same issue with the mmc card since that design uses the SFRMOUT pin directly for the chip select.

Quoted 1 line

Quoted 1 line

Is there anyway to keep the transfers going in a muiltipart message?

You should still be getting a chip select deassertion between every transfer in a multipart message since the Sim.One uses the SFRM1 line for the chip select. I haven't actually looked through the mmc_spi driver to see if this happens.

Is there any possibility you could look at the actual bus with a logic analyzer?

Quoted 8 lines · H Hartley Sweeten

Quoted 7 lines · Martin Guy

Quoted 6 lines · H Hartley Sweeten

So, since each transfer does a wait_for_completion, all the data is transmitted which causes the SFRMOUT pin to go HIGH between each transfer in the message.
...
I would think the Sim.One board (Martin?) would have the same issue with the mmc card since that design uses the SFRMOUT pin directly for the chip select.

Quoted 5 lines

Quoted 1 line

Quoted 1 line

Is there anyway to keep the transfers going in a muiltipart message?

You should still be getting a chip select deassertion between every transfer in a multipart message since the Sim.One uses the SFRM1 line for the chip select. I haven't actually looked through the mmc_spi driver to see if this happens.

OK, can I see if understand what you're saying, that ep93xx_spi_process_message() calls ep93xx_spi_process_transfer() multiple times using:

       list_for_each_entry(t, &msg->transfers, transfer_list) {
               ep93xx_spi_process_transfer(espi, msg, t);

...
and process_transfer() waits for each message block to be completely transmitted and received, so a transaction that consists of several messages will (or may) cause the device to be deselected between parts of the same message, which may cause a failure.

I have noticed on card insertion, the last line of:

mmc0: problem reading switch capabilities, performance might suffer.
mmc0: host does not support reading read-only switch. assuming write-enable.
mmc0: new SD card on SPI
mmcblk0: mmc0:0000 SD 952 MiB
mmcblk0: p1 p2 p4
mmcblk0: retrying using single block read

and the SDHC cards I have don't work at all, spewing tons of:
mmcblk0: error -38 sending status comand
mmcblk0: error -38 sending read/write command, response 0x4, card status 0xff04
end_request: I/O error, dev mmcblk0, sector 7744509

Quoted 1 line

Is there any possibility you could look at the actual bus with a logic analyzer?

Not easily, but it seems a likely cause.
To prevent card deselection mid-message I think we would need to handle multi-transfer messages by making the start of transfers 2..N continuous with the end of transfers 1..N-1 instead of draining the reply from each one before starting the next.

Am I on the right track?

Quoted 8 lines · Martin Guy

I have noticed on card insertion, the last line of:

mmc0: problem reading switch capabilities, performance might suffer.
mmc0: host does not support reading read-only switch. assuming write-enable.
mmc0: new SD card on SPI
mmcblk0: mmc0:0000 SD 952 MiB
mmcblk0: p1 p2 p4
mmcblk0: retrying using single block read

That message might be related to the SFRM1 line being deasserted.

You could try cutting the trace going to pin 1 (CS pin) of the mmc socket and jumpering it to one of the EGPIO pins on JP15 (the LCD header) or JP16 (the keypad header). You will of course need to modify your platform init to use the new chip select line.

Quoted 4 lines

and the SDHC cards I have don't work at all, spewing tons of:
mmcblk0: error -38 sending status comand
mmcblk0: error -38 sending read/write command, response 0x4, card status 0xff04
end_request: I/O error, dev mmcblk0, sector 7744509

I haven't seen those error messages before. Of course I don't have any SDHC cards... ;-)

Quoted 7 lines · Martin Guy

Not easily, but it seems a likely cause.
To prevent card deselection mid-message I think we would need to handle multi-transfer messages by making the start of transfers 2..N continuous with the end of transfers 1..N-1 instead of draining the reply from each one before starting the next.

Am I on the right track?

Yes. I'm just not sure how to implement this.

Quoted 10 lines · H Hartley Sweeten

Quoted 7 lines · Martin Guy

Not easily, but it seems a likely cause.
To prevent card deselection mid-message I think we would need to handle multi-transfer messages by making the start of transfers 2..N continuous with the end of transfers 1..N-1 instead of draining the reply from each one before starting the next.

Am I on the right track?

Yes. I'm just not sure how to implement this.

I made a hack that allows transfer handler to continue to the next transfer directly if there is no special per transfer settings in the message. Could you try that out and report whether SFRMOUT works better with it?

Note that this is just to make sure that we are on the right track.

Patch is against v4 of the driver.

Thanks,
MW

drivers/spi/ep93xx_spi.c +52 -6

index 310032d..11dcdd1 100644
@@ -106,6 +106,8 @@ struct ep93xx_spi {
 	unsigned long			min_rate;
 	unsigned long			max_rate;
 	bool				running;
+	bool				can_continue;
+	struct spi_transfer 		*last_transfer;
 	struct workqueue_struct		*wq;
 	struct work_struct		msg_work;
 	struct completion		wait;

@@ -380,6 +382,7 @@ static int ep93xx_spi_transfer(struct spi_device *spi, struct spi_message *msg)
 	struct ep93xx_spi *espi = spi_master_get_devdata(spi->master);
 	struct spi_transfer *t;
 	unsigned long flags;
+	bool can_continue = true;
 
 	if (!msg || !msg->complete)
 		return -EINVAL;

@@ -387,11 +390,21 @@ static int ep93xx_spi_transfer(struct spi_device *spi, struct spi_message *msg)
 	/* first validate each transfer */
 	list_for_each_entry(t, &msg->transfers, transfer_list) {
 		if (t->bits_per_word) {
+			can_continue = false;
 			if (t->bits_per_word < 4 || t->bits_per_word > 16)
 				return -EINVAL;
 		}
-		if (t->speed_hz && t->speed_hz < espi->min_rate)
+		if (t->speed_hz) {
+			can_continue = false;
+			if (t->speed_hz < espi->min_rate)
 				return -EINVAL;
+		}
+		if (t->cs_change &&
+			!list_is_last(&t->transfer_list, &msg->transfers)) {
+			can_continue = false;
+		}
+		if (t->delay_usecs)
+			can_continue = false;
 	}
 
 	/*

@@ -400,7 +413,7 @@ static int ep93xx_spi_transfer(struct spi_device *spi, struct spi_message *msg)
 	 * error in transfer and @msg->state is used to hold pointer to the
 	 * current transfer (or %NULL if no active current transfer).
 	 */
-	msg->state = NULL;
+	msg->state = (void *)can_continue;
 	msg->status = 0;
 	msg->actual_length = 0;
 

@@ -606,10 +619,33 @@ static void ep93xx_spi_process_message(struct ep93xx_spi *espi,
 	ep93xx_spi_chip_setup(espi, spi_get_ctldata(msg->spi));
 	ep93xx_spi_select_device(espi, msg->spi);
 
-	list_for_each_entry(t, &msg->transfers, transfer_list) {
-		ep93xx_spi_process_transfer(espi, msg, t);
-		if (msg->status)
-			break;
+	if (msg->state) {
+		espi->can_continue = true;
+		espi->last_transfer = list_entry(msg->transfers.prev, struct spi_transfer,
+						 transfer_list);
+	} else {
+		espi->can_continue = false;
+		espi->last_transfer = NULL;
+	}
+
+	/*
+	 * In case there is no transfer specific settings in this message we
+	 * can transfer the whole message with interrupts. Otherwise we need
+	 * to perform transfer specific stuff in thread context.
+	 */
+	if (espi->can_continue) {
+		msg->state = list_first_entry(&msg->transfers, struct spi_transfer,
+					      transfer_list);
+		espi->rx = 0;
+		espi->tx = 0;
+		ep93xx_spi_enable_interrupts(espi);
+		wait_for_completion(&espi->wait);
+	} else {
+		list_for_each_entry(t, &msg->transfers, transfer_list) {
+			ep93xx_spi_process_transfer(espi, msg, t);
+			if (msg->status)
+				break;
+		}
 	}
 
 	/*

@@ -792,6 +828,24 @@ static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id)
 			 * interrupt then.
 			 */
 			return IRQ_HANDLED;
+		} else {
+			/*
+			 * If we can continue directly to the next transfer, do
+			 * that now.
+			 */
+			if (espi->can_continue) {
+				struct spi_message *msg = espi->current_msg;
+				struct spi_transfer *t = msg->state;
+
+				if (t != espi->last_transfer) {

Same results are your v4 driver. But, I think your on the right track.

I think the problem is in the ep93xx_spi_read_write routine. That function returns 0 as long as there is still data left in the current transfer. The only time it doesn't return 0, which will cause the can_continue, is when:

	if (espi->rx == t->len) {
		msg->actual_length += t->len;
		return t->len;
	}

At this point the tx and rx fifos will both be empty, which causes the SSP peripheral to raise the SFRM signal.

A picture is worth a thousand words... Attached is a logic analyzer capture of the Read-ID command and response from the SPI Flash. EGPIO7 is my chip select to the flash. The first 4 SPI MOSI (Tx) blocks are the 0x90, 0x00, 0x00, 0x00 command to the flash. You will notice that the SFRM line is asserted for those bytes. A bit later are the 2 SPI MISO (Rx) responses from the flash with the ManID/DevID, again with the SFRM line asserted.

Quoted 1 line · H Hartley Sweeten

Same results are your v4 driver. But, I think your on the right track.

Thanks for testing.

Quoted 11 lines

I think the problem is in the ep93xx_spi_read_write routine. That function returns 0 as long as there is still data left in the current transfer. The only time it doesn't return 0, which will cause the can_continue, is when:

	if (espi->rx == t->len) {
		msg->actual_length += t->len;
		return t->len;
	}

At this point the tx and rx fifos will both be empty, which causes the SSP peripheral to raise the SFRM signal.

True.

Quoted 6 lines

A picture is worth a thousand words... Attached is a logic analyzer capture of the Read-ID command and response from the SPI Flash. EGPIO7 is my chip select to the flash. The first 4 SPI MOSI (Tx) blocks are the 0x90, 0x00, 0x00, 0x00 command to the flash. You will notice that the SFRM line is asserted for those bytes. A bit later are the 2 SPI MISO (Rx) responses from the flash with the ManID/DevID, again with the SFRM line asserted.

Yeah, it clearly shows that SFRMOUT is deasserted between transfers :(

I will try to work out some sort of hack which will keep the FIFOs from emptying.

MW

Same results are your v4 driver. But, I think your on the right track.

I think the problem is in the ep93xx_spi_read_write routine. That function returns 0 as long as there is still data left in the current transfer. The only time it doesn't return 0, which will cause the can_continue, is when:

	if (espi->rx == t->len) {
		msg->actual_length += t->len;
		return t->len;
	}

At this point the tx and rx fifos will both be empty, which causes the SSP peripheral to raise the SFRM signal.

Quoted 12 lines · H Hartley Sweeten

Same results are your v4 driver. But, I think your on the right track.

I think the problem is in the ep93xx_spi_read_write routine. That function returns 0 as long as there is still data left in the current transfer. The only time it doesn't return 0, which will cause the can_continue, is when:

	if (espi->rx == t->len) {
		msg->actual_length += t->len;
		return t->len;
	}

Quoted 2 lines

At this point the tx and rx fifos will both be empty, which causes the SSP peripheral to raise the SFRM signal.

Hi again,

I crafted up next hack. It includes also your priming the FIFO finding (thanks for that).

Following patch is against v4 version of the driver.

It performs another read/write after given transfer is finished. I'm hoping that it could make it before SFRMOUT is deasserted.

Could you test this in your setup?

drivers/spi/ep93xx_spi.c +39 -4

+/**
+ * ep93xx_spi_read_write() - perform next RX/TX transfer
+ * @espi: ep93xx SPI controller struct
+ *
+ * This function transfers next bytes (or half-words) to/from RX/TX FIFOs. If
+ * called several times, the whole transfer will be completed. Returns %0 when
+ * current transfer was not yet completed otherwise length of the transfer
+ * (>%0). When this function is finished, RX FIFO should be empty and TX FIFO
+ * should be full.
+ */
+static int ep93xx_spi_read_write(struct ep93xx_spi *espi)
+{
+	struct spi_message *msg = espi->current_msg;
+	struct spi_transfer *t = msg->state;
+
+	/* read as long as RX FIFO has frames in it */
+	while ((ep93xx_spi_read_u8(espi, SSPSR) & SSPSR_RNE)) {
+		ep93xx_do_read(espi, t);
+		espi->fifo_level--;
+	}
+
+	/* write as long as TX FIFO has room */
+	while (espi->fifo_level < SPI_FIFO_SIZE && espi->tx < t->len) {
+		ep93xx_do_write(espi, t);
+		espi->fifo_level++;
+	}
+
+	if (espi->rx == t->len) {
+		msg->actual_length += t->len;
+		return t->len;
+	}
+
+	return 0;
+}
+
+
 /*
  * ep93xx_spi_process_message() - process one SPI message
  * @espi: ep93xx SPI controller struct

@@ -569,6 +669,8 @@ static void ep93xx_spi_process_message(struct ep93xx_spi *espi,
 	struct spi_transfer *t;
 	int err;
 
+	espi->continuous = (bool)msg->state;
+
 	/*
 	 * Enable the SPI controller and its clock.
 	 */

@@ -606,12 +708,43 @@ static void ep93xx_spi_process_message(struct ep93xx_spi *espi,
 	ep93xx_spi_chip_setup(espi, spi_get_ctldata(msg->spi));
 	ep93xx_spi_select_device(espi, msg->spi);
 
-	list_for_each_entry(t, &msg->transfers, transfer_list) {
-		ep93xx_spi_process_transfer(espi, msg, t);
-		if (msg->status)
-			break;
+	if (espi->continuous) {

drivers/spi/ep93xx_spi.c +7 -0

+			/*
+			 * Transfer was completed. Pick next one and continue
+			 * if necessary.
+			 */
+			t = msg->state;
+			if (list_is_last(&t->transfer_list, &msg->transfers))
+				goto done;

drivers/spi/ep93xx_spi.c +10 -1

-/**
  * ep93xx_spi_interrupt() - SPI interrupt handler
  * @irq: IRQ number (not used)
  * @dev_id: pointer to EP93xx controller struct

@@ -792,6 +841,27 @@ static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id)
 			 * interrupt then.
 			 */
 			return IRQ_HANDLED;
+		} else {
+			struct spi_message *msg = espi->current_msg;
+			struct spi_transfer *t = msg->state;
+
+			if (!espi->continuous)
+				goto done;
+
+			if (!list_is_last(&t->transfer_list, &msg->transfers)) {
+				/*
+				 * Pick next transfer and refill the FIFO.

Quoted 1 line · Mika Westerberg

Could you test this in your setup?

Same results.

I hacked your driver to allow me to toggle a gpio when the interrupt routine starts and exits.

I captured the 4 byte write/2 byte read caused by the spi_write_then_read in the sst25l driver when it does the Read-ID command. The chip select goes low followed by the first 4 byte transfer of the message due to the fifo's getting primed. This transfer actually doesn't finish since the rx data has not been received. So the interrupts just get enabled as normal.

The data starts transferring when the first byte is placed in the tx fifo. It takes about 4.45us to send/receive the 4 bytes. Approximately 15.9us after the last byte has been received the interrupt routine starts. It then takes about 5.7us for the second 2 byte transfer to start. So there is a total of about 21.6us between the two transfers where SFRM is deasserted.

Quoted 21 lines

/*
* ep93xx_spi_process_message() - process one SPI message
* @espi: ep93xx SPI controller struct

drivers/spi/ep93xx_spi.c +3 -4

@@ -569,6 +669,8 @@ static void ep93xx_spi_process_message(struct ep93xx_spi *espi,
 	struct spi_transfer *t;
 	int err;
 
+	espi->continuous = (bool)msg->state;
+
 	/*
 	 * Enable the SPI controller and its clock.
 	 */

@@ -606,12 +708,43 @@ static void ep93xx_spi_process_message(struct ep93xx_spi *espi,
 	ep93xx_spi_chip_setup(espi, spi_get_ctldata(msg->spi));
 	ep93xx_spi_select_device(espi, msg->spi);
 
-	list_for_each_entry(t, &msg->transfers, transfer_list) {
-		ep93xx_spi_process_transfer(espi, msg, t);
-		if (msg->status)
-			break;
+	if (espi->continuous) {

drivers/spi/ep93xx_spi.c static void ep93xx_spi_process_message(struct ep93xx_spi *espi, · line 721

Quoted 10 lines

drivers/spi/ep93xx_spi.c +10 -0

+		/*
+		 * We can transfer all the transfers continuously.
+		 */
+		espi->rx = 0;
+		espi->tx = 0;
+		msg->state = list_first_entry(&msg->transfers, struct spi_transfer,
+					transfer_list);
+
+		/* prime the first transfer */
+		if (ep93xx_spi_read_write(espi)) {

On the first read/write for a transfer this condition can never succeed.

The ep93xx_spi_read_write function first empties the rx fifo then fills the tx fifo. Putting any data in the tx fifo means that you are going to have to come back to empty the rx fifo.

So, the block below never executes.

Quoted 13 lines

drivers/spi/ep93xx_spi.c +13 -0

+			/*
+			 * Transfer was completed. Pick next one and continue
+			 * if necessary.
+			 */
+			t = msg->state;
+			if (list_is_last(&t->transfer_list, &msg->transfers))
+				goto done;
+
+			espi->rx = 0;
+			espi->tx = 0;
+			msg->state = list_entry(t->transfer_list.next,
+						struct spi_transfer,
+						transfer_list);

drivers/spi/ep93xx_spi.c static void ep93xx_spi_process_message(struct ep93xx_spi *espi, · line 738

Quoted 4 lines

drivers/spi/ep93xx_spi.c +4 -0

+		}
+
+		ep93xx_spi_enable_interrupts(espi);
+		wait_for_completion(&espi->wait);

Even if the first read/write did succeed you are not starting the second transfer. This is just passing the operation off to the interrupt routine using the second transfer in the message.

Quoted 27 lines

drivers/spi/ep93xx_spi.c +19 -1

-/**
  * ep93xx_spi_interrupt() - SPI interrupt handler
  * @irq: IRQ number (not used)
  * @dev_id: pointer to EP93xx controller struct

@@ -792,6 +841,27 @@ static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id)
 			 * interrupt then.
 			 */
 			return IRQ_HANDLED;
+		} else {
+			struct spi_message *msg = espi->current_msg;
+			struct spi_transfer *t = msg->state;
+
+			if (!espi->continuous)
+				goto done;
+
+			if (!list_is_last(&t->transfer_list, &msg->transfers)) {
+				/*
+				 * Pick next transfer and refill the FIFO.
+				 */
+				msg->state = list_entry(t->transfer_list.next,
+							struct spi_transfer,
+							transfer_list);
+				espi->rx = 0;
+				espi->tx = 0;
+
+				ep93xx_spi_read_write(espi);
+				return IRQ_HANDLED;

Review static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id) · line 863

Quoted 1 line

(quoted diff) +1 -0

+			}

Here the N transfer has been completed and we are trying to continue with the N+1 transfer. This should work but I need to figure out a way to capture it with my logic analyzer. The only problem is if the N transfer is complete that means the tx and rx fifos are empty so the SFRM signal is going to be deasserted. The N+1 transfer might start really soon after but there will still be a glitch in the SFRM signal.

Quoted 4 lines · H Hartley Sweeten

Quoted 1 line · Mika Westerberg

Could you test this in your setup?

Same results.

OK. thanks for testing.

Quoted 9 lines

I hacked your driver to allow me to toggle a gpio when the interrupt routine starts and exits.

I captured the 4 byte write/2 byte read caused by the spi_write_then_read in the sst25l driver when it does the Read-ID command. The chip select goes low followed by the first 4 byte transfer of the message due to the fifo's getting primed. This transfer actually doesn't finish since the rx data has not been received. So the interrupts just get enabled as normal.

Quoted 5 lines

The data starts transferring when the first byte is placed in the tx fifo. It takes about 4.45us to send/receive the 4 bytes. Approximately 15.9us after the last byte has been received the interrupt routine starts. It then takes about 5.7us for the second 2 byte transfer to start. So there is a total of about 21.6us between the two transfers where SFRM is deasserted.

If I understand this correctly, for this particular message it might have worked if the priming is not done? What I mean is that if we start the transfer in interrupt handler and after the transfer is finished, immediately continue with the next one (like in the patch) could it make SFRM to stay asserted (at least shorten the gap)?

Quoted 34 lines

Quoted 21 lines

/*
* ep93xx_spi_process_message() - process one SPI message
* @espi: ep93xx SPI controller struct

drivers/spi/ep93xx_spi.c +3 -4

@@ -569,6 +669,8 @@ static void ep93xx_spi_process_message(struct ep93xx_spi *espi,
 	struct spi_transfer *t;
 	int err;
 
+	espi->continuous = (bool)msg->state;
+
 	/*
 	 * Enable the SPI controller and its clock.
 	 */

@@ -606,12 +708,43 @@ static void ep93xx_spi_process_message(struct ep93xx_spi *espi,
 	ep93xx_spi_chip_setup(espi, spi_get_ctldata(msg->spi));
 	ep93xx_spi_select_device(espi, msg->spi);
 
-	list_for_each_entry(t, &msg->transfers, transfer_list) {
-		ep93xx_spi_process_transfer(espi, msg, t);
-		if (msg->status)
-			break;
+	if (espi->continuous) {

drivers/spi/ep93xx_spi.c static void ep93xx_spi_process_message(struct ep93xx_spi *espi, · line 721

Quoted 10 lines

drivers/spi/ep93xx_spi.c +10 -0

+		/*
+		 * We can transfer all the transfers continuously.
+		 */
+		espi->rx = 0;
+		espi->tx = 0;
+		msg->state = list_first_entry(&msg->transfers, struct spi_transfer,
+					transfer_list);
+
+		/* prime the first transfer */
+		if (ep93xx_spi_read_write(espi)) {

On the first read/write for a transfer this condition can never succeed.

Quoted 5 lines

The ep93xx_spi_read_write function first empties the rx fifo then fills the tx fifo. Putting any data in the tx fifo means that you are going to have to come back to empty the rx fifo.

So, the block below never executes.

I also figured that out after I sent the hack to you...

Quoted 13 lines

Quoted 13 lines

drivers/spi/ep93xx_spi.c +13 -0

+			/*
+			 * Transfer was completed. Pick next one and continue
+			 * if necessary.
+			 */
+			t = msg->state;
+			if (list_is_last(&t->transfer_list, &msg->transfers))
+				goto done;
+
+			espi->rx = 0;
+			espi->tx = 0;
+			msg->state = list_entry(t->transfer_list.next,
+						struct spi_transfer,
+						transfer_list);

Quoted 8 lines

drivers/spi/ep93xx_spi.c static void ep93xx_spi_process_message(struct ep93xx_spi *espi, · line 738

Quoted 4 lines

drivers/spi/ep93xx_spi.c +4 -0

+		}
+
+		ep93xx_spi_enable_interrupts(espi);
+		wait_for_completion(&espi->wait);

Even if the first read/write did succeed you are not starting the second transfer. This is just passing the operation off to the interrupt routine using the second transfer in the message.

Right.

Quoted 27 lines

Quoted 27 lines

drivers/spi/ep93xx_spi.c +19 -1

-/**
  * ep93xx_spi_interrupt() - SPI interrupt handler
  * @irq: IRQ number (not used)
  * @dev_id: pointer to EP93xx controller struct

@@ -792,6 +841,27 @@ static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id)
 			 * interrupt then.
 			 */
 			return IRQ_HANDLED;
+		} else {
+			struct spi_message *msg = espi->current_msg;
+			struct spi_transfer *t = msg->state;
+
+			if (!espi->continuous)
+				goto done;
+
+			if (!list_is_last(&t->transfer_list, &msg->transfers)) {
+				/*
+				 * Pick next transfer and refill the FIFO.
+				 */
+				msg->state = list_entry(t->transfer_list.next,
+							struct spi_transfer,
+							transfer_list);
+				espi->rx = 0;
+				espi->tx = 0;
+
+				ep93xx_spi_read_write(espi);
+				return IRQ_HANDLED;

Quoted 8 lines

Review static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id) · line 863

Quoted 1 line

(quoted diff) +1 -0

+			}

Here the N transfer has been completed and we are trying to continue with the N+1 transfer. This should work but I need to figure out a way to capture it with my logic analyzer. The only problem is if the N transfer is complete that means the tx and rx fifos are empty so the SFRM signal is going to be deasserted. The N+1 transfer might start really soon after but there will still be a glitch in the SFRM signal.

It would be great if you manage to capture it with the logic analyzer. Even if we get small glitch it means that we are going into right direction (then we just need to figure out how to keep the FIFO from emptying).

Other alternative is to use polling mode in v3 of the driver and see if it works better.

I was able to hack your driver to keep the continuous transfer going in a multi-transaction message. Unfortunately I don't have an easy way to generate a diff to send the necessary patch. The hack does work with the sst25l driver. It correctly detects the end of the first 4 byte transfer and immediately chains in the second 2 byte transfer. It ends up looking like this:

Quoted 2 lines · H Hartley Sweeten

I was able to hack your driver to keep the continuous transfer going in a multi-transaction message.

I modified my hack a bit so that the message transactions are dumped at the end of all transactions in the message.

I'm using Mika's hack to determine if the message could be continuous

about the message. By setting the 'continuous' flag to false right after that it disables my hack to make the transaction continuous so the message gets processed in the normal fashion.

The debug output decodes as:
S a transfer specific setting was made (in ep93xx_spi_process_transfer)
NOTE: I have never actually seen this happen.

  +   start of a transfer (in ep93xx_spi_process_fransfer).
  -   end of a transfer (in ep93xx_spi_process_fransfer).
  I   entered ep93xx_spi_interrupt
  I   exited ep93xx_spi_interrupt
  C   chained to the next transfer in a continuous transaction
  nR  the n number of fifo reads (in ep93xx_spi_read_write) or just 'R' if

only 1 byte is read; !8R if more than 8 reads were done (should never happen).
nW the n number of fifo writes (in ep93xx_spi_read_write) or just 'W' if
only 1 byte is written; !8W if more than 8 writes were done (should never happen).
nw the n number of fifo writes from the next transfer (in
ep93xx_spi_read_write) or just 'w' if only 1 byte is written; !8w if more than 8 writes were done (should never happen). NOTE: this only occurs with doing a continuous transaction.
F the transfer is complete ((espi->rx == t->len) in ep93xx_spi_read_write)

For the sst25l driver, with continuous transactions disabled, I get this for the Read-ID command (this is with SPI_DEBUG and MMC_DEBUG enabled):

spi spi0.0: setup mode 3, 8 bits/w, 20000000 Hz max --> 0 ep93xx-spi ep93xx-spi.0: 2 part continuous transaction (6 bytes) for sst25l ep93xx-spi ep93xx-spi.0: Done: +4WI4RFi-+2WI2RFi-
sst25l spi0.0: sst25lf040a (512 KiB)
Creating 3 MTD partitions on "SPI Flash":
0x000000000000-0x000000001000 : "SPI bootstrap" 0x000000001000-0x000000002000 : "Bootstrap config" 0x000000002000-0x000000080000 : "unallocated"
ep93xx-spi ep93xx-spi.0: registered child spi0.0

With continuous transactions enabled its:

spi spi0.0: setup mode 3, 8 bits/w, 20000000 Hz max --> 0 ep93xx-spi ep93xx-spi.0: 2 part continuous transaction (6 bytes) for sst25l ep93xx-spi ep93xx-spi.0: Done: 4W2wI4RFC2RFi
sst25l spi0.0: sst25lf040a (512 KiB)
Creating 3 MTD partitions on "SPI Flash":
0x000000000000-0x000000001000 : "SPI bootstrap" 0x000000001000-0x000000002000 : "Bootstrap config" 0x000000002000-0x000000080000 : "unallocated"
ep93xx-spi ep93xx-spi.0: registered child spi0.0

The '+' and '-' are missing since the transfer is started in ep93xx_spi_process_messange and then handed off to the interrupt routine without first calling ep93xx_spi_process_transfer. You can see the 2 bytes of the second transfer getting chained to the first 4 bytes during the priming of the fifo. Then after the interrupt the first 4 reads finish the first transaction. The next transaction is then chained in and also finishes (2 reads) then the interrupt completes. Message done!

With the mmc_spi driver the output is WAY to long to post. But, with continuous transactions disabled the card works fine. With continuous transactions enabled it doesn't for some reason.

I have looked over the output a number of times and it appears the continuous transfer works correctly. There are 3 that happen when the mmc_spi driver has the spi clock set to 400000 Hz max, I believe this is while the upper layers are detecting/configuring the card. One happens after a CMD10 and is a "mmc_spi: read block, 16 bytes" and is a 16+2 byte message. This is followed by a CMD9 then another "mmc_spi: read block, 16 bytes" which is another 16+2 byte message. Then there is a CMD55 and a CMD51. The CMD51 does a 9 byte message then a 1 byte message followed by a "mmc_spi: read block, 8 bytes". That's followed by 4 separate 1 byte messages then an 8+2 byte message. All of these n+m messages appear to work fine as a continuous transfer.

Shortly after this the following is output:

mmc0: new SD card on SPI

mmcblk0: mmc0:0000 S064B 60.6 MiB (ro)
mmcblk0:

Right after that there is a CMD18. I think this is where the upper layer is trying to read the partition table. This is where it getting interesting compared to the non- continuous transfer.

mmcblk0: mmc0:0000 S064B 60.6 MiB (ro)
mmcblk0:
mmc0: starting CMD18 arg 00000000 flags 000000b5
mmc0: blksz 512 blocks 8 flags 00000200 tsac 100 ms nsac 10000
mmc0: CMD12 arg 00000000 flags 0000049d
mmc_spi spi0.1: mmc_spi: CMD18, resp R1
ep93xx-spi ep93xx-spi.0: 1 part transaction (9 bytes) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +8WI8RWiIRFi-
ep93xx-spi ep93xx-spi.0: 1 part transaction (1 byte) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +WIRFi-
mmc_spi spi0.1: mmc_spi: read block, 512 bytes
ep93xx-spi ep93xx-spi.0: 1 part transaction (1 byte) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +WIRFi-
ep93xx-spi ep93xx-spi.0: 1 part transaction (1 byte) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +WIRFi-
ep93xx-spi ep93xx-spi.0: 1 part transaction (1 byte) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +WIRFi-
ep93xx-spi ep93xx-spi.0: 1 part transaction (1 byte) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +WIRFi-
ep93xx-spi ep93xx-spi.0: 3 part continuous transaction (515 bytes) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: 8WI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi

                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                               I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R2wFC2RwFCiIRFi
 mmc_spi spi0.1:     mmc_spi: read block, 512 bytes
 ep93xx-spi ep93xx-spi.0: 1 part transaction (1 byte) for mmc_spi
 ep93xx-spi ep93xx-spi.0: Done: +WIRFi-
 ep93xx-spi ep93xx-spi.0: 1 part transaction (1 byte) for mmc_spi
 ep93xx-spi ep93xx-spi.0: Done: +WIRFi-
 ep93xx-spi ep93xx-spi.0: 3 part continuous transaction (515 bytes) for mmc_spi
 ep93xx-spi ep93xx-spi.0: Done: 8WI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8WiI8R8Wi
                                         I8R2wFC2RwFCiIRFi

mmc_spi spi0.1: read - crc error: crc_val=0x0000, computed=0x8258 len=512 mmc_spi spi0.1: read status -84
mmc_spi spi0.1: mmc_spi: CMD12, resp R1B
ep93xx-spi ep93xx-spi.0: 1 part transaction (29 bytes) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +8WI8R8WiI8R8WiI8R5WiI5RFi- mmc_spi spi0.1: setup mode 3, 8 bits/w, 20000000 Hz max --> 0
mmc0: req done (CMD18): 0: 00000000 00000000 00000000 00000000
mmc0: 512 bytes transferred: -84
mmc0: (CMD12): 0: 00000000 00000000 00000000 00000000

mmcblk0: retrying using single block read

At this point everything just goes nuts. There are a bunch of CMD17 then CMD13.

mmc0: starting CMD17 arg 00000000 flags 000000b5
mmc0: blksz 512 blocks 1 flags 00000200 tsac 100 ms nsac 10000
mmc_spi spi0.1: mmc_spi: CMD17, resp R1
ep93xx-spi ep93xx-spi.0: 1 part transaction (9 bytes) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +8WI8RWiIRFi-
mmc_spi spi0.1: ... CMD17 response SPI_R1: resp 0004 00000000
mmc_spi spi0.1: setup mode 3, 8 bits/w, 20000000 Hz max --> 0
mmc0: req done (CMD17): -38: 00000004 00000000 00000000 00000000
mmc0: 0 bytes transferred: 0
mmc0: starting CMD13 arg 00000000 flags 00000195
mmc_spi spi0.1: mmc_spi: CMD13, resp R2/R5
ep93xx-spi ep93xx-spi.0: 1 part transaction (18 bytes) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +8WI8R8WiI8R2WiI2RFi-
mmc_spi spi0.1: ... CMD13 response SPI_R2/R5: resp 0004 00000000
mmc_spi spi0.1: setup mode 3, 8 bits/w, 20000000 Hz max --> 0
mmc0: req done (CMD13): -38: 00000004 00000000 00000000 00000000

mmcblk0: error -38 sending status comand

mmcblk0: error -38 sending read/write command, response 0x4, card status 0x4
end_request: I/O error, dev mmcblk0, sector 0

The only difference appears the be the arg for CMD17 and the end_request output:

mmc0: starting CMD17 arg 00000000 flags 000000b5
...
end_request: I/O error, dev mmcblk0, sector 0
mmc0: starting CMD17 arg 00000200 flags 000000b5
...
end_request: I/O error, dev mmcblk0, sector 1
mmc0: starting CMD17 arg 00000400 flags 000000b5
...
end_request: I/O error, dev mmcblk0, sector 2
mmc0: starting CMD17 arg 00000600 flags 000000b5
...
end_request: I/O error, dev mmcblk0, sector 3
mmc0: starting CMD17 arg 00000800 flags 000000b5
...
end_request: I/O error, dev mmcblk0, sector 4
mmc0: starting CMD17 arg 00000a00 flags 000000b5
...
end_request: I/O error, dev mmcblk0, sector 5
mmc0: starting CMD17 arg 00000c00 flags 000000b5
...
end_request: I/O error, dev mmcblk0, sector 6
mmc0: starting CMD17 arg 00000e00 flags 000000b5
...

end_request: I/O error, dev mmcblk0, sector 7

Then there is a:

Buffer I/O error on device mmcblk0, logical block 0
mmc0: starting CMD18 arg 00000000 flags 000000b5
mmc0: blksz 512 blocks 8 flags 00000200 tsac 100 ms nsac 10000
mmc0: CMD12 arg 00000000 flags 0000049d
mmc_spi spi0.1: mmc_spi: CMD18, resp R1
ep93xx-spi ep93xx-spi.0: 1 part transaction (9 bytes) for mmc_spi ep93xx-spi ep93xx-spi.0: Done: +8WI8RWiIRFi-
mmc_spi spi0.1: ... CMD18 response SPI_R1: resp 0004 00000000
mmc_spi spi0.1: setup mode 3, 8 bits/w, 20000000 Hz max --> 0
mmc0: req done (CMD18): -38: 00000004 00000000 00000000 00000000
mmc0: 0 bytes transferred: 0
mmc0: (CMD12): 0: 00000000 00000000 00000000 00000000

mmcblk0: retrying using single block read

Followed by all the CMD17 stuff above again. Then it ends with:

Buffer I/O error on device mmcblk0, logical block 0
unable to read partition table

I can't see any reason why the continuous transfer would cause a crc error. Even if it did I can't see why the single block reads would all fail, all of then are composed of 1 transfer messages.

Anyone have some suggestions?

Again, sorry for the long message.

Regards,

Hartley

SFRMOUT will have gone high:

        if (espi->tx  >0 && espi->tx < t->len

                /* More to transmit but device has gone idle means that
                 * SFRMOUT will have gone high */
                printk("ep93xx-spi: Underrun\n");
        }

Sorry about the incomplete message. Finger trouble.

On 4/25/10, Martin Guy <[email protected]> wrote:
...

Quoted 8 lines

SFRMOUT will have gone high:

        if (espi->tx  >0 && espi->tx < t->len
              && !(ep93xx_spi_read_u16(espi, SSPSR) & SSPSR_BSY)) {
                /* More to transmit but device has gone idle means that
                 * SFRMOUT will have gone high */
                printk("ep93xx-spi: Underrun\n");
        }

I've also done a version that doesn't printk() in the middle of it, which affects timing, but counts the underruns and reports at end of transfer. The result is that every 512-byte block underruns at least once, sometimes twice.

I'll run a few more tests, e.g. dropping the clock rate, but at this point I'd be inclined to declare it impossible to keep SFRMOUT low during a transfer, and just use the simple non-continuous code with a comment at the top of the file that explains why you can't use SFRMOUT as a chip select for devices that require CS to remain asserted for the duration of a transfer.

Quoted 6 lines · Martin Guy

I'll run a few more tests, e.g. dropping the clock rate, but at this point I'd be inclined to declare it impossible to keep SFRMOUT low during a transfer, and just use the simple non-continuous code with a comment at the top of the file that explains why you can't use SFRMOUT as a chip select for devices that require CS to remain asserted for the duration of a transfer.

With a slow enough clock you can probably get to a point where SFRMOUT will stay deasserted during the entire 512 byte transfer. But it would still de-assert during the switch to the next transfer in the message.

Regardless, I tend to agree with you. Because of the chip design, it's impossible (or at least you can't guarantee) to keep the SFRMOUT low during a transfer. Some devices can live with this others can't. Oh well.

A Documentation/spi/ep93xx_spi.txt really should be created to note this limitation as well as showing an example of the platform support needed to use this driver.

Hi,

I just answer to this last message as it seems that continuous transfer is really not a 100% solid solution; It makes things more complicated than need to be and benefit for that is probably not worth the additional complexity. Feel free to disagree :)

Quoted 8 lines · Martin Guy

Quoted 6 lines

I'll run a few more tests, e.g. dropping the clock rate, but at this point I'd be inclined to declare it impossible to keep SFRMOUT low during a transfer, and just use the simple non-continuous code with a comment at the top of the file that explains why you can't use SFRMOUT as a chip select for devices that require CS to remain asserted for the duration of a transfer.

Quoted 7 lines

With a slow enough clock you can probably get to a point where SFRMOUT will stay deasserted during the entire 512 byte transfer. But it would still de-assert during the switch to the next transfer in the message.

Regardless, I tend to agree with you. Because of the chip design, it's impossible (or at least you can't guarantee) to keep the SFRMOUT low during a transfer. Some devices can live with this others can't. Oh well.

For the MMC over SPI at least we really cannot ever make fully compliant solution with SFRMOUT because, during init phase protocol sends CMD0 (reset) with chipselect active high (SPI_CS_HIGH). This forces SD/MMC card to SPI mode. However, many cards seem to work without any chipselect logic at all (with Sim.One for example).

A Documentation/spi/ep93xx_spi.txt really should be created to note this limitation as well as showing an example of the platform support needed to use this driver.

Do we all agree that we can live with GPIOs as chipselect? Or do we continue investigation to make it work with boards that use SFRMOUT as CS?

I have following plans for v5 (in case we don't care about SFRMOUT). Before doing the changes I would like to hear your opinnion.

- prime the FIFO first in every transfer (Hartley's suggestion)

- address review comments for v4
- add Martin's patch for clearing SSPICR only if we get ROR

- add necessary documentation under Documentation/spi/ep93xx_spi.txt

With a slow enough clock you can probably get to a point where SFRMOUT will stay deasserted during the entire 512 byte transfer. But it would still de-assert during the switch to the next transfer in the message.

Regardless, I tend to agree with you. Because of the chip design, it's impossible (or at least you can't guarantee) to keep the SFRMOUT low during a transfer. Some devices can live with this others can't. Oh well.

During the 512+2+1 message that is sent when the mmc_spi driver is doing a block read, the 512 byte transfer goes like this:

1) 8 writes, prime the tx fifo
SFRMOUT asserts when the first bit of the first byte starts transmitting
2) Interrupt
3) 8 reads, we must have emptied the tx fifo and filled the rx fifo SFRMOUT de-asserts after the the last bit of the last byte is received

I have modifed my board so that I now have an external GPIO for the chip select.

Quoted 1 line

I'll run a few more tests, e.g. dropping the clock rate

With a slow enough clock you can probably get to a point where SFRMOUT
will stay deasserted during the entire 512 byte transfer

But it would
still de-assert during the switch to the next transfer in the message.

Quoted 10 lines · H Hartley Sweeten

During the 512+2+1 message that is sent when the mmc_spi driver is doing a block read, the 512 byte transfer goes like this:

1) 8 writes, prime the tx fifo
SFRMOUT asserts when the first bit of the first byte starts transmitting
2) Interrupt
3) 8 reads, we must have emptied the tx fifo and filled the rx fifo SFRMOUT de-asserts after the the last bit of the last byte is received

Are you printk-ing the diagnostics in the irq routine?
If so, that slows it down enough to ensure every 8-word block underruns.

Here are stats for 512-word transfers from SD card at 3.7 MHz; figures are number of underruns/total number of reads and the distribution of words read per interrupt [0..8], collected during the transfer and printed once the transfer is over)

1/102 [1,0,2,0,0,100,0,0,1]
1/102 [1,0,2,0,0,100,0,0,1]
1/102 [1,0,2,0,0,100,0,0,1]
2/104 [1,1,0,2,21,76,3,1,2]
3/102 [1,1,1,1,11,85,1,1,3]
4/100 [1,0,1,1,7,83,3,2,4]

while 1- and 2-word transfers seem to be caught on the second interrupt. [1,2,0,0,0,0,0,0,0]
[1,1,0,0,0,0,0,0,0]

Quoted 2 lines

I have modifed my board so that I now have an external GPIO for the chip select.

Yes, we'll have to do that too.

Quoted 4 lines

Quoted 1 line

I'll run a few more tests, e.g. dropping the clock rate

With a slow enough clock you can probably get to a point where SFRMOUT
will stay deasserted during the entire 512 byte transfer

I've tried it down to 400kHz and transfers still underrun (!)

So it does seem that SFRMOUT cannot be used as a chip select unless we revert to the old busy-wait-for-whole-transfer code, which causes 100% CPU usage during transfers.
and 1ms-long pauses (for 512 words at 3.7MHz).

This suggests that we can only handle SFRMOUT used as chip select correctly by reinstating polling mode, adding the continuous transfer code and allowing the board definition code to specify that polling mode be used for such devices.
(much as I hate polling mode :)

Quoted 1 line

But it would

still de-assert during the switch to the next transfer in the message.

Am I right in thinking that the GPIO chip select currently remains asserted as it should during a multi-transfer message?

and the SDHC cards I have don't work at all, spewing tons of:
mmcblk0: error -38 sending status comand
mmcblk0: error -38 sending read/write command, response 0x4, card status 0xff04
end_request: I/O error, dev mmcblk0, sector 7744509

From d98fcab5f26e3ee8185232b51baeaae66b1ac0f3 Mon Sep 17 00:00:00 2001

Quoted 5 lines · Mika Westerberg

This patch adds an SPI master driver for the Cirrus EP93xx SPI controller found in EP93xx chips (EP9301, EP9302, EP9307, EP9312 and EP9315).

Signed-off-by Mika Westerberg <[email protected]>

Mika,

I discovered one gotcha with this driver. Bear with me through this...

1. ep93xx_spi_process_message is called by the workqueue worker function to handle a spi_message queued in by the ep93xx_spi_transfer function. Before starting the actual transfer the chip select to the device is asserted.
2. ep93xx_spi_process_transfer processes each transfer in the spi_message, one at a time, and does a wait_for_completion on each transfer.
3. When the entire spi_message has been transferred the chip select to the device is deasserted.

The problem is if a hardware design uses the SFRM1 pin as part of the chip select logic. The EP93xx User's Guide states that "the SFRMOUT signal behaves as a slave Select" in Motorola SPI mode. This signal behaves differently depending on the SPI_MODE_* used.

Modes 0 and 2:
SFRMOUT is forced HIGH during idle periods.
Start of transmission, SFRMOUT is driven LOW.
Single word transmission, SFRMOUT is returned to its idle HIGH state one
SCLKOUT period after the last bit has been captured.
Back-to-back transmissions, SFRMOUT signal is pulsed HIGH between each
data word transfer. On completion the SFRMOUT pin returns to its idle HIGH state one SCLKOUT period after the last bit has been captured.

Modes 1 and 3:
SFRMOUT is forced HIGH during idle periods.
Start of transmission, SFRMOUT is driven LOW.
Single word transmission, SFRMOUT is returned to its idle HIGH state one
SCLKOUT period after the last bit has been captured.
Back-to-back transmissions, SFRMOUT signal is held LOW between each
data word transfer. On completion the SFRMOUT pin returns to its idle HIGH state one SCLKOUT period after the last bit has been captured.

So, since each transfer does a wait_for_completion, all the data is transmitted which causes the SFRMOUT pin to go HIGH between each transfer in the message.

For devices like the sst25lf040a SPI Flash on the EDB93xx boards this causes a problem. To read data from that device you need to send a two part message. The first part is a write transfer with the one byte command and three byte address. This is then followed by a read transfer to get the data. But since the SFRMOUT pin goes high this ends the command.

The driver Ryan and I worked on actually does work with the sst25lf040a on the EDB93xx boards. But now that I know what the issue is, it was actually a race condition. Sometimes it worked and sometimes it didn't...

I would think the Sim.One board (Martin?) would have the same issue with the mmc card since that design uses the SFRMOUT pin directly for the chip select.

Is there anyway to keep the transfers going in a muiltipart message?

Quoted 11 lines · H Hartley Sweeten

Quoted 5 lines · Mika Westerberg

This patch adds an SPI master driver for the Cirrus EP93xx SPI controller found in EP93xx chips (EP9301, EP9302, EP9307, EP9312 and EP9315).

Signed-off-by Mika Westerberg <[email protected]>

Mika,

I discovered one gotcha with this driver. Bear with me through this...

Quoted 7 lines

1. ep93xx_spi_process_message is called by the workqueue worker function to handle a spi_message queued in by the ep93xx_spi_transfer function. Before starting the actual transfer the chip select to the device is asserted.
2. ep93xx_spi_process_transfer processes each transfer in the spi_message, one at a time, and does a wait_for_completion on each transfer.
3. When the entire spi_message has been transferred the chip select to the device is deasserted.

Yes. There is possibility that last transfer ->cs_change is set which can be used by SPI controller drivers to keep the chip selected in case next message is going to the same device.

However, I haven't implemented that as it is optional (but trivial to add afterwards).

Quoted 23 lines

The problem is if a hardware design uses the SFRM1 pin as part of the chip select logic. The EP93xx User's Guide states that "the SFRMOUT signal behaves as a slave Select" in Motorola SPI mode. This signal behaves differently depending on the SPI_MODE_* used.

Modes 0 and 2:
SFRMOUT is forced HIGH during idle periods.
Start of transmission, SFRMOUT is driven LOW.
Single word transmission, SFRMOUT is returned to its idle HIGH state one
SCLKOUT period after the last bit has been captured.
Back-to-back transmissions, SFRMOUT signal is pulsed HIGH between each
data word transfer. On completion the SFRMOUT pin returns to its idle HIGH state one SCLKOUT period after the last bit has been captured.

Modes 1 and 3:
SFRMOUT is forced HIGH during idle periods.
Start of transmission, SFRMOUT is driven LOW.
Single word transmission, SFRMOUT is returned to its idle HIGH state one
SCLKOUT period after the last bit has been captured.
Back-to-back transmissions, SFRMOUT signal is held LOW between each
data word transfer. On completion the SFRMOUT pin returns to its idle HIGH state one SCLKOUT period after the last bit has been captured.

Quoted 2 lines

So, since each transfer does a wait_for_completion, all the data is transmitted which causes the SFRMOUT pin to go HIGH between each transfer in the message.

Yes, unfortunately. SFRMOUT cannot be software controlled so it is not suitable for general purpose chipselect. This was the sole purpose of using GPIO lines for chipselects.

Quoted 6 lines

For devices like the sst25lf040a SPI Flash on the EDB93xx boards this causes a problem. To read data from that device you need to send a two part message. The first part is a write transfer with the one byte command and three byte address. This is then followed by a read transfer to get the data. But since the SFRMOUT pin goes high this ends the command.>

Quoted 8 lines

The driver Ryan and I worked on actually does work with the sst25lf040a on the EDB93xx boards. But now that I know what the issue is, it was actually a race condition. Sometimes it worked and sometimes it didn't...

I would think the Sim.One board (Martin?) would have the same issue with the mmc card since that design uses the SFRMOUT pin directly for the chip select.

Is there anyway to keep the transfers going in a muiltipart message?

I'm not exactly sure but "polling" mode sounds something that could work around that. It just need to be sure that it transmits continuously to keep the SFRMOUT asserted.

You could try v3 of the driver with:

	# modprobe ep93xx-spi transfer_method=0

and see if it works there.

An alternative would be that interrupt handler schedules tasklet which then takes next transfer immediately once current transfer is finished.

Hi, another little fix:

EP93xx User's Manual -> Synchronous Serial Port -> Registers

SSPIIR description:
"Read Only
Note: A write to this register clears the receive overrun interrupt, regardless of the data
value written."

It doesn't affect the RIS/TIS ones, which are caused by the state of the device.

so

static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id) {
...
if (!(irq_status & (SSPIIR_RORIS | SSPIIR_TIS | SSPIIR_RIS)))
return IRQ_NONE; /* not for us */

(quoted diff) +0 -2

-       /* clear the interrupt */
-       ep93xx_spi_write_u8(espi, SSPICR, 0);
 
        /*

Quoted 10 lines · Martin Guy

static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id) {
...
if (!(irq_status & (SSPIIR_RORIS | SSPIIR_TIS | SSPIIR_RIS)))
return IRQ_NONE; /* not for us */

(quoted diff) +0 -2

-       /* clear the interrupt */
-       ep93xx_spi_write_u8(espi, SSPICR, 0);
 
        /*

Review

Quoted 10 lines

(quoted diff) +2 -0

         * If we got ROR (receive overrun) interrupt we know that something is
         * wrong. Just abort the message.
         */
        if (unlikely(irq_status & SSPIIR_RORIS)) {
+               /* clear the overrun interrupt */
+               ep93xx_spi_write_u8(espi, SSPICR, 0);
                dev_warn(&espi->pdev->dev,
                         "receive overrun, aborting the message\n");
                espi->current_msg->status = -EIO;
        } else {

With this patch the interrupts only get cleared if it's a ROR interrupt.

Do the RIS/TIS interrupts get cleared by the upper layers? If so, do we really need to clear the ROR interrupt here?

Quoted 4 lines

(quoted diff) +0 -2

-       /* clear the interrupt */
-       ep93xx_spi_write_u8(espi, SSPICR, 0);
 
        /*

Review

Quoted 10 lines

(quoted diff) +2 -0

         * If we got ROR (receive overrun) interrupt we know that something is
         * wrong. Just abort the message.
         */
        if (unlikely(irq_status & SSPIIR_RORIS)) {
+               /* clear the overrun interrupt */
+               ep93xx_spi_write_u8(espi, SSPICR, 0);
                dev_warn(&espi->pdev->dev,
                         "receive overrun, aborting the message\n");
                espi->current_msg->status = -EIO;
        } else {

With this patch the interrupts only get cleared if it's a ROR interrupt.

Do the RIS/TIS interrupts get cleared by the upper layers? If so, do we really need to clear the ROR interrupt here?

This patch adds platform side support code for EP93xx SPI driver. This includes clock, resources and muxing. There is a new function: ep93xx_register_spi() that can be used by board support code to register new SPI devices for the board.

This patch depends on following ARM patch:
5998/1 ep93xx: added chip revision reading function

Signed-off-by Mika Westerberg <[email protected]>

---
 arch/arm/mach-ep93xx/clock.c                    |   14 +++++++
 arch/arm/mach-ep93xx/core.c                     |   48 +++++++++++++++++++++++
 arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |    1 +
 arch/arm/mach-ep93xx/include/mach/platform.h    |    2 +
 4 files changed, 65 insertions(+), 0 deletions(-)

arch/arm/mach-ep93xx/clock.c +0 -0

index 5f80092..5986fae 100644

arch/arm/mach-ep93xx/clock.c +0 -0

 	INIT_CK(NULL,			"pwm_clk",	&clk_pwm),
 	INIT_CK(NULL,			"m2p0",		&clk_m2p0),
 	INIT_CK(NULL,			"m2p1",		&clk_m2p1),

arch/arm/mach-ep93xx/clock.c +2 -0

+
 	pr_info("PLL1 running at %ld MHz, PLL2 at %ld MHz\n",
 		clk_pll1.rate / 1000000, clk_pll2.rate / 1000000);
 	pr_info("FCLK %ld MHz, HCLK %ld MHz, PCLK %ld MHz\n",

@@ -480,6 +493,7 @@ static int __init ep93xx_clock_init(void)
 		clk_p.rate / 1000000);
 
 	clkdev_add_table(clocks, ARRAY_SIZE(clocks));
+
 	return 0;
 }
 arch_initcall(ep93xx_clock_init);

arch/arm/mach-ep93xx/core.c +9 -0

index 90fb591..e237309 100644
@@ -35,6 +35,7 @@
 #include <mach/hardware.h>
 #include <mach/fb.h>
 #include <mach/ep93xx_keypad.h>
+#include <mach/ep93xx_spi.h>
 
 #include <asm/mach/map.h>
 #include <asm/mach/time.h>

@@ -363,6 +364,53 @@ void __init ep93xx_register_eth(struct ep93xx_eth_data *data, int copy_addr)
 	platform_device_register(&ep93xx_eth_device);
 }
 
+static struct ep93xx_spi_info ep93xx_spi_master_data;
+
+static struct resource ep93xx_spi_resources[] = {
+	{
+		.start	= EP93XX_SPI_PHYS_BASE,
+		.end	= EP93XX_SPI_PHYS_BASE + 0x18 - 1,
+		.flags	= IORESOURCE_MEM,
+	},

arch/arm/mach-ep93xx/core.c +5 -0

+	.dev		= {
+		.platform_data = &ep93xx_spi_master_data,
+	},
+	.num_resources	= ARRAY_SIZE(ep93xx_spi_resources),
+	.resource	= ep93xx_spi_resources,

arch/arm/mach-ep93xx/core.c +10 -0

+{
+	/*
+	 * When SPI is used, we need to make sure that I2S is muxed off from
+	 * SPI pins.
+	 */
+	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_I2SONSSP);
+
+	ep93xx_spi_master_data = *info;
+	return platform_device_register(&ep93xx_spi_device);
+}
 
 /*************************************************************************
  * EP93xx i2c peripheral handling

arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h +1 -0

index 93e2ecc..b1e096f 100644
@@ -106,6 +106,7 @@
 
 #define EP93XX_AAC_BASE			EP93XX_APB_IOMEM(0x00080000)
 
+#define EP93XX_SPI_PHYS_BASE		EP93XX_APB_PHYS(0x000a0000)
 #define EP93XX_SPI_BASE			EP93XX_APB_IOMEM(0x000a0000)
 
 #define EP93XX_IRDA_BASE		EP93XX_APB_IOMEM(0x000b0000)

arch/arm/mach-ep93xx/include/mach/platform.h +2 -0

index c6dc14d..5816581 100644
@@ -9,6 +9,7 @@ struct i2c_board_info;
 struct platform_device;
 struct ep93xxfb_mach_info;
 struct ep93xx_keypad_platform_data;
+struct ep93xx_spi_info;
 
 struct ep93xx_eth_data
 {

@@ -34,6 +35,7 @@ static inline void ep93xx_devcfg_clear_bits(unsigned int bits)
 }
 
 void ep93xx_register_eth(struct ep93xx_eth_data *data, int copy_addr);
+int ep93xx_register_spi(struct ep93xx_spi_info *info);
 void ep93xx_register_i2c(struct i2c_gpio_platform_data *data,
 			 struct i2c_board_info *devices, int num);
 void ep93xx_register_fb(struct ep93xxfb_mach_info *data);