[PATCH 00/10] spi: sh-msiof: Add support for R-Car H2 and M2

[PATCH 00/10] spi: sh-msiof: Add support for R-Car H2 and M2

[Geert Uytterhoeven]

	Hi Mark,

This patch series refactors the sh-msiof SPI driver and adds support for
the MSIOF variant in the Renesas R-Car H2 (r8a7790) and M2 (r8a7791) SoCs.

It was tested on the Lager and Koelsch development boards, using a dummy
driver for the Renesas r2a11302ft PMIC that reads out the PMIC's version ID.

  [01/10] spi: sh-msiof: Fix SPI bus population from DT
  [02/10] spi: sh-msiof: Typo in comment s/tx/rx/
  [03/10] spi: sh-msiof: Change hz from unsigned long to u32
  [04/10] spi: sh-msiof: Add more register documentation
  [05/10] spi: sh-msiof: Use the core cs_gpio field, and make it optional
  [06/10] spi: sh-msiof: Improve binding documentation
  [07/10] spi: sh-msiof: Add support for R-Car H2 and M2
  [08/10] spi: sh-msiof: Move clock management to (un)prepare_message()
  [09/10] spi: sh-msiof: Convert to let spi core validate xfer->bits_per_word
  [10/10] spi: sh-msiof: Use core message handling instead of spi-bitbang

Some of this work was based on a patch series by Takashi Yoshii
<takasi-y@ops.dti.ne.jp>.

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

[PATCH 01/10] spi: sh-msiof: Fix SPI bus population from DT

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

DT doesn't instantiate SPI children if spi_master.dev.of_node is not set up
properly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
Cc: devicetree@vger.kernel.org
---
 drivers/spi/spi-sh-msiof.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 33474061b742..e6f79b2f2616 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -710,6 +710,7 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	master->mode_bits |= SPI_LSB_FIRST | SPI_3WIRE;
 	master->flags = 0;
 	master->bus_num = pdev->id;
+	master->dev.of_node = pdev->dev.of_node;
 	master->num_chipselect = p->info->num_chipselect;
 	master->setup = spi_bitbang_setup;
 	master->cleanup = spi_bitbang_cleanup;
-- 
1.7.9.5

[PATCH 02/10] spi: sh-msiof: Typo in comment s/tx/rx/

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/spi/spi-sh-msiof.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index e6f79b2f2616..cc12e755131d 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -486,7 +486,7 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
 	/* clear status bits */
 	sh_msiof_reset_str(p);
 
-	/* shut down frame, tx/tx and clock signals */
+	/* shut down frame, rx/tx and clock signals */
 	ret = sh_msiof_modify_ctr_wait(p, CTR_TFSE, 0);
 	ret = ret ? ret : sh_msiof_modify_ctr_wait(p, CTR_TXE, 0);
 	if (rx_buf)
-- 
1.7.9.5

[PATCH 03/10] spi: sh-msiof: Change hz from unsigned long to u32

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Both spi_transfer.speed_hz and spi_master.max_speed_hz are u32

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/spi/spi-sh-msiof.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index cc12e755131d..af9f2db41b16 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -144,8 +144,7 @@ static struct {
 };
 
 static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p,
-				      unsigned long parent_rate,
-				      unsigned long spi_hz)
+				      unsigned long parent_rate, u32 spi_hz)
 {
 	unsigned long div = 1024;
 	size_t k;
@@ -372,10 +371,9 @@ static int sh_msiof_spi_bits(struct spi_device *spi, struct spi_transfer *t)
 	return bits;
 }
 
-static unsigned long sh_msiof_spi_hz(struct spi_device *spi,
-				     struct spi_transfer *t)
+static u32 sh_msiof_spi_hz(struct spi_device *spi, struct spi_transfer *t)
 {
-	unsigned long hz;
+	u32 hz;
 
 	hz = t ? t->speed_hz : 0;
 	if (!hz)
-- 
1.7.9.5

[PATCH 04/10] spi: sh-msiof: Add more register documentation

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/spi/spi-sh-msiof.c |  152 +++++++++++++++++++++++++++++---------------
 1 file changed, 100 insertions(+), 52 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index af9f2db41b16..79e14586049b 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -41,32 +41,80 @@ struct sh_msiof_spi_priv {
 	int rx_fifo_size;
 };
 
-#define TMDR1	0x00
-#define TMDR2	0x04
-#define TMDR3	0x08
-#define RMDR1	0x10
-#define RMDR2	0x14
-#define RMDR3	0x18
-#define TSCR	0x20
-#define RSCR	0x22
-#define CTR	0x28
-#define FCTR	0x30
-#define STR	0x40
-#define IER	0x44
-#define TDR1	0x48
-#define TDR2	0x4c
-#define TFDR	0x50
-#define RDR1	0x58
-#define RDR2	0x5c
-#define RFDR	0x60
-
-#define CTR_TSCKE (1 << 15)
-#define CTR_TFSE  (1 << 14)
-#define CTR_TXE   (1 << 9)
-#define CTR_RXE   (1 << 8)
-
-#define STR_TEOF  (1 << 23)
-#define STR_REOF  (1 << 7)
+#define TMDR1	0x00	/* Transmit Mode Register 1 */
+#define TMDR2	0x04	/* Transmit Mode Register 2 */
+#define TMDR3	0x08	/* Transmit Mode Register 3 */
+#define RMDR1	0x10	/* Receive Mode Register 1 */
+#define RMDR2	0x14	/* Receive Mode Register 2 */
+#define RMDR3	0x18	/* Receive Mode Register 3 */
+#define TSCR	0x20	/* Transmit Clock Select Register */
+#define RSCR	0x22	/* Receive Clock Select Register (SH, A1, APE6) */
+#define CTR	0x28	/* Control Register */
+#define FCTR	0x30	/* FIFO Control Register */
+#define STR	0x40	/* Status Register */
+#define IER	0x44	/* Interrupt Enable Register */
+#define TDR1	0x48	/* Transmit Control Data Register 1 (SH, A1) */
+#define TDR2	0x4c	/* Transmit Control Data Register 2 (SH, A1) */
+#define TFDR	0x50	/* Transmit FIFO Data Register */
+#define RDR1	0x58	/* Receive Control Data Register 1 (SH, A1) */
+#define RDR2	0x5c	/* Receive Control Data Register 2 (SH, A1) */
+#define RFDR	0x60	/* Receive FIFO Data Register */
+
+/* TMDR1 and RMDR1 */
+#define MDR1_TRMD	 0x80000000 /* Transfer Mode (1 = Master mode) */
+#define MDR1_SYNCMD_MASK 0x30000000 /* SYNC Mode */
+#define MDR1_SYNCMD_SPI	 0x20000000 /*   Level mode/SPI */
+#define MDR1_SYNCMD_LR	 0x30000000 /*   L/R mode */
+#define MDR1_SYNCAC_SHIFT	 25 /* Sync Polarity (1 = Active-low) */
+#define MDR1_BITLSB_SHIFT	 24 /* MSB/LSB First (1 = LSB first) */
+#define MDR1_FLD_MASK	 0x000000c0 /* Frame Sync Signal Interval (0-3) */
+#define MDR1_FLD_SHIFT		  2
+#define MDR1_XXSTP	 0x00000001 /* Transmission/Reception Stop on FIFO */
+/* TMDR1 */
+#define TMDR1_PCON	 0x40000000 /* Transfer Signal Connection */
+
+/* TMDR2 and RMDR2 */
+#define MDR2_BITLEN1(i)	(((i) - 1) << 24) /* Data Size (8-32 bits) */
+#define MDR2_WDLEN1(i)	(((i) - 1) << 16) /* Word Count (1-64/256 (SH, A1))) */
+#define MDR2_GRPMASK1	0x00000001 /* Group Output Mask 1 (SH, A1) */
+
+/* TSCR and RSCR */
+#define SCR_BRPS_MASK	    0x1f00 /* Prescaler Setting (1-32) */
+#define SCR_BRPS(i)	(((i) - 1) << 8)
+#define SCR_BRDV_MASK	    0x0007 /* Baud Rate Generator's Division Ratio */
+#define SCR_BRDV_DIV_2	    0x0000
+#define SCR_BRDV_DIV_4	    0x0001
+#define SCR_BRDV_DIV_8	    0x0002
+#define SCR_BRDV_DIV_16	    0x0003
+#define SCR_BRDV_DIV_32	    0x0004
+#define SCR_BRDV_DIV_1	    0x0007
+
+/* CTR */
+#define CTR_TSCKIZ_MASK	0xc0000000 /* Transmit Clock I/O Polarity Select */
+#define CTR_TSCKIZ_SCK	0x80000000 /*   Disable SCK when TX disabled */
+#define CTR_TSCKIZ_POL_SHIFT	30 /*   Transmit Clock Polarity */
+#define CTR_RSCKIZ_MASK	0x30000000 /* Receive Clock Polarity Select */
+#define CTR_RSCKIZ_SCK	0x20000000 /*   Must match CTR_TSCKIZ_SCK */
+#define CTR_RSCKIZ_POL_SHIFT	28 /*   Receive Clock Polarity */
+#define CTR_TEDG_SHIFT		27 /* Transmit Timing (1 = falling edge) */
+#define CTR_REDG_SHIFT		26 /* Receive Timing (1 = falling edge) */
+#define CTR_TXDIZ_MASK	0x00c00000 /* Pin Output When TX is Disabled */
+#define CTR_TXDIZ_LOW	0x00000000 /*   0 */
+#define CTR_TXDIZ_HIGH	0x00400000 /*   1 */
+#define CTR_TXDIZ_HIZ	0x00800000 /*   High-impedance */
+#define CTR_TSCKE	0x00008000 /* Transmit Serial Clock Output Enable */
+#define CTR_TFSE	0x00004000 /* Transmit Frame Sync Signal Output Enable */
+#define CTR_TXE		0x00000200 /* Transmit Enable */
+#define CTR_RXE		0x00000100 /* Receive Enable */
+
+/* STR and IER */
+#define STR_TEOF	0x00800000 /* Frame Transmission End */
+#define STR_REOF	0x00000080 /* Frame Reception End */
+
+
+#define DEFAULT_TX_FIFO_SIZE	64
+#define DEFAULT_RX_FIFO_SIZE	64
+
 
 static u32 sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs)
 {
@@ -130,17 +178,17 @@ static struct {
 	unsigned short div;
 	unsigned short scr;
 } const sh_msiof_spi_clk_table[] = {
-	{ 1, 0x0007 },
-	{ 2, 0x0000 },
-	{ 4, 0x0001 },
-	{ 8, 0x0002 },
-	{ 16, 0x0003 },
-	{ 32, 0x0004 },
-	{ 64, 0x1f00 },
-	{ 128, 0x1f01 },
-	{ 256, 0x1f02 },
-	{ 512, 0x1f03 },
-	{ 1024, 0x1f04 },
+	{ 1,	SCR_BRPS( 1) | SCR_BRDV_DIV_1 },
+	{ 2,	SCR_BRPS( 1) | SCR_BRDV_DIV_2 },
+	{ 4,	SCR_BRPS( 1) | SCR_BRDV_DIV_4 },
+	{ 8,	SCR_BRPS( 1) | SCR_BRDV_DIV_8 },
+	{ 16,	SCR_BRPS( 1) | SCR_BRDV_DIV_16 },
+	{ 32,	SCR_BRPS( 1) | SCR_BRDV_DIV_32 },
+	{ 64,	SCR_BRPS(32) | SCR_BRDV_DIV_2 },
+	{ 128,	SCR_BRPS(32) | SCR_BRDV_DIV_4 },
+	{ 256,	SCR_BRPS(32) | SCR_BRDV_DIV_8 },
+	{ 512,	SCR_BRPS(32) | SCR_BRDV_DIV_16 },
+	{ 1024,	SCR_BRPS(32) | SCR_BRDV_DIV_32 },
 };
 
 static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p,
@@ -181,21 +229,21 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p,
 	 */
 	sh_msiof_write(p, FCTR, 0);
 
-	tmp = 0;
-	tmp |= !cs_high << 25;
-	tmp |= lsb_first << 24;
-	sh_msiof_write(p, TMDR1, 0xe0000005 | tmp);
-	sh_msiof_write(p, RMDR1, 0x20000005 | tmp);
+	tmp = MDR1_SYNCMD_SPI | 1 << MDR1_FLD_SHIFT | MDR1_XXSTP;
+	tmp |= !cs_high << MDR1_SYNCAC_SHIFT;
+	tmp |= lsb_first << MDR1_BITLSB_SHIFT;
+	sh_msiof_write(p, TMDR1, tmp | MDR1_TRMD | TMDR1_PCON);
+	sh_msiof_write(p, RMDR1, tmp);
 
-	tmp = 0xa0000000;
-	tmp |= cpol << 30; /* TSCKIZ */
-	tmp |= cpol << 28; /* RSCKIZ */
+	tmp = 0;
+	tmp |= CTR_TSCKIZ_SCK | cpol << CTR_TSCKIZ_POL_SHIFT;
+	tmp |= CTR_RSCKIZ_SCK | cpol << CTR_RSCKIZ_POL_SHIFT;
 
 	edge = cpol ^ !cpha;
 
-	tmp |= edge << 27; /* TEDG */
-	tmp |= edge << 26; /* REDG */
-	tmp |= (tx_hi_z ? 2 : 0) << 22; /* TXDIZ */
+	tmp |= edge << CTR_TEDG_SHIFT;
+	tmp |= edge << CTR_REDG_SHIFT;
+	tmp |= tx_hi_z ? CTR_TXDIZ_HIZ : CTR_TXDIZ_LOW;
 	sh_msiof_write(p, CTR, tmp);
 }
 
@@ -203,12 +251,12 @@ static void sh_msiof_spi_set_mode_regs(struct sh_msiof_spi_priv *p,
 				       const void *tx_buf, void *rx_buf,
 				       u32 bits, u32 words)
 {
-	u32 dr2 = ((bits - 1) << 24) | ((words - 1) << 16);
+	u32 dr2 = MDR2_BITLEN1(bits) | MDR2_WDLEN1(words);
 
 	if (tx_buf)
 		sh_msiof_write(p, TMDR2, dr2);
 	else
-		sh_msiof_write(p, TMDR2, dr2 | 1);
+		sh_msiof_write(p, TMDR2, dr2 | MDR2_GRPMASK1);
 
 	if (rx_buf)
 		sh_msiof_write(p, RMDR2, dr2);
@@ -694,8 +742,8 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 
 	/* The standard version of MSIOF use 64 word FIFOs */
-	p->tx_fifo_size = 64;
-	p->rx_fifo_size = 64;
+	p->tx_fifo_size = DEFAULT_TX_FIFO_SIZE;
+	p->rx_fifo_size = DEFAULT_RX_FIFO_SIZE;
 
 	/* Platform data may override FIFO sizes */
 	if (p->info->tx_fifo_override)
-- 
1.7.9.5

[PATCH 05/10] spi: sh-msiof: Use the core cs_gpio field, and make it optional

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

In current implementation, CS is controlled by GPIO, which is passed
through spi->controller_data.  However, the MSIOF HW module has a function
to output CS by itself, which is already enabled and actual switch will be
done by pinmux.

Store the GPIO number in the core cs_gpio field, and ignore it if it is
an invalid (negative) GPIO number.

Loosely based on a patch from Takashi Yoshii <takasi-y@ops.dti.ne.jp>.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
Cc: Takashi Yoshii <takasi-y@ops.dti.ne.jp>
---
 drivers/spi/spi-sh-msiof.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 79e14586049b..92515c1ececa 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -445,6 +445,21 @@ static int sh_msiof_spi_setup_transfer(struct spi_device *spi,
 	return spi_bitbang_setup_transfer(spi, t);
 }
 
+static int sh_msiof_spi_setup(struct spi_device *spi)
+{
+	struct device_node	*np = spi->master->dev.of_node;
+
+	if (!np) {
+		/*
+		 * Use spi->controller_data for CS (same strategy as spi_gpio),
+		 * if any. otherwise let HW control CS
+		 */
+		spi->cs_gpio = (uintptr_t)spi->controller_data;
+	}
+
+	return spi_bitbang_setup(spi);
+}
+
 static void sh_msiof_spi_chipselect(struct spi_device *spi, int is_on)
 {
 	struct sh_msiof_spi_priv *p = spi_master_get_devdata(spi->master);
@@ -470,8 +485,8 @@ static void sh_msiof_spi_chipselect(struct spi_device *spi, int is_on)
 					  !!(spi->mode & SPI_CS_HIGH));
 	}
 
-	/* use spi->controller data for CS (same strategy as spi_gpio) */
-	gpio_set_value((uintptr_t)spi->controller_data, value);
+	if (spi->cs_gpio >= 0)
+		gpio_set_value(spi->cs_gpio, value);
 
 	if (is_on = BITBANG_CS_INACTIVE) {
 		if (test_and_clear_bit(0, &p->flags)) {
@@ -758,7 +773,7 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	master->bus_num = pdev->id;
 	master->dev.of_node = pdev->dev.of_node;
 	master->num_chipselect = p->info->num_chipselect;
-	master->setup = spi_bitbang_setup;
+	master->setup = sh_msiof_spi_setup;
 	master->cleanup = spi_bitbang_cleanup;
 
 	p->bitbang.master = master;
-- 
1.7.9.5

[PATCH 06/10] spi: sh-msiof: Improve binding documentation

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

  - Add missing "interrupt-parent", "#address-cells", "#size-cells", and
    "clocks" properties,
  - Add missing default values for "renesas,tx-fifo-size" and
    "renesas,rx-fifo-size",
  - Add a reference to the pinctrl documentation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/spi/sh-msiof.txt |   24 ++++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index e6222106ca36..52cf5c78705b 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -1,12 +1,22 @@
 Renesas MSIOF spi controller
 
 Required properties:
-- compatible : 	"renesas,sh-msiof" for SuperH or
-		"renesas,sh-mobile-msiof" for SH Mobile series
-- reg : Offset and length of the register set for the device
-- interrupts : interrupt line used by MSIOF
+- compatible           : "renesas,sh-msiof" for SuperH, or
+			 "renesas,sh-mobile-msiof" for SH Mobile series.
+- reg                  : Offset and length of the register set for the device
+- interrupt-parent     : The phandle for the interrupt controller that
+			 services interrupts for this device
+- interrupts           : Interrupt specifier
+- #address-cells       : Must be <1>
+- #size-cells          : Must be <0>
 
 Optional properties:
-- num-cs		: total number of chip-selects
-- renesas,tx-fifo-size	: Overrides the default tx fifo size given in words
-- renesas,rx-fifo-size	: Overrides the default rx fifo size given in words
+- clocks               : Must contain a reference to the functional clock.
+- num-cs               : Total number of chip-selects
+- renesas,tx-fifo-size : Overrides the default tx fifo size given in words
+			 (default is 64)
+- renesas,rx-fifo-size : Overrides the default rx fifo size given in words
+			 (default is 64)
+
+Pinctrl properties might be needed, too.  See
+Documentation/devicetree/bindings/pinctrl/renesas,*.
-- 
1.7.9.5

[PATCH 07/10] spi: sh-msiof: Add support for R-Car H2 and M2

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Add support for the MSIOF variant in the R-Car H2 (r8a7790) and M2
(r8a7791) SoCs.

Binding documentation:
  - Add future-proof "renesas,msiof-<soctype>" compatible values,
  - Add example bindings.

Implementation:
  - MSIOF on R-Car H2 and M2 requires the transmission of dummy data if
    data is being received only (cfr. "Set SICTR.TSCKE to 1" and "Write
    dummy transmission data to SITFDR" in paragraph "Transmit and Receive
    Procedures" of the Hardware User's Manual).
  - As RX depends on TX, MSIOF on R-Car H2 and M2 also lacks the RSCR
    register (Receive Clock Select Register), and some bits in the RMDR1
    (Receive Mode Register 1) and TMDR2 (Transmit Mode Register 2)
    registers.
  - Use the recently introduced SPI_MASTER_MUST_TX flag to enable support
    for dummy transmission in the SPI core, and to differentiate from other
    MSIOF implementations in code paths that need this.
  - New DT compatible values ("renesas,msiof-r8a7790" and
    "renesas,msiof-r8a7791") are added, as well as new platform device
    names ("spi_r8a7790_msiof" and "spi_r8a7791_msiof").
  - Hardware features are indicated using a new struct sh_msiof_chipdata,
    which is used for both DT and legacy binding. For now this contains the
    SPI master flags only.

This is loosely based on a set of patches from Takashi Yoshii
<takasi-y@ops.dti.ne.jp>.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
Cc: Takashi Yoshii <takasi-y@ops.dti.ne.jp>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/spi/sh-msiof.txt |   21 +++++++-
 drivers/spi/spi-sh-msiof.c                         |   57 ++++++++++++++++----
 2 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index 52cf5c78705b..1528d30a6f6d 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -1,8 +1,13 @@
 Renesas MSIOF spi controller
 
 Required properties:
-- compatible           : "renesas,sh-msiof" for SuperH, or
+- compatible           : "renesas,msiof-<soctype>" for SoCs,
+			 "renesas,sh-msiof" for SuperH, or
 			 "renesas,sh-mobile-msiof" for SH Mobile series.
+			 Examples with soctypes are:
+			 "renesas,msiof-sh7724" (SH)
+			 "renesas,msiof-r8a7790" (R-Car H2)
+			 "renesas,msiof-r8a7791" (R-Car M2)
 - reg                  : Offset and length of the register set for the device
 - interrupt-parent     : The phandle for the interrupt controller that
 			 services interrupts for this device
@@ -20,3 +25,17 @@ Optional properties:
 
 Pinctrl properties might be needed, too.  See
 Documentation/devicetree/bindings/pinctrl/renesas,*.
+
+Example:
+
+	spi1: spi@e6e20000 {
+		compatible = "renesas,msiof-r8a7791";
+		reg = <0 0xe6e20000 0 0x0064>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 156 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7791_CLK_MSIOF0>;
+		num-cs = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 92515c1ececa..31624fb4997d 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -20,6 +20,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
@@ -29,11 +30,17 @@
 
 #include <asm/unaligned.h>
 
+
+struct sh_msiof_chipdata {
+	u16 master_flags;
+};
+
 struct sh_msiof_spi_priv {
 	struct spi_bitbang bitbang; /* must be first for spi_bitbang.c */
 	void __iomem *mapbase;
 	struct clk *clk;
 	struct platform_device *pdev;
+	const struct sh_msiof_chipdata *chipdata;
 	struct sh_msiof_spi_info *info;
 	struct completion done;
 	unsigned long flags;
@@ -210,7 +217,8 @@ static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p,
 	k = min_t(int, k, ARRAY_SIZE(sh_msiof_spi_clk_table) - 1);
 
 	sh_msiof_write(p, TSCR, sh_msiof_spi_clk_table[k].scr);
-	sh_msiof_write(p, RSCR, sh_msiof_spi_clk_table[k].scr);
+	if (!(p->chipdata->master_flags & SPI_MASTER_MUST_TX))
+		sh_msiof_write(p, RSCR, sh_msiof_spi_clk_table[k].scr);
 }
 
 static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p,
@@ -233,6 +241,10 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p,
 	tmp |= !cs_high << MDR1_SYNCAC_SHIFT;
 	tmp |= lsb_first << MDR1_BITLSB_SHIFT;
 	sh_msiof_write(p, TMDR1, tmp | MDR1_TRMD | TMDR1_PCON);
+	if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) {
+		/* These bits are reserved if RX needs TX */
+		tmp &= ~0x0000ffff;
+	}
 	sh_msiof_write(p, RMDR1, tmp);
 
 	tmp = 0;
@@ -253,7 +265,7 @@ static void sh_msiof_spi_set_mode_regs(struct sh_msiof_spi_priv *p,
 {
 	u32 dr2 = MDR2_BITLEN1(bits) | MDR2_WDLEN1(words);
 
-	if (tx_buf)
+	if (tx_buf || (p->chipdata->master_flags & SPI_MASTER_MUST_TX))
 		sh_msiof_write(p, TMDR2, dr2);
 	else
 		sh_msiof_write(p, TMDR2, dr2 | MDR2_GRPMASK1);
@@ -659,6 +671,23 @@ static u32 sh_msiof_spi_txrx_word(struct spi_device *spi, unsigned nsecs,
 }
 
 #ifdef CONFIG_OF
+static const struct sh_msiof_chipdata sh_data = {
+	.master_flags = 0,
+};
+
+static const struct sh_msiof_chipdata r8a779x_data = {
+	.master_flags = SPI_MASTER_MUST_TX,
+};
+
+static const struct of_device_id sh_msiof_match[] = {
+	{ .compatible = "renesas,sh-msiof",        .data = &sh_data },
+	{ .compatible = "renesas,sh-mobile-msiof", .data = &sh_data },
+	{ .compatible = "renesas,msiof-r8a7790",   .data = &r8a779x_data },
+	{ .compatible = "renesas,msiof-r8a7791",   .data = &r8a779x_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sh_msiof_match);
+
 static struct sh_msiof_spi_info *sh_msiof_spi_parse_dt(struct device *dev)
 {
 	struct sh_msiof_spi_info *info;
@@ -693,6 +722,7 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 {
 	struct resource	*r;
 	struct spi_master *master;
+	const struct of_device_id *of_id;
 	struct sh_msiof_spi_priv *p;
 	int i;
 	int ret;
@@ -706,10 +736,15 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	p = spi_master_get_devdata(master);
 
 	platform_set_drvdata(pdev, p);
-	if (pdev->dev.of_node)
+
+	of_id = of_match_device(sh_msiof_match, &pdev->dev);
+	if (of_id) {
+		p->chipdata = of_id->data;
 		p->info = sh_msiof_spi_parse_dt(&pdev->dev);
-	else
+	} else {
+		p->chipdata = (const void *)pdev->id_entry->driver_data;
 		p->info = dev_get_platdata(&pdev->dev);
+	}
 
 	if (!p->info) {
 		dev_err(&pdev->dev, "failed to obtain device info\n");
@@ -769,7 +804,7 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	/* init master and bitbang code */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 	master->mode_bits |= SPI_LSB_FIRST | SPI_3WIRE;
-	master->flags = 0;
+	master->flags = p->chipdata->master_flags;
 	master->bus_num = pdev->id;
 	master->dev.of_node = pdev->dev.of_node;
 	master->num_chipselect = p->info->num_chipselect;
@@ -810,18 +845,18 @@ static int sh_msiof_spi_remove(struct platform_device *pdev)
 	return ret;
 }
 
-#ifdef CONFIG_OF
-static const struct of_device_id sh_msiof_match[] = {
-	{ .compatible = "renesas,sh-msiof", },
-	{ .compatible = "renesas,sh-mobile-msiof", },
+static struct platform_device_id spi_driver_ids[] = {
+	{ "spi_sh_msiof",	(kernel_ulong_t)&sh_data },
+	{ "spi_r8a7790_msiof",	(kernel_ulong_t)&r8a779x_data },
+	{ "spi_r8a7791_msiof",	(kernel_ulong_t)&r8a779x_data },
 	{},
 };
-MODULE_DEVICE_TABLE(of, sh_msiof_match);
-#endif
+MODULE_DEVICE_TABLE(platform, spi_driver_ids);
 
 static struct platform_driver sh_msiof_spi_drv = {
 	.probe		= sh_msiof_spi_probe,
 	.remove		= sh_msiof_spi_remove,
+	.id_table	= spi_driver_ids,
 	.driver		= {
 		.name		= "spi_sh_msiof",
 		.owner		= THIS_MODULE,
-- 
1.7.9.5

[PATCH 08/10] spi: sh-msiof: Move clock management to (un)prepare_message()

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Move clock management and pin configuration from the bitbang chipselect()
method to the SPI core prepare_message() and unprepare_message() methods.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/spi/spi-sh-msiof.c |   56 +++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 31624fb4997d..7add4be37987 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -472,9 +472,40 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
 	return spi_bitbang_setup(spi);
 }
 
+static int sh_msiof_prepare_message(struct spi_master *master,
+				    struct spi_message *msg)
+{
+	struct sh_msiof_spi_priv *p = spi_master_get_devdata(master);
+	const struct spi_device *spi = msg->spi;
+
+	if (!test_and_set_bit(0, &p->flags)) {
+		pm_runtime_get_sync(&p->pdev->dev);
+		clk_enable(p->clk);
+	}
+
+	/* Configure pins before asserting CS */
+	sh_msiof_spi_set_pin_regs(p, !!(spi->mode & SPI_CPOL),
+				  !!(spi->mode & SPI_CPHA),
+				  !!(spi->mode & SPI_3WIRE),
+				  !!(spi->mode & SPI_LSB_FIRST),
+				  !!(spi->mode & SPI_CS_HIGH));
+	return 0;
+}
+
+static int sh_msiof_unprepare_message(struct spi_master *master,
+				      struct spi_message *msg)
+{
+	struct sh_msiof_spi_priv *p = spi_master_get_devdata(master);
+
+	if (test_and_clear_bit(0, &p->flags)) {
+		clk_disable(p->clk);
+		pm_runtime_put(&p->pdev->dev);
+	}
+	return 0;
+}
+
 static void sh_msiof_spi_chipselect(struct spi_device *spi, int is_on)
 {
-	struct sh_msiof_spi_priv *p = spi_master_get_devdata(spi->master);
 	int value;
 
 	/* chip select is active low unless SPI_CS_HIGH is set */
@@ -483,29 +514,8 @@ static void sh_msiof_spi_chipselect(struct spi_device *spi, int is_on)
 	else
 		value = (is_on = BITBANG_CS_ACTIVE) ? 0 : 1;
 
-	if (is_on = BITBANG_CS_ACTIVE) {
-		if (!test_and_set_bit(0, &p->flags)) {
-			pm_runtime_get_sync(&p->pdev->dev);
-			clk_enable(p->clk);
-		}
-
-		/* Configure pins before asserting CS */
-		sh_msiof_spi_set_pin_regs(p, !!(spi->mode & SPI_CPOL),
-					  !!(spi->mode & SPI_CPHA),
-					  !!(spi->mode & SPI_3WIRE),
-					  !!(spi->mode & SPI_LSB_FIRST),
-					  !!(spi->mode & SPI_CS_HIGH));
-	}
-
 	if (spi->cs_gpio >= 0)
 		gpio_set_value(spi->cs_gpio, value);
-
-	if (is_on = BITBANG_CS_INACTIVE) {
-		if (test_and_clear_bit(0, &p->flags)) {
-			clk_disable(p->clk);
-			pm_runtime_put(&p->pdev->dev);
-		}
-	}
 }
 
 static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
@@ -810,6 +820,8 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	master->num_chipselect = p->info->num_chipselect;
 	master->setup = sh_msiof_spi_setup;
 	master->cleanup = spi_bitbang_cleanup;
+	master->prepare_message = sh_msiof_prepare_message;
+	master->unprepare_message = sh_msiof_unprepare_message;
 
 	p->bitbang.master = master;
 	p->bitbang.chipselect = sh_msiof_spi_chipselect;
-- 
1.7.9.5

[PATCH 09/10] spi: sh-msiof: Convert to let spi core validate xfer->bits_per_word

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Set bits_per_word_mask so the spi core will reject transfers that attempt
to use an unsupported bits_per_word value.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/spi/spi-sh-msiof.c |   19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 7add4be37987..bd300fcb7116 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -441,22 +441,6 @@ static u32 sh_msiof_spi_hz(struct spi_device *spi, struct spi_transfer *t)
 	return hz;
 }
 
-static int sh_msiof_spi_setup_transfer(struct spi_device *spi,
-				       struct spi_transfer *t)
-{
-	int bits;
-
-	/* noting to check hz values against since parent clock is disabled */
-
-	bits = sh_msiof_spi_bits(spi, t);
-	if (bits < 8)
-		return -EINVAL;
-	if (bits > 32)
-		return -EINVAL;
-
-	return spi_bitbang_setup_transfer(spi, t);
-}
-
 static int sh_msiof_spi_setup(struct spi_device *spi)
 {
 	struct device_node	*np = spi->master->dev.of_node;
@@ -822,10 +806,11 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	master->cleanup = spi_bitbang_cleanup;
 	master->prepare_message = sh_msiof_prepare_message;
 	master->unprepare_message = sh_msiof_unprepare_message;
+	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(8, 32);
 
 	p->bitbang.master = master;
 	p->bitbang.chipselect = sh_msiof_spi_chipselect;
-	p->bitbang.setup_transfer = sh_msiof_spi_setup_transfer;
+	p->bitbang.setup_transfer = spi_bitbang_setup_transfer;
 	p->bitbang.txrx_bufs = sh_msiof_spi_txrx;
 	p->bitbang.txrx_word[SPI_MODE_0] = sh_msiof_spi_txrx_word;
 	p->bitbang.txrx_word[SPI_MODE_1] = sh_msiof_spi_txrx_word;
-- 
1.7.9.5

[PATCH 10/10] spi: sh-msiof: Use core message handling instead of spi-bitbang

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

The only remaining feature of spi-bitbang used by this driver is the
chipselect() callback, which just does conditional GPIO.
This is handled fine by the SPI core's spi_set_cs(), hence switch the
driver to use the core message handling through our own transfer_one()
method.

Remove the call to spi_master_put() in sh_msiof_spi_remove(), as our SPI
master is now registered using devm_spi_register_master()
(spi_bitbang_start() uses the non-managed version).

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/spi/Kconfig        |    1 -
 drivers/spi/spi-sh-msiof.c |   65 ++++++++++++--------------------------------
 2 files changed, 17 insertions(+), 49 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 339da4b15ec2..35d5c029393a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -416,7 +416,6 @@ config SPI_SH_MSIOF
 	tristate "SuperH MSIOF SPI controller"
 	depends on HAVE_CLK
 	depends on SUPERH || ARCH_SHMOBILE || COMPILE_TEST
-	select SPI_BITBANG
 	help
 	  SPI driver for SuperH and SH Mobile MSIOF blocks.
 
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index bd300fcb7116..642b0b637df1 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -26,7 +26,6 @@
 
 #include <linux/spi/sh_msiof.h>
 #include <linux/spi/spi.h>
-#include <linux/spi/spi_bitbang.h>
 
 #include <asm/unaligned.h>
 
@@ -36,7 +35,6 @@ struct sh_msiof_chipdata {
 };
 
 struct sh_msiof_spi_priv {
-	struct spi_bitbang bitbang; /* must be first for spi_bitbang.c */
 	void __iomem *mapbase;
 	struct clk *clk;
 	struct platform_device *pdev;
@@ -452,8 +450,7 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
 		 */
 		spi->cs_gpio = (uintptr_t)spi->controller_data;
 	}
-
-	return spi_bitbang_setup(spi);
+	return 0;
 }
 
 static int sh_msiof_prepare_message(struct spi_master *master,
@@ -488,20 +485,6 @@ static int sh_msiof_unprepare_message(struct spi_master *master,
 	return 0;
 }
 
-static void sh_msiof_spi_chipselect(struct spi_device *spi, int is_on)
-{
-	int value;
-
-	/* chip select is active low unless SPI_CS_HIGH is set */
-	if (spi->mode & SPI_CS_HIGH)
-		value = (is_on = BITBANG_CS_ACTIVE) ? 1 : 0;
-	else
-		value = (is_on = BITBANG_CS_ACTIVE) ? 0 : 1;
-
-	if (spi->cs_gpio >= 0)
-		gpio_set_value(spi->cs_gpio, value);
-}
-
 static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
 				  void (*tx_fifo)(struct sh_msiof_spi_priv *,
 						  const void *, int, int),
@@ -571,9 +554,11 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
 	return ret;
 }
 
-static int sh_msiof_spi_txrx(struct spi_device *spi, struct spi_transfer *t)
+static int sh_msiof_transfer_one(struct spi_master *master,
+				 struct spi_device *spi,
+				 struct spi_transfer *t)
 {
-	struct sh_msiof_spi_priv *p = spi_master_get_devdata(spi->master);
+	struct sh_msiof_spi_priv *p = spi_master_get_devdata(master);
 	void (*tx_fifo)(struct sh_msiof_spi_priv *, const void *, int, int);
 	void (*rx_fifo)(struct sh_msiof_spi_priv *, void *, int, int);
 	int bits;
@@ -654,13 +639,6 @@ static int sh_msiof_spi_txrx(struct spi_device *spi, struct spi_transfer *t)
 		words -= n;
 	}
 
-	return bytes_done;
-}
-
-static u32 sh_msiof_spi_txrx_word(struct spi_device *spi, unsigned nsecs,
-				  u32 word, u8 bits)
-{
-	BUG(); /* unused but needed by bitbang code */
 	return 0;
 }
 
@@ -795,7 +773,7 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	if (p->info->rx_fifo_override)
 		p->rx_fifo_size = p->info->rx_fifo_override;
 
-	/* init master and bitbang code */
+	/* init master code */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 	master->mode_bits |= SPI_LSB_FIRST | SPI_3WIRE;
 	master->flags = p->chipdata->master_flags;
@@ -803,24 +781,20 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	master->dev.of_node = pdev->dev.of_node;
 	master->num_chipselect = p->info->num_chipselect;
 	master->setup = sh_msiof_spi_setup;
-	master->cleanup = spi_bitbang_cleanup;
 	master->prepare_message = sh_msiof_prepare_message;
 	master->unprepare_message = sh_msiof_unprepare_message;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(8, 32);
+	master->transfer_one = sh_msiof_transfer_one;
 
-	p->bitbang.master = master;
-	p->bitbang.chipselect = sh_msiof_spi_chipselect;
-	p->bitbang.setup_transfer = spi_bitbang_setup_transfer;
-	p->bitbang.txrx_bufs = sh_msiof_spi_txrx;
-	p->bitbang.txrx_word[SPI_MODE_0] = sh_msiof_spi_txrx_word;
-	p->bitbang.txrx_word[SPI_MODE_1] = sh_msiof_spi_txrx_word;
-	p->bitbang.txrx_word[SPI_MODE_2] = sh_msiof_spi_txrx_word;
-	p->bitbang.txrx_word[SPI_MODE_3] = sh_msiof_spi_txrx_word;
+	ret = devm_spi_register_master(&pdev->dev, master);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "spi_register_master error.\n");
+		goto err2;
+	}
 
-	ret = spi_bitbang_start(&p->bitbang);
-	if (ret = 0)
-		return 0;
+	return 0;
 
+ err2:
 	pm_runtime_disable(&pdev->dev);
 	clk_unprepare(p->clk);
  err1:
@@ -831,15 +805,10 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 static int sh_msiof_spi_remove(struct platform_device *pdev)
 {
 	struct sh_msiof_spi_priv *p = platform_get_drvdata(pdev);
-	int ret;
 
-	ret = spi_bitbang_stop(&p->bitbang);
-	if (!ret) {
-		pm_runtime_disable(&pdev->dev);
-		clk_unprepare(p->clk);
-		spi_master_put(p->bitbang.master);
-	}
-	return ret;
+	pm_runtime_disable(&pdev->dev);
+	clk_unprepare(p->clk);
+	return 0;
 }
 
 static struct platform_device_id spi_driver_ids[] = {
-- 
1.7.9.5

Re: [PATCH 07/10] spi: sh-msiof: Add support for R-Car H2 and M2

[Magnus Damm]

On Thu, Feb 20, 2014 at 11:43 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>
> Add support for the MSIOF variant in the R-Car H2 (r8a7790) and M2
> (r8a7791) SoCs.
>
> Binding documentation:
>   - Add future-proof "renesas,msiof-<soctype>" compatible values,
>   - Add example bindings.
>
> Implementation:
>   - MSIOF on R-Car H2 and M2 requires the transmission of dummy data if
>     data is being received only (cfr. "Set SICTR.TSCKE to 1" and "Write
>     dummy transmission data to SITFDR" in paragraph "Transmit and Receive
>     Procedures" of the Hardware User's Manual).
>   - As RX depends on TX, MSIOF on R-Car H2 and M2 also lacks the RSCR
>     register (Receive Clock Select Register), and some bits in the RMDR1
>     (Receive Mode Register 1) and TMDR2 (Transmit Mode Register 2)
>     registers.
>   - Use the recently introduced SPI_MASTER_MUST_TX flag to enable support
>     for dummy transmission in the SPI core, and to differentiate from other
>     MSIOF implementations in code paths that need this.
>   - New DT compatible values ("renesas,msiof-r8a7790" and
>     "renesas,msiof-r8a7791") are added, as well as new platform device
>     names ("spi_r8a7790_msiof" and "spi_r8a7791_msiof").
>   - Hardware features are indicated using a new struct sh_msiof_chipdata,
>     which is used for both DT and legacy binding. For now this contains the
>     SPI master flags only.
>
> This is loosely based on a set of patches from Takashi Yoshii
> <takasi-y@ops.dti.ne.jp>.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> Cc: Takashi Yoshii <takasi-y@ops.dti.ne.jp>
> Cc: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/spi/sh-msiof.txt |   21 +++++++-
>  drivers/spi/spi-sh-msiof.c                         |   57 ++++++++++++++++----
>  2 files changed, 66 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 92515c1ececa..31624fb4997d 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -659,6 +671,23 @@ static u32 sh_msiof_spi_txrx_word(struct spi_device *spi, unsigned nsecs,
>  }
>
>  #ifdef CONFIG_OF
> +static const struct sh_msiof_chipdata sh_data = {
> +       .master_flags = 0,
> +};
> +
> +static const struct sh_msiof_chipdata r8a779x_data = {
> +       .master_flags = SPI_MASTER_MUST_TX,
> +};
> +
> +static const struct of_device_id sh_msiof_match[] = {
> +       { .compatible = "renesas,sh-msiof",        .data = &sh_data },
> +       { .compatible = "renesas,sh-mobile-msiof", .data = &sh_data },
> +       { .compatible = "renesas,msiof-r8a7790",   .data = &r8a779x_data },
> +       { .compatible = "renesas,msiof-r8a7791",   .data = &r8a779x_data },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, sh_msiof_match);

Hi Geert,

Thanks for your patches. They all look good in general I think.

On thing stuck out a bit with the bindings. I can see that you specify
both fifo size and use the SoC suffix for the r8a7790 and r8a7791
bindings. Isn't that a bit of redundant information there, if we know
that the SoC is r8a7790 or r8a7791 then can't we simply put that
information in r8a779x_data above and perhaps keep the binding
simpler? Perhaps same thing applies to other properties as well?

Cheers,

/ magnus

Re: [PATCH 07/10] spi: sh-msiof: Add support for R-Car H2 and M2

[Geert Uytterhoeven]

On Thu, Feb 20, 2014 at 4:41 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On thing stuck out a bit with the bindings. I can see that you specify
> both fifo size and use the SoC suffix for the r8a7790 and r8a7791
> bindings. Isn't that a bit of redundant information there, if we know
> that the SoC is r8a7790 or r8a7791 then can't we simply put that
> information in r8a779x_data above and perhaps keep the binding
> simpler? Perhaps same thing applies to other properties as well?

"renesas,tx-fifo-size" and "renesas,rx-fifo-size" are part of the existing
bindings, so I'm a bit reluctant to change these.
Hmm, since the original bindings didn't specify the default values,
I could make them chip-specific, though.

The only other property is "num-cs", which can have any value if you
start using the generic "cs-gpios".

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH 01/10] spi: sh-msiof: Fix SPI bus population from DT

[Mark Brown]

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

On Thu, Feb 20, 2014 at 03:43:00PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> DT doesn't instantiate SPI children if spi_master.dev.of_node is not set up
> properly.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 02/10] spi: sh-msiof: Typo in comment s/tx/rx/

[Mark Brown]

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

On Thu, Feb 20, 2014 at 03:43:01PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 03/10] spi: sh-msiof: Change hz from unsigned long to u32

[Mark Brown]

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

On Thu, Feb 20, 2014 at 03:43:02PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Both spi_transfer.speed_hz and spi_master.max_speed_hz are u32

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 04/10] spi: sh-msiof: Add more register documentation

[Mark Brown]

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

On Thu, Feb 20, 2014 at 03:43:03PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 05/10] spi: sh-msiof: Use the core cs_gpio field, and make it optional

[Mark Brown]

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

On Thu, Feb 20, 2014 at 03:43:04PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> In current implementation, CS is controlled by GPIO, which is passed
> through spi->controller_data.  However, the MSIOF HW module has a function
> to output CS by itself, which is already enabled and actual switch will be
> done by pinmux.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 07/10] spi: sh-msiof: Add support for R-Car H2 and M2

[Mark Brown]

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

On Thu, Feb 20, 2014 at 05:13:16PM +0100, Geert Uytterhoeven wrote:

> "renesas,tx-fifo-size" and "renesas,rx-fifo-size" are part of the existing
> bindings, so I'm a bit reluctant to change these.
> Hmm, since the original bindings didn't specify the default values,
> I could make them chip-specific, though.

That sounds like a good idea, you could also change the properties to be
deprecatedn and recommend that people just don't bother setting them at
all.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 08/10] spi: sh-msiof: Move clock management to (un)prepare_message()

[Mark Brown]

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

On Thu, Feb 20, 2014 at 03:43:07PM +0100, Geert Uytterhoeven wrote:

> +	if (!test_and_set_bit(0, &p->flags)) {
> +		pm_runtime_get_sync(&p->pdev->dev);
> +		clk_enable(p->clk);
> +	}

That test_and_set_bit() is a bit odd - what's going on there, perhaps a
comment is in order?  I'd also like to see return value checks, though
the original didn't have them so it's not a blocker, and ideally that
should be clk_prepare_enable().  I guess the clock stuff could even be
moved inside the runtime PM callbacks?  Again not a blocker if the
existing code doesn't have things but it'd be nice to do.

There's also auto_runtime_pm in the SPI core which will do the runtime
PM for you if you can do it unconditionally.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 07/10] spi: sh-msiof: Add support for R-Car H2 and M2

[Magnus Damm]

Hi Geert,

On Fri, Feb 21, 2014 at 1:13 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Feb 20, 2014 at 4:41 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> On thing stuck out a bit with the bindings. I can see that you specify
>> both fifo size and use the SoC suffix for the r8a7790 and r8a7791
>> bindings. Isn't that a bit of redundant information there, if we know
>> that the SoC is r8a7790 or r8a7791 then can't we simply put that
>> information in r8a779x_data above and perhaps keep the binding
>> simpler? Perhaps same thing applies to other properties as well?
>
> "renesas,tx-fifo-size" and "renesas,rx-fifo-size" are part of the existing
> bindings, so I'm a bit reluctant to change these.
> Hmm, since the original bindings didn't specify the default values,
> I could make them chip-specific, though.

Thanks, yes I think treating the "renesas,tx-fifo-size" and
"renesas,rx-fifo-size" bindings as optional and allow us to rely on
chip-specific default values makes sense.

> The only other property is "num-cs", which can have any value if you
> start using the generic "cs-gpios".

I see. It looks to me that such a setting is board-specific, so what
is a sane default in the SoC DTSI? You seem to have it set to "1"
today, but if the board is supposed to override it anyway then do we
need to set it?

Anyway, "num-cs" is a minor detail so no need to bother about that too much!

Cheers,

/ magnus

Re: [PATCH 07/10] spi: sh-msiof: Add support for R-Car H2 and M2

[Geert Uytterhoeven]

Hi Magnus,

On Mon, Feb 24, 2014 at 3:45 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On Fri, Feb 21, 2014 at 1:13 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, Feb 20, 2014 at 4:41 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> On thing stuck out a bit with the bindings. I can see that you specify
>>> both fifo size and use the SoC suffix for the r8a7790 and r8a7791
>>> bindings. Isn't that a bit of redundant information there, if we know
>>> that the SoC is r8a7790 or r8a7791 then can't we simply put that
>>> information in r8a779x_data above and perhaps keep the binding
>>> simpler? Perhaps same thing applies to other properties as well?
>>
>> "renesas,tx-fifo-size" and "renesas,rx-fifo-size" are part of the existing
>> bindings, so I'm a bit reluctant to change these.
>> Hmm, since the original bindings didn't specify the default values,
>> I could make them chip-specific, though.
>
> Thanks, yes I think treating the "renesas,tx-fifo-size" and
> "renesas,rx-fifo-size" bindings as optional and allow us to rely on
> chip-specific default values makes sense.
>
>> The only other property is "num-cs", which can have any value if you
>> start using the generic "cs-gpios".
>
> I see. It looks to me that such a setting is board-specific, so what
> is a sane default in the SoC DTSI? You seem to have it set to "1"
> today, but if the board is supposed to override it anyway then do we
> need to set it?
>
> Anyway, "num-cs" is a minor detail so no need to bother about that too much!

In v2, I'll drop the "num-cs" from the dtsi, so it will default to the driver's
default (which is 1, for the simple case of using hardware controlled CS).

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH 07/10] spi: sh-msiof: Add support for R-Car H2 and M2

[Magnus Damm]

Hi Geert,

On Mon, Feb 24, 2014 at 4:39 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Mon, Feb 24, 2014 at 3:45 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> On Fri, Feb 21, 2014 at 1:13 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Thu, Feb 20, 2014 at 4:41 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>> On thing stuck out a bit with the bindings. I can see that you specify
>>>> both fifo size and use the SoC suffix for the r8a7790 and r8a7791
>>>> bindings. Isn't that a bit of redundant information there, if we know
>>>> that the SoC is r8a7790 or r8a7791 then can't we simply put that
>>>> information in r8a779x_data above and perhaps keep the binding
>>>> simpler? Perhaps same thing applies to other properties as well?
>>>
>>> "renesas,tx-fifo-size" and "renesas,rx-fifo-size" are part of the existing
>>> bindings, so I'm a bit reluctant to change these.
>>> Hmm, since the original bindings didn't specify the default values,
>>> I could make them chip-specific, though.
>>
>> Thanks, yes I think treating the "renesas,tx-fifo-size" and
>> "renesas,rx-fifo-size" bindings as optional and allow us to rely on
>> chip-specific default values makes sense.
>>
>>> The only other property is "num-cs", which can have any value if you
>>> start using the generic "cs-gpios".
>>
>> I see. It looks to me that such a setting is board-specific, so what
>> is a sane default in the SoC DTSI? You seem to have it set to "1"
>> today, but if the board is supposed to override it anyway then do we
>> need to set it?
>>
>> Anyway, "num-cs" is a minor detail so no need to bother about that too much!
>
> In v2, I'll drop the "num-cs" from the dtsi, so it will default to the driver's
> default (which is 1, for the simple case of using hardware controlled CS).

Sounds good, thanks for your help!

/ magnus

Re: [PATCH 08/10] spi: sh-msiof: Move clock management to (un)prepare_message()

[Geert Uytterhoeven]

Hi Mark, Magnus,

On Sat, Feb 22, 2014 at 4:27 AM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Feb 20, 2014 at 03:43:07PM +0100, Geert Uytterhoeven wrote:
>
>> +     if (!test_and_set_bit(0, &p->flags)) {
>> +             pm_runtime_get_sync(&p->pdev->dev);
>> +             clk_enable(p->clk);
>> +     }
>
> That test_and_set_bit() is a bit odd - what's going on there, perhaps a
> comment is in order?  I'd also like to see return value checks, though

My first guess was to support multiple CS, but you can't have multiple
active SPI slaves at the same time.

Perhaps it's because the bitbang core may call the .chipselect() callback
multiple times with is_on = BITBANG_CS_ACTIVE, and obviously the
clock should be enabled/disabled only once?
The current code doesn't seem to do that, but perhaps it was different when
the sh-msiof driver was written?

Ah, there's also the initial .chipselect(..., BITBANG_CS_INACTIVE) call
in spi_bitbang_setup(), which should not disable the clock.
But it should still call sh_msiof_spi_set_pin_regs() and set the optional
GPIO CS. Which is no longer done after my series. I'll fix that.

Magnus, do you remember the rationale for the test_and_set_bit()?

Anyway, it seems safe to remove it, as .(un)prepare_message() is
guaranteed to be called in matching pairs.

> the original didn't have them so it's not a blocker, and ideally that
> should be clk_prepare_enable().  I guess the clock stuff could even be
> moved inside the runtime PM callbacks?  Again not a blocker if the
> existing code doesn't have things but it'd be nice to do.
>
> There's also auto_runtime_pm in the SPI core which will do the runtime
> PM for you if you can do it unconditionally.

As you mention yourself, all of this code existed before. It just got moved,
to make the conversion to the SPI core message handling easier.
If you don't mind, I'd like to defer these, and tackle runtime PM later.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH 08/10] spi: sh-msiof: Move clock management to (un)prepare_message()

[Mark Brown]

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

On Mon, Feb 24, 2014 at 10:48:26AM +0100, Geert Uytterhoeven wrote:

> Perhaps it's because the bitbang core may call the .chipselect() callback
> multiple times with is_on == BITBANG_CS_ACTIVE, and obviously the
> clock should be enabled/disabled only once?
> The current code doesn't seem to do that, but perhaps it was different when
> the sh-msiof driver was written?

Possibly, or perhaps someone was being overly cautious.

> As you mention yourself, all of this code existed before. It just got moved,
> to make the conversion to the SPI core message handling easier.
> If you don't mind, I'd like to defer these, and tackle runtime PM later.

Sure.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 08/10] spi: sh-msiof: Move clock management to (un)prepare_message()

[Magnus Damm]

Hi Geert,

On Mon, Feb 24, 2014 at 6:48 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Mark, Magnus,
>
> On Sat, Feb 22, 2014 at 4:27 AM, Mark Brown <broonie@kernel.org> wrote:
>> On Thu, Feb 20, 2014 at 03:43:07PM +0100, Geert Uytterhoeven wrote:
>>
>>> +     if (!test_and_set_bit(0, &p->flags)) {
>>> +             pm_runtime_get_sync(&p->pdev->dev);
>>> +             clk_enable(p->clk);
>>> +     }
>>
>> That test_and_set_bit() is a bit odd - what's going on there, perhaps a
>> comment is in order?  I'd also like to see return value checks, though
>
> My first guess was to support multiple CS, but you can't have multiple
> active SPI slaves at the same time.
>
> Perhaps it's because the bitbang core may call the .chipselect() callback
> multiple times with is_on = BITBANG_CS_ACTIVE, and obviously the
> clock should be enabled/disabled only once?
> The current code doesn't seem to do that, but perhaps it was different when
> the sh-msiof driver was written?
>
> Ah, there's also the initial .chipselect(..., BITBANG_CS_INACTIVE) call
> in spi_bitbang_setup(), which should not disable the clock.
> But it should still call sh_msiof_spi_set_pin_regs() and set the optional
> GPIO CS. Which is no longer done after my series. I'll fix that.
>
> Magnus, do you remember the rationale for the test_and_set_bit()?

Sorry, I don't remember. Perhaps it was related to the CS bitbang handling.

> Anyway, it seems safe to remove it, as .(un)prepare_message() is
> guaranteed to be called in matching pairs.

Yes, I agree!

Thanks,

/ magnus