[U-Boot] [PATCH v6] spi: Add support SH Quad SPI driver

Nobuhiro Iwamatsu nobuhiro.iwamatsu.yj at renesas.com
Mon Dec 16 09:06:44 CET 2013


Hi, Jagan.

Thanks for your review.

2013/12/16 Jagan Teki <jagannadh.teki at gmail.com>:
> On Mon, Dec 2, 2013 at 1:42 PM, Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj at renesas.com> wrote:
>> 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 at renesas.com>
>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj at renesas.com>
>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna at xilinx.com>
>> ---
>>
>> Changes for v6:
>>   - Update Makefile to new style.
>> Changes for v5:
>>   - Add print abort when call ctrlc().
>>   - Move source code in spi_xfer() which should be processed by
>>     spi_cs_activate().
>>   - Move source code in spi_xfer() which should be processed by
>>     spi_cs_deactivate().
>>   - Remove sh_qspi_xfer, move to spi_xfer().
>> Changes for v4:
>>   - Added tabs
>>   - Added comments
>>   - Added sh_qspi_init()
>> Changes for v3:
>>   - Change Queued to Quad.
>>   - Remove path of file from file header.
>>   - Use read* and write* directly instead of sh_qspi_write* sh_qspi_read*.
>>   - Change driver format.
>> Changes for v2:
>>   - "SH QSPI" to "SH QSPI (Queued SPI)".
>>   - Remove magic number.
>>
>>  drivers/spi/Makefile  |   1 +
>>  drivers/spi/sh_qspi.c | 244 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 245 insertions(+)
>>  create mode 100644 drivers/spi/sh_qspi.c
>>
>> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
>> index 27902fe..914e71f 100644
>> --- a/drivers/spi/Makefile
>> +++ b/drivers/spi/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_OC_TINY_SPI) += oc_tiny_spi.o
>>  obj-$(CONFIG_OMAP3_SPI) += omap3_spi.o
>>  obj-$(CONFIG_SOFT_SPI) += soft_spi.o
>>  obj-$(CONFIG_SH_SPI) += sh_spi.o
>> +obj-$(CONFIG_SH_QSPI) += sh_qspi.o
>>  obj-$(CONFIG_FSL_ESPI) += fsl_espi.o
>>  obj-$(CONFIG_FDT_SPI) += fdt_spi.o
>>  obj-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o
>> diff --git a/drivers/spi/sh_qspi.c b/drivers/spi/sh_qspi.c
>> new file mode 100644
>> index 0000000..8f53552
>> --- /dev/null
>> +++ b/drivers/spi/sh_qspi.c
>> @@ -0,0 +1,244 @@
>> +/*
>> + * SH QSPI (Quad SPI) driver
>> + *
>> + * Copyright (C) 2013 Renesas Electronics Corporation
>> + * Copyright (C) 2013 Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj at renesas.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0
>> + */
>> +
>> +#include <common.h>
>> +#include <malloc.h>
>> +#include <spi.h>
>> +#include <asm/io.h>
>> +
>> +/* SH QSPI register bit masks <REG>_<BIT> */
>> +#define SPCR_MSTR      0x08
>> +#define SPCR_SPE       0x40
>> +#define SPSR_SPRFF     0x80
>> +#define SPSR_SPTEF     0x20
>> +#define SPPCR_IO3FV    0x04
>> +#define SPPCR_IO2FV    0x02
>> +#define SPPCR_IO1FV    0x01
>> +#define SPBDCR_RXBC0   (1 << 0)
>> +#define SPCMD_SCKDEN   (1 << 15)
>> +#define SPCMD_SLNDEN   (1 << 14)
>> +#define SPCMD_SPNDEN   (1 << 13)
>> +#define SPCMD_SSLKP    (1 << 7)
>> +#define SPCMD_BRDV0    (1 << 2)
>> +#define SPCMD_INIT1    SPCMD_SCKDEN | SPCMD_SLNDEN | \
>> +                       SPCMD_SPNDEN | SPCMD_SSLKP | \
>> +                       SPCMD_BRDV0
>> +#define SPCMD_INIT2    SPCMD_SPNDEN | SPCMD_SSLKP | \
>> +                       SPCMD_BRDV0
>> +#define SPBFCR_TXRST   (1 << 7)
>> +#define SPBFCR_RXRST   (1 << 6)
>> +
>> +/* SH QSPI register set */
>> +struct sh_qspi_regs {
>> +       unsigned char spcr;
>> +       unsigned char sslp;
>> +       unsigned char sppcr;
>> +       unsigned char spsr;
>> +       unsigned long spdr;
>> +       unsigned char spscr;
>> +       unsigned char spssr;
>> +       unsigned char spbr;
>> +       unsigned char spdcr;
>> +       unsigned char spckd;
>> +       unsigned char sslnd;
>> +       unsigned char spnd;
>> +       unsigned char dummy0;
>> +       unsigned short spcmd0;
>> +       unsigned short spcmd1;
>> +       unsigned short spcmd2;
>> +       unsigned short spcmd3;
>> +       unsigned char spbfcr;
>> +       unsigned char dummy1;
>> +       unsigned short spbdcr;
>> +       unsigned long spbmul0;
>> +       unsigned long spbmul1;
>> +       unsigned long spbmul2;
>> +       unsigned long spbmul3;
>> +};
>> +
>> +struct sh_qspi_slave {
>> +       struct spi_slave        slave;
>> +       struct sh_qspi_regs     *regs;
>> +};
>> +
>> +static inline struct sh_qspi_slave *to_sh_qspi(struct spi_slave *slave)
>> +{
>> +       return container_of(slave, struct sh_qspi_slave, slave);
>> +}
>> +
>> +static void sh_qspi_init(struct sh_qspi_slave *ss)
>> +{
>> +       /* QSPI initialize */
>> +       writeb(SPCR_MSTR, &ss->regs->spcr);
>> +       writeb(0x00, &ss->regs->sslp);
>> +       writeb(SPPCR_IO3FV|SPPCR_IO2FV, &ss->regs->sppcr);
>> +
>> +       /* Set bit rate. See 58.3.8 Quad Serial Peripheral Interface */
>> +       writeb(0x01, &ss->regs->spbr);
>> +
>> +       writeb(0x00, &ss->regs->spdcr);
>> +       writeb(0x00, &ss->regs->spckd);
>> +       writeb(0x00, &ss->regs->sslnd);
>> +       writeb(0x00, &ss->regs->spnd);
>> +       writew(SPCMD_INIT1, &ss->regs->spcmd0);
>> +       writew(SPCMD_INIT2, &ss->regs->spcmd0);
>> +       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);
>> +}
> Please try to write comments on each initialization steps.
> - refer code in drivers/spi/zynq_spi.c

OK. I add comment each steps.
>
>> +
>> +int spi_cs_is_valid(unsigned int bus, unsigned int cs)
>> +{
>> +       return 1;
>> +}
>> +
>> +void spi_cs_activate(struct spi_slave *slave)
>> +{
>> +       struct sh_qspi_slave *ss = to_sh_qspi(slave);
>> +
>> +       writeb(SPCR_MSTR, &ss->regs->spcr);
>> +       writew(SPCMD_INIT1, &ss->regs->spcmd0);
>> +       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);
>> +}
> Better to use clrbits_le* and comments too.. - refer code in
> drivers/spi/zynq_spi.c

Indeed.

>
>> +
>> +void spi_cs_deactivate(struct spi_slave *slave)
>> +{
>> +       struct sh_qspi_slave *ss = to_sh_qspi(slave);
>> +
>> +       writeb(SPCR_MSTR, &ss->regs->spcr);
> Better to use setbits_le* - refer code in drivers/spi/zynq_spi.c

Indeed. I will rewrite.

>
>> +}
>> +
>> +void spi_init(void)
>> +{
>> +       /* nothing to do */
>> +}
>> +
>> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>> +               unsigned int max_hz, unsigned int mode)
>> +{
>> +       struct sh_qspi_slave *ss;
>> +
>> +       if (!spi_cs_is_valid(bus, cs))
>> +               return NULL;
>> +
>> +       ss = spi_alloc_slave(struct sh_qspi_slave, bus, cs);
>> +       if (!ss) {
>> +               printf("SPI_error: Fail to allocate sh_qspi_slave\n");
>> +               return NULL;
>> +       }
>> +
>> +       ss->regs = (struct sh_qspi_regs *)CONFIG_SH_QSPI_BASE;
>> +
>> +       /* Init SH QSPI */
>> +       sh_qspi_init(ss);
>> +
>> +       return &ss->slave;
>> +}
>> +
>> +void spi_free_slave(struct spi_slave *slave)
>> +{
>> +       struct sh_qspi_slave *spi = to_sh_qspi(slave);
>> +
>> +       free(spi);
>> +}
>> +
>> +int spi_claim_bus(struct spi_slave *slave)
>> +{
>> +       return 0;
>> +}
>> +
>> +void spi_release_bus(struct spi_slave *slave)
>> +{
>> +}
>> +
>> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>> +            void *din, unsigned long flags)
>> +{
>> +       struct sh_qspi_slave *ss = to_sh_qspi(slave);
>> +       unsigned long nbyte;
>> +       int ret = 0;
>> +       unsigned char dtdata = 0, drdata;
>> +       unsigned char *tdata = &dtdata, *rdata = &drdata;
>> +       unsigned long *spbmul0 = &ss->regs->spbmul0;
>> +
>> +       if (dout == NULL && din == NULL) {
>> +               if (flags & SPI_XFER_END)
>> +                       spi_cs_deactivate(slave);
>> +               return 0;
>> +       }
>> +
>> +       if (bitlen % 8) {
>> +               printf("%s: bitlen is not 8bit alined %d", __func__, bitlen);
>> +               return 1;
>> +       }
>> +
>> +       nbyte = bitlen / 8;
>> +
>> +       if (flags & SPI_XFER_BEGIN) {
>> +               spi_cs_activate(slave);
>> +
>> +               /* Set 1048576 byte */
>> +               writel(0x100000, spbmul0);
>> +       }
>> +
>> +       if (flags & SPI_XFER_END)
>> +               writel(nbyte, spbmul0);
>> +
>> +       if (dout != NULL)
>> +               tdata = (unsigned char *)dout;
>> +
>> +       if (din != NULL)
>> +               rdata = din;
>> +
>> +       while (nbyte > 0) {
>> +               while (!(readb(&ss->regs->spsr) & SPSR_SPTEF)) {
>> +                       if (ctrlc()) {
>> +                               puts("abort\n");
>> +                               return 1;
>> +                       }
>> +                       udelay(10);
>> +               }
>> +
>> +               writeb(*tdata, (unsigned char *)(&ss->regs->spdr));
>> +
>> +               while ((readw(&ss->regs->spbdcr) != SPBDCR_RXBC0)) {
>> +                       if (ctrlc()) {
>> +                               puts("abort\n");
>> +                               return 1;
>> +                       }
>> +                       udelay(1);
>> +               }
>> +
>> +               while (!(readb(&ss->regs->spsr) & SPSR_SPRFF)) {
>> +                       if (ctrlc()) {
>> +                               puts("abort\n");
>> +                               return 1;
>> +                       }
>> +                       udelay(10);
>> +               }
>> +
>> +               *rdata = readb((unsigned char *)(&ss->regs->spdr));
>> +
>> +               if (dout != NULL)
>> +                       tdata++;
>> +               if (din != NULL)
>> +                       rdata++;
>> +
>> +               nbyte--;
>> +       }
>> +
>> +       if (flags & SPI_XFER_END)
>> +               spi_cs_deactivate(slave);
>> +
>> +       return ret;
>> +}
>> --
>
> Did you tested this - please add test documentation on doc folder.
> That might give you good insight in future.

I see. I will resend with update patch and document.

>
> --
> Thanks,
> Jagan.

Best regards,
  Nobuhiro

-- 
Nobuhiro Iwamatsu


More information about the U-Boot mailing list