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

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

[Geert Uytterhoeven]

	Hi all,

This is a second 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 nature 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,
  - ...

(Un)binding an SPI slave handler to the SPI slave device represented by an
SPI slave controller is done by (un)registering the slave device through
a sysfs virtual file named "slave", cfr. Documentation/spi/spi-summary.

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 system uptime at 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, and this is fully
supported.

>From the point of view of an SPI slave protocol handler, an SPI slave
controller looks almost like an ordinary SPI master controller. The only
exception is that a transfer request will block on the remote SPI
master, and may be cancelled using spi_slave_abort().
Hence "struct spi_master" has become a misnomer. For now I didn't bother
fixing that.  Should we rename spi_master (and the spi_*master*()
functions) to spi_controller? And create temporary wrappers until all
drivers have been converted?  Or should 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.

There's also a known issue with spi_slave_abort(), which does manage to
abort an ongoing transfer, but causes immediate aborts for any further
transfers. See my question in "spi: sh-msiof: Add slave mode support".

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.

Changes since v1:
  - Do not create a child node in SPI slave mode. Instead, add an
    "spi-slave" property, and put the mode properties in the controller
    node.
  - Attach SPI slave controllers to a new "spi_slave" device class,
  - Don't call of_spi_register_master() instead of letting it return
    early for SPI slave controllers,
  - Skip registration of children from DT or ACPI for SPI slave
    controllers,
  - Use a "slave" virtual file in sysfs to (un)register the (single)
    slave device for an SPI slave controller, incl. specifying the slave
    protocol handler,
  - Parse slave-specific mode properties in the SPI slave controller DT
    node for the (single) slave device using of_spi_parse_dt(),
  - Add cancellation support using spi_master.slave_abort() and
    spi_slave_abort(),
  - Rename flag SPI_MASTER_IS_SLAVE to SPI_CONTROLLER_IS_SLAVE,
  - Introduce helper function spi_controller_is_slave(), making it easy
    to leave out SPI slave support where appropriate,
  - Document "spi-slave" property in the MSIOF DT bindings,
  - Check for "spi-slave" property instead of "slave" child node in the
    MSIOF SPI driver,
  - Implement cancellation in the MSIOF SPI driver,
  - Resolve semantic differences in patch description, file header, and
    module description for spi-slave-time,
  - Use spi_async() instead of spi_read() in slave handlers,
  - Submit the next transfer from the previous transfer's completion
    callback, removing the need for a thread in slave handlers,
  - Let .remove() in slave handlers call spi_slave_abort() to cancel the
    current ongoing transfer, and wait for the completion to terminate,
  - Remove FIXME about hanging kthread_stop() in slave handlers,
  - Fix copy-and-pasted module description of spi-slave-system-control,
  - Added "spi: core: Extract of_spi_parse_dt()" and "spi: Document SPI
    slave controller support",
  - Dropped "spi: spidev: Allow direct references in DT from SPI slave
    controllers".

This patch series applies to v4.8-rc1..v4.8-rc6.
It appplies to next-20160912 with some small context changes.
For your convenience, I've pushed this series and its dependencies to
the topic/spi-slave-v2 branch of the git repository at
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git

Full test information is also available on the eLinux wiki
(http://elinux.org/Tests:MSIOF-SPI-Slave).

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).

Preparation for all examples below:
    # overlay add a-msiof1-spidev # buggy DT: spidev listed directly in DT
    # overlay add a-msiof2-slave

Example 1:

    # echo spi-slave-time > /sys/class/spi_slave/spi3/slave 
    # 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:

    # echo spi-slave-system-control > /sys/class/spi_slave/spi3/slave 
    # reboot='\x7c\x50'
    # poweroff='\x71\x3f'
    # halt='\x38\x76'
    # suspend='\x1b\x1b'
    # spidev_test -D /dev/spidev2.0 -p $suspend # or $reboot, $poweroff, $halt

Example 3:

    # echo spidev > /sys/class/spi_slave/spi3/slave 
    # spidev_test -D /dev/spidev3.0 -p slave-hello-to-master &
    # spidev_test -D /dev/spidev2.0 -p master-hello-to-slave

Thanks for your comments!


Geert Uytterhoeven (6):
  [RFC] spi: Document DT bindings for SPI controllers in slave mode
  [RFC] spi: core: Extract of_spi_parse_dt()
  [RFC] spi: core: Add support for registering SPI slave controllers
  [RFC] spi: Document SPI slave controller support
  [RFC] spi: slave: Add SPI slave handler reporting uptime at previous
    message
  [RFC] spi: slave: Add SPI slave handler controlling system state

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

 Documentation/devicetree/bindings/spi/sh-msiof.txt |   2 +
 Documentation/devicetree/bindings/spi/spi-bus.txt  |  34 ++--
 Documentation/spi/spi-summary                      |  27 ++-
 drivers/spi/Kconfig                                |  26 ++-
 drivers/spi/Makefile                               |   4 +
 drivers/spi/spi-sh-msiof.c                         |  67 +++++--
 drivers/spi/spi-slave-system-control.c             | 154 +++++++++++++++
 drivers/spi/spi-slave-time.c                       | 126 ++++++++++++
 drivers/spi/spi.c                                  | 220 +++++++++++++++++----
 include/linux/spi/sh_msiof.h                       |   6 +
 include/linux/spi/spi.h                            |  17 +-
 11 files changed, 608 insertions(+), 75 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-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
v2:
  - Do not create a child node in SPI slave mode. Instead, add an
    "spi-slave" property, and put the mode properties in the controller
    node.
---
 Documentation/devicetree/bindings/spi/spi-bus.txt | 34 ++++++++++++++---------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
index 17822860cb98c34d..1ae28d7cafb68dc5 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 mode, the SPI controller node requires one additional property:
+- spi-slave       - Empty property.
+
 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,14 @@ cs1 : native
 cs2 : &gpio1 1 0
 cs3 : &gpio1 2 0
 
-SPI slave nodes must be children of the SPI master node and can
-contain the following properties.
-- reg             - (required) chip select address of device.
+In master mode, SPI slave nodes must be children of the SPI controller node.
+In slave mode, the (single) slave device is represented by the controller node
+itself. SPI slave nodes can contain the following properties.
+- 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

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC v2 2/7] spi: core: Extract of_spi_parse_dt()

[Geert Uytterhoeven]

Extract the parsing of SPI slave-specific properties into its own
function, so it can be reused later for SPI slave controllers.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/spi/spi.c | 60 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 51ad42fad5679133..79e5c1ba23c81dc1 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1471,37 +1471,18 @@ err_init_queue:
 /*-------------------------------------------------------------------------*/
 
 #if defined(CONFIG_OF)
-static struct spi_device *
-of_register_spi_device(struct spi_master *master, struct device_node *nc)
+static int of_spi_parse_dt(struct spi_master *master, struct spi_device *spi,
+			   struct device_node *nc)
 {
-	struct spi_device *spi;
-	int rc;
 	u32 value;
-
-	/* Alloc an spi_device */
-	spi = spi_alloc_device(master);
-	if (!spi) {
-		dev_err(&master->dev, "spi_device alloc error for %s\n",
-			nc->full_name);
-		rc = -ENOMEM;
-		goto err_out;
-	}
-
-	/* Select device driver */
-	rc = of_modalias_node(nc, spi->modalias,
-				sizeof(spi->modalias));
-	if (rc < 0) {
-		dev_err(&master->dev, "cannot find modalias for %s\n",
-			nc->full_name);
-		goto err_out;
-	}
+	int rc;
 
 	/* 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;
+		return rc;
 	}
 	spi->chip_select = value;
 
@@ -1559,10 +1540,41 @@ of_register_spi_device(struct spi_master *master, struct device_node *nc)
 	if (rc) {
 		dev_err(&master->dev, "%s has no valid 'spi-max-frequency' property (%d)\n",
 			nc->full_name, rc);
-		goto err_out;
+		return rc;
 	}
 	spi->max_speed_hz = value;
 
+	return 0;
+}
+
+static struct spi_device *
+of_register_spi_device(struct spi_master *master, struct device_node *nc)
+{
+	struct spi_device *spi;
+	int rc;
+
+	/* Alloc an spi_device */
+	spi = spi_alloc_device(master);
+	if (!spi) {
+		dev_err(&master->dev, "spi_device alloc error for %s\n",
+			nc->full_name);
+		rc = -ENOMEM;
+		goto err_out;
+	}
+
+	/* Select device driver */
+	rc = of_modalias_node(nc, spi->modalias,
+				sizeof(spi->modalias));
+	if (rc < 0) {
+		dev_err(&master->dev, "cannot find modalias for %s\n",
+			nc->full_name);
+		goto err_out;
+	}
+
+	rc = of_spi_parse_dt(master, spi, nc);
+	if (rc)
+		goto err_out;
+
 	/* Store a pointer to the node in the device structure */
 	of_node_get(nc);
 	spi->dev.of_node = nc;
-- 
1.9.1

[PATCH/RFC v2 3/7] 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_CONTROLLER_IS_SLAVE flag in
    spi_master.flags, and should provide an additional callback
    "slave_abort" to abort an ongoing SPI transfer request.
  - SPI slave controllers are added to a new "spi_slave" device class,
  - (Un)binding an SPI slave handler to the SPI slave device represented
    by an SPI slave controller is done by (un)registering the slave
    device through a sysfs virtual file named "slave",
  - Initial slave-specific mode properties are parsed from the SPI slave
    controller DT node.

>From the point of view of an SPI slave protocol handler, an SPI slave
controller looks almost like an ordinary SPI master controller. The only
exception is that a transfer request will block on the remote SPI
master, and may be cancelled using spi_slave_abort().

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
v2:
  - Attach SPI slave controllers to a new "spi_slave" device class,
  - Don't call of_spi_register_master() instead of letting it return
    early for SPI slave controllers,
  - Skip registration of children from DT or ACPI for SPI slave
    controllers,
  - Use a "slave" virtual file in sysfs to (un)register the (single)
    slave device for an SPI slave controller, incl. specifying the slave
    protocol handler,
  - Parse slave-specific mode properties in the SPI slave controller DT
    node for the (single) slave device using of_spi_parse_dt(),
  - Add cancellation support using spi_master.slave_abort() and
    spi_slave_abort(),
  - Rename flag SPI_MASTER_IS_SLAVE to SPI_CONTROLLER_IS_SLAVE,
  - Introduce helper function spi_controller_is_slave(), making it easy
    to leave out SPI slave support where appropriate.

TBD:
  - s/spi_master/spi_controller/ where appropriate,
  - Provide wrappers (e.g. "#define spi_master spi_controller" until all
    SPI drivers have been converted),
  - Do we want a separate spi_register_slave() instead?
---
 drivers/spi/Kconfig     |  14 ++++-
 drivers/spi/Makefile    |   2 +
 drivers/spi/spi.c       | 162 +++++++++++++++++++++++++++++++++++++++++++-----
 include/linux/spi/spi.h |  17 ++++-
 4 files changed, 175 insertions(+), 20 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index d6fb8d4b778672fd..fa13a3663de5bde0 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -737,6 +737,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 185367ef65761a74..96e5c0d6e5bc6151 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -98,3 +98,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 79e5c1ba23c81dc1..cfb0d573fc4dedba 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1477,15 +1477,6 @@ static int of_spi_parse_dt(struct spi_master *master, struct spi_device *spi,
 	u32 value;
 	int rc;
 
-	/* 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);
-		return rc;
-	}
-	spi->chip_select = value;
-
 	/* Mode (clock phase/polarity/etc.) */
 	if (of_find_property(nc, "spi-cpha", NULL))
 		spi->mode |= SPI_CPHA;
@@ -1535,6 +1526,18 @@ static int of_spi_parse_dt(struct spi_master *master, struct spi_device *spi,
 		}
 	}
 
+	if (spi_controller_is_slave(master))
+		return 0;
+
+	/* 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);
+		return rc;
+	}
+	spi->chip_select = value;
+
 	/* Device speed */
 	rc = of_property_read_u32(nc, "spi-max-frequency", &value);
 	if (rc) {
@@ -1619,6 +1622,8 @@ static void of_register_spi_devices(struct spi_master *master)
 	}
 }
 #else
+static int of_spi_parse_dt(struct spi_master *master, struct spi_device *spi,
+			   struct device_node *nc) { return 0; }
 static void of_register_spi_devices(struct spi_master *master) { }
 #endif
 
@@ -1764,6 +1769,110 @@ static struct class spi_master_class = {
 	.dev_groups	= spi_master_groups,
 };
 
+#ifdef CONFIG_SPI_SLAVE
+/**
+ * spi_slave_abort - abort the ongoing transfer request on an SPI slave
+ *		     controller
+ * @spi: device used for the current transfer
+ */
+int spi_slave_abort(struct spi_device *spi)
+{
+	struct spi_master *master = spi->master;
+
+	if (spi_controller_is_slave(master) && master->slave_abort)
+		return master->slave_abort(master);
+
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(spi_slave_abort);
+
+static int match_true(struct device *dev, void *data)
+{
+	return 1;
+}
+
+static ssize_t spi_slave_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct spi_master *ctlr = container_of(dev, struct spi_master, dev);
+	struct device *child;
+
+	child = device_find_child(&ctlr->dev, NULL, match_true);
+	return sprintf(buf, "%s\n",
+		       child ? to_spi_device(child)->modalias : NULL);
+}
+
+static ssize_t spi_slave_store(struct device *dev,
+			       struct device_attribute *attr, const char *buf,
+			       size_t count)
+{
+	struct spi_master *ctlr = container_of(dev, struct spi_master, dev);
+	struct spi_device *spi;
+	struct device *child;
+	char name[32];
+	int rc;
+
+	rc = sscanf(buf, "%31s", name);
+	if (rc != 1 || !name[0])
+		return -EINVAL;
+
+	child = device_find_child(&ctlr->dev, NULL, match_true);
+	if (child) {
+		/* Remove registered slave */
+		device_unregister(child);
+		put_device(child);
+	}
+
+	if (strcmp(name, "(null)")) {
+		/* Register new slave */
+		spi = spi_alloc_device(ctlr);
+		if (!spi)
+			return -ENOMEM;
+
+		strlcpy(spi->modalias, name, sizeof(spi->modalias));
+
+		rc = of_spi_parse_dt(ctlr, spi, ctlr->dev.of_node);
+		if (rc)
+			goto err_out;
+
+		rc = spi_add_device(spi);
+		if (rc)
+			goto err_out;
+	}
+
+	return count;
+
+err_out:
+	spi_dev_put(spi);
+	return rc;
+}
+
+static DEVICE_ATTR(slave, S_IWUSR|S_IRUGO, spi_slave_show, spi_slave_store);
+
+static struct attribute *spi_slave_attrs[] = {
+	&dev_attr_slave.attr,
+	NULL,
+};
+
+static const struct attribute_group spi_slave_group = {
+	.attrs = spi_slave_attrs,
+};
+
+static const struct attribute_group *spi_slave_groups[] = {
+	&spi_master_statistics_group,
+	&spi_slave_group,
+	NULL,
+};
+
+static struct class spi_slave_class = {
+	.name		= "spi_slave",
+	.owner		= THIS_MODULE,
+	.dev_release	= spi_master_release,
+	.dev_groups	= spi_slave_groups,
+};
+#else
+extern struct class spi_slave_class;	/* dummy */
+#endif
 
 /**
  * spi_alloc_master - allocate SPI master controller
@@ -1799,7 +1908,6 @@ struct spi_master *spi_alloc_master(struct device *dev, unsigned size)
 	device_initialize(&master->dev);
 	master->bus_num = -1;
 	master->num_chipselect = 1;
-	master->dev.class = &spi_master_class;
 	master->dev.parent = dev;
 	pm_suspend_ignore_children(&master->dev, true);
 	spi_master_set_devdata(master, &master[1]);
@@ -1882,9 +1990,18 @@ int spi_register_master(struct spi_master *master)
 	if (!dev)
 		return -ENODEV;
 
-	status = of_spi_register_master(master);
-	if (status)
-		return status;
+	if (master->flags & SPI_CONTROLLER_IS_SLAVE) {
+		if (!IS_ENABLED(CONFIG_SPI_SLAVE))
+			return -ENOSYS;
+
+		master->dev.class = &spi_slave_class;
+	} else {
+		master->dev.class = &spi_master_class;
+
+		status = of_spi_register_master(master);
+		if (status)
+			return status;
+	}
 
 	/* even if it's just one always-selected device, there must
 	 * be at least one chipselect
@@ -1921,8 +2038,9 @@ 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",
+			spi_controller_is_slave(master) ? "slave" : "master",
+			dev_name(&master->dev), dynamic ? " (dynamic)" : "");
 
 	/* If we're using a queued driver, start the queue */
 	if (master->transfer)
@@ -1944,8 +2062,10 @@ int spi_register_master(struct spi_master *master)
 	mutex_unlock(&board_lock);
 
 	/* Register devices from the device tree and ACPI */
-	of_register_spi_devices(master);
-	acpi_register_spi_devices(master);
+	if (!spi_controller_is_slave(master)) {
+		of_register_spi_devices(master);
+		acpi_register_spi_devices(master);
+	}
 done:
 	return status;
 }
@@ -3247,6 +3367,12 @@ static int __init spi_init(void)
 	if (status < 0)
 		goto err2;
 
+	if (IS_ENABLED(CONFIG_SPI_SLAVE)) {
+		status = class_register(&spi_slave_class);
+		if (status < 0)
+			goto err3;
+	}
+
 	if (IS_ENABLED(CONFIG_OF_DYNAMIC))
 		WARN_ON(of_reconfig_notifier_register(&spi_of_notifier));
 	if (IS_ENABLED(CONFIG_ACPI))
@@ -3254,6 +3380,8 @@ static int __init spi_init(void)
 
 	return 0;
 
+err3:
+	class_unregister(&spi_master_class);
 err2:
 	bus_unregister(&spi_bus_type);
 err1:
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 072cb2aa24139981..9a278e92b5b70e07 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;
 
@@ -371,6 +371,7 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  * @handle_err: the subsystem calls the driver to handle an error that occurs
  *		in the generic implementation of transfer_one_message().
  * @unprepare_message: undo any work done by prepare_message().
+ * @slave_abort: abort the ongoing transfer request on an SPI slave controller
  * @spi_flash_read: to support spi-controller hardwares that provide
  *                  accelerated interface to read from flash devices.
  * @flash_read_supported: spi device supports flash read
@@ -440,6 +441,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_CONTROLLER_IS_SLAVE	BIT(5)		/* SPI slave controller */
 
 	/*
 	 * on some hardware transfer size may be constrained
@@ -532,6 +534,7 @@ struct spi_master {
 			       struct spi_message *message);
 	int (*unprepare_message)(struct spi_master *master,
 				 struct spi_message *message);
+	int (*slave_abort)(struct spi_master *spi);
 	int (*spi_flash_read)(struct  spi_device *spi,
 			      struct spi_flash_read_message *msg);
 	bool (*flash_read_supported)(struct spi_device *spi);
@@ -586,6 +589,15 @@ static inline void spi_master_put(struct spi_master *master)
 		put_device(&master->dev);
 }
 
+static inline bool spi_controller_is_slave(struct spi_master *ctlr)
+{
+#ifdef CONFIG_SPI_SLAVE
+	return ctlr->flags & SPI_CONTROLLER_IS_SLAVE;
+#else
+	return false;
+#endif
+}
+
 /* PM calls that need to be issued by the driver */
 extern int spi_master_suspend(struct spi_master *master);
 extern int spi_master_resume(struct spi_master *master);
@@ -903,6 +915,7 @@ extern int spi_setup(struct spi_device *spi);
 extern int spi_async(struct spi_device *spi, struct spi_message *message);
 extern int spi_async_locked(struct spi_device *spi,
 			    struct spi_message *message);
+extern int spi_slave_abort(struct spi_device *spi);
 
 static inline size_t
 spi_max_transfer_size(struct spi_device *spi)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC v2 4/7] spi: Document SPI slave controller support

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
v2:
  - New.
---
 Documentation/spi/spi-summary | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/Documentation/spi/spi-summary b/Documentation/spi/spi-summary
index d1824b399b2d1d79..1721c1b570c32466 100644
--- a/Documentation/spi/spi-summary
+++ b/Documentation/spi/spi-summary
@@ -62,8 +62,8 @@ chips described as using "three wire" signaling: SCK, data, nCSx.
 (That data line is sometimes called MOMI or SISO.)
 
 Microcontrollers often support both master and slave sides of the SPI
-protocol.  This document (and Linux) currently only supports the master
-side of SPI interactions.
+protocol.  This document (and Linux) supports both the master and slave
+sides of SPI interactions.
 
 
 Who uses it?  On what kinds of systems?
@@ -154,9 +154,8 @@ control audio interfaces, present touchscreen sensors as input interfaces,
 or monitor temperature and voltage levels during industrial processing.
 And those might all be sharing the same controller driver.
 
-A "struct spi_device" encapsulates the master-side interface between
-those two types of driver.  At this writing, Linux has no slave side
-programming interface.
+A "struct spi_device" encapsulates the controller-side interface between
+those two types of drivers.
 
 There is a minimal core of SPI programming interfaces, focussing on
 using the driver model to connect controller and protocol drivers using
@@ -177,10 +176,24 @@ shows up in sysfs in several locations:
    /sys/bus/spi/drivers/D ... driver for one or more spi*.* devices
 
    /sys/class/spi_master/spiB ... symlink (or actual device node) to
-	a logical node which could hold class related state for the
-	controller managing bus "B".  All spiB.* devices share one
+	a logical node which could hold class related state for the SPI
+	master controller managing bus "B".  All spiB.* devices share one
 	physical SPI bus segment, with SCLK, MOSI, and MISO.
 
+   /sys/devices/.../CTLR/slave ... virtual file for (un)registering the
+	slave device for an SPI slave controller.
+	Writing the driver name of an SPI slave handler to this file
+	registers the slave device; writing "(null)" unregisters the slave
+	device.
+	Reading from this file shows the name of the slave device ("(null)"
+	if not registered).
+
+   /sys/class/spi_slave/spiB ... symlink (or actual device node) to
+	a logical node which could hold class related state for the SPI
+	slave controller on bus "B".  When registered, a single spiB.*
+	device is present here, possible sharing the physical SPI bus
+	segment with other SPI slave devices.
+
 Note that the actual location of the controller's class state depends
 on whether you enabled CONFIG_SYSFS_DEPRECATED or not.  At this time,
 the only class-specific state is the bus number ("B" in "spiB"), so
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC v2 5/7] 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, in both PIO and DMA mode.

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, cancellation,
	rewording]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
FIXME What's the proper way to cancel a completion blocked on
      wait_for_completion_interruptible() from the kernel?

  - When an in-kernel SPI slave handler calls signal_wake_up() on the
    blocked thread (the SPI worker thread), this does cancel the
    completion, and makes wait_for_completion_interruptible() return
    -ERESTARTSYS.
    However, all subsequent calls to wait_for_completion_interruptible()
    fail immediately with -ERESTARTSYS :-(
  - When using spidev as an SPI slave handler, stopping (CTRL-C) the
    userspace process also cancels the completion, but subsequent calls
    to wait_for_completion_interruptible() work fine.

Thanks!

v2:
  - Document "spi-slave" property in DT bindings,
  - Use spi_controller_is_slave() helper,
  - Check for "spi-slave" property instead of "slave" child node,
  - Replace SPI_MASTER_IS_SLAVE by SPI_CONTROLLER_IS_SLAVE,
  - Implement cancellation.
---
 Documentation/devicetree/bindings/spi/sh-msiof.txt |  2 +
 drivers/spi/spi-sh-msiof.c                         | 67 ++++++++++++++++++----
 include/linux/spi/sh_msiof.h                       |  6 ++
 3 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index aa005c1d10d95756..7782c6abc7890bb0 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -31,6 +31,8 @@ Optional properties:
 			 specifiers, one for transmission, and one for
 			 reception.
 - dma-names            : Must contain a list of two DMA names, "tx" and "rx".
+- spi-slave            : Empty property indicating the SPI controller is used
+			 in slave mode.
 - renesas,dtdl         : delay sync signal (setup) in transmit mode.
 			 Must contain one of the following values:
 			 0   (no bit delay)
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 0f83ad1d5a5858dd..c585c9f3eb126738 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 (spi_controller_is_slave(p->master))
+		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 = spi_controller_is_slave(p->master);
+	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,20 +584,42 @@ 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 = spi_controller_is_slave(p->master);
+	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;
 }
 
+static int sh_msiof_slave_abort(struct spi_master *master)
+{
+	struct sh_msiof_spi_priv *p = spi_master_get_devdata(master);
+	unsigned long flags;
+
+	spin_lock_irqsave(&p->done.wait.lock, flags);
+	if (!p->done.done) {
+		wait_queue_t *curr, *next;
+
+		list_for_each_entry_safe(curr, next, &p->done.wait.task_list,
+					 task_list) {
+			// FIXME Find a better way to signal and wake up
+			signal_wake_up(curr->private, 1);
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&p->done.wait.lock, flags);
+	return 0;
+}
+
 static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
 				  void (*tx_fifo)(struct sh_msiof_spi_priv *,
 						  const void *, int, int),
@@ -633,7 +660,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 (spi_controller_is_slave(p->master))
+		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 +774,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 (spi_controller_is_slave(p->master))
+		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 +875,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 (!spi_controller_is_slave(p->master))
+		sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
 
 	while (master->dma_tx && len > 15) {
 		/*
@@ -992,8 +1028,12 @@ static struct sh_msiof_spi_info *sh_msiof_spi_parse_dt(struct device *dev)
 	if (!info)
 		return NULL;
 
+	info->mode = of_property_read_bool(np, "spi-slave") ? MSIOF_SPI_SLAVE
+							    : 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,11 +1268,14 @@ 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_CONTROLLER_IS_SLAVE;
 	master->bus_num = pdev->id;
 	master->dev.of_node = pdev->dev.of_node;
 	master->num_chipselect = p->info->num_chipselect;
 	master->setup = sh_msiof_spi_setup;
 	master->prepare_message = sh_msiof_prepare_message;
+	master->slave_abort = sh_msiof_slave_abort;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(8, 32);
 	master->auto_runtime_pm = true;
 	master->transfer_one = sh_msiof_transfer_one;
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 v2 6/7] spi: slave: Add SPI slave handler reporting uptime at previous message

[Geert Uytterhoeven]

Add an example SPI slave handler responding with the uptime at 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>
---
v2:
  - Resolve semantic differences in patch description, file header, and
    module description,
  - Use spi_async() instead of spi_read(),
  - Submit the next transfer from the previous transfer's completion
    callback, removing the need for a thread,
  - Let .remove() call spi_slave_abort() to cancel the current ongoing
    transfer, and wait for the completion to terminate,
  - Remove FIXME about hanging kthread_stop().
---
 drivers/spi/Kconfig          |   6 +++
 drivers/spi/Makefile         |   1 +
 drivers/spi/spi-slave-time.c | 126 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)
 create mode 100644 drivers/spi/spi-slave-time.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index fa13a3663de5bde0..515bddee4018c246 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -749,6 +749,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 96e5c0d6e5bc6151..ba4a7e79fe113d3e 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -100,3 +100,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..69e970ecb579adc7
--- /dev/null
+++ b/drivers/spi/spi-slave-time.c
@@ -0,0 +1,126 @@
+/*
+ * SPI slave handler reporting uptime at reception of previous SPI message
+ *
+ * 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/completion.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+
+
+struct spi_slave_time_priv {
+	struct spi_device *spi;
+	struct completion finished;
+	struct spi_transfer xfer;
+	struct spi_message msg;
+	__be32 buf[2];
+};
+
+static int spi_slave_time_submit(struct spi_slave_time_priv *priv);
+
+static void spi_slave_time_complete(void *arg)
+{
+	struct spi_slave_time_priv *priv = arg;
+	int ret;
+
+	ret = priv->msg.status;
+	if (ret)
+		goto terminate;
+
+	ret = spi_slave_time_submit(priv);
+	if (ret)
+		goto terminate;
+
+	return;
+
+terminate:
+	pr_info("%s: Terminating\n", __func__);
+	complete(&priv->finished);
+}
+
+static int spi_slave_time_submit(struct spi_slave_time_priv *priv)
+{
+	u32 rem_ns;
+	int ret;
+	u64 ts;
+
+	ts = local_clock();
+	rem_ns = do_div(ts, 1000000000) / 1000;
+
+	priv->buf[0] = cpu_to_be32(ts);
+	priv->buf[1] = cpu_to_be32(rem_ns);
+
+	spi_message_init_with_transfers(&priv->msg, &priv->xfer, 1);
+
+	priv->msg.complete = spi_slave_time_complete;
+	priv->msg.context = priv;
+
+	ret = spi_async(priv->spi, &priv->msg);
+	if (ret)
+		pr_err("%s: spi_async() failed %d\n", __func__, ret);
+
+	return ret;
+}
+
+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;
+	init_completion(&priv->finished);
+	priv->xfer.tx_buf = priv->buf;
+	priv->xfer.len = sizeof(priv->buf);
+
+	ret = spi_slave_time_submit(priv);
+	if (ret)
+		return ret;
+
+	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);
+
+	spi_slave_abort(spi);
+	wait_for_completion(&priv->finished);
+	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 uptime at previous SPI message");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH/RFC v2 7/7] spi: slave: Add SPI slave handler controlling system state

[Geert Uytterhoeven]

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

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Use spi_async() instead of spi_read(),
  - Submit the next transfer from the previous transfer's completion
    callback, removing the need for a thread,
  - Let .remove() call spi_slave_abort() to cancel the current ongoing
    transfer, and wait for the completion to terminate,
  - Remove FIXME about hanging kthread_stop(),
  - Fix copy-and-pasted module description.
---
 drivers/spi/Kconfig                    |   6 ++
 drivers/spi/Makefile                   |   1 +
 drivers/spi/spi-slave-system-control.c | 154 +++++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 drivers/spi/spi-slave-system-control.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 515bddee4018c246..3d8ae9505ef95992 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -755,6 +755,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 ba4a7e79fe113d3e..e402c1a12f47c38f 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -101,3 +101,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..736dd59928cb3bc3
--- /dev/null
+++ b/drivers/spi/spi-slave-system-control.c
@@ -0,0 +1,154 @@
+/*
+ * 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/completion.h>
+#include <linux/module.h>
+#include <linux/reboot.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 completion finished;
+	struct spi_transfer xfer;
+	struct spi_message msg;
+	__le16 cmd;
+};
+
+static
+int spi_slave_system_control_submit(struct spi_slave_system_control_priv *priv);
+
+static void spi_slave_system_control_complete(void *arg)
+{
+	struct spi_slave_system_control_priv *priv = arg;
+	u16 cmd;
+	int ret;
+
+	if (priv->msg.status)
+		goto terminate;
+
+	cmd = le16_to_cpu(priv->cmd);
+	switch (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;
+	}
+
+	ret = spi_slave_system_control_submit(priv);
+	if (ret)
+		goto terminate;
+
+	return;
+
+terminate:
+	pr_info("%s: Terminating\n", __func__);
+	complete(&priv->finished);
+}
+
+static
+int spi_slave_system_control_submit(struct spi_slave_system_control_priv *priv)
+{
+	int ret;
+
+	spi_message_init_with_transfers(&priv->msg, &priv->xfer, 1);
+
+	priv->msg.complete = spi_slave_system_control_complete;
+	priv->msg.context = priv;
+
+	ret = spi_async(priv->spi, &priv->msg);
+	if (ret)
+		pr_err("%s: spi_async() failed %d\n", __func__, ret);
+
+	return ret;
+}
+
+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;
+	init_completion(&priv->finished);
+	priv->xfer.rx_buf = &priv->cmd;
+	priv->xfer.len = sizeof(priv->cmd);
+
+	ret = spi_slave_system_control_submit(priv);
+	if (ret)
+		return ret;
+
+	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);
+
+	spi_slave_abort(spi);
+	wait_for_completion(&priv->finished);
+	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 handler controlling system state");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH/RFC v2 3/7] spi: core: Add support for registering SPI slave controllers

[Geert Uytterhoeven]

On Mon, Sep 12, 2016 at 10:50 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1477,15 +1477,6 @@ static int of_spi_parse_dt(struct spi_master *master, struct spi_device *spi,

> @@ -1799,7 +1908,6 @@ struct spi_master *spi_alloc_master(struct device *dev, unsigned size)
>         device_initialize(&master->dev);
>         master->bus_num = -1;
>         master->num_chipselect = 1;
> -       master->dev.class = &spi_master_class;
>         master->dev.parent = dev;
>         pm_suspend_ignore_children(&master->dev, true);
>         spi_master_set_devdata(master, &master[1]);
> @@ -1882,9 +1990,18 @@ int spi_register_master(struct spi_master *master)
>         if (!dev)
>                 return -ENODEV;
>
> -       status = of_spi_register_master(master);
> -       if (status)
> -               return status;
> +       if (master->flags & SPI_CONTROLLER_IS_SLAVE) {
> +               if (!IS_ENABLED(CONFIG_SPI_SLAVE))
> +                       return -ENOSYS;
> +
> +               master->dev.class = &spi_slave_class;
> +       } else {
> +               master->dev.class = &spi_master_class;
> +
> +               status = of_spi_register_master(master);
> +               if (status)
> +                       return status;
> +       }
>
>         /* even if it's just one always-selected device, there must
>          * be at least one chipselect

0day reported a warning during boot:

    WARNING: CPU: 0 PID: 1 at drivers/base/core.c:251 device_release+0x7d/0x8a
    Device '(null)' does not have a release() function, it is broken
and must be fixed.

This is caused by moving the setup of master->dev.class.
To fix this, I can
  1) Introduce a separate spi_alloc_slave() function, which sets up
     spi_slave_class instead of spi_master class,
  OR
  2) Keep the setup of spi_master_class in spi_alloc_master(), and override it
     later. Both classes use the same dev_release method anyway.

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

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

[Rob Herring]

On Mon, Sep 12, 2016 at 10:50:40PM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> ---
> v2:
>   - Do not create a child node in SPI slave mode. Instead, add an
>     "spi-slave" property, and put the mode properties in the controller
>     node.
> ---
>  Documentation/devicetree/bindings/spi/spi-bus.txt | 34 ++++++++++++++---------
>  1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
> index 17822860cb98c34d..1ae28d7cafb68dc5 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.

We'll probably need some way to define what interface/protocol 
the slave has. Perhaps the most specific compatible should be the 
protocol the slave uses? Maybe that is how you use a child node?

> +
> +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 mode, the SPI controller node requires one additional property:
> +- spi-slave       - Empty property.
> +
>  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,14 @@ cs1 : native
>  cs2 : &gpio1 1 0
>  cs3 : &gpio1 2 0
>  
> -SPI slave nodes must be children of the SPI master node and can
> -contain the following properties.
> -- reg             - (required) chip select address of device.
> +In master mode, SPI slave nodes must be children of the SPI controller node.
> +In slave mode, the (single) slave device is represented by the controller node
> +itself. SPI slave nodes can contain the following properties.

I find this a bit confusing as you talk about master mode, then slave 
mode, then slave nodes (master mode again).

> +- 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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Geert Uytterhoeven]

Hi Rob,

On Tue, Sep 20, 2016 at 5:00 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Sep 12, 2016 at 10:50:40PM +0200, Geert Uytterhoeven wrote:
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> v2:
>>   - Do not create a child node in SPI slave mode. Instead, add an
>>     "spi-slave" property, and put the mode properties in the controller
>>     node.
>> ---
>>  Documentation/devicetree/bindings/spi/spi-bus.txt | 34 ++++++++++++++---------
>>  1 file changed, 21 insertions(+), 13 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
>> index 17822860cb98c34d..1ae28d7cafb68dc5 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.
>
> We'll probably need some way to define what interface/protocol
> the slave has. Perhaps the most specific compatible should be the
> protocol the slave uses? Maybe that is how you use a child node?

That was indeed an advantage of using a child node (which you suggested
_not_ doing in your review of v1?): you can specify which protocol to use.

In v2, the protocol is specified through sysfs, like for i2c slave.

Note that SPI is different than I2C: an SPI slave is connected to a single
master, and can assume a single role only, while I2C is a shared bus, and
a slave can assume multiple roles (an I2C slave can respond to multiple
addresses, and can e.g. provide more than one software I2C EEPROM).
So you could argue the protocol is fixed by the hardware topology, cfr.
my v1.

>> +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 mode, the SPI controller node requires one additional property:
>> +- spi-slave       - Empty property.
>> +
>>  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,14 @@ cs1 : native
>>  cs2 : &gpio1 1 0
>>  cs3 : &gpio1 2 0
>>
>> -SPI slave nodes must be children of the SPI master node and can
>> -contain the following properties.
>> -- reg             - (required) chip select address of device.
>> +In master mode, SPI slave nodes must be children of the SPI controller node.
>> +In slave mode, the (single) slave device is represented by the controller node
>> +itself. SPI slave nodes can contain the following properties.
>
> I find this a bit confusing as you talk about master mode, then slave
> mode, then slave nodes (master mode again).

The last part is actually about both master and slave mode: in slave mode,
the properties apply to the controller node itself, instead of to child nodes.

I wanted to reuse as much of the existing text as possible.
But I agree the description could use some refactoring.

Thanks for your comments!

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

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

[Rob Herring]

On Wed, Sep 21, 2016 at 02:47:50PM +0200, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Tue, Sep 20, 2016 at 5:00 PM, Rob Herring <robh@kernel.org> wrote:
> > On Mon, Sep 12, 2016 at 10:50:40PM +0200, Geert Uytterhoeven wrote:
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >> v2:
> >>   - Do not create a child node in SPI slave mode. Instead, add an
> >>     "spi-slave" property, and put the mode properties in the controller
> >>     node.
> >> ---
> >>  Documentation/devicetree/bindings/spi/spi-bus.txt | 34 ++++++++++++++---------
> >>  1 file changed, 21 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
> >> index 17822860cb98c34d..1ae28d7cafb68dc5 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.
> >
> > We'll probably need some way to define what interface/protocol
> > the slave has. Perhaps the most specific compatible should be the
> > protocol the slave uses? Maybe that is how you use a child node?
> 
> That was indeed an advantage of using a child node (which you suggested
> _not_ doing in your review of v1?): you can specify which protocol to use.

Yeah, maybe V1 was better... One thing though, the child node should be 
optional IMO. Maybe you keep "spi-slave" too to define the controller is 
in slave mode, but the protocol is not defined. Or maybe no child nodes 
is sufficient?

> In v2, the protocol is specified through sysfs, like for i2c slave.

That's fine, because it may be purely a s/w decision what the protocol 
is. If it is fixed, then in DT is fine.

> Note that SPI is different than I2C: an SPI slave is connected to a single
> master, and can assume a single role only, while I2C is a shared bus, and
> a slave can assume multiple roles (an I2C slave can respond to multiple
> addresses, and can e.g. provide more than one software I2C EEPROM).
> So you could argue the protocol is fixed by the hardware topology, cfr.
> my v1.

If the protocol is s/w on both sides, then the protocol could easily 
change.


> >> +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 mode, the SPI controller node requires one additional property:
> >> +- spi-slave       - Empty property.
> >> +
> >>  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,14 @@ cs1 : native
> >>  cs2 : &gpio1 1 0
> >>  cs3 : &gpio1 2 0
> >>
> >> -SPI slave nodes must be children of the SPI master node and can
> >> -contain the following properties.
> >> -- reg             - (required) chip select address of device.
> >> +In master mode, SPI slave nodes must be children of the SPI controller node.
> >> +In slave mode, the (single) slave device is represented by the controller node
> >> +itself. SPI slave nodes can contain the following properties.
> >
> > I find this a bit confusing as you talk about master mode, then slave
> > mode, then slave nodes (master mode again).
> 
> The last part is actually about both master and slave mode: in slave mode,
> the properties apply to the controller node itself, instead of to child nodes.
> 
> I wanted to reuse as much of the existing text as possible.
> But I agree the description could use some refactoring.

Even with a child node, I think it is better to just have 2 sections and 
list common properties twice.

Rob

Re: [PATCH/RFC v2 3/7] spi: core: Add support for registering SPI slave controllers

[Mark Brown]

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

On Sun, Sep 18, 2016 at 11:04:18AM +0200, Geert Uytterhoeven wrote:

> This is caused by moving the setup of master->dev.class.
> To fix this, I can
>   1) Introduce a separate spi_alloc_slave() function, which sets up
>      spi_slave_class instead of spi_master class,

This seems more idiomatic.

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

Re: [PATCH/RFC v2 3/7] spi: core: Add support for registering SPI slave controllers

[Mark Brown]

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

On Mon, Sep 12, 2016 at 10:50:42PM +0200, Geert Uytterhoeven wrote:

> TBD:
>   - s/spi_master/spi_controller/ where appropriate,
>   - Provide wrappers (e.g. "#define spi_master spi_controller" until all
>     SPI drivers have been converted),
>   - Do we want a separate spi_register_slave() instead?

This basically looks fine to me - there's these TBDs so I might be
missing things and we probably need some GPIO chip select handling but
that's a separate thing.  Sorry it took me so long to review this.

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

Applied "spi: core: Extract of_spi_parse_dt()" to the spi tree

[Mark Brown]

The patch

   spi: core: Extract of_spi_parse_dt()

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From c2e51ac3d0542440d5b2b8b52ff2ad00751af4da Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Mon, 12 Sep 2016 22:50:41 +0200
Subject: [PATCH] spi: core: Extract of_spi_parse_dt()

Extract the parsing of SPI slave-specific properties into its own
function, so it can be reused later for SPI slave controllers.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi.c | 60 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 656dd3e3220c..1861255866d7 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1502,37 +1502,18 @@ static int spi_master_initialize_queue(struct spi_master *master)
 /*-------------------------------------------------------------------------*/
 
 #if defined(CONFIG_OF)
-static struct spi_device *
-of_register_spi_device(struct spi_master *master, struct device_node *nc)
+static int of_spi_parse_dt(struct spi_master *master, struct spi_device *spi,
+			   struct device_node *nc)
 {
-	struct spi_device *spi;
-	int rc;
 	u32 value;
-
-	/* Alloc an spi_device */
-	spi = spi_alloc_device(master);
-	if (!spi) {
-		dev_err(&master->dev, "spi_device alloc error for %s\n",
-			nc->full_name);
-		rc = -ENOMEM;
-		goto err_out;
-	}
-
-	/* Select device driver */
-	rc = of_modalias_node(nc, spi->modalias,
-				sizeof(spi->modalias));
-	if (rc < 0) {
-		dev_err(&master->dev, "cannot find modalias for %s\n",
-			nc->full_name);
-		goto err_out;
-	}
+	int rc;
 
 	/* 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;
+		return rc;
 	}
 	spi->chip_select = value;
 
@@ -1590,10 +1571,41 @@ of_register_spi_device(struct spi_master *master, struct device_node *nc)
 	if (rc) {
 		dev_err(&master->dev, "%s has no valid 'spi-max-frequency' property (%d)\n",
 			nc->full_name, rc);
-		goto err_out;
+		return rc;
 	}
 	spi->max_speed_hz = value;
 
+	return 0;
+}
+
+static struct spi_device *
+of_register_spi_device(struct spi_master *master, struct device_node *nc)
+{
+	struct spi_device *spi;
+	int rc;
+
+	/* Alloc an spi_device */
+	spi = spi_alloc_device(master);
+	if (!spi) {
+		dev_err(&master->dev, "spi_device alloc error for %s\n",
+			nc->full_name);
+		rc = -ENOMEM;
+		goto err_out;
+	}
+
+	/* Select device driver */
+	rc = of_modalias_node(nc, spi->modalias,
+				sizeof(spi->modalias));
+	if (rc < 0) {
+		dev_err(&master->dev, "cannot find modalias for %s\n",
+			nc->full_name);
+		goto err_out;
+	}
+
+	rc = of_spi_parse_dt(master, spi, nc);
+	if (rc)
+		goto err_out;
+
 	/* Store a pointer to the node in the device structure */
 	of_node_get(nc);
 	spi->dev.of_node = nc;
-- 
2.11.0

Re: [PATCH/RFC v2 3/7] spi: core: Add support for registering SPI slave controllers

[Geert Uytterhoeven]

Hi Mark,

On Thu, Dec 15, 2016 at 6:53 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Sep 12, 2016 at 10:50:42PM +0200, Geert Uytterhoeven wrote:
>> TBD:
>>   - s/spi_master/spi_controller/ where appropriate,
>>   - Provide wrappers (e.g. "#define spi_master spi_controller" until all
>>     SPI drivers have been converted),
>>   - Do we want a separate spi_register_slave() instead?
>
> This basically looks fine to me - there's these TBDs so I might be
> missing things and we probably need some GPIO chip select handling but

Given the hard real-time requirements of SPI slave, supporting GPIO chip
select may not be feasible.

> that's a separate thing.  Sorry it took me so long to review this.

Thanks for the review!

As I managed to fix the issue with spi_slave_abort() on MSIOF, I think
the remaining
obstacle is the DT binding.  Do you have any feedback or other
suggestions in that area?

IMHO having the ability to bind to an SPI slave handler either from DT or
by using the sysfs virtual file is useful to have.

Thanks again!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC v2 3/7] spi: core: Add support for registering SPI slave controllers

[Mark Brown]

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

On Mon, Dec 19, 2016 at 11:02:07AM +0100, Geert Uytterhoeven wrote:
> On Thu, Dec 15, 2016 at 6:53 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, Sep 12, 2016 at 10:50:42PM +0200, Geert Uytterhoeven wrote:
> >> TBD:
> >>   - s/spi_master/spi_controller/ where appropriate,
> >>   - Provide wrappers (e.g. "#define spi_master spi_controller" until all
> >>     SPI drivers have been converted),
> >>   - Do we want a separate spi_register_slave() instead?

> > This basically looks fine to me - there's these TBDs so I might be
> > missing things and we probably need some GPIO chip select handling but

> Given the hard real-time requirements of SPI slave, supporting GPIO chip
> select may not be feasible.

It's not unknown for SPI devices to have some minimum time requirement
from chip select to first clock so it's workable and probably something
people will want at some point.  It can always be done later though.

> As I managed to fix the issue with spi_slave_abort() on MSIOF, I think
> the remaining
> obstacle is the DT binding.  Do you have any feedback or other
> suggestions in that area?

> IMHO having the ability to bind to an SPI slave handler either from DT or
> by using the sysfs virtual file is useful to have.

I think it'd be useful to have DT support but really I think the DT
maintainers are going to have more opinions on this than me.

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

Applied "spi: Document SPI slave controller support" to the spi tree

[Mark Brown]

The patch

   spi: Document SPI slave controller support

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From aa2ea9115bc3f0735aa65b833076cc5fe3da1489 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Date: Mon, 22 May 2017 15:11:42 +0200
Subject: [PATCH] spi: Document SPI slave controller support

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/spi/spi-summary | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/Documentation/spi/spi-summary b/Documentation/spi/spi-summary
index d1824b399b2d..1721c1b570c3 100644
--- a/Documentation/spi/spi-summary
+++ b/Documentation/spi/spi-summary
@@ -62,8 +62,8 @@ chips described as using "three wire" signaling: SCK, data, nCSx.
 (That data line is sometimes called MOMI or SISO.)
 
 Microcontrollers often support both master and slave sides of the SPI
-protocol.  This document (and Linux) currently only supports the master
-side of SPI interactions.
+protocol.  This document (and Linux) supports both the master and slave
+sides of SPI interactions.
 
 
 Who uses it?  On what kinds of systems?
@@ -154,9 +154,8 @@ control audio interfaces, present touchscreen sensors as input interfaces,
 or monitor temperature and voltage levels during industrial processing.
 And those might all be sharing the same controller driver.
 
-A "struct spi_device" encapsulates the master-side interface between
-those two types of driver.  At this writing, Linux has no slave side
-programming interface.
+A "struct spi_device" encapsulates the controller-side interface between
+those two types of drivers.
 
 There is a minimal core of SPI programming interfaces, focussing on
 using the driver model to connect controller and protocol drivers using
@@ -177,10 +176,24 @@ shows up in sysfs in several locations:
    /sys/bus/spi/drivers/D ... driver for one or more spi*.* devices
 
    /sys/class/spi_master/spiB ... symlink (or actual device node) to
-	a logical node which could hold class related state for the
-	controller managing bus "B".  All spiB.* devices share one
+	a logical node which could hold class related state for the SPI
+	master controller managing bus "B".  All spiB.* devices share one
 	physical SPI bus segment, with SCLK, MOSI, and MISO.
 
+   /sys/devices/.../CTLR/slave ... virtual file for (un)registering the
+	slave device for an SPI slave controller.
+	Writing the driver name of an SPI slave handler to this file
+	registers the slave device; writing "(null)" unregisters the slave
+	device.
+	Reading from this file shows the name of the slave device ("(null)"
+	if not registered).
+
+   /sys/class/spi_slave/spiB ... symlink (or actual device node) to
+	a logical node which could hold class related state for the SPI
+	slave controller on bus "B".  When registered, a single spiB.*
+	device is present here, possible sharing the physical SPI bus
+	segment with other SPI slave devices.
+
 Note that the actual location of the controller's class state depends
 on whether you enabled CONFIG_SYSFS_DEPRECATED or not.  At this time,
 the only class-specific state is the bus number ("B" in "spiB"), so
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html