linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Add support for the Armada 3700 SPI controller
@ 2016-12-01 10:27 Romain Perier
       [not found] ` <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

The Marvell Armada 3700 SoC includes an SPI controller. This controller
supports up to 4 SPI slave devices, with dedicated chip selects, CPIO or
FIFO mode with DMA or CPU transfers and different SPI transfer modes
(Standard single, Dual or Quad).

This set of patches adds a basic support for the CPIO mode, then it
enables the FIFO mode (CPU-side only, DMA not supported yet). It also
adds the required definitions of the spi nodes to the devicetree.

Romain Perier (5):
  spi: Add basic support for Armada 3700 SPI Controller
  spi: armada-3700: Add support for the FIFO mode
  spi: armada-3700: Add documentation for the Armada 3700 SPI Controller
  arm64: dts: marvell: Add definition of SPI controller for Armada 3700
  arm64: dts: marvell: Enable spi0 on the board Armada-3720-db

 .../devicetree/bindings/spi/spi-armada-3700.txt    |   25 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts     |   30 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |   11 +
 drivers/spi/Kconfig                                |    7 +
 drivers/spi/Makefile                               |    1 +
 drivers/spi/spi-armada-3700.c                      | 1046 ++++++++++++++++++++
 6 files changed, 1120 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt
 create mode 100644 drivers/spi/spi-armada-3700.c

-- 
2.9.3

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

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

* [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller
       [not found] ` <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
@ 2016-12-01 10:27   ` Romain Perier
  2016-12-05 12:05     ` Mark Brown
  2016-12-01 10:27   ` [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode Romain Perier
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

Marvell Armada 3700 SoC comprises an SPI Controller. This Controller
supports up to 4 SPI slave devices, with dedicated chip selects, supports
SPI mode 0/1/2 and 3, CPIO or Fifo mode with DMA transfers and different
SPI transfer mode (Single, Dual or Quad).

This commit adds basic driver support for CPIO mode and single SPI
transfer. In this mode, the CPU asserts cs, outputs or inputs data from
the current SPI device. Data transfers are copied by 1 or 4 bytes using
the SPI registers.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---

Changes in v3:
 - Fixed wrong variable passed as MODULE_DEVICE_TABLE
 - Added missing null terminated entry in a3700_spi_dt_ids
 - Added the tag "Tested-by" by Gregory

 drivers/spi/Kconfig           |   7 +
 drivers/spi/Makefile          |   1 +
 drivers/spi/spi-armada-3700.c | 652 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 660 insertions(+)
 create mode 100644 drivers/spi/spi-armada-3700.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b799547..6ade1ca 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -67,6 +67,13 @@ config SPI_ATH79
 	  This enables support for the SPI controller present on the
 	  Atheros AR71XX/AR724X/AR913X SoCs.
 
+config SPI_ARMADA_3700
+	tristate "Marvell Armada 3700 SPI Controller"
+	depends on ARCH_MVEBU && OF
+	help
+	  This enables support for the SPI controller present on the
+	  Marvell Armada 3700 SoCs.
+
 config SPI_ATMEL
 	tristate "Atmel SPI Controller"
 	depends on HAS_DMA
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index aa939d9..140ca45 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 
 # SPI master controller drivers (bus)
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
+obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
new file mode 100644
index 0000000..4115685
--- /dev/null
+++ b/drivers/spi/spi-armada-3700.c
@@ -0,0 +1,652 @@
+/*
+ * Marvell Armada-3700 SPI controller driver
+ *
+ * Copyright (C) 2016 Marvell Ltd.
+ *
+ * Author: Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
+ * Author: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * 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
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/spi/spi.h>
+
+#define DRIVER_NAME			"armada_3700_spi"
+
+#define A3700_SPI_TIMEOUT		10
+
+/* SPI Register Offest */
+#define A3700_SPI_IF_CTRL_REG		0x00
+#define A3700_SPI_IF_CFG_REG		0x04
+#define A3700_SPI_DATA_OUT_REG		0x08
+#define A3700_SPI_DATA_IN_REG		0x0C
+#define A3700_SPI_IF_INST_REG		0x10
+#define A3700_SPI_IF_ADDR_REG		0x14
+#define A3700_SPI_IF_RMODE_REG		0x18
+#define A3700_SPI_IF_HDR_CNT_REG	0x1C
+#define A3700_SPI_IF_DIN_CNT_REG	0x20
+#define A3700_SPI_IF_TIME_REG		0x24
+#define A3700_SPI_INT_STAT_REG		0x28
+#define A3700_SPI_INT_MASK_REG		0x2C
+
+/* A3700_SPI_IF_CTRL_REG */
+#define A3700_SPI_EN			BIT(16)
+#define A3700_SPI_ADDR_NOT_CONFIG	BIT(12)
+#define A3700_SPI_WFIFO_OVERFLOW	BIT(11)
+#define A3700_SPI_WFIFO_UNDERFLOW	BIT(10)
+#define A3700_SPI_RFIFO_OVERFLOW	BIT(9)
+#define A3700_SPI_RFIFO_UNDERFLOW	BIT(8)
+#define A3700_SPI_WFIFO_FULL		BIT(7)
+#define A3700_SPI_WFIFO_EMPTY		BIT(6)
+#define A3700_SPI_RFIFO_FULL		BIT(5)
+#define A3700_SPI_RFIFO_EMPTY		BIT(4)
+#define A3700_SPI_WFIFO_RDY		BIT(3)
+#define A3700_SPI_RFIFO_RDY		BIT(2)
+#define A3700_SPI_XFER_RDY		BIT(1)
+#define A3700_SPI_XFER_DONE		BIT(0)
+
+/* A3700_SPI_IF_CFG_REG */
+#define A3700_SPI_WFIFO_THRS		BIT(28)
+#define A3700_SPI_RFIFO_THRS		BIT(24)
+#define A3700_SPI_AUTO_CS		BIT(20)
+#define A3700_SPI_DMA_RD_EN		BIT(18)
+#define A3700_SPI_FIFO_MODE		BIT(17)
+#define A3700_SPI_SRST			BIT(16)
+#define A3700_SPI_XFER_START		BIT(15)
+#define A3700_SPI_XFER_STOP		BIT(14)
+#define A3700_SPI_INST_PIN		BIT(13)
+#define A3700_SPI_ADDR_PIN		BIT(12)
+#define A3700_SPI_DATA_PIN1		BIT(11)
+#define A3700_SPI_DATA_PIN0		BIT(10)
+#define A3700_SPI_FIFO_FLUSH		BIT(9)
+#define A3700_SPI_RW_EN			BIT(8)
+#define A3700_SPI_CLK_POL		BIT(7)
+#define A3700_SPI_CLK_PHA		BIT(6)
+#define A3700_SPI_BYTE_LEN		BIT(5)
+#define A3700_SPI_CLK_PRESCALE		BIT(0)
+#define A3700_SPI_CLK_PRESCALE_MASK	(0x1f)
+
+#define A3700_SPI_WFIFO_THRS_BIT	28
+#define A3700_SPI_RFIFO_THRS_BIT	24
+#define A3700_SPI_FIFO_THRS_MASK	0x7
+
+#define A3700_SPI_DATA_PIN_MASK		0x3
+
+/* A3700_SPI_IF_HDR_CNT_REG */
+#define A3700_SPI_DUMMY_CNT_BIT		12
+#define A3700_SPI_DUMMY_CNT_MASK	0x7
+#define A3700_SPI_RMODE_CNT_BIT		8
+#define A3700_SPI_RMODE_CNT_MASK	0x3
+#define A3700_SPI_ADDR_CNT_BIT		4
+#define A3700_SPI_ADDR_CNT_MASK		0x7
+#define A3700_SPI_INSTR_CNT_BIT		0
+#define A3700_SPI_INSTR_CNT_MASK	0x3
+
+/* A3700_SPI_IF_TIME_REG */
+#define A3700_SPI_CLK_CAPT_EDGE		BIT(7)
+
+struct a3700_spi {
+	struct spi_master *master;
+	void __iomem *base;
+	struct clk *clk;
+	unsigned int irq;
+	unsigned int flags;
+	bool last_xfer;
+	const u8 *tx_buf;
+	u8 *rx_buf;
+	size_t buf_len;
+	u8 byte_len;
+	u32 wait_mask;
+	struct completion done;
+};
+
+static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
+{
+	return readl(a3700_spi->base + offset);
+}
+
+static void spireg_write(struct a3700_spi *a3700_spi, u32 offset, u32 data)
+{
+	writel(data, a3700_spi->base + offset);
+}
+
+static void a3700_spi_auto_cs_unset(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_AUTO_CS;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_activate_cs(struct a3700_spi *a3700_spi, unsigned int cs)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	val |= (A3700_SPI_EN << cs);
+	spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static void a3700_spi_deactivate_cs(struct a3700_spi *a3700_spi,
+				    unsigned int cs)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	val &= ~(A3700_SPI_EN << cs);
+	spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
+				  unsigned int pin_mode)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~(A3700_SPI_INST_PIN | A3700_SPI_ADDR_PIN);
+	val &= ~(A3700_SPI_DATA_PIN0 | A3700_SPI_DATA_PIN1);
+
+	switch (pin_mode) {
+	case 1:
+		break;
+	case 2:
+		val |= A3700_SPI_DATA_PIN0;
+		break;
+	case 4:
+		val |= A3700_SPI_DATA_PIN1;
+		break;
+	default:
+		dev_err(&a3700_spi->master->dev, "wrong pin mode %u", pin_mode);
+		return -EINVAL;
+	}
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	return 0;
+}
+
+static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_FIFO_MODE;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_mode_set(struct a3700_spi *a3700_spi,
+			       unsigned int mode_bits)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+
+	if (mode_bits & SPI_CPOL)
+		val |= A3700_SPI_CLK_POL;
+	else
+		val &= ~A3700_SPI_CLK_POL;
+
+	if (mode_bits & SPI_CPHA)
+		val |= A3700_SPI_CLK_PHA;
+	else
+		val &= ~A3700_SPI_CLK_PHA;
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_clock_set(struct a3700_spi *a3700_spi,
+				unsigned int speed_hz, u16 mode)
+{
+	u32 val;
+	u32 prescale;
+
+	prescale = DIV_ROUND_UP(clk_get_rate(a3700_spi->clk), speed_hz);
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val = val & ~A3700_SPI_CLK_PRESCALE_MASK;
+
+	val = val | (prescale & A3700_SPI_CLK_PRESCALE_MASK);
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	if (prescale <= 2) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_TIME_REG);
+		val |= A3700_SPI_CLK_CAPT_EDGE;
+		spireg_write(a3700_spi, A3700_SPI_IF_TIME_REG, val);
+	}
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~(A3700_SPI_CLK_POL | A3700_SPI_CLK_PHA);
+
+	if (mode & SPI_CPOL)
+		val |= A3700_SPI_CLK_POL;
+
+	if (mode & SPI_CPHA)
+		val |= A3700_SPI_CLK_PHA;
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	if (len == 4)
+		val |= A3700_SPI_BYTE_LEN;
+	else
+		val &= ~A3700_SPI_BYTE_LEN;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	a3700_spi->byte_len = len;
+}
+
+static int a3700_spi_init(struct a3700_spi *a3700_spi)
+{
+	struct spi_master *master = a3700_spi->master;
+	u32 val;
+	int i;
+
+	/* Reset SPI unit */
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_SRST;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	for (i = 0; i < A3700_SPI_TIMEOUT; i++)
+		udelay(1);
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_SRST;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	/* Disable AUTO_CS and deactivate all chip-selects */
+	a3700_spi_auto_cs_unset(a3700_spi);
+	for (i = 0; i < master->num_chipselect; i++)
+		a3700_spi_deactivate_cs(a3700_spi, i);
+
+	a3700_spi_pin_mode_set(a3700_spi, 0);
+
+	/* Be sure that FIFO mode is disabled */
+	a3700_spi_fifo_mode_unset(a3700_spi);
+
+	/* Set SPI mode */
+	a3700_spi_mode_set(a3700_spi, master->mode_bits);
+
+	/* Reset counters */
+	spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG, 0);
+
+	/* Mask the interrupts and clear cause bits */
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
+
+	return 0;
+}
+
+static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
+{
+	struct spi_master *master = dev_id;
+	struct a3700_spi *a3700_spi;
+	u32 cause;
+
+	a3700_spi = spi_master_get_devdata(master);
+
+	/* Get interrupt causes */
+	cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
+
+	/* mask and acknowledge the SPI interrupts */
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
+
+	/* Wake up the transfer */
+	if (a3700_spi->wait_mask & cause)
+		complete(&a3700_spi->done);
+
+	return IRQ_HANDLED;
+}
+
+static bool a3700_spi_wait_completion(struct spi_device *spi)
+{
+	struct a3700_spi *a3700_spi;
+	unsigned int timeout;
+	unsigned int ctrl_reg;
+	unsigned long timeout_jiffies;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+
+	/* SPI interrupt is edge-triggered, which means an interrupt will
+	 * be generated only when detecting a specific status bit changed
+	 * from '0' to '1'. So when we start waiting for a interrupt, we
+	 * need to check status bit in control reg first, if it is already 1,
+	 * then we do not need to wait for interrupt
+	 */
+	ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	if (a3700_spi->wait_mask & ctrl_reg)
+		return true;
+
+	reinit_completion(&a3700_spi->done);
+
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG,
+		     a3700_spi->wait_mask);
+
+	timeout_jiffies = msecs_to_jiffies(A3700_SPI_TIMEOUT);
+	timeout = wait_for_completion_timeout(&a3700_spi->done,
+					      timeout_jiffies);
+
+	a3700_spi->wait_mask = 0;
+
+	if (timeout)
+		return true;
+
+	/* there might be the case that right after we checked the
+	 * status bits in this routine and before start to wait for
+	 * interrupt by wait_for_completion_timeout, the interrupt
+	 * happens, to avoid missing it we need to double check
+	 * status bits in control reg, if it is already 1, then
+	 * consider that we have the interrupt successfully and
+	 * return true.
+	 */
+	ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	if (a3700_spi->wait_mask & ctrl_reg)
+		return true;
+
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+
+	return true;
+}
+
+static bool a3700_spi_transfer_wait(struct spi_device *spi,
+				    unsigned int bit_mask)
+{
+	struct a3700_spi *a3700_spi;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+	a3700_spi->wait_mask = bit_mask;
+
+	return a3700_spi_wait_completion(spi);
+}
+
+static void a3700_spi_transfer_setup(struct spi_device *spi,
+				    struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+
+	a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+}
+
+static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
+{
+	u32 val, data;
+
+	if (a3700_spi->buf_len % a3700_spi->byte_len)
+		return -EINVAL;
+
+	/* Read bytes from data in register */
+	val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+
+	if (a3700_spi->byte_len == 4)
+		data = be32_to_cpu(val);
+	else
+		data = val;
+
+	memcpy(a3700_spi->rx_buf, &data, a3700_spi->byte_len);
+
+	a3700_spi->buf_len -= a3700_spi->byte_len;
+	a3700_spi->rx_buf  += a3700_spi->byte_len;
+
+	/* Request next 1 or 4 bytes data */
+	if (a3700_spi->buf_len)
+		spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+	return 0;
+}
+
+static int a3700_spi_write_data(struct a3700_spi *a3700_spi)
+{
+	u32 val = 0;
+
+	if (a3700_spi->buf_len % a3700_spi->byte_len)
+		return -EINVAL;
+
+	/* Write bytes from data out register */
+	if (a3700_spi->byte_len == 4)
+		val = cpu_to_be32(*(u32 *)a3700_spi->tx_buf);
+	else
+		val = a3700_spi->tx_buf[0];
+
+	spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+	a3700_spi->buf_len -= a3700_spi->byte_len;
+	a3700_spi->tx_buf  += a3700_spi->byte_len;
+
+	return 0;
+}
+
+static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(spi->master);
+
+	if (!enable)
+		a3700_spi_activate_cs(a3700_spi, spi->chip_select);
+	else
+		a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
+}
+
+static int a3700_spi_prepare_message(struct spi_master *master,
+				     struct spi_message *message)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	struct spi_device *spi = message->spi;
+
+	a3700_spi_bytelen_set(a3700_spi, 1);
+
+	if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+		dev_err(&spi->dev, "wait transfer ready timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int a3700_spi_transfer_one(struct spi_master *master,
+				  struct spi_device *spi,
+				  struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret = 0;
+
+	a3700_spi_transfer_setup(spi, xfer);
+
+	a3700_spi->tx_buf  = xfer->tx_buf;
+	a3700_spi->rx_buf  = xfer->rx_buf;
+	a3700_spi->buf_len = xfer->len;
+
+	/* Start READ transfer by writing dummy data to DOUT register */
+	if (xfer->rx_buf)
+		spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+	while (a3700_spi->buf_len) {
+		if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+			dev_err(&spi->dev, "wait transfer ready timed out\n");
+			ret = -ETIMEDOUT;
+			goto err;
+		}
+
+		if (a3700_spi->tx_buf) {
+			ret = a3700_spi_write_data(a3700_spi);
+			if (ret)
+				goto err;
+		}
+
+		if (a3700_spi->rx_buf) {
+			ret = a3700_spi_read_data(a3700_spi);
+			if (ret)
+				goto err;
+		}
+	}
+
+err:
+	spi_finalize_current_transfer(master);
+	return ret;
+}
+
+static int a3700_spi_unprepare_message(struct spi_master *master,
+				       struct spi_message *message)
+{
+	struct spi_device *spi = message->spi;
+
+	if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+		dev_err(&spi->dev, "wait transfer ready timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id a3700_spi_dt_ids[] = {
+	{ .compatible = "marvell,armada-3700-spi", .data = NULL },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, a3700_spi_dt_ids);
+
+static int a3700_spi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *of_node = dev->of_node;
+	struct resource *res;
+	struct spi_master *master;
+	struct a3700_spi *spi;
+	u32 num_cs = 0;
+	int ret = 0;
+
+	master = spi_alloc_master(dev, sizeof(*spi));
+	if (!master) {
+		dev_err(dev, "master allocation failed\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	if (of_property_read_u32(of_node, "num-cs", &num_cs)) {
+		dev_err(dev, "could not find num-cs\n");
+		ret = -ENXIO;
+		goto error;
+	}
+
+	master->bus_num = (pdev->id != -1) ? pdev->id : 0;
+	master->dev.of_node = of_node;
+	master->mode_bits = SPI_MODE_3;
+	master->num_chipselect = num_cs;
+	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(32);
+	master->prepare_message =  a3700_spi_prepare_message;
+	master->transfer_one = a3700_spi_transfer_one;
+	master->unprepare_message = a3700_spi_unprepare_message;
+	master->set_cs = a3700_spi_set_cs;
+
+	platform_set_drvdata(pdev, master);
+
+	spi = spi_master_get_devdata(master);
+	memset(spi, 0, sizeof(struct a3700_spi));
+
+	spi->master = master;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	spi->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(spi->base)) {
+		ret = PTR_ERR(spi->base);
+		goto error;
+	}
+
+	spi->irq = platform_get_irq(pdev, 0);
+	if (spi->irq < 0) {
+		dev_err(dev, "could not get irq: %d\n", spi->irq);
+		ret = -ENXIO;
+		goto error;
+	}
+
+	init_completion(&spi->done);
+
+	spi->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(spi->clk)) {
+		dev_err(dev, "could not find clk: %ld\n", PTR_ERR(spi->clk));
+		goto error;
+	}
+
+	ret = clk_prepare_enable(spi->clk);
+	if (ret) {
+		dev_err(dev, "could not prepare clk: %d\n", ret);
+		goto error;
+	}
+
+	ret = a3700_spi_init(spi);
+	if (ret)
+		goto error_clk;
+
+	ret = devm_request_irq(dev, spi->irq, a3700_spi_interrupt, 0,
+			       dev_name(dev), master);
+	if (ret) {
+		dev_err(dev, "could not request IRQ: %d\n", ret);
+		goto error_clk;
+	}
+
+	ret = devm_spi_register_master(dev, master);
+	if (ret) {
+		dev_err(dev, "Failed to register master\n");
+		goto error_clk;
+	}
+
+	dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
+		 (unsigned long)res->start, spi->irq);
+
+	return 0;
+
+error_clk:
+	clk_disable_unprepare(spi->clk);
+error:
+	spi_master_put(master);
+out:
+	return ret;
+}
+
+static int a3700_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct a3700_spi *spi = spi_master_get_devdata(master);
+
+	clk_disable_unprepare(spi->clk);
+	spi_master_put(master);
+
+	return 0;
+}
+
+static struct platform_driver a3700_spi_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(a3700_spi_dt_ids),
+	},
+	.probe		= a3700_spi_probe,
+	.remove		= a3700_spi_remove,
+};
+
+module_platform_driver(a3700_spi_driver);
+
+MODULE_DESCRIPTION("Armada-3700 SPI driver");
+MODULE_AUTHOR("Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
2.9.3

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

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

* [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
       [not found] ` <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  2016-12-01 10:27   ` [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller Romain Perier
@ 2016-12-01 10:27   ` Romain Perier
       [not found]     ` <20161201102719.4291-3-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  2016-12-01 10:27   ` [PATCH v3 3/5] spi: armada-3700: Add documentation for the Armada 3700 SPI Controller Romain Perier
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

In FIFO mode, dedicated registers are used to store the instruction,
the address, the read mode and the data. Write and Read FIFO are used
to store the outcoming or incoming data. The CPU no longer has to assert
each byte. The data FIFOs are accessible via DMA or by the CPU.

This commit adds support for the FIFO mode with the CPU.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---

Changes in v3:
 - Don't enable the fifo mode based on the compatible string, we introduce
   a module parameter "pio_mode". By default this option is set to zero, so
   the fifo mode is enabled. Pass pio_mode=1 to the driver enables the PIO
   mode.
 - Added tag "Tested-by" by Gregory

Changes in v2:
 - Removed a3700_spi_bytelen_set from the setup function, it was accidentally
   let here and not required, as it is configured in the prepare callback now
   (defaults to 4 for fifo mode). It solves unrecognized spi-nor flash memory
   detection with jedec. 

 drivers/spi/spi-armada-3700.c | 414 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 404 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
index 4115685..27a46cb 100644
--- a/drivers/spi/spi-armada-3700.c
+++ b/drivers/spi/spi-armada-3700.c
@@ -25,6 +25,11 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/spi/spi.h>
 
+static bool pio_mode;
+
+module_param(pio_mode, bool, 0);
+MODULE_PARM_DESC(pio_mode, "enable the PIO mode");
+
 #define DRIVER_NAME			"armada_3700_spi"
 
 #define A3700_SPI_TIMEOUT		10
@@ -99,19 +104,28 @@
 /* A3700_SPI_IF_TIME_REG */
 #define A3700_SPI_CLK_CAPT_EDGE		BIT(7)
 
+/* Flags and macros for struct a3700_spi */
+#define HAS_FIFO			BIT(0)
+#define A3700_INSTR_CNT			1
+#define A3700_ADDR_CNT			3
+#define A3700_DUMMY_CNT			1
+
 struct a3700_spi {
 	struct spi_master *master;
 	void __iomem *base;
 	struct clk *clk;
 	unsigned int irq;
 	unsigned int flags;
-	bool last_xfer;
+	bool xmit_data;
 	const u8 *tx_buf;
 	u8 *rx_buf;
 	size_t buf_len;
 	u8 byte_len;
 	u32 wait_mask;
 	struct completion done;
+	u32 addr_cnt;
+	u32 instr_cnt;
+	size_t hdr_cnt;
 };
 
 static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
@@ -180,12 +194,15 @@ static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
 	return 0;
 }
 
-static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+static void a3700_spi_fifo_mode_set(struct a3700_spi *a3700_spi)
 {
 	u32 val;
 
 	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
-	val &= ~A3700_SPI_FIFO_MODE;
+	if (a3700_spi->flags & HAS_FIFO)
+		val |= A3700_SPI_FIFO_MODE;
+	else
+		val &= ~A3700_SPI_FIFO_MODE;
 	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
 }
 
@@ -255,11 +272,30 @@ static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
 	a3700_spi->byte_len = len;
 }
 
+static int a3700_spi_fifo_flush(struct a3700_spi *a3700_spi)
+{
+	int timeout = A3700_SPI_TIMEOUT;
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_FIFO_FLUSH;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_FIFO_FLUSH))
+			return 0;
+		udelay(1);
+	}
+
+	return -ETIMEDOUT;
+}
+
 static int a3700_spi_init(struct a3700_spi *a3700_spi)
 {
 	struct spi_master *master = a3700_spi->master;
 	u32 val;
-	int i;
+	int i, ret = 0;
 
 	/* Reset SPI unit */
 	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
@@ -278,10 +314,8 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
 	for (i = 0; i < master->num_chipselect; i++)
 		a3700_spi_deactivate_cs(a3700_spi, i);
 
-	a3700_spi_pin_mode_set(a3700_spi, 0);
-
-	/* Be sure that FIFO mode is disabled */
-	a3700_spi_fifo_mode_unset(a3700_spi);
+	/* Enable FIFO mode */
+	a3700_spi_fifo_mode_set(a3700_spi);
 
 	/* Set SPI mode */
 	a3700_spi_mode_set(a3700_spi, master->mode_bits);
@@ -294,7 +328,7 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
 	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
 	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
 
-	return 0;
+	return ret;
 }
 
 static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
@@ -380,14 +414,34 @@ static bool a3700_spi_transfer_wait(struct spi_device *spi,
 	return a3700_spi_wait_completion(spi);
 }
 
+static void a3700_spi_fifo_thres_set(struct a3700_spi *a3700_spi,
+				     unsigned int bytes)
+{
+	u32 val;
+
+	if (a3700_spi->flags & HAS_FIFO) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_RFIFO_THRS_BIT);
+		val |= (bytes - 1) << A3700_SPI_RFIFO_THRS_BIT;
+		val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_WFIFO_THRS_BIT);
+		val |= (7 - bytes) << A3700_SPI_WFIFO_THRS_BIT;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	}
+}
+
 static void a3700_spi_transfer_setup(struct spi_device *spi,
 				    struct spi_transfer *xfer)
 {
 	struct a3700_spi *a3700_spi;
+	unsigned int byte_len;
 
 	a3700_spi = spi_master_get_devdata(spi->master);
 
 	a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+
+	byte_len = xfer->bits_per_word >> 3;
+
+	a3700_spi_fifo_thres_set(a3700_spi, byte_len);
 }
 
 static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
@@ -447,6 +501,168 @@ static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
 		a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
 }
 
+static void a3700_spi_header_set(struct a3700_spi *a3700_spi)
+{
+	u32 instr_cnt = 0, addr_cnt = 0, dummy_cnt = 0;
+	u32 val = 0;
+
+	/* Clear the header registers */
+	spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_RMODE_REG, 0);
+
+	/* Set header counters */
+	if (a3700_spi->tx_buf) {
+		if (a3700_spi->buf_len <= a3700_spi->instr_cnt) {
+			instr_cnt = a3700_spi->buf_len;
+		} else if (a3700_spi->buf_len <= (a3700_spi->instr_cnt +
+						  a3700_spi->addr_cnt)) {
+			instr_cnt = a3700_spi->instr_cnt;
+			addr_cnt = a3700_spi->buf_len - instr_cnt;
+		} else if (a3700_spi->buf_len <= a3700_spi->hdr_cnt) {
+			instr_cnt = a3700_spi->instr_cnt;
+			addr_cnt = a3700_spi->addr_cnt;
+			/* Need to handle the normal write case with 1 byte
+			 * data
+			 */
+			if (!a3700_spi->tx_buf[instr_cnt + addr_cnt])
+				dummy_cnt = a3700_spi->buf_len - instr_cnt -
+					    addr_cnt;
+		}
+		val |= ((instr_cnt & A3700_SPI_INSTR_CNT_MASK)
+			<< A3700_SPI_INSTR_CNT_BIT);
+		val |= ((addr_cnt & A3700_SPI_ADDR_CNT_MASK)
+			<< A3700_SPI_ADDR_CNT_BIT);
+		val |= ((dummy_cnt & A3700_SPI_DUMMY_CNT_MASK)
+			<< A3700_SPI_DUMMY_CNT_BIT);
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, val);
+
+	/* Update the buffer length to be transferred */
+	a3700_spi->buf_len -= (instr_cnt + addr_cnt + dummy_cnt);
+
+	/* Set Instruction */
+	val = 0;
+	while (instr_cnt--) {
+		val = (val << 8) | a3700_spi->tx_buf[0];
+		a3700_spi->tx_buf++;
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, val);
+
+	/* Set Address */
+	val = 0;
+	while (addr_cnt--) {
+		val = (val << 8) | a3700_spi->tx_buf[0];
+		a3700_spi->tx_buf++;
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, val);
+}
+
+static int a3700_is_wfifo_full(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	return (val & A3700_SPI_WFIFO_FULL);
+}
+
+static int a3700_spi_fifo_write(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+	int i = 0;
+
+	while (!a3700_is_wfifo_full(a3700_spi) && a3700_spi->buf_len) {
+		val = 0;
+		if (a3700_spi->buf_len >= 4) {
+			val = cpu_to_le32(*(u32 *)a3700_spi->tx_buf);
+			spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+
+			a3700_spi->buf_len -= 4;
+			a3700_spi->tx_buf += 4;
+		} else {
+			/*
+			 * If the remained buffer length is less than 4-bytes,
+			 * we should pad the write buffer with all ones. So that
+			 * it avoids overwrite the unexpected bytes following
+			 * the last one.
+			 */
+			val = GENMASK(31, 0);
+			while (a3700_spi->buf_len) {
+				val &= ~(0xff << (8 * i));
+				val |= *a3700_spi->tx_buf++ << (8 * i);
+				i++;
+				a3700_spi->buf_len--;
+
+				spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG,
+					     val);
+			}
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static int a3700_is_rfifo_empty(struct a3700_spi *a3700_spi)
+{
+	u32 val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+
+	return (val & A3700_SPI_RFIFO_EMPTY);
+}
+
+static int a3700_spi_fifo_read(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	while (!a3700_is_rfifo_empty(a3700_spi) && a3700_spi->buf_len) {
+		val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+		if (a3700_spi->buf_len >= 4) {
+			u32 data = le32_to_cpu(val);
+			memcpy(a3700_spi->rx_buf, &data, 4);
+
+			a3700_spi->buf_len -= 4;
+			a3700_spi->rx_buf += 4;
+		} else {
+			/*
+			 * When remain bytes is not larger than 4, we should
+			 * avoid memory overwriting and just write the left rx
+			 * buffer bytes.
+			 */
+			while (a3700_spi->buf_len) {
+				*a3700_spi->rx_buf = val & 0xff;
+				val >>= 8;
+
+				a3700_spi->buf_len--;
+				a3700_spi->rx_buf++;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void a3700_spi_transfer_abort_fifo(struct a3700_spi *a3700_spi)
+{
+	int timeout = A3700_SPI_TIMEOUT;
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_XFER_START))
+			break;
+		udelay(1);
+	}
+
+	a3700_spi_fifo_flush(a3700_spi);
+
+	val &= ~A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
 static int a3700_spi_prepare_message(struct spi_master *master,
 				     struct spi_message *message)
 {
@@ -463,12 +679,28 @@ static int a3700_spi_prepare_message(struct spi_master *master,
 	return 0;
 }
 
+static int a3700_spi_prepare_fifo_message(struct spi_master *master,
+					  struct spi_message *message)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret;
+
+	/* Flush the FIFOs */
+	ret = a3700_spi_fifo_flush(a3700_spi);
+	if (ret)
+		return ret;
+
+	a3700_spi_bytelen_set(a3700_spi, 4);
+
+	return 0;
+}
+
 static int a3700_spi_transfer_one(struct spi_master *master,
 				  struct spi_device *spi,
 				  struct spi_transfer *xfer)
 {
 	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
-	int ret = 0;
+	int ret;
 
 	a3700_spi_transfer_setup(spi, xfer);
 
@@ -505,6 +737,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
 	return ret;
 }
 
+static int a3700_spi_fifo_transfer_one(struct spi_master *master,
+				       struct spi_device *spi,
+				       struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret = 0, timeout = A3700_SPI_TIMEOUT;
+	unsigned int nbits = 0;
+	u32 val;
+
+	a3700_spi_transfer_setup(spi, xfer);
+
+	a3700_spi->tx_buf  = xfer->tx_buf;
+	a3700_spi->rx_buf  = xfer->rx_buf;
+	a3700_spi->buf_len = xfer->len;
+
+	/* SPI transfer headers */
+	a3700_spi_header_set(a3700_spi);
+
+	if (xfer->tx_buf)
+		nbits = xfer->tx_nbits;
+	else if (xfer->rx_buf)
+		nbits = xfer->rx_nbits;
+
+	a3700_spi_pin_mode_set(a3700_spi, nbits);
+
+	if (xfer->rx_buf) {
+		/* Set read data length */
+		spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
+			     a3700_spi->buf_len);
+		/* Start READ transfer */
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val &= ~A3700_SPI_RW_EN;
+		val |= A3700_SPI_XFER_START;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	} else if (xfer->tx_buf) {
+		/* Start Write transfer */
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val |= (A3700_SPI_XFER_START | A3700_SPI_RW_EN);
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+		/*
+		 * If there are data to be written to the SPI device, xmit_data
+		 * flag is set true; otherwise the instruction in SPI_INSTR does
+		 * not require data to be written to the SPI device, then
+		 * xmit_data flag is set false.
+		 */
+		a3700_spi->xmit_data = (a3700_spi->buf_len != 0);
+	}
+
+	while (a3700_spi->buf_len) {
+		if (a3700_spi->tx_buf) {
+			/* Wait wfifo ready */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_WFIFO_RDY)) {
+				dev_err(&spi->dev,
+					"wait wfifo ready timed out\n");
+				ret = -ETIMEDOUT;
+				goto error;
+			}
+			/* Fill up the wfifo */
+			ret = a3700_spi_fifo_write(a3700_spi);
+			if (ret)
+				goto error;
+		} else if (a3700_spi->rx_buf) {
+			/* Wait rfifo ready */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_RFIFO_RDY)) {
+				dev_err(&spi->dev,
+					"wait rfifo ready timed out\n");
+				ret = -ETIMEDOUT;
+				goto error;
+			}
+			/* Drain out the rfifo */
+			ret = a3700_spi_fifo_read(a3700_spi);
+			if (ret)
+				goto error;
+		}
+	}
+
+	/*
+	 * Stop a write transfer in fifo mode:
+	 *	- wait all the bytes in wfifo to be shifted out
+	 *	 - set XFER_STOP bit
+	 *	- wait XFER_START bit clear
+	 *	- clear XFER_STOP bit
+	 * Stop a read transfer in fifo mode:
+	 *	- the hardware is to reset the XFER_START bit
+	 *	   after the number of bytes indicated in DIN_CNT
+	 *	   register
+	 *	- just wait XFER_START bit clear
+	 */
+	if (a3700_spi->tx_buf) {
+		if (a3700_spi->xmit_data) {
+			/*
+			 * If there are data written to the SPI device, wait
+			 * until SPI_WFIFO_EMPTY is 1 to wait for all data to
+			 * transfer out of write FIFO.
+			 */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_WFIFO_EMPTY)) {
+				dev_err(&spi->dev, "wait wfifo empty timed out\n");
+				return -ETIMEDOUT;
+			}
+		} else {
+			/*
+			 * If the instruction in SPI_INSTR does not require data
+			 * to be written to the SPI device, wait until SPI_RDY
+			 * is 1 for the SPI interface to be in idle.
+			 */
+			if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+				dev_err(&spi->dev, "wait xfer ready timed out\n");
+				return -ETIMEDOUT;
+			}
+		}
+
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val |= A3700_SPI_XFER_STOP;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	}
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_XFER_START))
+			break;
+		udelay(1);
+	}
+
+	if (timeout == 0) {
+		dev_err(&spi->dev, "wait transfer start clear timed out\n");
+		ret = -ETIMEDOUT;
+		goto error;
+	}
+
+	val &= ~A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	goto out;
+
+error:
+	a3700_spi_transfer_abort_fifo(a3700_spi);
+out:
+	spi_finalize_current_transfer(master);
+
+	return ret;
+}
+
 static int a3700_spi_unprepare_message(struct spi_master *master,
 				       struct spi_message *message)
 {
@@ -593,6 +970,23 @@ static int a3700_spi_probe(struct platform_device *pdev)
 		goto error;
 	}
 
+	if (!pio_mode) {
+		master->prepare_message =  a3700_spi_prepare_fifo_message;
+		master->transfer_one = a3700_spi_fifo_transfer_one;
+
+		spi->flags |= HAS_FIFO;
+		spi->instr_cnt = A3700_INSTR_CNT;
+		spi->addr_cnt = A3700_ADDR_CNT;
+		spi->hdr_cnt = A3700_INSTR_CNT + A3700_ADDR_CNT +
+			       A3700_DUMMY_CNT;
+		master->mode_bits |= (SPI_RX_DUAL | SPI_RX_DUAL |
+				      SPI_RX_QUAD | SPI_TX_QUAD);
+	} else {
+		master->prepare_message =  a3700_spi_prepare_message;
+		master->transfer_one = a3700_spi_transfer_one;
+		master->unprepare_message = a3700_spi_unprepare_message;
+	}
+
 	ret = a3700_spi_init(spi);
 	if (ret)
 		goto error_clk;
-- 
2.9.3

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

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

* [PATCH v3 3/5] spi: armada-3700: Add documentation for the Armada 3700 SPI Controller
       [not found] ` <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  2016-12-01 10:27   ` [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller Romain Perier
  2016-12-01 10:27   ` [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode Romain Perier
@ 2016-12-01 10:27   ` Romain Perier
       [not found]     ` <20161201102719.4291-4-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  2016-12-01 10:27   ` [PATCH v3 4/5] arm64: dts: marvell: Add definition of SPI controller for Armada 3700 Romain Perier
  2016-12-01 10:27   ` [PATCH v3 5/5] arm64: dts: marvell: Enable spi0 on the board Armada-3720-db Romain Perier
  4 siblings, 1 reply; 12+ messages in thread
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

This adds the devicetree bindings documentation for the SPI controller
present in the Marvell Armada 3700 SoCs.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---

Changes in v3:
 - Added tag "Tested-by" by Gregory
 - Fixed commit title, as requested by Mark Brown

 .../devicetree/bindings/spi/spi-armada-3700.txt    | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-armada-3700.txt b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
new file mode 100644
index 0000000..1564aa8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
@@ -0,0 +1,25 @@
+* Marvell Armada 3700 SPI Controller
+
+Required Properties:
+
+- compatible: should be "marvell,armada-3700-spi"
+- reg: physical base address of the controller and length of memory mapped
+       region.
+- interrupts: The interrupt number. The interrupt specifier format depends on
+	      the interrupt controller and of its driver.
+- clocks: Must contain the clock source, usually from the North Bridge clocks.
+- num-cs: The number of chip selects that is supported by this SPI Controller
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+
+Example:
+
+	spi0: spi@10600 {
+		compatible = "marvell,armada-3700-spi";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10600 0x5d>;
+		clocks = <&nb_perih_clk 7>;
+		interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+		num-cs = <4>;
+	};
-- 
2.9.3

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

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

* [PATCH v3 4/5] arm64: dts: marvell: Add definition of SPI controller for Armada 3700
       [not found] ` <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
                     ` (2 preceding siblings ...)
  2016-12-01 10:27   ` [PATCH v3 3/5] spi: armada-3700: Add documentation for the Armada 3700 SPI Controller Romain Perier
@ 2016-12-01 10:27   ` Romain Perier
  2016-12-01 10:27   ` [PATCH v3 5/5] arm64: dts: marvell: Enable spi0 on the board Armada-3720-db Romain Perier
  4 siblings, 0 replies; 12+ messages in thread
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

Armada 3700 SoC has an SPI Controller, this commit adds the definition
of the SPI device node at the SoC level.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---

Changes in v3:
 - Fixed wrong register size for spi0, as suggested by the maintainer
   on the ML.
 - Added tag "Tested-by" by Gregory

Changes in v2:
 - Removed properties max-frequency and clock-frequency, it is no
   longer required and not used by the DT-bindings.

 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e9bd587..fcef9a5 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -98,6 +98,17 @@
 			/* 32M internal register @ 0xd000_0000 */
 			ranges = <0x0 0x0 0xd0000000 0x2000000>;
 
+			spi0: spi@10600 {
+				compatible = "marvell,armada-3700-spi";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x10600 0xA00>;
+				clocks = <&nb_periph_clk 7>;
+				interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+				num-cs = <4>;
+				status = "disabled";
+			};
+
 			uart0: serial@12000 {
 				compatible = "marvell,armada-3700-uart";
 				reg = <0x12000 0x400>;
-- 
2.9.3

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

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

* [PATCH v3 5/5] arm64: dts: marvell: Enable spi0 on the board Armada-3720-db
       [not found] ` <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
                     ` (3 preceding siblings ...)
  2016-12-01 10:27   ` [PATCH v3 4/5] arm64: dts: marvell: Add definition of SPI controller for Armada 3700 Romain Perier
@ 2016-12-01 10:27   ` Romain Perier
  4 siblings, 0 replies; 12+ messages in thread
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
  To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

This commit enables the device node spi0 on the official development
board for the Marvell Armada 3700. It also adds sub-node for the 128Mb
SPI-NOR present on the board.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---

Changes in v3:
 - Added tag "Tested-by" by Gregory

 arch/arm64/boot/dts/marvell/armada-3720-db.dts | 30 ++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 1372e9a6..0c4eb98 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -67,6 +67,36 @@
 	status = "okay";
 };
 
+&spi0 {
+	status = "okay";
+
+	m25p80@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <108000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
+
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			partition@0 {
+				label = "bootloader";
+				reg = <0x0 0x200000>;
+			};
+			partition@200000 {
+				label = "U-boot Env";
+				reg = <0x200000 0x10000>;
+			};
+			partition@210000 {
+				label = "Linux";
+				reg = <0x210000 0xDF0000>;
+			};
+		};
+	};
+};
+
 /* Exported on the micro USB connector CON32 through an FTDI */
 &uart0 {
 	status = "okay";
-- 
2.9.3

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

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

* Re: [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller
  2016-12-01 10:27   ` [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller Romain Perier
@ 2016-12-05 12:05     ` Mark Brown
       [not found]       ` <20161205120540.mvihgafelszitzhq-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2016-12-05 12:05 UTC (permalink / raw)
  To: Romain Perier
  Cc: Mark Rutland, Andrew Lunn, Jason Cooper, Pawel Moll, devicetree,
	Ian Campbell, linux-spi, Nadav Haklai, Rob Herring, Kumar Gala,
	Gregory Clement, xigu, dingwei, Thomas Petazzoni,
	linux-arm-kernel, Sebastian Hesselbarth


[-- Attachment #1.1: Type: text/plain, Size: 1933 bytes --]

On Thu, Dec 01, 2016 at 11:27:15AM +0100, Romain Perier wrote:

> +config SPI_ARMADA_3700
> +	tristate "Marvell Armada 3700 SPI Controller"
> +	depends on ARCH_MVEBU && OF

Why does this not have a COMPILE_TEST dependency?

> +	/* Reset SPI unit */
> +	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
> +	val |= A3700_SPI_SRST;
> +	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
> +
> +	for (i = 0; i < A3700_SPI_TIMEOUT; i++)
> +		udelay(1);

Why not just do a single udelay()?

> +static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
> +{
> +	struct spi_master *master = dev_id;
> +	struct a3700_spi *a3700_spi;
> +	u32 cause;
> +
> +	a3700_spi = spi_master_get_devdata(master);
> +
> +	/* Get interrupt causes */
> +	cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
> +
> +	/* mask and acknowledge the SPI interrupts */
> +	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
> +	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
> +
> +	/* Wake up the transfer */
> +	if (a3700_spi->wait_mask & cause)
> +		complete(&a3700_spi->done);
> +
> +	return IRQ_HANDLED;
> +}

This reports that we handled an interrupt even if we did not in fact
handle an interrupt.  It's also a bit dodgy that it doesn't check what
the interrupt was but that's less serious.

> +	master->bus_num = (pdev->id != -1) ? pdev->id : 0;

At most this should be just setting the bus number to pdev->id like
other drivers do.

> +	ret = clk_prepare_enable(spi->clk);
> +	if (ret) {
> +		dev_err(dev, "could not prepare clk: %d\n", ret);
> +		goto error;
> +	}

I'd expect the hardware prepare/unprepare to be managing the enable and
disable of the clock in order to save a little power.

> +	dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
> +		 (unsigned long)res->start, spi->irq);

This is just adding noise to the boot, remove it.  It's not telling us
anything we read from the hardware or anything.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
       [not found]     ` <20161201102719.4291-3-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
@ 2016-12-05 13:37       ` Mark Brown
       [not found]         ` <20161205133728.sza2pt4tfkywu35k-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2016-12-05 13:37 UTC (permalink / raw)
  To: Romain Perier
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

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

On Thu, Dec 01, 2016 at 11:27:16AM +0100, Romain Perier wrote:

> Changes in v3:
>  - Don't enable the fifo mode based on the compatible string, we introduce
>    a module parameter "pio_mode". By default this option is set to zero, so
>    the fifo mode is enabled. Pass pio_mode=1 to the driver enables the PIO
>    mode.

Why?  If the hardware supports a more efficient mode of operation it
doesn't seem sensible not to use it.

> -	int i;
> +	int i, ret = 0;

Coding style - don't combine initialized and non-initalized variables on
one line.

>  static int a3700_spi_transfer_one(struct spi_master *master,
>  				  struct spi_device *spi,
>  				  struct spi_transfer *xfer)
>  {
>  	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
> -	int ret = 0;
> +	int ret;
>  
>  	a3700_spi_transfer_setup(spi, xfer);
>  
> @@ -505,6 +737,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
>  	return ret;
>  }

This appears to be a random unrelated change, should probably be part of
the initial driver commit.

> +static int a3700_spi_fifo_transfer_one(struct spi_master *master,
> +				       struct spi_device *spi,
> +				       struct spi_transfer *xfer)
> +{
> +	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
> +	int ret = 0, timeout = A3700_SPI_TIMEOUT;
> +	unsigned int nbits = 0;
> +	u32 val;
> +
> +	a3700_spi_transfer_setup(spi, xfer);
> +
> +	a3700_spi->tx_buf  = xfer->tx_buf;
> +	a3700_spi->rx_buf  = xfer->rx_buf;
> +	a3700_spi->buf_len = xfer->len;
> +
> +	/* SPI transfer headers */
> +	a3700_spi_header_set(a3700_spi);
> +
> +	if (xfer->tx_buf)
> +		nbits = xfer->tx_nbits;
> +	else if (xfer->rx_buf)
> +		nbits = xfer->rx_nbits;
> +
> +	a3700_spi_pin_mode_set(a3700_spi, nbits);
> +
> +	if (xfer->rx_buf) {
> +		/* Set read data length */
> +		spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
> +			     a3700_spi->buf_len);
> +		/* Start READ transfer */
> +		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
> +		val &= ~A3700_SPI_RW_EN;
> +		val |= A3700_SPI_XFER_START;
> +		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
> +	} else if (xfer->tx_buf) {
> +		/* Start Write transfer */

So this only supports single duplex transfers but there doesn't seem to
be anything that enforces this.

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

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

* Re: [PATCH v3 3/5] spi: armada-3700: Add documentation for the Armada 3700 SPI Controller
       [not found]     ` <20161201102719.4291-4-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
@ 2016-12-06  0:27       ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2016-12-06  0:27 UTC (permalink / raw)
  To: Romain Perier
  Cc: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA, Jason Cooper,
	Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Ian Campbell, Pawel Moll,
	Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

On Thu, Dec 01, 2016 at 11:27:17AM +0100, Romain Perier wrote:
> This adds the devicetree bindings documentation for the SPI controller
> present in the Marvell Armada 3700 SoCs.
> 
> Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> 
> Changes in v3:
>  - Added tag "Tested-by" by Gregory
>  - Fixed commit title, as requested by Mark Brown
> 
>  .../devicetree/bindings/spi/spi-armada-3700.txt    | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt

Didn't catch the duplicate with the subject change...

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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

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

* Re: [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller
       [not found]       ` <20161205120540.mvihgafelszitzhq-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2016-12-07 12:42         ` Romain Perier
  0 siblings, 0 replies; 12+ messages in thread
From: Romain Perier @ 2016-12-07 12:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

Hello,

Le 05/12/2016 à 13:05, Mark Brown a écrit :
> On Thu, Dec 01, 2016 at 11:27:15AM +0100, Romain Perier wrote:
>
>> +config SPI_ARMADA_3700
>> +	tristate "Marvell Armada 3700 SPI Controller"
>> +	depends on ARCH_MVEBU && OF
>
> Why does this not have a COMPILE_TEST dependency?

Because that's a mistake, I will fix it.

>
>> +	/* Reset SPI unit */
>> +	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
>> +	val |= A3700_SPI_SRST;
>> +	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
>> +
>> +	for (i = 0; i < A3700_SPI_TIMEOUT; i++)
>> +		udelay(1);
>
> Why not just do a single udelay()?

Mhhhh... good point.


>
>> +static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
>> +{
>> +	struct spi_master *master = dev_id;
>> +	struct a3700_spi *a3700_spi;
>> +	u32 cause;
>> +
>> +	a3700_spi = spi_master_get_devdata(master);
>> +
>> +	/* Get interrupt causes */
>> +	cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
>> +
>> +	/* mask and acknowledge the SPI interrupts */
>> +	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
>> +	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
>> +
>> +	/* Wake up the transfer */
>> +	if (a3700_spi->wait_mask & cause)
>> +		complete(&a3700_spi->done);
>> +
>> +	return IRQ_HANDLED;
>> +}
>
> This reports that we handled an interrupt even if we did not in fact
> handle an interrupt.  It's also a bit dodgy that it doesn't check what
> the interrupt was but that's less serious.

I don't understand, what do you expect ? That I return something != 
IRQ_HANDLED when the cause of the interrupt is not present in wait_mask ?

>
>> +	master->bus_num = (pdev->id != -1) ? pdev->id : 0;
>
> At most this should be just setting the bus number to pdev->id like
> other drivers do.

ack

>
>> +	ret = clk_prepare_enable(spi->clk);
>> +	if (ret) {
>> +		dev_err(dev, "could not prepare clk: %d\n", ret);
>> +		goto error;
>> +	}
>
> I'd expect the hardware prepare/unprepare to be managing the enable and
> disable of the clock in order to save a little power.

Ok, if that's better for power management, why not then.

>
>> +	dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
>> +		 (unsigned long)res->start, spi->irq);
>
> This is just adding noise to the boot, remove it.  It's not telling us
> anything we read from the hardware or anything.

ack


Thanks,
Romain

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

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

* Re: [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
       [not found]         ` <20161205133728.sza2pt4tfkywu35k-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2016-12-07 16:19           ` Romain Perier
       [not found]             ` <a9b86043-b631-8154-edc8-5abe49e47114-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: Romain Perier @ 2016-12-07 16:19 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

Hello,

Le 05/12/2016 à 14:37, Mark Brown a écrit :
> On Thu, Dec 01, 2016 at 11:27:16AM +0100, Romain Perier wrote:
>
>> Changes in v3:
>>  - Don't enable the fifo mode based on the compatible string, we introduce
>>    a module parameter "pio_mode". By default this option is set to zero, so
>>    the fifo mode is enabled. Pass pio_mode=1 to the driver enables the PIO
>>    mode.
>
> Why?  If the hardware supports a more efficient mode of operation it
> doesn't seem sensible not to use it.

That's just that our customer want to keep both modes, this is why I 
decided to use the more efficient mode by default and disable it via a 
module parameter. Previously the more efficient mode was always enabled, 
based on the "compatible string" of the DT (the PIO mode was simply 
useless in this case and dead code)

>
>> -	int i;
>> +	int i, ret = 0;
>
> Coding style - don't combine initialized and non-initalized variables on
> one line.

Ok

>
>>  static int a3700_spi_transfer_one(struct spi_master *master,
>>  				  struct spi_device *spi,
>>  				  struct spi_transfer *xfer)
>>  {
>>  	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
>> -	int ret = 0;
>> +	int ret;
>>
>>  	a3700_spi_transfer_setup(spi, xfer);
>>
>> @@ -505,6 +737,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
>>  	return ret;
>>  }
>
> This appears to be a random unrelated change, should probably be part of
> the initial driver commit.

Good catch

>
>> +static int a3700_spi_fifo_transfer_one(struct spi_master *master,
>> +				       struct spi_device *spi,
>> +				       struct spi_transfer *xfer)
>> +{
>> +	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
>> +	int ret = 0, timeout = A3700_SPI_TIMEOUT;
>> +	unsigned int nbits = 0;
>> +	u32 val;
>> +
>> +	a3700_spi_transfer_setup(spi, xfer);
>> +
>> +	a3700_spi->tx_buf  = xfer->tx_buf;
>> +	a3700_spi->rx_buf  = xfer->rx_buf;
>> +	a3700_spi->buf_len = xfer->len;
>> +
>> +	/* SPI transfer headers */
>> +	a3700_spi_header_set(a3700_spi);
>> +
>> +	if (xfer->tx_buf)
>> +		nbits = xfer->tx_nbits;
>> +	else if (xfer->rx_buf)
>> +		nbits = xfer->rx_nbits;
>> +
>> +	a3700_spi_pin_mode_set(a3700_spi, nbits);
>> +
>> +	if (xfer->rx_buf) {
>> +		/* Set read data length */
>> +		spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
>> +			     a3700_spi->buf_len);
>> +		/* Start READ transfer */
>> +		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
>> +		val &= ~A3700_SPI_RW_EN;
>> +		val |= A3700_SPI_XFER_START;
>> +		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
>> +	} else if (xfer->tx_buf) {
>> +		/* Start Write transfer */
>
> So this only supports single duplex transfers but there doesn't seem to
> be anything that enforces this.
>

A flag or a capability, typically? I will investigate

Thanks,
Romain

-- 
Romain Perier, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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

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

* Re: [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
       [not found]             ` <a9b86043-b631-8154-edc8-5abe49e47114-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
@ 2016-12-07 16:34               ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2016-12-07 16:34 UTC (permalink / raw)
  To: Romain Perier
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ

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

On Wed, Dec 07, 2016 at 05:19:57PM +0100, Romain Perier wrote:
> Le 05/12/2016 à 14:37, Mark Brown a écrit :

> > Why?  If the hardware supports a more efficient mode of operation it
> > doesn't seem sensible not to use it.

> That's just that our customer want to keep both modes, this is why I decided
> to use the more efficient mode by default and disable it via a module
> parameter. Previously the more efficient mode was always enabled, based on
> the "compatible string" of the DT (the PIO mode was simply useless in this
> case and dead code)

Keep that out of tree, if the user wants to do silly things then let
them pay the cost.

> > > +	if (xfer->rx_buf) {
> > > +		/* Set read data length */
> > > +		spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
> > > +			     a3700_spi->buf_len);
> > > +		/* Start READ transfer */
> > > +		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
> > > +		val &= ~A3700_SPI_RW_EN;
> > > +		val |= A3700_SPI_XFER_START;
> > > +		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
> > > +	} else if (xfer->tx_buf) {
> > > +		/* Start Write transfer */

> > So this only supports single duplex transfers but there doesn't seem to
> > be anything that enforces this.

> A flag or a capability, typically? I will investigate

SPI_MASTER_HALF_DUPLEX.

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

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

end of thread, other threads:[~2016-12-07 16:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-01 10:27 [PATCH v3 0/5] Add support for the Armada 3700 SPI controller Romain Perier
     [not found] ` <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2016-12-01 10:27   ` [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller Romain Perier
2016-12-05 12:05     ` Mark Brown
     [not found]       ` <20161205120540.mvihgafelszitzhq-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2016-12-07 12:42         ` Romain Perier
2016-12-01 10:27   ` [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode Romain Perier
     [not found]     ` <20161201102719.4291-3-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2016-12-05 13:37       ` Mark Brown
     [not found]         ` <20161205133728.sza2pt4tfkywu35k-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2016-12-07 16:19           ` Romain Perier
     [not found]             ` <a9b86043-b631-8154-edc8-5abe49e47114-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2016-12-07 16:34               ` Mark Brown
2016-12-01 10:27   ` [PATCH v3 3/5] spi: armada-3700: Add documentation for the Armada 3700 SPI Controller Romain Perier
     [not found]     ` <20161201102719.4291-4-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2016-12-06  0:27       ` Rob Herring
2016-12-01 10:27   ` [PATCH v3 4/5] arm64: dts: marvell: Add definition of SPI controller for Armada 3700 Romain Perier
2016-12-01 10:27   ` [PATCH v3 5/5] arm64: dts: marvell: Enable spi0 on the board Armada-3720-db Romain Perier

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