| Message ID | 1434302705-31104-1-git-send-email-ykaneko0929@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Simon Horman |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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, };