[U-Boot] [PATCH 03/10] dm: spi: add BCM63xx SPI driver
Jagan Teki
jagannadh.teki at gmail.com
Mon Jun 12 06:02:39 UTC 2017
On Thu, Jun 8, 2017 at 12:05 AM, Álvaro Fernández Rojas
<noltari at gmail.com> 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
>> <noltari at gmail.com> 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
>>>> <noltari at gmail.com> 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 <noltari at gmail.com>
>>>>> ---
>>>>> 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 <noltari at gmail.com>
>>>>> + *
>>>>> + * Derived from linux/drivers/spi/spi-bcm63xx.c:
>>>>> + * Copyright (C) 2009-2012 Florian Fainelli <florian at openwrt.org>
>>>>> + * Copyright (C) 2010 Tanguy Bouzeloc <tanguy.bouzeloc at efixo.com>
>>>>> + *
>>>>> + * SPDX-License-Identifier: GPL-2.0+
>>>>> + */
>>>>> +
>>>>> +#include <common.h>
>>>>> +#include <clk.h>
>>>>> +#include <dm.h>
>>>>> +#include <spi.h>
>>>>> +#include <reset.h>
>>>>> +#include <asm/io.h>
>>>>> +
>>>>> +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.
More information about the U-Boot
mailing list