Re: [PATCH v5 6/6] spi: slave: Add SPI slave handler controlling system state

[Andy Shevchenko]

On Mon, May 22, 2017 at 4:11 PM, Geert Uytterhoeven
 wrote:
> Add an example SPI slave handler to allow remote control of system
> reboot, power off, halt, and suspend.
>

FWIW:
Reviewed-by: Andy Shevchenko 

> Signed-off-by: Geert Uytterhoeven 
> ---
> v5:
>   - Add usage documentation to file header,
>   - Use network byte order for commands, to match the "-p" parameter of
> spidev_test,
>   - Replace pr_*() by dev_*(), stop printing __func__,
>   - Remove spi_setup() call to configure 8 bits per word, as that's the
> default,
>
> v3, v4:
>   - No changes,
>
> 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 9972ee2a8454a2fc..82cd818aa06293f5 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -803,6 +803,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 fb078693dbe40da4..1d7923e8c63bc22b 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -108,3 +108,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 ..c0257e937995ec53
> --- /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-2017 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.
> + *
> + * Usage (assuming /dev/spidev2.0 corresponds to the SPI master on the remote
> + * system):
> + *
> + *   # reboot='\x7c\x50'
> + *   # poweroff='\x71\x3f'
> + *   # halt='\x38\x76'
> + *   # suspend='\x1b\x1b'
> + *   # spidev_test -D /dev/spidev2.0 -p $suspend # or $reboot, $poweroff, 
> $halt
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * The numbers are chosen to display something human-readable on two 
> 7-segment
> + * displays connected to two 74HC595 shift registers
> + */
> +#define CMD_REBOOT 0x7c50  /* rb */
> +#define CMD_POWEROFF   0x713f  /* OF */
> +#define CMD_HALT   0x3876  /* HL */
> +#define CMD_SUSPEND0x1b1b  /* ZZ */
> +
> +struct spi_slave_system_control_priv {
> +   struct spi_device *spi;
> +   struct completion finished;
> +   struct spi_transfer xfer;
> +   struct spi_message msg;
> +   __be16 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 = be16_to_cpu(priv->cmd);
> +   switch (cmd) {
> +   case CMD_REBOOT:
> +   dev_info(>spi->dev, "Rebooting system...\n");
> +   kernel_restart(NULL);
> +
> +   case CMD_POWEROFF:
> +   dev_info(>spi->dev, "Powering off system...\n");
> +   kernel_power_off();
> +   break;
> +
> +   case CMD_HALT:
> +   dev_info(>spi->dev, "Halting system...\n");
> +   kernel_halt();
> +   break;
> +
> +   case CMD_SUSPEND:
> +   dev_info(>spi->dev, "Suspending system...\n");
> +   pm_suspend(PM_SUSPEND_MEM);
> +   break;
> +
> +   default:
> +   

[PATCH v5 6/6] 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 
---
v5:
  - Add usage documentation to file header,
  - Use network byte order for commands, to match the "-p" parameter of
spidev_test,
  - Replace pr_*() by dev_*(), stop printing __func__,
  - Remove spi_setup() call to configure 8 bits per word, as that's the
default,

v3, v4:
  - No changes,

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 9972ee2a8454a2fc..82cd818aa06293f5 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -803,6 +803,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 fb078693dbe40da4..1d7923e8c63bc22b 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -108,3 +108,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 ..c0257e937995ec53
--- /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-2017 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.
+ *
+ * Usage (assuming /dev/spidev2.0 corresponds to the SPI master on the remote
+ * system):
+ *
+ *   # reboot='\x7c\x50'
+ *   # poweroff='\x71\x3f'
+ *   # halt='\x38\x76'
+ *   # suspend='\x1b\x1b'
+ *   # spidev_test -D /dev/spidev2.0 -p $suspend # or $reboot, $poweroff, $halt
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * The numbers are chosen to display something human-readable on two 7-segment
+ * displays connected to two 74HC595 shift registers
+ */
+#define CMD_REBOOT 0x7c50  /* rb */
+#define CMD_POWEROFF   0x713f  /* OF */
+#define CMD_HALT   0x3876  /* HL */
+#define CMD_SUSPEND0x1b1b  /* ZZ */
+
+struct spi_slave_system_control_priv {
+   struct spi_device *spi;
+   struct completion finished;
+   struct spi_transfer xfer;
+   struct spi_message msg;
+   __be16 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 = be16_to_cpu(priv->cmd);
+   switch (cmd) {
+   case CMD_REBOOT:
+   dev_info(>spi->dev, "Rebooting system...\n");
+   kernel_restart(NULL);
+
+   case CMD_POWEROFF:
+   dev_info(>spi->dev, "Powering off system...\n");
+   kernel_power_off();
+   break;
+
+   case CMD_HALT:
+   dev_info(>spi->dev, "Halting system...\n");
+   kernel_halt();
+   break;
+
+   case CMD_SUSPEND:
+   dev_info(>spi->dev, "Suspending system...\n");
+   pm_suspend(PM_SUSPEND_MEM);
+   break;
+
+   default:
+   dev_warn(>spi->dev, "Unknown command 0x%x\n", cmd);
+   break;
+   }
+
+   ret = spi_slave_system_control_submit(priv);
+   if (ret)
+   goto terminate;
+
+   return;
+
+terminate:
+   dev_info(>spi->dev, "Terminating\n");
+   complete(>finished);
+}
+
+static
+int spi_slave_system_control_submit(struct spi_slave_system_control_priv *priv)
+{
+   int ret;
+
+