Hi, Jagan. Thanks for your review.
2013/12/16 Jagan Teki <jagannadh.t...@gmail.com>: > On Mon, Dec 2, 2013 at 1:42 PM, Nobuhiro Iwamatsu > <nobuhiro.iwamatsu...@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...@renesas.com> >> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu...@renesas.com> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaga...@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...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot