diff mbox

mxs/spi: Add SPI slave mode operation DT prop

Message ID 1345776161-9778-1-git-send-email-marex@denx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Marek Vasut Aug. 24, 2012, 2:42 a.m. UTC
This allows user to select the slave mode of operation of the controller.
This is by no means standard, see the binding documentation for details,
there is plenty of them. Sadly, such knowledge is not provided in the chip
documentation. Hopefully, this mode of operation might come useful for
people who do know very well what they are doing, otherwise this should
never be touched.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Chris Ball <cjb@laptop.org>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 Documentation/devicetree/bindings/spi/mxs-spi.txt |   16 +++++++++++++
 drivers/spi/spi-mxs.c                             |   25 +++++++++++++--------
 include/linux/spi/mxs-spi.h                       |    1 +
 3 files changed, 33 insertions(+), 9 deletions(-)

NOTE1: I think I'll be crucified for bringing this up ;-)
NOTE2: I wonder if bringing such option in is good idea, but I hope it is.
       This thing seems to be useful at least to some people.
NOTE3: This at least documents the actual way of how to use the SPI slave mode
       on the MX28. The documentation for MX28 provided by Freescale is severely
       lacking in this matter. Some information (CPOL and CPHA note) were
       therefore retrieved from STMP36xx manual, which seems to have similar SSP
       IP core. The rest of the information (that the clock must run at much
       higher speed) was retrieved by performing long going slave work.
       Freescale only immersed themself in management shenanigans and were in
       the end not helpful at all. I hope someone will find the supplied
       information useful, even if this patch was only to be archived in the
       kernel list archives.

Comments

Mark Brown Aug. 27, 2012, 4:39 p.m. UTC | #1
On Fri, Aug 24, 2012 at 04:42:41AM +0200, Marek Vasut wrote:
> This allows user to select the slave mode of operation of the controller.
> This is by no means standard, see the binding documentation for details,
> there is plenty of them. Sadly, such knowledge is not provided in the chip
> documentation. Hopefully, this mode of operation might come useful for
> people who do know very well what they are doing, otherwise this should
> never be touched.

I was going to touch this but I appear to have misplaced my bargepole :)

More seriously as you say this is a bit controversial so given that
Grant's now finished his move and should reappear soon I'll punt for
now.
Trent Piepho March 20, 2013, 10:58 p.m. UTC | #2
On Thu, Aug 23, 2012 at 7:42 PM, Marek Vasut
<marex-ynQEQJNshbs@public.gmane.org> wrote:
> This allows user to select the slave mode of operation of the controller.
> This is by no means standard, see the binding documentation for details,
> there is plenty of them. Sadly, such knowledge is not provided in the chip
> documentation. Hopefully, this mode of operation might come useful for
> people who do know very well what they are doing, otherwise this should
> never be touched.

Thanks for posting this, as it's something I'm trying to get it working too.

You're right that the Freescale documentation says almost nothing
about slave mode.  I found some STMP36xx on the rockbox site,
http://www.rockbox.org/wiki/pub/Main/SigmaTelSTMP3xxx/stmp36xx-Datasheet-1-02_050306.pdf,
however it doesn't provide any more information about slave mode than
the imx28 RM.  In fact it's virtually the same word for word, other
than some bits that are marked reserved in the imx28 are defined in
stmp36xx (wonder if the FIFO stuff is still there in the imx28?).
What STMP36xx documentation did you find that had more information?

> +		   - If the SPI master generates clock at n MHz, the speed
> +		     of the SPI slave controller must be at least 4-8 times
> +		     faster, this is due to the controller samples the CLK

When you say the SPI slave clock must be 4x higher than the master
clock, do you mean the rate programmed for what would be the SPI clock
in master mode: SSP_SCK in the HW_SSP_TIMING register.  Or the rate of
the clock to the SSP block: SSPCLK as programmed via the CLKCTRL
block's registers, e..g. HW_CLKCTRL_SSP1?

> +		   - The DMA has to wait indefinitelly for the arriving data.

Is there a reason that this must be done?  I'd guess that after the
SSP is told to start a transfer in slave mode it waits for the master
to assert the SSn line and begin the transfer.  If the completion
times out before this happens then it makes sense it wouldn't work.
But does it still not work if the SPI transfer completes before the
completion timeout?  Or is this necessary because in your application
there might be a long wait before the master chooses to initiate a
transfer after you program it on the Linux side?

In my case I have a device that is mostly a SPI slave, except it wants
to drive the clock itself, most unfortunately.  It doesn't drive the
slave select, rather the imx28 will still drive that (I assume I'll
need to use a GPIO and loop it back to SSn to trick the SSP into
thinking the "master" has asserted the slave select).  So really, I'm
doing SPI master transactions on the Linux side, but need to put the
SSP into slave mode to get the clock right.  This makes it quite a bit
easier to do on Linux than a general slave mode, which is quite the
can of worms.  I tried creating an I2C slave mode framework for
another application but was never happy with it.  I imagine this is
why no one has ever created a general spi slave framework for Linux.
Marek Vasut March 20, 2013, 11:35 p.m. UTC | #3
Dear Trent Piepho,

> On Thu, Aug 23, 2012 at 7:42 PM, Marek Vasut
> 
> <marex-ynQEQJNshbs@public.gmane.org> wrote:
> > This allows user to select the slave mode of operation of the controller.
> > This is by no means standard, see the binding documentation for details,
> > there is plenty of them. Sadly, such knowledge is not provided in the
> > chip documentation. Hopefully, this mode of operation might come useful
> > for people who do know very well what they are doing, otherwise this
> > should never be touched.
> 
> Thanks for posting this, as it's something I'm trying to get it working
> too.
> 
> You're right that the Freescale documentation says almost nothing
> about slave mode.  I found some STMP36xx on the rockbox site,
> http://www.rockbox.org/wiki/pub/Main/SigmaTelSTMP3xxx/stmp36xx-Datasheet-1-
> 02_050306.pdf, however it doesn't provide any more information about slave
> mode than the imx28 RM.  In fact it's virtually the same word for word,
> other than some bits that are marked reserved in the imx28 are defined in
> stmp36xx (wonder if the FIFO stuff is still there in the imx28?).
> What STMP36xx documentation did you find that had more information?

I spammed Freescale with service requests.

> > +		   - If the SPI master generates clock at n MHz, the speed
> > +		     of the SPI slave controller must be at least 4-8 times
> > +		     faster, this is due to the controller samples the CLK
> 
> When you say the SPI slave clock must be 4x higher than the master
> clock, do you mean the rate programmed for what would be the SPI clock
> in master mode: SSP_SCK in the HW_SSP_TIMING register.  Or the rate of
> the clock to the SSP block: SSPCLK as programmed via the CLKCTRL
> block's registers, e..g. HW_CLKCTRL_SSP1?

The SPI clock (aka. sampling clock of the SCK line).

> > +		   - The DMA has to wait indefinitelly for the arriving data.
> 
> Is there a reason that this must be done?  I'd guess that after the
> SSP is told to start a transfer in slave mode it waits for the master
> to assert the SSn line and begin the transfer.  If the completion
> times out before this happens then it makes sense it wouldn't work.
> But does it still not work if the SPI transfer completes before the
> completion timeout?  Or is this necessary because in your application
> there might be a long wait before the master chooses to initiate a
> transfer after you program it on the Linux side?

The problem is the SPI block in slave mode ignores SS being pulled back up.

> In my case I have a device that is mostly a SPI slave, except it wants
> to drive the clock itself, most unfortunately.  It doesn't drive the
> slave select, rather the imx28 will still drive that (I assume I'll
> need to use a GPIO and loop it back to SSn to trick the SSP into
> thinking the "master" has asserted the slave select).  So really, I'm
> doing SPI master transactions on the Linux side, but need to put the
> SSP into slave mode to get the clock right.  This makes it quite a bit
> easier to do on Linux than a general slave mode, which is quite the
> can of worms.  I tried creating an I2C slave mode framework for
> another application but was never happy with it.  I imagine this is
> why no one has ever created a general spi slave framework for Linux.

SPI slave is hard because you can not anticipate the amount of data you will 
receive. You can not anticipate when you will receive those either.

Best regards,
Marek Vasut
Trent Piepho March 23, 2013, 6:20 a.m. UTC | #4
On Wed, Mar 20, 2013 at 4:35 PM, Marek Vasut <marex@denx.de> wrote:
>> On Thu, Aug 23, 2012 at 7:42 PM, Marek Vasut
>> > +              - The DMA has to wait indefinitelly for the arriving data.
>>
>> Is there a reason that this must be done?  I'd guess that after the
>> SSP is told to start a transfer in slave mode it waits for the master
>> to assert the SSn line and begin the transfer.  If the completion
>> times out before this happens then it makes sense it wouldn't work.
>> But does it still not work if the SPI transfer completes before the
>> completion timeout?  Or is this necessary because in your application
>> there might be a long wait before the master chooses to initiate a
>> transfer after you program it on the Linux side?
>
> The problem is the SPI block in slave mode ignores SS being pulled back up.

I've setup a board with a spi-gpio port looped to a spi-mxs port in
slave mode so I can test this.  I did not need to change the DMA
timeout to get it to work.  If the master does not send enough within
the SSP timeout the slave transaction will timeout.  It does not seem
to ignore SS, but rather the DMA will not finish until all the
requested words are received, no matter how many SS pulses that
happens to take.  For general purpose slave support it would of course
be nice to be able to end the message when the master drops SS, but
the SSP either doesn't support this or a different technique is
needed.

I've found that if PHASE = 0, the SSP expects the SS to be pulsed high
between each word (w/ current driver, word == 8 bits).  This is
slightly alluded to in the imx28 manual, where the PHASE = 0
descriptions in sections 17.5.3 and 17.5.5 contain, "... in the case
of continuous back-to-back transmissions, the SSn signal must be
pulsed high between each data word[.]"  While for the PHASE = 1
descriptions in sections 17.5.4 and 17.5.6 it says, "For continuous
back-to-back transfers, the SSn pin is held low between successive
data words[.]"

One would think this is talking about master mode like the rest of the
section, but it seems to only apply to slave mode.  In master mode one
can easily produce a SPI signal without SS pulsed between each word
with either phase setting.

Testing with the master TX sending 1 byte per SS asserted period:
$ echo abcd1234ABCDwxyz | dd of=/dev/spidev4.0 bs=1

Slave RX receives the first four bytes:
$ dd if=/dev/spidev2.0 count=1 bs=4 | hd
[  638.673625] spidev spi2.0: Start POL 0 PHA 0
00000000  61 62 63 64                                       |abcd|

Trying with 4 bytes per SS assertion:
$ echo abcd1234ABCDwxyz | dd of=/dev/spidev4.0 bs=4
$ dd if=/dev/spidev2.0 count=1 bs=4 | hd
00000000  61 31 41 77                                       |a1Aw|

The slave receives the first byte of each SS assertion pulse.  If I
change the protocol to PHASE=1, then this case works as well and I
receive the first four bytes.  I didn't need to set POLARITY to 1,
just phase.

$ dd if=/dev/spidev2.0 count=1 bs=16 | hd
[  261.277000] spidev spi2.0: Start POL 0 PHA 1
$ echo abcd1234ABCDwxyz | dd of=/dev/spidev4.0 bs=4
00000000  61 62 63 64 31 32 33 34  41 42 43 44 77 78 79 7a  |abcd1234ABCDwxyz|

The same thing happens if the bs for the master size is to 1, 2, 8, or
16.  Setting it to another values like 3, 5, 32 doesn't work.  Maybe
the slave ignores extra SS pulses in the middle of a transfer, but
will fail to complete the transfer if it doesn't get SS de-asserted at
the end as expected?

>> another application but was never happy with it.  I imagine this is
>> why no one has ever created a general spi slave framework for Linux.
>
> SPI slave is hard because you can not anticipate the amount of data you will
> receive. You can not anticipate when you will receive those either.

The same can be said for a serial UART or an ethernet MAC, yet those
don't seem to be too hard.  I think the problem is that these devices
usually have things like FIFOs, IRQ trigger levels, pools of buffer
descriptors, etc. that are designed to handle this.  The design of
slave mode in SPI and I2C controllers seems to be more of an
afterthought, with none of the features one would except to deal with
it.  Also, the slave protocol design for things like EEPROMs often
have nanosecond scale latencies that are nearly impossible to achieve
in a general purpose CPU running a multitasking OS.  No one would
design a network protocol that requires an ACK packet within 10 ns of
receiving a REQ, and expect the CPU to handle that.  Yet that can be
expected of a SPI EEPROM.

However, there are applications where one can predict what the master
will do ahead of time and don't have these problems.  In my case I
know what and when the master will do something and can prime the SSP
with a matching slave mode transaction before the master initiates.  I
know I'm not the first person who has needed to do this on Linux.  So
maybe it's worthwhile to add a limited slave support system without
solving the problem of general purpose slave support.
Marek Vasut April 14, 2013, 6:09 p.m. UTC | #5
Dear Trent Piepho,

> On Wed, Mar 20, 2013 at 4:35 PM, Marek Vasut <marex@denx.de> wrote:
> >> On Thu, Aug 23, 2012 at 7:42 PM, Marek Vasut
> >> 
> >> > +              - The DMA has to wait indefinitelly for the arriving
> >> > data.
> >> 
> >> Is there a reason that this must be done?  I'd guess that after the
> >> SSP is told to start a transfer in slave mode it waits for the master
> >> to assert the SSn line and begin the transfer.  If the completion
> >> times out before this happens then it makes sense it wouldn't work.
> >> But does it still not work if the SPI transfer completes before the
> >> completion timeout?  Or is this necessary because in your application
> >> there might be a long wait before the master chooses to initiate a
> >> transfer after you program it on the Linux side?
> > 
> > The problem is the SPI block in slave mode ignores SS being pulled back
> > up.
> 
> I've setup a board with a spi-gpio port looped to a spi-mxs port in
> slave mode so I can test this.  I did not need to change the DMA
> timeout to get it to work.  If the master does not send enough within
> the SSP timeout the slave transaction will timeout.  It does not seem
> to ignore SS, but rather the DMA will not finish until all the
> requested words are received, no matter how many SS pulses that
> happens to take.  For general purpose slave support it would of course
> be nice to be able to end the message when the master drops SS, but
> the SSP either doesn't support this or a different technique is
> needed.

It ignores the SS getting back up, confirmed by FSL. Sad :-(

> I've found that if PHASE = 0, the SSP expects the SS to be pulsed high
> between each word (w/ current driver, word == 8 bits).  This is
> slightly alluded to in the imx28 manual, where the PHASE = 0
> descriptions in sections 17.5.3 and 17.5.5 contain, "... in the case
> of continuous back-to-back transmissions, the SSn signal must be
> pulsed high between each data word[.]"  While for the PHASE = 1
> descriptions in sections 17.5.4 and 17.5.6 it says, "For continuous
> back-to-back transfers, the SSn pin is held low between successive
> data words[.]"

When it comes to slave mode, you can not trust anything you don't research 
yourself, sorry :-(

> One would think this is talking about master mode like the rest of the
> section, but it seems to only apply to slave mode.  In master mode one
> can easily produce a SPI signal without SS pulsed between each word
> with either phase setting.
> 
> Testing with the master TX sending 1 byte per SS asserted period:
> $ echo abcd1234ABCDwxyz | dd of=/dev/spidev4.0 bs=1
> 
> Slave RX receives the first four bytes:
> $ dd if=/dev/spidev2.0 count=1 bs=4 | hd
> [  638.673625] spidev spi2.0: Start POL 0 PHA 0
> 00000000  61 62 63 64                                       |abcd|
> 
> Trying with 4 bytes per SS assertion:
> $ echo abcd1234ABCDwxyz | dd of=/dev/spidev4.0 bs=4
> $ dd if=/dev/spidev2.0 count=1 bs=4 | hd
> 00000000  61 31 41 77                                       |a1Aw|
> 
> The slave receives the first byte of each SS assertion pulse.  If I
> change the protocol to PHASE=1, then this case works as well and I
> receive the first four bytes.  I didn't need to set POLARITY to 1,
> just phase.
> 
> $ dd if=/dev/spidev2.0 count=1 bs=16 | hd
> [  261.277000] spidev spi2.0: Start POL 0 PHA 1
> $ echo abcd1234ABCDwxyz | dd of=/dev/spidev4.0 bs=4
> 00000000  61 62 63 64 31 32 33 34  41 42 43 44 77 78 79 7a 
> |abcd1234ABCDwxyz|
> 
> The same thing happens if the bs for the master size is to 1, 2, 8, or
> 16.  Setting it to another values like 3, 5, 32 doesn't work.  Maybe
> the slave ignores extra SS pulses in the middle of a transfer, but
> will fail to complete the transfer if it doesn't get SS de-asserted at
> the end as expected?

Fabio , can you tell?

> >> another application but was never happy with it.  I imagine this is
> >> why no one has ever created a general spi slave framework for Linux.
> > 
> > SPI slave is hard because you can not anticipate the amount of data you
> > will receive. You can not anticipate when you will receive those either.
> 
> The same can be said for a serial UART or an ethernet MAC, yet those
> don't seem to be too hard.

You have a fixed-size MTU for those. But you can do gigabytes-long continuous 
transfer on generic SPI slave.

> I think the problem is that these devices
> usually have things like FIFOs, IRQ trigger levels, pools of buffer
> descriptors, etc. that are designed to handle this.  The design of
> slave mode in SPI and I2C controllers seems to be more of an
> afterthought, with none of the features one would except to deal with
> it.  Also, the slave protocol design for things like EEPROMs often
> have nanosecond scale latencies that are nearly impossible to achieve
> in a general purpose CPU running a multitasking OS.  No one would
> design a network protocol that requires an ACK packet within 10 ns of
> receiving a REQ, and expect the CPU to handle that.  Yet that can be
> expected of a SPI EEPROM.

That can be handled with the DMA ... more or less. The problem really is that 
you generally can't anticipate the length of transfer.

Note that UART will either start dropping characters if it's buffers blow or 
something, ethernet will start dropping packets. 

> However, there are applications where one can predict what the master
> will do ahead of time and don't have these problems.  In my case I
> know what and when the master will do something and can prime the SSP
> with a matching slave mode transaction before the master initiates.  I
> know I'm not the first person who has needed to do this on Linux.  So
> maybe it's worthwhile to add a limited slave support system without
> solving the problem of general purpose slave support.

You're welcome to try this, many people did before and failed :(

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/spi/mxs-spi.txt b/Documentation/devicetree/bindings/spi/mxs-spi.txt
index e2e1395..16c7287 100644
--- a/Documentation/devicetree/bindings/spi/mxs-spi.txt
+++ b/Documentation/devicetree/bindings/spi/mxs-spi.txt
@@ -9,6 +9,22 @@  Required properties:
 Optional properties:
 - clock-frequency : Input clock frequency to the SPI block in Hz.
 		    Default is 160000000 Hz.
+- fsl,slave-mode : Enable the slave mode operation of the controller.
+		   USE THIS OPTION WITH UTMOST CAUTION, READ ON FOR DETAILS!
+		   The i.MX23/i.MX28 controller can do SPI slave mode and
+		   behaves very closely as it does in master mode, but:
+		   - This is by no mean standard mode of operation, please
+		     use only if you do know what you're doing.
+		   - If the SPI master generates clock at n MHz, the speed
+		     of the SPI slave controller must be at least 4-8 times
+		     faster, this is due to the controller samples the CLK
+		     line multiple times in one clock pulse to be able to
+		     reliably deploy data. Otherwise, no data are received
+		     at all. Successful test was done with:
+		     - Master controller @ 120MHz, master device @ 30MHz
+		     - Slave controller @ 240MHz, slave device @ 120MHz
+		   - The CPOL and CPHA bits must be set.
+		   - The DMA has to wait indefinitelly for the arriving data.
 
 Example:
 
diff --git a/drivers/spi/spi-mxs.c b/drivers/spi/spi-mxs.c
index c965cc6..746359e 100644
--- a/drivers/spi/spi-mxs.c
+++ b/drivers/spi/spi-mxs.c
@@ -61,6 +61,7 @@ 
 struct mxs_spi {
 	struct mxs_ssp		ssp;
 	struct completion	c;
+	bool			slave_mode;
 };
 
 static int mxs_spi_setup_transfer(struct spi_device *dev,
@@ -95,7 +96,8 @@  static int mxs_spi_setup_transfer(struct spi_device *dev,
 		     BF_SSP_CTRL1_WORD_LENGTH
 		     (BV_SSP_CTRL1_WORD_LENGTH__EIGHT_BITS) |
 		     ((dev->mode & SPI_CPOL) ? BM_SSP_CTRL1_POLARITY : 0) |
-		     ((dev->mode & SPI_CPHA) ? BM_SSP_CTRL1_PHASE : 0),
+		     ((dev->mode & SPI_CPHA) ? BM_SSP_CTRL1_PHASE : 0) |
+		     (spi->slave_mode ? BM_SSP_CTRL1_SLAVE_MODE : 0),
 		     ssp->base + HW_SSP_CTRL1(ssp));
 
 	writel(0x0, ssp->base + HW_SSP_CMD0);
@@ -287,16 +289,20 @@  static int mxs_spi_txrx_dma(struct mxs_spi *spi, int cs,
 	dmaengine_submit(desc);
 	dma_async_issue_pending(ssp->dmach);
 
-	ret = wait_for_completion_timeout(&spi->c,
+	if (spi->slave_mode)
+		ret = wait_for_completion_killable(&spi->c);
+	else {
+		ret = wait_for_completion_timeout(&spi->c,
 				msecs_to_jiffies(SSP_TIMEOUT));
 
-	if (!ret) {
-		dev_err(ssp->dev, "DMA transfer timeout\n");
-		ret = -ETIMEDOUT;
-		goto err;
-	}
+		if (!ret) {
+			dev_err(ssp->dev, "DMA transfer timeout\n");
+			ret = -ETIMEDOUT;
+			goto err;
+		}
 
-	ret = 0;
+		ret = 0;
+	}
 
 err:
 	for (--sg_count; sg_count >= 0; sg_count--) {
@@ -410,7 +416,7 @@  static int mxs_spi_transfer_one(struct spi_master *master,
 		 * DMA only: 2.164808 seconds, 473.0KB/s
 		 * Combined: 1.676276 seconds, 610.9KB/s
 		 */
-		if (t->len <= 256) {
+		if ((t->len <= 256) && !spi->slave_mode) {
 			writel(BM_SSP_CTRL1_DMA_ENABLE,
 				ssp->base + HW_SSP_CTRL1(ssp) +
 				STMP_OFFSET_REG_CLR);
@@ -561,6 +567,7 @@  static int __devinit mxs_spi_probe(struct platform_device *pdev)
 	ssp->dma_channel = dma_channel;
 
 	init_completion(&spi->c);
+	spi->slave_mode = of_property_read_bool(np, "fsl,slave-mode");
 
 	ret = devm_request_irq(&pdev->dev, irq_err, mxs_ssp_irq_handler, 0,
 			       DRIVER_NAME, ssp);
diff --git a/include/linux/spi/mxs-spi.h b/include/linux/spi/mxs-spi.h
index 61ae130..6c6ae34 100644
--- a/include/linux/spi/mxs-spi.h
+++ b/include/linux/spi/mxs-spi.h
@@ -94,6 +94,7 @@ 
 #define  BM_SSP_CTRL1_DMA_ENABLE		(1 << 13)
 #define  BM_SSP_CTRL1_PHASE			(1 << 10)
 #define  BM_SSP_CTRL1_POLARITY			(1 << 9)
+#define  BM_SSP_CTRL1_SLAVE_MODE		(1 << 8)
 #define  BP_SSP_CTRL1_WORD_LENGTH		4
 #define  BM_SSP_CTRL1_WORD_LENGTH		(0xf << 4)
 #define  BF_SSP_CTRL1_WORD_LENGTH(v)		\