[PATCH/RFC] spi: sh-msiof: Fix FIFO size to 64 word from 256 word

[PATCH/RFC] spi: sh-msiof: Fix FIFO size to 64 word from 256 word

[Yoshihiro Kaneko]

From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

The upper limit of Tx/Rx FIFO size is 64 word by the
specification of H/W. This patch corrects to 64 word from 256 word.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---

This patch is based on the for-next branch of Mark Brown's spi tree.

 drivers/spi/spi-sh-msiof.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index d3370a6..a7629f8 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -48,8 +48,8 @@ struct sh_msiof_spi_priv {
 	const struct sh_msiof_chipdata *chipdata;
 	struct sh_msiof_spi_info *info;
 	struct completion done;
-	int tx_fifo_size;
-	int rx_fifo_size;
+	unsigned int tx_fifo_size;
+	unsigned int rx_fifo_size;
 	void *tx_dma_page;
 	void *rx_dma_page;
 	dma_addr_t tx_dma_addr;
@@ -95,8 +95,6 @@ struct sh_msiof_spi_priv {
 #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 MAX_WDLEN	256U
-
 /* TSCR and RSCR */
 #define SCR_BRPS_MASK	    0x1f00 /* Prescaler Setting (1-32) */
 #define SCR_BRPS(i)	(((i) - 1) << 8)
@@ -850,7 +848,12 @@ static int sh_msiof_transfer_one(struct spi_master *master,
 		 *  DMA supports 32-bit words only, hence pack 8-bit and 16-bit
 		 *  words, with byte resp. word swapping.
 		 */
-		unsigned int l = min(len, MAX_WDLEN * 4);
+		unsigned int l = 0;
+
+		if (tx_buf)
+			l = min(len, p->tx_fifo_size * 4);
+		if (rx_buf)
+			l = min(len, p->rx_fifo_size * 4);
 
 		if (bits <= 8) {
 			if (l & 3)
@@ -963,7 +966,7 @@ static const struct sh_msiof_chipdata sh_data = {
 
 static const struct sh_msiof_chipdata r8a779x_data = {
 	.tx_fifo_size = 64,
-	.rx_fifo_size = 256,
+	.rx_fifo_size = 64,
 	.master_flags = SPI_MASTER_MUST_TX,
 };
 
-- 
1.9.1

Re: [PATCH/RFC] spi: sh-msiof: Fix FIFO size to 64 word from 256 word

[Geert Uytterhoeven]

Hi Kaneko-san, Matsuoka-san,

On Sun, Jun 14, 2015 at 7:25 PM, Yoshihiro Kaneko <ykaneko0929-Re5JQEeQqe8@public.gmane.orgm> wrote:
> From: Koji Matsuoka <koji.matsuoka.xm-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>
> The upper limit of Tx/Rx FIFO size is 64 word by the
> specification of H/W. This patch corrects to 64 word from 256 word.

Are you sure about that?

The R-Car Gen2 datasheet (up to Rev 1.01) says:

"FIFO capacity: 32 bits × 64 stages for transmission and 32 bits × 256 stages
 for reception"

Has this been changed in a more recent version of the datasheet?

Thanks!

> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>
> This patch is based on the for-next branch of Mark Brown's spi tree.
>
>  drivers/spi/spi-sh-msiof.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index d3370a6..a7629f8 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -48,8 +48,8 @@ struct sh_msiof_spi_priv {
>         const struct sh_msiof_chipdata *chipdata;
>         struct sh_msiof_spi_info *info;
>         struct completion done;
> -       int tx_fifo_size;
> -       int rx_fifo_size;
> +       unsigned int tx_fifo_size;
> +       unsigned int rx_fifo_size;
>         void *tx_dma_page;
>         void *rx_dma_page;
>         dma_addr_t tx_dma_addr;
> @@ -95,8 +95,6 @@ struct sh_msiof_spi_priv {
>  #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 MAX_WDLEN      256U

This is not related to FIFO size, but to max. DMA chunk transfer size.

> -
>  /* TSCR and RSCR */
>  #define SCR_BRPS_MASK      0x1f00 /* Prescaler Setting (1-32) */
>  #define SCR_BRPS(i)    (((i) - 1) << 8)
> @@ -850,7 +848,12 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>                  *  DMA supports 32-bit words only, hence pack 8-bit and 16-bit
>                  *  words, with byte resp. word swapping.
>                  */
> -               unsigned int l = min(len, MAX_WDLEN * 4);

This is not related to FIFO size, but to max. DMA chunk transfer size.

> +               unsigned int l = 0;
> +
> +               if (tx_buf)
> +                       l = min(len, p->tx_fifo_size * 4);
> +               if (rx_buf)
> +                       l = min(len, p->rx_fifo_size * 4);
>
>                 if (bits <= 8) {
>                         if (l & 3)
> @@ -963,7 +966,7 @@ static const struct sh_msiof_chipdata sh_data = {
>
>  static const struct sh_msiof_chipdata r8a779x_data = {
>         .tx_fifo_size = 64,
> -       .rx_fifo_size = 256,
> +       .rx_fifo_size = 64,

Please note the R-Car Gen2 FIFO size is also documented to be 256 in
Documentation/devicetree/bindings/spi/sh-msiof.txt

>         .master_flags = SPI_MASTER_MUST_TX,
>  };

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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH/RFC] spi: sh-msiof: Fix FIFO size to 64 word from 256 word

[KOJI MATSUOKA]

Hi Geert-san

> > The upper limit of Tx/Rx FIFO size is 64 word by the specification of
> > H/W. This patch corrects to 64 word from 256 word.
> 
> Are you sure about that?

As a result of querying the H/W designer, it has proven to be a misdescription of
the H/W specification documentation.

In fact, the value that can be set to the WDLEN1[7:0] bit of MSIOF Transmit Mode
Register 2 is 64 words.

Best regards, Koji Matsuoka

> -----Original Message-----
> From: geert.uytterhoeven@gmail.com
> [mailto:geert.uytterhoeven@gmail.com] On Behalf Of Geert Uytterhoeven
> Sent: Monday, June 15, 2015 4:56 PM
> To: Yoshihiro Kaneko; KOJI MATSUOKA
> Cc: linux-spi; Mark Brown; Simon Horman; Magnus Damm; Linux-sh list
> Subject: Re: [PATCH/RFC] spi: sh-msiof: Fix FIFO size to 64 word from 256
> word
> 
> Hi Kaneko-san, Matsuoka-san,
> 
> On Sun, Jun 14, 2015 at 7:25 PM, Yoshihiro Kaneko <ykaneko0929@gmail.com>
> wrote:
> > From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> >
> > The upper limit of Tx/Rx FIFO size is 64 word by the specification of
> > H/W. This patch corrects to 64 word from 256 word.
> 
> Are you sure about that?
> 
> The R-Car Gen2 datasheet (up to Rev 1.01) says:
> 
> "FIFO capacity: 32 bits × 64 stages for transmission and 32 bits × 256 stages
> for reception"
> 
> Has this been changed in a more recent version of the datasheet?
> 
> Thanks!
> 
> > Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > ---
> >
> > This patch is based on the for-next branch of Mark Brown's spi tree.
> >
> >  drivers/spi/spi-sh-msiof.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> > index d3370a6..a7629f8 100644
> > --- a/drivers/spi/spi-sh-msiof.c
> > +++ b/drivers/spi/spi-sh-msiof.c
> > @@ -48,8 +48,8 @@ struct sh_msiof_spi_priv {
> >         const struct sh_msiof_chipdata *chipdata;
> >         struct sh_msiof_spi_info *info;
> >         struct completion done;
> > -       int tx_fifo_size;
> > -       int rx_fifo_size;
> > +       unsigned int tx_fifo_size;
> > +       unsigned int rx_fifo_size;
> >         void *tx_dma_page;
> >         void *rx_dma_page;
> >         dma_addr_t tx_dma_addr;
> > @@ -95,8 +95,6 @@ struct sh_msiof_spi_priv {  #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 MAX_WDLEN      256U
> 
> This is not related to FIFO size, but to max. DMA chunk transfer size.
> 
> > -
> >  /* TSCR and RSCR */
> >  #define SCR_BRPS_MASK      0x1f00 /* Prescaler Setting (1-32) */
> >  #define SCR_BRPS(i)    (((i) - 1) << 8)
> > @@ -850,7 +848,12 @@ static int sh_msiof_transfer_one(struct spi_master
> *master,
> >                  *  DMA supports 32-bit words only, hence pack 8-bit and
> 16-bit
> >                  *  words, with byte resp. word swapping.
> >                  */
> > -               unsigned int l = min(len, MAX_WDLEN * 4);
> 
> This is not related to FIFO size, but to max. DMA chunk transfer size.
> 
> > +               unsigned int l = 0;
> > +
> > +               if (tx_buf)
> > +                       l = min(len, p->tx_fifo_size * 4);
> > +               if (rx_buf)
> > +                       l = min(len, p->rx_fifo_size * 4);
> >
> >                 if (bits <= 8) {
> >                         if (l & 3)
> > @@ -963,7 +966,7 @@ static const struct sh_msiof_chipdata sh_data = {
> >
> >  static const struct sh_msiof_chipdata r8a779x_data = {
> >         .tx_fifo_size = 64,
> > -       .rx_fifo_size = 256,
> > +       .rx_fifo_size = 64,
> 
> Please note the R-Car Gen2 FIFO size is also documented to be 256 in
> Documentation/devicetree/bindings/spi/sh-msiof.txt
> 
> >         .master_flags = SPI_MASTER_MUST_TX,  };
> 
> 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

Applied "spi: sh-msiof: Fix FIFO size to 64 word from 256 word" to the spi tree

[Mark Brown]

The patch

   spi: sh-msiof: Fix FIFO size to 64 word from 256 word

has been applied to the spi tree at

   git://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 8ed545ee4d491d6f2762024524842fe029c4ae86 Mon Sep 17 00:00:00 2001
From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Date: Mon, 15 Jun 2015 02:25:05 +0900
Subject: [PATCH] spi: sh-msiof: Fix FIFO size to 64 word from 256 word

The upper limit of Tx/Rx FIFO size is 64 word by the
specification of H/W. This patch corrects to 64 word from 256 word.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/spi/spi-sh-msiof.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 118c652..a7934ab 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -48,8 +48,8 @@ struct sh_msiof_spi_priv {
 	const struct sh_msiof_chipdata *chipdata;
 	struct sh_msiof_spi_info *info;
 	struct completion done;
-	int tx_fifo_size;
-	int rx_fifo_size;
+	unsigned int tx_fifo_size;
+	unsigned int rx_fifo_size;
 	void *tx_dma_page;
 	void *rx_dma_page;
 	dma_addr_t tx_dma_addr;
@@ -95,8 +95,6 @@ struct sh_msiof_spi_priv {
 #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 MAX_WDLEN	256U
-
 /* TSCR and RSCR */
 #define SCR_BRPS_MASK	    0x1f00 /* Prescaler Setting (1-32) */
 #define SCR_BRPS(i)	(((i) - 1) << 8)
@@ -850,7 +848,12 @@ static int sh_msiof_transfer_one(struct spi_master *master,
 		 *  DMA supports 32-bit words only, hence pack 8-bit and 16-bit
 		 *  words, with byte resp. word swapping.
 		 */
-		unsigned int l = min(len, MAX_WDLEN * 4);
+		unsigned int l = 0;
+
+		if (tx_buf)
+			l = min(len, p->tx_fifo_size * 4);
+		if (rx_buf)
+			l = min(len, p->rx_fifo_size * 4);
 
 		if (bits <= 8) {
 			if (l & 3)
@@ -963,7 +966,7 @@ static const struct sh_msiof_chipdata sh_data = {
 
 static const struct sh_msiof_chipdata r8a779x_data = {
 	.tx_fifo_size = 64,
-	.rx_fifo_size = 256,
+	.rx_fifo_size = 64,
 	.master_flags = SPI_MASTER_MUST_TX,
 };
 
-- 
2.5.0