linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/5] *** SPI Slave mode support ***
@ 2017-04-13 12:13 jiada_wang
  2017-04-13 12:14 ` [PATCH RFC 1/5] spi: core: add support to work in Slave mode jiada_wang
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: jiada_wang @ 2017-04-13 12:13 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam
  Cc: linux-spi, devicetree, linux-kernel, linux-arm-kernel, Jiada Wang

From: Jiada Wang <jiada_wang@mentor.com>

v1:
  add Slave mode support in SPI core
  spidev create slave device when SPI controller work in slave mode
  spi-imx support to work in slave mode

Jiada Wang (5):
  spi: core: add support to work in Slave mode
  spi: spidev: use different name for SPI controller slave mode device
  spi: imx: add selection for iMX53 and iMX6 controller
  ARM: dts: imx: change compatiblity for SPI controllers on imx53 later
    soc
  spi: imx: Add support for SPI Slave mode for imx53 and imx6 chips

 Documentation/devicetree/bindings/spi/spi-bus.txt |  27 ++-
 Documentation/spi/spi-summary                     |  19 +-
 arch/arm/boot/dts/imx53.dtsi                      |   4 +-
 arch/arm/boot/dts/imx6q.dtsi                      |   2 +-
 arch/arm/boot/dts/imx6qdl.dtsi                    |   8 +-
 arch/arm/boot/dts/imx6sl.dtsi                     |   8 +-
 arch/arm/boot/dts/imx6sx.dtsi                     |   8 +-
 arch/arm/boot/dts/imx6ul.dtsi                     |   8 +-
 drivers/spi/Kconfig                               |  14 +-
 drivers/spi/spi-imx.c                             | 216 ++++++++++++++++++++--
 drivers/spi/spi.c                                 |  23 ++-
 drivers/spi/spidev.c                              |  15 +-
 include/linux/spi/spi.h                           |  15 ++
 13 files changed, 310 insertions(+), 57 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH RFC 1/5] spi: core: add support to work in Slave mode
  2017-04-13 12:13 [PATCH RFC 0/5] *** SPI Slave mode support *** jiada_wang
@ 2017-04-13 12:14 ` jiada_wang
  2017-04-13 12:14 ` [PATCH RFC 2/5] spi: spidev: use different name for SPI controller slave mode device jiada_wang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: jiada_wang @ 2017-04-13 12:14 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam
  Cc: linux-spi, devicetree, linux-kernel, linux-arm-kernel, Jiada Wang

From: Jiada Wang <jiada_wang@mentor.com>

Add support for SPI bus controller to work in slave mode using
the existing SPI master framework.
- SPI device on SPI bus controller with 'spi-slave' property
  declared in DT node represents SPI controller itself to work
  as a slave device and listening to external SPI master devices
- when SPI bus controller works in slave mode, 'chip_select' and
  'max_speed_hz' are not required.
- SPI slave mode continue to use 'struct spi_master'

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 Documentation/devicetree/bindings/spi/spi-bus.txt | 27 ++++++++++++++---------
 Documentation/spi/spi-summary                     | 19 +++++++++++-----
 drivers/spi/Kconfig                               | 14 +++++++++++-
 drivers/spi/spi.c                                 | 23 ++++++++++++++++++-
 include/linux/spi/spi.h                           | 15 +++++++++++++
 5 files changed, 80 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
index 4b1d6e7..96e93ba 100644
--- a/Documentation/devicetree/bindings/spi/spi-bus.txt
+++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -1,17 +1,20 @@
 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 can work either in master mode or in slave mode, based on the
+child node on it.
 
-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.
+
 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
@@ -43,10 +46,11 @@ 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.
+- 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
@@ -63,6 +67,9 @@ contain the following properties.
                       used for MISO. Defaults to 1 if not present.
 - spi-rx-delay-us  - (optional) Microsecond delay after a read transfer.
 - spi-tx-delay-us  - (optional) Microsecond delay after a write transfer.
+- spi-slave        - (optional) Empty property indicating SPI bus controller
+		itself works in slave mode to interface with external master
+		devices.
 
 Some SPI controllers and devices support Dual and Quad SPI transfer mode.
 It allows data in the SPI system to be transferred using 2 wires (DUAL) or 4
diff --git a/Documentation/spi/spi-summary b/Documentation/spi/spi-summary
index d1824b3..4c2ceaa 100644
--- a/Documentation/spi/spi-summary
+++ b/Documentation/spi/spi-summary
@@ -62,9 +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 +153,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
@@ -168,12 +166,21 @@ shows up in sysfs in several locations:
    /sys/devices/.../CTLR/spiB.C ... spi_device on bus "B",
 	chipselect C, accessed through CTLR.
 
+   /sys/devices/.../CTLR/spiB-slv ... SPI bus "B" controller itself as a
+	spi_device works in slave mode, accessed through CTRL.
+
    /sys/bus/spi/devices/spiB.C ... symlink to that physical
    	.../CTLR/spiB.C device
 
+   /sys/bus/spi/devices/spiB-slv ... symlink to that physical
+	.../CTLR/spiB-slv device
+
    /sys/devices/.../CTLR/spiB.C/modalias ... identifies the driver
 	that should be used with this device (for hotplug/coldplug)
 
+   /sys/devices/.../CTLR/spiB-slv/modalias ... identifies the driver
+	that should be used with this device (for hotplug/coldplug)
+
    /sys/bus/spi/drivers/D ... driver for one or more spi*.* devices
 
    /sys/class/spi_master/spiB ... symlink (or actual device node) to
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 25ae7f2e..1096c7d 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -784,6 +784,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/spi.c b/drivers/spi/spi.c
index 90b5b2e..3af26e2 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -475,6 +475,12 @@ static void spi_dev_set_name(struct spi_device *spi)
 		return;
 	}
 
+	if (spi->slave_mode) {
+		dev_set_name(&spi->dev, "%s-slv",
+			     dev_name(&spi->master->dev));
+		return;
+	}
+
 	dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->master->dev),
 		     spi->chip_select);
 }
@@ -484,6 +490,9 @@ static int spi_dev_check(struct device *dev, void *data)
 	struct spi_device *spi = to_spi_device(dev);
 	struct spi_device *new_spi = data;
 
+	if (spi->slave_mode)
+		return 0;
+
 	if (spi->master == new_spi->master &&
 	    spi->chip_select == new_spi->chip_select)
 		return -EBUSY;
@@ -523,6 +532,9 @@ int spi_add_device(struct spi_device *spi)
 	 */
 	mutex_lock(&spi_add_lock);
 
+	if (spi->slave_mode)
+		goto setup_spi;
+
 	status = bus_for_each_dev(&spi_bus_type, NULL, spi, spi_dev_check);
 	if (status) {
 		dev_err(dev, "chipselect %d already in use\n",
@@ -533,6 +545,7 @@ int spi_add_device(struct spi_device *spi)
 	if (master->cs_gpios)
 		spi->cs_gpio = master->cs_gpios[spi->chip_select];
 
+setup_spi:
 	/* Drivers may modify this initial i/o setup, but will
 	 * normally rely on the device being setup.  Devices
 	 * using SPI_CS_HIGH can't coexist well otherwise...
@@ -1511,6 +1524,14 @@ static int of_spi_parse_dt(struct spi_master *master, struct spi_device *spi,
 	u32 value;
 	int rc;
 
+	if (of_find_property(nc, "spi-slave", NULL)) {
+		if (!spi_controller_has_slavemode(master))
+			return -EINVAL;
+
+		spi->slave_mode = 1;
+		return 0;
+	}
+
 	/* Device address */
 	rc = of_property_read_u32(nc, "reg", &value);
 	if (rc) {
@@ -1961,7 +1982,7 @@ 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),
+	dev_dbg(dev, "registered controller %s%s\n", dev_name(&master->dev),
 			dynamic ? " (dynamic)" : "");
 
 	/* If we're using a queued driver, start the queue */
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 75c6bd0..bb81425 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -115,6 +115,8 @@ void spi_statistics_add_transfer_stats(struct spi_statistics *stats,
  *	This may be changed by the device's driver, or left at the
  *	default (0) indicating protocol words are eight bit bytes.
  *	The spi_transfer.bits_per_word can override this for each transfer.
+ * @slave_mode: indicates whether SPI controller works in master mode
+ *	or slave mode to transfer data with external spi devices.
  * @irq: Negative, or the number passed to request_irq() to receive
  *	interrupts from this device.
  * @controller_state: Controller's runtime state
@@ -144,6 +146,7 @@ struct spi_device {
 	u8			chip_select;
 	u8			bits_per_word;
 	u16			mode;
+	u8			slave_mode;
 #define	SPI_CPHA	0x01			/* clock phase */
 #define	SPI_CPOL	0x02			/* clock polarity */
 #define	SPI_MODE_0	(0|0)			/* (original MicroWire) */
@@ -372,6 +375,7 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  *                    transfer_one callback.
  * @handle_err: the subsystem calls the driver to handle an error that occurs
  *		in the generic implementation of transfer_one_message().
+ * @has_slavemode: checks whether SPI Controller supports slave mode or not.
  * @unprepare_message: undo any work done by prepare_message().
  * @spi_flash_read: to support spi-controller hardwares that provide
  *                  accelerated interface to read from flash devices.
@@ -549,6 +553,7 @@ struct spi_master {
 			    struct spi_transfer *transfer);
 	void (*handle_err)(struct spi_master *master,
 			   struct spi_message *message);
+	bool (*has_slavemode)(struct spi_master *master);
 
 	/* gpio chip select */
 	int			*cs_gpios;
@@ -590,6 +595,16 @@ static inline void spi_master_put(struct spi_master *master)
 		put_device(&master->dev);
 }
 
+
+static inline bool spi_controller_has_slavemode(struct spi_master *master)
+{
+#ifdef CONFIG_SPI_SLAVE
+	if (master->has_slavemode)
+		return master->has_slavemode(master);
+#endif
+	return false;
+}
+
 /* 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);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 2/5] spi: spidev: use different name for SPI controller slave mode device
  2017-04-13 12:13 [PATCH RFC 0/5] *** SPI Slave mode support *** jiada_wang
  2017-04-13 12:14 ` [PATCH RFC 1/5] spi: core: add support to work in Slave mode jiada_wang
@ 2017-04-13 12:14 ` jiada_wang
  2017-04-13 12:14 ` [PATCH RFC 3/5] spi: imx: add selection for iMX53 and iMX6 controller jiada_wang
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: jiada_wang @ 2017-04-13 12:14 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam
  Cc: linux-spi, devicetree, linux-kernel, linux-arm-kernel, Jiada Wang

From: Jiada Wang <jiada_wang@mentor.com>

SPI bus controller has started to support to work in slave mode,
for device SPI controller itself works in slave mode, use name
'spidev[bus]-slv' as its name to differentiate from other
SPI devices

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/spi/spidev.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 9e2e099..e2996fb 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -781,9 +781,18 @@ static int spidev_probe(struct spi_device *spi)
 		struct device *dev;
 
 		spidev->devt = MKDEV(SPIDEV_MAJOR, minor);
-		dev = device_create(spidev_class, &spi->dev, spidev->devt,
-				    spidev, "spidev%d.%d",
-				    spi->master->bus_num, spi->chip_select);
+		if (spi->slave_mode)
+			dev = device_create(spidev_class, &spi->dev,
+					    spidev->devt, spidev,
+					    "spidev%d-slv",
+					    spi->master->bus_num);
+		else
+			dev = device_create(spidev_class, &spi->dev,
+					    spidev->devt, spidev,
+					    "spidev%d.%d",
+					    spi->master->bus_num,
+					    spi->chip_select);
+
 		status = PTR_ERR_OR_ZERO(dev);
 	} else {
 		dev_dbg(&spi->dev, "no minor number available!\n");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 3/5] spi: imx: add selection for iMX53 and iMX6 controller
  2017-04-13 12:13 [PATCH RFC 0/5] *** SPI Slave mode support *** jiada_wang
  2017-04-13 12:14 ` [PATCH RFC 1/5] spi: core: add support to work in Slave mode jiada_wang
  2017-04-13 12:14 ` [PATCH RFC 2/5] spi: spidev: use different name for SPI controller slave mode device jiada_wang
@ 2017-04-13 12:14 ` jiada_wang
  2017-04-13 12:14 ` [PATCH RFC 4/5] ARM: dts: imx: change compatiblity for SPI controllers on imx53 later soc jiada_wang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: jiada_wang @ 2017-04-13 12:14 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam
  Cc: linux-spi, devicetree, linux-kernel, linux-arm-kernel, Jiada Wang

From: Jiada Wang <jiada_wang@mentor.com>

ECSPI contorller for iMX53 and iMX6 has few hardware issues
comparing to iMX51.
The change add possibility to detect which controller is used
to apply possible workaround and limitations.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/spi/spi-imx.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 9a7c62f..b2323b9 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -67,7 +67,8 @@ enum spi_imx_devtype {
 	IMX27_CSPI,
 	IMX31_CSPI,
 	IMX35_CSPI,	/* CSPI on all i.mx except above */
-	IMX51_ECSPI,	/* ECSPI on i.mx51 and later */
+	IMX51_ECSPI,	/* ECSPI on i.mx51 */
+	IMX53_ECSPI,	/* ECSPI on i.mx53 and later */
 };
 
 struct spi_imx_data;
@@ -127,9 +128,32 @@ static inline int is_imx51_ecspi(struct spi_imx_data *d)
 	return d->devtype_data->devtype == IMX51_ECSPI;
 }
 
+static inline int is_imx53_ecspi(struct spi_imx_data *d)
+{
+	return d->devtype_data->devtype == IMX53_ECSPI;
+}
+
 static inline unsigned spi_imx_get_fifosize(struct spi_imx_data *d)
 {
-	return is_imx51_ecspi(d) ? 64 : 8;
+	switch (d->devtype_data->devtype) {
+	case IMX51_ECSPI:
+	case IMX53_ECSPI:
+		return 64;
+	default:
+		return 8;
+	}
+}
+
+static inline bool spi_imx_has_dmamode(struct spi_imx_data *d)
+{
+	switch (d->devtype_data->devtype) {
+	case IMX35_CSPI:
+	case IMX51_ECSPI:
+	case IMX53_ECSPI:
+		return true;
+	default:
+		return false;
+	}
 }
 
 #define MXC_SPI_BUF_RX(type)						\
@@ -746,6 +770,15 @@ static struct spi_imx_devtype_data imx51_ecspi_devtype_data = {
 	.devtype = IMX51_ECSPI,
 };
 
+static struct spi_imx_devtype_data imx53_ecspi_devtype_data = {
+	.intctrl = mx51_ecspi_intctrl,
+	.config = mx51_ecspi_config,
+	.trigger = mx51_ecspi_trigger,
+	.rx_available = mx51_ecspi_rx_available,
+	.reset = mx51_ecspi_reset,
+	.devtype = IMX53_ECSPI,
+};
+
 static const struct platform_device_id spi_imx_devtype[] = {
 	{
 		.name = "imx1-cspi",
@@ -766,6 +799,9 @@ static const struct platform_device_id spi_imx_devtype[] = {
 		.name = "imx51-ecspi",
 		.driver_data = (kernel_ulong_t) &imx51_ecspi_devtype_data,
 	}, {
+		.name = "imx53-ecspi",
+		.driver_data = (kernel_ulong_t) &imx53_ecspi_devtype_data,
+	}, {
 		/* sentinel */
 	}
 };
@@ -777,6 +813,7 @@ static const struct of_device_id spi_imx_dt_ids[] = {
 	{ .compatible = "fsl,imx31-cspi", .data = &imx31_cspi_devtype_data, },
 	{ .compatible = "fsl,imx35-cspi", .data = &imx35_cspi_devtype_data, },
 	{ .compatible = "fsl,imx51-ecspi", .data = &imx51_ecspi_devtype_data, },
+	{ .compatible = "fsl,imx53-ecspi", .data = &imx53_ecspi_devtype_data, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, spi_imx_dt_ids);
@@ -1266,7 +1303,7 @@ static int spi_imx_probe(struct platform_device *pdev)
 	 * Only validated on i.mx35 and i.mx6 now, can remove the constraint
 	 * if validated on other chips.
 	 */
-	if (is_imx35_cspi(spi_imx) || is_imx51_ecspi(spi_imx)) {
+	if (spi_imx_has_dmamode(spi_imx)) {
 		ret = spi_imx_sdma_init(&pdev->dev, spi_imx, master);
 		if (ret == -EPROBE_DEFER)
 			goto out_clk_put;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 4/5] ARM: dts: imx: change compatiblity for SPI controllers on imx53 later soc
  2017-04-13 12:13 [PATCH RFC 0/5] *** SPI Slave mode support *** jiada_wang
                   ` (2 preceding siblings ...)
  2017-04-13 12:14 ` [PATCH RFC 3/5] spi: imx: add selection for iMX53 and iMX6 controller jiada_wang
@ 2017-04-13 12:14 ` jiada_wang
  2017-04-13 12:14 ` [PATCH RFC 5/5] spi: imx: Add support for SPI Slave mode for imx53 and imx6 chips jiada_wang
  2017-04-13 12:59 ` [PATCH RFC 0/5] *** SPI Slave mode support *** Mark Brown
  5 siblings, 0 replies; 20+ messages in thread
From: jiada_wang @ 2017-04-13 12:14 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam
  Cc: linux-spi, devicetree, linux-kernel, linux-arm-kernel, Jiada Wang

From: Jiada Wang <jiada_wang@mentor.com>

for SPI controllers on imx53 and later SoCs, there is HW issue when
work in slave mode, as new device type 'IMX53_ECSPI' has been added
for these SPI controllers which is compatible with 'fsl,imx53-ecspi'.

This patch updates DTS to make imx53 later SPI controller only be
compatibile with 'fsl,imx53-ecspi'.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 arch/arm/boot/dts/imx53.dtsi   | 4 ++--
 arch/arm/boot/dts/imx6q.dtsi   | 2 +-
 arch/arm/boot/dts/imx6qdl.dtsi | 8 ++++----
 arch/arm/boot/dts/imx6sl.dtsi  | 8 ++++----
 arch/arm/boot/dts/imx6sx.dtsi  | 8 ++++----
 arch/arm/boot/dts/imx6ul.dtsi  | 8 ++++----
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 2e516f4..9eeafb9 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -243,7 +243,7 @@
 				ecspi1: ecspi@50010000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx53-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx53-ecspi";
 					reg = <0x50010000 0x4000>;
 					interrupts = <36>;
 					clocks = <&clks IMX5_CLK_ECSPI1_IPG_GATE>,
@@ -662,7 +662,7 @@
 			ecspi2: ecspi@63fac000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-				compatible = "fsl,imx53-ecspi", "fsl,imx51-ecspi";
+				compatible = "fsl,imx53-ecspi";
 				reg = <0x63fac000 0x4000>;
 				interrupts = <37>;
 				clocks = <&clks IMX5_CLK_ECSPI2_IPG_GATE>,
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index e9a5d0b..7373671 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -90,7 +90,7 @@
 				ecspi5: ecspi@02018000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6q-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02018000 0x4000>;
 					interrupts = <0 35 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6Q_CLK_ECSPI5>,
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 6d7bf64..4a4b509 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -265,7 +265,7 @@
 				ecspi1: ecspi@02008000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6q-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02008000 0x4000>;
 					interrupts = <0 31 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6QDL_CLK_ECSPI1>,
@@ -279,7 +279,7 @@
 				ecspi2: ecspi@0200c000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6q-ecspi", "fsl,imx53-ecspi";
 					reg = <0x0200c000 0x4000>;
 					interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6QDL_CLK_ECSPI2>,
@@ -293,7 +293,7 @@
 				ecspi3: ecspi@02010000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6q-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02010000 0x4000>;
 					interrupts = <0 33 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6QDL_CLK_ECSPI3>,
@@ -307,7 +307,7 @@
 				ecspi4: ecspi@02014000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6q-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02014000 0x4000>;
 					interrupts = <0 34 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6QDL_CLK_ECSPI4>,
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index cc9572e..1d1200c 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -168,7 +168,7 @@
 				ecspi1: ecspi@02008000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6sl-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6sl-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02008000 0x4000>;
 					interrupts = <0 31 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6SL_CLK_ECSPI1>,
@@ -180,7 +180,7 @@
 				ecspi2: ecspi@0200c000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6sl-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6sl-ecspi", "fsl,imx53-ecspi";
 					reg = <0x0200c000 0x4000>;
 					interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6SL_CLK_ECSPI2>,
@@ -192,7 +192,7 @@
 				ecspi3: ecspi@02010000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6sl-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6sl-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02010000 0x4000>;
 					interrupts = <0 33 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6SL_CLK_ECSPI3>,
@@ -204,7 +204,7 @@
 				ecspi4: ecspi@02014000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6sl-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6sl-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02014000 0x4000>;
 					interrupts = <0 34 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6SL_CLK_ECSPI4>,
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index dd4ec85..c115ed3 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -251,7 +251,7 @@
 				ecspi1: ecspi@02008000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6sx-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6sx-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02008000 0x4000>;
 					interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6SX_CLK_ECSPI1>,
@@ -263,7 +263,7 @@
 				ecspi2: ecspi@0200c000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6sx-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6sx-ecspi", "fsl,imx53-ecspi";
 					reg = <0x0200c000 0x4000>;
 					interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6SX_CLK_ECSPI2>,
@@ -275,7 +275,7 @@
 				ecspi3: ecspi@02010000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6sx-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6sx-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02010000 0x4000>;
 					interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6SX_CLK_ECSPI3>,
@@ -287,7 +287,7 @@
 				ecspi4: ecspi@02014000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6sx-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6sx-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02014000 0x4000>;
 					interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6SX_CLK_ECSPI4>,
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index b9d7d2d..d35ad5b 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -204,7 +204,7 @@
 				ecspi1: ecspi@02008000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6ul-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6ul-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02008000 0x4000>;
 					interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6UL_CLK_ECSPI1>,
@@ -216,7 +216,7 @@
 				ecspi2: ecspi@0200c000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6ul-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6ul-ecspi", "fsl,imx53-ecspi";
 					reg = <0x0200c000 0x4000>;
 					interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6UL_CLK_ECSPI2>,
@@ -228,7 +228,7 @@
 				ecspi3: ecspi@02010000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6ul-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6ul-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02010000 0x4000>;
 					interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6UL_CLK_ECSPI3>,
@@ -240,7 +240,7 @@
 				ecspi4: ecspi@02014000 {
 					#address-cells = <1>;
 					#size-cells = <0>;
-					compatible = "fsl,imx6ul-ecspi", "fsl,imx51-ecspi";
+					compatible = "fsl,imx6ul-ecspi", "fsl,imx53-ecspi";
 					reg = <0x02014000 0x4000>;
 					interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6UL_CLK_ECSPI4>,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH RFC 5/5] spi: imx: Add support for SPI Slave mode for imx53 and imx6 chips
  2017-04-13 12:13 [PATCH RFC 0/5] *** SPI Slave mode support *** jiada_wang
                   ` (3 preceding siblings ...)
  2017-04-13 12:14 ` [PATCH RFC 4/5] ARM: dts: imx: change compatiblity for SPI controllers on imx53 later soc jiada_wang
@ 2017-04-13 12:14 ` jiada_wang
  2017-04-13 12:59 ` [PATCH RFC 0/5] *** SPI Slave mode support *** Mark Brown
  5 siblings, 0 replies; 20+ messages in thread
From: jiada_wang @ 2017-04-13 12:14 UTC (permalink / raw)
  To: broonie, robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam
  Cc: linux-spi, devicetree, linux-kernel, linux-arm-kernel, Jiada Wang

From: Jiada Wang <jiada_wang@mentor.com>

Previously i.MX SPI controller only works in Master mode.
This patch adds support to work also in Slave mode.

Currently SPI Slave mode support patch has the following limitations:
1. The stale data in RXFIFO will be dropped when the Slave does any new
   transfer.
2. One transfer can be finished only after all transfer->len data been
   transferred to master device
3. Slave device only accepts transfer->len data. Any data longer than this
   from master device will be dropped. Any data shorter than this from
   master will cause SPI to stuck due to mentioned HW limitation 2.
4. Only PIO transfer is supported in Slave mode.

Following HW limitation applies:
1.  ECSPI has a HW issue when works in Slave mode, after 64
    words written to TXFIFO, even TXFIFO becomes empty,
    ECSPI_TXDATA keeps shift out the last word data,
    so we have to disable ECSPI when in slave mode after the
    transfer completes
2.  Due to Freescale errata ERR003775 "eCSPI: Burst completion by Chip
    Select (SS) signal in Slave mode is not functional" burst size must
    be set exactly to the size of the transfer. This limit SPI transaction
    with maximum 2^12 bits.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/spi/spi-imx.c | 173 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 159 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index b2323b9..f6e1baa 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -53,9 +53,13 @@
 /* generic defines to abstract from the different register layouts */
 #define MXC_INT_RR	(1 << 0) /* Receive data ready interrupt */
 #define MXC_INT_TE	(1 << 1) /* Transmit FIFO empty interrupt */
+#define MXC_INT_RDR	BIT(4) /* Receive date threshold interrupt */
 
 /* The maximum  bytes that a sdma BD can transfer.*/
 #define MAX_SDMA_BD_BYTES  (1 << 15)
+/* The maximum bytes that IMX53_ECSPI can transfer in slave mode.*/
+#define MX53_MAX_TRANSFER_BYTES		512
+
 struct spi_imx_config {
 	unsigned int speed_hz;
 	unsigned int bpw;
@@ -79,6 +83,7 @@ struct spi_imx_devtype_data {
 	void (*trigger)(struct spi_imx_data *);
 	int (*rx_available)(struct spi_imx_data *);
 	void (*reset)(struct spi_imx_data *);
+	void (*disable)(struct spi_imx_data *);
 	enum spi_imx_devtype devtype;
 };
 
@@ -104,6 +109,10 @@ struct spi_imx_data {
 	const void *tx_buf;
 	unsigned int txfifo; /* number of words pushed in tx FIFO */
 
+	/* Slave mode */
+	unsigned int slave_mode;
+	unsigned int slave_burst;
+
 	/* DMA */
 	bool usedma;
 	u32 wml;
@@ -156,6 +165,19 @@ static inline bool spi_imx_has_dmamode(struct spi_imx_data *d)
 	}
 }
 
+static bool spi_imx_has_slavemode(struct spi_master *master)
+{
+	struct spi_imx_data *spi_imx = spi_master_get_devdata(master);
+
+	switch (spi_imx->devtype_data->devtype) {
+	case IMX51_ECSPI:
+	case IMX53_ECSPI:
+		return true;
+	default:
+		return false;
+	}
+}
+
 #define MXC_SPI_BUF_RX(type)						\
 static void spi_imx_buf_rx_##type(struct spi_imx_data *spi_imx)		\
 {									\
@@ -285,6 +307,7 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi,
 #define MX51_ECSPI_INT		0x10
 #define MX51_ECSPI_INT_TEEN		(1 <<  0)
 #define MX51_ECSPI_INT_RREN		(1 <<  3)
+#define MX51_ECSPI_INT_RDREN		(1 <<  4)
 
 #define MX51_ECSPI_DMA      0x14
 #define MX51_ECSPI_DMA_TX_WML(wml)	((wml) & 0x3f)
@@ -301,6 +324,51 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi,
 #define MX51_ECSPI_TESTREG	0x20
 #define MX51_ECSPI_TESTREG_LBC	BIT(31)
 
+static void mx53_ecspi_rx_slave(struct spi_imx_data *spi_imx)
+{
+	u32 val = be32_to_cpu(readl(spi_imx->base + MXC_CSPIRXDATA));
+
+	if (spi_imx->rx_buf) {
+		int shift = spi_imx->slave_burst % sizeof(val);
+
+		if (shift) {
+			memcpy(spi_imx->rx_buf,
+			       ((u8 *)&val) + sizeof(val) - shift, shift);
+		} else {
+			*((u32 *)spi_imx->rx_buf) = val;
+			shift = sizeof(val);
+		}
+
+		spi_imx->rx_buf += shift;
+		spi_imx->slave_burst -= shift;
+	}
+}
+
+static void mx53_ecspi_tx_slave(struct spi_imx_data *spi_imx)
+{
+	u32 val = 0;
+	int shift = spi_imx->count % sizeof(val);
+
+	if (spi_imx->tx_buf) {
+		if (shift) {
+			memcpy(((u8 *)&val) + sizeof(val) - shift,
+			       spi_imx->tx_buf, shift);
+		} else {
+			val = *((u32 *)spi_imx->tx_buf);
+			shift = sizeof(val);
+		}
+		val = cpu_to_be32(val);
+		spi_imx->tx_buf += shift;
+	}
+
+	if (!shift)
+		shift = sizeof(val);
+
+	spi_imx->count -= shift;
+
+	writel(val, spi_imx->base + MXC_CSPITXDATA);
+}
+
 /* MX51 eCSPI */
 static unsigned int mx51_ecspi_clkdiv(struct spi_imx_data *spi_imx,
 				      unsigned int fspi, unsigned int *fres)
@@ -350,6 +418,9 @@ static void mx51_ecspi_intctrl(struct spi_imx_data *spi_imx, int enable)
 	if (enable & MXC_INT_RR)
 		val |= MX51_ECSPI_INT_RREN;
 
+	if (enable & MXC_INT_RDR)
+		val |= MX51_ECSPI_INT_RDREN;
+
 	writel(val, spi_imx->base + MX51_ECSPI_INT);
 }
 
@@ -362,6 +433,15 @@ static void mx51_ecspi_trigger(struct spi_imx_data *spi_imx)
 	writel(reg, spi_imx->base + MX51_ECSPI_CTRL);
 }
 
+static void __maybe_unused mx51_ecspi_disable(struct spi_imx_data *spi_imx)
+{
+	u32 ctrl;
+
+	ctrl = readl(spi_imx->base + MX51_ECSPI_CTRL);
+	ctrl &= ~MX51_ECSPI_CTRL_ENABLE;
+	writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL);
+}
+
 static int mx51_ecspi_config(struct spi_device *spi,
 			     struct spi_imx_config *config)
 {
@@ -370,14 +450,13 @@ static int mx51_ecspi_config(struct spi_device *spi,
 	u32 clk = config->speed_hz, delay, reg;
 	u32 cfg = readl(spi_imx->base + MX51_ECSPI_CONFIG);
 
-	/*
-	 * The hardware seems to have a race condition when changing modes. The
-	 * current assumption is that the selection of the channel arrives
-	 * earlier in the hardware than the mode bits when they are written at
-	 * the same time.
-	 * So set master mode for all channels as we do not support slave mode.
-	 */
-	ctrl |= MX51_ECSPI_CTRL_MODE_MASK;
+	writel(0, spi_imx->base + MX51_ECSPI_CTRL);
+
+	/* set Master or Slave mode */
+	if (spi_imx->slave_mode)
+		ctrl &= ~MX51_ECSPI_CTRL_MODE_MASK;
+	else
+		ctrl |= MX51_ECSPI_CTRL_MODE_MASK;
 
 	/* set clock speed */
 	ctrl |= mx51_ecspi_clkdiv(spi_imx, config->speed_hz, &clk);
@@ -386,9 +465,21 @@ static int mx51_ecspi_config(struct spi_device *spi,
 	/* set chip select to use */
 	ctrl |= MX51_ECSPI_CTRL_CS(spi->chip_select);
 
-	ctrl |= (config->bpw - 1) << MX51_ECSPI_CTRL_BL_OFFSET;
+	if (spi_imx->slave_mode && is_imx53_ecspi(spi_imx))
+		ctrl |= (spi_imx->slave_burst * 8 - 1)
+			<< MX51_ECSPI_CTRL_BL_OFFSET;
+	else
+		ctrl |= (config->bpw - 1) << MX51_ECSPI_CTRL_BL_OFFSET;
 
-	cfg |= MX51_ECSPI_CONFIG_SBBCTRL(spi->chip_select);
+	/*
+	 * eCSPI burst completion by Chip Select signal in Slave mode
+	 * is not functional, config SPI burst completed when
+	 * BURST_LENGTH + 1 bits are received
+	 */
+	if (spi_imx->slave_mode && is_imx53_ecspi(spi_imx))
+		cfg &= ~MX51_ECSPI_CONFIG_SBBCTRL(spi->chip_select);
+	else
+		cfg |= MX51_ECSPI_CONFIG_SBBCTRL(spi->chip_select);
 
 	if (spi->mode & SPI_CPHA)
 		cfg |= MX51_ECSPI_CONFIG_SCLKPHA(spi->chip_select);
@@ -767,6 +858,7 @@ static struct spi_imx_devtype_data imx51_ecspi_devtype_data = {
 	.trigger = mx51_ecspi_trigger,
 	.rx_available = mx51_ecspi_rx_available,
 	.reset = mx51_ecspi_reset,
+	.disable = mx51_ecspi_disable,
 	.devtype = IMX51_ECSPI,
 };
 
@@ -776,6 +868,7 @@ static struct spi_imx_devtype_data imx53_ecspi_devtype_data = {
 	.trigger = mx51_ecspi_trigger,
 	.rx_available = mx51_ecspi_rx_available,
 	.reset = mx51_ecspi_reset,
+	.disable = mx51_ecspi_disable,
 	.devtype = IMX53_ECSPI,
 };
 
@@ -838,14 +931,16 @@ static void spi_imx_push(struct spi_imx_data *spi_imx)
 		spi_imx->txfifo++;
 	}
 
-	spi_imx->devtype_data->trigger(spi_imx);
+	if (!spi_imx->slave_mode)
+		spi_imx->devtype_data->trigger(spi_imx);
 }
 
 static irqreturn_t spi_imx_isr(int irq, void *dev_id)
 {
 	struct spi_imx_data *spi_imx = dev_id;
 
-	while (spi_imx->devtype_data->rx_available(spi_imx)) {
+	while (spi_imx->txfifo &&
+	       spi_imx->devtype_data->rx_available(spi_imx)) {
 		spi_imx->rx(spi_imx);
 		spi_imx->txfifo--;
 	}
@@ -927,6 +1022,8 @@ static int spi_imx_setupxfer(struct spi_device *spi,
 	config.bpw = t ? t->bits_per_word : spi->bits_per_word;
 	config.speed_hz  = t ? t->speed_hz : spi->max_speed_hz;
 
+	spi_imx->slave_mode = spi->slave_mode;
+
 	if (!config.speed_hz)
 		config.speed_hz = spi->max_speed_hz;
 	if (!config.bpw)
@@ -944,7 +1041,8 @@ static int spi_imx_setupxfer(struct spi_device *spi,
 		spi_imx->tx = spi_imx_buf_tx_u32;
 	}
 
-	if (spi_imx_can_dma(spi_imx->bitbang.master, spi, t))
+	if (!spi->slave_mode &&
+	    spi_imx_can_dma(spi_imx->bitbang.master, spi, t))
 		spi_imx->usedma = 1;
 	else
 		spi_imx->usedma = 0;
@@ -956,6 +1054,12 @@ static int spi_imx_setupxfer(struct spi_device *spi,
 			return ret;
 	}
 
+	if (is_imx53_ecspi(spi_imx) && spi_imx->slave_mode) {
+		spi_imx->rx = mx53_ecspi_rx_slave;
+		spi_imx->tx = mx53_ecspi_tx_slave;
+		spi_imx->slave_burst = t->len;
+	}
+
 	spi_imx->devtype_data->config(spi, &config);
 
 	return 0;
@@ -1117,16 +1221,46 @@ static int spi_imx_pio_transfer(struct spi_device *spi,
 	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
 	unsigned long transfer_timeout;
 	unsigned long timeout;
+	int ret = transfer->len;
+
+	if (is_imx53_ecspi(spi_imx) && spi_imx->slave_mode &&
+	    transfer->len > MX53_MAX_TRANSFER_BYTES) {
+		pr_err("Transaction too big, max size is %d bytes\n",
+		       MX53_MAX_TRANSFER_BYTES);
+		return -EMSGSIZE;
+	}
 
 	spi_imx->tx_buf = transfer->tx_buf;
 	spi_imx->rx_buf = transfer->rx_buf;
 	spi_imx->count = transfer->len;
 	spi_imx->txfifo = 0;
 
+	if (spi_imx->slave_mode)
+		spi_imx->slave_burst = spi_imx->count;
+
 	reinit_completion(&spi_imx->xfer_done);
 
 	spi_imx_push(spi_imx);
 
+	if (spi_imx->slave_mode) {
+		spi_imx->devtype_data->intctrl(spi_imx, MXC_INT_TE |
+							MXC_INT_RDR);
+
+		if (wait_for_completion_interruptible(&spi_imx->xfer_done) < 0)
+			ret = -EINTR;
+
+		/* ecspi has a HW issue when works in Slave mode,
+		 * after 64 words writtern to TXFIFO, even TXFIFO becomes empty,
+		 * ECSPI_TXDATA keeps shift out the last word data,
+		 * so we have to disable ECSPI when in slave mode after the
+		 * transfer completes
+		 */
+		if (spi_imx->devtype_data->disable)
+			spi_imx->devtype_data->disable(spi_imx);
+
+		goto out;
+	}
+
 	spi_imx->devtype_data->intctrl(spi_imx, MXC_INT_TE);
 
 	transfer_timeout = spi_imx_calculate_timeout(spi_imx, transfer->len);
@@ -1139,7 +1273,8 @@ static int spi_imx_pio_transfer(struct spi_device *spi,
 		return -ETIMEDOUT;
 	}
 
-	return transfer->len;
+out:
+	return ret;
 }
 
 static int spi_imx_transfer(struct spi_device *spi,
@@ -1147,6 +1282,10 @@ static int spi_imx_transfer(struct spi_device *spi,
 {
 	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
 
+	/* flush rxfifo before transfer */
+	while (spi_imx->devtype_data->rx_available(spi_imx))
+		spi_imx->rx(spi_imx);
+
 	if (spi_imx->usedma)
 		return spi_imx_dma_transfer(spi_imx, transfer);
 	else
@@ -1155,6 +1294,11 @@ static int spi_imx_transfer(struct spi_device *spi,
 
 static int spi_imx_setup(struct spi_device *spi)
 {
+	if (spi->slave_mode) {
+		dev_dbg(&spi->dev, "%s: slave mode\n", __func__);
+		return 0;
+	}
+
 	dev_dbg(&spi->dev, "%s: mode %d, %u bpw, %d hz\n", __func__,
 		 spi->mode, spi->bits_per_word, spi->max_speed_hz);
 
@@ -1254,6 +1398,7 @@ static int spi_imx_probe(struct platform_device *pdev)
 	spi_imx->bitbang.master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 	if (is_imx35_cspi(spi_imx) || is_imx51_ecspi(spi_imx))
 		spi_imx->bitbang.master->mode_bits |= SPI_LOOP;
+	spi_imx->bitbang.master->has_slavemode = spi_imx_has_slavemode;
 
 	init_completion(&spi_imx->xfer_done);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-13 12:13 [PATCH RFC 0/5] *** SPI Slave mode support *** jiada_wang
                   ` (4 preceding siblings ...)
  2017-04-13 12:14 ` [PATCH RFC 5/5] spi: imx: Add support for SPI Slave mode for imx53 and imx6 chips jiada_wang
@ 2017-04-13 12:59 ` Mark Brown
  2017-04-13 19:47   ` Geert Uytterhoeven
  5 siblings, 1 reply; 20+ messages in thread
From: Mark Brown @ 2017-04-13 12:59 UTC (permalink / raw)
  To: jiada_wang
  Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam,
	linux-spi, devicetree, linux-kernel, linux-arm-kernel,
	Geert Uytterhoeven

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

On Thu, Apr 13, 2017 at 05:13:59AM -0700, jiada_wang@mentor.com wrote:
> From: Jiada Wang <jiada_wang@mentor.com>
> 
> v1:
>   add Slave mode support in SPI core
>   spidev create slave device when SPI controller work in slave mode
>   spi-imx support to work in slave mode

Adding Geert who also had a series doing this in progress that was
getting very near to being merged.

> 
> Jiada Wang (5):
>   spi: core: add support to work in Slave mode
>   spi: spidev: use different name for SPI controller slave mode device
>   spi: imx: add selection for iMX53 and iMX6 controller
>   ARM: dts: imx: change compatiblity for SPI controllers on imx53 later
>     soc
>   spi: imx: Add support for SPI Slave mode for imx53 and imx6 chips
> 
>  Documentation/devicetree/bindings/spi/spi-bus.txt |  27 ++-
>  Documentation/spi/spi-summary                     |  19 +-
>  arch/arm/boot/dts/imx53.dtsi                      |   4 +-
>  arch/arm/boot/dts/imx6q.dtsi                      |   2 +-
>  arch/arm/boot/dts/imx6qdl.dtsi                    |   8 +-
>  arch/arm/boot/dts/imx6sl.dtsi                     |   8 +-
>  arch/arm/boot/dts/imx6sx.dtsi                     |   8 +-
>  arch/arm/boot/dts/imx6ul.dtsi                     |   8 +-
>  drivers/spi/Kconfig                               |  14 +-
>  drivers/spi/spi-imx.c                             | 216 ++++++++++++++++++++--
>  drivers/spi/spi.c                                 |  23 ++-
>  drivers/spi/spidev.c                              |  15 +-
>  include/linux/spi/spi.h                           |  15 ++
>  13 files changed, 310 insertions(+), 57 deletions(-)
> 
> -- 
> 2.7.4
> 
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-13 12:59 ` [PATCH RFC 0/5] *** SPI Slave mode support *** Mark Brown
@ 2017-04-13 19:47   ` Geert Uytterhoeven
  2017-04-14  5:39     ` Jiada Wang
  0 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2017-04-13 19:47 UTC (permalink / raw)
  To: Mark Brown
  Cc: jiada_wang, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

On Thu, Apr 13, 2017 at 2:59 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Apr 13, 2017 at 05:13:59AM -0700, jiada_wang@mentor.com wrote:
>> From: Jiada Wang <jiada_wang@mentor.com>
>>
>> v1:
>>   add Slave mode support in SPI core
>>   spidev create slave device when SPI controller work in slave mode
>>   spi-imx support to work in slave mode
>
> Adding Geert who also had a series doing this in progress that was
> getting very near to being merged.

Thank you!

Actually my plan is to fix the last remaining issues and resubmit for v4.13.

References:
  - v2: https://lkml.org/lkml/2016/9/12/1065
  - v1: https://lkml.org/lkml/2016/6/22/423

BTW Jiada, what's your use case? Just spidev?

Thx!

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-13 19:47   ` Geert Uytterhoeven
@ 2017-04-14  5:39     ` Jiada Wang
  2017-04-24 10:55       ` Geert Uytterhoeven
  0 siblings, 1 reply; 20+ messages in thread
From: Jiada Wang @ 2017-04-14  5:39 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

Hello Geert

On 04/13/2017 12:47 PM, Geert Uytterhoeven wrote:
> On Thu, Apr 13, 2017 at 2:59 PM, Mark Brown<broonie@kernel.org>  wrote:
>> On Thu, Apr 13, 2017 at 05:13:59AM -0700, jiada_wang@mentor.com wrote:
>>> From: Jiada Wang<jiada_wang@mentor.com>
>>>
>>> v1:
>>>    add Slave mode support in SPI core
>>>    spidev create slave device when SPI controller work in slave mode
>>>    spi-imx support to work in slave mode
>> Adding Geert who also had a series doing this in progress that was
>> getting very near to being merged.
> Thank you!
>
> Actually my plan is to fix the last remaining issues and resubmit for v4.13.
I noticed your patch set for SPI slave support,
(I am sure you can find out some of the change
in this patch set is based on your work).
we have similar requirement to add slave mode support to ecspi IP on 
imx6 Soc.

Our use case is to use spidev as an interface to communicate with 
external SPI master devices.
meanwhile the SPI bus controller can also act as master device to send 
data to other
SPI slave devices on the board.

I found in your implementation, SPI bus controller is limited to either 
work in master mode or
slave mode, is there any reasoning to not configure SPI mode based on 
SPI devices use case?


Thanks,
Jiada

> References:
>    - v2: https://lkml.org/lkml/2016/9/12/1065
>    - v1: https://lkml.org/lkml/2016/6/22/423
>
> BTW Jiada, what's your use case? Just spidev?
>
> Thx!
>
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-14  5:39     ` Jiada Wang
@ 2017-04-24 10:55       ` Geert Uytterhoeven
  2017-04-24 12:48         ` Jiada Wang
  2017-04-25 10:31         ` Mark Brown
  0 siblings, 2 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2017-04-24 10:55 UTC (permalink / raw)
  To: Jiada Wang
  Cc: Mark Brown, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

Hi Jiada,

On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang <jiada_wang@mentor.com> wrote:
> On 04/13/2017 12:47 PM, Geert Uytterhoeven wrote:
>> On Thu, Apr 13, 2017 at 2:59 PM, Mark Brown<broonie@kernel.org>  wrote:
>>> On Thu, Apr 13, 2017 at 05:13:59AM -0700, jiada_wang@mentor.com wrote:
>>>> From: Jiada Wang<jiada_wang@mentor.com>
>>>>
>>>> v1:
>>>>    add Slave mode support in SPI core
>>>>    spidev create slave device when SPI controller work in slave mode
>>>>    spi-imx support to work in slave mode
>>>
>>> Adding Geert who also had a series doing this in progress that was
>>> getting very near to being merged.
>>
>> Thank you!
>>
>> Actually my plan is to fix the last remaining issues and resubmit for
>> v4.13.
>
> I noticed your patch set for SPI slave support,
> (I am sure you can find out some of the change
> in this patch set is based on your work).
> we have similar requirement to add slave mode support to ecspi IP on imx6
> Soc.
>
> Our use case is to use spidev as an interface to communicate with external
> SPI master devices.
> meanwhile the SPI bus controller can also act as master device to send data
> to other
> SPI slave devices on the board.

That sounds a bit hackish to me. SPI was never meant to be a multi-master bus.
While it can be done, you will need external synchronization (signals) to
avoid conflicts between the SPI masters.

> I found in your implementation, SPI bus controller is limited to either work
> in master mode or
> slave mode, is there any reasoning to not configure SPI mode based on SPI
> devices use case?

If you really need both master and slave support, you can use 2 subnodes
in DT, the first representing the master, the second the slave.

Mark, what's your opinion about this?

Thanks!

Gr{oetje,eeting}s,

                        Geert

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

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-24 10:55       ` Geert Uytterhoeven
@ 2017-04-24 12:48         ` Jiada Wang
  2017-04-24 13:10           ` Geert Uytterhoeven
  2017-04-25 10:31         ` Mark Brown
  1 sibling, 1 reply; 20+ messages in thread
From: Jiada Wang @ 2017-04-24 12:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

Hello Geert

On 04/24/2017 03:55 AM, Geert Uytterhoeven wrote:
> Hi Jiada,
>
> On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang<jiada_wang@mentor.com>  wrote:
>> On 04/13/2017 12:47 PM, Geert Uytterhoeven wrote:
>>> On Thu, Apr 13, 2017 at 2:59 PM, Mark Brown<broonie@kernel.org>   wrote:
>>>> On Thu, Apr 13, 2017 at 05:13:59AM -0700, jiada_wang@mentor.com wrote:
>>>>> From: Jiada Wang<jiada_wang@mentor.com>
>>>>>
>>>>> v1:
>>>>>     add Slave mode support in SPI core
>>>>>     spidev create slave device when SPI controller work in slave mode
>>>>>     spi-imx support to work in slave mode
>>>> Adding Geert who also had a series doing this in progress that was
>>>> getting very near to being merged.
>>> Thank you!
>>>
>>> Actually my plan is to fix the last remaining issues and resubmit for
>>> v4.13.
>> I noticed your patch set for SPI slave support,
>> (I am sure you can find out some of the change
>> in this patch set is based on your work).
>> we have similar requirement to add slave mode support to ecspi IP on imx6
>> Soc.
>>
>> Our use case is to use spidev as an interface to communicate with external
>> SPI master devices.
>> meanwhile the SPI bus controller can also act as master device to send data
>> to other
>> SPI slave devices on the board.
> That sounds a bit hackish to me. SPI was never meant to be a multi-master bus.
> While it can be done, you will need external synchronization (signals) to
> avoid conflicts between the SPI masters.
It doesn't need to be a multi-master bus,
for example A is master device for slave device B.
while B has its own slave device C
for each SPI connection A <=> B, and B <=> C, there is only one master 
device.

and I think from use case point of view, it's very normal,
one CPU upon receives command from external SPI master device,
it writes data to its own slave device (EEPROM) connected to it.

Thanks,
Jiada
>> I found in your implementation, SPI bus controller is limited to either work
>> in master mode or
>> slave mode, is there any reasoning to not configure SPI mode based on SPI
>> devices use case?
> If you really need both master and slave support, you can use 2 subnodes
> in DT, the first representing the master, the second the slave.
>
> Mark, what's your opinion about this?
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
>                          Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-24 12:48         ` Jiada Wang
@ 2017-04-24 13:10           ` Geert Uytterhoeven
  2017-04-25  7:56             ` Jiada Wang
  0 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2017-04-24 13:10 UTC (permalink / raw)
  To: Jiada Wang
  Cc: Mark Brown, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

Hi Jiada,

On Mon, Apr 24, 2017 at 2:48 PM, Jiada Wang <jiada_wang@mentor.com> wrote:
> On 04/24/2017 03:55 AM, Geert Uytterhoeven wrote:
>> On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang<jiada_wang@mentor.com>  wrote:
>>> On 04/13/2017 12:47 PM, Geert Uytterhoeven wrote:
>>>> On Thu, Apr 13, 2017 at 2:59 PM, Mark Brown<broonie@kernel.org>   wrote:
>>>>> On Thu, Apr 13, 2017 at 05:13:59AM -0700, jiada_wang@mentor.com wrote:
>>>>>> From: Jiada Wang<jiada_wang@mentor.com>
>>>>>>
>>>>>> v1:
>>>>>>     add Slave mode support in SPI core
>>>>>>     spidev create slave device when SPI controller work in slave mode
>>>>>>     spi-imx support to work in slave mode
>>>>>
>>>>> Adding Geert who also had a series doing this in progress that was
>>>>> getting very near to being merged.
>>>>
>>>> Thank you!
>>>>
>>>> Actually my plan is to fix the last remaining issues and resubmit for
>>>> v4.13.
>>>
>>> I noticed your patch set for SPI slave support,
>>> (I am sure you can find out some of the change
>>> in this patch set is based on your work).
>>> we have similar requirement to add slave mode support to ecspi IP on imx6
>>> Soc.
>>>
>>> Our use case is to use spidev as an interface to communicate with
>>> external
>>> SPI master devices.
>>> meanwhile the SPI bus controller can also act as master device to send
>>> data
>>> to other
>>> SPI slave devices on the board.
>>
>> That sounds a bit hackish to me. SPI was never meant to be a multi-master
>> bus.
>> While it can be done, you will need external synchronization (signals) to
>> avoid conflicts between the SPI masters.
>
> It doesn't need to be a multi-master bus,
> for example A is master device for slave device B.
> while B has its own slave device C
> for each SPI connection A <=> B, and B <=> C, there is only one master
> device.
>
> and I think from use case point of view, it's very normal,
> one CPU upon receives command from external SPI master device,
> it writes data to its own slave device (EEPROM) connected to it.

So "A <=> B" and "B <=> C" are two distinct SPI buses?
Or do they share some signals?

Your comment seems to suggest otherwise:

> > > I found in your implementation, SPI bus controller is limited to either work in master mode or
> > > slave mode, is there any reasoning to not configure SPI mode based on SPI devices use case?

If they are distinct, it should work. Then B has two SPI controllers: one slave
controller controlled by A, and one master controller to control C.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-24 13:10           ` Geert Uytterhoeven
@ 2017-04-25  7:56             ` Jiada Wang
  2017-04-25  8:07               ` Uwe Kleine-König
  2017-04-25  8:09               ` Geert Uytterhoeven
  0 siblings, 2 replies; 20+ messages in thread
From: Jiada Wang @ 2017-04-25  7:56 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

Hi Geert

On 04/24/2017 06:10 AM, Geert Uytterhoeven wrote:
> Hi Jiada,
>
> On Mon, Apr 24, 2017 at 2:48 PM, Jiada Wang<jiada_wang@mentor.com>  wrote:
>> On 04/24/2017 03:55 AM, Geert Uytterhoeven wrote:
>>> On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang<jiada_wang@mentor.com>   wrote:
>>>> On 04/13/2017 12:47 PM, Geert Uytterhoeven wrote:
>>>>> On Thu, Apr 13, 2017 at 2:59 PM, Mark Brown<broonie@kernel.org>    wrote:
>>>>>> On Thu, Apr 13, 2017 at 05:13:59AM -0700, jiada_wang@mentor.com wrote:
>>>>>>> From: Jiada Wang<jiada_wang@mentor.com>
>>>>>>>
>>>>>>> v1:
>>>>>>>      add Slave mode support in SPI core
>>>>>>>      spidev create slave device when SPI controller work in slave mode
>>>>>>>      spi-imx support to work in slave mode
>>>>>> Adding Geert who also had a series doing this in progress that was
>>>>>> getting very near to being merged.
>>>>> Thank you!
>>>>>
>>>>> Actually my plan is to fix the last remaining issues and resubmit for
>>>>> v4.13.
>>>> I noticed your patch set for SPI slave support,
>>>> (I am sure you can find out some of the change
>>>> in this patch set is based on your work).
>>>> we have similar requirement to add slave mode support to ecspi IP on imx6
>>>> Soc.
>>>>
>>>> Our use case is to use spidev as an interface to communicate with
>>>> external
>>>> SPI master devices.
>>>> meanwhile the SPI bus controller can also act as master device to send
>>>> data
>>>> to other
>>>> SPI slave devices on the board.
>>> That sounds a bit hackish to me. SPI was never meant to be a multi-master
>>> bus.
>>> While it can be done, you will need external synchronization (signals) to
>>> avoid conflicts between the SPI masters.
>> It doesn't need to be a multi-master bus,
>> for example A is master device for slave device B.
>> while B has its own slave device C
>> for each SPI connection A<=>  B, and B<=>  C, there is only one master
>> device.
>>
>> and I think from use case point of view, it's very normal,
>> one CPU upon receives command from external SPI master device,
>> it writes data to its own slave device (EEPROM) connected to it.
> So "A<=>  B" and "B<=>  C" are two distinct SPI buses?
> Or do they share some signals?
>
> Your comment seems to suggest otherwise:
the use case of
"A (master) <=> B (slave)", "B (master) <=> C(slave)", do share MISO and 
MOSI lines,
but there is no SS line between A and C. so for each SPI slave device, 
there is only one
master device.

so I think the question becomes whether the above mentioned hardware 
setup is valid or not.

Thanks,
Jiada
>>>> I found in your implementation, SPI bus controller is limited to either work in master mode or
>>>> slave mode, is there any reasoning to not configure SPI mode based on SPI devices use case?
> If they are distinct, it should work. Then B has two SPI controllers: one slave
> controller controlled by A, and one master controller to control C.
>
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-25  7:56             ` Jiada Wang
@ 2017-04-25  8:07               ` Uwe Kleine-König
  2017-04-25  8:09               ` Geert Uytterhoeven
  1 sibling, 0 replies; 20+ messages in thread
From: Uwe Kleine-König @ 2017-04-25  8:07 UTC (permalink / raw)
  To: Jiada Wang
  Cc: Geert Uytterhoeven, Mark Rutland, devicetree, linux-kernel,
	Rob Herring, linux-spi, Mark Brown, Sascha Hauer, Fabio Estevam,
	Shawn Guo, linux-arm-kernel

Hello Jiada,

On Tue, Apr 25, 2017 at 12:56:23AM -0700, Jiada Wang wrote:
> the use case of
> "A (master) <=> B (slave)", "B (master) <=> C(slave)", do share MISO and
> MOSI lines,
> but there is no SS line between A and C. so for each SPI slave device, there
> is only one
> master device.

So you need a mutex to make A not use the bus while B communicates to C.
Otherwise you have two drivers on MOSI (A and B) and MISO (B and C).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-25  7:56             ` Jiada Wang
  2017-04-25  8:07               ` Uwe Kleine-König
@ 2017-04-25  8:09               ` Geert Uytterhoeven
  1 sibling, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2017-04-25  8:09 UTC (permalink / raw)
  To: Jiada Wang
  Cc: Mark Brown, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

Hi Jiada,

On Tue, Apr 25, 2017 at 9:56 AM, Jiada Wang <jiada_wang@mentor.com> wrote:
> On 04/24/2017 06:10 AM, Geert Uytterhoeven wrote:
>> On Mon, Apr 24, 2017 at 2:48 PM, Jiada Wang<jiada_wang@mentor.com>  wrote:
>>> On 04/24/2017 03:55 AM, Geert Uytterhoeven wrote:
>>>> On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang<jiada_wang@mentor.com>
>>>> wrote:
>>>>> Our use case is to use spidev as an interface to communicate with
>>>>> external
>>>>> SPI master devices.
>>>>> meanwhile the SPI bus controller can also act as master device to send
>>>>> data
>>>>> to other
>>>>> SPI slave devices on the board.
>>>>
>>>> That sounds a bit hackish to me. SPI was never meant to be a
>>>> multi-master
>>>> bus.
>>>> While it can be done, you will need external synchronization (signals)
>>>> to
>>>> avoid conflicts between the SPI masters.
>>>
>>> It doesn't need to be a multi-master bus,
>>> for example A is master device for slave device B.
>>> while B has its own slave device C
>>> for each SPI connection A<=>  B, and B<=>  C, there is only one master
>>> device.
>>>
>>> and I think from use case point of view, it's very normal,
>>> one CPU upon receives command from external SPI master device,
>>> it writes data to its own slave device (EEPROM) connected to it.
>>
>> So "A<=>  B" and "B<=>  C" are two distinct SPI buses?
>> Or do they share some signals?
>>
>> Your comment seems to suggest otherwise:
>
> the use case of
> "A (master) <=> B (slave)", "B (master) <=> C(slave)", do share MISO and
> MOSI lines,
> but there is no SS line between A and C. so for each SPI slave device, there
> is only one
> master device.

Do you share CLK, too? Then you need a slave select from B to C.
If you use a separate clock, the slave select from B to C can be optional.

> so I think the question becomes whether the above mentioned hardware setup
> is valid or not.

It's a non-conventional SPI bus setup, but it can work, provided you have
some form of synchronization between A and B.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-24 10:55       ` Geert Uytterhoeven
  2017-04-24 12:48         ` Jiada Wang
@ 2017-04-25 10:31         ` Mark Brown
  2017-04-27  6:43           ` Jiada Wang
  1 sibling, 1 reply; 20+ messages in thread
From: Mark Brown @ 2017-04-25 10:31 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jiada Wang, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

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

On Mon, Apr 24, 2017 at 12:55:21PM +0200, Geert Uytterhoeven wrote:
> On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang <jiada_wang@mentor.com> wrote:

> > Our use case is to use spidev as an interface to communicate with external
> > SPI master devices.
> > meanwhile the SPI bus controller can also act as master device to send data
> > to other
> > SPI slave devices on the board.

> That sounds a bit hackish to me. SPI was never meant to be a multi-master bus.
> While it can be done, you will need external synchronization (signals) to
> avoid conflicts between the SPI masters.

> > I found in your implementation, SPI bus controller is limited to either work
> > in master mode or
> > slave mode, is there any reasoning to not configure SPI mode based on SPI
> > devices use case?

> If you really need both master and slave support, you can use 2 subnodes
> in DT, the first representing the master, the second the slave.

> Mark, what's your opinion about this?

That sounds like a mess...   we *could* put the slave flag on the device
rather than the controller I guess but there's also going to need to be
something representing whatever avoids collisions on the bus somewhere.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-25 10:31         ` Mark Brown
@ 2017-04-27  6:43           ` Jiada Wang
  2017-05-24 17:29             ` Mark Brown
  2017-05-29 12:01             ` Fabio Estevam
  0 siblings, 2 replies; 20+ messages in thread
From: Jiada Wang @ 2017-04-27  6:43 UTC (permalink / raw)
  To: Mark Brown
  Cc: Geert Uytterhoeven, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

Hello Geert and Mark

On 04/25/2017 03:31 AM, Mark Brown wrote:
> On Mon, Apr 24, 2017 at 12:55:21PM +0200, Geert Uytterhoeven wrote:
>> On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang<jiada_wang@mentor.com>  wrote:
>>> Our use case is to use spidev as an interface to communicate with external
>>> SPI master devices.
>>> meanwhile the SPI bus controller can also act as master device to send data
>>> to other
>>> SPI slave devices on the board.
>> That sounds a bit hackish to me. SPI was never meant to be a multi-master bus.
>> While it can be done, you will need external synchronization (signals) to
>> avoid conflicts between the SPI masters.
>>> I found in your implementation, SPI bus controller is limited to either work
>>> in master mode or
>>> slave mode, is there any reasoning to not configure SPI mode based on SPI
>>> devices use case?
>> If you really need both master and slave support, you can use 2 subnodes
>> in DT, the first representing the master, the second the slave.
>> Mark, what's your opinion about this?
> That sounds like a mess...   we *could* put the slave flag on the device
> rather than the controller I guess but there's also going to need to be
> something representing whatever avoids collisions on the bus somewhere.
The reason I gave the example use case is want to point out that
with Geert's patch set, a SPI device (with only one controller) can no 
longer
act as master and slave at the same time. because IMO as a SPI core 
function,
it needs to cover all the use cases, and to be as generic as possible.

BUT if you think the use case don't need to be supported from SPI core,
then I don't have objection either, I will only submit imx SPI slave 
support patch,
after your SPI slave support patch set been applied


Thanks,
Jiada

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-27  6:43           ` Jiada Wang
@ 2017-05-24 17:29             ` Mark Brown
  2017-05-29 12:01             ` Fabio Estevam
  1 sibling, 0 replies; 20+ messages in thread
From: Mark Brown @ 2017-05-24 17:29 UTC (permalink / raw)
  To: Jiada Wang
  Cc: Geert Uytterhoeven, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Fabio Estevam, linux-spi, devicetree, linux-kernel,
	linux-arm-kernel

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

On Wed, Apr 26, 2017 at 11:43:18PM -0700, Jiada Wang wrote:

> The reason I gave the example use case is want to point out that
> with Geert's patch set, a SPI device (with only one controller) can no
> longer
> act as master and slave at the same time. because IMO as a SPI core
> function,
> it needs to cover all the use cases, and to be as generic as possible.

> BUT if you think the use case don't need to be supported from SPI core,
> then I don't have objection either, I will only submit imx SPI slave support
> patch,
> after your SPI slave support patch set been applied

I think there's a bunch of other problems with how that works reliably
that'd need to be addressed before someone could actually do that, and
it seems like such hardware will be very unusual.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-04-27  6:43           ` Jiada Wang
  2017-05-24 17:29             ` Mark Brown
@ 2017-05-29 12:01             ` Fabio Estevam
  2017-05-30  2:53               ` Jiada Wang
  1 sibling, 1 reply; 20+ messages in thread
From: Fabio Estevam @ 2017-05-29 12:01 UTC (permalink / raw)
  To: Jiada Wang
  Cc: Mark Brown, Mark Rutland, devicetree, linux-kernel, linux-spi,
	Rob Herring, Geert Uytterhoeven, Sascha Hauer, Fabio Estevam,
	Shawn Guo, linux-arm-kernel

Hi Jiada,

On Thu, Apr 27, 2017 at 3:43 AM, Jiada Wang <jiada_wang@mentor.com> wrote:

> BUT if you think the use case don't need to be supported from SPI core,
> then I don't have objection either, I will only submit imx SPI slave support
> patch,
> after your SPI slave support patch set been applied

Geert's SPI slave series has been applied.

Do you still plan to send imx spi slave support on top of it?

Thanks

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
  2017-05-29 12:01             ` Fabio Estevam
@ 2017-05-30  2:53               ` Jiada Wang
  0 siblings, 0 replies; 20+ messages in thread
From: Jiada Wang @ 2017-05-30  2:53 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Mark Brown, Mark Rutland, devicetree, linux-kernel, linux-spi,
	Rob Herring, Geert Uytterhoeven, Sascha Hauer, Fabio Estevam,
	Shawn Guo, linux-arm-kernel

Hi Fabio

On 05/29/2017 05:01 AM, Fabio Estevam wrote:
> Hi Jiada,
>
> On Thu, Apr 27, 2017 at 3:43 AM, Jiada Wang<jiada_wang@mentor.com>  wrote:
>
>> BUT if you think the use case don't need to be supported from SPI core,
>> then I don't have objection either, I will only submit imx SPI slave support
>> patch,
>> after your SPI slave support patch set been applied
> Geert's SPI slave series has been applied.
>
> Do you still plan to send imx spi slave support on top of it?
Yes, I am working on it

Thanks,
Jiada
> Thanks

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-05-30  2:53 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-13 12:13 [PATCH RFC 0/5] *** SPI Slave mode support *** jiada_wang
2017-04-13 12:14 ` [PATCH RFC 1/5] spi: core: add support to work in Slave mode jiada_wang
2017-04-13 12:14 ` [PATCH RFC 2/5] spi: spidev: use different name for SPI controller slave mode device jiada_wang
2017-04-13 12:14 ` [PATCH RFC 3/5] spi: imx: add selection for iMX53 and iMX6 controller jiada_wang
2017-04-13 12:14 ` [PATCH RFC 4/5] ARM: dts: imx: change compatiblity for SPI controllers on imx53 later soc jiada_wang
2017-04-13 12:14 ` [PATCH RFC 5/5] spi: imx: Add support for SPI Slave mode for imx53 and imx6 chips jiada_wang
2017-04-13 12:59 ` [PATCH RFC 0/5] *** SPI Slave mode support *** Mark Brown
2017-04-13 19:47   ` Geert Uytterhoeven
2017-04-14  5:39     ` Jiada Wang
2017-04-24 10:55       ` Geert Uytterhoeven
2017-04-24 12:48         ` Jiada Wang
2017-04-24 13:10           ` Geert Uytterhoeven
2017-04-25  7:56             ` Jiada Wang
2017-04-25  8:07               ` Uwe Kleine-König
2017-04-25  8:09               ` Geert Uytterhoeven
2017-04-25 10:31         ` Mark Brown
2017-04-27  6:43           ` Jiada Wang
2017-05-24 17:29             ` Mark Brown
2017-05-29 12:01             ` Fabio Estevam
2017-05-30  2:53               ` Jiada Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).