From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
Date: Wed, 09 Oct 2013 11:35:39 +0900
Subject: [U-Boot] [PATCH v4] spi: Add support SH Quad SPI driver
In-Reply-To: <CAD6G_RQJYsvoepboy1D_0RgNqUQLp2G-A-YeUpdBKBcrDeZKng@mail.gmail.com>
References: <d7f066ea-17e8-42f3-9310-9aeb7b88f95d@CH1EHSMHS043.ehs.local>
	<CAD6G_RQJYsvoepboy1D_0RgNqUQLp2G-A-YeUpdBKBcrDeZKng@mail.gmail.com>
Message-ID: <5254C0FB.2050203@renesas.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi,

(2013/10/04 4:45), Jagan Teki wrote:
> Hi Nobuhiro Iwamatsu,
>
> I have send your patch for v4.
> Eliminated few checks see on change log:
>
> Please fix below comments and try to apply the same on u-boot-spi.git
> repo with master-probe branch.

OK, I will check with your repo.

>
> On Fri, Oct 4, 2013 at 1:04 AM, Jagannadha Sutradharudu Teki
> <jagannadha.sutradharudu-teki@xilinx.com>  wrote:
>> From: Nobuhiro Iwamatsu<nobuhiro.iwamatsu.yj@renesas.com>
>>
>> This patch adds a driver for Renesas SoC's Quad SPI bus.
>> This supports with 8 bits per transfer to use with SPI flash.
>>
>> Signed-off-by: Kouei Abe<kouei.abe.cp@renesas.com>
>> Signed-off-by: Nobuhiro Iwamatsu<nobuhiro.iwamatsu.yj@renesas.com>
>> Signed-off-by: Jagannadha Sutradharudu Teki<jaganna@xilinx.com>
>> ---
>>
>> Changes for v4:
>>          - Added tabs
>>          - Added comments
>>          - Added sh_qspi_init()

<snip>

>> +
>> +static int sh_qspi_xfer(struct sh_qspi_slave *ss, unsigned char *tdata,
>> +                       unsigned char *rdata, unsigned long flags)
>> +{
>> +       while (!(readb(&ss->regs->spsr)&  SPSR_SPTEF)) {
>> +               if (ctrlc())
> Looks like you are trying for error check - Just add a print

OK, I will add print.

>
>> +                       return 1;
>> +               udelay(10);
>> +       }
>> +
>> +       writeb(*tdata, (unsigned char *)(&ss->regs->spdr));
>> +
>> +       while ((readw(&ss->regs->spbdcr) != SPBDCR_RXBC0)) {
>> +               int i = 100;
>> +
>> +               if (ctrlc())
> same as above.
>
>> +                       return 1;
>> +               while (i--)
>> +                       ;
>> +       }
>> +
>> +       while (!(readb(&ss->regs->spsr)&  SPSR_SPRFF)) {
>> +               if (ctrlc())
>> +                       return 1;
> same as above.
>

<snip>

>
> ------TAG+
>> +       if (flags&  SPI_XFER_BEGIN) {
>> +               unsigned long *spbmul0 =&ss->regs->spbmul0;
>> +               writeb(SPCR_MSTR,&ss->regs->spcr);
>> +
>> +               writew(SPCMD_INIT1,&ss->regs->spcmd0);
>> +
>> +               if (flags&  SPI_XFER_END)
>> +                       writel(nbyte, spbmul0);
>> +               else
>> +                       /* Set 1048576 byte */
>> +                       writel(0x100000, spbmul0);
>> +
>> +               writeb(SPBFCR_TXRST|SPBFCR_RXRST,&ss->regs->spbfcr);
>> +               writeb(0x00,&ss->regs->spbfcr);
>> +               writeb(0x00,&ss->regs->spscr);
>> +               writeb(SPCR_SPE|SPCR_MSTR,&ss->regs->spcr);
>> +       }
> ---TAG-
>
> TAG+ .. TAG- code should be part of spi_cs_activate() func..please adjust
>

OK, I will re-write spi_cs_activate.

>> +
>> +       if (dout != NULL)
>> +               tdata = (unsigned char *)dout;
>> +       else
>> +               tdata =&dtdata;
>> +
>> +       if (din != NULL)
>> +               rdata = din;
>> +       else
>> +               rdata =&drdata;
>> +
>> +       while (nbyte>  0) {
>
> TAG+
>> +               ret = sh_qspi_xfer(ss, tdata, rdata, flags);
>> +               if (ret)
>> +                       break;
>> +               if (dout != NULL)
>> +                       tdata++;
>> +               if (din != NULL)
>> +                       rdata++;
>> +               nbyte--;
> TAG-
>
> Can you try to write the all code in this function- I think no need to
> write extra sh_qspi_xfer()
> try to optimize more.. see driver/spi/zynq_spi.c


OK, I will remove sh_qspi_xfer, and merge code of sh_qspi_xfer?to spi_xfer.

>
>> +       }
>> +
>> +       if (flags&  SPI_XFER_END)
>> +               writeb(SPCR_MSTR,&ss->regs->spcr);
>
> This should be part of spi_cs_deactivate().
>

OK, I will re-write spi_cs_deactivate.

Best regards,
   NObuhiro