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

[KOJI MATSUOKA <koji.matsuoka.xm@xxxxxxxxxxx>]

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@xxxxxxxxx
> [mailto:geert.uytterhoeven@xxxxxxxxx] 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@xxxxxxxxx>
> wrote:
> > From: Koji Matsuoka <koji.matsuoka.xm@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@xxxxxxxxx>
> > ---
> >
> > 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@xxxxxxxxxxxxxx
> 
> 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
��.n��������+%������w��{.n�����{����)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥