[PATCH v2 0/3] spi: sh-msiof: Add reset of registers before starting transfer

[PATCH v2 0/3] spi: sh-msiof: Add reset of registers before starting transfer

[Geert Uytterhoeven]

	Hi Mark,

This patch series resets the MSIOF registers before starting a transfer,
to comply with the procedure documented in the Hardware Manual.

Changes compared to v1:
  - New patches 1 and 2, in response to review comments on v1
    (although sh_msiof_modify_ctr_wait() didn't turn out to be suitable
    for calling by sh_msiof_spi_reset_regs()),
  - Use BIT(),
  - Use readl_poll_timeout_atomic().

Thanks!

Geert Uytterhoeven (3):
  spi: sh-msiof: Use BIT() and GENMASK()
  spi: sh-msiof: Use readl_poll_timeout_atomic() instead of open-coding
  spi: sh-msiof: Add reset of registers before starting transfer

 drivers/spi/spi-sh-msiof.c | 204 ++++++++++++++++++++-----------------
 1 file changed, 108 insertions(+), 96 deletions(-)

-- 
2.17.1

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 v2 1/3] spi: sh-msiof: Use BIT() and GENMASK()

[Geert Uytterhoeven]

Improve maintainability by converting the register bit, bitmask, and
bitfield definitions from hexadecimal constants to constructs using
BIT(), GENMASK(), or "val << shift".

Suggested-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
All definitions keep their original value (checked by comparing
assembler output).

v2:
  - New.
---
 drivers/spi/spi-sh-msiof.c | 174 ++++++++++++++++++-------------------
 1 file changed, 87 insertions(+), 87 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index e2eb466db10a4d74..6edc76636e81a801 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -82,111 +82,111 @@ struct sh_msiof_spi_priv {
 #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_DTDL_SHIFT		 20 /* Data Pin Bit Delay for MSIOF_SYNC */
-#define MDR1_SYNCDL_SHIFT	 16 /* Frame Sync Signal Timing Delay */
-#define MDR1_FLD_MASK	 0x0000000c /* Frame Sync Signal Interval (0-3) */
-#define MDR1_FLD_SHIFT		  2
-#define MDR1_XXSTP	 0x00000001 /* Transmission/Reception Stop on FIFO */
+#define MDR1_TRMD	   BIT(31)  /* Transfer Mode (1 = Master mode) */
+#define MDR1_SYNCMD_MASK   GENMASK(29, 28) /* SYNC Mode */
+#define MDR1_SYNCMD_SPI	   (2 << 28)/*   Level mode/SPI */
+#define MDR1_SYNCMD_LR	   (3 << 28)/*   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_DTDL_SHIFT	   20       /* Data Pin Bit Delay for MSIOF_SYNC */
+#define MDR1_SYNCDL_SHIFT  16       /* Frame Sync Signal Timing Delay */
+#define MDR1_FLD_MASK	   GENMASK(3, 2) /* Frame Sync Signal Interval (0-3) */
+#define MDR1_FLD_SHIFT	   2
+#define MDR1_XXSTP	   BIT(0)   /* Transmission/Reception Stop on FIFO */
 /* TMDR1 */
-#define TMDR1_PCON	 0x40000000 /* Transfer Signal Connection */
-#define TMDR1_SYNCCH_MASK 0xc000000 /* Synchronization Signal Channel Select */
-#define TMDR1_SYNCCH_SHIFT	 26 /* 0=MSIOF_SYNC, 1=MSIOF_SS1, 2=MSIOF_SS2 */
+#define TMDR1_PCON	   BIT(30)  /* Transfer Signal Connection */
+#define TMDR1_SYNCCH_MASK  GENMASK(27, 26) /* Sync Signal Channel Select */
+#define TMDR1_SYNCCH_SHIFT 26       /* 0=MSIOF_SYNC, 1=MSIOF_SS1, 2=MSIOF_SS2 */
 
 /* 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) */
+#define MDR2_GRPMASK1	BIT(0)      /* Group Output Mask 1 (SH, A1) */
 
 /* TSCR and RSCR */
-#define SCR_BRPS_MASK	    0x1f00 /* Prescaler Setting (1-32) */
+#define SCR_BRPS_MASK	GENMASK(12, 8) /* 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
+#define SCR_BRDV_MASK	GENMASK(2, 0) /* Baud Rate Generator's Division Ratio */
+#define SCR_BRDV_DIV_2	0
+#define SCR_BRDV_DIV_4	1
+#define SCR_BRDV_DIV_8	2
+#define SCR_BRDV_DIV_16	3
+#define SCR_BRDV_DIV_32	4
+#define SCR_BRDV_DIV_1	7
 
 /* 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 */
+#define CTR_TSCKIZ_MASK	GENMASK(31, 30) /* Transmit Clock I/O Polarity Select */
+#define CTR_TSCKIZ_SCK	BIT(31)   /*   Disable SCK when TX disabled */
+#define CTR_TSCKIZ_POL_SHIFT 30   /*   Transmit Clock Polarity */
+#define CTR_RSCKIZ_MASK	GENMASK(29, 28) /* Receive Clock Polarity Select */
+#define CTR_RSCKIZ_SCK	BIT(29)   /*   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	GENMASK(23, 22) /* Pin Output When TX is Disabled */
+#define CTR_TXDIZ_LOW	(0 << 22) /*   0 */
+#define CTR_TXDIZ_HIGH	(1 << 22) /*   1 */
+#define CTR_TXDIZ_HIZ	(2 << 22) /*   High-impedance */
+#define CTR_TSCKE	BIT(15)   /* Transmit Serial Clock Output Enable */
+#define CTR_TFSE	BIT(14)   /* Transmit Frame Sync Signal Output Enable */
+#define CTR_TXE		BIT(9)    /* Transmit Enable */
+#define CTR_RXE		BIT(8)    /* Receive Enable */
 
 /* FCTR */
-#define FCTR_TFWM_MASK	0xe0000000 /* Transmit FIFO Watermark */
-#define FCTR_TFWM_64	0x00000000 /*  Transfer Request when 64 empty stages */
-#define FCTR_TFWM_32	0x20000000 /*  Transfer Request when 32 empty stages */
-#define FCTR_TFWM_24	0x40000000 /*  Transfer Request when 24 empty stages */
-#define FCTR_TFWM_16	0x60000000 /*  Transfer Request when 16 empty stages */
-#define FCTR_TFWM_12	0x80000000 /*  Transfer Request when 12 empty stages */
-#define FCTR_TFWM_8	0xa0000000 /*  Transfer Request when 8 empty stages */
-#define FCTR_TFWM_4	0xc0000000 /*  Transfer Request when 4 empty stages */
-#define FCTR_TFWM_1	0xe0000000 /*  Transfer Request when 1 empty stage */
-#define FCTR_TFUA_MASK	0x07f00000 /* Transmit FIFO Usable Area */
-#define FCTR_TFUA_SHIFT		20
+#define FCTR_TFWM_MASK	GENMASK(31, 29) /* Transmit FIFO Watermark */
+#define FCTR_TFWM_64	(0 << 29) /*  Transfer Request when 64 empty stages */
+#define FCTR_TFWM_32	(1 << 29) /*  Transfer Request when 32 empty stages */
+#define FCTR_TFWM_24	(2 << 29) /*  Transfer Request when 24 empty stages */
+#define FCTR_TFWM_16	(3 << 29) /*  Transfer Request when 16 empty stages */
+#define FCTR_TFWM_12	(4 << 29) /*  Transfer Request when 12 empty stages */
+#define FCTR_TFWM_8	(5 << 29) /*  Transfer Request when 8 empty stages */
+#define FCTR_TFWM_4	(6 << 29) /*  Transfer Request when 4 empty stages */
+#define FCTR_TFWM_1	(7 << 29) /*  Transfer Request when 1 empty stage */
+#define FCTR_TFUA_MASK	GENMASK(26, 20) /* Transmit FIFO Usable Area */
+#define FCTR_TFUA_SHIFT	20
 #define FCTR_TFUA(i)	((i) << FCTR_TFUA_SHIFT)
-#define FCTR_RFWM_MASK	0x0000e000 /* Receive FIFO Watermark */
-#define FCTR_RFWM_1	0x00000000 /*  Transfer Request when 1 valid stages */
-#define FCTR_RFWM_4	0x00002000 /*  Transfer Request when 4 valid stages */
-#define FCTR_RFWM_8	0x00004000 /*  Transfer Request when 8 valid stages */
-#define FCTR_RFWM_16	0x00006000 /*  Transfer Request when 16 valid stages */
-#define FCTR_RFWM_32	0x00008000 /*  Transfer Request when 32 valid stages */
-#define FCTR_RFWM_64	0x0000a000 /*  Transfer Request when 64 valid stages */
-#define FCTR_RFWM_128	0x0000c000 /*  Transfer Request when 128 valid stages */
-#define FCTR_RFWM_256	0x0000e000 /*  Transfer Request when 256 valid stages */
-#define FCTR_RFUA_MASK	0x00001ff0 /* Receive FIFO Usable Area (0x40 = full) */
-#define FCTR_RFUA_SHIFT		 4
+#define FCTR_RFWM_MASK	GENMASK(15, 13) /* Receive FIFO Watermark */
+#define FCTR_RFWM_1	(0 << 13) /*  Transfer Request when 1 valid stages */
+#define FCTR_RFWM_4	(1 << 13) /*  Transfer Request when 4 valid stages */
+#define FCTR_RFWM_8	(2 << 13) /*  Transfer Request when 8 valid stages */
+#define FCTR_RFWM_16	(3 << 13) /*  Transfer Request when 16 valid stages */
+#define FCTR_RFWM_32	(4 << 13) /*  Transfer Request when 32 valid stages */
+#define FCTR_RFWM_64	(5 << 13) /*  Transfer Request when 64 valid stages */
+#define FCTR_RFWM_128	(6 << 13) /*  Transfer Request when 128 valid stages */
+#define FCTR_RFWM_256	(7 << 13) /*  Transfer Request when 256 valid stages */
+#define FCTR_RFUA_MASK	GENMASK(12, 4) /* Receive FIFO Usable Area (0x40 = full) */
+#define FCTR_RFUA_SHIFT	4
 #define FCTR_RFUA(i)	((i) << FCTR_RFUA_SHIFT)
 
 /* STR */
-#define STR_TFEMP	0x20000000 /* Transmit FIFO Empty */
-#define STR_TDREQ	0x10000000 /* Transmit Data Transfer Request */
-#define STR_TEOF	0x00800000 /* Frame Transmission End */
-#define STR_TFSERR	0x00200000 /* Transmit Frame Synchronization Error */
-#define STR_TFOVF	0x00100000 /* Transmit FIFO Overflow */
-#define STR_TFUDF	0x00080000 /* Transmit FIFO Underflow */
-#define STR_RFFUL	0x00002000 /* Receive FIFO Full */
-#define STR_RDREQ	0x00001000 /* Receive Data Transfer Request */
-#define STR_REOF	0x00000080 /* Frame Reception End */
-#define STR_RFSERR	0x00000020 /* Receive Frame Synchronization Error */
-#define STR_RFUDF	0x00000010 /* Receive FIFO Underflow */
-#define STR_RFOVF	0x00000008 /* Receive FIFO Overflow */
+#define STR_TFEMP	BIT(29) /* Transmit FIFO Empty */
+#define STR_TDREQ	BIT(28) /* Transmit Data Transfer Request */
+#define STR_TEOF	BIT(23) /* Frame Transmission End */
+#define STR_TFSERR	BIT(21) /* Transmit Frame Synchronization Error */
+#define STR_TFOVF	BIT(20) /* Transmit FIFO Overflow */
+#define STR_TFUDF	BIT(19) /* Transmit FIFO Underflow */
+#define STR_RFFUL	BIT(13) /* Receive FIFO Full */
+#define STR_RDREQ	BIT(12) /* Receive Data Transfer Request */
+#define STR_REOF	BIT(7)  /* Frame Reception End */
+#define STR_RFSERR	BIT(5)  /* Receive Frame Synchronization Error */
+#define STR_RFUDF	BIT(4)  /* Receive FIFO Underflow */
+#define STR_RFOVF	BIT(3)  /* Receive FIFO Overflow */
 
 /* IER */
-#define IER_TDMAE	0x80000000 /* Transmit Data DMA Transfer Req. Enable */
-#define IER_TFEMPE	0x20000000 /* Transmit FIFO Empty Enable */
-#define IER_TDREQE	0x10000000 /* Transmit Data Transfer Request Enable */
-#define IER_TEOFE	0x00800000 /* Frame Transmission End Enable */
-#define IER_TFSERRE	0x00200000 /* Transmit Frame Sync Error Enable */
-#define IER_TFOVFE	0x00100000 /* Transmit FIFO Overflow Enable */
-#define IER_TFUDFE	0x00080000 /* Transmit FIFO Underflow Enable */
-#define IER_RDMAE	0x00008000 /* Receive Data DMA Transfer Req. Enable */
-#define IER_RFFULE	0x00002000 /* Receive FIFO Full Enable */
-#define IER_RDREQE	0x00001000 /* Receive Data Transfer Request Enable */
-#define IER_REOFE	0x00000080 /* Frame Reception End Enable */
-#define IER_RFSERRE	0x00000020 /* Receive Frame Sync Error Enable */
-#define IER_RFUDFE	0x00000010 /* Receive FIFO Underflow Enable */
-#define IER_RFOVFE	0x00000008 /* Receive FIFO Overflow Enable */
+#define IER_TDMAE	BIT(31) /* Transmit Data DMA Transfer Req. Enable */
+#define IER_TFEMPE	BIT(29) /* Transmit FIFO Empty Enable */
+#define IER_TDREQE	BIT(28) /* Transmit Data Transfer Request Enable */
+#define IER_TEOFE	BIT(23) /* Frame Transmission End Enable */
+#define IER_TFSERRE	BIT(21) /* Transmit Frame Sync Error Enable */
+#define IER_TFOVFE	BIT(20) /* Transmit FIFO Overflow Enable */
+#define IER_TFUDFE	BIT(19) /* Transmit FIFO Underflow Enable */
+#define IER_RDMAE	BIT(15) /* Receive Data DMA Transfer Req. Enable */
+#define IER_RFFULE	BIT(13) /* Receive FIFO Full Enable */
+#define IER_RDREQE	BIT(12) /* Receive Data Transfer Request Enable */
+#define IER_REOFE	BIT(7)  /* Frame Reception End Enable */
+#define IER_RFSERRE	BIT(5)  /* Receive Frame Sync Error Enable */
+#define IER_RFUDFE	BIT(4)  /* Receive FIFO Underflow Enable */
+#define IER_RFOVFE	BIT(3)  /* Receive FIFO Overflow Enable */
 
 
 static u32 sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs)
-- 
2.17.1

Re: [PATCH v2 1/3] spi: sh-msiof: Use BIT() and GENMASK()

[Wolfram Sang]

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

On Tue, Apr 02, 2019 at 04:40:21PM +0200, Geert Uytterhoeven wrote:
> Improve maintainability by converting the register bit, bitmask, and
> bitfield definitions from hexadecimal constants to constructs using
> BIT(), GENMASK(), or "val << shift".
> 
> Suggested-by: Simon Horman <horms+renesas@verge.net.au>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> All definitions keep their original value (checked by comparing
> assembler output).

I didn't review it, I trust you :) But I'm good with this change:

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


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

Applied "spi: sh-msiof: Use BIT() and GENMASK()" to the spi tree

[Mark Brown]

The patch

   spi: sh-msiof: Use BIT() and GENMASK()

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 5a4df21e83710dea9c6c4636f3183090b018cbcc Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Tue, 2 Apr 2019 16:40:21 +0200
Subject: [PATCH] spi: sh-msiof: Use BIT() and GENMASK()

Improve maintainability by converting the register bit, bitmask, and
bitfield definitions from hexadecimal constants to constructs using
BIT(), GENMASK(), or "val << shift".

Suggested-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-sh-msiof.c | 174 ++++++++++++++++++-------------------
 1 file changed, 87 insertions(+), 87 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index e2eb466db10a..6edc76636e81 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -82,111 +82,111 @@ struct sh_msiof_spi_priv {
 #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_DTDL_SHIFT		 20 /* Data Pin Bit Delay for MSIOF_SYNC */
-#define MDR1_SYNCDL_SHIFT	 16 /* Frame Sync Signal Timing Delay */
-#define MDR1_FLD_MASK	 0x0000000c /* Frame Sync Signal Interval (0-3) */
-#define MDR1_FLD_SHIFT		  2
-#define MDR1_XXSTP	 0x00000001 /* Transmission/Reception Stop on FIFO */
+#define MDR1_TRMD	   BIT(31)  /* Transfer Mode (1 = Master mode) */
+#define MDR1_SYNCMD_MASK   GENMASK(29, 28) /* SYNC Mode */
+#define MDR1_SYNCMD_SPI	   (2 << 28)/*   Level mode/SPI */
+#define MDR1_SYNCMD_LR	   (3 << 28)/*   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_DTDL_SHIFT	   20       /* Data Pin Bit Delay for MSIOF_SYNC */
+#define MDR1_SYNCDL_SHIFT  16       /* Frame Sync Signal Timing Delay */
+#define MDR1_FLD_MASK	   GENMASK(3, 2) /* Frame Sync Signal Interval (0-3) */
+#define MDR1_FLD_SHIFT	   2
+#define MDR1_XXSTP	   BIT(0)   /* Transmission/Reception Stop on FIFO */
 /* TMDR1 */
-#define TMDR1_PCON	 0x40000000 /* Transfer Signal Connection */
-#define TMDR1_SYNCCH_MASK 0xc000000 /* Synchronization Signal Channel Select */
-#define TMDR1_SYNCCH_SHIFT	 26 /* 0=MSIOF_SYNC, 1=MSIOF_SS1, 2=MSIOF_SS2 */
+#define TMDR1_PCON	   BIT(30)  /* Transfer Signal Connection */
+#define TMDR1_SYNCCH_MASK  GENMASK(27, 26) /* Sync Signal Channel Select */
+#define TMDR1_SYNCCH_SHIFT 26       /* 0=MSIOF_SYNC, 1=MSIOF_SS1, 2=MSIOF_SS2 */
 
 /* 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) */
+#define MDR2_GRPMASK1	BIT(0)      /* Group Output Mask 1 (SH, A1) */
 
 /* TSCR and RSCR */
-#define SCR_BRPS_MASK	    0x1f00 /* Prescaler Setting (1-32) */
+#define SCR_BRPS_MASK	GENMASK(12, 8) /* 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
+#define SCR_BRDV_MASK	GENMASK(2, 0) /* Baud Rate Generator's Division Ratio */
+#define SCR_BRDV_DIV_2	0
+#define SCR_BRDV_DIV_4	1
+#define SCR_BRDV_DIV_8	2
+#define SCR_BRDV_DIV_16	3
+#define SCR_BRDV_DIV_32	4
+#define SCR_BRDV_DIV_1	7
 
 /* 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 */
+#define CTR_TSCKIZ_MASK	GENMASK(31, 30) /* Transmit Clock I/O Polarity Select */
+#define CTR_TSCKIZ_SCK	BIT(31)   /*   Disable SCK when TX disabled */
+#define CTR_TSCKIZ_POL_SHIFT 30   /*   Transmit Clock Polarity */
+#define CTR_RSCKIZ_MASK	GENMASK(29, 28) /* Receive Clock Polarity Select */
+#define CTR_RSCKIZ_SCK	BIT(29)   /*   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	GENMASK(23, 22) /* Pin Output When TX is Disabled */
+#define CTR_TXDIZ_LOW	(0 << 22) /*   0 */
+#define CTR_TXDIZ_HIGH	(1 << 22) /*   1 */
+#define CTR_TXDIZ_HIZ	(2 << 22) /*   High-impedance */
+#define CTR_TSCKE	BIT(15)   /* Transmit Serial Clock Output Enable */
+#define CTR_TFSE	BIT(14)   /* Transmit Frame Sync Signal Output Enable */
+#define CTR_TXE		BIT(9)    /* Transmit Enable */
+#define CTR_RXE		BIT(8)    /* Receive Enable */
 
 /* FCTR */
-#define FCTR_TFWM_MASK	0xe0000000 /* Transmit FIFO Watermark */
-#define FCTR_TFWM_64	0x00000000 /*  Transfer Request when 64 empty stages */
-#define FCTR_TFWM_32	0x20000000 /*  Transfer Request when 32 empty stages */
-#define FCTR_TFWM_24	0x40000000 /*  Transfer Request when 24 empty stages */
-#define FCTR_TFWM_16	0x60000000 /*  Transfer Request when 16 empty stages */
-#define FCTR_TFWM_12	0x80000000 /*  Transfer Request when 12 empty stages */
-#define FCTR_TFWM_8	0xa0000000 /*  Transfer Request when 8 empty stages */
-#define FCTR_TFWM_4	0xc0000000 /*  Transfer Request when 4 empty stages */
-#define FCTR_TFWM_1	0xe0000000 /*  Transfer Request when 1 empty stage */
-#define FCTR_TFUA_MASK	0x07f00000 /* Transmit FIFO Usable Area */
-#define FCTR_TFUA_SHIFT		20
+#define FCTR_TFWM_MASK	GENMASK(31, 29) /* Transmit FIFO Watermark */
+#define FCTR_TFWM_64	(0 << 29) /*  Transfer Request when 64 empty stages */
+#define FCTR_TFWM_32	(1 << 29) /*  Transfer Request when 32 empty stages */
+#define FCTR_TFWM_24	(2 << 29) /*  Transfer Request when 24 empty stages */
+#define FCTR_TFWM_16	(3 << 29) /*  Transfer Request when 16 empty stages */
+#define FCTR_TFWM_12	(4 << 29) /*  Transfer Request when 12 empty stages */
+#define FCTR_TFWM_8	(5 << 29) /*  Transfer Request when 8 empty stages */
+#define FCTR_TFWM_4	(6 << 29) /*  Transfer Request when 4 empty stages */
+#define FCTR_TFWM_1	(7 << 29) /*  Transfer Request when 1 empty stage */
+#define FCTR_TFUA_MASK	GENMASK(26, 20) /* Transmit FIFO Usable Area */
+#define FCTR_TFUA_SHIFT	20
 #define FCTR_TFUA(i)	((i) << FCTR_TFUA_SHIFT)
-#define FCTR_RFWM_MASK	0x0000e000 /* Receive FIFO Watermark */
-#define FCTR_RFWM_1	0x00000000 /*  Transfer Request when 1 valid stages */
-#define FCTR_RFWM_4	0x00002000 /*  Transfer Request when 4 valid stages */
-#define FCTR_RFWM_8	0x00004000 /*  Transfer Request when 8 valid stages */
-#define FCTR_RFWM_16	0x00006000 /*  Transfer Request when 16 valid stages */
-#define FCTR_RFWM_32	0x00008000 /*  Transfer Request when 32 valid stages */
-#define FCTR_RFWM_64	0x0000a000 /*  Transfer Request when 64 valid stages */
-#define FCTR_RFWM_128	0x0000c000 /*  Transfer Request when 128 valid stages */
-#define FCTR_RFWM_256	0x0000e000 /*  Transfer Request when 256 valid stages */
-#define FCTR_RFUA_MASK	0x00001ff0 /* Receive FIFO Usable Area (0x40 = full) */
-#define FCTR_RFUA_SHIFT		 4
+#define FCTR_RFWM_MASK	GENMASK(15, 13) /* Receive FIFO Watermark */
+#define FCTR_RFWM_1	(0 << 13) /*  Transfer Request when 1 valid stages */
+#define FCTR_RFWM_4	(1 << 13) /*  Transfer Request when 4 valid stages */
+#define FCTR_RFWM_8	(2 << 13) /*  Transfer Request when 8 valid stages */
+#define FCTR_RFWM_16	(3 << 13) /*  Transfer Request when 16 valid stages */
+#define FCTR_RFWM_32	(4 << 13) /*  Transfer Request when 32 valid stages */
+#define FCTR_RFWM_64	(5 << 13) /*  Transfer Request when 64 valid stages */
+#define FCTR_RFWM_128	(6 << 13) /*  Transfer Request when 128 valid stages */
+#define FCTR_RFWM_256	(7 << 13) /*  Transfer Request when 256 valid stages */
+#define FCTR_RFUA_MASK	GENMASK(12, 4) /* Receive FIFO Usable Area (0x40 = full) */
+#define FCTR_RFUA_SHIFT	4
 #define FCTR_RFUA(i)	((i) << FCTR_RFUA_SHIFT)
 
 /* STR */
-#define STR_TFEMP	0x20000000 /* Transmit FIFO Empty */
-#define STR_TDREQ	0x10000000 /* Transmit Data Transfer Request */
-#define STR_TEOF	0x00800000 /* Frame Transmission End */
-#define STR_TFSERR	0x00200000 /* Transmit Frame Synchronization Error */
-#define STR_TFOVF	0x00100000 /* Transmit FIFO Overflow */
-#define STR_TFUDF	0x00080000 /* Transmit FIFO Underflow */
-#define STR_RFFUL	0x00002000 /* Receive FIFO Full */
-#define STR_RDREQ	0x00001000 /* Receive Data Transfer Request */
-#define STR_REOF	0x00000080 /* Frame Reception End */
-#define STR_RFSERR	0x00000020 /* Receive Frame Synchronization Error */
-#define STR_RFUDF	0x00000010 /* Receive FIFO Underflow */
-#define STR_RFOVF	0x00000008 /* Receive FIFO Overflow */
+#define STR_TFEMP	BIT(29) /* Transmit FIFO Empty */
+#define STR_TDREQ	BIT(28) /* Transmit Data Transfer Request */
+#define STR_TEOF	BIT(23) /* Frame Transmission End */
+#define STR_TFSERR	BIT(21) /* Transmit Frame Synchronization Error */
+#define STR_TFOVF	BIT(20) /* Transmit FIFO Overflow */
+#define STR_TFUDF	BIT(19) /* Transmit FIFO Underflow */
+#define STR_RFFUL	BIT(13) /* Receive FIFO Full */
+#define STR_RDREQ	BIT(12) /* Receive Data Transfer Request */
+#define STR_REOF	BIT(7)  /* Frame Reception End */
+#define STR_RFSERR	BIT(5)  /* Receive Frame Synchronization Error */
+#define STR_RFUDF	BIT(4)  /* Receive FIFO Underflow */
+#define STR_RFOVF	BIT(3)  /* Receive FIFO Overflow */
 
 /* IER */
-#define IER_TDMAE	0x80000000 /* Transmit Data DMA Transfer Req. Enable */
-#define IER_TFEMPE	0x20000000 /* Transmit FIFO Empty Enable */
-#define IER_TDREQE	0x10000000 /* Transmit Data Transfer Request Enable */
-#define IER_TEOFE	0x00800000 /* Frame Transmission End Enable */
-#define IER_TFSERRE	0x00200000 /* Transmit Frame Sync Error Enable */
-#define IER_TFOVFE	0x00100000 /* Transmit FIFO Overflow Enable */
-#define IER_TFUDFE	0x00080000 /* Transmit FIFO Underflow Enable */
-#define IER_RDMAE	0x00008000 /* Receive Data DMA Transfer Req. Enable */
-#define IER_RFFULE	0x00002000 /* Receive FIFO Full Enable */
-#define IER_RDREQE	0x00001000 /* Receive Data Transfer Request Enable */
-#define IER_REOFE	0x00000080 /* Frame Reception End Enable */
-#define IER_RFSERRE	0x00000020 /* Receive Frame Sync Error Enable */
-#define IER_RFUDFE	0x00000010 /* Receive FIFO Underflow Enable */
-#define IER_RFOVFE	0x00000008 /* Receive FIFO Overflow Enable */
+#define IER_TDMAE	BIT(31) /* Transmit Data DMA Transfer Req. Enable */
+#define IER_TFEMPE	BIT(29) /* Transmit FIFO Empty Enable */
+#define IER_TDREQE	BIT(28) /* Transmit Data Transfer Request Enable */
+#define IER_TEOFE	BIT(23) /* Frame Transmission End Enable */
+#define IER_TFSERRE	BIT(21) /* Transmit Frame Sync Error Enable */
+#define IER_TFOVFE	BIT(20) /* Transmit FIFO Overflow Enable */
+#define IER_TFUDFE	BIT(19) /* Transmit FIFO Underflow Enable */
+#define IER_RDMAE	BIT(15) /* Receive Data DMA Transfer Req. Enable */
+#define IER_RFFULE	BIT(13) /* Receive FIFO Full Enable */
+#define IER_RDREQE	BIT(12) /* Receive Data Transfer Request Enable */
+#define IER_REOFE	BIT(7)  /* Frame Reception End Enable */
+#define IER_RFSERRE	BIT(5)  /* Receive Frame Sync Error Enable */
+#define IER_RFUDFE	BIT(4)  /* Receive FIFO Underflow Enable */
+#define IER_RFOVFE	BIT(3)  /* Receive FIFO Overflow Enable */
 
 
 static u32 sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs)
-- 
2.20.1

[PATCH v2 2/3] spi: sh-msiof: Use readl_poll_timeout_atomic() instead of open-coding

[Geert Uytterhoeven]

Replace the open-coded loop in sh_msiof_modify_ctr_wait() by a call to
the readl_poll_timeout_atomic() helper macro.

Suggested-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/spi/spi-sh-msiof.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 6edc76636e81a801..3243ff258896595b 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -18,6 +18,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -219,21 +220,14 @@ static int sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p,
 {
 	u32 mask = clr | set;
 	u32 data;
-	int k;
 
 	data = sh_msiof_read(p, CTR);
 	data &= ~clr;
 	data |= set;
 	sh_msiof_write(p, CTR, data);
 
-	for (k = 100; k > 0; k--) {
-		if ((sh_msiof_read(p, CTR) & mask) == set)
-			break;
-
-		udelay(10);
-	}
-
-	return k > 0 ? 0 : -ETIMEDOUT;
+	return readl_poll_timeout_atomic(p->mapbase + CTR, data,
+					 (data & mask) == set, 10, 1000);
 }
 
 static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
-- 
2.17.1

Re: [PATCH v2 2/3] spi: sh-msiof: Use readl_poll_timeout_atomic() instead of open-coding

[Wolfram Sang]

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

On Tue, Apr 02, 2019 at 04:40:22PM +0200, Geert Uytterhoeven wrote:
> Replace the open-coded loop in sh_msiof_modify_ctr_wait() by a call to
> the readl_poll_timeout_atomic() helper macro.
> 
> Suggested-by: Wolfram Sang <wsa@the-dreams.de>

Please use me Renesas related address. Like in this new tag:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


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

Applied "spi: sh-msiof: Use readl_poll_timeout_atomic() instead of open-coding" to the spi tree

[Mark Brown]

The patch

   spi: sh-msiof: Use readl_poll_timeout_atomic() instead of open-coding

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 9115b4d89b213dc73fd35844e0199b1763f751d6 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Tue, 2 Apr 2019 16:40:22 +0200
Subject: [PATCH] spi: sh-msiof: Use readl_poll_timeout_atomic() instead of
 open-coding

Replace the open-coded loop in sh_msiof_modify_ctr_wait() by a call to
the readl_poll_timeout_atomic() helper macro.

Suggested-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-sh-msiof.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 6edc76636e81..3243ff258896 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -18,6 +18,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -219,21 +220,14 @@ static int sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p,
 {
 	u32 mask = clr | set;
 	u32 data;
-	int k;
 
 	data = sh_msiof_read(p, CTR);
 	data &= ~clr;
 	data |= set;
 	sh_msiof_write(p, CTR, data);
 
-	for (k = 100; k > 0; k--) {
-		if ((sh_msiof_read(p, CTR) & mask) == set)
-			break;
-
-		udelay(10);
-	}
-
-	return k > 0 ? 0 : -ETIMEDOUT;
+	return readl_poll_timeout_atomic(p->mapbase + CTR, data,
+					 (data & mask) == set, 10, 1000);
 }
 
 static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
-- 
2.20.1

Re: Applied "spi: sh-msiof: Use readl_poll_timeout_atomic() instead of open-coding" to the spi tree

[Wolfram Sang]

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

Mark,

> Suggested-by: Wolfram Sang <wsa@the-dreams.de>

Any chance this email address can be changed to the one I used for my
Rev-by two lines below? Nothing major, but would be nice.

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks,

   Wolfram


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

Re: Applied "spi: sh-msiof: Use readl_poll_timeout_atomic() instead of open-coding" to the spi tree

[Mark Brown]

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

On Wed, Apr 03, 2019 at 08:54:06AM +0200, Wolfram Sang wrote:

> Any chance this email address can be changed to the one I used for my
> Rev-by two lines below? Nothing major, but would be nice.

> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Required rebasing but it's only been published briefly so OK...

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

[PATCH v2 3/3] spi: sh-msiof: Add reset of registers before starting transfer

[Geert Uytterhoeven]

In accordance with hardware specification Ver 1.0, reset register
transmission / reception setting before transfer.

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
[geert: Use readl_poll_timeout_atomic()]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Use BIT(),
  - Use readl_poll_timeout_atomic().
---
 drivers/spi/spi-sh-msiof.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 3243ff258896595b..d3794499915c17cb 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -132,6 +132,8 @@ struct sh_msiof_spi_priv {
 #define CTR_TFSE	BIT(14)   /* Transmit Frame Sync Signal Output Enable */
 #define CTR_TXE		BIT(9)    /* Transmit Enable */
 #define CTR_RXE		BIT(8)    /* Receive Enable */
+#define CTR_TXRST	BIT(1)    /* Transmit Reset */
+#define CTR_RXRST	BIT(0)    /* Receive Reset */
 
 /* FCTR */
 #define FCTR_TFWM_MASK	GENMASK(31, 29) /* Transmit FIFO Watermark */
@@ -241,6 +243,19 @@ static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void sh_msiof_spi_reset_regs(struct sh_msiof_spi_priv *p)
+{
+	u32 mask = CTR_TXRST | CTR_RXRST;
+	u32 data;
+
+	data = sh_msiof_read(p, CTR);
+	data |= mask;
+	sh_msiof_write(p, CTR, data);
+
+	readl_poll_timeout_atomic(p->mapbase + CTR, data, !(data & mask), 1,
+				  100);
+}
+
 static const u32 sh_msiof_spi_div_array[] = {
 	SCR_BRDV_DIV_1, SCR_BRDV_DIV_2,	 SCR_BRDV_DIV_4,
 	SCR_BRDV_DIV_8,	SCR_BRDV_DIV_16, SCR_BRDV_DIV_32,
@@ -920,6 +935,9 @@ static int sh_msiof_transfer_one(struct spi_controller *ctlr,
 	bool swab;
 	int ret;
 
+	/* reset registers */
+	sh_msiof_spi_reset_regs(p);
+
 	/* setup clocks (clock already enabled in chipselect()) */
 	if (!spi_controller_is_slave(p->ctlr))
 		sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
-- 
2.17.1

Re: [PATCH v2 3/3] spi: sh-msiof: Add reset of registers before starting transfer

[Wolfram Sang]

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

On Tue, Apr 02, 2019 at 04:40:23PM +0200, Geert Uytterhoeven wrote:
> In accordance with hardware specification Ver 1.0, reset register
> transmission / reception setting before transfer.
> 
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> [geert: Use readl_poll_timeout_atomic()]
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Minor nit:

> +	/* reset registers */
> +	sh_msiof_spi_reset_regs(p);

I don't think this comment adds value.


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

Applied "spi: sh-msiof: Add reset of registers before starting transfer" to the spi tree

[Mark Brown]

The patch

   spi: sh-msiof: Add reset of registers before starting transfer

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From fedd6940682afeddb22bfd855d8eaf527bcc473a Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Tue, 2 Apr 2019 16:40:23 +0200
Subject: [PATCH] spi: sh-msiof: Add reset of registers before starting
 transfer

In accordance with hardware specification Ver 1.0, reset register
transmission / reception setting before transfer.

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
[geert: Use readl_poll_timeout_atomic()]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-sh-msiof.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 3243ff258896..d3794499915c 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -132,6 +132,8 @@ struct sh_msiof_spi_priv {
 #define CTR_TFSE	BIT(14)   /* Transmit Frame Sync Signal Output Enable */
 #define CTR_TXE		BIT(9)    /* Transmit Enable */
 #define CTR_RXE		BIT(8)    /* Receive Enable */
+#define CTR_TXRST	BIT(1)    /* Transmit Reset */
+#define CTR_RXRST	BIT(0)    /* Receive Reset */
 
 /* FCTR */
 #define FCTR_TFWM_MASK	GENMASK(31, 29) /* Transmit FIFO Watermark */
@@ -241,6 +243,19 @@ static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void sh_msiof_spi_reset_regs(struct sh_msiof_spi_priv *p)
+{
+	u32 mask = CTR_TXRST | CTR_RXRST;
+	u32 data;
+
+	data = sh_msiof_read(p, CTR);
+	data |= mask;
+	sh_msiof_write(p, CTR, data);
+
+	readl_poll_timeout_atomic(p->mapbase + CTR, data, !(data & mask), 1,
+				  100);
+}
+
 static const u32 sh_msiof_spi_div_array[] = {
 	SCR_BRDV_DIV_1, SCR_BRDV_DIV_2,	 SCR_BRDV_DIV_4,
 	SCR_BRDV_DIV_8,	SCR_BRDV_DIV_16, SCR_BRDV_DIV_32,
@@ -920,6 +935,9 @@ static int sh_msiof_transfer_one(struct spi_controller *ctlr,
 	bool swab;
 	int ret;
 
+	/* reset registers */
+	sh_msiof_spi_reset_regs(p);
+
 	/* setup clocks (clock already enabled in chipselect()) */
 	if (!spi_controller_is_slave(p->ctlr))
 		sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
-- 
2.20.1

Re: [PATCH v2 0/3] spi: sh-msiof: Add reset of registers before starting transfer

[Wolfram Sang]

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


>   - New patches 1 and 2, in response to review comments on v1
>     (although sh_msiof_modify_ctr_wait() didn't turn out to be suitable
>     for calling by sh_msiof_spi_reset_regs()),

Well, the overhead of readl_poll_timeout_atomic() is low enough to
ignore reusing sh_msiof_modify_ctr_wait IMO. Thanks for doing it!


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

Re: [PATCH v2 0/3] spi: sh-msiof: Add reset of registers before starting transfer

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Apr 2, 2019 at 4:47 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> >   - New patches 1 and 2, in response to review comments on v1
> >     (although sh_msiof_modify_ctr_wait() didn't turn out to be suitable
> >     for calling by sh_msiof_spi_reset_regs()),
>
> Well, the overhead of readl_poll_timeout_atomic() is low enough to
> ignore reusing sh_msiof_modify_ctr_wait IMO. Thanks for doing it!

sh_msiof_modify_ctr_wait() cannot be used because it checks when the
change just made to the register becomes visible (i.e. new state can
be read back).
For reset the check is different: the write sets the bit, the check
must wait until
the hardware has cleared the bit again.

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