[U-Boot] [PATCH v4 05/10] SPI: Add Dove SPI driver

[Jagan Teki]

Hi,

Looks ok to me as per coding style after a quick look.

On Mon, May 27, 2013 at 12:06 AM, Sascha Silbe <t-uboot at infra-silbe.de> wrote:
> From: Sebastian Hesselbarth <sebastian.hesselbarth at gmail.com>
>
> This adds an SPI driver for Marvell Dove SoCs. This driver is taken
> from kirkwood_spi but removes mpp configuration as dove has dedicated
> spi pins.
>
> As a future clean-up step, the code for orion5x, kirkwood and dove
> could be merged, with MPP configuration being be handled as part of
> cpu/board-specific setup.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth at gmail.com>
> Signed-off-by: Sascha Silbe <t-uboot at infra-silbe.de>
> ---
>  v3->v4: renamed to dove, adjusted description, removed unused
>          variable, made checkpatch clean
>
>  drivers/spi/Makefile   |   1 +
>  drivers/spi/dove_spi.c | 212 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 213 insertions(+)
>  create mode 100644 drivers/spi/dove_spi.c
>
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index d08609e..62ad970 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -38,6 +38,7 @@ COBJS-$(CONFIG_BFIN_SPI6XX) += bfin_spi6xx.o
>  COBJS-$(CONFIG_CF_SPI) += cf_spi.o
>  COBJS-$(CONFIG_CF_QSPI) += cf_qspi.o
>  COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
> +COBJS-$(CONFIG_DOVE_SPI) += dove_spi.o
>  COBJS-$(CONFIG_EXYNOS_SPI) += exynos_spi.o
>  COBJS-$(CONFIG_ICH_SPI) +=  ich.o
>  COBJS-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
> diff --git a/drivers/spi/dove_spi.c b/drivers/spi/dove_spi.c
> new file mode 100644
> index 0000000..c61ba89
> --- /dev/null
> +++ b/drivers/spi/dove_spi.c
> @@ -0,0 +1,212 @@
> +/*
> + * Marvell Dove SoCs common spi driver
> + *
> + * Sebastian Hesselbarth <sebastian.hesselbarth at gmail.com>
> + * based on kirkwood_spi.c written by
> + *  Prafulla Wadaskar <prafulla at marvell.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#include <common.h>
> +#include <malloc.h>
> +#include <spi.h>
> +#include <asm/io.h>
> +#include <asm/arch/config.h>
> +
> +/* SPI Registers on dove SOC */
> +struct dovespi_registers {
> +       u32 ctrl;       /* 0x00 */
> +       u32 cfg;        /* 0x04 */
> +       u32 dout;       /* 0x08 */
> +       u32 din;        /* 0x0c */
> +       u32 irq_cause;  /* 0x10 */
> +       u32 irq_mask;   /* 0x14 */
> +};
> +
> +#define DOVESPI_CLKPRESCL_MASK 0x1f
> +#define DOVESPI_CLKPRESCL_MIN  0x12
> +#define DOVESPI_CSN_ACT        1 /* Activates serial memory interface */
> +#define DOVESPI_SMEMRDY        (1 << 1) /* SerMem Data xfer ready */
> +#define DOVESPI_IRQUNMASK      1 /* unmask SPI interrupt */
> +#define DOVESPI_IRQMASK        0 /* mask SPI interrupt */
> +#define DOVESPI_SMEMRDIRQ      1 /* SerMem data xfer ready irq */
> +#define DOVESPI_XFERLEN_1BYTE  0
> +#define DOVESPI_XFERLEN_2BYTE  (1 << 5)
> +#define DOVESPI_XFERLEN_MASK   (1 << 5)
> +#define DOVESPI_ADRLEN_1BYTE   0
> +#define DOVESPI_ADRLEN_2BYTE   (1 << 8)
> +#define DOVESPI_ADRLEN_3BYTE   (2 << 8)
> +#define DOVESPI_ADRLEN_4BYTE   (3 << 8)
> +#define DOVESPI_ADRLEN_MASK    (3 << 8)
> +#define DOVESPI_TIMEOUT        10000
> +
> +static struct dovespi_registers *spireg =
> +       (struct dovespi_registers *)DOVE_SPI_BASE;
> +
> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> +                               unsigned int max_hz, unsigned int mode)
> +{
> +       struct spi_slave *slave;
> +       u32 data;
> +
> +       if (!spi_cs_is_valid(bus, cs))
> +               return NULL;
> +

Done use the below tag code instead go for spi_alloc_slave()
see the sample code on "drivers/spi/exynos_spi.c"

----------------------- TAG+
> +       slave = malloc(sizeof(struct spi_slave));
> +       if (!slave)
> +               return NULL;
> +
> +       slave->bus = bus;
> +       slave->cs = cs;
> +
--------------------- TAG-

> +       writel(~DOVESPI_CSN_ACT | DOVESPI_SMEMRDY, &spireg->ctrl);
> +
> +       /* calculate spi clock prescaller using max_hz */
> +       data = ((CONFIG_SYS_TCLK / 2) / max_hz) + 0x10;
> +       data = data < DOVESPI_CLKPRESCL_MIN ? DOVESPI_CLKPRESCL_MIN : data;
> +       data = data > DOVESPI_CLKPRESCL_MASK ? DOVESPI_CLKPRESCL_MASK : data;
> +
> +       /* program spi clock prescaller using max_hz */
> +       writel(DOVESPI_ADRLEN_3BYTE | data, &spireg->cfg);
> +       debug("data = 0x%08x\n", data);
> +
> +       writel(DOVESPI_SMEMRDIRQ, &spireg->irq_cause);
> +       writel(DOVESPI_IRQMASK, &spireg->irq_mask);
> +
> +       return slave;
> +}
> +
> +void spi_free_slave(struct spi_slave *slave)
> +{
> +       free(slave);
> +}
> +
> +__attribute__((weak)) int board_spi_claim_bus(struct spi_slave *slave)

Why your using __attribute__((weak)) here, may be use to pre-load the
symbol library
but what is the use case here?

--
Thanks,
Jagan.