[PATCH/RFC 0/6] spi: Add slave mode support

[PATCH/RFC 0/6] spi: Add slave mode support

[Geert Uytterhoeven]

	Hi all,

This is a first take at adding support for SPI slave controllers to the
Linux SPI subsystem, including:
  - DT binding updates for SPI slave support,
  - Core support for SPI slave controllers,
  - SPI slave support for the Renesas MSIOF device driver (thanks to
    Nakamura-san for the initial implementation in the R-Car BSP!),
  - Sample SPI slave handlers.

Due to the naure of SPI slave (simultaneous transmit and receive, while
everything runs at the pace of the master), it has hard real-time
requirements: once an SPI transfer is started by the SPI master, a
software SPI slave must have prepared all data to be sent back to the
SPI master.  Hence without additional hardware support, an SPI slave
response can never be a reply to a command being simultaneously
transmitted, and SPI slave replies must be received by the SPI master in
a subsequent SPI transfer.

Examples of possible use cases:
  - Receiving streams of data in fixed-size messages (e.g. from a
    tuner),
  - Receiving and transmitting fixed-size messages of data (e.g. network
    frames),
  - Sending commands, and querying for responses,
  - ...

Originally I wanted to implement a simple SPI slave handler that could
interface with an existing Linux SPI slave driver, cfr. Wolfram Sang's
I2C slave mode EEPROM simulator for the i2c subsystem.
Unfortunately I couldn't find any existing driver using an SPI slave
protocol that fulfills the above requirements. The Nordic Semiconductor
nRF8001 BLE controller seems to use a suitable protocol, but I couldn't
find a Linux driver for it.  Hence I created two sample SPI slave
protocols and drivers myself:
  1. "spi-slave-time" responds with the time of reception of the
     last SPI message, which can be used by an external microcontroller
     as a dead man's switch.
  2. "spi-slave-system-control" allows remote control of system reboot,
     power off, halt, and suspend.

For some use cases, using spidev from user space may be a more appropriate
solution than an in-kernel SPI protocol handler.

>From the point of view of an SPI slave protocol handler, an SPI slave
controller looks exactly like an ordinary SPI master controller. Hence
"struct spi_master" may have become a misnomer. For now I didn't bother
fixing that.  Should we rename spi_master (and the spi_*master()
functions) to spi_controller? Or create wrappers/defines with "slave"
in their name?

For now, the MSIOF SPI slave driver only supports the transmission of
messages with a size that is known in advance (the hardware can provide
an interrupt when CS is deasserted before, though).
I.e. when the SPI master sends a shorter message, the slave won't
receive it.  When the SPI master sends a longer message, the slave will
receive the first part, and the rest will remain in the FIFO.

Handshaking (5-pin SPI, RDY-signal) is optional. An RDY-signal may be
used for one or both of:
  1. The SPI slave asserts RDY when it has data available, and wants to
     be queried by the SPI master.
       -> This can be handled on top, in the SPI slave protocol handler,
	  using a GPIO.
  2. After the SPI master has asserted CS, the SPI slave asserts RDY
     when it is ready to accept the transfer.
       -> This may need hardware support in the SPI slave controller,
	  or dynamic GPIO vs. CS pinmuxing.

This patch series applies to both v4.7-rc1 and next-20160622, with the
following 2 patches applied:
      1. [PATCH] spi: Improve DT binding documentation,
      2. [PATCH] spi: sh-msiof: Remove sh_msiof_spi_priv.chipdata.
For your convenience, I've pushed this series and its dependencies to
the topic/spi-slave-v1 branch of the git repository at
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git

For testing, device tree overlays enabling SPI master and slave
controllers on an expansion I/O connector on r8a7791/koelsch are
available in the topic/renesas-overlays branch of my renesas-drivers git
repository.  Please see http://elinux.org/R-Car/DT-Overlays for more
information about using these overlays.

Test wiring on r8a7791/koelsch, between MSIOF1 and MSIOF2 on EXIO
connector A:
   - Connect pin 48 (MSIOF1 CS#) to pin 63 (MSIOF2 CS#),
   - Connect pin 46 (MSIOF1 SCK) to pin 61 (MSIOF2 SCK),
   - Connect pin 54 (MSIOF1 TX/MOSI) to pin 70 (MSIOF2 RX/MOSI),
   - Connect pin 56 (MSIOF1 RX/MISO) to pin 68 (MSIOF2 TX/MISO).

Example 1:

    # overlay add a-msiof1-spidev
    # overlay add a-msiof2-slave-time
    # spidev_test -D /dev/spidev2.0 -p dummy-8B
    spi mode: 0x0
    bits per word: 8
    max speed: 500000 Hz (500 KHz)
    RX | 00 00 04 6D 00 09 5B BB __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ __  | ...m..[�
               ^^^^^    ^^^^^^^^
	       seconds  microseconds

Example 2:

    # overlay add a-msiof1-spidev
    # overlay add a-msiof2-slave-system-control
    # reboot='\x7c\x50'
    # poweroff='\x71\x3f'
    # halt='\x38\x76'
    # suspend='\x1b\x1b'
    # spidev_test -D /dev/spidev2.0 -p $suspend # or $reboot, $poweroff, $halt

Thanks for your comments!

Geert Uytterhoeven (5):
  [RFC] spi: Document DT bindings for SPI controllers in slave mode
  [RFC] spi: core: Add support for registering SPI slave controllers
  [RFC] spi: slave: Add SPI slave handler reporting boot up time
  [RFC] spi: slave: Add SPI slave handler controlling system state
  [RFC] spi: spidev: Allow direct references in DT from SPI slave
    controllers

Hisashi Nakamura (1):
  [RFC] spi: sh-msiof: Add slave mode support

 Documentation/devicetree/bindings/spi/spi-bus.txt |  31 +++--
 drivers/spi/Kconfig                               |  26 ++++-
 drivers/spi/Makefile                              |   4 +
 drivers/spi/spi-sh-msiof.c                        |  52 +++++++--
 drivers/spi/spi-slave-system-control.c            | 134 ++++++++++++++++++++++
 drivers/spi/spi-slave-time.c                      | 103 +++++++++++++++++
 drivers/spi/spi.c                                 |  39 ++++---
 drivers/spi/spidev.c                              |   3 +-
 include/linux/spi/sh_msiof.h                      |   6 +
 include/linux/spi/spi.h                           |   5 +-
 10 files changed, 360 insertions(+), 43 deletions(-)
 create mode 100644 drivers/spi/spi-slave-system-control.c
 create mode 100644 drivers/spi/spi-slave-time.c

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH/RFC 1/6] spi: Document DT bindings for SPI controllers in slave mode

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/spi/spi-bus.txt | 31 ++++++++++++++---------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
index 17822860cb98c34d..bcd024e0bb9e0009 100644
--- a/Documentation/devicetree/bindings/spi/spi-bus.txt
+++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -1,17 +1,23 @@
 SPI (Serial Peripheral Interface) busses
 
-SPI busses can be described with a node for the SPI master device
-and a set of child nodes for each SPI slave on the bus.  For this
-discussion, it is assumed that the system's SPI controller is in
-SPI master mode.  This binding does not describe SPI controllers
-in slave mode.
+SPI busses can be described with a node for the SPI controller device
+and a set of child nodes for each SPI slave on the bus.  The system's SPI
+controller may be described for use in SPI master mode or in SPI slave mode,
+but not for both at the same time.
 
-The SPI master node requires the following properties:
+The SPI controller node requires the following properties:
+- compatible      - name of SPI bus controller following generic names
+		recommended practice.
+
+In master mode, the SPI controller node requires the following additional
+properties:
 - #address-cells  - number of cells required to define a chip select
 		address on the SPI bus.
 - #size-cells     - should be zero.
-- compatible      - name of SPI bus controller following generic names
-		recommended practice.
+
+In slave node, the SPI controller node requires the presence of a child node
+named "slave", further following the practices for SPI slave nodes below.
+
 No other properties are required in the SPI bus node.  It is assumed
 that a driver for an SPI bus device will understand that it is an SPI bus.
 However, the binding does not attempt to define the specific method for
@@ -21,7 +27,7 @@ assumption that board specific platform code will be used to manage
 chip selects.  Individual drivers can define additional properties to
 support describing the chip select layout.
 
-Optional properties:
+Optional properties (master mode only):
 - cs-gpios	  - gpios chip select.
 - num-cs	  - total number of chipselects.
 
@@ -41,12 +47,13 @@ cs1 : native
 cs2 : &gpio1 1 0
 cs3 : &gpio1 2 0
 
-SPI slave nodes must be children of the SPI master node and can
+SPI slave nodes must be children of the SPI controller node and can
 contain the following properties.
-- reg             - (required) chip select address of device.
+- reg             - (required, master mode only) chip select address of device.
 - compatible      - (required) name of SPI device following generic names
 		recommended practice.
-- spi-max-frequency - (required) Maximum SPI clocking speed of device in Hz.
+- spi-max-frequency - (required, master mode only) Maximum SPI clocking speed
+		of device in Hz.
 - spi-cpol        - (optional) Empty property indicating device requires
 		inverse clock polarity (CPOL) mode.
 - spi-cpha        - (optional) Empty property indicating device requires
-- 
1.9.1

Re: [PATCH/RFC 1/6] spi: Document DT bindings for SPI controllers in slave mode

[Rob Herring]

On Wed, Jun 22, 2016 at 03:42:04PM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  Documentation/devicetree/bindings/spi/spi-bus.txt | 31 ++++++++++++++---------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
> index 17822860cb98c34d..bcd024e0bb9e0009 100644
> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> @@ -1,17 +1,23 @@
>  SPI (Serial Peripheral Interface) busses
>  
> -SPI busses can be described with a node for the SPI master device
> -and a set of child nodes for each SPI slave on the bus.  For this
> -discussion, it is assumed that the system's SPI controller is in
> -SPI master mode.  This binding does not describe SPI controllers
> -in slave mode.
> +SPI busses can be described with a node for the SPI controller device
> +and a set of child nodes for each SPI slave on the bus.  The system's SPI
> +controller may be described for use in SPI master mode or in SPI slave mode,
> +but not for both at the same time.
>  
> -The SPI master node requires the following properties:
> +The SPI controller node requires the following properties:
> +- compatible      - name of SPI bus controller following generic names
> +		recommended practice.
> +
> +In master mode, the SPI controller node requires the following additional
> +properties:
>  - #address-cells  - number of cells required to define a chip select
>  		address on the SPI bus.
>  - #size-cells     - should be zero.
> -- compatible      - name of SPI bus controller following generic names
> -		recommended practice.
> +
> +In slave node, the SPI controller node requires the presence of a child node
> +named "slave", further following the practices for SPI slave nodes below.
> +

I wouldn't create a child node. Just add a property for slave mode and 
put the timing mode properties in controller node.

Rob

Re: [PATCH/RFC 1/6] spi: Document DT bindings for SPI controllers in slave mode

[Geert Uytterhoeven]

Hi Rob,

On Fri, Jun 24, 2016 at 7:06 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Jun 22, 2016 at 03:42:04PM +0200, Geert Uytterhoeven wrote:
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  Documentation/devicetree/bindings/spi/spi-bus.txt | 31 ++++++++++++++---------
>>  1 file changed, 19 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
>> index 17822860cb98c34d..bcd024e0bb9e0009 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
>> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
>> @@ -1,17 +1,23 @@
>>  SPI (Serial Peripheral Interface) busses
>>
>> -SPI busses can be described with a node for the SPI master device
>> -and a set of child nodes for each SPI slave on the bus.  For this
>> -discussion, it is assumed that the system's SPI controller is in
>> -SPI master mode.  This binding does not describe SPI controllers
>> -in slave mode.
>> +SPI busses can be described with a node for the SPI controller device
>> +and a set of child nodes for each SPI slave on the bus.  The system's SPI
>> +controller may be described for use in SPI master mode or in SPI slave mode,
>> +but not for both at the same time.
>>
>> -The SPI master node requires the following properties:
>> +The SPI controller node requires the following properties:
>> +- compatible      - name of SPI bus controller following generic names
>> +             recommended practice.
>> +
>> +In master mode, the SPI controller node requires the following additional
>> +properties:
>>  - #address-cells  - number of cells required to define a chip select
>>               address on the SPI bus.
>>  - #size-cells     - should be zero.
>> -- compatible      - name of SPI bus controller following generic names
>> -             recommended practice.
>> +
>> +In slave node, the SPI controller node requires the presence of a child node
>> +named "slave", further following the practices for SPI slave nodes below.
>> +
>
> I wouldn't create a child node. Just add a property for slave mode and
> put the timing mode properties in controller node.

Originally I wanted to just add a "slave" property, too.
However, not having a child node means you cannot bind a driver for an SPI
slave handler from DT, as the existing compatible property is meant for the SPI
slave controller. That's why I went with the child naming rule instead.

Not being able to bind a driver from DT could be a good thing, as this could
be considered software configuration, not hardware description.

E.g. i2c slave requires binding slave handlers manually, by creating a new
device on the bus, using the existing mechanism for adding new devices to an
i2c bus, cfr. Documentation/i2c/slave-interface:

        echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-1/new_device

Alternatively, we could always create an spidev interface for SPI slave
controllers, but that precludes (or makes it more difficult) binding another
driver if that is available.

This could definitely use some more discussion or feedback... Thoughts?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH/RFC 2/6] spi: core: Add support for registering SPI slave controllers

[Geert Uytterhoeven]

Add support for registering SPI slave controllers using the existing SPI
master framework:
  - SPI slave controllers must set the SPI_MASTER_IS_SLAVE flag in
    spi_master.flags,
  - The "cs-gpios" property is ignored,
  - The bus is described in DT as having a single slave node, "reg" and
    "spi-max-frequency" properties are ignored.

>From the point of view of an SPI slave protocol handler, an SPI slave
controller looks exactly like an ordinary SPI master controller.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Should we rename spi_master to spi_controller?
---
 drivers/spi/Kconfig     | 14 +++++++++++++-
 drivers/spi/Makefile    |  2 ++
 drivers/spi/spi.c       | 39 ++++++++++++++++++++++++---------------
 include/linux/spi/spi.h |  5 +++--
 4 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 4b931ec8d90b610f..00e990d91a715c0e 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -736,6 +736,18 @@ config SPI_TLE62X0
 
 endif # SPI_MASTER
 
-# (slave support would go here)
+#
+# SLAVE side ... listening to other SPI masters
+#
+
+config SPI_SLAVE
+	bool "SPI slave protocol handlers"
+	help
+	  If your system has a slave-capable SPI controller, you can enable
+	  slave protocol handlers.
+
+if SPI_SLAVE
+
+endif # SPI_SLAVE
 
 endif # SPI
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 3c74d003535bb9fb..71d02939080fb747 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -97,3 +97,5 @@ obj-$(CONFIG_SPI_XILINX)		+= spi-xilinx.o
 obj-$(CONFIG_SPI_XLP)			+= spi-xlp.o
 obj-$(CONFIG_SPI_XTENSA_XTFPGA)		+= spi-xtensa-xtfpga.o
 obj-$(CONFIG_SPI_ZYNQMP_GQSPI)		+= spi-zynqmp-gqspi.o
+
+# SPI slave protocol handlers
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 77e6e45951f4c5e1..6ea32255d6d9e780 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1462,6 +1462,7 @@ err_init_queue:
 static struct spi_device *
 of_register_spi_device(struct spi_master *master, struct device_node *nc)
 {
+	bool slave = master->flags & SPI_MASTER_IS_SLAVE;
 	struct spi_device *spi;
 	int rc;
 	u32 value;
@@ -1485,13 +1486,16 @@ of_register_spi_device(struct spi_master *master, struct device_node *nc)
 	}
 
 	/* Device address */
-	rc = of_property_read_u32(nc, "reg", &value);
-	if (rc) {
-		dev_err(&master->dev, "%s has no valid 'reg' property (%d)\n",
-			nc->full_name, rc);
-		goto err_out;
+	if (!slave) {
+		rc = of_property_read_u32(nc, "reg", &value);
+		if (rc) {
+			dev_err(&master->dev,
+				"%s has no valid 'reg' property (%d)\n",
+				nc->full_name, rc);
+			goto err_out;
+		}
+		spi->chip_select = value;
 	}
-	spi->chip_select = value;
 
 	/* Mode (clock phase/polarity/etc.) */
 	if (of_find_property(nc, "spi-cpha", NULL))
@@ -1543,13 +1547,16 @@ of_register_spi_device(struct spi_master *master, struct device_node *nc)
 	}
 
 	/* Device speed */
-	rc = of_property_read_u32(nc, "spi-max-frequency", &value);
-	if (rc) {
-		dev_err(&master->dev, "%s has no valid 'spi-max-frequency' property (%d)\n",
-			nc->full_name, rc);
-		goto err_out;
+	if (!slave) {
+		rc = of_property_read_u32(nc, "spi-max-frequency", &value);
+		if (rc) {
+			dev_err(&master->dev,
+				"%s has no valid 'spi-max-frequency' property (%d)\n",
+				nc->full_name, rc);
+			goto err_out;
+		}
+		spi->max_speed_hz = value;
 	}
-	spi->max_speed_hz = value;
 
 	/* Store a pointer to the node in the device structure */
 	of_node_get(nc);
@@ -1779,7 +1786,7 @@ static int of_spi_register_master(struct spi_master *master)
 	int nb, i, *cs;
 	struct device_node *np = master->dev.of_node;
 
-	if (!np)
+	if (!np || master->flags & SPI_MASTER_IS_SLAVE)
 		return 0;
 
 	nb = of_gpio_named_count(np, "cs-gpios");
@@ -1885,8 +1892,10 @@ int spi_register_master(struct spi_master *master)
 	status = device_add(&master->dev);
 	if (status < 0)
 		goto done;
-	dev_dbg(dev, "registered master %s%s\n", dev_name(&master->dev),
-			dynamic ? " (dynamic)" : "");
+	dev_dbg(dev, "registered %s %s%s\n",
+			master->flags & SPI_MASTER_IS_SLAVE ? "slave"
+							    : "master",
+			dev_name(&master->dev), dynamic ? " (dynamic)" : "");
 
 	/* If we're using a queued driver, start the queue */
 	if (master->transfer)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 1f03483f61e5714b..0fbebaa7d93990a4 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -28,8 +28,8 @@ struct spi_transfer;
 struct spi_flash_read_message;
 
 /*
- * INTERFACES between SPI master-side drivers and SPI infrastructure.
- * (There's no SPI slave support for Linux yet...)
+ * INTERFACES between SPI master-side drivers and SPI slave protocol handers,
+ * and SPI infrastructure.
  */
 extern struct bus_type spi_bus_type;
 
@@ -439,6 +439,7 @@ struct spi_master {
 #define SPI_MASTER_NO_TX	BIT(2)		/* can't do buffer write */
 #define SPI_MASTER_MUST_RX      BIT(3)		/* requires rx */
 #define SPI_MASTER_MUST_TX      BIT(4)		/* requires tx */
+#define SPI_MASTER_IS_SLAVE     BIT(5)		/* SPI slave controller */
 
 	/*
 	 * on some hardware transfer size may be constrained
-- 
1.9.1

Re: [PATCH/RFC 2/6] spi: core: Add support for registering SPI slave controllers

[Mark Brown]

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

On Wed, Jun 22, 2016 at 03:42:05PM +0200, Geert Uytterhoeven wrote:
> Add support for registering SPI slave controllers using the existing SPI
> master framework:
>   - SPI slave controllers must set the SPI_MASTER_IS_SLAVE flag in
>     spi_master.flags,
>   - The "cs-gpios" property is ignored,
>   - The bus is described in DT as having a single slave node, "reg" and
>     "spi-max-frequency" properties are ignored.
> 
> From the point of view of an SPI slave protocol handler, an SPI slave
> controller looks exactly like an ordinary SPI master controller.

I think this needs a *bit* more fleshing out around cancellation of
transfers, the inability to remove any of the modules due to them being
blocked in SPI calls.  Probably just an API call that allows us to
inject a timeout/cancellation but I think it does need to be there and
used before we start getting bad practice propagating around.

I'm also wondering about supporting varible length transfers but that's
going to be rather controller specific I fear and I'm not sure there's
much demand.

Otherwise the basic idea looks OK.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH/RFC 3/6] spi: sh-msiof: Add slave mode support

[Geert Uytterhoeven]

From: Hisashi Nakamura <hisashi.nakamura.ak@renesas.com>

Add slave mode support to the MSIOF driver.

For now this only supports the transmission of messages with a size
that is known in advance.

Signed-off-by: Hisashi Nakamura <hisashi.nakamura.ak@renesas.com>
Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
[geert: Timeout handling cleanup, spi core integration, rewording]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/spi/spi-sh-msiof.c   | 52 ++++++++++++++++++++++++++++++++++----------
 include/linux/spi/sh_msiof.h |  6 +++++
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 0f83ad1d5a5858dd..afde3fe12bce844f 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2009 Magnus Damm
  * Copyright (C) 2014 Glider bvba
+ * Copyright (C) 2014 Renesas Electronics Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -33,7 +34,6 @@
 
 #include <asm/unaligned.h>
 
-
 struct sh_msiof_chipdata {
 	u16 tx_fifo_size;
 	u16 rx_fifo_size;
@@ -334,7 +334,10 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p,
 	tmp |= !cs_high << MDR1_SYNCAC_SHIFT;
 	tmp |= lsb_first << MDR1_BITLSB_SHIFT;
 	tmp |= sh_msiof_spi_get_dtdl_and_syncdl(p);
-	sh_msiof_write(p, TMDR1, tmp | MDR1_TRMD | TMDR1_PCON);
+	if (p->master->flags & SPI_MASTER_IS_SLAVE)
+		sh_msiof_write(p, TMDR1, tmp | TMDR1_PCON);
+	else
+		sh_msiof_write(p, TMDR1, tmp | MDR1_TRMD | TMDR1_PCON);
 	if (p->master->flags & SPI_MASTER_MUST_TX) {
 		/* These bits are reserved if RX needs TX */
 		tmp &= ~0x0000ffff;
@@ -561,17 +564,19 @@ static int sh_msiof_prepare_message(struct spi_master *master,
 
 static int sh_msiof_spi_start(struct sh_msiof_spi_priv *p, void *rx_buf)
 {
-	int ret;
+	bool slave = p->master->flags & SPI_MASTER_IS_SLAVE;
+	int ret = 0;
 
 	/* setup clock and rx/tx signals */
-	ret = sh_msiof_modify_ctr_wait(p, 0, CTR_TSCKE);
+	if (!slave)
+		ret = sh_msiof_modify_ctr_wait(p, 0, CTR_TSCKE);
 	if (rx_buf && !ret)
 		ret = sh_msiof_modify_ctr_wait(p, 0, CTR_RXE);
 	if (!ret)
 		ret = sh_msiof_modify_ctr_wait(p, 0, CTR_TXE);
 
 	/* start by setting frame bit */
-	if (!ret)
+	if (!ret && !slave)
 		ret = sh_msiof_modify_ctr_wait(p, 0, CTR_TFSE);
 
 	return ret;
@@ -579,15 +584,17 @@ static int sh_msiof_spi_start(struct sh_msiof_spi_priv *p, void *rx_buf)
 
 static int sh_msiof_spi_stop(struct sh_msiof_spi_priv *p, void *rx_buf)
 {
-	int ret;
+	bool slave = p->master->flags & SPI_MASTER_IS_SLAVE;
+	int ret = 0;
 
 	/* shut down frame, rx/tx and clock signals */
-	ret = sh_msiof_modify_ctr_wait(p, CTR_TFSE, 0);
+	if (!slave)
+		ret = sh_msiof_modify_ctr_wait(p, CTR_TFSE, 0);
 	if (!ret)
 		ret = sh_msiof_modify_ctr_wait(p, CTR_TXE, 0);
 	if (rx_buf && !ret)
 		ret = sh_msiof_modify_ctr_wait(p, CTR_RXE, 0);
-	if (!ret)
+	if (!ret && !slave)
 		ret = sh_msiof_modify_ctr_wait(p, CTR_TSCKE, 0);
 
 	return ret;
@@ -633,7 +640,11 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
 	}
 
 	/* wait for tx fifo to be emptied / rx fifo to be filled */
-	if (!wait_for_completion_timeout(&p->done, HZ)) {
+	if (p->master->flags & SPI_MASTER_IS_SLAVE)
+		ret = !wait_for_completion_interruptible(&p->done);
+	else
+		ret = wait_for_completion_timeout(&p->done, HZ);
+	if (!ret) {
 		dev_err(&p->pdev->dev, "PIO timeout\n");
 		ret = -ETIMEDOUT;
 		goto stop_reset;
@@ -743,7 +754,11 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
 	}
 
 	/* wait for tx fifo to be emptied / rx fifo to be filled */
-	if (!wait_for_completion_timeout(&p->done, HZ)) {
+	if (p->master->flags & SPI_MASTER_IS_SLAVE)
+		ret = !wait_for_completion_interruptible(&p->done);
+	else
+		ret = wait_for_completion_timeout(&p->done, HZ);
+	if (!ret) {
 		dev_err(&p->pdev->dev, "DMA timeout\n");
 		ret = -ETIMEDOUT;
 		goto stop_reset;
@@ -840,7 +855,8 @@ static int sh_msiof_transfer_one(struct spi_master *master,
 	int ret;
 
 	/* setup clocks (clock already enabled in chipselect()) */
-	sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
+	if (!(p->master->flags & SPI_MASTER_IS_SLAVE))
+		sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
 
 	while (master->dma_tx && len > 15) {
 		/*
@@ -986,14 +1002,24 @@ static struct sh_msiof_spi_info *sh_msiof_spi_parse_dt(struct device *dev)
 {
 	struct sh_msiof_spi_info *info;
 	struct device_node *np = dev->of_node;
+	struct device_node *slave;
 	u32 num_cs = 1;
 
 	info = devm_kzalloc(dev, sizeof(struct sh_msiof_spi_info), GFP_KERNEL);
 	if (!info)
 		return NULL;
 
+	slave = of_get_child_by_name(np, "slave");
+	if (slave) {
+		info->mode = MSIOF_SPI_SLAVE;
+		of_node_put(slave);
+	} else {
+		info->mode = MSIOF_SPI_MASTER;
+	}
+
 	/* Parse the MSIOF properties */
-	of_property_read_u32(np, "num-cs", &num_cs);
+	if (info->mode == MSIOF_SPI_MASTER)
+		of_property_read_u32(np, "num-cs", &num_cs);
 	of_property_read_u32(np, "renesas,tx-fifo-size",
 					&info->tx_fifo_override);
 	of_property_read_u32(np, "renesas,rx-fifo-size",
@@ -1228,6 +1254,8 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 	master->mode_bits |= SPI_LSB_FIRST | SPI_3WIRE;
 	master->flags = chipdata->master_flags;
+	if (p->info->mode == MSIOF_SPI_SLAVE)
+		master->flags |= SPI_MASTER_IS_SLAVE;
 	master->bus_num = pdev->id;
 	master->dev.of_node = pdev->dev.of_node;
 	master->num_chipselect = p->info->num_chipselect;
diff --git a/include/linux/spi/sh_msiof.h b/include/linux/spi/sh_msiof.h
index b087a85f5f72a351..f74b581f242f8c43 100644
--- a/include/linux/spi/sh_msiof.h
+++ b/include/linux/spi/sh_msiof.h
@@ -1,10 +1,16 @@
 #ifndef __SPI_SH_MSIOF_H__
 #define __SPI_SH_MSIOF_H__
 
+enum {
+	MSIOF_SPI_MASTER,
+	MSIOF_SPI_SLAVE,
+};
+
 struct sh_msiof_spi_info {
 	int tx_fifo_override;
 	int rx_fifo_override;
 	u16 num_chipselect;
+	int mode;
 	unsigned int dma_tx_id;
 	unsigned int dma_rx_id;
 	u32 dtdl;
-- 
1.9.1

[PATCH/RFC 4/6] spi: slave: Add SPI slave handler reporting boot up time

[Geert Uytterhoeven]

Add an SPI slave handler responding with the time of reception of the
last SPI message.

This can be used by an external microcontroller as a dead man's switch.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
FIXME kthread_stop() hangs, as spi_write() is blocked on a completion
---
 drivers/spi/Kconfig          |   6 +++
 drivers/spi/Makefile         |   1 +
 drivers/spi/spi-slave-time.c | 103 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 drivers/spi/spi-slave-time.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 00e990d91a715c0e..e9b2f48574464949 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -748,6 +748,12 @@ config SPI_SLAVE
 
 if SPI_SLAVE
 
+config SPI_SLAVE_TIME
+	tristate "SPI slave handler reporting boot up time"
+	help
+	  SPI slave handler responding with the time of reception of the last
+	  SPI message.
+
 endif # SPI_SLAVE
 
 endif # SPI
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 71d02939080fb747..dc67b8f137e2ced0 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -99,3 +99,4 @@ obj-$(CONFIG_SPI_XTENSA_XTFPGA)		+= spi-xtensa-xtfpga.o
 obj-$(CONFIG_SPI_ZYNQMP_GQSPI)		+= spi-zynqmp-gqspi.o
 
 # SPI slave protocol handlers
+obj-$(CONFIG_SPI_SLAVE_TIME)		+= spi-slave-time.o
diff --git a/drivers/spi/spi-slave-time.c b/drivers/spi/spi-slave-time.c
new file mode 100644
index 0000000000000000..3d606af318024573
--- /dev/null
+++ b/drivers/spi/spi-slave-time.c
@@ -0,0 +1,103 @@
+/*
+ * SPI slave handler reporting boot up time
+ *
+ * This SPI slave handler sends the time of reception of the last SPI message
+ * as two 32-bit unsigned integers in binary format and in network byte order,
+ * representing the number of seconds and fractional seconds (in microseconds)
+ * since boot up.
+ *
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/spi/spi.h>
+
+
+struct spi_slave_time_priv {
+	struct spi_device *spi;
+	struct task_struct *thread;
+};
+
+static int spi_slave_time_send(struct spi_device *spi)
+{
+	__be32 msg[2];
+	u32 rem_ns;
+	u64 ts;
+
+	ts = local_clock();
+	rem_ns = do_div(ts, 1000000000) / 1000;
+
+	msg[0] = cpu_to_be32(ts);
+	msg[1] = cpu_to_be32(rem_ns);
+
+	return spi_write(spi, &msg, sizeof(msg));
+}
+
+static int spi_slave_time_thread(void *data)
+{
+	struct spi_slave_time_priv *priv = data;
+	int error;
+
+	while (!kthread_should_stop()) {
+		error = spi_slave_time_send(priv->spi);
+		if (error)
+			pr_err("%s: SPI transfer failed %d\n", __func__, error);
+	}
+
+	return 0;
+}
+
+static int spi_slave_time_probe(struct spi_device *spi)
+{
+	struct spi_slave_time_priv *priv;
+	int ret;
+
+	/*
+	 * bits_per_word cannot be configured in platform data
+	 */
+	spi->bits_per_word = 8;
+
+	ret = spi_setup(spi);
+	if (ret < 0)
+		return ret;
+
+	priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->spi = spi;
+	priv->thread = kthread_run(spi_slave_time_thread, priv,
+				   "spi-slave-time/%s", dev_name(&spi->dev));
+	if (IS_ERR(priv->thread))
+		return PTR_ERR(priv->thread);
+
+	spi_set_drvdata(spi, priv);
+	return 0;
+}
+
+static int spi_slave_time_remove(struct spi_device *spi)
+{
+	struct spi_slave_time_priv *priv = spi_get_drvdata(spi);
+
+	/* FIXME Doesn't work, as spi_write() is blocked on a completion */
+	kthread_stop(priv->thread);
+	return 0;
+}
+
+static struct spi_driver spi_slave_time_driver = {
+	.driver = {
+		.name	= "spi-slave-time",
+	},
+	.probe		= spi_slave_time_probe,
+	.remove		= spi_slave_time_remove,
+};
+module_spi_driver(spi_slave_time_driver);
+
+MODULE_AUTHOR("Geert Uytterhoeven <geert+renesas@glider.be>");
+MODULE_DESCRIPTION("SPI slave reporting boot up time");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH/RFC 4/6] spi: slave: Add SPI slave handler reporting boot up time

[Mark Brown]

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

On Wed, Jun 22, 2016 at 03:42:07PM +0200, Geert Uytterhoeven wrote:
> Add an SPI slave handler responding with the time of reception of the
> last SPI message.
> 
> This can be used by an external microcontroller as a dead man's switch.

The subject says boot up time, this says time of reception of the last
message.  Which is it?

> +static int spi_slave_time_send(struct spi_device *spi)
> +{
> +	__be32 msg[2];
> +	u32 rem_ns;
> +	u64 ts;
> +
> +	ts = local_clock();
> +	rem_ns = do_div(ts, 1000000000) / 1000;
> +
> +	msg[0] = cpu_to_be32(ts);
> +	msg[1] = cpu_to_be32(rem_ns);
> +
> +	return spi_write(spi, &msg, sizeof(msg));
> +}

Looks like uptime which is a third thing.

> +static int spi_slave_time_remove(struct spi_device *spi)
> +{
> +	struct spi_slave_time_priv *priv = spi_get_drvdata(spi);
> +
> +	/* FIXME Doesn't work, as spi_write() is blocked on a completion */
> +	kthread_stop(priv->thread);

spi_async()?  Still no cancellation on the actual operation but it
pushes it more inside the framework.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH/RFC 5/6] spi: slave: Add SPI slave handler controlling system state

[Geert Uytterhoeven]

Add an SPI slave handler to allow remote control of system reboot, power
off, halt, and suspend.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
FIXME kthread_stop() hangs, as spi_write() is blocked on a completion
---
 drivers/spi/Kconfig                    |   6 ++
 drivers/spi/Makefile                   |   1 +
 drivers/spi/spi-slave-system-control.c | 134 +++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+)
 create mode 100644 drivers/spi/spi-slave-system-control.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index e9b2f48574464949..9657415125ccb631 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -754,6 +754,12 @@ config SPI_SLAVE_TIME
 	  SPI slave handler responding with the time of reception of the last
 	  SPI message.
 
+config SPI_SLAVE_SYSTEM_CONTROL
+	tristate "SPI slave handler controlling system state"
+	help
+	  SPI slave handler to allow remote control of system reboot, power
+	  off, halt, and suspend.
+
 endif # SPI_SLAVE
 
 endif # SPI
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index dc67b8f137e2ced0..3b0487a9da91dfa8 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -100,3 +100,4 @@ obj-$(CONFIG_SPI_ZYNQMP_GQSPI)		+= spi-zynqmp-gqspi.o
 
 # SPI slave protocol handlers
 obj-$(CONFIG_SPI_SLAVE_TIME)		+= spi-slave-time.o
+obj-$(CONFIG_SPI_SLAVE_SYSTEM_CONTROL)	+= spi-slave-system-control.o
diff --git a/drivers/spi/spi-slave-system-control.c b/drivers/spi/spi-slave-system-control.c
new file mode 100644
index 0000000000000000..350711dec1281675
--- /dev/null
+++ b/drivers/spi/spi-slave-system-control.c
@@ -0,0 +1,134 @@
+/*
+ * SPI slave handler controlling system state
+ *
+ * This SPI slave handler allows remote control of system reboot, power off,
+ * halt, and suspend.
+ *
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/reboot.h>
+#include <linux/sched.h>
+#include <linux/suspend.h>
+#include <linux/spi/spi.h>
+
+/*
+ * The numbers are chosen to display something human-readable on two 7-segment
+ * displays connected to two 74HC595 shift registers
+ */
+#define CMD_REBOOT	0x507c	/* rb */
+#define CMD_POWEROFF	0x3f71	/* OF */
+#define CMD_HALT	0x7638	/* HL */
+#define CMD_SUSPEND	0x1b1b	/* ZZ */
+
+struct spi_slave_system_control_priv {
+	struct spi_device *spi;
+	struct task_struct *thread;
+};
+
+static int spi_slave_system_control_send(struct spi_device *spi)
+{
+	__le16 cmd;
+	int error;
+
+	error = spi_read(spi, &cmd, sizeof(cmd));
+	if (error)
+		return error;
+
+	switch (le16_to_cpu(cmd)) {
+	case CMD_REBOOT:
+		pr_info("Rebooting system...\n");
+		kernel_restart(NULL);
+
+	case CMD_POWEROFF:
+		pr_info("Powering off system...\n");
+		kernel_power_off();
+		break;
+
+	case CMD_HALT:
+		pr_info("Halting system...\n");
+		kernel_halt();
+		break;
+
+	case CMD_SUSPEND:
+		pr_info("Suspending system...\n");
+		pm_suspend(PM_SUSPEND_MEM);
+		break;
+
+	default:
+		pr_warn("%s: Unknown command 0x%x\n", __func__, cmd);
+		break;
+	}
+	return 0;
+}
+
+static int spi_slave_system_control_thread(void *data)
+{
+	struct spi_slave_system_control_priv *priv = data;
+	int error;
+
+	while (!kthread_should_stop()) {
+		error = spi_slave_system_control_send(priv->spi);
+		if (error)
+			pr_err("%s: SPI transfer failed %d\n", __func__, error);
+	}
+
+	return 0;
+}
+
+static int spi_slave_system_control_probe(struct spi_device *spi)
+{
+	struct spi_slave_system_control_priv *priv;
+	int ret;
+
+	/*
+	 * bits_per_word cannot be configured in platform data
+	 */
+	spi->bits_per_word = 8;
+
+	ret = spi_setup(spi);
+	if (ret < 0)
+		return ret;
+
+	priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->spi = spi;
+	priv->thread = kthread_run(spi_slave_system_control_thread, priv,
+				   "spi-slave-system-control/%s",
+				   dev_name(&spi->dev));
+	if (IS_ERR(priv->thread))
+		return PTR_ERR(priv->thread);
+
+	spi_set_drvdata(spi, priv);
+
+	return 0;
+}
+
+static int spi_slave_system_control_remove(struct spi_device *spi)
+{
+	struct spi_slave_system_control_priv *priv = spi_get_drvdata(spi);
+
+	/* FIXME Doesn't work, as spi_read() is blocked on a completion */
+	kthread_stop(priv->thread);
+	return 0;
+}
+
+static struct spi_driver spi_slave_system_control_driver = {
+	.driver = {
+		.name	= "spi-slave-system-control",
+	},
+	.probe		= spi_slave_system_control_probe,
+	.remove		= spi_slave_system_control_remove,
+};
+module_spi_driver(spi_slave_system_control_driver);
+
+MODULE_AUTHOR("Geert Uytterhoeven <geert+renesas@glider.be>");
+MODULE_DESCRIPTION("SPI slave reporting boot up time");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH/RFC 6/6] spi: spidev: Allow direct references in DT from SPI slave controllers

[Geert Uytterhoeven]

SPI slave protocols are typically handled in userspace through spidev,
hence suppress the warning when the SPI controller is running in slave
mode.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/spi/spidev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index e3c19f30f591115a..8b590dd96480a9f9 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -713,7 +713,8 @@ static int spidev_probe(struct spi_device *spi)
 	 * compatible string, it is a Linux implementation thing
 	 * rather than a description of the hardware.
 	 */
-	if (spi->dev.of_node && !of_match_device(spidev_dt_ids, &spi->dev)) {
+	if (spi->dev.of_node && !of_match_device(spidev_dt_ids, &spi->dev) &&
+	    !(spi->master->flags & SPI_MASTER_IS_SLAVE)) {
 		dev_err(&spi->dev, "buggy DT: spidev listed directly in DT\n");
 		WARN_ON(spi->dev.of_node &&
 			!of_match_device(spidev_dt_ids, &spi->dev));
-- 
1.9.1