From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jagan Teki Date: Mon, 12 Jun 2017 11:32:39 +0530 Subject: [U-Boot] [PATCH 03/10] dm: spi: add BCM63xx SPI driver In-Reply-To: <4e8e577a-e073-91fd-bece-235e6318dbf8@gmail.com> References: <1495135788-9152-1-git-send-email-noltari@gmail.com> <1495135788-9152-4-git-send-email-noltari@gmail.com> <1ca6fcd4-5c8d-e36d-5446-64b7735c4a27@gmail.com> <4e8e577a-e073-91fd-bece-235e6318dbf8@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Thu, Jun 8, 2017 at 12:05 AM, Álvaro Fernández Rojas wrote: > Hi Jagan, > > El 07/06/2017 a las 19:29, Jagan Teki escribió: >> On Wed, Jun 7, 2017 at 9:05 PM, Álvaro Fernández Rojas >> wrote: >>> Hi Jagan, >>> >>> El 7/6/17 a las 9:35, Jagan Teki escribió: >>>> On Fri, May 19, 2017 at 12:59 AM, Álvaro Fernández Rojas >>>> wrote: >>>>> This driver is a simplified version of linux/drivers/spi/spi-bcm63xx.c >>>>> Instead of supporting both HW revisions of the controller in a single build, >>>>> support has been split by the selected config to save space. >>>>> >>>>> Signed-off-by: Álvaro Fernández Rojas >>>>> --- >>>>> drivers/spi/Kconfig | 23 +++ >>>>> drivers/spi/Makefile | 1 + >>>>> drivers/spi/bcm63xx_spi.c | 404 ++++++++++++++++++++++++++++++++++++++++++++++ >>>>> 3 files changed, 428 insertions(+) >>>>> create mode 100644 drivers/spi/bcm63xx_spi.c >>>>> >>>>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig >>>>> index a00d401..d458dd7 100644 >>>>> --- a/drivers/spi/Kconfig >>>>> +++ b/drivers/spi/Kconfig >>>>> @@ -43,6 +43,29 @@ config ATMEL_SPI >>>>> many AT32 (AVR32) and AT91 (ARM) chips. This driver can be >>>>> used to access the SPI Flash, such as AT25DF321. >>>>> >>>>> +choice >>>>> + prompt "BCM63xx SPI driver" >>>>> + depends on ARCH_BMIPS >>>>> + optional >>>>> + >>>>> +config BCM6338_SPI >>>>> + bool "BCM6338 SPI driver" >>>>> + select SPI_MAX_WRITE_CMD_BYTES >>>>> + help >>>>> + Enable the BCM6338 SPI driver. This driver can be used to >>>>> + access the SPI NOR flash on platforms embedding this Broadcom >>>>> + SPI core. >>>>> + >>>>> +config BCM6358_SPI >>>>> + bool "BCM6358 SPI driver" >>>>> + select SPI_MAX_WRITE_CMD_BYTES >>>>> + help >>>>> + Enable the BCM6358 SPI driver. This driver can be used to >>>>> + access the SPI NOR flash on platforms embedding this Broadcom >>>>> + SPI core. >>>>> + >>>>> +endchoice >>>>> + >>>>> config CADENCE_QSPI >>>>> bool "Cadence QSPI driver" >>>>> help >>>>> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile >>>>> index c090562..c9ba648 100644 >>>>> --- a/drivers/spi/Makefile >>>>> +++ b/drivers/spi/Makefile >>>>> @@ -19,6 +19,7 @@ obj-$(CONFIG_ALTERA_SPI) += altera_spi.o >>>>> obj-$(CONFIG_ATH79_SPI) += ath79_spi.o >>>>> obj-$(CONFIG_ATMEL_DATAFLASH_SPI) += atmel_dataflash_spi.o >>>>> obj-$(CONFIG_ATMEL_SPI) += atmel_spi.o >>>>> +obj-$(CONFIG_BCM6338_SPI)$(CONFIG_BCM6358_SPI) += bcm63xx_spi.o >>>>> obj-$(CONFIG_CADENCE_QSPI) += cadence_qspi.o cadence_qspi_apb.o >>>>> obj-$(CONFIG_CF_SPI) += cf_spi.o >>>>> obj-$(CONFIG_DAVINCI_SPI) += davinci_spi.o >>>>> diff --git a/drivers/spi/bcm63xx_spi.c b/drivers/spi/bcm63xx_spi.c >>>>> new file mode 100644 >>>>> index 0000000..6e64607 >>>>> --- /dev/null >>>>> +++ b/drivers/spi/bcm63xx_spi.c >>>>> @@ -0,0 +1,404 @@ >>>>> +/* >>>>> + * Copyright (C) 2017 Álvaro Fernández Rojas >>>>> + * >>>>> + * Derived from linux/drivers/spi/spi-bcm63xx.c: >>>>> + * Copyright (C) 2009-2012 Florian Fainelli >>>>> + * Copyright (C) 2010 Tanguy Bouzeloc >>>>> + * >>>>> + * SPDX-License-Identifier: GPL-2.0+ >>>>> + */ >>>>> + >>>>> +#include >>>>> +#include >>>>> +#include >>>>> +#include >>>>> +#include >>>>> +#include >>>>> + >>>>> +DECLARE_GLOBAL_DATA_PTR; >>>>> + >>>>> +#if defined(CONFIG_BCM6338_SPI) >>>>> + >>>>> +# define SPI_DT_ID "brcm,bcm6338-spi" >>>>> + >>>>> +/* SPI Command register */ >>>>> +# define SPI_CMD_REG 0x00 >>>>> + >>>>> +/* SPI Interrupt registers */ >>>>> +# define SPI_IR_STAT_REG 0x02 >>>>> +# define SPI_IR_MASK_REG 0x04 >>>>> + >>>>> +/* SPI Clock register */ >>>>> +# define SPI_CLK_REG 0x06 >>>>> + >>>>> +/* SPI Fill register */ >>>>> +# define SPI_FILL_REG 0x07 >>>>> + >>>>> +/* SPI Control register (8 bit) */ >>>>> +# define SPI_CTL_REG 0x40 >>>>> +# define SPI_CTL_BYTES_SHIFT 0 >>>>> +# define SPI_CTL_BYTES_MASK (0x3f << SPI_CTL_BYTES_SHIFT) >>>>> +# define SPI_CTL_TYPE_SHIFT 6 >>>>> +# define SPI_CTL_TYPE_FD_RW (0 << SPI_CTL_TYPE_SHIFT) >>>>> +# define SPI_CTL_TYPE_HD_W (1 << SPI_CTL_TYPE_SHIFT) >>>>> +# define SPI_CTL_TYPE_HD_R (2 << SPI_CTL_TYPE_SHIFT) >>>>> + >>>>> +# define bcm63xx_spi_wctl(v,a) writeb_be(v, a); >>>>> + >>>>> +/* SPI TX Data registers */ >>>>> +# define SPI_TX_DATA_REG 0x41 >>>>> +# define SPI_TX_DATA_SIZE 0x3f >>>>> + >>>>> +/* SPI RX Data registers */ >>>>> +# define SPI_RX_DATA_REG 0x80 >>>>> +# define SPI_RX_DATA_SIZE 0x3f >>>>> + >>>>> +#elif defined(CONFIG_BCM6358_SPI) >>>>> + >>>>> +# define SPI_DT_ID "brcm,bcm6358-spi" >>>>> + >>>>> +/* SPI Control register (16 bit) */ >>>>> +# define SPI_CTL_REG 0x000 >>>>> +# define SPI_CTL_BYTES_SHIFT 0 >>>>> +# define SPI_CTL_BYTES_MASK (0x3ff << SPI_CTL_BYTES_SHIFT) >>>>> +# define SPI_CTL_TYPE_SHIFT 14 >>>>> +# define SPI_CTL_TYPE_FD_RW (0 << SPI_CTL_TYPE_SHIFT) >>>>> +# define SPI_CTL_TYPE_HD_W (1 << SPI_CTL_TYPE_SHIFT) >>>>> +# define SPI_CTL_TYPE_HD_R (2 << SPI_CTL_TYPE_SHIFT) >>>>> + >>>>> +# define bcm63xx_spi_wctl(v,a) writew_be(v, a); >>>>> + >>>>> +/* SPI TX Data registers */ >>>>> +# define SPI_TX_DATA_REG 0x002 >>>>> +# define SPI_TX_DATA_SIZE 0x21e >>>>> + >>>>> +/* SPI RX Data registers */ >>>>> +# define SPI_RX_DATA_REG 0x400 >>>>> +# define SPI_RX_DATA_SIZE 0x220 >>>>> + >>>>> +/* SPI Command register */ >>>>> +# define SPI_CMD_REG 0x700 >>>>> + >>>>> +/* SPI Interrupt registers */ >>>>> +# define SPI_IR_STAT_REG 0x702 >>>>> +# define SPI_IR_MASK_REG 0x704 >>>>> + >>>>> +/* SPI Clock register */ >>>>> +# define SPI_CLK_REG 0x706 >>>>> + >>>>> +/* SPI Fill register */ >>>>> +# define SPI_FILL_REG 0x707 >>>>> + >>>>> +#endif >>>>> + >>>>> +/* SPI Command register */ >>>>> +#define SPI_CMD_OP_SHIFT 0 >>>>> +#define SPI_CMD_OP_START (0x3 << SPI_CMD_OP_SHIFT) >>>>> +#define SPI_CMD_SLAVE_SHIFT 4 >>>>> +#define SPI_CMD_SLAVE_MASK (0xf << SPI_CMD_SLAVE_SHIFT) >>>>> +#define SPI_CMD_PREPEND_SHIFT 8 >>>>> +#define SPI_CMD_PREPEND_BYTES 0xf >>>>> +#define SPI_CMD_3WIRE_SHIFT 12 >>>>> +#define SPI_CMD_3WIRE_MASK (1 << SPI_CMD_3WIRE_SHIFT) >>>>> + >>>>> +/* SPI Interrupt registers */ >>>>> +#define SPI_IR_DONE_SHIFT 0 >>>>> +#define SPI_IR_DONE_MASK (1 << SPI_IR_DONE_SHIFT) >>>>> +#define SPI_IR_RXOVER_SHIFT 1 >>>>> +#define SPI_IR_RXOVER_MASK (1 << SPI_IR_RXOVER_SHIFT) >>>>> +#define SPI_IR_TXUNDER_SHIFT 2 >>>>> +#define SPI_IR_TXUNDER_MASK (1 << SPI_IR_TXUNDER_SHIFT) >>>>> +#define SPI_IR_TXOVER_SHIFT 3 >>>>> +#define SPI_IR_TXOVER_MASK (1 << SPI_IR_TXOVER_SHIFT) >>>>> +#define SPI_IR_RXUNDER_SHIFT 4 >>>>> +#define SPI_IR_RXUNDER_MASK (1 << SPI_IR_RXUNDER_SHIFT) >>>>> +#define SPI_IR_CLEAR_MASK (SPI_IR_DONE_MASK |\ >>>>> + SPI_IR_RXOVER_MASK |\ >>>>> + SPI_IR_TXUNDER_MASK |\ >>>>> + SPI_IR_TXOVER_MASK |\ >>>>> + SPI_IR_RXUNDER_MASK) >>>>> + >>>>> +/* SPI Clock register */ >>>>> +#define SPI_CLK_SHIFT 0 >>>>> +#define SPI_CLK_20MHZ (0 << SPI_CLK_SHIFT) >>>>> +#define SPI_CLK_0_391MHZ (1 << SPI_CLK_SHIFT) >>>>> +#define SPI_CLK_0_781MHZ (2 << SPI_CLK_SHIFT) >>>>> +#define SPI_CLK_1_563MHZ (3 << SPI_CLK_SHIFT) >>>>> +#define SPI_CLK_3_125MHZ (4 << SPI_CLK_SHIFT) >>>>> +#define SPI_CLK_6_250MHZ (5 << SPI_CLK_SHIFT) >>>>> +#define SPI_CLK_12_50MHZ (6 << SPI_CLK_SHIFT) >>>>> +#define SPI_CLK_25MHZ (7 << SPI_CLK_SHIFT) >>>>> +#define SPI_CLK_MASK (7 << SPI_CLK_SHIFT) >>>>> +#define SPI_CLK_SSOFF_SHIFT 3 >>>>> +#define SPI_CLK_SSOFF_2 (2 << SPI_CLK_SSOFF_SHIFT) >>>>> +#define SPI_CLK_SSOFF_MASK (7 << SPI_CLK_SSOFF_SHIFT) >>>>> +#define SPI_CLK_BSWAP_SHIFT 7 >>>>> +#define SPI_CLK_BSWAP_MASK (1 << SPI_CLK_BSWAP_SHIFT) >>>>> + >>>>> +#define SPI_CLK_CNT 8 >>>>> +static const unsigned bcm63xx_spi_freq_table[SPI_CLK_CNT][2] = { >>>>> + { 25000000, SPI_CLK_25MHZ }, >>>>> + { 20000000, SPI_CLK_20MHZ }, >>>>> + { 12500000, SPI_CLK_12_50MHZ }, >>>>> + { 6250000, SPI_CLK_6_250MHZ }, >>>>> + { 3125000, SPI_CLK_3_125MHZ }, >>>>> + { 1563000, SPI_CLK_1_563MHZ }, >>>>> + { 781000, SPI_CLK_0_781MHZ }, >>>>> + { 391000, SPI_CLK_0_391MHZ } >>>>> +}; >>>>> + >>>>> +struct bcm63xx_spi_priv { >>>>> + void __iomem *regs; >>>>> + uint8_t num_cs; >>>>> + size_t tx_bytes; >>>>> +}; >>>>> + >>>>> +static int bcm63xx_spi_cs_info(struct udevice *bus, uint cs, >>>>> + struct spi_cs_info *info) >>>>> +{ >>>>> + struct bcm63xx_spi_priv *priv = dev_get_priv(bus); >>>>> + >>>>> + if (cs >= priv->num_cs) { >>>>> + error("no cs %u\n", cs); >>>>> + return -ENODEV; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int bcm63xx_spi_set_mode(struct udevice *bus, uint mode) >>>>> +{ >>>>> + struct bcm63xx_spi_priv *priv = dev_get_priv(bus); >>>>> + >>>>> + if (mode & SPI_LSB_FIRST) >>>>> + setbits_8(priv->regs + SPI_CLK_REG, SPI_CLK_BSWAP_MASK); >>>>> + else >>>>> + clrbits_8(priv->regs + SPI_CLK_REG, SPI_CLK_BSWAP_MASK); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int bcm63xx_spi_set_speed(struct udevice *bus, uint speed) >>>>> +{ >>>>> + struct bcm63xx_spi_priv *priv = dev_get_priv(bus); >>>>> + uint8_t clk_cfg; >>>>> + int i; >>>>> + >>>>> + /* default to lowest clock configuration */ >>>>> + clk_cfg = SPI_CLK_0_391MHZ; >>>>> + >>>>> + /* find the closest clock configuration */ >>>>> + for (i = 0; i < SPI_CLK_CNT; i++) { >>>>> + if (speed >= bcm63xx_spi_freq_table[i][0]) { >>>>> + clk_cfg = bcm63xx_spi_freq_table[i][1]; >>>>> + break; >>>>> + } >>>>> + } >>>>> + >>>>> + /* write clock configuration */ >>>>> + clrsetbits_8(priv->regs + SPI_CLK_REG, >>>>> + SPI_CLK_SSOFF_MASK | SPI_CLK_MASK, >>>>> + clk_cfg | SPI_CLK_SSOFF_2); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +/* >>>>> + * BCM63xx SPI driver doesn't allow keeping CS active between transfers since >>>>> + * they are HW controlled. >>>>> + * However, it provides a mechanism to prepend write transfers prior to read >>>>> + * transfers (with a maximum prepend of 15 bytes), which is usually enough for >>>>> + * SPI-connected flashes since reading requires prepending a write transfer of >>>>> + * 5 bytes. >>>>> + * >>>>> + * This implementation takes advantage of the prepend mechanism and combines >>>>> + * multiple transfers into a single one where possible (single/multiple write >>>>> + * transfer(s) followed by a final read/write transfer). >>>>> + * However, it's not possible to buffer reads, which means that read transfers >>>>> + * should always be done as the final ones. >>>>> + * On the other hand, take into account that combining write transfers into >>>>> + * a single one is just buffering and doesn't require prepend mechanism. >>>>> + */ >>>>> +static int bcm63xx_spi_xfer(struct udevice *dev, unsigned int bitlen, >>>>> + const void *dout, void *din, unsigned long flags) >>>>> +{ >>>>> + struct bcm63xx_spi_priv *priv = dev_get_priv(dev->parent); >>>>> + size_t data_bytes = bitlen / 8; >>>>> + >>>>> + if (flags & SPI_XFER_BEGIN) { >>>>> + /* clear prepends */ >>>>> + priv->tx_bytes = 0; >>>>> + >>>>> + /* initialize hardware */ >>>>> + writeb_be(0, priv->regs + SPI_IR_MASK_REG); >>>>> + } >>>>> + >>>>> + if (din) { >>>>> + /* buffering reads not possible since cs is hw controlled */ >>>>> + if (!(flags & SPI_XFER_END)) { >>>>> + error("unable to buffer reads\n"); >>>>> + return -EINVAL; >>>>> + } >>>>> + >>>>> + /* check rx size */ >>>>> + if (data_bytes > SPI_RX_DATA_SIZE) { >>>>> + error("max rx bytes exceeded\n"); >>>>> + return -EMSGSIZE; >>>>> + } >>>>> + } >>>>> + >>>>> + if (dout) { >>>>> + /* check tx size */ >>>>> + if (priv->tx_bytes + data_bytes > SPI_TX_DATA_SIZE) { >>>>> + error("max tx bytes exceeded\n"); >>>>> + return -EMSGSIZE; >>>>> + } >>>>> + >>>>> + /* copy tx data */ >>>>> + memcpy_toio(priv->regs + SPI_TX_DATA_REG + priv->tx_bytes, >>>>> + dout, data_bytes); >>>>> + priv->tx_bytes += data_bytes; >>>>> + } >>>>> + >>>>> + if (flags & SPI_XFER_END) { >>>>> + struct dm_spi_slave_platdata *plat = >>>>> + dev_get_parent_platdata(dev); >>>>> + uint16_t val = 0; >>>>> + uint8_t irq; >>>>> + >>>>> + /* determine control config */ >>>>> + if (dout && !din) { >>>>> + /* buffered write transfers */ >>>>> + val |= (priv->tx_bytes << SPI_CTL_BYTES_SHIFT); >>>>> + val |= SPI_CTL_TYPE_HD_W; >>>>> + priv->tx_bytes = 0; >>>>> + } else { >>>>> + if (dout && din && (flags & SPI_XFER_ONCE)) { >>>>> + /* full duplex read/write */ >>>>> + val |= (data_bytes << SPI_CTL_BYTES_SHIFT); >>>>> + val |= SPI_CTL_TYPE_FD_RW; >>>>> + priv->tx_bytes = 0; >>>>> + } else { >>>>> + /* prepended write transfer */ >>>>> + val |= (data_bytes << SPI_CTL_BYTES_SHIFT); >>>>> + val |= SPI_CTL_TYPE_HD_R; >>>>> + if (priv->tx_bytes > SPI_CMD_PREPEND_BYTES) { >>>>> + error("max prepend bytes exceeded\n"); >>>>> + return -EMSGSIZE; >>>>> + } >>>>> + } >>>>> + } >>>>> + bcm63xx_spi_wctl(val, priv->regs + SPI_CTL_REG); >>>>> + >>>>> + /* clear interrupts */ >>>>> + writeb_be(SPI_IR_CLEAR_MASK, priv->regs + SPI_IR_STAT_REG); >>>>> + >>>>> + /* issue the transfer */ >>>>> + val = SPI_CMD_OP_START; >>>>> + val |= (plat->cs << SPI_CMD_SLAVE_SHIFT) & SPI_CMD_SLAVE_MASK; >>>>> + val |= (priv->tx_bytes << SPI_CMD_PREPEND_SHIFT); >>>>> + if (plat->mode & SPI_3WIRE) >>>>> + val |= SPI_CMD_3WIRE_MASK; >>>>> + writew_be(val, priv->regs + SPI_CMD_REG); >>>>> + >>>>> + /* enable interrupts */ >>>>> + writeb_be(SPI_IR_DONE_MASK, priv->regs + SPI_IR_MASK_REG); >>>>> + >>>>> + do { >>>>> + /* read interupts */ >>>>> + irq = readb_be(priv->regs + SPI_IR_STAT_REG); >>>>> + >>>>> + /* transfer completed */ >>>>> + if (irq & SPI_IR_DONE_MASK) >>>>> + break; >>>>> + } while (1); >>>>> + >>>>> + /* copy rx data */ >>>>> + if (din) >>>>> + memcpy_fromio(din, priv->regs + SPI_RX_DATA_REG, >>>>> + data_bytes); >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static const struct dm_spi_ops bcm63xx_spi_ops = { >>>>> + .cs_info = bcm63xx_spi_cs_info, >>>>> + .set_mode = bcm63xx_spi_set_mode, >>>>> + .set_speed = bcm63xx_spi_set_speed, >>>>> + .xfer = bcm63xx_spi_xfer, >>>>> +}; >>>>> + >>>>> +static const struct udevice_id bcm63xx_spi_ids[] = { >>>>> + { .compatible = SPI_DT_ID, }, >>>>> + { /* sentinel */ } >>>> >>>> Try to add .data with reg_space of respective controller, this will >>>> eventually reduce to driver configs and make it CONFIG_BCM63XX and >>>> reduce unneeded#ifdef. >>> This implies adding considerable bloat to the driver... >>> Is it imperative for the driver to be accepted? >> >> Yes, as per as recent U-boot developement we're trying to reduce >> defconfig code as possible and even this method can improve code >> quality. > I don't think that method applies here, since u-boot is usally built for each SoC and supporting two cores in the same build makes no sense to me. > These should be done in two different drivers but I merged them into a single one to improve maintainability. I don't like the approch of adding ifdef with two controller reg_space, we have .data that has been used in many dm-driver, let's have a try. > BTW, two u-boot developers already reviewed this patch and didn't complain about it... Wrong statement, it is community project every-body comment need to be considrable. thanks! -- Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India.