diff mbox

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

Message ID 1466602929-4191-3-git-send-email-geert+renesas@glider.be (mailing list archive)
State New, archived
Headers show

Commit Message

Geert Uytterhoeven June 22, 2016, 1:42 p.m. UTC
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(-)

Comments

Mark Brown July 18, 2016, 5:02 p.m. UTC | #1
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.
diff mbox

Patch

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