[U-Boot] [PATCH v2] spi: Add support SH Queued SPI driver
Nobuhiro Iwamatsu
nobuhiro.iwamatsu.yj at renesas.com
Tue Aug 20 04:09:03 CEST 2013
Hi,
Thanks for your comment.
2013/8/9 Jagan Teki <jagannadh.teki at gmail.com>:
> Hi,
>
>
> On 08-08-2013 15:02, Nobuhiro Iwamatsu wrote:
>>
>> This patch adds a driver for Renesas SoC's Queued 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>
>> ---
>> v2: Change "SH QSPI" to "SH QSPI (Queued SPI)".
>> Remove magic number.
>>
>> drivers/spi/Makefile | 1 +
>> drivers/spi/sh_qspi.c | 232
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/spi/sh_qspi.h | 52 +++++++++++
>
>
> Please move the structure decelerations on to sh_qspi.c
OK. I will move to .c .
> FYI: I am trying to prepare the spi driver code to more readable
>
> << header file inclusion >>
> << Register bit masks >>
>
> << MISC macro definitions >>
>
> << controller reg structure >>
>
> << controller private slave structure >>
>
> << inline func defination >>
>
> << spi_xfer_sub() >>
>
> << spi_setup_slave_sub >>
>
> << spi_cs_is_valid >>
>
> << spi_cs_activate >>
>
> << spi_cs_deactivate >>
>
> << spi_init >>
>
> << spi_setup_slave >>
> {
> spi_setup_slave_sub()
> }
>
> << spi_free_slave >>
>
> << spi_claim_bus >>
>
> << spi_release_bus >>
>
> << spi_xfer code >>
> {
> spi_xfer_sub()
> }
>
> I am just trying to do the above format atleast from the drivers which are
> pushing now onwards.
>
> Please see the reff driver
> http://git.denx.de/?p=u-boot/u-boot-spi.git;a=commitdiff;h=1465d055f9d7a81edacf30c9d20a1b51dfcbfa8d
>
> Let me know your views.
>
>
>> 3 files changed, 285 insertions(+)
>> create mode 100644 drivers/spi/sh_qspi.c
>> create mode 100644 drivers/spi/sh_qspi.h
>>
>> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
>> index 019132e..f71c089 100644
>> --- a/drivers/spi/Makefile
>> +++ b/drivers/spi/Makefile
>> @@ -33,6 +33,7 @@ COBJS-$(CONFIG_OC_TINY_SPI) += oc_tiny_spi.o
>> COBJS-$(CONFIG_OMAP3_SPI) += omap3_spi.o
>> COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
>> COBJS-$(CONFIG_SH_SPI) += sh_spi.o
>> +COBJS-$(CONFIG_SH_QSPI) += sh_qspi.o
>> COBJS-$(CONFIG_FSL_ESPI) += fsl_espi.o
>> COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o
>> COBJS-$(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..25ce60a
>> --- /dev/null
>> +++ b/drivers/spi/sh_qspi.c
>> @@ -0,0 +1,232 @@
>> +/*
>> + * drivers/spi/sh_qspi.c
>
> Not required, i guess.
Removed.
>
>
>> + * SH QSPI (Queued 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>
>> +#include "sh_qspi.h"
>> +
>> +static void sh_qspi_writeb(unsigned char data, unsigned char *reg)
>> +{
>> + writeb(data, reg);
>> +}
>> +
>> +static void sh_qspi_writew(unsigned short data, unsigned short *reg)
>> +{
>> + writew(data, reg);
>> +}
>> +
>> +static void sh_qspi_writel(unsigned long data, unsigned long *reg)
>> +{
>> + writel(data, reg);
>> +}
>> +
>> +static unsigned char sh_qspi_readb(unsigned char *reg)
>> +{
>> + return readb(reg);
>> +}
>> +
>> +static unsigned short sh_qspi_readw(unsigned short *reg)
>> +{
>> + return readw(reg);
>> +}
>> +
>> +void spi_init(void)
>> +{
>> +}
>
>
> Can you use read* and write* directly instead of sh_qspi_write*
> sh_qspi_read*
>
> Do you have any specific reason for using these?
No reason. I will change to use read* and write*.
>
>
>> +
>> +/* SPCR */
>> +#define SPCR_MSTR 0x08
>> +#define SPCR_SPE 0x40
>> +/* SPSR */
>> +#define SPSR_SPRFF 0x80
>> +#define SPSR_SPTEF 0x20
>> +/* SPPCR */
>> +#define SPPCR_IO3FV 0x04
>> +#define SPPCR_IO2FV 0x02
>> +#define SPPCR_IO1FV 0x01
>> +/* SPBDCR */
>> +#define SPBDCR_RXBC0 (1 << 0)
>> +/* SPCMD */
>> +#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)
>> +/* SPBFCR */
>> +#define SPBFCR_TXRST (1 << 7)
>> +#define SPBFCR_RXRST (1 << 6)
>> +
>> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>> + unsigned int max_hz, unsigned int mode)
>> +{
>> + struct sh_qspi *ss;
>> +
>> + if (!spi_cs_is_valid(bus, cs))
>> + return NULL;
>> +
>> + ss = spi_alloc_slave(struct sh_qspi, bus, cs);
>> + if (!ss)
>> + return NULL;
>> +
>> + ss->regs = (struct sh_qspi_regs *)CONFIG_SH_QSPI_BASE;
>> +
>> + /* QSPI initialize */
>> + sh_qspi_writeb(SPCR_MSTR, &ss->regs->spcr);
>> + sh_qspi_writeb(0x00, &ss->regs->sslp);
>> + sh_qspi_writeb(SPPCR_IO3FV|SPPCR_IO2FV, &ss->regs->sppcr);
>> +
>> + /* Set bit rate. See 58.3.8 Quad Serial Peripheral Interface */
>> + sh_qspi_writeb(0x01, &ss->regs->spbr);
>> +
>> + sh_qspi_writeb(0x00, &ss->regs->spdcr);
>> + sh_qspi_writeb(0x00, &ss->regs->spckd);
>> + sh_qspi_writeb(0x00, &ss->regs->sslnd);
>> + sh_qspi_writeb(0x00, &ss->regs->spnd);
>> + sh_qspi_writew(SPCMD_INIT1, &ss->regs->spcmd0);
>> + sh_qspi_writew(SPCMD_INIT2, &ss->regs->spcmd0);
>> + sh_qspi_writeb(SPBFCR_TXRST|SPBFCR_RXRST, &ss->regs->spbfcr);
>> + sh_qspi_writeb(0x00, &ss->regs->spbfcr);
>> + sh_qspi_writeb(0x00, &ss->regs->spscr);
>> + sh_qspi_writeb(SPCR_SPE|SPCR_MSTR, &ss->regs->spcr);
>> +
>> + return &ss->slave;
>> +}
>> +
>> +void spi_free_slave(struct spi_slave *slave)
>> +{
>> + struct sh_qspi *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)
>> +{
>> +}
>> +
>> +static int sh_qspi_xfer(struct sh_qspi *ss, unsigned char *tdata,
>> + unsigned char *rdata, unsigned long flags)
>> +{
>> + while (!(sh_qspi_readb(&ss->regs->spsr) & SPSR_SPTEF)) {
>> + if (ctrlc())
>> + return 1;
>> + udelay(10);
>> + }
>> +
>> + sh_qspi_writeb(*tdata, (unsigned char *)(&ss->regs->spdr));
>> +
>> + while ((sh_qspi_readw(&ss->regs->spbdcr) != SPBDCR_RXBC0)) {
>> + int i = 100;
>> +
>> + if (ctrlc())
>> + return 1;
>> + while (i--)
>> + ;
>> + }
>> +
>> + while (!(sh_qspi_readb(&ss->regs->spsr) & SPSR_SPRFF)) {
>> + if (ctrlc())
>> + return 1;
>> + udelay(10);
>> + }
>> +
>> + *rdata = sh_qspi_readb((unsigned char *)(&ss->regs->spdr));
>> +
>> + return 0;
>> +}
>> +
>> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void
>> *dout,
>> + void *din, unsigned long flags)
>> +{
>> + struct sh_qspi *ss = to_sh_qspi(slave);
>> + unsigned int nbyte;
>> + int ret = 0;
>> + unsigned char *tdata, *rdata, dtdata = 0, drdata;
>> +
>> + if (dout == NULL && din == NULL) {
>> + if (flags & SPI_XFER_END)
>> + sh_qspi_writeb(SPCR_MSTR, &ss->regs->spcr);
>> + 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) {
>> + sh_qspi_writeb(SPCR_MSTR, &ss->regs->spcr);
>> +
>> + sh_qspi_writew(SPCMD_INIT1, &ss->regs->spcmd0);
>> +
>> + if (flags & SPI_XFER_END)
>> + sh_qspi_writel(nbyte, &ss->regs->spbmul0);
>> + else
>> + /* Set 1048576 byte */
>> + sh_qspi_writel(0x100000, &ss->regs->spbmul0);
>> + sh_qspi_writeb(SPBFCR_TXRST|SPBFCR_RXRST,
>> &ss->regs->spbfcr);
>> + sh_qspi_writeb(0x00, &ss->regs->spbfcr);
>> + sh_qspi_writeb(0x00, &ss->regs->spscr);
>> + sh_qspi_writeb(SPCR_SPE|SPCR_MSTR, &ss->regs->spcr);
>> + }
>> +
>> + if (dout != NULL)
>> + tdata = (unsigned char *)dout;
>> + else
>> + tdata = &dtdata;
>> +
>> + if (din != NULL)
>> + rdata = din;
>> + else
>> + rdata = &drdata;
>> +
>> + while (nbyte > 0) {
>> + ret = sh_qspi_xfer(ss, tdata, rdata, flags);
>> + if (ret)
>> + break;
>> +
>> + if (dout != NULL)
>> + tdata++;
>> +
>> + if (din != NULL)
>> + rdata++;
>> +
>> + nbyte--;
>> + }
>> +
>> + if (flags & SPI_XFER_END)
>> + sh_qspi_writeb(SPCR_MSTR, &ss->regs->spcr);
>> +
>> + return ret;
>> +}
>> +
>> +int spi_cs_is_valid(unsigned int bus, unsigned int cs)
>> +{
>> + return 1;
>> +}
>> +
>> +void spi_cs_activate(struct spi_slave *slave)
>> +{
>> +}
>> +
>> +void spi_cs_deactivate(struct spi_slave *slave)
>> +{
>> +}
>
>
> I will give few more comments, please fix these if you agree with the driver
> format i suggested.
> if not please let me know your views.
I agreed the driver format which you propose.
I will update your point and driver format, and re-send.
>
> --
> Thanks,
> Jagan
Best regards,
Nobuhiro
--
Nobuhiro Iwamatsu
More information about the U-Boot
mailing list